diff --git a/conformance/runner/python/runner.py b/conformance/runner/python/runner.py index 7179a4ab..fdf94cd9 100644 --- a/conformance/runner/python/runner.py +++ b/conformance/runner/python/runner.py @@ -64,8 +64,13 @@ class OperationMapper: def __init__(self, account_client): self._account = account_client - def __call__(self, operation: str, *, path_params: dict, query_params: dict, body: dict | None) -> Any: + def __call__(self, operation: str, *, path_params: dict, query_params: dict, body: dict | None, path: str = "") -> Any: match operation: + case "DownloadURL": + if not path: + raise ValueError("DownloadURL test case requires a non-empty path") + raw_url = "https://storage.3.basecamp.com" + path + return self._account.download_url(raw_url) case "ListProjects": return self._account.projects.list() case "GetProject": @@ -144,6 +149,7 @@ def run(self) -> TestResult: path_params=self._test.get("pathParams", {}), query_params=self._test.get("queryParams", {}), body=self._test.get("requestBody"), + path=self._test.get("path", ""), ) return self._verify_assertions(result=result, error=None) except Exception as e: @@ -184,7 +190,19 @@ def side_effect(request: httpx.Request) -> httpx.Response: else: return httpx.Response(500, content=b'{"error":"No more mock responses"}', headers={"Content-Type": "application/json"}) - respx.route(method=method, url__regex=f".*{re.escape(path)}.*").mock(side_effect=side_effect) + if self._test["operation"] == "DownloadURL": + # Catch-all on the active client's host: the SDK rewrites the synthetic + # download URL onto base_url, then resolves a relative Location to a + # second path on the same host. Constraining to the origin (derived + # from configOverrides.baseUrl when present) ensures a misroute to a + # different host fails instead of consuming a queued response. + overrides = self._test.get("configOverrides") or {} + download_base = overrides.get("baseUrl", "https://3.basecampapi.com") + parsed = urlparse(download_base) + origin = f"{parsed.scheme}://{parsed.netloc}" + respx.route(method=method, url__regex=rf"{re.escape(origin)}/.*").mock(side_effect=side_effect) + else: + respx.route(method=method, url__regex=f".*{re.escape(path)}.*").mock(side_effect=side_effect) def _auto_paginates(self) -> bool: return any( @@ -192,9 +210,31 @@ def _auto_paginates(self) -> bool: for r in self._test.get("mockResponses", []) ) + def _request_headers_at(self, index: int) -> dict | None: + """Return captured headers at index (0-based; negative counts from end), or None if out of range.""" + requests = self._tracker.requests + n = len(requests) + if n == 0: + return None + if index < 0: + index += n + if index < 0 or index >= n: + return None + return requests[index]["headers"] + def _verify_assertions(self, *, result: Any, error: Exception | None) -> TestResult: failures: list[str] = [] + # DownloadURL implicit invariant: hop 1 must hit the test case path. + # The mock route is origin-wide so hop 2's relative-resolved URL is + # served, but a regression that misroutes hop 1 to a different path + # on the same origin would otherwise pass silently. + if self._test["operation"] == "DownloadURL" and self._tracker.requests: + expected_path = self._test["path"] + actual_path = urlparse(self._tracker.requests[0]["url"]).path + if actual_path != expected_path: + failures.append(f"DownloadURL hop 1 expected path {expected_path!r}, got {actual_path!r}") + for assertion in self._test.get("assertions", []): match assertion["type"]: case "requestCount": @@ -292,21 +332,36 @@ def _verify_assertions(self, *, result: Any, error: Exception | None) -> TestRes case "headerInjected": header_name = assertion["path"] expected = assertion["expected"] - if not self._tracker.requests: - failures.append(f"Expected header {header_name}={expected!r}, but no requests recorded") + idx = assertion.get("index", 0) + headers = self._request_headers_at(idx) + if headers is None: + failures.append(f"Expected header {header_name}={expected!r} on request index {idx}, but only {self._tracker.request_count} requests were recorded") else: - actual = self._tracker.requests[0]["headers"].get(header_name.lower()) + actual = headers.get(header_name.lower()) if actual != expected: - failures.append(f"Expected header {header_name}={expected!r}, got {actual!r}") + failures.append(f"Expected header {header_name}={expected!r} on request index {idx}, got {actual!r}") case "headerPresent": header_name = assertion["path"] - if not self._tracker.requests: - failures.append(f"Expected header {header_name} to be present, but no requests recorded") + idx = assertion.get("index", 0) + headers = self._request_headers_at(idx) + if headers is None: + failures.append(f"Expected header {header_name} on request index {idx}, but only {self._tracker.request_count} requests were recorded") else: - actual = self._tracker.requests[0]["headers"].get(header_name.lower()) + actual = headers.get(header_name.lower()) if not actual: - failures.append(f"Expected header {header_name} to be present, but it was missing") + failures.append(f"Expected header {header_name} on request index {idx}, but it was empty or missing") + + case "headerAbsent": + header_name = assertion["path"] + idx = assertion.get("index", 0) + headers = self._request_headers_at(idx) + if headers is None: + failures.append(f"Expected header {header_name} absent on request index {idx}, but only {self._tracker.request_count} requests were recorded") + else: + actual = headers.get(header_name.lower()) + if actual: + failures.append(f"Expected header {header_name} absent on request index {idx}, got {actual!r}") case "requestScheme": expected = assertion["expected"] @@ -372,24 +427,18 @@ def _get_error_field(error: Exception, field_path: str) -> Any: class ConformanceRunner: - _DOWNLOAD_SKIP = "Python runner does not yet dispatch DownloadURL (tracked as follow-up)" + _DOWNLOAD_RETRY_SKIP = "Python SDK download path uses get_no_retry; retry on 5xx / Retry-After is not implemented" _MULTIHOP_SKIP = "Python runner's respx stub matches a single path; multi-hop download fixtures need per-hop stub wiring (tracked as follow-up with DownloadURL)" SKIPS: set[str] = { "maxItems caps results across pages", - "DownloadURL auth'd first hop 302s to signed URL", - "DownloadURL direct 2xx body", "DownloadURL retries on 503 at the auth'd first hop", "DownloadURL honors Retry-After on 429 at the auth'd first hop", - "DownloadURL surfaces redirect with no Location", "UploadsDownload delegates through DownloadURL primitive", } SKIP_REASONS: dict[str, str] = { "maxItems caps results across pages": "Python SDK list methods don't expose a public max_items parameter", - "DownloadURL auth'd first hop 302s to signed URL": _DOWNLOAD_SKIP, - "DownloadURL direct 2xx body": _DOWNLOAD_SKIP, - "DownloadURL retries on 503 at the auth'd first hop": _DOWNLOAD_SKIP, - "DownloadURL honors Retry-After on 429 at the auth'd first hop": _DOWNLOAD_SKIP, - "DownloadURL surfaces redirect with no Location": _DOWNLOAD_SKIP, + "DownloadURL retries on 503 at the auth'd first hop": _DOWNLOAD_RETRY_SKIP, + "DownloadURL honors Retry-After on 429 at the auth'd first hop": _DOWNLOAD_RETRY_SKIP, "UploadsDownload delegates through DownloadURL primitive": _MULTIHOP_SKIP, } diff --git a/conformance/runner/ruby/runner.rb b/conformance/runner/ruby/runner.rb index 23e5a549..8983f8a6 100644 --- a/conformance/runner/ruby/runner.rb +++ b/conformance/runner/ruby/runner.rb @@ -66,8 +66,12 @@ def initialize(account_client) @account = account_client end - def call(operation, path_params: {}, query_params: {}, body: nil) + def call(operation, path_params: {}, query_params: {}, body: nil, path: "") case operation + when "DownloadURL" + raise "DownloadURL test case requires a non-empty path" if path.nil? || path.empty? + raw_url = "https://storage.3.basecamp.com" + path + @account.download_url(raw_url) when "ListProjects" @account.projects.list.to_a when "GetProject" @@ -170,15 +174,12 @@ def call(operation, path_params: {}, query_params: {}, body: nil) "Missing X-Total-Count returns zero", "Pagination stops at maxPages safety cap", "maxItems caps results across pages", - "DownloadURL auth'd first hop 302s to signed URL", - "DownloadURL direct 2xx body", "DownloadURL retries on 503 at the auth'd first hop", "DownloadURL honors Retry-After on 429 at the auth'd first hop", - "DownloadURL surfaces redirect with no Location", "UploadsDownload delegates through DownloadURL primitive", ].freeze) -DOWNLOAD_SKIP = "Ruby runner does not yet dispatch DownloadURL (tracked as follow-up)".freeze +DOWNLOAD_RETRY_SKIP = "Ruby SDK download path uses http.get_no_retry; retry on 5xx / Retry-After is not implemented".freeze MULTIHOP_SKIP = "Ruby runner's WebMock stub matches a single path; multi-hop download fixtures need per-hop stub wiring (tracked as follow-up with DownloadURL)".freeze RUBY_SKIP_REASONS = { "PUT operation is naturally idempotent" => "Ruby SDK only retries GET", @@ -187,11 +188,8 @@ def call(operation, path_params: {}, query_params: {}, body: nil) "Missing X-Total-Count returns zero" => "Ruby SDK paginate doesn't expose X-Total-Count metadata", "Pagination stops at maxPages safety cap" => "Ruby SDK paginate doesn't expose truncation metadata", "maxItems caps results across pages" => "Ruby SDK paginate doesn't support maxItems", - "DownloadURL auth'd first hop 302s to signed URL" => DOWNLOAD_SKIP, - "DownloadURL direct 2xx body" => DOWNLOAD_SKIP, - "DownloadURL retries on 503 at the auth'd first hop" => DOWNLOAD_SKIP, - "DownloadURL honors Retry-After on 429 at the auth'd first hop" => DOWNLOAD_SKIP, - "DownloadURL surfaces redirect with no Location" => DOWNLOAD_SKIP, + "DownloadURL retries on 503 at the auth'd first hop" => DOWNLOAD_RETRY_SKIP, + "DownloadURL honors Retry-After on 429 at the auth'd first hop" => DOWNLOAD_RETRY_SKIP, "UploadsDownload delegates through DownloadURL primitive" => MULTIHOP_SKIP, }.freeze @@ -212,7 +210,8 @@ def run @test["operation"], path_params: @test["pathParams"] || {}, query_params: @test["queryParams"] || {}, - body: @test["requestBody"] + body: @test["requestBody"], + path: @test["path"] || "" ) verify_assertions(result: result, error: nil) rescue StandardError => e @@ -243,7 +242,23 @@ def setup_mock_responses # Register the stub with a block to track requests and return queued responses method = @test["method"]&.downcase&.to_sym || :get - url_pattern = %r{#{Regexp.escape(path)}} + url_pattern = if @test["operation"] == "DownloadURL" + # Catch-all on the active client's host: the SDK rewrites the synthetic + # download URL onto base_url, then resolves a relative Location to a + # second path on the same host. Constraining to the origin (derived + # from configOverrides.baseUrl when present) ensures a misroute to a + # different host fails instead of consuming a queued response. + overrides = @test["configOverrides"] || {} + download_base = overrides["baseUrl"] || "https://3.basecampapi.com" + download_uri = URI.parse(download_base) + port_part = download_uri.port && download_uri.port != download_uri.default_port \ + ? ":#{download_uri.port}" \ + : "" + download_origin = "#{download_uri.scheme}://#{download_uri.host}#{port_part}" + %r{\A#{Regexp.escape(download_origin)}/} + else + %r{#{Regexp.escape(path)}} + end stub = WebMock.stub_request(method, url_pattern) @@ -273,9 +288,29 @@ def auto_paginates? end end + # Return captured headers at index (0-based; negative counts from end), or nil if out of range. + def request_headers_at(index) + requests = @tracker.requests + n = requests.size + resolved = index < 0 ? index + n : index + resolved >= 0 && resolved < n ? requests[resolved][:headers] : nil + end + def verify_assertions(result:, error:) failures = [] + # DownloadURL implicit invariant: hop 1 must hit the test case path. + # The mock route is origin-wide so hop 2's relative-resolved URL is + # served, but a regression that misroutes hop 1 to a different path + # on the same origin would otherwise pass silently. + if @test["operation"] == "DownloadURL" && @tracker.requests.any? + expected_path = @test["path"] + actual_path = URI.parse(@tracker.requests.first[:uri]).path + unless actual_path == expected_path + failures << "DownloadURL hop 1 expected path #{expected_path.inspect}, got #{actual_path.inspect}" + end + end + (@test["assertions"] || []).each do |assertion| case assertion["type"] when "requestCount" @@ -411,25 +446,40 @@ def verify_assertions(result:, error:) when "headerInjected" header_name = assertion["path"] expected = assertion["expected"] - requests = @tracker.requests - if requests.empty? - failures << "Expected header #{header_name}=#{expected.inspect}, but no requests recorded" + idx = assertion["index"] || 0 + headers = request_headers_at(idx) + if headers.nil? + failures << "Expected header #{header_name}=#{expected.inspect} on request index #{idx}, but only #{@tracker.request_count} requests were recorded" else - actual = requests.first[:headers]&.[](header_name) + actual = headers[header_name] unless actual == expected - failures << "Expected header #{header_name}=#{expected.inspect}, got #{actual.inspect}" + failures << "Expected header #{header_name}=#{expected.inspect} on request index #{idx}, got #{actual.inspect}" end end when "headerPresent" header_name = assertion["path"] - requests = @tracker.requests - if requests.empty? - failures << "Expected header #{header_name} to be present, but no requests recorded" + idx = assertion["index"] || 0 + headers = request_headers_at(idx) + if headers.nil? + failures << "Expected header #{header_name} on request index #{idx}, but only #{@tracker.request_count} requests were recorded" else - actual = requests.first[:headers]&.[](header_name) + actual = headers[header_name] if actual.nil? || actual.empty? - failures << "Expected header #{header_name} to be present, but it was missing or empty" + failures << "Expected header #{header_name} on request index #{idx}, but it was empty or missing" + end + end + + when "headerAbsent" + header_name = assertion["path"] + idx = assertion["index"] || 0 + headers = request_headers_at(idx) + if headers.nil? + failures << "Expected header #{header_name} absent on request index #{idx}, but only #{@tracker.request_count} requests were recorded" + else + actual = headers[header_name] + unless actual.nil? || actual.empty? + failures << "Expected header #{header_name} absent on request index #{idx}, got #{actual.inspect}" end end diff --git a/conformance/runner/typescript/runner.test.ts b/conformance/runner/typescript/runner.test.ts index 6efd83f7..55a40d63 100644 --- a/conformance/runner/typescript/runner.test.ts +++ b/conformance/runner/typescript/runner.test.ts @@ -33,6 +33,7 @@ interface Assertion { min?: number; max?: number; path?: string; + index?: number; } interface TestCase { @@ -71,16 +72,10 @@ const TS_SDK_SKIPS: Record = { "TS SDK retry middleware yields at most 1 retry per middleware pass", "Large integer IDs preserved without precision loss": "JavaScript loses precision on integers > Number.MAX_SAFE_INTEGER (2^53)", - "DownloadURL auth'd first hop 302s to signed URL": - "TS runner does not yet dispatch DownloadURL (tracked as follow-up)", - "DownloadURL direct 2xx body": - "TS runner does not yet dispatch DownloadURL (tracked as follow-up)", "DownloadURL retries on 503 at the auth'd first hop": - "TS runner does not yet dispatch DownloadURL (tracked as follow-up)", + "TS SDK downloadURL uses raw fetch bypassing the retry middleware; 5xx / Retry-After retry is not implemented", "DownloadURL honors Retry-After on 429 at the auth'd first hop": - "TS runner does not yet dispatch DownloadURL (tracked as follow-up)", - "DownloadURL surfaces redirect with no Location": - "TS runner does not yet dispatch DownloadURL (tracked as follow-up)", + "TS SDK downloadURL uses raw fetch bypassing the retry middleware; 5xx / Retry-After retry is not implemented", "UploadsDownload delegates through DownloadURL primitive": "TS runner's MSW stub matches a single path; multi-hop download fixtures need per-hop stub wiring (tracked as follow-up with DownloadURL)", }; @@ -232,8 +227,22 @@ async function executeOperation( break; } + case "DownloadURL": { + if (!tc.path) { + throw new Error("DownloadURL test case requires a non-empty path"); + } + const rawURL = "https://storage.3.basecamp.com" + tc.path; + const result = await client.downloadURL(rawURL); + // Fire-and-forget cancel — matches typescript/tests/download.test.ts. + // Awaiting MSW's mocked ReadableStream.cancel() can hang past vitest's + // default 5s test timeout, so don't await it here. void marks intent; + // .catch() suppresses any unhandled-rejection from the discarded Promise. + void result.body.cancel().catch(() => {}); + return {}; + } + default: - throw new Error(`Unknown operation: ${tc.operation}`); + throw new Error(`Unknown operation: ${tc.operation}`); } // Success path: no error @@ -351,6 +360,19 @@ function installMockHandlers(tc: TestCase): { // Assertion checker // ============================================================================= +/** Resolve captured headers at index (0-based; negative counts from end), or undefined if out of range. */ +function requestHeadersAt( + all: Record[], + index: number, +): Record | undefined { + const n = all.length; + if (n === 0) return undefined; + let i = index; + if (i < 0) i += n; + if (i < 0 || i >= n) return undefined; + return all[i]; +} + function checkAssertions( tc: TestCase, tracker: ReturnType, @@ -364,6 +386,19 @@ function checkAssertions( (r) => r.headers?.["Link"]?.includes('rel="next"'), ); + // 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 + // the same origin would otherwise pass silently. + if (tc.operation === "DownloadURL") { + const recordedPaths = tracker.requestPaths(); + if (recordedPaths.length > 0 && recordedPaths[0] !== tc.path) { + throw new Error( + `[${tc.name}] DownloadURL hop 1 expected path ${tc.path}, got ${recordedPaths[0]}`, + ); + } + } + for (const assertion of tc.assertions) { switch (assertion.type) { case "requestCount": { @@ -445,19 +480,38 @@ function checkAssertions( case "headerPresent": { const headerName = assertion.path!; - const headers = tracker.requestHeaders(); - expect( - headers.length, - `[${tc.name}] expected at least one request for header presence check`, - ).toBeGreaterThan(0); - const actual = headers[0]![headerName.toLowerCase()]; + const idx = assertion.index ?? 0; + const headers = requestHeadersAt(tracker.requestHeaders(), idx); + if (headers === undefined) { + throw new Error( + `[${tc.name}] expected header ${headerName} on request index ${idx}, but only ${tracker.requestCount()} requests were recorded`, + ); + } + const actual = headers[headerName.toLowerCase()]; expect( actual, - `[${tc.name}] expected header ${headerName} to be present, but it was empty or missing`, + `[${tc.name}] expected header ${headerName} on request index ${idx}, but it was empty or missing`, ).toBeTruthy(); break; } + case "headerAbsent": { + const headerName = assertion.path!; + const idx = assertion.index ?? 0; + const headers = requestHeadersAt(tracker.requestHeaders(), idx); + if (headers === undefined) { + throw new Error( + `[${tc.name}] expected header ${headerName} absent on request index ${idx}, but only ${tracker.requestCount()} requests were recorded`, + ); + } + const actual = headers[headerName.toLowerCase()]; + expect( + actual, + `[${tc.name}] expected header ${headerName} absent on request index ${idx}, got "${actual}"`, + ).toBeFalsy(); + break; + } + case "headerValue": { const headerName = assertion.path!; const expected = String(assertion.expected); @@ -546,15 +600,17 @@ function checkAssertions( case "headerInjected": { const headerName = assertion.path!; const expected = String(assertion.expected); - const headers = tracker.requestHeaders(); - expect( - headers.length, - `[${tc.name}] expected at least one request for header check`, - ).toBeGreaterThan(0); - const actual = headers[0]![headerName.toLowerCase()]; + const idx = assertion.index ?? 0; + const headers = requestHeadersAt(tracker.requestHeaders(), idx); + if (headers === undefined) { + throw new Error( + `[${tc.name}] expected header ${headerName}="${expected}" on request index ${idx}, but only ${tracker.requestCount()} requests were recorded`, + ); + } + const actual = headers[headerName.toLowerCase()]; expect( actual, - `[${tc.name}] expected header ${headerName}="${expected}", got "${actual}"`, + `[${tc.name}] expected header ${headerName}="${expected}" on request index ${idx}, got "${actual}"`, ).toBe(expected); break; }