Copilot commented on code in PR #37402:
URL: https://github.com/apache/beam/pull/37402#discussion_r2721860967
##########
runners/google-cloud-dataflow-java/src/test/java/org/apache/beam/runners/dataflow/DataflowRunnerTest.java:
##########
@@ -2308,13 +2308,17 @@ public void testBatchGroupIntoBatchesOverrideBytes() {
@Test
public void testBatchGroupIntoBatchesWithShardedKeyOverrideCount() throws
IOException {
PipelineOptions options = buildPipelineOptions();
+ // Ignore this test for streaming pipelines.
+ assumeFalse(options.as(StreamingOptions.class).isStreaming());
Pipeline p = Pipeline.create(options);
verifyGroupIntoBatchesOverrideCount(p, true, true);
}
@Test
public void testBatchGroupIntoBatchesWithShardedKeyOverrideBytes() throws
IOException {
PipelineOptions options = buildPipelineOptions();
+ // Ignore this test for streaming pipelines.
+ assumeFalse(options.as(StreamingOptions.class).isStreaming());
Review Comment:
Same issue as above: `buildPipelineOptions()` does not set `streaming`, so
`assumeFalse(options.as(StreamingOptions).isStreaming())` will always pass and
won’t skip under a streaming-configured test run. Use the test run’s configured
options (e.g., `pipeline.getOptions()` /
`TestPipeline.testingPipelineOptions()`) to decide whether to skip, or set
these local options to batch explicitly.
##########
runners/google-cloud-dataflow-java/src/test/java/org/apache/beam/runners/dataflow/DataflowRunnerTest.java:
##########
@@ -2308,13 +2308,17 @@ public void testBatchGroupIntoBatchesOverrideBytes() {
@Test
public void testBatchGroupIntoBatchesWithShardedKeyOverrideCount() throws
IOException {
PipelineOptions options = buildPipelineOptions();
+ // Ignore this test for streaming pipelines.
+ assumeFalse(options.as(StreamingOptions.class).isStreaming());
Review Comment:
The PR description is still the template placeholder (“Please add a
meaningful description…”). Please update the PR description to explain the root
cause of the flake, why skipping is the right fix (vs forcing batch options),
and link any relevant Beam issue/Jira if one exists.
##########
runners/google-cloud-dataflow-java/src/test/java/org/apache/beam/runners/dataflow/DataflowRunnerTest.java:
##########
@@ -2308,13 +2308,17 @@ public void testBatchGroupIntoBatchesOverrideBytes() {
@Test
public void testBatchGroupIntoBatchesWithShardedKeyOverrideCount() throws
IOException {
PipelineOptions options = buildPipelineOptions();
+ // Ignore this test for streaming pipelines.
+ assumeFalse(options.as(StreamingOptions.class).isStreaming());
Review Comment:
The new assumption checks `options.as(StreamingOptions).isStreaming()` on an
options instance created by `buildPipelineOptions()`, which does not set
`streaming` (see `buildPipelineOptions()` around
DataflowRunnerTest.java:368-384). Since `StreamingOptions#isStreaming()`
defaults to false, this assumption will never skip the test and likely won’t
address flakiness when the suite is executed with streaming-enabled
TestPipeline options. Consider basing the assumption on the test environment’s
pipeline options (e.g., the `@Rule TestPipeline pipeline` /
`TestPipeline.testingPipelineOptions()`), or explicitly forcing these
test-local options to non-streaming if the intent is to always test the batch
override behavior.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
