[BEAM-5709] Fix flaky tests in BeamFnControlServiceTest. #6639
Conversation
|
R: @boyuanzz |
|
CC: @kennknowles For committer approval once this is reviewed. |
|
Thanks for this! Please wait for all tests pass. |
| @@ -88,6 +88,8 @@ public void testClientConnecting() throws Exception { | |||
| server.shutdown(); | |||
| server.awaitTermination(1, TimeUnit.SECONDS); | |||
| server.shutdownNow(); | |||
| Thread.sleep(1000); // Wait for stub to close stream. | |||
There was a problem hiding this comment.
Fixing a flake is important enough that we can merge this, but please keep a ticket open. If you have time and inclination to fix the test to use a latch instead of a sleep, that would be preferable. I imagine you can do this with the mocks by inserting code to run when onCompleted is called, or you could move away from mocks since StreamObserver is easy to implement.
There was a problem hiding this comment.
That sounds like a good idea. I'm assuming with that method some kind of timeout would be set on the latch, so that if onCompleted doesn't get called in a certain timespan we can fail the test?
There was a problem hiding this comment.
Yes. And since it is just a unit test that won't have wide variance, we can just put @Test(timeout=2000) on the method using JUnit's support. So in the happy case it succeeds instantly, and failure by timeout in 2 seconds.
See the Jira issue for a description of the cause of the flakiness. This attempts to fix the issue by waiting a sufficiently long time for any actions that rely on the server shutdown to finish.
|
Fixed formatting errors that were causing test failures due to Spotless. |
See the Jira issue for a description of the cause of the flakiness. This attempts to fix the issue by waiting a sufficiently long time for any actions that rely on the server shutdown to finish.
Follow this checklist to help us incorporate your contribution quickly and easily:
[BEAM-XXX] Fixes bug in ApproximateQuantiles, where you replaceBEAM-XXXwith the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.It will help us expedite review of your Pull Request if you tag someone (e.g.
@username) to look at it.Post-Commit Tests Status (on master branch)