Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Disable two flaky tests (BEAM-8035, BEAM-9164) #11614

Merged
merged 1 commit into from May 6, 2020
Merged

Disable two flaky tests (BEAM-8035, BEAM-9164) #11614

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions ...ache/beam/runners/flink/translation/wrappers/streaming/io/UnboundedSourceWrapperTest.java
View file
Open in desktop
Expand Up @@ -66,6 +66,7 @@
import org.apache.flink.streaming.util.AbstractStreamOperatorTestHarness;
import org.apache.flink.util.InstantiationUtil;
import org.apache.flink.util.OutputTag;
import org.junit.Ignore;
import org.junit.Test;
import org.junit.experimental.runners.Enclosed;
import org.junit.runner.RunWith;
Expand Down Expand Up @@ -242,6 +243,7 @@ public void close() {}
* <p>This test verifies that watermarks are correctly forwarded.
*/
@Test(timeout = 30_000)
@Ignore("https://issues.apache.org/jira/browse/BEAM-9164")
Copy link
Member

Choose a reason for hiding this comment

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

This one is apparently only flaky on Flink can we better exclude it manually only for Flink?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is not @Category(ValidatesRunner) test, looks like it is Flink-specific already?

Copy link
Member

Choose a reason for hiding this comment

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

Oh my bad sorry.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@iemejia can I merge this? Is it fine for you?

Copy link
Member

Choose a reason for hiding this comment

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

Sure go ahead!

Copy link
Contributor

Choose a reason for hiding this comment

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

IMHO disabling tests should be the last resort. Please inform the mailing list or at least the JIRA issue when you disabled tests. This may be causing regressions. For example, I changed logic around UnboundedSourceWrapper a couple days ago and was not aware of tests being disabled.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mxm I started a discussion thread here. Agree that we should have a better way of tracking flaky and disabled tests. There was also additional thread about precommit stability. The two tests disabled in this PR were causing practically permanent failure in precommits.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for starting the discussion. I'll try to enable these tests again. The recent changes in UnboundedSourceWrapper should have improved the test stability.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'll run some tests here: #11679

public void testWatermarkEmission() throws Exception {
final int numElements = 500;
PipelineOptions options = PipelineOptionsFactory.create();
Expand Down
2 changes: 2 additions & 0 deletions sdks/java/core/src/test/java/org/apache/beam/sdk/transforms/WatchTest.java
View file
Open in desktop
Expand Up @@ -72,6 +72,7 @@
import org.joda.time.Duration;
import org.joda.time.Instant;
import org.joda.time.ReadableDuration;
import org.junit.Ignore;
import org.junit.Rule;
import org.junit.Test;
import org.junit.experimental.categories.Category;
Expand Down Expand Up @@ -306,6 +307,7 @@ public PollResult<KV<String, Integer>> apply(String element, Context c)

@Test
@Category({NeedsRunner.class, UsesUnboundedSplittableParDo.class})
@Ignore("https://issues.apache.org/jira/browse/BEAM-8035")
public void testMultiplePollsWithManyResults() {
final long numResults = 3000;
List<Integer> all = Lists.newArrayList();
Expand Down