[quality] cover poller run() ticker + checkDeveloperChannel branches via #23356 seams - #23357
[quality] cover poller run() ticker + checkDeveloperChannel branches via #23356 seams#23357kubestellar-hive[bot] wants to merge 1 commit into
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
✅ Deploy Preview for kubestellarconsole canceled.
|
|
👋 Hey @kubestellar-hive[bot] — thanks for opening this PR!
This is an automated message. |
|
Important Held for human sign-off on the direction, not on the code. This PR's only tracked rationale is #23341, which the hive filed itself — issue #23341 was filed by kubestellar-hive[bot] and no human has acknowledged it. An agent-filed issue does not, on its own, establish that anyone agreed to the direction (hivecommons/hive#5117). The change may well be right; nothing here is a review of it. To release the hold, acknowledge the direction on that issue — comment on it, assign yourself, or add the |
Refs #23341. Uses the fetchLatestMainSHAFn / detectCurrentSHAFn function-var seams landed by PR #23356 to add three unit tests in poller_seams_test.go that cover previously-unreachable branches in checkDeveloperChannel: - TestCheckDeveloperChannel_FetchLatestSHAError — locks the fetch- failure return path so a dropped 'return' after slog.Error can't silently emit a bogus 'already up to date' broadcast. - TestCheckDeveloperChannel_AlreadyUpToDate — locks the Progress=100 'no changes on main' broadcast and asserts detectCurrentSHAFn is invoked exactly once with its result stored on the checker. - TestCheckDeveloperChannel_FreshSHANotDetected — locks the guard that keeps an empty detectCurrentSHA result from clobbering uc.currentSHA. Coverage on pkg/agent/updater: poller.go checkDeveloperChannel 44.4% -> 92.6% A previous version of this file also had a TestRun_TickerFiresAndCanUpdate that shrank the developerCheckInterval / initialStartupDelay interval seams to force run()'s ticker arm to fire. The race detector caught a legitimate data race between that write and the read of initialStartupDelay done by run() goroutines that pre-existing tests (TestStart_Stop in lifecycle_checksum_test.go) leave dangling in time.After(initialStartupDelay) even after they cancel their contexts. Covering the ticker arm safely needs run() to atomic-load the interval seams — a coder-lane change, not a quality-lane one — so run() coverage stays at 40.9% for now. Signed-off-by: kubestellar-hive[bot] <223556219+Copilot@users.noreply.github.com>
a7728ff to
2a042e6
Compare
Update — dropped racy
|
| function | before | after |
|---|---|---|
poller.go checkDeveloperChannel |
44.4% | 92.6% |
run() coverage stays at 40.9%. Covering the ticker arm safely requires run() to atomic-load the interval seams — a coder-lane change on top of #23356, not a quality-lane one. Will file a follow-up issue once this lands.
Also removed the now-unused context and time imports and the atomic scaffolding for the deleted test.
The commit-message body of 2a042e6 records all of the above; nothing was silently changed.
— hive: agent=quality backend=copilot model=claude-opus-4.7
Test Improvement
Adds
pkg/agent/updater/poller_seams_test.go— 4 tests exploiting the test seams introduced by #23356 to cover previously-unreachable branches inpoller.go.Refs #23341. Depends on #23356 landing first — that PR flipped
developerCheckIntervalfromconsttovar, introducedinitialStartupDelay/fetchLatestMainSHAFn/detectCurrentSHAFn. This PR uses those seams; if #23356 is not merged first, this branch will not compile.Coverage delta (
pkg/agent/updater)poller.gorunpoller.gocheckDeveloperChannelTests added
TestRun_TickerFiresAndCanUpdate— first test to reach thecase <-ticker.C:arm ofrun().TestRun_StopsOnContextCancelonly covered the context-cancel arm.TestCheckDeveloperChannel_FetchLatestSHAError— locks the fetch-failure return path. A regression that dropped thereturnafterslog.Error(...)would fall through withlatestSHA="", compare equal to a fresh currentSHA of"", and silently emit a bogus"already up to date"broadcast — hiding the fetch failure from the frontend.TestCheckDeveloperChannel_AlreadyUpToDate— locks theProgress=100"no changes on main" broadcast and assertsdetectCurrentSHAFnis invoked exactly once and its result is stored on the checker.TestCheckDeveloperChannel_FreshSHANotDetected— locks theif freshSHA := ...; freshSHA != ""guard. Without it an emptydetectCurrentSHAresult would clobberuc.currentSHAon every tick.Seam etiquette
withSwappedSeams(t, ...)at the top of the file saves the original vars and registers at.Cleanupto restore them, so parallel test runs and later tests are unaffected.Not covered (intentional)
checkDeveloperChannelbranch that dispatchesexecuteDeveloperUpdatewhen SHAs differ — that's the heavy path (git operations, npm build, restart) that already has extensive coverage inchecker_test.go.Filed by quality agent (ACMM L4/L6 — full mode)
— hive: agent=quality backend=copilot model=claude-opus-4.7