From 366f045c11949fb866b65618b13cf04983370cb5 Mon Sep 17 00:00:00 2001 From: Mark Johnson <739719+virgofx@users.noreply.github.com> Date: Tue, 10 Jun 2025 18:18:47 +0000 Subject: [PATCH 1/2] refactor: remove GITHUB_ACTIONS_BOT_USER_ID constant and update related logic; add getGitHubActionsBotEmail function for dynamic email retrieval --- __tests__/pull-request.test.ts | 17 +++-------------- __tests__/utils/constants.test.ts | 8 ++++---- __tests__/utils/github.test.ts | 29 +++++++++++++++++++++++++++++ src/main.ts | 2 +- src/pull-request.ts | 3 +-- src/releases.ts | 6 ++++-- src/utils/constants.ts | 3 +-- src/utils/github.ts | 19 +++++++++++++++++++ src/wiki.ts | 10 ++++++---- 9 files changed, 68 insertions(+), 29 deletions(-) create mode 100644 __tests__/utils/github.test.ts create mode 100644 src/utils/github.ts diff --git a/__tests__/pull-request.test.ts b/__tests__/pull-request.test.ts index d2bca33..20cf77b 100644 --- a/__tests__/pull-request.test.ts +++ b/__tests__/pull-request.test.ts @@ -7,7 +7,6 @@ import { createMockTerraformModule } from '@/tests/helpers/terraform-module'; import type { GitHubRelease } from '@/types'; import { BRANDING_COMMENT, - GITHUB_ACTIONS_BOT_USER_ID, PR_RELEASE_MARKER, PR_SUMMARY_MARKER, WIKI_STATUS, @@ -55,27 +54,17 @@ describe('pull-request', () => { context.useMockOctokit(); }); - it('should return false when release marker is found in comments from non github-actions user', async () => { - stubOctokitReturnData('issues.listComments', { - data: [ - { user: { id: 123 }, body: 'Some comment' }, - { user: { id: 123 }, body: PR_RELEASE_MARKER }, - { user: { id: 123 }, body: 'Another comment' }, - ], - }); - expect(await hasReleaseComment()).toBe(false); - }); - it('should return true when release marker is found in comments from github-actions user', async () => { + it('should return true when release marker is found in comments', async () => { stubOctokitReturnData('issues.listComments', { data: [ { user: { id: 123 }, body: 'Some comment' }, - { user: { id: GITHUB_ACTIONS_BOT_USER_ID }, body: PR_RELEASE_MARKER }, + { user: { id: 123 }, body: PR_RELEASE_MARKER }, { user: { id: 123 }, body: 'Another comment' }, ], }); expect(await hasReleaseComment()).toBe(true); - }); + }) it('should return false when no release marker is found', async () => { stubOctokitReturnData('issues.listComments', { diff --git a/__tests__/utils/constants.test.ts b/__tests__/utils/constants.test.ts index dcaab89..af78271 100644 --- a/__tests__/utils/constants.test.ts +++ b/__tests__/utils/constants.test.ts @@ -1,8 +1,8 @@ import { BRANDING_COMMENT, BRANDING_WIKI, - GITHUB_ACTIONS_BOT_EMAIL, GITHUB_ACTIONS_BOT_NAME, + GITHUB_ACTIONS_BOT_USERNAME, PROJECT_URL, PR_RELEASE_MARKER, PR_SUMMARY_MARKER, @@ -10,13 +10,13 @@ import { } from '@/utils/constants'; import { describe, expect, it } from 'vitest'; -describe('constants', () => { +describe('utils/constants', () => { it('should have the correct GitHub Actions bot name', () => { expect(GITHUB_ACTIONS_BOT_NAME).toBe('GitHub Actions'); }); - it('should have the correct GitHub Actions bot email', () => { - expect(GITHUB_ACTIONS_BOT_EMAIL).toBe('41898282+github-actions[bot]@users.noreply.github.com'); + it('should have the correct GitHub Actions bot username', () => { + expect(GITHUB_ACTIONS_BOT_USERNAME).toBe('github-actions[bot]'); }); it('should have the correct PR summary marker', () => { diff --git a/__tests__/utils/github.test.ts b/__tests__/utils/github.test.ts new file mode 100644 index 0000000..8c7b0c8 --- /dev/null +++ b/__tests__/utils/github.test.ts @@ -0,0 +1,29 @@ +import { context } from '@/mocks/context'; +import { getGitHubActionsBotEmail } from '@/utils/github'; +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; + +describe('utils/github', () => { + describe('getGitHubActionsBotEmail - real API queries', () => { + beforeAll(async () => { + if (!process.env.GITHUB_TOKEN) { + throw new Error('GITHUB_TOKEN environment variable must be set for these tests'); + } + await context.useRealOctokit(); + }); + + afterAll(() => { + context.useMockOctokit(); + }); + + it('should return the correct email format for GitHub.com public API', async () => { + // This test uses the real GitHub API and expects the standard GitHub.com user ID + // for the github-actions[bot] user, which is 41898282 + + // Act + const result = await getGitHubActionsBotEmail(); + + // Assert + expect(result).toBe('41898282+github-actions[bot]@users.noreply.github.com'); + }); + }); +}); diff --git a/src/main.ts b/src/main.ts index 7cb7b82..b493e2d 100644 --- a/src/main.ts +++ b/src/main.ts @@ -78,7 +78,7 @@ async function handlePullRequestMergedEvent( ensureTerraformDocsConfigDoesNotExist(); checkoutWiki(); await generateWikiFiles(terraformModules); - commitAndPushWikiChanges(); + await commitAndPushWikiChanges(); } } diff --git a/src/pull-request.ts b/src/pull-request.ts index 4382869..f526a5c 100644 --- a/src/pull-request.ts +++ b/src/pull-request.ts @@ -5,7 +5,6 @@ import { TerraformModule } from '@/terraform-module'; import type { CommitDetails, GitHubRelease, WikiStatusResult } from '@/types'; import { BRANDING_COMMENT, - GITHUB_ACTIONS_BOT_USER_ID, PROJECT_URL, PR_RELEASE_MARKER, PR_SUMMARY_MARKER, @@ -38,7 +37,7 @@ export async function hasReleaseComment(): Promise { for await (const { data } of iterator) { for (const comment of data) { - if (comment.user?.id === GITHUB_ACTIONS_BOT_USER_ID && comment.body?.includes(PR_RELEASE_MARKER)) { + if (comment.body?.includes(PR_RELEASE_MARKER)) { return true; } } diff --git a/src/releases.ts b/src/releases.ts index 418f7a6..fd8c6ab 100644 --- a/src/releases.ts +++ b/src/releases.ts @@ -7,8 +7,9 @@ import { config } from '@/config'; import { context } from '@/context'; import { TerraformModule } from '@/terraform-module'; import type { GitHubRelease } from '@/types'; -import { GITHUB_ACTIONS_BOT_EMAIL, GITHUB_ACTIONS_BOT_NAME } from '@/utils/constants'; +import { GITHUB_ACTIONS_BOT_NAME } from '@/utils/constants'; import { copyModuleContents } from '@/utils/file'; +import { getGitHubActionsBotEmail } from '@/utils/github'; import { debug, endGroup, info, startGroup } from '@actions/core'; import type { RestEndpointMethodTypes } from '@octokit/plugin-rest-endpoint-methods'; import { RequestError } from '@octokit/request-error'; @@ -139,13 +140,14 @@ export async function createTaggedReleases(terraformModules: TerraformModule[]): // Git operations: commit the changes and tag the release const commitMessage = `${module.getReleaseTag()}\n\n${prTitle}\n\n${prBody}`.trim(); const gitPath = await which('git'); + const githubActionsBotEmail = await getGitHubActionsBotEmail(); // Execute git commands in temp directory without inheriting stdio to avoid output pollution const gitOpts: ExecSyncOptions = { cwd: tmpDir }; for (const cmd of [ ['config', '--local', 'user.name', GITHUB_ACTIONS_BOT_NAME], - ['config', '--local', 'user.email', GITHUB_ACTIONS_BOT_EMAIL], + ['config', '--local', 'user.email', githubActionsBotEmail], ['add', '.'], ['commit', '-m', commitMessage.trim()], ['tag', releaseTag], diff --git a/src/utils/constants.ts b/src/utils/constants.ts index 5235299..922f45d 100644 --- a/src/utils/constants.ts +++ b/src/utils/constants.ts @@ -46,9 +46,8 @@ export const WIKI_HOME_FILENAME = 'Home.md'; export const WIKI_SIDEBAR_FILENAME = '_Sidebar.md'; export const WIKI_FOOTER_FILENAME = '_Footer.md'; -export const GITHUB_ACTIONS_BOT_USER_ID = 41898282; export const GITHUB_ACTIONS_BOT_NAME = 'GitHub Actions'; -export const GITHUB_ACTIONS_BOT_EMAIL = '41898282+github-actions[bot]@users.noreply.github.com'; +export const GITHUB_ACTIONS_BOT_USERNAME = 'github-actions[bot]'; export const PR_SUMMARY_MARKER = ''; export const PR_RELEASE_MARKER = ''; diff --git a/src/utils/github.ts b/src/utils/github.ts new file mode 100644 index 0000000..25132d2 --- /dev/null +++ b/src/utils/github.ts @@ -0,0 +1,19 @@ +import { context } from '@/context'; +import { GITHUB_ACTIONS_BOT_USERNAME } from '@/utils/constants'; + +/** + * Retrieves the GitHub Actions bot email address dynamically by querying the GitHub API. + * This function handles both GitHub.com and GitHub Enterprise Server environments. + * + * The email format follows GitHub's standard: {user_id}+{username}@users.noreply.github.com + * + * @returns {Promise} The GitHub Actions bot email address + * @throws {Error} If the API request fails or the user information cannot be retrieved + */ +export async function getGitHubActionsBotEmail(): Promise { + const response = await context.octokit.rest.users.getByUsername({ + username: GITHUB_ACTIONS_BOT_USERNAME, + }); + + return `${response.data.id}+${GITHUB_ACTIONS_BOT_USERNAME}@users.noreply.github.com`; +} diff --git a/src/wiki.ts b/src/wiki.ts index daba1df..36d6fec 100644 --- a/src/wiki.ts +++ b/src/wiki.ts @@ -12,7 +12,6 @@ import type { TerraformModule } from '@/terraform-module'; import type { ExecSyncError, WikiStatusResult } from '@/types'; import { BRANDING_WIKI, - GITHUB_ACTIONS_BOT_EMAIL, GITHUB_ACTIONS_BOT_NAME, PROJECT_URL, WIKI_FOOTER_FILENAME, @@ -22,6 +21,7 @@ import { WIKI_TITLE_REPLACEMENTS, } from '@/utils/constants'; import { removeDirectoryContents } from '@/utils/file'; +import { getGitHubActionsBotEmail } from '@/utils/github'; import { endGroup, info, startGroup } from '@actions/core'; import pLimit from 'p-limit'; import which from 'which'; @@ -558,9 +558,9 @@ export async function generateWikiFiles(terraformModules: TerraformModule[]): Pr * pushes them to the remote wiki repository. If no changes are detected, it logs a * message and exits gracefully without creating a commit. * - * @returns {void} + * @returns {Promise} */ -export function commitAndPushWikiChanges(): void { +export async function commitAndPushWikiChanges(): Promise { startGroup('Committing and pushing changes to wiki'); try { @@ -583,9 +583,11 @@ export function commitAndPushWikiChanges(): void { if (status !== null && status.toString().trim() !== '') { // There are changes, commit and push + const botEmail = await getGitHubActionsBotEmail(); + for (const cmd of [ ['config', '--local', 'user.name', GITHUB_ACTIONS_BOT_NAME], - ['config', '--local', 'user.email', GITHUB_ACTIONS_BOT_EMAIL], + ['config', '--local', 'user.email', botEmail], ['add', '.'], ['commit', '-m', commitMessage.trim()], ['push', 'origin'], From f185a20059d43c58b808c92ad4c1e69ba4667a34 Mon Sep 17 00:00:00 2001 From: Mark Johnson <739719+virgofx@users.noreply.github.com> Date: Tue, 10 Jun 2025 19:02:28 +0000 Subject: [PATCH 2/2] refactor: enhance GitHub API integration by adding user retrieval and updating related logic in tests --- __tests__/helpers/octokit.ts | 18 ++++++++++++++++++ __tests__/pull-request.test.ts | 10 ++-------- __tests__/utils/github.test.ts | 9 ++------- __tests__/wiki.test.ts | 22 ++++++++++++++-------- src/pull-request.ts | 8 +------- src/utils/github.ts | 14 +++++++------- src/wiki.ts | 6 +++--- 7 files changed, 47 insertions(+), 40 deletions(-) diff --git a/__tests__/helpers/octokit.ts b/__tests__/helpers/octokit.ts index 46e5676..36ddc83 100644 --- a/__tests__/helpers/octokit.ts +++ b/__tests__/helpers/octokit.ts @@ -19,6 +19,7 @@ type EndpointNames = { issues: 'createComment' | 'deleteComment' | 'listComments'; pulls: 'listCommits' | 'listFiles'; repos: 'getCommit' | 'listTags' | 'listReleases' | 'createRelease' | 'deleteRelease'; + users: 'getByUsername'; }; // Type guard that ensures the method name is valid for the given namespace K. @@ -164,12 +165,26 @@ export function resetMockStore() { headers: {}, }, }, + users: { + getByUsername: { + data: { + login: 'github-actions[bot]', + id: 41898282, + type: 'Bot', + site_admin: false, + }, + status: 200, + url: 'https://api.github.com/users/github-actions[bot]', + headers: {}, + }, + }, }, implementations: { git: {}, issues: {}, pulls: {}, repos: {}, + users: {}, }, }; } @@ -302,6 +317,9 @@ export function createDefaultOctokitMock(): OctokitRestApi { createRelease: vi.fn().mockImplementation(() => getMockResponse('repos.createRelease')), deleteRelease: vi.fn().mockImplementation(() => getMockResponse('repos.deleteRelease')), }, + users: { + getByUsername: vi.fn().mockImplementation((params) => getMockResponse('users.getByUsername', params)), + }, }, paginate: { iterator: ( diff --git a/__tests__/pull-request.test.ts b/__tests__/pull-request.test.ts index 20cf77b..cd0e851 100644 --- a/__tests__/pull-request.test.ts +++ b/__tests__/pull-request.test.ts @@ -5,12 +5,7 @@ import type { TerraformModule } from '@/terraform-module'; import { stubOctokitImplementation, stubOctokitReturnData } from '@/tests/helpers/octokit'; import { createMockTerraformModule } from '@/tests/helpers/terraform-module'; import type { GitHubRelease } from '@/types'; -import { - BRANDING_COMMENT, - PR_RELEASE_MARKER, - PR_SUMMARY_MARKER, - WIKI_STATUS, -} from '@/utils/constants'; +import { BRANDING_COMMENT, PR_RELEASE_MARKER, PR_SUMMARY_MARKER, WIKI_STATUS } from '@/utils/constants'; import { debug, endGroup, info, startGroup } from '@actions/core'; import { RequestError } from '@octokit/request-error'; import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'; @@ -54,7 +49,6 @@ describe('pull-request', () => { context.useMockOctokit(); }); - it('should return true when release marker is found in comments', async () => { stubOctokitReturnData('issues.listComments', { data: [ @@ -64,7 +58,7 @@ describe('pull-request', () => { ], }); expect(await hasReleaseComment()).toBe(true); - }) + }); it('should return false when no release marker is found', async () => { stubOctokitReturnData('issues.listComments', { diff --git a/__tests__/utils/github.test.ts b/__tests__/utils/github.test.ts index 8c7b0c8..cb8213e 100644 --- a/__tests__/utils/github.test.ts +++ b/__tests__/utils/github.test.ts @@ -1,6 +1,6 @@ import { context } from '@/mocks/context'; import { getGitHubActionsBotEmail } from '@/utils/github'; -import { afterAll, beforeAll, describe, expect, it } from 'vitest'; +import { beforeAll, describe, expect, it } from 'vitest'; describe('utils/github', () => { describe('getGitHubActionsBotEmail - real API queries', () => { @@ -11,15 +11,10 @@ describe('utils/github', () => { await context.useRealOctokit(); }); - afterAll(() => { - context.useMockOctokit(); - }); - it('should return the correct email format for GitHub.com public API', async () => { // This test uses the real GitHub API and expects the standard GitHub.com user ID // for the github-actions[bot] user, which is 41898282 - - // Act + const result = await getGitHubActionsBotEmail(); // Assert diff --git a/__tests__/wiki.test.ts b/__tests__/wiki.test.ts index 3a0b2bb..5c2563d 100644 --- a/__tests__/wiki.test.ts +++ b/__tests__/wiki.test.ts @@ -205,6 +205,7 @@ describe('wiki', async () => { afterAll(() => { vi.mocked(execFileSync).mockImplementation(vi.fn()); + vi.resetAllMocks(); // Unclears the console.log }); it('should generate all required wiki files', async () => { @@ -259,11 +260,16 @@ describe('wiki', async () => { }); describe('commitAndPushWikiChanges()', () => { - it('should commit and push changes when changes are detected', () => { + beforeAll(() => { + // Ensure we're using the mock octokit, not real one + context.useMockOctokit(); + }); + + it('should commit and push changes when changes are detected', async () => { // Mock git status to indicate changes exist vi.mocked(execFileSync).mockImplementationOnce(() => Buffer.from('M _Sidebar.md\n')); - commitAndPushWikiChanges(); + await commitAndPushWikiChanges(); // Verify git commands were called in correct order const gitCalls = vi.mocked(execFileSync).mock.calls.map((call) => call?.[1]?.join(' ') || ''); @@ -284,11 +290,11 @@ describe('wiki', async () => { expect(endGroup).toHaveBeenCalled(); }); - it('should skip commit and push when no changes are detected', () => { + it('should skip commit and push when no changes are detected', async () => { // Mock git status to indicate no changes vi.mocked(execFileSync).mockImplementationOnce(() => Buffer.from('')); - commitAndPushWikiChanges(); + await commitAndPushWikiChanges(); // Verify only status check was called const gitCalls = vi.mocked(execFileSync).mock.calls.map((call) => call?.[1]?.join(' ') || ''); @@ -301,7 +307,7 @@ describe('wiki', async () => { expect(endGroup).toHaveBeenCalled(); }); - it('should handle git command failures gracefully', () => { + it('should handle git command failures gracefully', async () => { // Mock git status to indicate changes exist but make add command fail vi.mocked(execFileSync) .mockImplementationOnce(() => Buffer.from('M _Sidebar.md\n')) @@ -309,7 +315,7 @@ describe('wiki', async () => { throw new Error('Git command failed'); }); - expect(() => commitAndPushWikiChanges()).toThrow('Git command failed'); + await expect(commitAndPushWikiChanges()).rejects.toThrow('Git command failed'); expect(startGroup).toHaveBeenCalledWith('Committing and pushing changes to wiki'); expect(info).toHaveBeenCalledWith('Checking for changes in wiki repository'); @@ -317,7 +323,7 @@ describe('wiki', async () => { expect(endGroup).toHaveBeenCalled(); }); - it('should not use complete PR information in commit message', () => { + it('should not use complete PR information in commit message', async () => { // Set up PR context with multiline body context.set({ prBody: 'Line 1\nLine 2\nLine 3', @@ -328,7 +334,7 @@ describe('wiki', async () => { // Mock git status to indicate changes exist vi.mocked(execFileSync).mockImplementationOnce(() => Buffer.from('M _Sidebar.md\n')); - commitAndPushWikiChanges(); + await commitAndPushWikiChanges(); // Verify commit message format const commitCall = vi.mocked(execFileSync).mock.calls.find((call) => call?.[1]?.includes('commit')); diff --git a/src/pull-request.ts b/src/pull-request.ts index f526a5c..ad2f9a8 100644 --- a/src/pull-request.ts +++ b/src/pull-request.ts @@ -3,13 +3,7 @@ import { config } from '@/config'; import { context } from '@/context'; import { TerraformModule } from '@/terraform-module'; import type { CommitDetails, GitHubRelease, WikiStatusResult } from '@/types'; -import { - BRANDING_COMMENT, - PROJECT_URL, - PR_RELEASE_MARKER, - PR_SUMMARY_MARKER, - WIKI_STATUS, -} from '@/utils/constants'; +import { BRANDING_COMMENT, PROJECT_URL, PR_RELEASE_MARKER, PR_SUMMARY_MARKER, WIKI_STATUS } from '@/utils/constants'; import { getWikiLink } from '@/wiki'; import { debug, endGroup, info, startGroup } from '@actions/core'; diff --git a/src/utils/github.ts b/src/utils/github.ts index 25132d2..109b385 100644 --- a/src/utils/github.ts +++ b/src/utils/github.ts @@ -4,16 +4,16 @@ import { GITHUB_ACTIONS_BOT_USERNAME } from '@/utils/constants'; /** * Retrieves the GitHub Actions bot email address dynamically by querying the GitHub API. * This function handles both GitHub.com and GitHub Enterprise Server environments. - * + * * The email format follows GitHub's standard: {user_id}+{username}@users.noreply.github.com - * + * * @returns {Promise} The GitHub Actions bot email address * @throws {Error} If the API request fails or the user information cannot be retrieved */ export async function getGitHubActionsBotEmail(): Promise { - const response = await context.octokit.rest.users.getByUsername({ - username: GITHUB_ACTIONS_BOT_USERNAME, - }); - - return `${response.data.id}+${GITHUB_ACTIONS_BOT_USERNAME}@users.noreply.github.com`; + const response = await context.octokit.rest.users.getByUsername({ + username: GITHUB_ACTIONS_BOT_USERNAME, + }); + + return `${response.data.id}+${GITHUB_ACTIONS_BOT_USERNAME}@users.noreply.github.com`; } diff --git a/src/wiki.ts b/src/wiki.ts index 36d6fec..89b95c9 100644 --- a/src/wiki.ts +++ b/src/wiki.ts @@ -583,11 +583,11 @@ export async function commitAndPushWikiChanges(): Promise { if (status !== null && status.toString().trim() !== '') { // There are changes, commit and push - const botEmail = await getGitHubActionsBotEmail(); - + const githubActionsBotEmail = await getGitHubActionsBotEmail(); + for (const cmd of [ ['config', '--local', 'user.name', GITHUB_ACTIONS_BOT_NAME], - ['config', '--local', 'user.email', botEmail], + ['config', '--local', 'user.email', githubActionsBotEmail], ['add', '.'], ['commit', '-m', commitMessage.trim()], ['push', 'origin'],