Improve test quality: pkg/cli/update_cooldown_test.go#29522
Conversation
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/951b394f-d495-4675-b37b-63079ff8f37f Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Improves the reliability and readability of the release cooldown test suite by strengthening assertions and consolidating repetitive cases into table-driven subtests.
Changes:
- Added named subtests to
TestIsExemptFromCoolDown. - Consolidated multiple
TestCheckReleaseCoolDown_*tests into one table-driven test with strongerPublishedAtassertions. - Strengthened cooldown message assertions and added a negative-duration edge case to duration formatting tests.
Show a summary per file
| File | Description |
|---|---|
pkg/cli/update_cooldown_test.go |
Refactors cooldown tests for clearer subtest naming, broader scenario coverage, and stronger assertions around timestamps/messages. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 1/1 changed files
- Comments generated: 1
| {"notactions/checkout", false}, | ||
| {"notgithub/repo", false}, | ||
| {name: "actions namespace", repo: "actions/checkout", want: true}, | ||
| {name: "actions namespace with version", repo: "actions/setup-node", want: true}, |
There was a problem hiding this comment.
Test case name "actions namespace with version" is misleading because the input repo value doesn't include a version (and gitutil.ExtractBaseRepo only handles subpaths, not @ref). Consider renaming the subtest to reflect what's actually being exercised (e.g., just another actions/* repo) to avoid confusion for future readers.
| {name: "actions namespace with version", repo: "actions/setup-node", want: true}, | |
| {name: "another actions namespace repo", repo: "actions/setup-node", want: true}, |
🧪 Test Quality Sentinel ReportTest Quality Score: 96/100✅ Excellent test quality
Test Classification Details📋 All 8 Test Functions
Flagged Tests — Requires ReviewNo tests flagged. All tests meet the quality bar. Language SupportTests analyzed:
Verdict
📊 Score Breakdown
📖 Understanding Test ClassificationsDesign Tests (High Value) verify what the system does:
Implementation Tests (Low Value) verify how the system does it:
Goal: Shift toward tests that describe the system's behavioral contract — the promises it makes to its users and collaborators. References: §25212425298
|
Addresses several test quality gaps in the release cooldown test suite: missing named subtests, five separate
TestCheckReleaseCoolDown_*functions that should be one table-driven test, weak message assertions, and a missing negative-duration edge case.Changes
TestIsExemptFromCoolDown— Addednamefield to struct; subtests now uset.Run(tt.name, ...)instead of the repo path stringTestCheckReleaseCoolDown_*→TestCheckReleaseCoolDown— Consolidated 5 separate functions into a single table-driven test; strengthensPublishedAtcheck to assert exact value equality (not just non-zero) and assertsIsZero()on paths that shouldn't populate itTestCheckReleaseCoolDownWithDate_InCoolDown— Adds assertions on"cool down","remaining", and"owner/repo"in the message bodyTestCheckReleaseCoolDownWithDate_NotInCoolDown— AddsPublishedAt.IsZero()andassert.Empty(Message)assertions (this variant never populatesPublishedAt)TestFormatCoolDownDuration— Adds-1 * time.Hour → "< 1h"case to cover the negative-duration clamp path