Repository navigation
Conversation
google-cloud-bigquery 2.14.0 hardcodes the googleapis.com host for the request that opens a resumable upload session (HttpBigQueryRpc#open()), so BigqueryClient#load() used to take a test-only jobs.insert path under test_host, leaving the actual upload code untested. Add TestHostBigQueryRpc, an HttpBigQueryRpc subclass that builds that one URL from the configured host (what upstream does since googleapis/java-bigquery#2696), install it through the test_host BigQueryOptions, and drop the jobs.insert bypass. The two test-host classes move to their own package (testhost.TestHostBigQueryOptions / Rpc). The MockWebServer tests now see the real session-open POST and file PUT and assert on the uploaded bytes as well. loadAttemptResponses() / assertLoadAttempt() describe one load attempt (open, PUT, jobs.get) so retry scenarios list attempts instead of individual responses, and responses() flattens those arrays among single responses. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
- Replace the Object-varargs responses() with the type-safe MockResponses builder (of/add/loadAttempt/toArray). - assertUploadLoadFile() now states that it models a single-chunk upload and fails with a clear message for files over the library's chunk size. - uploadedLines() detects gzip by its magic bytes instead of assuming it. - Update comments that still described the old jobs.insert load path. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
add(MockResponse...) already takes the array, so the shortcut only duplicated loadAttemptResponses(). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…g it Auto-detecting gzip would quietly accept a plain upload under compression: GZIP; stating the expectation turns it into a check. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
A request for which no response was enqueued used to block until the client's read timeout (300 + 300 s with #113's defaults), so a test that listed one response too few hung for 10 minutes before failing. The MockWebServer now answers such requests immediately with a 599, and test_host.yml sets 1 s open/read timeouts so a server that deliberately stays silent (SocketPolicy.NO_RESPONSE) fails the test in a second as well. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…b ids Groundwork for tests of BigqueryClient#load()'s failure handling at the HTTP level: startResumableUploadResponse() is public and loadAttemptResponses() is split into uploadLoadFileResponse() / waitForLoadJobResponse(), so a test can put failing PUTs between the session open and the final chunk; disconnectDuringUploadResponse() is the one socket policy the library sees as a retryable network failure; uploadChunkAcceptedResponse(), assertUploadChunk() and writeLoadFile() cover files over the 15 MB chunk size; jobIdOf() reads the job id from the session-open POST. Tests pin what the library does on its own today: a 5xx or a connection reset during a chunk PUT is re-sent `retries` times on the same session, a 4xx is not, a 16 MB file goes up as two chunks, and a job that failed is retried under a new job id. An e2e run with compression: NONE checks the plain upload body. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
- unansweredResponse() (SocketPolicy.NO_RESPONSE) and a test that a chunk PUT nobody answers fails after read_timeout_sec, retried once like any network failure: the timeout from #113 applies to the upload too. Costs about 2 s of real waiting. - A test of the harness itself: a request beyond the enqueued responses is answered with a 599 at once, and the run is reported as having made more requests than expected. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
readRequestBodyAsString() and uploadedLines() drained the RecordedRequest's Buffer, so a second read of the same request (jobIdOf() then jobConfig() on the session-open POST, or uploadedLines() twice on the same PUT) saw an empty body and failed in the JSON parser or GZIPInputStream instead of pointing at the harness. Both now read getBody().clone(); a harness self-test pins it. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… delete it The class re-creates private internals of google-cloud-bigquery 2.14.0 (the request factory, the resumable upload path, the upstream header typo), so a library bump could change its behavior without breaking compilation. Record that the version pin has no other reason, that the upstream fix ships in 2.26.1, and that the testhost seam goes away once the library is upgraded. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The resumable upload's Location was hand-built from server.getHostName() and
server.getPort() while the client's test_host came from server.url("/"). Had
the two ever disagreed (TLS, an IPv6 host literal), the chunk PUTs would have
gone to a different origin than the rest of the calls and failed with an
obscure connection error. Both now come from testHostOf(), which returns the
origin the client is pointed at.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The self-test asserted JUnit's "expected:<1> but was:<2>" wording, so a JUnit upgrade or a change to the count assertion would have failed it without the harness being broken. Give the count assertion its own message and match on that instead. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
spotless had split the inline comments in testRunReplaceModeStillUpdatesSchemaWhenTempTableDeleteFails across lines, leaving a ")" floating and "delete temp table (finally)" attached to toArray() instead of the errorResponse() it describes. They now sit on their own line above the response they explain, like the other tests' comments. The note on how a load attempt differs from copy/query (3 requests per attempt, each retry a new job) was on newLoadFile(), a helper that just writes a one-line file; it moves to the first retry test it describes. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This was referenced Oct 14, 2026
john-abre
approved these changes
Oct 14, 2026
There was a problem hiding this comment.
Approve します。#127 のマージコミット (6301893) と tree が完全に一致していることを確認しました。
#125 で挙げた 5 点はすべて反映されています: ボディのヘルパーが記録済み Buffer の clone を読む(セルフテストで固定)、モックサーバーの origin が testHostOf() に一本化、ハーネスのセルフテストがハーネス自身のメッセージにマッチ、TestHostBigQueryRpc にバージョン固定の理由と削除条件を記載、コメントが spotless に動かされない位置に配置。
approve の前提にしていた load() のアップロード例外の握りつぶしと部分ファイルの close は、#123 の説明で明示的に対象になっているので解消です。
手元でも ./gradlew spotlessCheck test を実行し、157 テスト(1 skip)がすべて通りました。
🤖 Generated with Claude Code
10 of 21 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Re-submission of #125 together with its follow-up #127. Both were squash-merged before the review on #125 (#125 (review)) had been addressed and approved, so
feature/ruby-compatibilityhas been reset to its pre-#125 state and the same content is put back under review here. The tree is identical to what #127 left on the branch; the commits are the original ones from #125 followed by the five review fixes.The change (from #125)
google-cloud-bigquery 2.14.0 hardcodes the
googleapis.comhost for the request that opens a resumable upload session (HttpBigQueryRpc#open()), soBigqueryClient#load()used to take a test-onlyjobs.insertpath undertest_host, leaving the actual upload code untested.TestHostBigQueryRpc, anHttpBigQueryRpcsubclass installed through thetest_hostBigQueryOptions, builds that one URL from the configured host (what upstream does since fix: custom host for resumable uploads googleapis/java-bigquery#2696); thejobs.insertbypass is gone. The two test-host classes move to their own package (testhost.TestHostBigQueryOptions/TestHostBigQueryRpc).MockWebServertests now see the real session-open POST and file PUT and assert on the uploaded bytes.loadAttemptResponses()/assertLoadAttempt()describe one load attempt (open, PUT, jobs.get) so retry scenarios list attempts instead of individual responses.retriestimes), 4xx (not re-sent), the read timeout applying to the PUT, and a file over the chunk size going up as several PUTs.Review fixes (from #127)
readRequestBodyAsString()anduploadedLines()readgetBody().clone()instead of draining theRecordedRequest'sBuffer; a harness self-test pins it.testHostOf()), used for bothtest_hostand the resumable upload'sLocation.runWithMockServer()'s own message instead of JUnit's wording.TestHostBigQueryRpcrecords why it is tied to 2.14.0, that the upstream fix ships in 2.26.1, and that thetesthostseam goes away once the library is upgraded.The remaining review premise, that #123 covers both the swallowed upload exception and the channel
close()finalizing a partial file, is handled in #123 itself.#126 (request cursor) is stacked on this branch; its base will move back to
feature/ruby-compatibilityonce this is merged.Test plan
./gradlew test(157 tests, 1 skipped)./gradlew spotlessApply🤖 Generated with Claude Code