Skip to content

Comments

upgrade Ava to 6#9081

Merged
mergify[bot] merged 18 commits intomasterfrom
ta/ava-6
Jan 22, 2026
Merged

upgrade Ava to 6#9081
mergify[bot] merged 18 commits intomasterfrom
ta/ava-6

Conversation

@turadg
Copy link
Member

@turadg turadg commented Mar 13, 2024

closes: #9083

Description

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

Relevant subset:

This removes our local patches of Ava

Security Considerations

none, no runtime change

Scaling Considerations

none, no runtime change

Documentation Considerations

none

Testing Considerations

CI

Upgrade Considerations

none, no runtime change

@turadg turadg changed the title chore(deps): bump Ava to 6.1.1 upgrade Ava to 6 Mar 15, 2024
@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented May 2, 2024

Deploying agoric-sdk with  Cloudflare Pages  Cloudflare Pages

Latest commit: af03497
Status: ✅  Deploy successful!
Preview URL: https://49a16dfe.agoric-sdk.pages.dev
Branch Preview URL: https://ta-ava-6.agoric-sdk.pages.dev

View logs

@turadg turadg force-pushed the ta/ava-6 branch 2 times, most recently from a8ac83e to 844cedd Compare May 6, 2024 18:50
@turadg turadg force-pushed the ta/ava-6 branch 3 times, most recently from 149982a to a2ab680 Compare December 31, 2024 04:48
mergify bot added a commit that referenced this pull request Apr 11, 2025
refs: #5575

## Description

As needed for Ava 6,
- #9083

It has different exception behavior that breaks the intentional `.failing` hack. This changes the four`test.failing` to use `test` that are confirmed to be the correct behavior. Since the test is of test failures, it uses a new spy instead.

### Security Considerations
none

### Scaling Considerations
none

### Documentation Considerations

~This is one package of the monorepo on Ava 6. I [tried bumping them all before](#9081) but ran into problems. We should tackle them all eventually,~ (DEFERRED)


### Testing Considerations
per se
### Upgrade Considerations
none
@turadg
Copy link
Member Author

turadg commented Dec 17, 2025

@copilot get this branch to green. It updates Ava which is causing some tests to fail. Keep track of the status of all packages and which tests are failing in a Markdown file. Fix one package at a time. When you've fixed a package, make a conventional commit ("test:") with the changes indicating that it's now compatible with Ava 6. If the problem is tracking unhandled rejections, consider using packages/internal/src/lib-nodejs/ava-unhandled-rejection.js. You're also free to improve that code as necessary. Don't change any other src code; the CI failure is due to the tests themselves.

Copy link
Contributor

Copilot AI commented Dec 17, 2025

@turadg I've opened a new pull request, #12289, to work on those changes. Once the pull request is ready, I'll request review from you.

@mergify
Copy link
Contributor

mergify bot commented Jan 5, 2026

⚠️ The sha of the head commit of this PR conflicts with #11255. Mergify cannot evaluate rules on this PR. ⚠️

@turadg turadg marked this pull request as ready for review January 5, 2026 19:26
@turadg turadg requested review from mhofman and michaelfig January 5, 2026 19:27
@turadg
Copy link
Member Author

turadg commented Jan 5, 2026

Reviewers, there may be a flake in the SwingSet tests. 9770a9a failed and then 0ea218a passed but it shouldn't have affected that package.

Update: a re-run of the failing test passed. It's probably just an existing flake

@socket-security

This comment was marked as resolved.

Copy link
Contributor

@LuqiPan LuqiPan left a comment

Choose a reason for hiding this comment

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

LGTM

@turadg
Copy link
Member Author

turadg commented Jan 22, 2026

@Mergifyio refresh

@mergify
Copy link
Contributor

mergify bot commented Jan 22, 2026

refresh

✅ Pull request refreshed

Copy link
Member

@mhofman mhofman left a comment

Choose a reason for hiding this comment

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

Some questions I'd prefer to be addressed before merging.

Comment on lines -99 to -100
--- a/lib/reporters/tap.js
+++ b/lib/reporters/tap.js
Copy link
Member

Choose a reason for hiding this comment

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

Any reason to remove the tap reporting patch ? Are we no longer using the duration?

Copy link
Member Author

Choose a reason for hiding this comment

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

Any reason to remove the tap reporting patch ?

Like for any patch removal, to reduce maintenance costs.

Are we no longer using the duration?

That's my understanding. It was added for Datadog CI that we stopped a while ago. Even when we had it we didn't do much with it.

});

test.serial.failing('test durable first-crank hazard 3', async t => {
test.serial.skip('test durable first-crank hazard 3', async t => {
Copy link
Member

Choose a reason for hiding this comment

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

Why switch from failing to skip ?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't recall the specifics but I'm sure it's because it caused the job to fail. Reading the v6 changes I bet it's this one:

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

console.log(' → cleaning generated files');
try {
execSync("git clean -f '*.d.ts' '*.d.ts.map' '*.js'", {
execSync("git clean -f '*.d.ts' '*.d.ts.map' '*.js' '*.mts'", {
Copy link
Member

Choose a reason for hiding this comment

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

Curious why this change is in the PR?

Copy link
Member Author

Choose a reason for hiding this comment

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

The commit preceding it adds an .mjs file which results in an .mts file being built by prepack that must then be cleaned in postpack to get back to a clean checkout for the porcelain check

Copy link
Member

Choose a reason for hiding this comment

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

Oh it's an .d.mts that gets generated. I'm surprised it even needs the m modifier, that whole package has type: module already.

turadg and others added 14 commits January 22, 2026 08:46
Skip test demonstrating bug #9377 that causes unhandled rejection with Ava 6

Co-authored-by: turadg <21505+turadg@users.noreply.github.com>
Properly await and assert on the rejected promise in paramGovernance test

Co-authored-by: turadg <21505+turadg@users.noreply.github.com>
Ava 6 requires tests to exit cleanly or they will timeout. Added test.after.always hook to call shutdown.

Co-authored-by: turadg <21505+turadg@users.noreply.github.com>
Was consistently timing out in CI on Node 22.21.1 (passing on Node 20)
The test intentionally creates unhandled rejections to validate promise watcher behavior. Added handler to prevent Ava 6 from killing the process when these expected unhandled rejections occur.

Co-authored-by: turadg <21505+turadg@users.noreply.github.com>
@mergify mergify bot merged commit 0ecc6a4 into master Jan 22, 2026
140 of 155 checks passed
@mergify mergify bot deleted the ta/ava-6 branch January 22, 2026 22:20
turadg added a commit that referenced this pull request Jan 23, 2026
turadg added a commit that referenced this pull request Jan 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automerge:rebase Automatically rebase updates, then merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

upgrade Ava to v6

4 participants