fix: guard all check_ functions on HAS_DBSYNC - #3603
Merged
Conversation
Add agent_docs/dbsync.md describing the dbsync and needs_dbsync markers and the pattern of using dbsync_utils.check_... functions without HAS_DBSYNC guards in test bodies. Document the testnets, long and smoke pytest markers in agent_docs/new_e2e_tests.md. Instruct in AGENTS.md to keep README.md brief and let user facing tooling handle incorrect usage.
Add the configuration.HAS_DBSYNC guard to the check_ functions that were missing it: check_address_reward, check_pool_off_chain_data, check_pool_off_chain_fetch_error, check_plutus_cost, check_plutus_costs and check_column_condition. The check_ functions are the high-level interface to db-sync meant to be called directly from tests, so all of them now return early when db-sync is not available. Remove the HAS_DBSYNC wrappers in test_pools.py that are now redundant, and make is_smash_running return early when smash is not configured.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR makes db-sync checks safe to call unconditionally from E2E tests by ensuring dbsync_utils.check_* entrypoints consistently short-circuit when db-sync isn’t configured, and updates tests/docs to match that intended usage pattern.
Changes:
- Add missing
configuration.HAS_DBSYNCguards to additionaldbsync_utils.check_*functions (returning early /Noneas appropriate). - Remove redundant
if configuration.HAS_DBSYNC:wrappers intest_pools.pyand rely on guardedcheck_*calls. - Improve agent-facing documentation around db-sync markers and recommended patterns (
agent_docs/dbsync.md), plus document common E2E markers (agent_docs/new_e2e_tests.md) and reinforce README brevity guidance (AGENTS.md).
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| cardano_node_tests/utils/smash_utils.py | Adds a configuration-based early return so SMASH checks are safely skipped when not configured. |
| cardano_node_tests/utils/dbsync_utils.py | Guards several check_* helpers on HAS_DBSYNC and adjusts return types/docstrings accordingly. |
| cardano_node_tests/tests/test_pools.py | Removes redundant db-sync availability guards and calls check_* helpers directly. |
| AGENTS.md | Adds explicit guidance to keep README brief and place detailed AI guidance in agent_docs/. |
| agent_docs/new_e2e_tests.md | Documents db-sync check guidance and introduces marker guidance for testnets/long/smoke. |
| agent_docs/dbsync.md | New doc describing db-sync markers and the “call check_* unguarded” pattern. |
💡 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.
All
check_...functions indbsync_utilsare the high-level interface to db-sync meant to be called directly from tests, but six of them (check_address_reward,check_pool_off_chain_data,check_pool_off_chain_fetch_error,check_plutus_cost,check_plutus_costs,check_column_condition) were missing theHAS_DBSYNCguard and would fail when db-sync is not available. All of them now return early, so tests no longer need to wrap the calls inif configuration.HAS_DBSYNC:. The now redundant wrappers were removed fromtest_pools.py, andis_smash_runningreturns early when smash is not configured.Also add AI agent documentation for the above:
agent_docs/dbsync.mddescribes the db-sync markers and the pattern of callingcheck_...functions unguarded, andagent_docs/new_e2e_tests.mdnewly documents thetestnets,longandsmokemarkers. AGENTS.md instructs to keep README.md brief and let user-facing tooling handle incorrect usage.