Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Grab compare SHA for GH pre-merge commits #147

Merged
merged 11 commits into from
Jul 19, 2024
Merged

feat: Grab compare SHA for GH pre-merge commits #147

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
ed67122
turn on debug mode for example apps
nicholas-codecov Jul 4, 2024
75e1bfc
add in compareSha to types
nicholas-codecov Jul 4, 2024
6f1ee34
update normalizeOptions to accept compareSha override
nicholas-codecov Jul 4, 2024
f900b7e
update GitHubActions to grab compare sha for pre-merge commits
nicholas-codecov Jul 4, 2024
e68b43b
remove compareSha from being sent
nicholas-codecov Jul 4, 2024
9e0809b
pass gh action event data
nicholas-codecov Jul 4, 2024
4ebe7d7
update rollup example to grab from gh action context
nicholas-codecov Jul 4, 2024
c5d9902
update node and pnpm action versions
nicholas-codecov Jul 4, 2024
8448dbc
update version of the cache action
nicholas-codecov Jul 9, 2024
319e880
Merge branch 'main' into feat-grab-compare-sha-for-pre-merge-commits
nicholas-codecov Jul 16, 2024
5869b73
remove temp removal
nicholas-codecov Jul 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 22 additions & 18 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,12 @@ jobs:
fetch-depth: 0

- name: Setup node
uses: actions/setup-node@v3
uses: actions/setup-node@v4
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The node setup action version was upgraded from v3 to v4. Compatibility for all related plugins and actions with the newer version should be ensured.

with:
node-version: 20

- name: Install pnpm
uses: pnpm/action-setup@v2
uses: pnpm/action-setup@v4
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pnpm setup action version was upgraded from v2 to v4. Make sure this plugin's functionalities are unbroken by this change.

with:
version: 9
run_install: false
Expand Down Expand Up @@ -62,12 +62,12 @@ jobs:
fetch-depth: 0

- name: Setup node
uses: actions/setup-node@v3
uses: actions/setup-node@v4
with:
node-version: 20

- name: Install pnpm
uses: pnpm/action-setup@v2
uses: pnpm/action-setup@v4
with:
version: 9
run_install: false
Expand Down Expand Up @@ -108,12 +108,12 @@ jobs:
fetch-depth: 0

- name: Setup node
uses: actions/setup-node@v3
uses: actions/setup-node@v4
with:
node-version: 20

- name: Install pnpm
uses: pnpm/action-setup@v2
uses: pnpm/action-setup@v4
with:
version: 9
run_install: false
Expand Down Expand Up @@ -154,12 +154,12 @@ jobs:
fetch-depth: 0

- name: Setup node
uses: actions/setup-node@v3
uses: actions/setup-node@v4
with:
node-version: 20

- name: Install pnpm
uses: pnpm/action-setup@v2
uses: pnpm/action-setup@v4
with:
version: 9
run_install: false
Expand Down Expand Up @@ -237,12 +237,12 @@ jobs:
fetch-depth: 0

- name: Setup node
uses: actions/setup-node@v3
uses: actions/setup-node@v4
with:
node-version: ${{ matrix.node-version }}

- name: Install pnpm
uses: pnpm/action-setup@v2
uses: pnpm/action-setup@v4
with:
version: 9
run_install: false
Expand Down Expand Up @@ -288,12 +288,12 @@ jobs:
fetch-depth: 0

- name: Setup node
uses: actions/setup-node@v3
uses: actions/setup-node@v4
with:
node-version: 20

- name: Install pnpm
uses: pnpm/action-setup@v2
uses: pnpm/action-setup@v4
with:
version: 9
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HEAD_SHA and BASE_SHA were added as environment variables. Make sure they are being declared and correctly manipulated in the context.

run_install: false
Expand Down Expand Up @@ -326,6 +326,8 @@ jobs:
- name: Build ${{ matrix.example }} app for production
working-directory: ./examples/${{ matrix.example }}
env:
HEAD_SHA: ${{ github.event.pull_request.head.sha }}
BASE_SHA: ${{ github.event.pull_request.base.sha }}
NEXT_UPLOAD_TOKEN: ${{ secrets.CODECOV_ORG_TOKEN }}
NEXT_API_URL: ${{ secrets.CODECOV_API_URL }}
NUXT_UPLOAD_TOKEN: ${{ secrets.CODECOV_ORG_TOKEN }}
Expand Down Expand Up @@ -358,12 +360,12 @@ jobs:
fetch-depth: 0

- name: Setup node
uses: actions/setup-node@v3
uses: actions/setup-node@v4
with:
node-version: 20

- name: Install pnpm
uses: pnpm/action-setup@v2
uses: pnpm/action-setup@v4
with:
version: 9
run_install: false
Expand Down Expand Up @@ -396,6 +398,8 @@ jobs:
- name: Build ${{ matrix.example }} app for staging
working-directory: ./examples/${{ matrix.example }}
env:
HEAD_SHA: ${{ github.event.pull_request.head.sha }}
BASE_SHA: ${{ github.event.pull_request.base.sha }}
NEXT_UPLOAD_TOKEN: ${{ secrets.CODECOV_ORG_TOKEN_STAGING }}
NEXT_API_URL: ${{ secrets.CODECOV_STAGING_API_URL }}
NUXT_UPLOAD_TOKEN: ${{ secrets.CODECOV_ORG_TOKEN }}
Expand Down Expand Up @@ -436,12 +440,12 @@ jobs:
fetch-depth: 0

- name: Setup node
uses: actions/setup-node@v3
uses: actions/setup-node@v4
with:
node-version: 20

- name: Install pnpm
uses: pnpm/action-setup@v2
uses: pnpm/action-setup@v4
with:
version: 9
run_install: false
Expand Down Expand Up @@ -502,12 +506,12 @@ jobs:
fetch-depth: 0

- name: Setup node
uses: actions/setup-node@v3
uses: actions/setup-node@v4
with:
node-version: 20

- name: Install pnpm
uses: pnpm/action-setup@v2
uses: pnpm/action-setup@v4
with:
version: 9
run_install: false
Expand Down
1 change: 1 addition & 0 deletions examples/next-js/next.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ const nextConfig = {
bundleName: "@codecov/example-next-app",
uploadToken: process.env.NEXT_UPLOAD_TOKEN,
apiUrl: process.env.NEXT_API_URL,
debug: true,
nicholas-codecov marked this conversation as resolved.
Show resolved Hide resolved
}),
);

Expand Down
1 change: 1 addition & 0 deletions examples/nuxt/nuxt.config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ export default defineNuxtConfig({
bundleName: "@codecov/example-nuxt-app",
uploadToken: process.env.NUXT_UPLOAD_TOKEN,
apiUrl: process.env.NUXT_API_URL,
debug: true,
nicholas-codecov marked this conversation as resolved.
Show resolved Hide resolved
},
],
],
Expand Down
1 change: 1 addition & 0 deletions examples/remix/vite.config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ export default defineConfig({
bundleName: "@codecov/example-remix-app",
uploadToken: process.env.REMIX_UPLOAD_TOKEN,
apiUrl: process.env.REMIX_API_URL,
debug: true,
nicholas-codecov marked this conversation as resolved.
Show resolved Hide resolved
}),
],
});
5 changes: 5 additions & 0 deletions examples/rollup/rollup.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,11 @@ export default defineConfig({
bundleName: "@codecov/example-rollup-app",
apiUrl: process.env.ROLLUP_API_URL,
uploadToken: process.env.ROLLUP_UPLOAD_TOKEN,
debug: true,
nicholas-codecov marked this conversation as resolved.
Show resolved Hide resolved
uploadOverrides: {
sha: process.env.HEAD_SHA,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding an uploadOverrides object with 'sha'/'compareSha' properties. Make sure this doesn't overlap or cause conflicts with existing functionality, and that the values are being correctly provided from the environment.

compareSha: process.env.BASE_SHA,
},
}),
],
});
1 change: 1 addition & 0 deletions examples/sveltekit/vite.config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ export default defineConfig({
bundleName: "@codecov/example-sveltekit-app",
uploadToken: process.env.SVELTEKIT_UPLOAD_TOKEN,
apiUrl: process.env.SVELTEKIT_API_URL,
debug: true,
nicholas-codecov marked this conversation as resolved.
Show resolved Hide resolved
}),
],
});
1 change: 1 addition & 0 deletions examples/vite/vite.config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ export default defineConfig({
bundleName: "@codecov/example-vite-app",
uploadToken: process.env.VITE_UPLOAD_TOKEN,
apiUrl: process.env.VITE_API_URL,
debug: true,
nicholas-codecov marked this conversation as resolved.
Show resolved Hide resolved
}),
],
});
1 change: 1 addition & 0 deletions examples/webpack/webpack.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ module.exports = {
bundleName: "@codecov/example-webpack-app",
uploadToken: process.env.WEBPACK_UPLOAD_TOKEN,
apiUrl: process.env.WEBPACK_API_URL,
debug: true,
nicholas-codecov marked this conversation as resolved.
Show resolved Hide resolved
}),
],
};
4 changes: 4 additions & 0 deletions packages/bundler-plugin-core/src/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,8 @@ export interface UploadOverrides {
branch?: string;
/** Specify the build number manually. */
build?: string;
/** Specify the compare SHA manually. **GH Actions only**. */
compareSha?: string;
nicholas-codecov marked this conversation as resolved.
Show resolved Hide resolved
/** Specify the pull request number manually. */
pr?: string;
/** Specify the commit SHA manually. */
Expand Down Expand Up @@ -153,6 +155,8 @@ export interface ProviderServiceParams {
build: string;
buildURL: string;
commit: string;
// this is a custom field that is only used for GH pre-merge commits
compareSha?: string;
job: string;
pr: string;
service: string;
Expand Down
5 changes: 5 additions & 0 deletions packages/bundler-plugin-core/src/utils/__tests__/normalizeOptions.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ const tests: Test[] = [
uploadOverrides: {
branch: "test-branch",
build: "test-build",
compareSha: "test-compare-sha",
sha: "test-sha",
slug: "test-slug",
pr: "1234",
Expand All @@ -74,6 +75,7 @@ const tests: Test[] = [
uploadOverrides: {
branch: "test-branch",
build: "test-build",
compareSha: "test-compare-sha",
sha: "test-sha",
slug: "test-slug",
pr: "1234",
Expand Down Expand Up @@ -104,6 +106,8 @@ const tests: Test[] = [
// @ts-expect-error - testing invalid input
build: 123,
// @ts-expect-error - testing invalid input
compareSha: 123,
// @ts-expect-error - testing invalid input
sha: 123,
// @ts-expect-error - testing invalid input
slug: 123,
Expand All @@ -123,6 +127,7 @@ const tests: Test[] = [
"`uploadToken` must be a string.",
"`branch` must be a string.",
"`build` must be a string.",
"`compareSha` must be a string.",
"`pr` must be a string.",
"`sha` must be a string.",
"`slug` must be a string.",
Expand Down
5 changes: 5 additions & 0 deletions packages/bundler-plugin-core/src/utils/normalizeOptions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@ const UploadOverridesSchema = z.object({
invalid_type_error: "`build` must be a string.",
})
.optional(),
compareSha: z
nicholas-codecov marked this conversation as resolved.
Show resolved Hide resolved
.string({
invalid_type_error: "`compareSha` must be a string.",
})
.optional(),
pr: z
.string({
invalid_type_error: "`pr` must be a string.",
Expand Down
5 changes: 5 additions & 0 deletions packages/bundler-plugin-core/src/utils/preProcessBody.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,11 @@
body[key] = encodeSlug(value);
}

// temporary removal for testing
nicholas-codecov marked this conversation as resolved.
Show resolved Hide resolved
if (key === "compareSha") {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The 'compareSha' key is being temporarily removed for testing. If this code is intended for production, this deletion should not occur here.

delete body[key];

Check warning on line 15 in packages/bundler-plugin-core/src/utils/preProcessBody.ts

View check run for this annotation

Codecov Notifications / codecov/patch

packages/bundler-plugin-core/src/utils/preProcessBody.ts#L15 

Added line #L15 was not covered by tests
}

if (!value || value === "") {
body[key] = null;
}
Expand Down
38 changes: 38 additions & 0 deletions packages/bundler-plugin-core/src/utils/providers/GitHubActions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,43 @@
return commit ?? "";
}

function _getCompareSHA(inputs: ProviderUtilInputs, output: Output): string {
const { args } = inputs;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_getCompareSHA function is added. The complexity of the function could potentially be a point of failure, ensure it is adequately tested and performs as expected in various scenarios.

if (args?.compareSha && args.compareSha !== "") {
debug(`Using commit: ${args.compareSha}`, { enabled: output.debug });
return args.compareSha;
}

Check warning on line 166 in packages/bundler-plugin-core/src/utils/providers/GitHubActions.ts

View check run for this annotation

Codecov Notifications / codecov/patch

packages/bundler-plugin-core/src/utils/providers/GitHubActions.ts#L164-L166 

Added lines #L164 - L166 were not covered by tests

let compareSha = "";
const pr = _getPR(inputs);
if (pr) {
const mergeCommitRegex = /^[a-z0-9]{40} [a-z0-9]{40}$/;
const mergeCommitMessage = runExternalProgram("git", [
"show",
"--no-patch",
"--format=%P",
]);

debug(`Merge commit message: ${mergeCommitMessage}`, {
enabled: output.debug,
});

if (mergeCommitRegex.exec(mergeCommitMessage)) {
const splitMergeCommit = mergeCommitMessage.split(" ");

debug(`Split commit message: ${splitMergeCommit}`, {
enabled: output.debug,
});

compareSha = splitMergeCommit?.[0] ? splitMergeCommit[0] : "";
}
}

debug(`Using compareSha: ${compareSha ?? ""}`, { enabled: output.debug });

return compareSha ?? "";
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't we return null or undefined if compareSha is not set?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a left-over style from the original Codecov Node uploader, tho we should probably adjust this. We do have a guard in the preProcessBody that does handle this for us. Tho we should probably update this overall to just return null.

}

function _getSlug(inputs: ProviderUtilInputs): string {
const { args, envs } = inputs;
if (args?.slug && args.slug !== "") {
Expand All @@ -175,6 +212,7 @@
build: _getBuild(inputs),
buildURL: await _getBuildURL(inputs),
commit: _getSHA(inputs, output),
compareSha: _getCompareSHA(inputs, output),
job: _getJob(inputs.envs),
pr: _getPR(inputs),
service: _getService(),
Expand Down
Loading
Loading