Repository navigation
Port BigQueryIOWriteTest off DoFnTester - #40871
Rishavbhattarai wants to merge 1 commit into
Conversation
testWritePartition now runs WritePartition through the test's TestPipeline, with the temp file prefix as a real side input and the same output coders BatchLoads sets. The existing checks move into a PAssert.satisfies on the expected output tag. Fixes apache#18689
There was a problem hiding this comment.
📋 Review Summary
This PR cleanly migrates the testWritePartition helper in BigQueryIOWriteTest away from the deprecated DoFnTester to a standard Beam pipeline execution with ParDo and PAssert. The change removes the need to disable abandoned node enforcement and eliminates the remaining DoFnTester usage in the file.
🔍 General Feedback
- The pipeline construction, side input wiring, multi-output tagging, and
PAssertverification accurately preserve all original test assertions. - Consider also asserting that the unselected output tag (
singlePartitionTagormultiPartitionsTag) is empty viaPAssert.that(...).empty()for slightly stronger coverage, though the current migration is already equivalent to the previousDoFnTesterbehavior.
💡 Tip: Comment /gemini review on this pull request to trigger a new Gemini code review.
|
Assigning reviewers: R: @ahmedabu98 for label java. Note: If you would like to opt out of this review, comment Available commands:
The PR bot will only process comments in the main thread (not review comments). |
Fixes #18689.
Ports
BigQueryIOWriteTestoff the deprecatedDoFnTester. The only use was intestWritePartition.Unlike the Datastore and SDF tests, nothing here blocks
TestPipeline:WritePartitionis serializable and holds no mocks. So the test now runs it through the class's existingp:Iterableelement, the wayBatchLoadshands it over, so there's one bundlesetSideInputKvCoder.of(ShardedKeyCoder.of(TableDestinationCoder.of()), WritePartition.ResultCoder.INSTANCE), matchingBatchLoadsPAssert.satisfieson the expected tag, unchanged except that the empty-table check usesFile.length()(no checked exception inside the lambda)enableAbandonedNodeEnforcement(false)is gone since the pipeline now runs. The twoCreates got names to keep transform names stable. The empty singleton file is still deleted after the check, becausetearDownasserts the temp dir has no files.Testing:
./gradlew :sdks:java:io:google-cloud-platform:test --tests 'org.apache.beam.sdk.io.gcp.bigquery.BigQueryIOWriteTest*'passes (220 run, 98 skipped, 0 failures), andspotlessCheckis clean. To make sure thePAssertactually runs, I changed the expected partition count intestWritePartitionLargeFileSizefrom 4 to 5 and both parameterizations failed.