test: exercise duration_by_model absent-key fallback (#869)#871
Merged
test: exercise duration_by_model absent-key fallback (#869)#871
duration_by_model absent-key fallback (#869)#871Conversation
Add two tests to TestRenderVscodeSummaryPerModelTable that verify the .get(model, 0) guard in render_vscode_summary when a model appears in requests_by_model but is missing from duration_by_model: - test_per_model_table_absent_duration_defaults_to_zero_avg - test_per_model_table_partial_duration_dict These catch regressions if the defensive fallback is accidentally removed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds regression tests to ensure render_vscode_summary safely handles models that appear in requests_by_model but are missing from duration_by_model (exercising the .get(model, 0) fallback).
Changes:
- Added a test for the fully absent
duration_by_modelcase (defaults to0ms). - Added a test for a partially populated
duration_by_modelwhere only some models have duration entries.
- Make test assertions target specific model rows instead of matching anywhere in the output, preventing false passes from incidental matches in the totals panel or other columns. - Set total_duration_ms to non-zero values to avoid ambiguous 0ms matches. - Apply same fix to pre-existing test_avg_ms_division_by_zero_guard which had the same class of issue (fix-forward). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
|
Commit pushed:
|
microsasa
added
the
aw-review-response-attempted
- Use regex word-boundary assertion for standalone '0ms' check to avoid matching '0ms' inside '100ms' (line 177) - Assert '500ms' count==2 to verify avg and total columns independently rather than relying on ambiguous substring match (line 220) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
|
Commit pushed:
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Quality Gate: APPROVED ✅
Low-impact test-only change — auto-approving for merge.
What was evaluated:
- 2 new tests exercising the defensive
.get(model, 0)fallback inrender_vscode_summaryfor absent/partialduration_by_modelkeys - 1 existing test (
test_avg_ms_division_by_zero_guard) refined with more precise regex assertion and explicittotal_duration_ms - All changes confined to
tests/copilot_usage/test_vscode_report.py(41 additions, 1 deletion) - Tests follow existing patterns, use established helpers (
_make_summary,_capture,_strip_ansi), and include meaningful docstrings - CI: all 10 checks green
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.
Closes #869
Summary
The
.get(model, 0)fallback inrender_vscode_summarywas never exercised by tests — every existing test inTestRenderVscodeSummaryPerModelTablesupplied matching keys in bothrequests_by_modelandduration_by_model.Changes
Added two tests to
TestRenderVscodeSummaryPerModelTableintests/copilot_usage/test_vscode_report.py:test_per_model_table_absent_duration_defaults_to_zero_avg— a single model present inrequests_by_modelbut completely absent fromduration_by_model; verifies no exception and0msavg output.test_per_model_table_partial_duration_dict— two models inrequests_by_model, only one induration_by_model; verifies the known model shows correct avg (500ms) and the absent one shows0ms.Both tests will fail with
KeyErrorif the defensive.get(model, 0)is ever replaced with a direct subscript, catching the regression immediately.Verification
make checkpasses: lint ✅ · typecheck ✅ · security ✅ · unit tests (99% coverage) ✅ · e2e tests ✅