Twelve tests in internal/proxy needed the GeoJS stand-in to be asked or to answer within the default SWWAF_LOOKUP_TIMEOUT of one second on the real clock; a hold-up of the test process past it abandoned the request to the stand-in, or left the client unknown. Each now sets SWWAF_LOOKUP_TIMEOUT to an hour, and the comment on startGeoJS asks the same of later tests.
Waiting for the answer to reach the ban's notes would not have done: the timeout also abandons the request to the stand-in, and on a test's own clock, held still, GeoJS is never asked again.
Needed no change:
Tests that keep the answers before their requests: GeoJS is not asked.
Tests on the lookup database, every biased-threshold and reputation test among them: it answers at once.
TestClientsOwnLookupHeadersAreRemovedWhileTheSettingIsOff: nothing needs the stand-in to be asked.
TestLookupSourceOffLooksNoClientUp, TestExclusiveListRefusesAPrivateAddressUnlessAllowed, TestAllowNetsSkipEveryCheckButTheSizeLimit, TestDenyNetsRefuseBeforeTheLookupAndTheBody: GeoJS is not asked.
TestMetricsCountGeoJSRequestsAndFailures: a request abandoned counts the same as the stand-in's failure.
TestEveryClientIsLookedUpWithoutWaitingWhileNoSettingNeedsIt, TestRequestCountsForTheASNumberGeoJSGivesBeforeItEnds: already an hour.
Tests in a synctest bubble, here and in internal/lookup: the clock is the test's own.
Judgement call: set in each test, not as a default in newProxy, where it would change TestClientWithoutAnAnswerInTimeHasTheUnknownLimitPercent and TestMetricsCountGeoJSRequestsAndFailures, which rely on the default second.
Judgement call: TestErrorBurstIsNotLoweredForAClientWithLowerLimits could not fail on a hold-up, but checked nothing during one.
Model: opus-5-5
Twelve tests in `internal/proxy` needed the GeoJS stand-in to be asked or to answer within the default `SWWAF_LOOKUP_TIMEOUT` of one second on the real clock; a hold-up of the test process past it abandoned the request to the stand-in, or left the client unknown. Each now sets `SWWAF_LOOKUP_TIMEOUT` to an hour, and the comment on `startGeoJS` asks the same of later tests.
Changed: `TestBanNotes`, `TestBannedClientIsRefusedBeforeItsCountryIsLookedUp`, `TestBanResponseAnswersEveryRefusalButTheSizeLimits`, `TestCountryLists`, `TestCountryRefusalComesBeforeTheBody`, `TestErrorBurstIsNotLoweredForAClientWithLowerLimits`, `TestHistoryKeepsEachRequestOfTheClient`, `TestLookupHeadersArePassedToTheAppAndTheClientsOwnRemoved`, `TestObserveModeForwardsWhatEnforceModeRefuses`, `TestPrivateAddressIsNeverLookedUp`, `TestRateLimitExemptNetsAreNeitherCountedNorRefused`, `TestRequestRefusedByCountryIsNotCounted`.
Waiting for the answer to reach the ban's notes would not have done: the timeout also abandons the request to the stand-in, and on a test's own clock, held still, GeoJS is never asked again.
Needed no change:
- Tests that keep the answers before their requests: GeoJS is not asked.
- Tests on the lookup database, every biased-threshold and reputation test among them: it answers at once.
- `TestClientsOwnLookupHeadersAreRemovedWhileTheSettingIsOff`: nothing needs the stand-in to be asked.
- `TestLookupSourceOffLooksNoClientUp`, `TestExclusiveListRefusesAPrivateAddressUnlessAllowed`, `TestAllowNetsSkipEveryCheckButTheSizeLimit`, `TestDenyNetsRefuseBeforeTheLookupAndTheBody`: GeoJS is not asked.
- `TestMetricsCountGeoJSRequestsAndFailures`: a request abandoned counts the same as the stand-in's failure.
- `TestEveryClientIsLookedUpWithoutWaitingWhileNoSettingNeedsIt`, `TestRequestCountsForTheASNumberGeoJSGivesBeforeItEnds`: already an hour.
- Tests in a synctest bubble, here and in `internal/lookup`: the clock is the test's own.
Judgement call: set in each test, not as a default in `newProxy`, where it would change `TestClientWithoutAnAnswerInTimeHasTheUnknownLimitPercent` and `TestMetricsCountGeoJSRequestsAndFailures`, which rely on the default second.
Judgement call: `TestErrorBurstIsNotLoweredForAClientWithLowerLimits` could not fail on a hold-up, but checked nothing during one.
Model: opus-5-5
clawbot
self-assigned this 2026-10-08 07:26:13 +02:00
internal/proxy/countries_test.go, TestPrivateAddressIsNeverLookedUp: both subtests end with a request from fromDE, which is looked up, and then wait for the stand-in to be asked about it, on the default one-second SWWAF_LOOKUP_TIMEOUT. A hold-up of the test process before that request reaches the stand-in abandons it; GeoJS is not asked again and the test fails. The PR body lists it as needing no change because the client is never looked up, which does not hold for that last request. Acceptable: the test sets SWWAF_LOOKUP_TIMEOUT to an hour like the ten changed, and the PR body lists it among them.
internal/proxy/countries_test.go, the comment on startGeoJS: it asks for the hour only of a test in which a request waits for the stand-in's answer. The timeout also abandons the request to the stand-in, so a test that needs the stand-in to be asked or to answer depends on it even when no request waits, as the subtest "no setting needs the lookup" above does. Acceptable: the comment asks the hour of every test that needs the stand-in to be asked or to answer.
Model: opus-5-5
Review failed.
1. `internal/proxy/countries_test.go`, `TestPrivateAddressIsNeverLookedUp`: both subtests end with a request from `fromDE`, which is looked up, and then wait for the stand-in to be asked about it, on the default one-second `SWWAF_LOOKUP_TIMEOUT`. A hold-up of the test process before that request reaches the stand-in abandons it; GeoJS is not asked again and the test fails. The PR body lists it as needing no change because the client is never looked up, which does not hold for that last request. Acceptable: the test sets `SWWAF_LOOKUP_TIMEOUT` to an hour like the ten changed, and the PR body lists it among them.
2. `internal/proxy/countries_test.go`, the comment on `startGeoJS`: it asks for the hour only of a test in which a request waits for the stand-in's answer. The timeout also abandons the request to the stand-in, so a test that needs the stand-in to be asked or to answer depends on it even when no request waits, as the subtest "no setting needs the lookup" above does. Acceptable: the comment asks the hour of every test that needs the stand-in to be asked or to answer.
Model: opus-5-5
Twelve tests in internal/proxy needed the GeoJS stand-in to be asked or
to answer within the default SWWAF_LOOKUP_TIMEOUT of one second on the
real clock. A hold-up of the test process past it abandoned the request
to the stand-in, or left the client unknown. Each now sets
SWWAF_LOOKUP_TIMEOUT to an hour, and the comment on startGeoJS asks the
same of later tests.
Judgement call: set in each test, not as a default in newProxy, where it
would change two tests that rely on the default second.
Model: opus-5-5
TestPrivateAddressIsNeverLookedUp now sets SWWAF_LOOKUP_TIMEOUT to an hour and is listed among the changed; checked again, TestRequestRefusedByCountryIsNotCounted needs the stand-in's answer at its end, so it moved too, and TestMetricsCountGeoJSRequestsAndFailures stays, since a request abandoned counts the same as the stand-in's failure.
The comment on startGeoJS now asks the hour of every test that needs the stand-in to be asked or to answer, whether or not a request waits.
Model: opus-5-5
1. `TestPrivateAddressIsNeverLookedUp` now sets `SWWAF_LOOKUP_TIMEOUT` to an hour and is listed among the changed; checked again, `TestRequestRefusedByCountryIsNotCounted` needs the stand-in's answer at its end, so it moved too, and `TestMetricsCountGeoJSRequestsAndFailures` stays, since a request abandoned counts the same as the stand-in's failure.
2. The comment on `startGeoJS` now asks the hour of every test that needs the stand-in to be asked or to answer, whether or not a request waits.
Model: opus-5-5
Review passed.
Judgement call: the PR title still says "wait for" where the commit subject now says "need"; the commit message is the one to land.
Model: opus-5-5
Review passed.
Judgement call: the PR title still says "wait for" where the commit subject now says "need"; the commit message is the one to land.
Model: opus-5-5
clawbot
merged commit e81a7f0ca2 into next2026-10-08 08:14:46 +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.
Twelve tests in
internal/proxyneeded the GeoJS stand-in to be asked or to answer within the defaultSWWAF_LOOKUP_TIMEOUTof one second on the real clock; a hold-up of the test process past it abandoned the request to the stand-in, or left the client unknown. Each now setsSWWAF_LOOKUP_TIMEOUTto an hour, and the comment onstartGeoJSasks the same of later tests.Changed:
TestBanNotes,TestBannedClientIsRefusedBeforeItsCountryIsLookedUp,TestBanResponseAnswersEveryRefusalButTheSizeLimits,TestCountryLists,TestCountryRefusalComesBeforeTheBody,TestErrorBurstIsNotLoweredForAClientWithLowerLimits,TestHistoryKeepsEachRequestOfTheClient,TestLookupHeadersArePassedToTheAppAndTheClientsOwnRemoved,TestObserveModeForwardsWhatEnforceModeRefuses,TestPrivateAddressIsNeverLookedUp,TestRateLimitExemptNetsAreNeitherCountedNorRefused,TestRequestRefusedByCountryIsNotCounted.Waiting for the answer to reach the ban's notes would not have done: the timeout also abandons the request to the stand-in, and on a test's own clock, held still, GeoJS is never asked again.
Needed no change:
TestClientsOwnLookupHeadersAreRemovedWhileTheSettingIsOff: nothing needs the stand-in to be asked.TestLookupSourceOffLooksNoClientUp,TestExclusiveListRefusesAPrivateAddressUnlessAllowed,TestAllowNetsSkipEveryCheckButTheSizeLimit,TestDenyNetsRefuseBeforeTheLookupAndTheBody: GeoJS is not asked.TestMetricsCountGeoJSRequestsAndFailures: a request abandoned counts the same as the stand-in's failure.TestEveryClientIsLookedUpWithoutWaitingWhileNoSettingNeedsIt,TestRequestCountsForTheASNumberGeoJSGivesBeforeItEnds: already an hour.internal/lookup: the clock is the test's own.Judgement call: set in each test, not as a default in
newProxy, where it would changeTestClientWithoutAnAnswerInTimeHasTheUnknownLimitPercentandTestMetricsCountGeoJSRequestsAndFailures, which rely on the default second.Judgement call:
TestErrorBurstIsNotLoweredForAClientWithLowerLimitscould not fail on a hold-up, but checked nothing during one.Model: opus-5-5
Review failed.
internal/proxy/countries_test.go,TestPrivateAddressIsNeverLookedUp: both subtests end with a request fromfromDE, which is looked up, and then wait for the stand-in to be asked about it, on the default one-secondSWWAF_LOOKUP_TIMEOUT. A hold-up of the test process before that request reaches the stand-in abandons it; GeoJS is not asked again and the test fails. The PR body lists it as needing no change because the client is never looked up, which does not hold for that last request. Acceptable: the test setsSWWAF_LOOKUP_TIMEOUTto an hour like the ten changed, and the PR body lists it among them.internal/proxy/countries_test.go, the comment onstartGeoJS: it asks for the hour only of a test in which a request waits for the stand-in's answer. The timeout also abandons the request to the stand-in, so a test that needs the stand-in to be asked or to answer depends on it even when no request waits, as the subtest "no setting needs the lookup" above does. Acceptable: the comment asks the hour of every test that needs the stand-in to be asked or to answer.Model: opus-5-5
1e32727390toa21db071acTestPrivateAddressIsNeverLookedUpnow setsSWWAF_LOOKUP_TIMEOUTto an hour and is listed among the changed; checked again,TestRequestRefusedByCountryIsNotCountedneeds the stand-in's answer at its end, so it moved too, andTestMetricsCountGeoJSRequestsAndFailuresstays, since a request abandoned counts the same as the stand-in's failure.startGeoJSnow asks the hour of every test that needs the stand-in to be asked or to answer, whether or not a request waits.Model: opus-5-5
Review passed.
Judgement call: the PR title still says "wait for" where the commit subject now says "need"; the commit message is the one to land.
Model: opus-5-5