Skip to content

Commit 138fe3d

Browse files
matejchalkmatejchalk
committed
feat(ci): add jobId option to prevent conflicting PR comments
1 parent 058e4bd commit 138fe3d

File tree

8 files changed

+59
-42
lines changed

8 files changed

+59
-42
lines changed

‎packages/ci/README.md‎

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -94,21 +94,22 @@ A `Comment` object has the following required properties:
9494

9595
Optionally, you can override default options for further customization:
9696

97-
| Property | Type | Default | Description |
98-
| :----------------- | :------------------------ | :------------------------------- | :------------------------------------------------------------------------------------------------------------------------------------------------------- |
99-
| `monorepo` | `boolean \| MonorepoTool` | `false` | Enables [monorepo mode](#monorepo-mode) |
100-
| `parallel` | `boolean \| number` | `false` | Enables parallel execution in [monorepo mode](#monorepo-mode) |
101-
| `projects` | `string[] \| null` | `null` | Custom projects configuration for [monorepo mode](#monorepo-mode) |
102-
| `task` | `string` | `'code-pushup'` | Name of command to run Code PushUp per project in [monorepo mode](#monorepo-mode) |
103-
| `nxProjectsFilter` | `string \| string[]` | `'--with-target={task}'` | Arguments passed to [`nx show projects`](https://nx.dev/nx-api/nx/documents/show#projects), only relevant for Nx in [monorepo mode](#monorepo-mode) [^2] |
104-
| `directory` | `string` | `process.cwd()` | Directory in which Code PushUp CLI should run |
105-
| `config` | `string \| null` | `null` [^1] | Path to config file (`--config` option) |
106-
| `silent` | `boolean` | `false` | Hides logs from CLI commands (errors will be printed) |
107-
| `bin` | `string` | `'npx --no-install code-pushup'` | Command for executing Code PushUp CLI |
108-
| `detectNewIssues` | `boolean` | `true` | Toggles if new issues should be detected and returned in `newIssues` property |
109-
| `skipComment` | `boolean` | `false` | Toggles if comparison comment is posted to PR |
110-
| `configPatterns` | `ConfigPatterns \| null` | `null` | Additional configuration which enables [faster CI runs](#faster-ci-runs-with-configpatterns) |
111-
| `searchCommits` | `boolean \| number` | `false` | If base branch has no cached report in portal, [extends search up to 100 recent commits](#search-latest-commits-for-previous-report) |
97+
| Property | Type | Default | Description |
98+
| :----------------- | :------------------------- | :------------------------------- | :------------------------------------------------------------------------------------------------------------------------------------------------------- |
99+
| `monorepo` | `boolean \| MonorepoTool` | `false` | Enables [monorepo mode](#monorepo-mode) |
100+
| `parallel` | `boolean \| number` | `false` | Enables parallel execution in [monorepo mode](#monorepo-mode) |
101+
| `projects` | `string[] \| null` | `null` | Custom projects configuration for [monorepo mode](#monorepo-mode) |
102+
| `task` | `string` | `'code-pushup'` | Name of command to run Code PushUp per project in [monorepo mode](#monorepo-mode) |
103+
| `nxProjectsFilter` | `string \| string[]` | `'--with-target={task}'` | Arguments passed to [`nx show projects`](https://nx.dev/nx-api/nx/documents/show#projects), only relevant for Nx in [monorepo mode](#monorepo-mode) [^2] |
104+
| `directory` | `string` | `process.cwd()` | Directory in which Code PushUp CLI should run |
105+
| `config` | `string \| null` | `null` [^1] | Path to config file (`--config` option) |
106+
| `silent` | `boolean` | `false` | Hides logs from CLI commands (errors will be printed) |
107+
| `bin` | `string` | `'npx --no-install code-pushup'` | Command for executing Code PushUp CLI |
108+
| `detectNewIssues` | `boolean` | `true` | Toggles if new issues should be detected and returned in `newIssues` property |
109+
| `skipComment` | `boolean` | `false` | Toggles if comparison comment is posted to PR |
110+
| `configPatterns` | `ConfigPatterns \| null` | `null` | Additional configuration which enables [faster CI runs](#faster-ci-runs-with-configpatterns) |
111+
| `searchCommits` | `boolean \| number` | `false` | If base branch has no cached report in portal, [extends search up to 100 recent commits](#search-latest-commits-for-previous-report) |
112+
| `jobId` | `string \| number \| null` | `null` | Differentiate PR comments (useful if multiple jobs run Code PushUp) |
112113

113114
[^1]: By default, the `code-pushup.config` file is autodetected as described in [`@code-pushup/cli` docs](../cli/README.md#configuration).
114115

‎packages/ci/src/lib/cli/context.unit.test.ts‎

Lines changed: 3 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,17 @@
11
import { expect } from 'vitest';
2+
import { DEFAULT_SETTINGS } from '../settings.js';
23
import { type CommandContext, createCommandContext } from './context.js';
34

45
describe('createCommandContext', () => {
56
it('should pick CLI-related settings in standalone mode', () => {
67
expect(
78
createCommandContext(
89
{
10+
...DEFAULT_SETTINGS,
911
bin: 'npx --no-install code-pushup',
1012
config: null,
11-
detectNewIssues: true,
1213
directory: '/test',
1314
silent: false,
14-
monorepo: false,
15-
parallel: false,
16-
nxProjectsFilter: '--with-target={task}',
17-
projects: null,
18-
task: 'code-pushup',
19-
skipComment: false,
20-
configPatterns: null,
21-
searchCommits: false,
2215
},
2316
null,
2417
),
@@ -34,19 +27,11 @@ describe('createCommandContext', () => {
3427
expect(
3528
createCommandContext(
3629
{
30+
...DEFAULT_SETTINGS,
3731
bin: 'npx --no-install code-pushup',
3832
config: null,
39-
detectNewIssues: true,
4033
directory: '/test',
4134
silent: false,
42-
monorepo: false,
43-
parallel: false,
44-
nxProjectsFilter: '--with-target={task}',
45-
projects: null,
46-
task: 'code-pushup',
47-
skipComment: false,
48-
configPatterns: null,
49-
searchCommits: false,
5035
},
5136
{
5237
name: 'ui',

‎packages/ci/src/lib/comment.ts‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,16 @@
11
import { readFile } from 'node:fs/promises';
22
import { logger } from '@code-pushup/utils';
3-
import type { ProviderAPIClient } from './models.js';
3+
import type { ProviderAPIClient, Settings } from './models.js';
44

55
export async function commentOnPR(
66
mdPath: string,
77
api: ProviderAPIClient,
8+
settings: Pick<Settings, 'jobId'>,
89
): Promise<number> {
910
const markdown = await readFile(mdPath, 'utf8');
10-
const identifier = `<!-- generated by @code-pushup/ci -->`;
11+
const identifier = settings.jobId
12+
? `<!-- generated by @code-pushup/ci [jobId=${settings.jobId}] -->`
13+
: '<!-- generated by @code-pushup/ci -->';
1114
const body = truncateBody(
1215
`${markdown}\n\n${identifier}\n`,
1316
api.maxCommentChars,

‎packages/ci/src/lib/comment.unit.test.ts‎

Lines changed: 31 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ describe('commentOnPR', () => {
2929
listComments: vi.fn(),
3030
} satisfies ProviderAPIClient;
3131

32+
const settings = { jobId: null };
33+
3234
beforeEach(() => {
3335
vol.fromJSON({ [diffFile]: diffText }, MEMFS_VOLUME);
3436
api.listComments.mockResolvedValue([]);
@@ -37,7 +39,9 @@ describe('commentOnPR', () => {
3739
it('should create new comment if none existing', async () => {
3840
api.listComments.mockResolvedValue([]);
3941

40-
await expect(commentOnPR(diffPath, api)).resolves.toBe(comment.id);
42+
await expect(commentOnPR(diffPath, api, settings)).resolves.toBe(
43+
comment.id,
44+
);
4145

4246
expect(api.listComments).toHaveBeenCalled();
4347
expect(api.createComment).toHaveBeenCalledWith(comment.body);
@@ -47,7 +51,9 @@ describe('commentOnPR', () => {
4751
it("should create new comment if existing comments don't match", async () => {
4852
api.listComments.mockResolvedValue([otherComment]);
4953

50-
await expect(commentOnPR(diffPath, api)).resolves.toBe(comment.id);
54+
await expect(commentOnPR(diffPath, api, settings)).resolves.toBe(
55+
comment.id,
56+
);
5157

5258
expect(api.listComments).toHaveBeenCalled();
5359
expect(api.createComment).toHaveBeenCalledWith(comment.body);
@@ -57,17 +63,35 @@ describe('commentOnPR', () => {
5763
it('should update previous comment if it matches', async () => {
5864
api.listComments.mockResolvedValue([comment]);
5965

60-
await expect(commentOnPR(diffPath, api)).resolves.toBe(comment.id);
66+
await expect(commentOnPR(diffPath, api, settings)).resolves.toBe(
67+
comment.id,
68+
);
6169

6270
expect(api.listComments).toHaveBeenCalled();
6371
expect(api.createComment).not.toHaveBeenCalled();
6472
expect(api.updateComment).toHaveBeenCalledWith(comment.id, comment.body);
6573
});
6674

75+
it('should not match comment with different jobId', async () => {
76+
api.listComments.mockResolvedValue([comment]);
77+
78+
await expect(
79+
commentOnPR(diffPath, api, { jobId: 'monorepo-mode' }),
80+
).resolves.toBe(comment.id);
81+
82+
expect(api.listComments).toHaveBeenCalled();
83+
expect(api.createComment).toHaveBeenCalledWith(
84+
`${diffText}\n\n<!-- generated by @code-pushup/ci [jobId=monorepo-mode] -->\n`,
85+
);
86+
expect(api.updateComment).not.toHaveBeenCalled();
87+
});
88+
6789
it('should update previous comment which matches and ignore other comments', async () => {
6890
api.listComments.mockResolvedValue([otherComment, comment]);
6991

70-
await expect(commentOnPR(diffPath, api)).resolves.toBe(comment.id);
92+
await expect(commentOnPR(diffPath, api, settings)).resolves.toBe(
93+
comment.id,
94+
);
7195

7296
expect(api.listComments).toHaveBeenCalled();
7397
expect(api.createComment).not.toHaveBeenCalled();
@@ -80,7 +104,9 @@ describe('commentOnPR', () => {
80104
.join('\n');
81105
await writeFile(diffPath, longDiffText);
82106

83-
await expect(commentOnPR(diffPath, api)).resolves.toBe(comment.id);
107+
await expect(commentOnPR(diffPath, api, settings)).resolves.toBe(
108+
comment.id,
109+
);
84110

85111
expect(api.createComment).toHaveBeenCalledWith(
86112
expect.stringContaining('...*[Comment body truncated]*'),

‎packages/ci/src/lib/models.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ export type Options = {
2020
skipComment?: boolean;
2121
configPatterns?: ConfigPatterns | null;
2222
searchCommits?: boolean | number;
23+
jobId?: string | number | null;
2324
};
2425

2526
/**

‎packages/ci/src/lib/run-monorepo.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ export async function runInMonorepoMode(
8181

8282
const commentId = settings.skipComment
8383
? null
84-
: await commentOnPR(diffPath, api);
84+
: await commentOnPR(diffPath, api, settings);
8585

8686
return {
8787
mode: 'monorepo',

‎packages/ci/src/lib/run-standalone.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ export async function runInStandaloneMode(
1414

1515
const commentMdPath = files.comparison?.md;
1616
if (!settings.skipComment && commentMdPath) {
17-
const commentId = await commentOnPR(commentMdPath, api);
17+
const commentId = await commentOnPR(commentMdPath, api, settings);
1818
return {
1919
mode: 'standalone',
2020
files,

‎packages/ci/src/lib/settings.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ export const DEFAULT_SETTINGS: Settings = {
1616
skipComment: false,
1717
configPatterns: null,
1818
searchCommits: false,
19+
jobId: null,
1920
};
2021

2122
export const MIN_SEARCH_COMMITS = 1;

0 commit comments

Comments
 (0)