From 68a70c2665e18b9eff9deb876ea4c0c4939f38e5 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Sun, 2 Aug 2026 23:22:04 -0700 Subject: [PATCH] requestCount is an exact count in every runner, not a lower bound (#573) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Go, Python, Ruby, TypeScript and Kotlin all evaluated the `requestCount` assertion as a LOWER bound whenever any mock response carried `Link: rel="next"`: if autoPaginates { if requestCount < expected { fail } } else if requestCount != expected { fail } That is backwards for the fixtures it covered. conformance/tests/pagination.json queues MORE pages than it expects requests in two cases, because stopping early is the behavior under test: Pagination stops at maxPages safety cap 3 pages queued, 2 expected maxItems caps results across pages 3 pages queued, 2 expected Auto-pagination follows Link headers ... 3 pages queued, 3 expected An SDK that ignored the cap and walked all three passed `3 >= 2`. The third is the exposed case: its only assertions are requestCount and noError, so an over-fetch had nothing else to catch it — the other two happen to carry a `responseMeta` truncated assertion that fires instead, coverage by luck. The relaxation was protecting exactly one fixture, "List operation returns first page with Link header" (tagged `link-header`), whose `requestCount: 1` counts FIRST-PAGE requests only and so cannot apply to an auto-paginating SDK. With that one fixture's count taken out of scope, nothing reaching the evaluator needs a lower bound, and `>=` becomes `!=` everywhere. Swift took this in #558 and its shape is what the five now match. The MockEngine/httptest/MSW/respx/WebMock auto-pagination tolerance stays: answering an over-walk with a terminal empty page rather than an error is what lets the tightened assertion report a clean count mismatch. Each runner's predicate moves into that language's SDK-free support module alongside `delay_gaps` — request_count.go, `check_request_count` in runner.py, `RequestCount` in runner.rb, request-count.ts, RequestCount.kt — with a unit test per language. Those five test files are discovered automatically thanks to #572; before it, three of them would have been executed by nothing. Take the ASSERTION out of scope, not the CASE ----------------------------------------------------------------------- An earlier revision of this commit did it the blunt way: it skipped the whole `link-header` CASE in Go, Python, Ruby and TypeScript, copying what Kotlin and Swift already did. That is wrong, and wrong in this stack's own signature shape. The fixture carries THREE assertions, not one: requestCount: 1 inapplicable — the SDK follows the Link header statusCode: 200 perfectly applicable noError perfectly applicable Kotlin and Swift had always skipped the case, so the moment the other four joined them the fixture was skipped by ALL SIX. Its `statusCode` and `noError` assertions had been running in four runners; they then ran in zero. And nothing reports that: the fixture still sits in `conformance/tests/pagination.json`, still passes `conformance-fixtures-check` and `check-fixture-coverage`, so the build stays green over a fixture no runner executes. That is precisely #572's defect — present in the tree, run by nothing — one layer down, committed inside the stack that exists to close it. So the exclusion is now one assertion wide. `requestCountApplies(tags)` (and its per-language spellings) returns false for `link-header`, the evaluator skips that one assertion, and the case runs. Verbatim before/after from the TypeScript runner, which is the arm where a skip is directly observable — pre-narrowing files taken from this stack's pushed head a378d8987, everything else identical: $ npx vitest run --reporter=verbose -t "List operation returns first page with Link header" ↓ runner.test.ts > conformance/pagination.json > List operation returns first page with Link header (TS SDK auto-paginates; follows Link headers by design) [...] Tests 182 skipped (182) REAL_EXIT=0 $ npx vitest run --reporter=verbose -t "List operation returns first page with Link header" ✓ runner.test.ts > conformance/pagination.json > List operation returns first page with Link header 19ms [...] Tests 1 passed | 184 skipped (185) REAL_EXIT=0 ([...] elides vitest's per-file listing of the other 180-odd cases the `-t` filter skipped, and its Test Files / Duration lines. The two quoted lines and the Tests summary are verbatim. Zero passed becomes one passed: the case was executed by nothing and now runs.) Kotlin and Swift keep the whole-case skip, deliberately ----------------------------------------------------------------------- They are not narrowed, and this is a stated exception rather than an oversight. Both derive the response status from the last mock response the SDK consumed, and an auto-paginating SDK walks past the end of a one-response queue, so `statusCode` has nothing to report. Narrowing them was tried; both fail. Verbatim, one line each from the pagination.json section plus the run tail: $ make conformance-kotlin FAIL: List operation returns first page with Link header Expected status code 200, but got no response [...] Passed: 142, Failed: 1, Skipped: 0, Total: 143 REAL_EXIT=2 $ make conformance-swift FAIL: List operation returns first page with Link header Expected status code 200, but got no response [...] Passed: 142, Failed: 1, Skipped: 0, Total: 143 REAL_EXIT=2 ([...] elides the other 142 result lines of each run. Note the exit code: through `make` a recipe failure is 2, not the runner binary's own 1.) Widening those two runners' status model is separate work, not a skip to delete here. Both call sites now carry that reasoning in a comment, so the asymmetry does not read as drift to be "aligned" away. Net effect on coverage: the fixture goes from run by four runners (before this stack) to run by four runners (after), instead of by zero. THIS COMMIT CHANGES NO SKIP COUNT IN ANY RUNNER — see the identical figures in the two verification blocks below. The general gap remains: nothing in the build detects a fixture that every runner skips. That is filed as #602 and referenced from SPEC §19; this commit fixes the instance, not the class. Red proof 1 — the runners on the real fixture ----------------------------------------------------------------------- Mutating "Auto-pagination follows Link headers across multiple pages" to expect 2 while the SDK makes 3 is exactly the over-fetch the lower bound waves through. Base is 07482b9ba, this stack's #553 tip, byte-identical to origin/main for every runner file involved. Each line is the fixture's own result line plus that command's real exit status: $ make conformance- # base 07482b9ba (lower bound) PASS: Auto-pagination follows Link headers across multiple pages REAL_EXIT[base-conformance-go]=0 REAL_EXIT[base-conformance-python]=0 REAL_EXIT[base-conformance-ruby]=0 REAL_EXIT[base-conformance-typescript]=0 REAL_EXIT[base-conformance-kotlin]=0 $ make conformance- # this commit (exact count) FAIL: Auto-pagination follows Link headers across multiple pages Expected 2 requests, got 3 REAL_EXIT[branch-conformance-go]=2 REAL_EXIT[branch-conformance-python]=2 REAL_EXIT[branch-conformance-ruby]=2 REAL_EXIT[branch-conformance-typescript]=2 REAL_EXIT[branch-conformance-kotlin]=2 (The PASS/FAIL lines above are one per language and identical in each; TS prints its as `FAIL runner.test.ts > conformance/pagination.json > Auto-pagination follows Link headers across multiple pages` with `Error: [Auto-pagination follows Link headers across multiple pages] Expected 2 requests, got 3`. Every exit code is real and from `make`, which reports a recipe failure as 2. The fixture edit was reverted; pagination.json is unchanged in this commit.) Red proof 2 — the new unit tests against the old predicate ----------------------------------------------------------------------- Restoring the lower bound in each support module fails the same three cases in every language. These are the bare test binaries, so the exit code is 1, not make's 2: Go --- FAIL: TestRequestCountRejectsAnOverFetch --- FAIL: TestRequestCountMessageNamesBothCounts --- FAIL: TestRequestCountZeroExpectedRequiresZeroActual FAIL github.com/basecamp/basecamp-sdk/conformance/runner/go REAL_EXIT=1 Python 3 failed, 6 passed in 0.10s REAL_EXIT=1 Ruby 9 runs, 13 assertions, 3 failures, 0 errors, 0 skips REAL_EXIT=1 TS Tests 3 failed | 6 passed (9) REAL_EXIT=1 Kotlin 23 tests completed, 3 failed (RequestCountTest: 6 tests, 3 failures — "an over-fetch fails", "the failure message names both counts", "zero expected requires zero actual") REAL_EXIT=1 The three new `requestCountApplies` tests per language are not part of that red: they pin the SCOPE of the exclusion, asserting that a `link-header` fixture keeps its statusCode and noError assertions live. If the suppression ever widens back to the whole case, those fail. Verification, exact-count code, real exit codes, measured on this commit ----------------------------------------------------------------------- make conformance-go Passed: 141, Failed: 0, Skipped: 2 REAL_EXIT=0 make conformance-python 143 passed, 0 failed, 0 skipped REAL_EXIT=0 make conformance-ruby 132 passed, 0 failed, 11 skipped REAL_EXIT=0 make conformance-typescript 183 passed | 2 skipped (185) REAL_EXIT=0 make conformance-kotlin Passed: 142, Failed: 0, Skipped: 1 REAL_EXIT=0 make conformance-swift Passed: 142, Failed: 0, Skipped: 1 REAL_EXIT=0 make conformance-runner-tests-go ok (cached) REAL_EXIT=0 make conformance-runner-tests-python 29 passed, 31 subtests REAL_EXIT=0 make conformance-runner-tests-ruby 11 + 6 + 9 runs, 0 fail REAL_EXIT=0 make conformance-runner-tests-kotlin (--quiet, no output) REAL_EXIT=0 make conformance-runner-tests-swift 39 tests, 0 failures REAL_EXIT=0 ./scripts/check-runner-test-reachability 9 checks passed REAL_EXIT=0 ./scripts/check-runner-test-reachability --self-test 6 cases REAL_EXIT=0 ./scripts/check-replay-decoder-parity 5 checks, 31 operations REAL_EXIT=0 cd conformance/runner/go && go build ./... && go vet ./... REAL_EXIT=0 make lint-actions No findings to report REAL_EXIT=0 And the same six suites at base 07482b9ba, for the skip-count comparison — the figures the "no skip count changes" claim above rests on: base conformance-go Passed: 141, Failed: 0, Skipped: 2 base conformance-python 143 passed, 0 failed, 0 skipped base conformance-ruby 132 passed, 0 failed, 11 skipped base conformance-typescript 174 passed | 2 skipped (176) base conformance-kotlin Passed: 142, Failed: 0, Skipped: 1 Identical skip counts throughout. TypeScript's total rises 176 → 185 because this commit adds request-count.test.ts's 9 tests; its SKIPPED count is 2 on both sides. (Those base figures come from the red-proof-1 base runs, which carry the mutated expectation — a changed expectation moves no skip.) The Go runner's `hasTag` helper is deleted. It existed only for the whole-case branch this commit no longer has, and it had been inserted between `goSDKSkips`' doc comment and `goSDKSkips` itself, so godoc read that comment as documenting `hasTag`. SPEC §19: the `link-header` entry is rewritten. Previously it was a per-runner repeat that claimed Swift's skip was "identical to Kotlin and TypeScript" — untrue, TypeScript had no `link-header` handling at all. It is now one block that states what each runner excludes, why Kotlin and Swift differ, and what the all-six-skipped shape would cost. Local figures. Cite the CI job's own numbers when they differ. Closes #573 --- SPEC.md | 43 +++++-- conformance/runner/go/main.go | 25 ++--- conformance/runner/go/request_count.go | 60 ++++++++++ conformance/runner/go/request_count_test.go | 106 ++++++++++++++++++ conformance/runner/python/runner.py | 69 ++++++++++-- .../runner/python/test_request_count.py | 77 +++++++++++++ conformance/runner/ruby/request_count_test.rb | 80 +++++++++++++ conformance/runner/ruby/runner.rb | 67 +++++++++-- .../ConformanceRunner/Assertions.swift | 10 ++ .../runner/typescript/request-count.test.ts | 81 +++++++++++++ .../runner/typescript/request-count.ts | 58 ++++++++++ conformance/runner/typescript/runner.test.ts | 26 ++--- .../com/basecamp/sdk/conformance/Main.kt | 8 +- .../basecamp/sdk/conformance/RequestCount.kt | 43 +++++++ .../sdk/conformance/RequestCountTest.kt | 57 ++++++++++ 15 files changed, 743 insertions(+), 67 deletions(-) create mode 100644 conformance/runner/go/request_count.go create mode 100644 conformance/runner/go/request_count_test.go create mode 100644 conformance/runner/python/test_request_count.py create mode 100644 conformance/runner/ruby/request_count_test.rb create mode 100644 conformance/runner/typescript/request-count.test.ts create mode 100644 conformance/runner/typescript/request-count.ts create mode 100644 kotlin/conformance/src/main/kotlin/com/basecamp/sdk/conformance/RequestCount.kt create mode 100644 kotlin/conformance/src/test/kotlin/com/basecamp/sdk/conformance/RequestCountTest.kt diff --git a/SPEC.md b/SPEC.md index 3c7972c1a9..ce5a777348 100644 --- a/SPEC.md +++ b/SPEC.md @@ -1911,12 +1911,43 @@ either **waiver-backed** (a `rubric-audit.json` waiver ID), **architectural** with no rubric record — tracked work, not an accepted divergence). A PR that closes a gap deletes exactly its own lines. +One fixture, "List operation returns first page with Link header" +(`conformance/tests/pagination.json`, tagged `link-header`), is handled by a +tag branch rather than a named-skip entry, because the exclusion is +architectural: every SDK auto-paginates by design, so its first-page-only +`requestCount` assertion is inapplicable. What that branch excludes differs by +runner, and the difference is deliberate: + +- **Go, Python, Ruby, TypeScript** suppress the `requestCount` ASSERTION only. + The case still runs, and its `statusCode: 200` and `noError` assertions still + fire. This is what lets `requestCount` be asserted as an exact count + everywhere else (#573) without shedding the rest of the case. + +- **Kotlin and Swift** skip the whole CASE, as they always have. Both derive a + response's status from the last mock response the SDK consumed, and an + auto-paginating SDK walks past the end of a one-response queue, so `statusCode` + reports "no response" and the case cannot pass on those two runners. Narrowing + them to the assertion was tried and reverted: `make conformance-kotlin` and + `make conformance-swift` each then report + `FAIL: List operation returns first page with Link header` / + `Expected status code 200, but got no response` and exit 2. Widening their + status model is separate work, not a skip to delete here. + +Note the shape this avoids: #573 first narrowed nothing and instead added the +whole-case skip to all four remaining runners, which left the fixture skipped by +all six — present in `pagination.json`, passing `conformance-fixtures-check` and +`check-fixture-coverage`, and executed by nothing. That is #572's defect one +layer down. Nothing in the build detects a fixture no runner runs; that gap is +tracked as #602. + **Go** (`conformance/runner/go/main.go` `goSDKSkips`) — architectural; same-origin logic is covered by `TestIsSameOrigin` unit tests: - "Mixed-case host and explicit default port stay on the mocked origin" — Go runner dials `configOverrides.baseUrl` directly; its `httptest` mock owns its origin, so origin-interception normalization does not apply. - "Bracketed IPv6 loopback origin stays on the mocked origin" — same as above. -**Python** (`conformance/runner/python/runner.py` `SKIPS`) — none. +**Python** (`conformance/runner/python/runner.py` `SKIPS`) — none. The +`link-header` fixture above runs; only its `requestCount` assertion is +suppressed. **Ruby** (`conformance/runner/ruby/runner.rb` `RUBY_SKIPS`): - "PUT operation is naturally idempotent" — GET-only retry (waiver 2B.3). @@ -1934,13 +1965,11 @@ logic is covered by `TestIsSameOrigin` unit tests: **TypeScript** (`conformance/runner/typescript/runner.test.ts` `TS_SDK_SKIPS`): - "Large integer IDs preserved without precision loss" — `Number` is 53-bit (waiver 1B.6). -**Kotlin** (`kotlin/conformance/.../Main.kt` — one tag-based branch; `KOTLIN_SKIPS` -is empty): -- "List operation returns first page with Link header" — skipped via the `link-header` tag branch, not `KOTLIN_SKIPS`: Kotlin auto-paginates by design, so a first-page-only requestCount assertion is inapplicable (architectural). +**Kotlin** (`kotlin/conformance/.../Main.kt` — `KOTLIN_SKIPS` is empty) — none +beyond the whole-case `link-header` tag branch described above. -**Swift** (`conformance/runner/swift/.../Runner.swift` — one tag-based branch; -`temporarySkips` is empty): -- "List operation returns first page with Link header" — skipped via the `link-header` tag branch, not `temporarySkips`: Swift auto-paginates by design, so a first-page-only requestCount assertion is inapplicable (architectural, identical to Kotlin and TypeScript). +**Swift** (`conformance/runner/swift/.../Runner.swift` — `temporarySkips` is +empty) — none beyond the whole-case `link-header` tag branch described above. Swift carries no capability skips. It is three-gate on retry (status, network, idempotent POST) and, since #563, retries the authenticated download hop, so diff --git a/conformance/runner/go/main.go b/conformance/runner/go/main.go index ad7c5512f3..d856547c45 100644 --- a/conformance/runner/go/main.go +++ b/conformance/runner/go/main.go @@ -1061,25 +1061,16 @@ func checkAssertion( ) *TestResult { sdkErr := opResult.err - // Detect if any mock response includes a Link header with rel="next". - // The real SDK auto-paginates, so actual requestCount will be >= expected. - hasLinkNextHeader := false - for _, mr := range tc.MockResponses { - if link, ok := mr.Headers["Link"]; ok && strings.Contains(link, `rel="next"`) { - hasLinkNextHeader = true - break - } - } - switch assertion.Type { case "requestCount": - expected := expectedInt(assertion.Expected) - if hasLinkNextHeader { - if requestCount < expected { - return fail(tc, fmt.Sprintf("Expected >= %d requests (SDK auto-paginates), got %d", expected, requestCount)) - } - } else if requestCount != expected { - return fail(tc, fmt.Sprintf("Expected %d requests, got %d", expected, requestCount)) + // The Go SDK auto-paginates list operations, so a fixture that counts + // first-page requests only is inapplicable — but ONLY its count is. + // The rest of the case still runs. See requestCountApplies (#573). + if !requestCountApplies(tc.Tags) { + return nil + } + if msg := checkRequestCount(requestCount, expectedInt(assertion.Expected)); msg != "" { + return fail(tc, msg) } case "delayBetweenRequests": diff --git a/conformance/runner/go/request_count.go b/conformance/runner/go/request_count.go new file mode 100644 index 0000000000..5964f0809c --- /dev/null +++ b/conformance/runner/go/request_count.go @@ -0,0 +1,60 @@ +package main + +import "fmt" + +// checkRequestCount validates one `requestCount` assertion, returning "" when +// it holds and a failure message otherwise. +// +// EXACT, always — including the auto-paginating fixtures. The runners used to +// relax this to a lower bound whenever any mock response carried a +// `Link: rel="next"` header, on the theory that an auto-paginating SDK would +// legitimately make more requests than the fixture named. That is backwards for +// the fixtures the relaxation covered: in conformance/tests/pagination.json, +// "Pagination stops at maxPages safety cap" and "maxItems caps results across +// pages" each queue THREE pages and expect TWO requests, because stopping early +// is the behavior under test. `>=` passes an SDK that ignored the cap and +// walked every page. "Auto-pagination follows Link headers across multiple +// pages" is the exposed case: its only assertions are requestCount and noError, +// so an over-fetch has nothing else to catch it — the other two happen to carry +// a `responseMeta` truncated assertion that fires instead, coverage by luck. +// +// The one fixture where the count genuinely does not apply to an +// auto-paginating SDK — "List operation returns first page with Link header", +// which asserts a single request — carries the `link-header` tag, and +// requestCountApplies reports false for it. Nothing that still reaches this +// function needs the relaxation. +// +// Swift took this in #558; #573 is the same fix for the other five runners. +func checkRequestCount(actual, expected int) string { + if actual != expected { + return fmt.Sprintf("Expected %d requests, got %d", expected, actual) + } + return "" +} + +// linkHeaderTag marks a fixture whose requestCount counts first-page requests +// only, which an auto-paginating SDK cannot satisfy. +const linkHeaderTag = "link-header" + +// requestCountApplies reports whether a fixture's `requestCount` assertion is +// meaningful for this SDK. +// +// SCOPE: this suppresses ONE ASSERTION, not the whole test case. An earlier +// revision skipped the entire `link-header` case in every runner, which took +// its `statusCode: 200` and `noError` assertions down with the inapplicable +// `requestCount` — Kotlin and Swift had always skipped the case wholesale, so +// once Go, Python, Ruby and TypeScript joined them the fixture was executed by +// nothing at all while still sitting in conformance/tests/pagination.json, +// passing conformance-fixtures-check and check-fixture-coverage. That is the +// #572 shape ("present, run by nothing") one layer down. Only the count is +// inapplicable; the status code and the absence of an error are not, and they +// are the assertions that catch an auto-paginating SDK that walked the Link +// header into an error. +func requestCountApplies(tags []string) bool { + for _, tag := range tags { + if tag == linkHeaderTag { + return false + } + } + return true +} diff --git a/conformance/runner/go/request_count_test.go b/conformance/runner/go/request_count_test.go new file mode 100644 index 0000000000..b4e2a5d29b --- /dev/null +++ b/conformance/runner/go/request_count_test.go @@ -0,0 +1,106 @@ +// Bounds contract for the requestCount assertion (#573). +// +// Until this commit the five non-Swift runners evaluated requestCount as a +// LOWER bound whenever any mock response carried `Link: rel="next"`. Every +// committed fixture passes under both rules, so nothing in the suite could tell +// them apart — the same shape as the #563 delayBetweenRequests regression these +// support modules exist to pin. The over-fetch case below is the one that +// distinguishes them, and it is the case that matters: pagination.json's +// maxPages and maxItems fixtures each queue three pages and assert two +// requests, so a lower bound green-passes an SDK that ignored the cap. + +package main + +import "testing" + +func TestRequestCountAcceptsTheExactCount(t *testing.T) { + if msg := checkRequestCount(2, 2); msg != "" { + t.Fatalf("exact match should pass; got %q", msg) + } +} + +func TestRequestCountRejectsAnUnderFetch(t *testing.T) { + if msg := checkRequestCount(1, 2); msg == "" { + t.Fatal("1 request where 2 were expected should fail") + } +} + +func TestRequestCountRejectsAnOverFetch(t *testing.T) { + // The regression. Under the old lower bound this returned "" — an SDK that + // walked all three queued pages instead of stopping at the maxPages cap + // reported a clean pass. + if msg := checkRequestCount(3, 2); msg == "" { + t.Fatal("3 requests where 2 were expected should fail; a lower bound would accept it") + } +} + +func TestRequestCountMessageNamesBothCounts(t *testing.T) { + msg := checkRequestCount(3, 2) + if msg != "Expected 2 requests, got 3" { + t.Fatalf("failure message should name expected and actual; got %q", msg) + } +} + +func TestRequestCountZeroRequestsIsNotAFreePass(t *testing.T) { + // A test whose operation never reached the wire records zero requests. + // That must fail an assertion expecting one, not read as "no data, no + // opinion". + if msg := checkRequestCount(0, 1); msg == "" { + t.Fatal("0 requests where 1 was expected should fail") + } +} + +func TestRequestCountZeroExpectedRequiresZeroActual(t *testing.T) { + if msg := checkRequestCount(0, 0); msg != "" { + t.Fatalf("0 expected and 0 actual should pass; got %q", msg) + } + if msg := checkRequestCount(1, 0); msg == "" { + t.Fatal("1 request where 0 were expected should fail") + } +} + +// Applicability contract (#573). The `link-header` fixture's requestCount is +// inapplicable to an auto-paginating SDK; its statusCode and noError +// assertions are not. Suppressing the CASE instead of the ASSERTION left the +// fixture executed by nothing at all — it stays in pagination.json and passes +// conformance-fixtures-check and check-fixture-coverage either way, so nothing +// else would have reported it. + +func TestRequestCountDoesNotApplyToLinkHeaderFixtures(t *testing.T) { + if requestCountApplies([]string{"pagination", "link-header"}) { + t.Fatal("link-header fixtures must not have their requestCount asserted") + } +} + +func TestRequestCountAppliesToEveryOtherFixture(t *testing.T) { + for _, tags := range [][]string{nil, {}, {"pagination"}, {"retry", "idempotent"}} { + if !requestCountApplies(tags) { + t.Fatalf("requestCount must be asserted for tags %v", tags) + } + } +} + +// The suppression is one assertion wide. If it ever grows to the whole case +// again, the fixture's other two assertions stop running everywhere they still +// run, and nothing downstream notices. +func TestLinkHeaderSuppressionIsScopedToTheCountAssertion(t *testing.T) { + tc := TestCase{ + Name: "List operation returns first page with Link header", + Tags: []string{"pagination", "link-header"}, + Assertions: []Assertion{ + {Type: "requestCount", Expected: float64(1)}, + {Type: "statusCode", Expected: float64(200)}, + {Type: "noError"}, + }, + } + live := 0 + for _, a := range tc.Assertions { + if a.Type == "requestCount" && !requestCountApplies(tc.Tags) { + continue + } + live++ + } + if live != 2 { + t.Fatalf("statusCode and noError must still be evaluated; %d assertion(s) live", live) + } +} diff --git a/conformance/runner/python/runner.py b/conformance/runner/python/runner.py index e8cb14ac04..6e07229a86 100644 --- a/conformance/runner/python/runner.py +++ b/conformance/runner/python/runner.py @@ -110,6 +110,57 @@ def check_delay_gaps( return None +def check_request_count(actual: int, expected: int) -> str | None: + """Validate one ``requestCount`` assertion; None when it holds. + + EXACT, always — including the auto-paginating fixtures. The runner used to + relax this to a lower bound whenever any mock response carried + ``Link: rel="next"``, on the theory that an auto-paginating SDK would + legitimately make more requests than the fixture named. That is backwards + for the fixtures the relaxation covered: in conformance/tests/pagination.json, + "Pagination stops at maxPages safety cap" and "maxItems caps results across + pages" each queue THREE pages and expect TWO requests, because stopping + early is the behavior under test. ``>=`` passes an SDK that ignored the cap + and walked every page. "Auto-pagination follows Link headers across multiple + pages" is the exposed case: its only assertions are requestCount and + noError, so an over-fetch has nothing else to catch it. + + The one fixture where the count genuinely does not apply to an + auto-paginating SDK — "List operation returns first page with Link header", + which asserts a single request — carries the ``link-header`` tag, and + ``request_count_applies`` reports False for it. Nothing that still reaches + this function needs the relaxation. + + Swift took this in #558; #573 is the same fix for the other five runners. + """ + if actual != expected: + return f"Expected {expected} requests, got {actual}" + return None + + +#: Marks a fixture whose requestCount counts first-page requests only, which an +#: auto-paginating SDK cannot satisfy. +LINK_HEADER_TAG = "link-header" + + +def request_count_applies(tags: list[str]) -> bool: + """Whether a fixture's ``requestCount`` assertion is meaningful for this SDK. + + SCOPE: this suppresses ONE ASSERTION, not the whole test case. An earlier + revision skipped the entire ``link-header`` case in every runner, which took + its ``statusCode: 200`` and ``noError`` assertions down with the + inapplicable ``requestCount`` — Kotlin and Swift had always skipped the case + wholesale, so once Go, Python, Ruby and TypeScript joined them the fixture + was executed by nothing at all while still sitting in + conformance/tests/pagination.json, passing conformance-fixtures-check and + check-fixture-coverage. That is the #572 shape ("present, run by nothing") + one layer down. Only the count is inapplicable; the status code and the + absence of an error are not, and they are the assertions that catch an + auto-paginating SDK that walked the Link header into an error. + """ + return LINK_HEADER_TAG not in tags + + @dataclass class TestTracker: requests: list[dict] = field(default_factory=list) @@ -564,13 +615,17 @@ def _verify_assertions(self, *, result: Any, error: Exception | None) -> TestRes for assertion in self._test.get("assertions", []): match assertion["type"]: case "requestCount": - actual = self._tracker.request_count - expected = assertion["expected"] - if self._auto_paginates(): - if actual < expected: - failures.append(f"Expected >= {expected} requests, got {actual}") - elif actual != expected: - failures.append(f"Expected {expected} requests, got {actual}") + # The Python SDK auto-paginates list operations, so a + # fixture that counts first-page requests only is + # inapplicable — but ONLY its count is. The rest of the + # case still runs. See request_count_applies (#573). + if not request_count_applies(self._test.get("tags", [])): + continue + failure = check_request_count( + self._tracker.request_count, assertion["expected"] + ) + if failure is not None: + failures.append(failure) case "delayBetweenRequests": # Not all gaps are retry gaps — the download flow's final diff --git a/conformance/runner/python/test_request_count.py b/conformance/runner/python/test_request_count.py new file mode 100644 index 0000000000..0127e4ad5a --- /dev/null +++ b/conformance/runner/python/test_request_count.py @@ -0,0 +1,77 @@ +"""Bounds contract for the requestCount assertion (#573). + +Until this commit the five non-Swift runners evaluated requestCount as a LOWER +bound whenever any mock response carried ``Link: rel="next"``. Every committed +fixture passes under both rules, so nothing in the suite could tell them apart — +the same shape as the #563 delayBetweenRequests regression these support modules +exist to pin. The over-fetch case below is the one that distinguishes them, and +it is the case that matters: pagination.json's maxPages and maxItems fixtures +each queue three pages and assert two requests, so a lower bound green-passes an +SDK that ignored the cap. +""" +from __future__ import annotations + +from runner import check_request_count, request_count_applies + + +def test_exact_count_passes(): + assert check_request_count(2, 2) is None + + +def test_under_fetch_fails(): + assert check_request_count(1, 2) is not None + + +def test_over_fetch_fails(): + # The regression. Under the old lower bound this returned None — an SDK + # that walked all three queued pages instead of stopping at the maxPages + # cap reported a clean pass. + assert check_request_count(3, 2) is not None + + +def test_failure_message_names_both_counts(): + assert check_request_count(3, 2) == "Expected 2 requests, got 3" + + +def test_zero_requests_is_not_a_free_pass(): + # A test whose operation never reached the wire records zero requests. + # That must fail an assertion expecting one, not read as "no data, no + # opinion". + assert check_request_count(0, 1) is not None + + +def test_zero_expected_requires_zero_actual(): + assert check_request_count(0, 0) is None + assert check_request_count(1, 0) is not None + + +# Applicability contract (#573). The ``link-header`` fixture's requestCount is +# inapplicable to an auto-paginating SDK; its statusCode and noError assertions +# are not. Suppressing the CASE instead of the ASSERTION left the fixture +# executed by nothing at all — it stays in pagination.json and passes +# conformance-fixtures-check and check-fixture-coverage either way, so nothing +# else would have reported it. + + +def test_request_count_does_not_apply_to_link_header_fixtures(): + assert not request_count_applies(["pagination", "link-header"]) + + +def test_request_count_applies_to_every_other_fixture(): + for tags in ([], ["pagination"], ["retry", "idempotent"]): + assert request_count_applies(tags) + + +def test_link_header_suppression_is_scoped_to_the_count_assertion(): + """The suppression is one assertion wide, not one case wide.""" + tags = ["pagination", "link-header"] + assertions = [ + {"type": "requestCount", "expected": 1}, + {"type": "statusCode", "expected": 200}, + {"type": "noError"}, + ] + live = [ + a for a in assertions + if not (a["type"] == "requestCount" and not request_count_applies(tags)) + ] + assert [a["type"] for a in live] == ["statusCode", "noError"] diff --git a/conformance/runner/ruby/request_count_test.rb b/conformance/runner/ruby/request_count_test.rb new file mode 100644 index 0000000000..e4a5e2dc99 --- /dev/null +++ b/conformance/runner/ruby/request_count_test.rb @@ -0,0 +1,80 @@ +# frozen_string_literal: true + +# Bounds contract for the requestCount assertion (#573). +# +# Until this commit the five non-Swift runners evaluated requestCount as a LOWER +# bound whenever any mock response carried `Link: rel="next"`. Every committed +# fixture passes under both rules, so nothing in the suite could tell them +# apart — the same shape as the #563 delayBetweenRequests regression these +# support modules exist to pin. The over-fetch case below is the one that +# distinguishes them, and it is the case that matters: pagination.json's +# maxPages and maxItems fixtures each queue three pages and assert two requests, +# so a lower bound green-passes an SDK that ignored the cap. +# +# Run: `bundle exec ruby request_count_test.rb` + +require "minitest/autorun" +require_relative "runner" + +class RequestCountTest < Minitest::Test + def test_exact_count_passes + assert_nil RequestCount.check(2, 2) + end + + def test_under_fetch_fails + refute_nil RequestCount.check(1, 2) + end + + def test_over_fetch_fails + # The regression. Under the old lower bound this returned nil — an SDK that + # walked all three queued pages instead of stopping at the maxPages cap + # reported a clean pass. + refute_nil RequestCount.check(3, 2) + end + + def test_failure_message_names_both_counts + assert_equal "Expected 2 requests, got 3", RequestCount.check(3, 2) + end + + def test_zero_requests_is_not_a_free_pass + # A test whose operation never reached the wire records zero requests. That + # must fail an assertion expecting one, not read as "no data, no opinion". + refute_nil RequestCount.check(0, 1) + end + + def test_zero_expected_requires_zero_actual + assert_nil RequestCount.check(0, 0) + refute_nil RequestCount.check(1, 0) + end + + # Applicability contract (#573). The `link-header` fixture's requestCount is + # inapplicable to an auto-paginating SDK; its statusCode and noError + # assertions are not. Suppressing the CASE instead of the ASSERTION left the + # fixture executed by nothing at all — it stays in pagination.json and passes + # conformance-fixtures-check and check-fixture-coverage either way, so + # nothing else would have reported it. + + def test_request_count_does_not_apply_to_link_header_fixtures + refute RequestCount.applies?(["pagination", "link-header"]) + end + + def test_request_count_applies_to_every_other_fixture + [nil, [], ["pagination"], ["retry", "idempotent"]].each do |tags| + assert RequestCount.applies?(tags), "requestCount must be asserted for #{tags.inspect}" + end + end + + # The suppression is one assertion wide, not one case wide. + def test_link_header_suppression_is_scoped_to_the_count_assertion + tags = ["pagination", "link-header"] + assertions = [ + { "type" => "requestCount", "expected" => 1 }, + { "type" => "statusCode", "expected" => 200 }, + { "type" => "noError" } + ] + live = assertions.reject do |a| + a["type"] == "requestCount" && !RequestCount.applies?(tags) + end + assert_equal ["statusCode", "noError"], live.map { |a| a["type"] } + end +end diff --git a/conformance/runner/ruby/runner.rb b/conformance/runner/ruby/runner.rb index 1619ac2a2f..d228069244 100644 --- a/conformance/runner/ruby/runner.rb +++ b/conformance/runner/ruby/runner.rb @@ -96,6 +96,57 @@ def self.named_gap_failure(delays, min_delay, index, request_count) end end +# The requestCount assertion contract, kept apart from the runner so its bounds +# branches are unit-testable (request_count_test.rb). +module RequestCount + # Validates one assertion, returning nil when it holds and a failure message + # otherwise. + # + # EXACT, always — including the auto-paginating fixtures. The runner used to + # relax this to a lower bound whenever any mock response carried + # `Link: rel="next"`, on the theory that an auto-paginating SDK would + # legitimately make more requests than the fixture named. That is backwards + # for the fixtures the relaxation covered: in conformance/tests/pagination.json, + # "Pagination stops at maxPages safety cap" and "maxItems caps results across + # pages" each queue THREE pages and expect TWO requests, because stopping + # early is the behavior under test. `>=` passes an SDK that ignored the cap + # and walked every page. "Auto-pagination follows Link headers across + # multiple pages" is the exposed case: its only assertions are requestCount + # and noError, so an over-fetch has nothing else to catch it. + # + # The one fixture where the count genuinely does not apply to an + # auto-paginating SDK — "List operation returns first page with Link header", + # which asserts a single request — carries the `link-header` tag, and + # `applies?` returns false for it. Nothing that still reaches this method + # needs the relaxation. + # + # Swift took this in #558; #573 is the same fix for the other five runners. + def self.check(actual, expected) + "Expected #{expected} requests, got #{actual}" unless actual == expected + end + + # Marks a fixture whose requestCount counts first-page requests only, which + # an auto-paginating SDK cannot satisfy. + LINK_HEADER_TAG = "link-header" + + # Whether a fixture's `requestCount` assertion is meaningful for this SDK. + # + # SCOPE: this suppresses ONE ASSERTION, not the whole test case. An earlier + # revision skipped the entire `link-header` case in every runner, which took + # its `statusCode: 200` and `noError` assertions down with the inapplicable + # `requestCount` — Kotlin and Swift had always skipped the case wholesale, so + # once Go, Python, Ruby and TypeScript joined them the fixture was executed by + # nothing at all while still sitting in conformance/tests/pagination.json, + # passing conformance-fixtures-check and check-fixture-coverage. That is the + # #572 shape ("present, run by nothing") one layer down. Only the count is + # inapplicable; the status code and the absence of an error are not, and they + # are the assertions that catch an auto-paginating SDK that walked the Link + # header into an error. + def self.applies?(tags) + !(tags || []).include?(LINK_HEADER_TAG) + end +end + # Test execution tracking class TestTracker attr_reader :requests @@ -656,16 +707,12 @@ def verify_assertions(result:, error:) (@test["assertions"] || []).each do |assertion| case assertion["type"] when "requestCount" - actual = @tracker.request_count - expected = assertion["expected"] - if auto_paginates? - unless actual >= expected - failures << "Expected >= #{expected} requests (SDK auto-paginates), got #{actual}" - end - else - unless actual == expected - failures << "Expected #{expected} requests, got #{actual}" - end + # The Ruby SDK auto-paginates list operations, so a fixture that counts + # first-page requests only is inapplicable — but ONLY its count is. The + # rest of the case still runs. See RequestCount.applies? (#573). + if RequestCount.applies?(@test["tags"]) + failure = RequestCount.check(@tracker.request_count, assertion["expected"]) + failures << failure if failure end when "delayBetweenRequests" diff --git a/conformance/runner/swift/Sources/ConformanceRunner/Assertions.swift b/conformance/runner/swift/Sources/ConformanceRunner/Assertions.swift index 23ac2dfcf5..86c67cb334 100644 --- a/conformance/runner/swift/Sources/ConformanceRunner/Assertions.swift +++ b/conformance/runner/swift/Sources/ConformanceRunner/Assertions.swift @@ -232,6 +232,16 @@ func evaluateAssertions( // every page. The first-page-only fixture, the one case where the // count genuinely does not apply to an auto-paginating SDK, is // excluded by its own `link-header` tag before it reaches here. + // + // Swift excludes the whole CASE where Go, Python, Ruby and + // TypeScript exclude only this ASSERTION (#573). Deliberate, not + // drift: `httpStatus` is the status of the last mock response the + // SDK consumed, and an auto-paginating SDK walks past the end of a + // one-response queue, so the fixture's `statusCode: 200` cannot be + // satisfied. Narrowing this arm was tried and reverted — `make + // conformance-swift` then reports `Expected status code 200, but + // got no response` and exits 2. Do not "align" it with the other + // four without widening the status model first. if requestCount != expected { return .fail("Expected \(expected) requests, got \(requestCount)") } diff --git a/conformance/runner/typescript/request-count.test.ts b/conformance/runner/typescript/request-count.test.ts new file mode 100644 index 0000000000..d404db3c2a --- /dev/null +++ b/conformance/runner/typescript/request-count.test.ts @@ -0,0 +1,81 @@ +/** + * Bounds contract for the requestCount assertion (#573). + * + * Until this commit the five non-Swift runners evaluated requestCount as a + * LOWER bound whenever any mock response carried `Link: rel="next"`. Every + * committed fixture passes under both rules, so nothing in the suite could tell + * them apart — the same shape as the #563 delayBetweenRequests regression these + * support modules exist to pin. The over-fetch case below is the one that + * distinguishes them, and it is the case that matters: pagination.json's + * maxPages and maxItems fixtures each queue three pages and assert two + * requests, so a lower bound green-passes an SDK that ignored the cap. + */ + +import { describe, expect, it } from "vitest"; +import { checkRequestCount, requestCountApplies } from "./request-count.js"; + +describe("checkRequestCount", () => { + it("passes on the exact count", () => { + expect(checkRequestCount(2, 2)).toBeUndefined(); + }); + + it("fails an under-fetch", () => { + expect(checkRequestCount(1, 2)).toBeDefined(); + }); + + it("fails an over-fetch", () => { + // The regression. Under the old lower bound this returned undefined — an + // SDK that walked all three queued pages instead of stopping at the + // maxPages cap reported a clean pass. + expect(checkRequestCount(3, 2)).toBeDefined(); + }); + + it("names both counts in the failure message", () => { + expect(checkRequestCount(3, 2)).toBe("Expected 2 requests, got 3"); + }); + + it("does not treat a zero-request run as a free pass", () => { + // A test whose operation never reached the wire records zero requests. + // That must fail an assertion expecting one, not read as "no data, no + // opinion". + expect(checkRequestCount(0, 1)).toBeDefined(); + }); + + it("requires zero actual when zero is expected", () => { + expect(checkRequestCount(0, 0)).toBeUndefined(); + expect(checkRequestCount(1, 0)).toBeDefined(); + }); +}); + +/** + * Applicability contract (#573). The `link-header` fixture's requestCount is + * inapplicable to an auto-paginating SDK; its statusCode and noError + * assertions are not. Suppressing the CASE instead of the ASSERTION left the + * fixture executed by nothing at all — it stays in pagination.json and passes + * conformance-fixtures-check and check-fixture-coverage either way, so nothing + * else would have reported it. + */ +describe("requestCountApplies", () => { + it("does not apply to link-header fixtures", () => { + expect(requestCountApplies(["pagination", "link-header"])).toBe(false); + }); + + it("applies to every other fixture", () => { + for (const tags of [undefined, [], ["pagination"], ["retry", "idempotent"]]) { + expect(requestCountApplies(tags)).toBe(true); + } + }); + + it("suppresses one assertion, not the whole case", () => { + const tags = ["pagination", "link-header"]; + const assertions = [ + { type: "requestCount", expected: 1 }, + { type: "statusCode", expected: 200 }, + { type: "noError" }, + ]; + const live = assertions.filter( + (a) => !(a.type === "requestCount" && !requestCountApplies(tags)), + ); + expect(live.map((a) => a.type)).toEqual(["statusCode", "noError"]); + }); +}); diff --git a/conformance/runner/typescript/request-count.ts b/conformance/runner/typescript/request-count.ts new file mode 100644 index 0000000000..a495f99733 --- /dev/null +++ b/conformance/runner/typescript/request-count.ts @@ -0,0 +1,58 @@ +/** + * The `requestCount` assertion contract, kept apart from the runner so its + * bounds branches are unit-testable (request-count.test.ts). + */ + +/** + * Validates one assertion, returning `undefined` when it holds and a failure + * message otherwise. + * + * EXACT, always — including the auto-paginating fixtures. The runner used to + * relax this to a lower bound whenever any mock response carried + * `Link: rel="next"`, on the theory that an auto-paginating SDK would + * legitimately make more requests than the fixture named. That is backwards for + * the fixtures the relaxation covered: in conformance/tests/pagination.json, + * "Pagination stops at maxPages safety cap" and "maxItems caps results across + * pages" each queue THREE pages and expect TWO requests, because stopping early + * is the behavior under test. `>=` passes an SDK that ignored the cap and + * walked every page. "Auto-pagination follows Link headers across multiple + * pages" is the exposed case: its only assertions are requestCount and noError, + * so an over-fetch has nothing else to catch it — the other two happen to carry + * a `responseMeta` truncated assertion that fires instead, coverage by luck. + * + * The one fixture where the count genuinely does not apply to an + * auto-paginating SDK — "List operation returns first page with Link header", + * which asserts a single request — carries the `link-header` tag, and + * `requestCountApplies` returns false for it. Nothing that still reaches this + * function needs the relaxation. + * + * Swift took this in #558; #573 is the same fix for the other five runners. + */ +export function checkRequestCount(actual: number, expected: number): string | undefined { + return actual === expected ? undefined : `Expected ${expected} requests, got ${actual}`; +} + +/** + * Marks a fixture whose requestCount counts first-page requests only, which an + * auto-paginating SDK cannot satisfy. + */ +export const LINK_HEADER_TAG = "link-header"; + +/** + * Whether a fixture's `requestCount` assertion is meaningful for this SDK. + * + * SCOPE: this suppresses ONE ASSERTION, not the whole test case. An earlier + * revision skipped the entire `link-header` case in every runner, which took + * its `statusCode: 200` and `noError` assertions down with the inapplicable + * `requestCount` — Kotlin and Swift had always skipped the case wholesale, so + * once Go, Python, Ruby and TypeScript joined them the fixture was executed by + * nothing at all while still sitting in conformance/tests/pagination.json, + * passing conformance-fixtures-check and check-fixture-coverage. That is the + * #572 shape ("present, run by nothing") one layer down. Only the count is + * inapplicable; the status code and the absence of an error are not, and they + * are the assertions that catch an auto-paginating SDK that walked the Link + * header into an error. + */ +export function requestCountApplies(tags: string[] | undefined): boolean { + return !(tags ?? []).includes(LINK_HEADER_TAG); +} diff --git a/conformance/runner/typescript/runner.test.ts b/conformance/runner/typescript/runner.test.ts index 2a7b029437..b38ff1ba63 100644 --- a/conformance/runner/typescript/runner.test.ts +++ b/conformance/runner/typescript/runner.test.ts @@ -17,6 +17,7 @@ import * as path from "node:path"; import { fileURLToPath } from "node:url"; import { checkDelayGaps } from "./delay-gaps.js"; import { errorRaisedFailure } from "./error-raised.js"; +import { checkRequestCount, requestCountApplies } from "./request-count.js"; // ============================================================================= // Types mirroring conformance/schema.json @@ -842,12 +843,6 @@ function checkAssertions( tracker: ReturnType, result: { error?: BasecampError | Error; httpStatus?: number; meta?: Record }, ): void { - // Detect if any mock response includes a Link header with rel="next". - // The TS SDK auto-paginates (follows all Link next headers), so the - // actual requestCount will be higher than what the conformance test - // expects. In this case, assert >= instead of strict equality. - const hasLinkNextHeader = suiteHasLinkNext(tc); - // DownloadURL implicit invariant: hop 1 must hit the test case path. // The MSW handler is origin-wide so hop 2's relative-resolved URL is // served, but a regression that misroutes hop 1 to a different path on @@ -895,19 +890,12 @@ function checkAssertions( for (const assertion of tc.assertions) { switch (assertion.type) { case "requestCount": { - const expected = Number(assertion.expected); - if (hasLinkNextHeader) { - // TS SDK auto-paginates: expect at least the specified count - expect( - tracker.requestCount(), - `[${tc.name}] expected >= ${expected} requests (SDK auto-paginates), got ${tracker.requestCount()}`, - ).toBeGreaterThanOrEqual(expected); - } else { - expect( - tracker.requestCount(), - `[${tc.name}] expected ${expected} requests, got ${tracker.requestCount()}`, - ).toBe(expected); - } + // The TS SDK auto-paginates list operations, so a fixture that counts + // first-page requests only is inapplicable — but ONLY its count is. + // The rest of the case still runs. See requestCountApplies (#573). + if (!requestCountApplies(tc.tags)) break; + const failure = checkRequestCount(tracker.requestCount(), Number(assertion.expected)); + if (failure) throw new Error(`[${tc.name}] ${failure}`); break; } diff --git a/kotlin/conformance/src/main/kotlin/com/basecamp/sdk/conformance/Main.kt b/kotlin/conformance/src/main/kotlin/com/basecamp/sdk/conformance/Main.kt index 03698233cb..95357831bd 100644 --- a/kotlin/conformance/src/main/kotlin/com/basecamp/sdk/conformance/Main.kt +++ b/kotlin/conformance/src/main/kotlin/com/basecamp/sdk/conformance/Main.kt @@ -347,13 +347,7 @@ private fun runTest(tc: TestCase): TestResult { "requestCount" -> { val expected = assertion.expected?.asInt() ?: return TestResult(false, "requestCount assertion missing expected value") - if (autoPaginates) { - if (requestCount < expected) { - return TestResult(false, "Expected >= $expected requests (SDK auto-paginates), got $requestCount") - } - } else if (requestCount != expected) { - return TestResult(false, "Expected $expected requests, got $requestCount") - } + checkRequestCount(requestCount, expected)?.let { return TestResult(false, it) } } "statusCode" -> { diff --git a/kotlin/conformance/src/main/kotlin/com/basecamp/sdk/conformance/RequestCount.kt b/kotlin/conformance/src/main/kotlin/com/basecamp/sdk/conformance/RequestCount.kt new file mode 100644 index 0000000000..2bedf6e268 --- /dev/null +++ b/kotlin/conformance/src/main/kotlin/com/basecamp/sdk/conformance/RequestCount.kt @@ -0,0 +1,43 @@ +package com.basecamp.sdk.conformance + +/** + * Validates one `requestCount` assertion, returning `null` when it holds and a + * failure message otherwise. + * + * EXACT, always — including the auto-paginating fixtures. The runner used to + * relax this to a lower bound whenever any mock response carried + * `Link: rel="next"`, on the theory that an auto-paginating SDK would + * legitimately make more requests than the fixture named. That is backwards for + * the fixtures the relaxation covered: in conformance/tests/pagination.json, + * "Pagination stops at maxPages safety cap" and "maxItems caps results across + * pages" each queue THREE pages and expect TWO requests, because stopping early + * is the behavior under test. `>=` passes an SDK that ignored the cap and + * walked every page. "Auto-pagination follows Link headers across multiple + * pages" is the exposed case: its only assertions are requestCount and noError, + * so an over-fetch has nothing else to catch it — the other two happen to carry + * a `responseMeta` truncated assertion that fires instead, coverage by luck. + * + * The one fixture where the count genuinely does not apply to an + * auto-paginating SDK — "List operation returns first page with Link header", + * which asserts a single request — carries the `link-header` tag and is already + * excluded by the runner before any assertion is evaluated. Nothing that still + * reaches this function needs the relaxation. + * + * Kotlin excludes the whole CASE where Go, Python, Ruby and TypeScript exclude + * only the requestCount ASSERTION (#573). That asymmetry is deliberate, not + * drift: `httpStatusCode` here is the status of the last MockEngine response + * the SDK consumed, and an auto-paginating SDK walks past the end of a + * one-response queue, so the fixture's `statusCode: 200` cannot be satisfied. + * Narrowing this arm to the assertion was tried and reverted — `make + * conformance-kotlin` then reports `Expected status code 200, but got no + * response` and exits 2. Widening the status model is separate work; until + * then, do not "align" this branch with the other four. + * + * The MockEngine's auto-pagination tolerance stays: answering an over-walk with + * a terminal empty page rather than an error is what lets this tightened + * assertion report a clean count mismatch instead of an opaque transport error. + * + * Swift took this in #558; #573 is the same fix for the other five runners. + */ +fun checkRequestCount(actual: Int, expected: Int): String? = + if (actual == expected) null else "Expected $expected requests, got $actual" diff --git a/kotlin/conformance/src/test/kotlin/com/basecamp/sdk/conformance/RequestCountTest.kt b/kotlin/conformance/src/test/kotlin/com/basecamp/sdk/conformance/RequestCountTest.kt new file mode 100644 index 0000000000..0da90264ec --- /dev/null +++ b/kotlin/conformance/src/test/kotlin/com/basecamp/sdk/conformance/RequestCountTest.kt @@ -0,0 +1,57 @@ +package com.basecamp.sdk.conformance + +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNotNull +import kotlin.test.assertNull + +/** + * Bounds contract for the requestCount assertion (#573). + * + * Until this commit the five non-Swift runners evaluated requestCount as a + * LOWER bound whenever any mock response carried `Link: rel="next"`. Every + * committed fixture passes under both rules, so nothing in the suite could tell + * them apart — the same shape as the #563 delayBetweenRequests regression these + * support modules exist to pin. The over-fetch case below is the one that + * distinguishes them, and it is the case that matters: pagination.json's + * maxPages and maxItems fixtures each queue three pages and assert two + * requests, so a lower bound green-passes an SDK that ignored the cap. + */ +class RequestCountTest { + @Test + fun `the exact count passes`() { + assertNull(checkRequestCount(2, 2)) + } + + @Test + fun `an under-fetch fails`() { + assertNotNull(checkRequestCount(1, 2)) + } + + @Test + fun `an over-fetch fails`() { + // The regression. Under the old lower bound this returned null — an SDK + // that walked all three queued pages instead of stopping at the + // maxPages cap reported a clean pass. + assertNotNull(checkRequestCount(3, 2)) + } + + @Test + fun `the failure message names both counts`() { + assertEquals("Expected 2 requests, got 3", checkRequestCount(3, 2)) + } + + @Test + fun `a zero-request run is not a free pass`() { + // A test whose operation never reached the wire records zero requests. + // That must fail an assertion expecting one, not read as "no data, no + // opinion". + assertNotNull(checkRequestCount(0, 1)) + } + + @Test + fun `zero expected requires zero actual`() { + assertNull(checkRequestCount(0, 0)) + assertNotNull(checkRequestCount(1, 0)) + } +}