Skip to content

fix: race condition between the Karma shutdown and coverage writing#463

Merged
jginsburgn merged 2 commits intokarma-runner:masterfrom
devoto13:fix-race
Feb 5, 2022
Merged

fix: race condition between the Karma shutdown and coverage writing#463
jginsburgn merged 2 commits intokarma-runner:masterfrom
devoto13:fix-race

Conversation

@devoto13
Copy link
Copy Markdown
Contributor

@devoto13 devoto13 commented Jan 19, 2022

The creation of parent directories is asynchronous process, which was not properly awaited, which resulted in the coverage reports not being written to the disk sometimes as Karma process has exited before the reporter completed the writing of the coverage report.

Also update the test case to use async implementation of the corresponding stub to prevent regressions in the future. It seems that manually calling done parameter in several tests was an attempt to solve the same race condition in the unit tests. They are not needed anymore. Nah, probably they are just leftovers of the old reporter implementation.

Fixes #434

The creation of parent directories is asynchronous process, which was not properly awaited, which resulted in the coverage reports not being written to the disk sometimes as Karma process has exited before the reporter completed the writing of the coverage report.

Also update the test case to use async implementation of the corresponding stub to prevent regressions in the future. Remove manual calls for `done` parameter in tests as it is handled by the stub now.

Fixes karma-runner#434
@devoto13 devoto13 force-pushed the fix-race branch 2 times, most recently from 3936737 to 372427f Compare January 21, 2022 14:58
The issue surfaced itself after d970028. The EEXIST error from concurrent attempts to create the same directory were previously silently swallowed, but started to show up once the call was properly synchronized.

The EEXIST will now result in `promiseComplete` being rejected and reported as `unhandledRejection` because it is awaited in the `onExit` callback. The unhandled rejection is then picked up by `karma` [here](https://github.com/karma-runner/karma/blob/c985155a4eac95c525e1217e98d4013ac5f53305/lib/server.js#L395) triggering [the close logic](https://github.com/karma-runner/karma/blob/c985155a4eac95c525e1217e98d4013ac5f53305/lib/server.js#L392) which (among other things) will trigger `onExit` callback causing an infinite loop.

The local fix is to handle the rejected promise directly and report failure to karma by passing a non-zero exit code.
@jginsburgn jginsburgn merged commit bca2c69 into karma-runner:master Feb 5, 2022
@karmarunnerbot
Copy link
Copy Markdown
Member

🎉 This PR is included in version 2.1.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@devoto13 devoto13 deleted the fix-race branch March 10, 2022 13:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

No coverage report generated for large project

3 participants