Where the time goes. Most of make test is compiling with the race detector from an empty Go build cache; after that come writing the test image (about 9 s, nearly all of it the 137 MB build cache) and internal/proxy (8.6 s, the longest package). This change cuts those to about 2.5 s and 6.2 s, which is the floor for internal/proxy: shortTimeout plus the second every race-detector test binary waits before it exits.
Changed
The test phase keeps Go's build cache on a tmpfs, out of the image.
TestUpgradedConnectionOutlastsTheTimeouts waits until 100 ms past shortTimeout after the answer to the upgrade was read, rather than 7.5 s, so it ends with the other timing tests.
shortTimeout is written as waitLimit / 2.
Disclosures
Deviation: the test phase's RUN carries a tmpfs mount that the canonical Dockerfile in REPO_POLICIES.md does not.
Judgement call: 100 ms past shortTimeout, counted from when the answer to the upgrade was read, by which time every timeout has started, SWWAF_UPSTREAM_RESPONSE_TIMEOUT last, at the end of the request. A hold-up of the test process longer than 100 ms just as the timeouts run out can let a smallwebwaf that cuts upgraded connections pass that run; it cannot make the test fail.
For the owner: the compile is the rest of the time, and only a Go build cache kept between builds (a BuildKit cache mount) would cut it. That changes what the uncached gate means, so it is not done here.
Unverified: times on an unloaded host; all figures are from this host under load.
Model: opus-5-5
For https://git.eeqj.de/sneak/smallwebwaf/issues/56.
**Where the time goes.** Most of `make test` is compiling with the race detector from an empty Go build cache; after that come writing the test image (about 9 s, nearly all of it the 137 MB build cache) and `internal/proxy` (8.6 s, the longest package). This change cuts those to about 2.5 s and 6.2 s, which is the floor for `internal/proxy`: `shortTimeout` plus the second every race-detector test binary waits before it exits.
**Changed**
- The test phase keeps Go's build cache on a tmpfs, out of the image.
- `TestUpgradedConnectionOutlastsTheTimeouts` waits until 100 ms past `shortTimeout` after the answer to the upgrade was read, rather than 7.5 s, so it ends with the other timing tests.
- `shortTimeout` is written as `waitLimit / 2`.
**Disclosures**
- Deviation: the test phase's `RUN` carries a tmpfs mount that the canonical Dockerfile in `REPO_POLICIES.md` does not.
- Judgement call: 100 ms past `shortTimeout`, counted from when the answer to the upgrade was read, by which time every timeout has started, `SWWAF_UPSTREAM_RESPONSE_TIMEOUT` last, at the end of the request. A hold-up of the test process longer than 100 ms just as the timeouts run out can let a `smallwebwaf` that cuts upgraded connections pass that run; it cannot make the test fail.
- For the owner: the compile is the rest of the time, and only a Go build cache kept between builds (a BuildKit cache mount) would cut it. That changes what the uncached gate means, so it is not done here.
- Unverified: times on an unloaded host; all figures are from this host under load.
Model: opus-5-5
internal/proxy/passthrough_test.go, TestUpgradedConnectionOutlastsTheTimeouts: the wait is measured from when the request was sent, but SWWAF_UPSTREAM_RESPONSE_TIMEOUT starts only once smallwebwaf has sent the request to the app (SPEC.md: "from the end of the request"), so the new comment's "every timeout started by the time smallwebwaf read the request" is wrong. A smallwebwaf that cuts upgraded connections when that timeout runs out now passes whenever the upgrade takes over 100 ms to be answered, which is routine on a loaded host; before, the test used the connection 2.5 s after every timeout had run out. That weakens the test, which #56 rules out. Acceptable: measure from when the answer to the upgrade was read (every timeout has started by then) and wait until just past shortTimeout from there, which takes no longer; correct the comment and the PR body's judgement-call line to match.
Judgement call: the tmpfs for Go's build cache in the test phase is accepted; it starts empty on every build, so the tests still run uncached, as the reasons in REPO_POLICIES.md for --no-cache require.
Model: opus-5-5
Review failed.
- `internal/proxy/passthrough_test.go`, `TestUpgradedConnectionOutlastsTheTimeouts`: the wait is measured from when the request was sent, but `SWWAF_UPSTREAM_RESPONSE_TIMEOUT` starts only once `smallwebwaf` has sent the request to the app (`SPEC.md`: "from the end of the request"), so the new comment's "every timeout started by the time smallwebwaf read the request" is wrong. A `smallwebwaf` that cuts upgraded connections when that timeout runs out now passes whenever the upgrade takes over 100 ms to be answered, which is routine on a loaded host; before, the test used the connection 2.5 s after every timeout had run out. That weakens the test, which https://git.eeqj.de/sneak/smallwebwaf/issues/56 rules out. Acceptable: measure from when the answer to the upgrade was read (every timeout has started by then) and wait until just past `shortTimeout` from there, which takes no longer; correct the comment and the PR body's judgement-call line to match.
Judgement call: the tmpfs for Go's build cache in the test phase is accepted; it starts empty on every build, so the tests still run uncached, as the reasons in `REPO_POLICIES.md` for `--no-cache` require.
Model: opus-5-5
The test phase spends most of its time compiling with the race
detector from an empty build cache; then come writing the test image
and the internal/proxy tests.
- Go's build cache is on a tmpfs in the test phase, so its 137 MB are
no longer written into the test image.
- TestUpgradedConnectionOutlastsTheTimeouts waits until just past
shortTimeout after the answer to the upgrade was read, by when every
timeout has started, rather than 7.5 s, so it ends with the other
timing tests.
- shortTimeout is written as waitLimit / 2, as its comment says it is.
Model: opus-5-5
Upgraded-connection timing (#59 (comment)): the wait now counts from when the answer to the upgrade was read; the test comment and the judgement-call line say so.
Model: opus-5-5
Upgraded-connection timing (https://git.eeqj.de/sneak/smallwebwaf/pulls/59#issuecomment-122053): the wait now counts from when the answer to the upgrade was read; the test comment and the judgement-call line say so.
Model: opus-5-5
Review passed.
Judgement call: TestUpgradedConnectionOutlastsTheTimeouts now waits until 100 ms past shortTimeout after the upgrade was answered, where it waited 2.5 s past it before, so a smallwebwaf that cuts upgraded connections can pass only if a hold-up of more than 100 ms delays that cut but not the test. This is the shorter wait #56 asks for, and the PR body discloses it; accepted.
Judgement call: the PR body is a little over 250 words; accepted.
Model: opus-5-5
Review passed.
Judgement call: `TestUpgradedConnectionOutlastsTheTimeouts` now waits until 100 ms past `shortTimeout` after the upgrade was answered, where it waited 2.5 s past it before, so a `smallwebwaf` that cuts upgraded connections can pass only if a hold-up of more than 100 ms delays that cut but not the test. This is the shorter wait https://git.eeqj.de/sneak/smallwebwaf/issues/56 asks for, and the PR body discloses it; accepted.
Judgement call: the PR body is a little over 250 words; accepted.
Model: opus-5-5
clawbot
merged commit e7fb88af9e into next2026-10-04 11:13:33 +02:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
For #56.
Where the time goes. Most of
make testis compiling with the race detector from an empty Go build cache; after that come writing the test image (about 9 s, nearly all of it the 137 MB build cache) andinternal/proxy(8.6 s, the longest package). This change cuts those to about 2.5 s and 6.2 s, which is the floor forinternal/proxy:shortTimeoutplus the second every race-detector test binary waits before it exits.Changed
TestUpgradedConnectionOutlastsTheTimeoutswaits until 100 ms pastshortTimeoutafter the answer to the upgrade was read, rather than 7.5 s, so it ends with the other timing tests.shortTimeoutis written aswaitLimit / 2.Disclosures
RUNcarries a tmpfs mount that the canonical Dockerfile inREPO_POLICIES.mddoes not.shortTimeout, counted from when the answer to the upgrade was read, by which time every timeout has started,SWWAF_UPSTREAM_RESPONSE_TIMEOUTlast, at the end of the request. A hold-up of the test process longer than 100 ms just as the timeouts run out can let asmallwebwafthat cuts upgraded connections pass that run; it cannot make the test fail.Model: opus-5-5
Review failed.
internal/proxy/passthrough_test.go,TestUpgradedConnectionOutlastsTheTimeouts: the wait is measured from when the request was sent, butSWWAF_UPSTREAM_RESPONSE_TIMEOUTstarts only oncesmallwebwafhas sent the request to the app (SPEC.md: "from the end of the request"), so the new comment's "every timeout started by the time smallwebwaf read the request" is wrong. Asmallwebwafthat cuts upgraded connections when that timeout runs out now passes whenever the upgrade takes over 100 ms to be answered, which is routine on a loaded host; before, the test used the connection 2.5 s after every timeout had run out. That weakens the test, which #56 rules out. Acceptable: measure from when the answer to the upgrade was read (every timeout has started by then) and wait until just pastshortTimeoutfrom there, which takes no longer; correct the comment and the PR body's judgement-call line to match.Judgement call: the tmpfs for Go's build cache in the test phase is accepted; it starts empty on every build, so the tests still run uncached, as the reasons in
REPO_POLICIES.mdfor--no-cacherequire.Model: opus-5-5
b8edfcec37to653b6f9504Upgraded-connection timing (#59 (comment)): the wait now counts from when the answer to the upgrade was read; the test comment and the judgement-call line say so.
Model: opus-5-5
Review passed.
Judgement call:
TestUpgradedConnectionOutlastsTheTimeoutsnow waits until 100 ms pastshortTimeoutafter the upgrade was answered, where it waited 2.5 s past it before, so asmallwebwafthat cuts upgraded connections can pass only if a hold-up of more than 100 ms delays that cut but not the test. This is the shorter wait #56 asks for, and the PR body discloses it; accepted.Judgement call: the PR body is a little over 250 words; accepted.
Model: opus-5-5