Skip to content

Comments

ci: fix ava hang in CI when v8 coverage is set#7619

Merged
mergify[bot] merged 1 commit intomasterfrom
mhofman/ava-coverage-hang-fix
May 4, 2023
Merged

ci: fix ava hang in CI when v8 coverage is set#7619
mergify[bot] merged 1 commit intomasterfrom
mhofman/ava-coverage-hang-fix

Conversation

@mhofman
Copy link
Member

@mhofman mhofman commented May 4, 2023

Description

This should fix the flakes we've been experiencing in CI with ava workers timing out on exit.
I believe I tracked it down to Node's v8 coverage tooling. Others have reported similar issues.

I seem to have found a work around by forcing the ava worker to write out its coverage file before exiting.
Regardless I also implemented a disablement of the timeout error if the test runner detects that coverage is set, which will allow the tests to no longer flake in CI even if the workaround is not sufficient.

Security Considerations

None

Scaling Considerations

None

Documentation Considerations

None

Testing Considerations

First I managed to locally reproduce the timeout failure fairly consistently (50+%)
Then I added and verified the timeout error disablement kicked in correctly (through the added console logging)
Finally I added the workaround and did not manage to reproduce the timeout.

@mhofman mhofman added automerge:squash Automatically squash merge bypass:integration Prevent integration tests from running on PR labels May 4, 2023
Copy link
Member

@turadg turadg left a comment

Choose a reason for hiding this comment

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

Awesome!

Congrats on getting the repro and solving it.

@mergify mergify bot merged commit eb9fb38 into master May 4, 2023
@mergify mergify bot deleted the mhofman/ava-coverage-hang-fix branch May 4, 2023 23:44
mergify bot added a commit that referenced this pull request Jan 22, 2026
closes: #9083

## Description

Breaking changes in v6: https://github.com/avajs/ava/releases/tag/v6.0.0

Relevant subset:
- When tests finish, worker threads or child processes are no longer exited through proces.exit(). If your test file does not exit on its own, the test run will time out. avajs/ava#3260

- Failed assertions now throw, meaning that any subsequent code is not executed. This also impacts the type definitions. avajs/ava#3246

- [Only native errors](https://nodejs.org/api/util.html#utiltypesisnativeerrorvalue) are now considered errors by the t.throws() and t.throwsAsync() assertions. [Object.create(Error.prototype) is not a native error](https://github.com/avajs/ava/blob/v6.0.0/Object.create(Error.prototype)). avajs/ava#3229

This removes our local patches of Ava
- #7619
- #7330

### Security Considerations

none, no runtime change

### Scaling Considerations

none, no runtime change

### Documentation Considerations

none

### Testing Considerations

CI

### Upgrade Considerations

none, no runtime change
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automerge:squash Automatically squash merge bypass:integration Prevent integration tests from running on PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants