fix(bench): preserve setup failure messages - #468
Conversation
- Replace benchmark setup unwrap helpers with postfix abort adapters that keep the original Result error text. - Keep Option setup failures explicit with caller-provided context. - Report abort messages without bench-logging so setup failures are visible in every benchmark build.
WalkthroughReplaces the ChangesOrAbort Trait Refactor Across Benchmark Suite
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 0 |
🟢 Coverage ∅ diff coverage · +0.00% coverage variation
Metric Results Coverage variation ✅ +0.00% coverage variation (-1.00%) Diff coverage ✅ ∅ diff coverage Coverage variation details
Coverable lines Covered lines Coverage Common ancestor commit (f6a85e8) 69019 63272 91.67% Head commit (5cd24d4) 69019 (+0) 63272 (+0) 91.67% (+0.00%) Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch:
<coverage of head commit> - <coverage of common ancestor commit>Diff coverage details
Coverable lines Covered lines Diff coverage Pull request (#468) 0 0 ∅ (not applicable) Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified:
<covered lines added or modified>/<coverable lines added or modified> * 100%
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
coderabbitai
Bot
added
bug
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@benches/common/bench_utils.rs`:
- Around line 31-35: In the abort_benchmark function, replace the eprintln!
macro call with tracing::error! to ensure benchmark diagnostics use the proper
logging framework instead of direct standard error output. Update the line that
currently uses eprintln!("{message}") to instead use
tracing::error!("{message}") or a similar tracing error call that captures the
message parameter.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 05ea91b0-42c9-426c-b23d-89770eb985c0
📒 Files selected for processing (11)
benches/allocation_hot_paths.rsbenches/boundary_uuid_iter.rsbenches/ci_performance_suite.rsbenches/circumsphere_containment.rsbenches/cold_path_predicates.rsbenches/common/bench_utils.rsbenches/common/flip_workflows.rsbenches/profiling_suite.rsbenches/remove_vertex.rsbenches/tds_clone.rsbenches/topology_guarantee_construction.rs
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #468 +/- ##
=======================================
Coverage 91.64% 91.64%
=======================================
Files 72 72
Lines 68799 68799
=======================================
Hits 63054 63054
Misses 5745 5745
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. |
- Initialize a default error-level tracing subscriber for fatal benchmark setup failures. - Emit abort diagnostics through tracing in all benchmark builds instead of using direct stderr output.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
benches/common/bench_utils.rs (1)
11-18:⚠️ Potential issue | 🟡 MinorGate
init_tracing()behind#[cfg(feature = "bench-logging")]to comply with diagnostic feature-gating guideline.The coding guideline requires: "Gate non-essential test/benchmark diagnostics behind feature flags: use 'diagnostics' for test diagnostics and 'bench-logging' for benchmark diagnostics."
The established codebase pattern confirms this—
benches/profiling_suite.rsgates itsinit_tracing()with#[cfg(feature = "bench-logging")]and provides a no-op const fn alternative when the feature is disabled. Similarly,benches/ci_performance_suite.rsgates all tracing output behind the same feature flag.The new
init_tracing()inbenches/common/bench_utils.rsruns unconditionally, violating both the explicit guideline and the project's established pattern. Move the function and its call inabort_benchmark()behind a feature gate, or provide a no-op alternative like the existing code does.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@benches/common/bench_utils.rs` around lines 11 - 18, The init_tracing() function is running unconditionally and needs to be gated behind the bench-logging feature flag to comply with the project's diagnostic feature-gating guideline. Add #[cfg(feature = "bench-logging")] above the init_tracing() function definition to gate the implementation. Then provide an alternative no-op const fn for when the bench-logging feature is disabled using #[cfg(not(feature = "bench-logging"))]. Similarly, gate any call to init_tracing() (such as in abort_benchmark()) behind the same #[cfg(feature = "bench-logging")] conditional to ensure tracing is only initialized when the feature is enabled, matching the pattern used in other benchmark files like benches/profiling_suite.rs and benches/ci_performance_suite.rs.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@benches/common/bench_utils.rs`:
- Around line 11-18: The init_tracing() function is running unconditionally and
needs to be gated behind the bench-logging feature flag to comply with the
project's diagnostic feature-gating guideline. Add #[cfg(feature =
"bench-logging")] above the init_tracing() function definition to gate the
implementation. Then provide an alternative no-op const fn for when the
bench-logging feature is disabled using #[cfg(not(feature = "bench-logging"))].
Similarly, gate any call to init_tracing() (such as in abort_benchmark()) behind
the same #[cfg(feature = "bench-logging")] conditional to ensure tracing is only
initialized when the feature is enabled, matching the pattern used in other
benchmark files like benches/profiling_suite.rs and
benches/ci_performance_suite.rs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 17d518d1-2948-4405-bd83-c709065bc580
📒 Files selected for processing (1)
benches/common/bench_utils.rs
1 participant
Closes #434