Skip to content

test(b2): pin reader end-of-file, empty-range, backoff and progress behavior #262

Open

Description

@ffumero2003

Context

A mutation re-check of main at 6ee4205 (Gremlins v0.6.0, run without B2 credentials, every surviving mutant replayed by hand) found several reader and writer behaviors that callers depend on, where a realistic code change passes the entire suite. These are stream and progress contracts of the SDK rather than rules of the B2 service, so their expected values are the SDK's own documented behavior and Go's io.Reader contract. Each test must use literal values and must fail against the break named below.

Part of this area is already covered by the open #257 (writer, reader and progress edges). The items marked below as following #257 should be written, or confirmed as covered, after it merges, to avoid two PRs editing the same tests.

Problem

A finished download can stop reporting end of file

Reader records its terminal error in b2/reader.go:120 (if r.err == nil { r.err = err }). Inverting that guard means a reader that has reached EOF later returns context canceled from Read, even though the caller's context is healthy, and the suite still passes. A caller that reads again after EOF (some decoders and io.Copy wrappers do) would see a spurious cancellation. Earlier, TestReaderDoubleClose covered this by accident by copying to EOF twice; #247 correctly rewrote that test to exercise Close, which removed the coverage. A dedicated test should upload the literal bytes abc, read them to EOF, then call Read twice more and require 0, io.EOF both times while the caller's context is still live.

An empty range read is not pinned to make no requests

Reader.initFunc in b2/reader.go:257 only issues a HEAD request to size progress when the range length is positive. Changing that condition makes a zero-length NewRangeReader send a request, which can require extra permission or wait until cancellation when HEAD is unavailable. The test should create a zero-length range reader with a progress callback, over a transport that records and rejects every request, and require an immediate io.EOF, a terminal progress report of (0, 0), and zero HEAD or GET requests.

Short-read retry backoff is not pinned

backoff.wait in b2/reader.go:459 doubles the delay after each truncated read while it is under ten seconds, so the sequence is 1 ms, 2 ms, 4 ms … 8,192 ms, 16,384 ms, and then stays at 16,384 ms. Both removing the doubling and lowering the cap pass the suite, so a regression could retry a failing download in a tight loop. The test should use an injected timer (no real sleeping) to record the delays of consecutive waits, and assert the literal values 1 ms, 2 ms and 4 ms, the milestones 128 ms, 256 ms and 8,192 ms, the cap of 16,384 ms, and that the next delay stays at 16,384 ms.

Following #257: progress, chunk size and write failures

These four behaviors are in the code that #257 tests. After it merges, confirm whether each is now pinned, and add a test for any that is not:

  • Writer progress fraction (b2/writer.go:1043, meteredReader.done): with a size of 4 and 2 bytes read, progress must be exactly 0.5. A mutation today reports 8, which is 800%, and passes. The same function backs reader status.
  • Final upload chunk progress (b2/writer.go:947): Client.Status must report [0.5] for one active writer that has read 2 of 4 bytes, not just return without blocking.
  • Completed reader progress (b2/reader.go:345): after a bounded two-chunk download completes and before Close, Client.Status must report [1, 1]. For a zero-length reader after EOF it must report an empty slice without panicking.
  • Chunk size honored (b2/reader.go:289): with ChunkSize = 1 and ConcurrentDownloads = 1, a short literal body must be fetched with one-byte ranges (bytes=0-0, bytes=1-1, …).
  • A failed buffer write is not finalized (b2/writer.go:318): if the temporary buffer returns a short write with a sentinel error (for example a full disk), Write, the writer's error and Close must all return that sentinel, and no upload may be finalized.

Acceptance criteria

  • Each behavior above has a test with literal expected values, which fails against its named break (the red output is shown in the PR) and passes on main.
  • Timing tests use injected timers or channels, not time.Sleep, and pass under go test -race -count=5 -shuffle=on ./b2/....
  • All tests run with B2_APPLICATION_KEY_ID, B2_APPLICATION_KEY, B2_ACCOUNT_ID and B2_ACCOUNT_KEY unset.
  • No production code changes. If a new test fails on current main (a zero-length reader panicking in Client.Status is a candidate), the test stays and the bug is filed as its own issue.

Scope

Tests only, in b2. The first three behaviors can start immediately; the last group follows #257. Out of scope: changing the backoff policy, the default chunk size, or the progress API.

Links

Metadata

Metadata

Assignees

Labels

testingTests, test coverage and test infrastructure

Type

Projects

No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions