Repository navigation
test(b2): tighten API-facing b2 test assertions - #240
Conversation
Cover zero-size uploads, inclusive and reversed ranges, copied total sizes around 10 TB, and acceptance of exactly 10000 parts. Assert waits for recoverable copy failures. The approved unexported platform-limit helper is the only non-test change in this slice. The production call passes the same native int limit as before; tests exercise math.MaxInt32 directly without mutable global state. Part of #204
Decode independent bucket response fixtures, assert every retained CORS field and delete identifier, and cover cursor stalls across all listing modes with bounded stack depth. Remove the redundant download-by-ID wire test; its fuller round-trip twin remains. Part of #204
Assert specific replication validation errors. Check SSE-C headers and propagation of server rejections separately from SDK rejection of a nil key, and label the method-type changelog check as a documentation gate. Part of #204
Use literal event JSON and retain signature behavior tests without grepping production source. Require harness scenarios to contain called Go entry points and failure assertions, with negative fixtures for no-op programs. Part of #204
|
A deeper SDK panel audit of this PR raised two findings and three nits. 10df302 addresses them:
|
There was a problem hiding this comment.
🟡 Changes recommended
The contract test rejects valid CRLF scenarios and accepts guards that ignore the entry point’s error result, allowing scenarios that silently succeed despite errors to pass.
2 open issues
What changed in this PR
Tightens API-facing b2 tests as part of #204’s test-integrity audit, replacing weak assertions with independent fixtures and boundary checks.
Changes:
- Adds compose boundary and retry-wait checks, with a behavior-preserving platform-size validation helper.
- Strengthens bucket, pagination, delete, replication, webhook, SSE-C and CORS assertions.
- Adds structural harness checks and removes a duplicate download test.
Verification here was limited to source inspection; no tests, live B2 checks or simulator scenarios were run. A human code owner should review the SSE-C credential-handling test changes.
| File | Description |
|---|---|
b2/webhook_test.go |
Uses literal event JSON and removes source-text checks. |
b2/update_cors_name_test.go |
Checks additional CORS fields using the wire-test helper. |
b2/ssec_wire_test.go |
Checks key headers, rejection propagation and nil-key handling. |
b2/ssec_test.go |
Labels the changelog assertion as a documentation gate. |
b2/sdkharness_contract_test.go |
Inspects scenario entry points and failure handling. |
b2/replication_setup_test.go |
Asserts specific replication errors. |
b2/iterator_advance_test.go |
Checks stalled cursors, stack depth and request counts. |
b2/downloadbyid_wire_test.go |
Removes a duplicate download-by-ID wire test. |
b2/delete_bypass_wire_test.go |
Checks file ID and name in delete requests. |
b2/default_retention_test.go |
Uses an independent HTTP response fixture. |
b2/create_cors_filelock_test.go |
Tests bucket attribute mapping from literal response JSON. |
b2/compose.go |
Extracts platform-size validation without changing behavior. |
b2/compose_fuzz_test.go |
Adds range, size and total-boundary assertions. |
b2/compose_extended_wire_test.go |
Checks retry delays and acceptance of exactly 10,000 parts. |
Review effort: Balanced
david-santacruz-backblaze
left a comment
There was a problem hiding this comment.
Let's review first the copilot flagged issues.
|
@david-santacruz-backblaze Both Copilot findings are fixed in a5922b9, and the branch is synced with main:
Each break was applied alone and fails its test. |
david-santacruz-backblaze
left a comment
There was a problem hiding this comment.
Two items before merge (Part of #204):
b2/compose_fuzz_test.go: the nonfinal minimum check (end-start < minPartSize-1,b2/compose.go) is only tested with small offsets (n = len(value)), soend+startwould still pass. Add to an existing table test:bytes=9000000-9000010→ rejected,bytes=9000000-14999999→ accepted. Parse-only, no runtime cost. Prove it by flipping the check toend+startand confirming the new row fails.b2/downloadbyid_wire_test.gois deleted. ConfirmTestDownloadFileByIDPublicRoundTripWireasserts methodGET, thefileIdquery parameter and theAuthorizationheader. If it doesn't, add those assertions there.
|
@david-santacruz-backblaze Thanks, both points:
The branch is synced with main; |


Context
Part of the test-integrity audit (#97). #204 is a checklist of low-risk items: tests that check less than their names say, plus request and response edges a single-line change could break with no test failing.
This PR covers the API-facing part of the
### b2section, 21 items: compose, bucket attrs, iteration, delete, download, replication, webhooks, SSE-C, CORS update and the harness contract. Where the other sections are:baseb2/sync,x/...,internal/...b2core items:b2_test.go,integration_test.go, account info, writer, reader, progress (includingTestObjectDeleteBypassGovernanceOption)Changes
Test changes, plus one behavior-identical seam in production code.
The one non-test change.
b2/compose.gogains an unexportedvalidateComposePlatformSize(size, intLimit). The existing check calls it with the same native limit as before (uint64(^uint(0)>>1)), so behavior is unchanged. The test calls it withmath.MaxInt32, so the 32-bit limit can be tested on a 64-bit host. #204 asks for this ("test the limit through a seam"), and the CI matrix only cross-builds for 32-bit, so this is the only way to test it.Compose (
compose_fuzz_test.go,compose_extended_wire_test.go)FuzzComposeRangealso rejects short, reversed and oversized canonical ranges.TestComposeUploadedPartPlatformSizechecks the explicit 32-bit boundaries and that the native call matches.bytes=5-5accepted;TestComposeCopyPartReauthAndBackoffasserts the wait (at least 1s) for 408, 429, 503 and 504, not just "retried once".Bucket, iteration and delete
TestBucketAttrsCORSAndFileLockandTestBucketAttrsDefaultRetentiondecode an independent HTTP fixture and assert every field, instead of a hand-builtbase.Bucket.fileIdandfileName.TestBucketDownloadFileByIDReachesTheWireis removed. It was a strict duplicate ofTestDownloadFileByIDPublicRoundTripWire, which stays.Replication, webhooks, SSE-C, CORS, harness
err != nil.webhook.gosource and keeps the behavioral checks;TestSDKHarnessContractparses each scenario's entry point and failure assertions, and a new test rejects a no-op scenario.TestDocsSSECReaderMethodTypeChangeDocumented, as a docs gate;TestSSECReadSendsRequestedKeyAndPropagatesRejectionandTestSSECReadRejectsExplicitNilKeyBeforeRequest.allowedOrigins,allowedOperationsandmaxAgeSeconds.Verification
Each compose break was applied on its own, as a flip and, for a guard, as a deletion, and run against the whole module:
<= 0→< 0validate zero-size part = <nil>, want positive-size errorend < start→<=parseComposeRange = (0, 0, …"bytes=5-5"), want (5, 5, nil)(6, 5, <nil>), want reversed-range rejectionknownSizearithmetic flippedknownSize = 5000000000199, true; want 4999999999999, true> 10000→>=Compose supports at most 10000 parts, want scripted error after successful validation>→>=32-bit validation of 2147483647 = … exceeds…, want nilTestMappingComposeCopyMinimumPart/TestComposeRejectsMoreThan10000PartsBeforeStartingEvery test passes once each break is restored.
go test -race -count=5 -shuffle=on ./b2/...passes withB2_APPLICATION_KEY_ID,B2_APPLICATION_KEY,B2_ACCOUNT_IDandB2_ACCOUNT_KEYunset.make cipasses every step except the local changelog check, which cannot see PR labels. This PR isskip-changelog, because the seam does not change behavior.TestObjectDeleteBypassGovernanceOption, is inb2/b2_test.goand belongs to the core slice. It reached this review through a scope-file mistake.Part of #204