fix: skip unnecessary GitHub output

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<string, boolean>) =>
+  ({
+    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);
+  });
+});

apps/service/src/controllers/githubController.ts

@@ -23,6 +23,12 @@ export const githubOutputController: FastifyValidatedRoute<GithubOutputRequestSc
       body: { git, output, auth },
     } = req;
 
+    if (!output.checkRun && !output.commitStatus && !output.prComment) {
+      req.log.info({ projectId, commitRecordId }, 'no GitHub output requested, nothing to post');
+      res.send({});
+      return;
+    }
+
     const project = await getProject(projectId);
 
     if (!project) {

packages/bundlemon/lib/main/outputs/outputs/__tests__/github.spec.ts

@@ -1,5 +1,20 @@
 import { Report, Status } from 'bundlemon-utils';
-import { validateOptions, GithubOutputOptions, GithubOutputPostOption, shouldPostOutput } from '../github';
+import githubOutput, {
+  validateOptions,
+  GithubOutputOptions,
+  GithubOutputPostOption,
+  shouldPostOutput,
+} from '../github';
+import { serviceClient } from '../../../../common/service';
+
+import type { NormalizedConfig } from '../../../types';
+
+jest.mock('../../../../common/service', () => ({ 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,80 @@ 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);
+
+      if (!instance) {
+        throw new Error('github output was not created');
+      }
+
+      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 } }),
+      ]);
+    });
+  });
 });

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 {