feat: move integration tests#1174
Merged
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR extracts tests into a new crate Changes
Sequence Diagram(s)(Skipped — changes are reorganizational and do not introduce new multi-component runtime control flow.) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/chain-tests/src/packing/remote_packing.rs`:
- Around line 34-38: Replace the bare #[tokio::test] attribute on the async test
function packing_worker_full_node_test with the tracing-aware macro attribute
#[test_log::test(tokio::test)] so tracing (RUST_LOG + initialize_tracing()) is
captured correctly; locate the packing_worker_full_node_test definition and
update its attribute accordingly and leave the existing RUST_LOG set and
initialize_tracing() calls in place.
ℹ️ Review info
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (101)
Cargo.tomlcrates/chain-tests/Cargo.tomlcrates/chain-tests/src/api/api.rscrates/chain-tests/src/api/client.rscrates/chain-tests/src/api/external_api.rscrates/chain-tests/src/api/hardfork_tests.rscrates/chain-tests/src/api/mod.rscrates/chain-tests/src/api/pricing_endpoint.rscrates/chain-tests/src/api/supply_endpoint.rscrates/chain-tests/src/api/tx.rscrates/chain-tests/src/api/tx_commitments.rscrates/chain-tests/src/api/tx_duplicates.rscrates/chain-tests/src/block_production/analytics.rscrates/chain-tests/src/block_production/basic_contract.rscrates/chain-tests/src/block_production/block_index.rscrates/chain-tests/src/block_production/block_production.rscrates/chain-tests/src/block_production/block_rebuilding.rscrates/chain-tests/src/block_production/block_validation.rscrates/chain-tests/src/block_production/difficulty_adjustment.rscrates/chain-tests/src/block_production/mod.rscrates/chain-tests/src/block_production/reset_seed.rscrates/chain-tests/src/block_production/test_double_spend.rscrates/chain-tests/src/block_production/testing_primitives.rscrates/chain-tests/src/block_production/treasury_tracking.rscrates/chain-tests/src/block_production/unpledge_refund.rscrates/chain-tests/src/block_production/unstake_refund.rscrates/chain-tests/src/data_sync/mod.rscrates/chain-tests/src/data_sync/sync_partition_data_tests.rscrates/chain-tests/src/ema_pricing/ema_pricing.rscrates/chain-tests/src/ema_pricing/mod.rscrates/chain-tests/src/external/api.rscrates/chain-tests/src/external/block_production.rscrates/chain-tests/src/external/mod.rscrates/chain-tests/src/external/programmable_data_basic.rscrates/chain-tests/src/external/readme.mdcrates/chain-tests/src/external/sync_partition_data_remote.rscrates/chain-tests/src/external/utils/api.rscrates/chain-tests/src/external/utils/client.rscrates/chain-tests/src/external/utils/mod.rscrates/chain-tests/src/external/utils/monitoring.rscrates/chain-tests/src/external/utils/signer.rscrates/chain-tests/src/external/utils/transactions.rscrates/chain-tests/src/external/utils/types.rscrates/chain-tests/src/external/utils/utils.rscrates/chain-tests/src/integration/cache_service.rscrates/chain-tests/src/integration/data_size.rscrates/chain-tests/src/integration/mod.rscrates/chain-tests/src/lib.rscrates/chain-tests/src/multi_node/api_ingress_validation.rscrates/chain-tests/src/multi_node/ema_forks.rscrates/chain-tests/src/multi_node/epoch_replay.rscrates/chain-tests/src/multi_node/fork_recovery.rscrates/chain-tests/src/multi_node/fork_recovery_epoch.rscrates/chain-tests/src/multi_node/mempool_tests.rscrates/chain-tests/src/multi_node/mod.rscrates/chain-tests/src/multi_node/peer_discovery.rscrates/chain-tests/src/multi_node/peer_mining.rscrates/chain-tests/src/multi_node/reth_restart.rscrates/chain-tests/src/multi_node/sync_chain_state.rscrates/chain-tests/src/multi_node/validation.rscrates/chain-tests/src/packing/mod.rscrates/chain-tests/src/packing/remote_packing.rscrates/chain-tests/src/partition_assignments/mod.rscrates/chain-tests/src/partition_assignments/sm_reassignment_tests.rscrates/chain-tests/src/programmable_data/basic.rscrates/chain-tests/src/programmable_data/mod.rscrates/chain-tests/src/promotion/data_promotion_basic.rscrates/chain-tests/src/promotion/data_promotion_double.rscrates/chain-tests/src/promotion/mod.rscrates/chain-tests/src/promotion/promotion_with_multiple_proofs.rscrates/chain-tests/src/startup/auto_stake.rscrates/chain-tests/src/startup/genesis.rscrates/chain-tests/src/startup/mod.rscrates/chain-tests/src/startup/startup.rscrates/chain-tests/src/synchronization/mod.rscrates/chain-tests/src/term_ledger_expiry/mod.rscrates/chain-tests/src/term_ledger_expiry/perm_refund.rscrates/chain-tests/src/utils.rscrates/chain-tests/src/validation/blobs_rejected.rscrates/chain-tests/src/validation/data_tx_pricing.rscrates/chain-tests/src/validation/ingress_proof_reanchor_dedup.rscrates/chain-tests/src/validation/ingress_proof_validation.rscrates/chain-tests/src/validation/invalid_perm_fee_refund.rscrates/chain-tests/src/validation/ledger_expiry_with_unstake.rscrates/chain-tests/src/validation/mempool_gossip_shape.rscrates/chain-tests/src/validation/mempool_ingress_proof_dedup.rscrates/chain-tests/src/validation/mod.rscrates/chain-tests/src/validation/poa_cases.rscrates/chain-tests/src/validation/unpledge_partition.rscrates/chain-tests/src/validation/unstake_edge_cases.rscrates/chain/Cargo.tomlcrates/chain/tests/block_production/mod.rscrates/chain/tests/data_sync/mod.rscrates/chain/tests/integration/mod.rscrates/chain/tests/mod.rscrates/chain/tests/multi_node/mod.rscrates/chain/tests/packing/mod.rscrates/chain/tests/partition_assignments/mod.rscrates/chain/tests/programmable_data/mod.rscrates/chain/tests/promotion/mod.rscrates/chain/tests/startup/mod.rs
💤 Files with no reviewable changes (11)
- crates/chain/tests/data_sync/mod.rs
- crates/chain/tests/packing/mod.rs
- crates/chain/tests/integration/mod.rs
- crates/chain/tests/multi_node/mod.rs
- crates/chain/Cargo.toml
- crates/chain/tests/partition_assignments/mod.rs
- crates/chain/tests/startup/mod.rs
- crates/chain/tests/block_production/mod.rs
- crates/chain/tests/mod.rs
- crates/chain/tests/promotion/mod.rs
- crates/chain/tests/programmable_data/mod.rs
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@xtask/src/main.rs`:
- Line 385: The Clippy invocation under the LocalChecks path currently calls
run_command(Commands::Clippy { args: vec![] }, sh) without the required flags;
update the args passed to Commands::Clippy so the command includes
"--workspace", "--tests", and "--all-targets" (e.g., populate the args vector
with these strings) so run_command executes "cargo clippy --workspace --tests
--all-targets"; modify the construction of Commands::Clippy in the LocalChecks
branch to include those flags.
| }, | ||
| sh, | ||
| )?; | ||
| run_command(Commands::Clippy { args: vec![] }, sh)?; |
Contributor
There was a problem hiding this comment.
LocalChecks should include --all-targets in Clippy invocation.
On Line 385, LocalChecks runs clippy without adding --all-targets, so this path does not fully enforce the required pre-PR lint gate.
Proposed fix
- run_command(Commands::Clippy { args: vec![] }, sh)?;
+ run_command(
+ Commands::Clippy {
+ args: vec!["--all-targets".to_string()],
+ },
+ sh,
+ )?;As per coding guidelines: "Run cargo clippy --workspace --tests --all-targets before pushing or creating PRs".
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| run_command(Commands::Clippy { args: vec![] }, sh)?; | |
| run_command( | |
| Commands::Clippy { | |
| args: vec!["--all-targets".to_string()], | |
| }, | |
| sh, | |
| )?; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@xtask/src/main.rs` at line 385, The Clippy invocation under the LocalChecks
path currently calls run_command(Commands::Clippy { args: vec![] }, sh) without
the required flags; update the args passed to Commands::Clippy so the command
includes "--workspace", "--tests", and "--all-targets" (e.g., populate the args
vector with these strings) so run_command executes "cargo clippy --workspace
--tests --all-targets"; modify the construction of Commands::Clippy in the
LocalChecks branch to include those flags.
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
xtask/src/main.rs (1)
324-331:⚠️ Potential issue | 🟠 MajorAdd
--all-targetsto clippy in both LocalChecks paths.Line 330 (
clippy --fix) and Line 385 (LocalChecks clippy call) currently do not enforce--all-targets. With--fix, the later verification block is skipped, so this can miss lint failures outside tests.Proposed fix
cmd!( sh, - "cargo clippy --fix --allow-dirty --allow-staged --workspace --tests {args...}" + "cargo clippy --fix --allow-dirty --allow-staged --workspace --tests --all-targets {args...}" ) .remove_and_run()?; @@ - run_command(Commands::Clippy { args: vec![] }, sh)?; + run_command( + Commands::Clippy { + args: vec!["--all-targets".to_string()], + }, + sh, + )?;As per coding guidelines: "Run
cargo clippy --workspace --tests --all-targetsbefore pushing or creating PRs".Also applies to: 373-385
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@xtask/src/main.rs` around lines 324 - 331, Update the two clippy invocations to include the --all-targets flag: modify the cmd! call that currently runs "cargo clippy --fix --allow-dirty --allow-staged --workspace --tests {args...}" (inside the RUSTFLAGS guard) and the other clippy invocation in LocalChecks (the second cmd! that runs clippy between lines ~373-385) to add "--all-targets" so they become "cargo clippy ... --workspace --tests --all-targets ..."; ensure you update both places where the clippy command string is built so clippy runs against all targets.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@xtask/src/main.rs`:
- Around line 324-331: Update the two clippy invocations to include the
--all-targets flag: modify the cmd! call that currently runs "cargo clippy --fix
--allow-dirty --allow-staged --workspace --tests {args...}" (inside the
RUSTFLAGS guard) and the other clippy invocation in LocalChecks (the second cmd!
that runs clippy between lines ~373-385) to add "--all-targets" so they become
"cargo clippy ... --workspace --tests --all-targets ..."; ensure you update both
places where the clippy command string is built so clippy runs against all
targets.
roberts-pumpurs
added a commit
that referenced
this pull request
Mar 9, 2026
Resolve conflicts from master's node startup simplification (#1169), test migration (#1174), and other refactors while preserving all PD (programmable data) features. Key resolutions: - Port PD chunk channels and services into master's new node_lifecycle() async architecture (replaces old init_services_thread + init_reth_thread) - Keep HEAD's modular packing_service sub-module structure, apply master's heavy4_ test rename - Merge test files into crates/chain-tests with PD modules and master's visibility/naming changes - Move alloy-consensus/network/signer-local to chain-tests deps (used by utils.rs lib code) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This was referenced Mar 16, 2026
3 tasks
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.
Describe the changes
Moves tests currently in chain/tests to chain-tests/src, to avoid the problems with integration tests and cfg!(test)
Checklist
Summary by CodeRabbit
Chores
Refactor