Assert the warm path does no I/O, not that it takes under 5ms - #14
Merged
Conversation
…r 5ms The test named "does not fetch the network on the warm path and verifies in <5ms p95" asserted only the second half. It never checked whether anything was fetched — the claim in its own name, and the one that matters. What it did assert was a wall-clock p95 over 200 Ed25519 verifications, on whatever hardware happened to run it. That failed CI at 5.92ms against a bound of 5. Nothing was wrong: a shared runner measures its neighbours as much as the code under test, so the assertion was going to fail on someone eventually, and "re-run it" is not a fix. The invariant worth protecting is that a warm verify touches no network. That is what makes signature verification usable on a request path, and unlike a duration it is exact — fetchImpl is injectable, so the calls can simply be counted: one cold fetch to populate the directory cache, then fifty verifies that add none. Timing is not asserted at all now. A number that fails for reasons unrelated to the code is worse than no number, because the failure teaches you to ignore it.
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.
Blocking #13, and unrelated to it.
What failed
The test never checked its own headline claim
The name says "does not fetch the network on the warm path and verifies in <5ms p95". The body asserted only the second half — it never checked whether anything was fetched.
What it did assert was a wall-clock p95 over 200 Ed25519 verifications on whatever hardware happened to run it. A shared CI runner measures its neighbours as much as the code, so this was going to fail on someone eventually. "Re-run it" isn't a fix.
What it asserts now
That a warm verify touches no network — the thing that makes signature verification usable on a request path, and unlike a duration it's exact.
fetchImplis injectable, so the calls are simply counted:Timing is not asserted at all. A number that fails for reasons unrelated to the code is worse than no number, because the failure teaches you to ignore it.
Testing
turbo test --force(cache disabled) — 169 tests green across the monorepo.