Repository navigation
fix(tests): avoid races and leaked state in tests - #3739
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The KES tests still read POOL2’s shared files without declaring that pool as in use, leaving a concurrency race.
Review effort: Balanced
Findings: 1
Open (1)
What changed in this PR
Improves E2E test isolation by preventing shared-cluster races and restoring modified state.
Changes:
- Adds cluster/resource isolation for transaction and Plutus tests.
- Protects shared KES counters and restores environment state.
- Requests cluster respins earlier and performs epoch checks before mutation.
| File | Description |
|---|---|
tests_plutus/test_delegation.py |
Respins when script stake registration leaks. |
tests_dijkstra/test_bls_rotation.py |
Checks epoch duration before requesting respin. |
test_rollback.py |
Requests respin before cluster splitting. |
test_kes.py |
Copies cold counters before negative commands. |
test_env_network_id.py |
Restores environment and CLI arguments via monkeypatch. |
test_chain_transactions.py |
Locks the cluster and marks the test as heavy. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- test_tx_chaining: lock the whole cluster instance. The test checks how many blocks the chained Txs take, so Txs of other tests must not take the block space, and the chained Txs fill the mempool for other tests. - Plutus script stake address tests: respin the cluster instance when a test leaves the script stake address registered, otherwise all the following tests using the script are skipped. - KES negative tests: take a pool marked as "in use" instead of a hardcoded one, and pass a copy of its cold counter file to `issue-op-cert`, as the command would increment the shared counter if it succeeded. - test_rotation_pair_keeps_voting: check the epoch length in the fixture, before the respin is requested. - test_permanent_fork: request the respin before splitting the cluster, so it is requested even when the split fails half-way. - set_network_id_env: restore the original `CARDANO_NODE_NETWORK_ID` value instead of deleting it.
mkoura
force-pushed
the
misc_test_races
branch
from
October 5, 2026 15:50
497c796 to
3ef5682
Compare
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.

issue-op-cert, as the command would increment the shared counter if it succeeded.CARDANO_NODE_NETWORK_IDvalue instead of deleting it.