From 3eb1a5869ed3e2797379314f97d40c99e16e5db7 Mon Sep 17 00:00:00 2001 From: LironEr Date: Fri, 9 Oct 2026 09:02:38 +0200 Subject: [PATCH 1/2] fix: skip unnecessary GitHub output --- .../__tests__/githubController.spec.ts | 61 +++++++++++++ .../src/controllers/githubController.ts | 6 ++ .../outputs/outputs/__tests__/github.spec.ts | 89 ++++++++++++++++++- .../lib/main/outputs/outputs/github.ts | 18 ++-- 4 files changed, 168 insertions(+), 6 deletions(-) create mode 100644 apps/service/src/controllers/__tests__/githubController.spec.ts diff --git a/apps/service/src/controllers/__tests__/githubController.spec.ts b/apps/service/src/controllers/__tests__/githubController.spec.ts new file mode 100644 index 0000000..44648e8 --- /dev/null +++ b/apps/service/src/controllers/__tests__/githubController.spec.ts @@ -0,0 +1,61 @@ +import { githubOutputController } from '../githubController'; +import { getProject } from '../../framework/mongo/projects'; +import { getCommitRecordWithBase } from '../../framework/mongo/commitRecords'; + +jest.mock('../../framework/mongo/projects'); +jest.mock('../../framework/mongo/commitRecords'); +jest.mock('../../framework/github'); +jest.mock('../utils/githubOutputs'); + +const createReply = () => { + const res = { send: jest.fn(), status: jest.fn(), log: { warn: jest.fn(), info: jest.fn() } }; + res.status.mockReturnValue(res); + + return res; +}; + +const createRequest = (output: Record) => + ({ + params: { projectId: 'project-id', commitRecordId: 'record-id' }, + body: { + git: { owner: 'owner', repo: 'repo', commitSha: 'sha' }, + auth: { runId: '1' }, + output, + }, + log: { warn: jest.fn(), info: jest.fn(), error: jest.fn() }, + }) as any; + +describe('githubOutputController', () => { + beforeEach(() => { + jest.resetAllMocks(); + }); + + test('no output flag is true -> return {} without touching DB or GitHub', async () => { + const res = createReply(); + + await githubOutputController.call( + undefined as any, + createRequest({ checkRun: false, commitStatus: false, prComment: false }), + res as any + ); + + expect(res.send).toHaveBeenCalledWith({}); + expect(res.status).not.toHaveBeenCalled(); + expect(getProject).not.toHaveBeenCalled(); + expect(getCommitRecordWithBase).not.toHaveBeenCalled(); + }); + + test('at least one output flag is true -> continue processing', async () => { + const res = createReply(); + jest.mocked(getProject).mockResolvedValue(undefined); + + await githubOutputController.call( + undefined as any, + createRequest({ checkRun: false, commitStatus: true, prComment: false }), + res as any + ); + + expect(getProject).toHaveBeenCalledWith('project-id'); + expect(res.status).toHaveBeenCalledWith(404); + }); +}); diff --git a/apps/service/src/controllers/githubController.ts b/apps/service/src/controllers/githubController.ts index 6e6f914..427d05d 100644 --- a/apps/service/src/controllers/githubController.ts +++ b/apps/service/src/controllers/githubController.ts @@ -23,6 +23,12 @@ export const githubOutputController: FastifyValidatedRoute ({ serviceClient: { post: jest.fn() } })); +jest.mock('../../../utils/ci', () => ({ + owner: 'owner', + repo: 'repo', + getCIVars: () => ({ provider: 'github', buildId: 'run-id' }), +})); describe('github output', () => { beforeEach(() => { @@ -142,4 +157,76 @@ describe('github output', () => { expect(result).toEqual(true); }); }); + + describe('generate', () => { + const createConfig = (prNumber?: string) => + ({ remote: true, projectId: 'project-id', gitVars: { commitSha: 'sha', prNumber } }) as NormalizedConfig; + + const createReport = (status: Status, prNumber?: string): Report => ({ + files: [], + groups: [], + stats: {} as any, + metadata: { record: { id: 'record-id', prNumber } as any }, + status, + }); + + const generate = async (options: unknown, report: Report, prNumber?: string) => { + const instance = await githubOutput.create({ options, config: createConfig(prNumber) } as any); + + await instance!.generate(report); + }; + + beforeEach(() => { + jest.mocked(serviceClient.post).mockResolvedValue({ data: {} }); + }); + + test('skip request when nothing to post (on-failure with passing report)', async () => { + const options = { + checkRun: GithubOutputPostOption.OnFailure, + commitStatus: GithubOutputPostOption.OnFailure, + prComment: GithubOutputPostOption.OnFailure, + }; + + await generate(options, createReport(Status.Pass), '1'); + + expect(serviceClient.post).not.toHaveBeenCalled(); + }); + + test('skip request when nothing to post (pr-only on a push build)', async () => { + const options = { + checkRun: GithubOutputPostOption.PROnly, + commitStatus: GithubOutputPostOption.PROnly, + prComment: GithubOutputPostOption.PROnly, + }; + + await generate(options, createReport(Status.Pass)); + + expect(serviceClient.post).not.toHaveBeenCalled(); + }); + + test('skip request when only prComment is enabled on a non PR build', async () => { + await generate({ checkRun: false, commitStatus: false, prComment: true }, createReport(Status.Pass)); + + expect(serviceClient.post).not.toHaveBeenCalled(); + }); + + test('prComment is false when there is no PR number', async () => { + await generate({ checkRun: false, commitStatus: true, prComment: true }, createReport(Status.Pass)); + + expect(serviceClient.post).toHaveBeenCalledTimes(1); + expect(jest.mocked(serviceClient.post).mock.calls[0][1]).toMatchObject({ + output: { checkRun: false, commitStatus: true, prComment: false }, + }); + }); + + test('send all enabled outputs on a PR build', async () => { + await generate({ checkRun: true, commitStatus: true, prComment: true }, createReport(Status.Pass, '1'), '1'); + + expect(serviceClient.post).toHaveBeenCalledTimes(1); + expect(jest.mocked(serviceClient.post).mock.calls[0]).toEqual([ + 'projects/project-id/commit-records/record-id/outputs/github', + expect.objectContaining({ output: { checkRun: true, commitStatus: true, prComment: true } }), + ]); + }); + }); }); diff --git a/packages/bundlemon/lib/main/outputs/outputs/github.ts b/packages/bundlemon/lib/main/outputs/outputs/github.ts index d18c54f..5256a11 100644 --- a/packages/bundlemon/lib/main/outputs/outputs/github.ts +++ b/packages/bundlemon/lib/main/outputs/outputs/github.ts @@ -107,14 +107,22 @@ const output: Output = { logger.debug(`Owner: "${owner}" Repo: "${repo}" sha: "${commitSha}" PR: "${prNumber}"`); + const outputFlags = { + checkRun: shouldPostOutput(normalizedOptions.checkRun, report), + commitStatus: shouldPostOutput(normalizedOptions.commitStatus, report), + // a PR comment can't be posted without a PR + prComment: !!prNumber && shouldPostOutput(normalizedOptions.prComment, report), + }; + + if (!outputFlags.checkRun && !outputFlags.commitStatus && !outputFlags.prComment) { + logger.info('nothing to post, skip'); + return; + } + const payload = { git: { owner, repo, commitSha, prNumber }, auth: authParams, - output: { - checkRun: shouldPostOutput(normalizedOptions.checkRun, report), - commitStatus: shouldPostOutput(normalizedOptions.commitStatus, report), - prComment: shouldPostOutput(normalizedOptions.prComment, report), - }, + output: outputFlags, }; try { From 6064651e4b44ee65ca01a43c0b52ee9c082abea1 Mon Sep 17 00:00:00 2001 From: LironEr Date: Fri, 9 Oct 2026 09:08:06 +0200 Subject: [PATCH 2/2] lint fix --- .../lib/main/outputs/outputs/__tests__/github.spec.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/bundlemon/lib/main/outputs/outputs/__tests__/github.spec.ts b/packages/bundlemon/lib/main/outputs/outputs/__tests__/github.spec.ts index efb9409..c7e386e 100644 --- a/packages/bundlemon/lib/main/outputs/outputs/__tests__/github.spec.ts +++ b/packages/bundlemon/lib/main/outputs/outputs/__tests__/github.spec.ts @@ -173,7 +173,11 @@ describe('github output', () => { const generate = async (options: unknown, report: Report, prNumber?: string) => { const instance = await githubOutput.create({ options, config: createConfig(prNumber) } as any); - await instance!.generate(report); + if (!instance) { + throw new Error('github output was not created'); + } + + await instance.generate(report); }; beforeEach(() => {