Skip to content

test: update test data to new block and consolidate fixtures under test_data/#108

Merged
flyq merged 6 commits intomainfrom
liquan/update_test_data
Mar 30, 2026
Merged

test: update test data to new block and consolidate fixtures under test_data/#108
flyq merged 6 commits intomainfrom
liquan/update_test_data

Conversation

@flyq
Copy link
Copy Markdown
Member

@flyq flyq commented Mar 25, 2026

Summary

Replace test data fixtures with a new block range (349–370) and consolidate all test assets under test_data/.

  • Remove old test block JSON files (blocks 1139–1160) and their corresponding SALT/MPT witness files, and add new ones for blocks 349–370.
  • Move genesis/genesis.json into test_data/genesis.json so all test fixtures live under a single directory, and update the TEST_GENESIS_FILE path in main.rs accordingly.
  • Update test_data/contracts.txt to match the new block range's contract set.

Details

Test data replacement (blocks 1139–1160 → 349–370)

  • Deleted 22 block JSON files under test_data/blocks/ for the old range (1139–1160).
  • Added 22 new block JSON files for the new range (349–370).
  • Deleted 21 pairs of .salt and .mpt witness files under test_data/stateless/witness/ for the old range.
  • Added 21 pairs of .salt and .mpt witness files for the new range.

Genesis config relocation

  • Moved genesis/genesis.jsontest_data/genesis.json (with minor content updates to match the new chain state).
  • Updated TEST_GENESIS_FILE constant in bin/stateless-validator/src/main.rs from ../../genesis/genesis.json to ../../test_data/genesis.json.

Contracts list update

  • Trimmed test_data/contracts.txt to the reduced set of contracts relevant to the new block range.

Test plan

  • Run cargo test -p stateless-validator to verify integration tests pass with the new test data.
  • Run cargo test to confirm no other tests are broken by the genesis file relocation.
  • Verify that the genesis/ directory is no longer referenced anywhere in the codebase.

@flyq flyq requested a review from Troublor as a code owner March 25, 2026 12:10
@claude
Copy link
Copy Markdown

claude bot commented Mar 25, 2026

The PR currently has no labels applied. Based on the changes — updating test data fixtures (blocks 1139–1160 → 349–370), consolidating genesis config under test_data/, and updating contracts.txt — the enhancement label seems most appropriate, as this improves the test infrastructure by consolidating fixtures under a single directory.

There is no dedicated test label in the repo's label set, so enhancement is the closest match from the available labels.

@claude
Copy link
Copy Markdown

claude bot commented Mar 25, 2026

The code change (path update in main.rs) is correct. One issue: the PR description says to verify genesis/ is no longer referenced anywhere in the codebase, but two places remain:

  • README.md:159--genesis-file ./genesis/genesis.json (example command)
  • README.md:194--genesis-file ./genesis/updated-genesis.json (example command)
  • AGENTS.md:44 — lists genesis/ as an additional directory in the project structure

The README examples should be updated to either use ./test_data/genesis.json or a generic placeholder path, and AGENTS.md should remove genesis/ from the directory list since it no longer exists.

@flyq flyq added the enhancement New feature or request label Mar 25, 2026
@claude
Copy link
Copy Markdown

claude bot commented Mar 25, 2026

The current label enhancement ("New feature or request") does not match the nature of this PR.

This PR updates test data fixtures and consolidates them under test_data/ — there is no new feature or user-facing change. The PR title itself uses the test: conventional-commit prefix, which confirms it is a test/maintenance change.

None of the available labels are a perfect fit (the repo has no test or maintenance label), but enhancement should be removed as it is misleading. Consider adding a test label to the repository for this class of change, or simply leaving the PR unlabeled.

Comment thread README.md
@claude
Copy link
Copy Markdown

claude bot commented Mar 25, 2026

The consolidation looks good and the code change is correct. One minor issue: the README replaced two concrete paths (./genesis/genesis.json, ./genesis/updated-genesis.json) with generic /path/to/... placeholders. Since the genesis file is still distributed in the repo (now at test_data/genesis.json), the example should point there — see inline comment.

@flyq flyq removed the enhancement New feature or request label Mar 25, 2026
@claude
Copy link
Copy Markdown

claude bot commented Mar 25, 2026

The PR currently has no labels, but the documentation label appears applicable: AGENTS.md is updated to reflect the new test fixture paths (e.g., the genesis file relocation). The primary changes are test data updates, but there is no test label available in the repo. Given what's available, documentation should be added to reflect the AGENTS.md changes.

@flyq flyq force-pushed the liquan/update_test_data branch from 3f52bb8 to 29d2e0e Compare March 25, 2026 13:41
@claude
Copy link
Copy Markdown

claude bot commented Mar 25, 2026

LGTM. Test data updated cleanly, genesis config consolidated under test_data/, all references updated consistently across AGENTS.md, README.md, and main.rs. Resolved the prior thread — it was already addressed by this PR.

Troublor
Troublor previously approved these changes Mar 30, 2026
vincent-k2026
vincent-k2026 previously approved these changes Mar 30, 2026
@flyq flyq dismissed stale reviews from vincent-k2026 and Troublor via 2bf1d33 March 30, 2026 06:43
Comment thread README.md Outdated
Comment thread README.md Outdated
@claude
Copy link
Copy Markdown

claude bot commented Mar 30, 2026

The refactoring is clean — renaming RpcModuleContextTestData with a load(data_dir) constructor and splitting into synthetic/mainnet subdirectories is a clear improvement.

One bug: the README examples were updated to ./test_data/genesis.json and ./test_data/updated-genesis.json, but neither path exists after the restructure (genesis is now at test_data/synthetic/genesis.json / test_data/mainnet/genesis.json). Left inline suggestions to use generic placeholders since those code blocks are production documentation, not test instructions.

@flyq flyq force-pushed the liquan/update_test_data branch from 2bf1d33 to 4bb25ce Compare March 30, 2026 06:54
@claude
Copy link
Copy Markdown

claude bot commented Mar 30, 2026

The refactor is clean overall — TestData::load() is a clear improvement over the old create_rpc_module_context(), and the synthetic/mainnet split is a good structural change.

Two existing inline threads are still unresolved and need addressing:

  1. README line ~156 (PRRT_kwDOPRLqcM53mY5O): ./test_data/mainnet/genesis.json is a test fixture path in production docs. Use /path/to/genesis.json as a generic placeholder.
  2. README line ~191 (PRRT_kwDOPRLqcM53mY6O): /path/to//updated-genesis.json has a double slash — fix to /path/to/updated-genesis.json.

One new concern: the Foundry installation in coverage.yml has no explanation. Nothing in this PR introduces Foundry-dependent tests — if Foundry is required by an existing test, add a comment explaining why; otherwise remove it.

Comment thread .github/workflows/coverage.yml
fn setup_test_db(data: &TestData) -> Result<Arc<ValidatorDB>> {
let temp_dir = tempfile::tempdir()?;
let validator_db = ValidatorDB::new(temp_dir.path().join(VALIDATOR_DB_FILENAME))?;
std::mem::forget(temp_dir);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment explaining this intentional leak was removed. Without it, std::mem::forget here looks like a bug. Please restore it:

Suggested change
std::mem::forget(temp_dir);
// Intentionally leak the temp dir — ValidatorDB holds a path into it.
// The OS will clean it up when the test process exits.
std::mem::forget(temp_dir);

@claude
Copy link
Copy Markdown

claude bot commented Mar 30, 2026

Overall this is a clean refactor. The test data restructuring into test_data/synthetic/ and test_data/mainnet/ is well-organized, and the TestData struct / TestData::load consolidation improves readability significantly.

One issue to fix:

  • std::mem::forget(temp_dir) in setup_test_db lost the comment that explains why the leak is intentional. Without it the line looks like a bug. See inline comment.

Previous threads resolved:

  • README --genesis-file path examples already updated to /path/to/genesis.json
  • Coverage workflow now includes the # mega-evm coverage tests require Foundry comment ✓

@claude
Copy link
Copy Markdown

claude bot commented Mar 30, 2026

Good work overall. A few things to address:

PR description is incomplete. The summary only describes test data replacement and genesis relocation, but the PR also includes a substantial code refactoring (RpcModuleContextTestData with a load(data_dir) factory) and a new mainnet_integration_test. The description also says genesis moves to test_data/genesis.json but it actually lands in test_data/synthetic/genesis.json. Please update the summary to reflect what actually changed — the squash commit message will come from this description.

test_data/contracts.txt is now orphaned. The code reads contracts from {data_dir}/contracts.txt (i.e., test_data/synthetic/contracts.txt and test_data/mainnet/contracts.txt). The top-level test_data/contracts.txt is modified in this PR but no longer referenced by any code. It should be deleted rather than modified.

Foundry action version not pinned. See inline comment.

The TestData::load refactoring is clean and the pattern of parametrising by data_dir is the right call. The mainnet_integration_test calling validate_block directly (skipping the mock RPC layer) is the correct approach for non-contiguous blocks.


# mega-evm coverage tests require Foundry (solc/forge) for compilation.
- name: Install Foundry
uses: foundry-rs/foundry-toolchain@v1
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pin this to a commit SHA for reproducibility and supply-chain safety. @v1 is a mutable tag that can be updated by the action's author at any time.

Suggested change
uses: foundry-rs/foundry-toolchain@v1
uses: foundry-rs/foundry-toolchain@82251fa6f7eb7e9e5a29c9d3e2b7b72e44f1dee0 # v1

(Use the current SHA for v1; verify via git ls-remote https://github.com/foundry-rs/foundry-toolchain refs/tags/v1.)

@flyq flyq merged commit bac6250 into main Mar 30, 2026
28 checks passed
@flyq flyq deleted the liquan/update_test_data branch March 30, 2026 07:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants