tests: drive the latch through the engine, not around it

Merged
IC
2mo ago
+73-13

Third version of this test. The reviewer was right twice, and the second time the fix was still wrong in the same way.

The test pins that credential.status reports stale_pending: false on a record that has left Active — the column is never cleared by any of the seven state-update paths, while the next get on a latched record fails fast without an upstream exchange. The field predicts cost; on a latched record the honest prediction is "cheap".

What kept being wrong was how the test reached that state.

v1  store.invalidate(...)                     unversioned, direct
v2  store.invalidate_if_version_reported(...)  version-fenced, still direct
v3  a real failing refresh through RefreshEngine

Both earlier versions reproduced the outcome of a failed refresh rather than running one. If the engine's failure arm ever also cleared stale_pending, they would have stayed green while the state they pin became unreachable — which is exactly the defect this test was rewritten to fix in the first place, surviving two rewrites aimed at it.

It now runs the path: a consumer reports a 401, the mark lands, the next get sees it and forces a refresh, the provider refuses. The engine latches the record and writes the refresh_failed observation no direct store call produces.

Two details that are load-bearing and easy to lose later

The fixture returns InvalidGrant specifically. That is the only provider verdict the engine treats as terminal. Every other error takes the transient arm, clears the intent, and leaves the record Active — so a stub that merely fails cannot reproduce a latch.

The get passes force_refresh: false. The refresh has to be driven by the stale_pending mark the report left behind. Forcing it would make the test pass on a path no consumer takes.

Neither is asserted — both are mutation-proved:

revert the production fix        RED on the pin's own message
adapter InvalidGrant -> Transport RED at the engine step: RefreshFailed, record still Active

The second is what makes the InvalidGrant choice a finding rather than a detail.

Heads-up: master does not currently resolve --locked

Independent of this PR, and I checked it on a clean detached origin/master before assuming my rebase caused it:

cargo metadata --locked --offline   ->   cannot update the lock file
cargo metadata --offline            ->   wants to REMOVE exactly two lines
                                          "reqwest", "rusqlite"  from subc-core

Those are the two lines 12a1b4c added. The lock now describes a subc-core that declares reqwest and rusqlite; the sibling revision published to the fleet (20029b3d) declares neither. They appear in 7e4a705a and later.

subconscious 20029b3d   subc-core 0.12.0   reqwest ✗  rusqlite ✗   <- current fleet pin
subconscious 7e4a705a   subc-core 0.12.0   reqwest ✓  rusqlite ✓
subconscious 989ddb27   subc-core 0.12.0   reqwest ✓  rusqlite ✓   <- resolves
subconscious 44e03c27   subc-core 0.13.0                            <- version no longer matches

So master's lock was refreshed against a subconscious newer than the one the fleet is pinned to. Anyone building against the published pin gets a --locked failure that looks like their own tree is dirty — it is not, and git status stays clean throughout.

Gated at 989ddb27: exit 0, nine real-daemon e2e arms executing.


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.


Summary by cubic

Rewrites the credential latch test to drive the failure through the engine instead of calling the store directly, so it catches regressions in both the store and the engine's failure arm.

Why the test changed

  • The prior test called the store directly, reproducing the outcome without running the engine's failure path.
  • Now the fixture returns InvalidGrant (the only terminal provider verdict) and the get passes force_refresh: false, so the stale_pending mark drives the refresh.
  • Both details are mutation-tested: reverting the production fix turns the pin red, and swapping the error to Transport fails at the engine step.

Heads-up for reviewers

  • cargo metadata --locked fails on master because the lock file was refreshed against a newer subconscious than the fleet pin; the lockfile currently resolves only until the version bump.