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.

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

[test] Do not set process.exitCode in tested code #203

Merged
merged 1 commit into from
Nov 16, 2023

Conversation

Drarig29
Copy link
Contributor

Using core.setFailed() makes the CI fail (for Node.js 20.x only) because this function sets process.exitCode to 1.

Related issue: jestjs/jest#14501

So, mocking the implementation of setFailed to a noop is required, since simply spying only registers the calls of that function.

Here is what was happening:

image

@Drarig29 Drarig29 requested a review from a team as a code owner November 13, 2023 14:34
@@ -43,7 +44,7 @@ describe('Run Github Action', () => {
})

test('Github Action core.getInput parameters are passed on to runTests', async () => {
jest.spyOn(synthetics, 'executeTests').mockImplementation(() => ({} as any))
jest.spyOn(synthetics, 'executeTests').mockImplementation()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The () => ({} as any) is not needed for tests to pass.

}
await expect(run()).rejects.toThrowError('Config file not found')
expect(process.exitCode).toBe(1)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can't assert on this property, since changing it will make Jest return that exit code, and fail the CI.

@Drarig29 Drarig29 force-pushed the corentin.girard/fix-node-20-issue-unit-tests branch from e1745fd to 70f1af5 Compare November 13, 2023 14:43
Copy link
Contributor

@etnbrd etnbrd left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@Drarig29 Drarig29 merged commit 35d60d9 into main Nov 16, 2023
4 checks passed
@Drarig29 Drarig29 deleted the corentin.girard/fix-node-20-issue-unit-tests branch November 16, 2023 13:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants