fix: restore the internal unit tests and run them in CI - #148
Open
husniadil wants to merge 1 commit into
Open
Conversation
`go test ./internal/...` fails on main. Two of the three packages fail to compile, which means internal/cache and internal/provider/vault have not actually been tested for a while — their test files reference struct fields that no longer exist. Nothing caught this because no workflow runs them. CI runs only `-short ./tests/end2end/...`; the Makefile's `test` target does run `./...`, but CI does not call it. So the tests rotted in place while every check stayed green. cache: keyringTested was replaced by a sync.Once guard, so the test set a field that no longer exists. Consuming keyringOnce before flipping keyringDisabled reproduces the intent without reaching for the removed field. TestCache_Stats was also timing-dependent — a 150ms TTL against two keyring writes that each shell out to the OS keyring, so the first entry expired while the second was still being written. It now asserts validity and expiry with separate TTLs and no sleeps. vault: the flat auth config (auth/authMount/role as top-level keys) moved into a nested Auth struct, and the JWT role error message became 'auth.role'. The tests still used the old shape, which no longer even unmarshals. Updated to the nested form that CONFIGURATION.md and SSO.md already document. gcsm: one table case omitted wantSecretID, so it asserted "" against a config that sets secret_id. All changes are to test files; no production behaviour is touched. The one non-test change adds `go test ./internal/...` to CI so these cannot rot again. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Hmh2p2Bg6kmxxvzpFDW2WL
This was referenced Aug 2, 2026
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.
The state on main
Two of these fail to compile, which is the part worth pausing on:
internal/cacheandinternal/provider/vaulthave not been running at all for a while. Their test files reference struct fields that no longer exist, so there is no coverage there — only the appearance of it.Why nothing noticed
No workflow runs them. The
testjob runs exactly one command:gotestsum ... -- -short ./tests/end2end/...The
Makefilehastest: go test ./..., which would have caught all three, but CI never calls it. So the packages under./internal/...were refactored, their tests were left behind, and every check stayed green throughout.This is the same shape as the
azure_keyvaultgap in #146: a check that proves something adjacent to what you actually care about.What was wrong
internal/cache—keyringTestedwas replaced by async.Onceguard, butTestCache_KeyringNotAvailablestill assigned to the removed field. ConsumingkeyringOncebefore settingkeyringDisabledpreserves the test's intent (force the unavailable path) without the deleted field.Once it compiled, a second failure surfaced underneath:
TestCache_Statsused a 150ms TTL and then performed twoSetcalls. EachSetshells out to the OS keyring and can cost hundreds of milliseconds, so entry one had already expired before entry two finished writing — the test was timing-dependent and would fail on any slow keyring. It now asserts the valid case with a generous TTL and the expired case with a negative TTL, so neither branch depends on wall-clock timing and nosleepis needed.internal/provider/vault— the auth config moved from flat top-level keys (auth: "jwt",authMount,role) into a nestedAuth *VaultAuthConfig{Method, Mount, Role, Token}. The old shape does not even unmarshal now:json.Unmarshalcannot put a string into a struct field. The tests are updated to the nested form, which is whatCONFIGURATION.mdandSSO.mdalready document.Two expectations also drifted with the code and are corrected against what
authenticateWithJWTactually returns: the JWT/OIDC role error is nowrequires 'auth.role' field, notrequires 'role' field. The top-leveltoken:shorthand still works —parseConfigfolds it intoAuth.Tokenfor backward compatibility — so those cases keep their flat config and only the assertion moved.internal/provider/gcsm— theconfig with empty project_idtable case omittedwantSecretID, so it asserted""against a config that setssecret_id: my-secret. Filled in, matching the sibling case directly below it.Change of scope
Everything above is test-only; no production code is touched. The single non-test change adds one step to the
testjob:Without it these tests can rot again the same way, and the fixes here would be worth very little. I put it before the e2e step so a compile failure fails fast.
Verification
go build ./...andgo vet ./internal/...are clean. Stashing the changes reproduces the original compile failures, so the fixes are load-bearing rather than incidental.Note that ten packages under
./internal/still report[no test files], includingconfig,secrets, andoidc. That is a real gap but out of scope here — this PR only restores what already existed.🤖 Generated with Claude Code
https://claude.ai/code/session_01Hmh2p2Bg6kmxxvzpFDW2WL