refactor: split framework tests into own directory - #165
Merged
Conversation
Martin proposed separating our own framework/helper unit tests from the real product tests (node_sync, db_sync, mainnet_tx_count) so the two are not mixed together in one directory, mirroring how Cardano itself keeps this distinction. Moved the five tests that only exercise our own utility code, with no live node or db-sync dependency, into a new sync_tests/framework_tests/ package: - test_ci_step_emit.py - test_config_downloads.py - test_disk_cleanup.py - test_request_with_retry.py - test_start_node.py sync_tests/tests/ keeps conftest.py and the real product tests that need a live node/db-sync session. pyproject.toml testpaths and the ANN per-file-ignore now cover both directories. No CI workflow changes needed: node_sync_test and db_sync_full_sync target files/markers that did not move. Pure file move, no behavior change. Verified with ruff, ruff format, mypy, and pytest (39 collected, both dirs run standalone and together).
Checked cardano-node-tests directly instead of guessing: its framework_tests/ is a top-level directory, sibling to cardano_node_tests/, not nested inside the product package. The previous commit nested it under sync_tests/ instead, which does not match that structure. Moved framework_tests/ up to repo root to match. Updated pyproject.toml accordingly: testpaths, the ANN per-file-ignore, pyrefly project_includes, and added an explicit setuptools packages.find include so framework_tests is not swept into the installable package, mirroring cardano-node-tests exactly. Also fixed a real bug this move exposed: test_ci_step_emit.py computed _REPO_ROOT as parents[2], which assumed the old two levels of nesting (sync_tests/tests/). At one level deep it must be parents[1]. Without this fix the e2e subprocess test fails because PYTHONPATH points one directory too high. Verified: ruff, ruff format, and mypy pass; pytest framework_tests standalone (20 passed, matching how node-tests runs it in code_checks.yaml) and the combined run (20 passed, 19 deselected) both pass.
mkoura
approved these changes
Jul 30, 2026
OlufemiAdeOlusile
added a commit
that referenced
this pull request
Jul 30, 2026
Framework tests moved to a top-level framework_tests/ directory in #165. This workflow still targeted sync_tests/tests/ with a marker-exclusion filter, which after that move would just select zero tests, silently. Now runs pytest framework_tests directly, no filter needed, same as how cardano-node-tests invokes its equivalent in code_checks.yaml. Verified by simulating the exact CI steps in a fresh venv: fresh install via pip install -e ., then pytest framework_tests -v, 20 passed.
OlufemiAdeOlusile
added a commit
that referenced
this pull request
Jul 30, 2026
Addresses both review comments: Martin: differentiate framework tests from actual sync tests. These four files are pure unit tests of our own code (log parsing, metrics extraction, graph generation) with no live node/db-sync dependency, so they belong in framework_tests/ (added in #165), not sync_tests/tests/: - test_log_analyzer.py - test_metrics_extractor_dbsync.py - test_metrics_extractor_node.py - test_sync_static_graphs.py Copilot: the comment explaining why epoch 3 is kept in test_epoch_timings_keeps_entries_with_valid_duration_or_blocks was contradictory. Reworded to state directly that has_valid_blocks treats any blocks_count >= 0, including 0, as valid, which is why epoch 3 survives despite duration_sec=0. Verified: ruff, ruff format, and mypy pass (47 source files); full suite collects 72 tests, 53 passed, 8 skipped (unrelated snapshot tests missing CLI args), 11 deselected (live-infra markers).
3 tasks
OlufemiAdeOlusile
added a commit
that referenced
this pull request
Aug 3, 2026
Self-contained unit tests belong alongside the other framework tests (no conftest fixtures, no CLI options), matching the layout PR #165 already established for this category of test.
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
2 participants
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.
Summary
Moves our framework/helper unit tests into a top-level
framework_tests/directory, matching cardano-node-tests.sync_tests/tests/keeps the real node/db-sync tests.Moved:
Updated pyproject.toml (testpaths, ruff ignores, pyrefly includes,
package excludes) to match.
Fixed a bug the move exposed: one test had a hardcoded path depth
that broke once the file moved up a level.
Test plan