[quality] 🧪 test(agent): hermetic shared-Copilot-config token tests (configTestHelper, helpers_coverage negative tests) - #5440
Merged
Conversation
![hive-quality[bot]](https://github.githubassets.com/images/gravatars/gravatar-user-420.png){kind=avatar}
…ome/.copilot/config.json configTestHelper and TestConfigHasTokens_WithActualFile wrote the LIVE shared Copilot credential file, clobbering real tokens for the duration of every test run (or permanently, if the binary died mid-run) and flaking on live hosts (TestFixSharedConfigPerms_FixesPerms: EPERM on a foreign-owned file). Redirect sharedCopilotConfigPath to t.TempDir() instead — the var exists for exactly this (manager.go). Also make the three skip-on-live-host negative tests (TestConfigHasTokens_NoFiles, TestCopilotConfigHasTokens_NoFile, TestClearExpiredTokens_NoFile) hermetic via emptySharedPaths so they assert everywhere instead of skipping. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: hive-quality[bot] <quality@hive>
kubestellar-prow
Bot
added
the
dco-signoff: yes
Contributor
|
[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 |
kubestellar-prow
Bot
added
the
size/M
|
Changelog: this PR changes code but does not touch If it is user-visible — a feature, a fix an operator would notice, a This is a reminder, not a gate; it never blocks a merge. |
kubestellar-prow
Bot
deleted the
quality/test-hermetic-shared-copilot-config
branch
September 1, 2026 13:05
|
Thank you for your contribution! Your PR has been merged. Check out what's new:
Stay connected: Slack #kubestellar-dev | Multi-Cluster Survey |
clubanderson
added a commit
to Danathar/hive
that referenced
this pull request
Sep 1, 2026
… watcher test TestWatcher_ReloadsOnChange failed the required v2 Tests gate on PRs that do not touch pkg/config (e.g. kubestellar#5440, run 33488167240, job `test (rest 2/3)`) with: watcher_test.go:62: expected org = "updated-org" after reload, got "" Root cause is a race in the test, not in the watcher. The onChange callback increments reloadCount before it stores lastOrg: reloadCount.Add(1) lastOrg.Store(cfg.Project.Org) while the wait loop exited as soon as reloadCount > 0 and then immediately read lastOrg. When the poll observes the counter in the window between those two statements, lastOrg is still unset and the type assertion yields "" — the exact reported failure. The window is tiny locally but is readily hit under -race on a loaded CI runner. Note this is NOT a partial/truncated read of a non-atomic os.WriteFile as first suspected: every truncation of the config fails to parse or fails validation ("project.org is required"), so onChange is never invoked and no empty org can ever be stored. The observed "" is the never-stored case. Fix: poll for the observable outcome (lastOrg == "updated-org") rather than for the reload counter, with a generous deadline and a tighter poll interval. The assertion is unchanged in strength — the test still proves the watcher reloads and delivers the new content, and still requires reloadCount > 0. Verification, all under -short -race: - Reproduced deterministically by injecting a stall between the increment and the store (modelling a scheduler preemption): 10/10 failures with the identical message, 0/25 after the fix; still 0/10 with a 3s stall. - Real test: 200/200 pass; all three TestWatcher_* 100x under 12-way CPU saturation: pass. Fixes kubestellar#5448 Signed-off-by: Andrew Anderson <andy@clubanderson.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
1 participant
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 Improvement
Claims exactly:
src/pkg/agent/coverage_boost_test.go(configTestHelper,TestConfigHasTokens_WithActualFile) andsrc/pkg/agent/helpers_coverage_test.go(TestConfigHasTokens_NoFiles,TestCopilotConfigHasTokens_NoFile,TestClearExpiredTokens_NoFile). No production code changes. Disjoint from open hold-gated PRs #5433 (pkg/config checkout root + dashboard terminal-urls), #5428 (pkg/config entrypoint boot), #5436 (systemd), #4032 (proxy/github/cmd).What was wrong:
configTestHelperandTestConfigHasTokens_WithActualFilewrote test payloads to the LIVE/data/home/.copilot/config.json(real shared Copilot credentials) with a save/restore dance:gho_testfakes for the duration of every test run,TestFixSharedConfigPerms_FixesPermsfails in full-suite runs (foreign-owned live file).Fix: redirect
sharedCopilotConfigPathintot.TempDir()— the var is documented insrc/pkg/agent/manager.goas existing precisely for test redirection (same pattern ascopilot_session_refresh_test.go).Also converts the three skip-on-live-host negative tests in
helpers_coverage_test.goto hermetic asserts via the existingemptySharedPaths(t)helper, so the no-file paths are actually verified on live hosts instead of skipped.Verification:
go test -run 'TestConfigHasTokens|TestFixSharedConfigPerms|TestClearExpiredTokens|TestCopilotConfigHasTokens' ./pkg/agent/— all pass on a live hive host, zero skips, and the previously-failingTestFixSharedConfigPerms_FixesPermsnow passes in full-suite runs.Related Issue
Fixes #5439
Filed by quality agent (hold-gated mode). Human review required.
— hive: agent=quality backend=copilot model=claude-fable-5