Skip to content

test: reorganize and table-drive the tests - #82

Merged
daichirata merged 2 commits into
mainfrom
test/reorganize
Nov 5, 2026
Merged

daichirata merged 2 commits into
mainfrom
test/reorganize

Conversation

@daichirata

Copy link
Copy Markdown
Owner
  • Fold near-identical tests into tables around shared helpers (142 tests / 3,530 lines → 49 tests / 1,975 lines)
  • Inject GCS faults to cover the generation-changed retry, lookup/read errors and the slow-header timeout
  • Coverage 95.2% → 96.4%

🤖 Generated with Claude Code

Near-identical tests are folded into tables around shared helpers (142 tests, 3,530 lines -> 49 tests, 1,975 lines). A fault-injecting transport in front of fake-gcs-server now covers the generation-changed retry, GCS errors on lookups and reads, and the slow-header timeout (readHeaderTimeout becomes a var so the test can shorten it).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The tests inject malformed GCS error JSON, no longer verify generation pinning, and omit the Content-Length absence check after failed reads, leaving important regressions undetected.

3 open issues
What changed in this PR

This PR consolidates gcsproxy’s tests into shared helpers and table-driven cases, while expanding coverage of storage failures and HTTP timeouts.

Changes:

  • Reorganizes routing, headers, ranges, conditional requests, and logging tests.
  • Adds fault injection for GCS lookup/read errors and generation-change retries.
  • Makes the header timeout adjustable for a slow-header test, retaining the 10-second default.
File Description
main.go Changes the header timeout from a constant to a variable for testing.
main_test.go Consolidates tests and adds fault-injection and timeout coverage.

Review effort: Balanced

Comment thread main_test.go Resolved
Comment thread main_test.go Resolved
Comment thread main_test.go Resolved
Inject GCS errors with a numeric code, assert Content-Length is absent after a failed read, and check that reads ask for the validated generation.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

The revised tests leave retry metadata and precondition re-evaluation unverified and drop error-header, range-validator, and response-body assertions, allowing regressions to pass.

No open issues 🎉

3 resolved since last review

Review effort: Balanced

@daichirata
daichirata merged commit 5d2a707 into main Nov 5, 2026
4 checks passed
@daichirata
daichirata deleted the test/reorganize branch November 5, 2026 13:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants