From bdd7eae6cd9967f63747c9a5e46fdf6c86b41f93 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mat=C4=9Bj=20Chalk?= Date: Fri, 8 Nov 2024 08:47:57 +0100 Subject: [PATCH 1/4] fix(ci): pass project name to downloadReportArtifact --- packages/ci/README.md | 14 +++++++------- packages/ci/src/lib/models.ts | 2 +- packages/ci/src/lib/run.integration.test.ts | 2 +- packages/ci/src/lib/run.ts | 5 +++-- 4 files changed, 12 insertions(+), 11 deletions(-) diff --git a/packages/ci/README.md b/packages/ci/README.md index 801422e3b..5d060b31e 100644 --- a/packages/ci/README.md +++ b/packages/ci/README.md @@ -74,13 +74,13 @@ This will additionally compare reports from both source and target branches and The PR flow requires interacting with the Git provider's API to post a comparison comment. Wrap these requests in functions and pass them in as an object which configures the provider. -| Property | Required | Type | Description | -| :----------------------- | :------: | :----------------------------------------------- | :----------------------------------------------------------------------------------------------------------------------- | -| `createComment` | yes | `(body: string) => Promise` | Posts a new comment to PR | -| `updateComment` | yes | `(id: number, body: string) => Promise` | Updates existing PR comment | -| `listComments` | yes | `() => Promise` | Fetches all comments from PR | -| `maxCommentChars` | yes | `number` | Character limit for comment body | -| `downloadReportArtifact` | no | `() => Promise` | Fetches previous report for base branch (returns path to downloaded `report.json`), used as cache to speed up comparison | +| Property | Required | Type | Description | +| :----------------------- | :------: | :----------------------------------------------- | :------------------------------------------------------------------------------------------------------------------- | +| `createComment` | yes | `(body: string) => Promise` | Posts a new comment to PR | +| `updateComment` | yes | `(id: number, body: string) => Promise` | Updates existing PR comment | +| `listComments` | yes | `() => Promise` | Fetches all comments from PR | +| `maxCommentChars` | yes | `number` | Character limit for comment body | +| `downloadReportArtifact` | no | `(project?: string) => Promise` | Fetches previous (root/project) `report.json` for base branch and returns path, used as cache to speed up comparison | A `Comment` object has the following required properties: diff --git a/packages/ci/src/lib/models.ts b/packages/ci/src/lib/models.ts index 99a59f9c3..9c91ad71b 100644 --- a/packages/ci/src/lib/models.ts +++ b/packages/ci/src/lib/models.ts @@ -37,7 +37,7 @@ export type GitRefs = { */ export type ProviderAPIClient = { maxCommentChars: number; - downloadReportArtifact?: () => Promise; + downloadReportArtifact?: (project?: string) => Promise; listComments: () => Promise; updateComment: (id: number, body: string) => Promise; createComment: (body: string) => Promise; diff --git a/packages/ci/src/lib/run.integration.test.ts b/packages/ci/src/lib/run.integration.test.ts index f306f0d70..dc3371c59 100644 --- a/packages/ci/src/lib/run.integration.test.ts +++ b/packages/ci/src/lib/run.integration.test.ts @@ -328,7 +328,7 @@ describe('runInCI', () => { expect.stringContaining(diffMdString), ); expect(api.createComment).not.toHaveBeenCalled(); - expect(api.downloadReportArtifact).toHaveBeenCalledWith(); + expect(api.downloadReportArtifact).toHaveBeenCalledWith(undefined); expect(utils.executeProcess).toHaveBeenCalledTimes(2); expect(utils.executeProcess).toHaveBeenNthCalledWith(1, { diff --git a/packages/ci/src/lib/run.ts b/packages/ci/src/lib/run.ts index bba1c4da7..f3c2e13dc 100644 --- a/packages/ci/src/lib/run.ts +++ b/packages/ci/src/lib/run.ts @@ -189,16 +189,17 @@ async function runOnProject(args: { } async function collectPreviousReport(args: { + project: ProjectConfig | null; base: GitBranch; api: ProviderAPIClient; settings: Settings; ctx: CommandContext; git: SimpleGit; }): Promise { - const { base, api, settings, ctx, git } = args; + const { project, base, api, settings, ctx, git } = args; const logger = settings.logger; - const cachedBaseReport = await api.downloadReportArtifact?.(); + const cachedBaseReport = await api.downloadReportArtifact?.(project?.name); if (api.downloadReportArtifact != null) { logger.info( `Previous report artifact ${cachedBaseReport ? 'found' : 'not found'}`, From af91a86220e2f50ed3d31aabad3b0791d9ba7477 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mat=C4=9Bj=20Chalk?= Date: Fri, 8 Nov 2024 08:51:17 +0100 Subject: [PATCH 2/4] fix(ci): improve misleading logs --- packages/ci/src/lib/models.ts | 2 +- packages/ci/src/lib/run.ts | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/ci/src/lib/models.ts b/packages/ci/src/lib/models.ts index 9c91ad71b..258a1101a 100644 --- a/packages/ci/src/lib/models.ts +++ b/packages/ci/src/lib/models.ts @@ -44,7 +44,7 @@ export type ProviderAPIClient = { }; /** - * PR comment from {@link ProviderAPIClient} + * PR/MR comment from {@link ProviderAPIClient} */ export type Comment = { id: number; diff --git a/packages/ci/src/lib/run.ts b/packages/ci/src/lib/run.ts index f3c2e13dc..d5d91cb87 100644 --- a/packages/ci/src/lib/run.ts +++ b/packages/ci/src/lib/run.ts @@ -140,7 +140,7 @@ async function runOnProject(args: { } logger.info( - `PR detected, preparing to compare base branch ${base.ref} to head ${head.ref}`, + `PR/MR detected, preparing to compare base branch ${base.ref} to head ${head.ref}`, ); const prevReport = await collectPreviousReport({ ...args, base, ctx }); @@ -236,7 +236,7 @@ async function collectPreviousReport(args: { logger.debug(`Collected previous report at ${prevReportPath}`); await git.checkout(['-f', '-']); - logger.info('Switched back to PR branch'); + logger.info('Switched back to PR/MR branch'); return prevReport; } @@ -267,7 +267,7 @@ async function findNewIssues(args: { logger.debug( `Found ${issues.length} relevant issues for ${ Object.keys(changedFiles).length - } changed files and created GitHub annotations`, + } changed files`, ); return issues; From 8a2fd8b2d861f9cc0380f9802634422ae1839ed4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mat=C4=9Bj=20Chalk?= Date: Fri, 8 Nov 2024 08:55:43 +0100 Subject: [PATCH 3/4] docs(ci): link to new GitLab integration --- packages/ci/README.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/ci/README.md b/packages/ci/README.md index 5d060b31e..c0522c5aa 100644 --- a/packages/ci/README.md +++ b/packages/ci/README.md @@ -14,10 +14,10 @@ This package exports **provider-agnostic core logic for running Code PushUp in CI pipelines**. It serves as the base for the following provider integrations: -| | | -| :------------- | :-------------------------------------------------------------------------------- | -| GitHub Actions | [`code-pushup/github-action`](https://github.com/marketplace/actions/code-pushup) | -| GitLab CI/CD | _coming soon_ | +| | | +| :------------- | :-------------------------------------------------------------------------------------------------- | +| GitHub Actions | [`code-pushup/github-action`](https://github.com/marketplace/actions/code-pushup) | +| GitLab CI/CD | [`code-pushup/gitlab-pipelines-template`](https://gitlab.com/code-pushup/gitlab-pipelines-template) | ## Setup From c432bb8d5ec9430593b4fc9cb3413e5df34548ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mat=C4=9Bj=20Chalk?= Date: Fri, 8 Nov 2024 09:07:17 +0100 Subject: [PATCH 4/4] fix(ci): catch errors from downloadReportArtifact - log warning and proceed --- packages/ci/src/lib/run.ts | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/packages/ci/src/lib/run.ts b/packages/ci/src/lib/run.ts index d5d91cb87..276f50ce0 100644 --- a/packages/ci/src/lib/run.ts +++ b/packages/ci/src/lib/run.ts @@ -101,14 +101,16 @@ export async function runInCI( return { mode: 'standalone', artifacts, newIssues }; } -// eslint-disable-next-line max-lines-per-function -async function runOnProject(args: { +type RunOnProjectArgs = { project: ProjectConfig | null; refs: GitRefs; api: ProviderAPIClient; settings: Settings; git: SimpleGit; -}): Promise { +}; + +// eslint-disable-next-line max-lines-per-function +async function runOnProject(args: RunOnProjectArgs): Promise { const { project, refs: { head, base }, @@ -188,18 +190,24 @@ async function runOnProject(args: { return { ...diffOutput, newIssues }; } -async function collectPreviousReport(args: { - project: ProjectConfig | null; +type CollectPreviousReportArgs = RunOnProjectArgs & { base: GitBranch; - api: ProviderAPIClient; - settings: Settings; ctx: CommandContext; - git: SimpleGit; -}): Promise { +}; + +async function collectPreviousReport( + args: CollectPreviousReportArgs, +): Promise { const { project, base, api, settings, ctx, git } = args; const logger = settings.logger; - const cachedBaseReport = await api.downloadReportArtifact?.(project?.name); + const cachedBaseReport = await api + .downloadReportArtifact?.(project?.name) + .catch((error: unknown) => { + logger.warn( + `Error when downloading previous report artifact, skipping - ${stringifyError(error)}`, + ); + }); if (api.downloadReportArtifact != null) { logger.info( `Previous report artifact ${cachedBaseReport ? 'found' : 'not found'}`,