Repository navigation
refactor: simplify pytest_configure, warn on era mismatch - #3702
Merged
Merged
Conversation
Running with a transaction era that doesn't match the cluster era is not a supported configuration. Warn about it at configure time, naming both eras, so unexpected failures are easier to explain.
Move report metadata collection into `_set_metadata` and the setup warnings into `_warn_on_setup`. Binding `config.stash[metadata_key]` to a local drops ~40 repetitions of the subscript chain. `_warn_on_setup` resolves the executable paths itself instead of reading them back from the metadata stash, so it does not depend on `_set_metadata` having run. It is now called only on the xdist controller, so the warnings are not repeated by every worker.
Contributor
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
No unresolved review issues were identified.
Review effort: Lite
Findings: None
What changed in this PR
Refactors pytest setup and adds warnings for transaction/cluster era mismatches.
Changes:
- Extracts metadata handling into
_set_metadata. - Extracts setup warnings into
_warn_on_setup. - Limits warnings to the xdist controller.
| File | Description |
|---|---|
cardano_node_tests/tests/conftest.py |
Refactored pytest configuration and added setup warnings. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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.
Two changes to
cardano_node_tests/tests/conftest.py.Warn when Tx era differs from cluster era
Running with a transaction era that doesn't match the cluster era is
not a supported configuration, but nothing said so - the run just
failed later in confusing ways.
pytest_configurenow logs a warningnaming both eras.
Split
pytest_configurepytest_configurehad grown to ~80 lines of mostlyconfig.stash[metadata_key][...] = .... Split into:_set_metadata(config)- report metadata. Bindsconfig.stash[metadata_key]to a local, dropping ~40 repetitions ofthe subscript chain.
_warn_on_setup()- the custom-path warnings plus the new erawarning. Resolves the executable paths with
shutil.whichitselfrather than reading them back from the metadata stash, so it has no
ordering dependency on
_set_metadata.pytest_configureis now the socket check, theskipallbail, andtwo calls.
Behavior change beyond the new warning:
_warn_on_setupruns only onthe xdist controller (
not hasattr(config, "workerinput")), so thewarnings are logged once instead of once per worker. Metadata
collection still runs everywhere, unchanged.