Repository navigation
test(service): Share the test setup, and fix the timing checks for coverage - #63
Merged
Merged
Conversation
…verage - `tests/harness.ts` has `startUpstream(handler)` and `startTestService(extra)`. Each closes its server, and the service deletes its directory, when the test finishes. They replace the copied setup and teardown of `recording`, `redaction`, `vitest`, and `service` tests, and the 5 services and 1 upstream that tests of `service.test.ts` started by hand. - `longestStall(work)` replaces the 5 copies of the event loop probe in `redaction.test.ts`. - The test of 64 credential fields in 8 MiB is now a case of the `it.each` of the 8 MiB request samples. Each case keeps its body and its check that no password stays in the sample. All 3 cases now also check the total time. - `budget(ms)` gives 3 times `ms` with coverage. CI runs the tests with coverage, where the form body stall took 1.4 s and 1.7 s against a limit of 1.5 s, and the decode stall took 2.9 s against 3 s. The stall checks of `redaction.test.ts` and the answer times of `service.test.ts` use it. The limits without coverage do not change, so each one still fails for the slow code that it guards against. - The answer time of the test of a key that fills the worker heap allows 1 s, not 500 ms. Without coverage it got 511 ms once. A key in the service process takes seconds there, or fills the heap. The test count stays at 114. Co-Authored-By: David Cramer <23610+dcramer@users.noreply.github.com>
…rage CI runs the tests only with coverage, so budget(1000) gave 3 s there and would not catch a parse of the 16.5 MB body in the service process (about 1.6 s). Coverage adds little to this check. Also detect coverage only for --coverage and --coverage.enabled[=true]. Co-Authored-By: David Cramer <23610+dcramer@users.noreply.github.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
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.
Fixes the 2 timing tests that fail on main's CI today (the form-body stall and the decode stall), and removes copied test setup.
tests/harness.ts:startUpstream(handler)andstartTestService(extra)replace the copied setup and teardown in 4 test files, plus 5 services and 1 upstream that tests started by hand.longestStall(work)replaces 5 copies of the stall probe.it.each, with its own leak check, so the test count stays 114.budget(ms)gives 3× a time limit only under coverage, so limits without coverage are unchanged. CI runs only with coverage.budget(). Coverage barely slows it, and 3 s would miss the regression it guards.AGENTS.md: wrap a tight timing limit inbudget()unless coverage barely slows that check.The pinned key tests are untouched.
pnpm checkpasses, andpnpm test:coveragepassed in repeated runs; on main it fails on the form-body test.via @dcramer.
--
View Session [Sentry]