Skip to content

Commit

Permalink
fix: Remove setting of process.exitCode that breaks Jest tests (#45562)
Browse files Browse the repository at this point in the history
Summary:
In Node 20, the script to run unit tests in CI (`scripts/run-ci-javascript-tests.js`) will fail, even when all the Jest tests pass. This happens because one of the JS modules being tested is setting `process.exitCode` (see jestjs/jest#9324 (comment)).

Changes:

- Modified the affected module to throw an exception when failing, instead of setting the exit code
- Adjusted the unit test for that module

## Changelog:

[General] [Fixed] - Remove setting of process.exitCode that breaks Jest tests

Pull Request resolved: #45562

Test Plan:
Before this change, running `node scripts/run-ci-javascript-tests.js` would fail with Node 20.
After this change, it succeeds.

Reviewed By: blakef

Differential Revision: D60033582

Pulled By: cipolleschi

fbshipit-source-id: 71b7f4495d414e719a9bd2d892bd1bc3045ddd5d
  • Loading branch information
douglowder authored and cipolleschi committed Jul 22, 2024
1 parent d4b3fc9 commit 72a38aa
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 17 deletions.
26 changes: 14 additions & 12 deletions scripts/releases-ci/__tests__/publish-updated-packages-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,15 +47,13 @@ describe('publishUpdatedPackages', () => {
.spyOn(console, 'error')
.mockImplementation(() => {});

await publishUpdatedPackages();

expect(consoleError.mock.calls).toMatchInlineSnapshot(`
Array [
Array [
"Failed to read Git commit message, exiting.",
],
]
`);
let message = '';
try {
await publishUpdatedPackages();
} catch (e) {
message = e.message;
}
expect(message).toEqual('Failed to read Git commit message, exiting.');
});

test("should exit when commit message does not include '#publish-packages-to-npm'", async () => {
Expand Down Expand Up @@ -246,7 +244,6 @@ describe('publishUpdatedPackages', () => {
]
`);
});

test('should exit with error if one or more packages fail after retry', async () => {
execMock.mockImplementationOnce(() => ({code: 0}));
execMock.mockImplementation(() => ({
Expand All @@ -258,10 +255,15 @@ describe('publishUpdatedPackages', () => {
.spyOn(console, 'log')
.mockImplementation(() => {});

await publishUpdatedPackages();
let message = '';
try {
await publishUpdatedPackages();
} catch (e) {
message = e.message;
}

expect(consoleLog).toHaveBeenLastCalledWith('--- Retrying once! ---');
expect(process.exitCode).toBe(1);
expect(message).toEqual('Failed packages count = 1');
});
});
});
Expand Down
7 changes: 2 additions & 5 deletions scripts/releases-ci/publish-updated-packages.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,7 @@ async function publishUpdatedPackages() {
try {
commitMessage = execSync('git log -1 --pretty=%B').toString();
} catch {
console.error('Failed to read Git commit message, exiting.');
process.exitCode = 1;
return;
throw new Error('Failed to read Git commit message, exiting.');
}

if (!commitMessage.includes(PUBLISH_PACKAGES_TAG)) {
Expand Down Expand Up @@ -117,8 +115,7 @@ async function publishUpdatedPackages() {
}

if (failedPackages.length) {
process.exitCode = 1;
return;
throw new Error(`Failed packages count = ${failedPackages.length}`);
}

console.log('Done ✅');
Expand Down

0 comments on commit 72a38aa

Please sign in to comment.