fix(test): speed up debug and codecov tests #6340
Conversation
WalkthroughConsolidates coverage generation into a Makefile target and sets RUSTFLAGS in CI, adjusts dev-profile optimization/debug settings in Cargo manifests, removes a CI debug-build skip so RPC snapshot tests run on CI, and adds two test-scoped helpers to detect debug/CI environment. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Potential focal points:
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
Comment |
hanabi1224
requested review from
LesnyRumcajs and
akaladarshi
and removed request for
a team
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/utils/mod.rs (1)
172-183: Removeis_debug_build()or integrate it if intentional.The
is_debug_build()function at line 173-176 is not used anywhere in the codebase. Theis_ci()function is actually used insrc/tool/subcommands/api_cmd/test_snapshot.rs:220, so the#[allow(dead_code)]on it is unnecessary. Either removeis_debug_build()if it's not needed, or remove the#[allow(dead_code)]fromis_ci()since it serves a real purpose.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.github/workflows/coverage.yml(2 hunks)Cargo.toml(1 hunks)Makefile(1 hunks)interop-tests/Cargo.toml(1 hunks)src/tool/subcommands/api_cmd/test_snapshot.rs(0 hunks)src/utils/mod.rs(1 hunks)
💤 Files with no reviewable changes (1)
- src/tool/subcommands/api_cmd/test_snapshot.rs
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5930
File: build.rs:64-77
Timestamp: 2025-08-13T09:43:20.301Z
Learning: hanabi1224 prefers hard compile-time errors in build scripts rather than runtime safeguards or collision detection, believing it's better to fail fast and fix root causes of issues like malformed snapshot names.
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 6057
File: src/cli/subcommands/f3_cmd.rs:0-0
Timestamp: 2025-09-09T10:37:17.947Z
Learning: hanabi1224 prefers having default timeouts (like 10m for --no-progress-timeout) to prevent commands from hanging indefinitely, even when the timeout flag isn't explicitly provided by users. This fail-fast approach is preferred over requiring explicit flag usage.
📚 Learning: 2025-08-07T13:39:36.962Z
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5886
File: interop-tests/src/tests/go_app/gen.go:88-90
Timestamp: 2025-08-07T13:39:36.962Z
Learning: Skip reviewing auto-generated files marked with "Generated by rust2go. Please DO NOT edit this C part manually." as these should not be manually edited and any issues should be addressed in the code generation tool or source templates instead.
Applied to files:
interop-tests/Cargo.toml
📚 Learning: 2025-08-08T12:10:45.218Z
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5867
File: src/ipld/util.rs:553-558
Timestamp: 2025-08-08T12:10:45.218Z
Learning: Forest pins Rust toolchain to 1.89.0 via rust-toolchain.toml; features stabilized in 1.88 (e.g., let-chains) are acceptable in this codebase.
Applied to files:
interop-tests/Cargo.toml
📚 Learning: 2025-08-07T13:39:15.107Z
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5886
File: interop-tests/src/tests/go_app/gen.go:29-29
Timestamp: 2025-08-07T13:39:15.107Z
Learning: Auto-generated files like those created by rust2go (indicated by "Generated by rust2go. Please DO NOT edit this C part manually." comment) in the Forest project should be skipped during code review as they are not intended for manual editing.
Applied to files:
interop-tests/Cargo.toml
📚 Learning: 2025-08-08T12:10:45.218Z
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5867
File: src/ipld/util.rs:553-558
Timestamp: 2025-08-08T12:10:45.218Z
Learning: Forest project targets Rust stable >=1.89; features stabilized in 1.88 like let-chains are acceptable in this codebase.
Applied to files:
interop-tests/Cargo.toml
📚 Learning: 2025-09-11T16:03:14.328Z
Learnt from: akaladarshi
Repo: ChainSafe/forest PR: 6000
File: src/tool/subcommands/api_cmd/state_decode_params_tests/miner.rs:4-9
Timestamp: 2025-09-11T16:03:14.328Z
Learning: In the Forest codebase's state_decode_params_tests module, common imports like `to_vec`, `TokenAmount`, `Cid`, etc. are centralized in the mod.rs file and made available to submodules through `use super::*;`, eliminating the need for duplicate imports in individual test files.
Applied to files:
src/utils/mod.rs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Coverage
- GitHub Check: Build forest binaries on Linux AMD64
- GitHub Check: tests
- GitHub Check: tests-release
- GitHub Check: Build MacOS
- GitHub Check: cargo-publish-dry-run
- GitHub Check: Build Ubuntu
- GitHub Check: All lint checks
🔇 Additional comments (4)
interop-tests/Cargo.toml (1)
44-54: LGTM! Profile optimizations mirror the main Cargo.toml.The dev profile configuration reduces debug info overhead (
line-tables-only,debug = falsefor dependencies) and enablesopt-level = 1for dependencies to speed up test execution. These settings align with Cargo's recommended build performance optimizations.Cargo.toml (1)
271-275: LGTM! Adding opt-level = 1 for dependencies is a sensible trade-off.This enables basic optimizations for dependencies while keeping the main crate unoptimized for fast compilation and debuggability. This should noticeably speed up test execution, especially for compute-heavy tests.
.github/workflows/coverage.yml (2)
34-35: LGTM! Linker optimization should speed up builds.Using
clangwithlldis a well-known technique for faster link times. The Ubuntu 22.04 runner includes both by default.
54-55: LGTM! Clean delegation to Makefile target.Centralizing the coverage command in the Makefile improves maintainability and allows local reproduction of CI coverage runs.
Makefile
Outdated
| codecov: | ||
| cargo llvm-cov --workspace --codecov --output-path lcov.info |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
cargo llvm-cov --codecov flag output format
💡 Result:
The --codecov flag makes cargo-llvm-cov export llvm-cov's JSON and convert it into Codecov's "custom coverage" JSON (a codecov.json file containing region/branch-aware coverage data suitable for the Codecov uploader). See the cargo-llvm-cov docs and repo README. [1][2]
Sources:
- cargo-llvm-cov README/docs (explains --codecov uses llvm-cov JSON then converts to codecov.json). [1]
- cargo-llvm-cov crate docs (details on JSON/Codecov output). [2]
🌐 Web query:
codecov-action lcov.info format support
💡 Result:
Yes. The Codecov GitHub Action will upload LCOV reports (lcov.info) to Codecov, and Codecov accepts LCOV (.info/.txt) format for processing. [1][2]
Sources:
- Codecov supported report formats (includes LcovProcessor). [1]
- codecov/codecov-action repository / Marketplace (uploader action used with GitHub Actions). [2]
Align the output filename with the actual coverage format.
The --codecov flag outputs Codecov's custom JSON format, but the filename lcov.info implies LCOV format, creating unnecessary confusion. Either rename to codecov.json to match the actual output format, or switch to --lcov and use lcov.info for actual LCOV format output.
🤖 Prompt for AI Agents
In Makefile around lines 113 to 114, the target writes Codecov JSON but names
the file lcov.info; change the output to a matching filename or change the flag.
Either rename the output to codecov.json (update the target to write to
codecov.json) or replace --codecov with --lcov so lcov.info is actually LCOV;
update any references elsewhere (CI, upload steps) to the new filename
accordingly.
3896d32 to
9fe52dd
Compare
hanabi1224
force-pushed
the
hm/speed-up-debug-tests
branch
from
9fe52dd to
41ddefc
Compare
Summary of changes
Changes introduced in this pull request:
Reference issue to close (if applicable)
Closes
Other information and links
Change checklist
Summary by CodeRabbit
Chores
Tests
✏️ Tip: You can customize this high-level summary in your review settings.