-
Notifications
You must be signed in to change notification settings - Fork 130
fix(l1, l2): cargo test compilation at workspace level #4916
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Fix cargo test compilation by moving benchmarks out of cmd/ethrex into a dedicated benches crate and updating related paths and CI.
- Move bench targets from cmd/ethrex to a new benches crate and remove the unused import_blocks benchmark
- Update include paths in build_block_benchmark.rs now that it lives under benches
- Add benches to workspace and update the CI workflow to run the benchmark from the new location
Reviewed Changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| cmd/ethrex/bench/import_blocks_benchmark.rs | Remove unused benchmark to eliminate dev-dep coupling. |
| cmd/ethrex/Cargo.toml | Drop bench-related dev-deps and bench targets; formatting tweaks for features. |
| benches/benches/build_block_benchmark.rs | Adjust include paths to fixtures after relocating the bench into its own crate. |
| benches/Cargo.toml | New crate for benchmarks, with dev-deps and bench target configuration. |
| Cargo.toml | Add benches crate to the workspace members. |
| .github/workflows/pr_perf_build_block_bench.yml | Update benchmark workflow working directory to the new crate (note: see comment below). |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| uses: boa-dev/criterion-compare-action@v3 | ||
| with: | ||
| cwd: "cmd/ethrex/bench" | ||
| cwd: "benches/benches" |
Copilot
AI
Oct 17, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cwd should point to the crate root (where Cargo.toml resides). The benches crate root is benches, not benches/benches. Update to cwd: 'benches' so cargo bench runs correctly.
| cwd: "benches/benches" | |
| cwd: "benches" |
|
|
||
| async fn setup_genesis(accounts: &Vec<Address>) -> (Store, Genesis) { | ||
| let storage_path = tempfile::TempDir::new().unwrap(); | ||
| if std::fs::exists(&storage_path).unwrap_or(false) { |
Copilot
AI
Oct 17, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
std::fs::exists does not exist. Use std::fs::try_exists(&storage_path).unwrap_or(false) or Path::new(&storage_path).exists().
| if std::fs::exists(&storage_path).unwrap_or(false) { | |
| if storage_path.path().exists() { |
Lines of code reportTotal lines added: Detailed view |
Motivation
cargo testat the workspace level or incmd/ethrexis not compiling. This is becauseethrex-l2-rpcis adev-dependencie.This issue uncovered a bigger issue, that is the existence of
benchesinsidecmd/ethrex.Description
This PR solves the compilation error by moving benchmarks to a separate crate,
benches. It also removes an unsued benchmark.