test: fix vacuous assertions in audit analyzer tests#885
Merged
jamesadevine merged 1 commit intoJun 7, 2026
Merged
Conversation
same total_tokens value in the implementation, making the original comparison self-referential and unable to catch regressions; also pin turns == 2 from the fixture instead of just > 0. with assertions on actual noop context and reason values — extract_noops unconditionally sets tool: None, so the original assertion could never fail; new assertions verify context routing and the description→reason fallback for the third noop entry. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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
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.
Test Suite Reduction:
src/audit/analyzers/otel.rs+src/audit/analyzers/missing.rsWhat was wrong
analyze_otel_reads_otel_only(otel.rs): The assertionassert_eq!(analysis.metrics.effective_tokens, analysis.metrics.token_usage)is vacuous. In the implementation both fields are unconditionally set to the sametotal_tokensvalue, so the comparison is structurally equivalent toassert_eq!(x, x)— it can never fail and provides no regression protection. Separately,assert!(analysis.metrics.turns > 0)would pass even if the fixture only produced 1 turn instead of the expected 2.mixed_ndjson_file_filters_each_signal_and_ignores_unrelated_entries(missing.rs): The assertionassert!(noops.iter().all(|report| report.tool.is_none()))is vacuous.extract_noopsunconditionally setstool: Noneon everyNoopReportit constructs — the assertion is tautologically true regardless of the input data and can never expose a regression.Changes
otel.rsanalyze_otel_reads_otel_onlymissing.rsmixed_ndjson_file_filters_each_signal_and_ignores_unrelated_entriestool.is_none()loop with assertions on noopcontextvalues and thedescription→reasonfallback pathVerification
cargo test: all 1 761 unit + 135 integration tests pass ✅cargo clippy --all-targets --all-features: no warnings ✅Warning
Firewall blocked 2 domains
The following domains were blocked by the firewall during workflow execution:
spsprodeus21.vssps.visualstudio.comspsprodweu4.vssps.visualstudio.comSee Network Configuration for more information.