Repository navigation
test(labs): isolate a test by naming it *.isolate.test.ts - #25559
Merged
Merged
Conversation
A test that binds a port has to run in its own container, and which tests those were was a list of package prefixes in labs' test_cmds. A list only works if whoever adds a test in a new package knows to extend it, and an unisolated test that spawns anvil passes until the day it races another one. Two were already in that position. cli/src/cmds/l1/attester_exit, added this month, fails with "Address already in use (os error 98)" before reaching an assertion when it meets another anvil in the same window. It failed the x-full run on #25362, a bb and bb.js migration it has nothing to do with; the two anvils it lost to were ethereum/blob_kzg_warmup, which is isolated because ethereum is listed, and epoch-cache/epoch_cache.integration, which is not listed and has been unisolated since April. The suffix says it where someone writes the test rather than somewhere else that has to be remembered. The package list stays, so nothing loses the isolation it has today. Rides as a labs patch; upstream to aztec-node separately. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NuUzj3qpkpJor6GMpWB4T4
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A test that binds a port has to run in its own container. Which tests those were was a list of package prefixes in labs'
test_cmds:A list only works if whoever adds a test in a new package knows to extend it, and an unisolated test that spawns anvil passes until the day it races another one.
Two tests were already in that position
cli/src/cmds/l1/attester_exit, added on 18 September, fails before reaching any assertion:three retries, all the same. It failed the
x-full-no-test-cacherun on #25362, a bb and bb.js migration it has nothing to do with.no-test-cacheis what surfaced it: every test executes in the same window rather than most being served from cache, which is exactly when two unisolated anvils meet.The two it lost to were
ethereum/src/test/blob_kzg_warmup, isolated becauseethereumis listed, andepoch-cache/src/epoch_cache.integration, which is not listed and has been unisolated since April. Neither is at fault;cliandepoch-cachewere simply never on the list.The change
Name a test
*.isolate.test.tsand it getsISOLATE=1, wherever it lives. The two above are renamed accordingly.The package list stays, so nothing loses the isolation it has today. Prefer the suffix for anything new; an entry can leave the list later by renaming its tests. The point is that the requirement is stated where someone writes the test, instead of somewhere else they have to remember.
Not a default-port change: fixed ports with full isolation is the established preference here, and ephemeral ports have brought their own flakiness before.
Verified
.isolate.test.tsis not caught by the.bench.test.tsskip.bootstrap.shparses with extglob and globstar, which is how ci3 runs it. Plainbash -nreports a syntax error on the!(...)glob both before and after this change.Landing
Rides as a labs patch,
labs-patches/0009, so CI gets it without waiting on a pin bump. It should be upstreamed to aztec-node and the patch dropped once the pin passes it. Worth a look from whoever ownscli, since it is their test failing other people's PRs.