ryucc commented on code in PR #26287:
URL: https://github.com/apache/beam/pull/26287#discussion_r1167255993
##########
runners/samza/src/test/java/org/apache/beam/runners/samza/runtime/ClassicBundleManagerTest.java:
##########
@@ -82,35 +82,20 @@ public void testTryStartBundleStartsBundle() {
assertTrue("tryStartBundle() did not start the bundle",
bundleManager.isBundleStarted());
}
- @Test
- public void testTryStartBundleThrowsExceptionAndSignalError() {
+ @Test(expected = IllegalArgumentException.class)
+ public void testWhenCurrentBundleDoneFutureIsNotNullThenStartBundleFails() {
bundleManager.setCurrentBundleDoneFuture(CompletableFuture.completedFuture(null));
- try {
- bundleManager.tryStartBundle();
- } catch (IllegalArgumentException e) {
- bundleManager.signalFailure(e);
- }
-
- // verify if the signal failure only resets appropriate attributes of
bundle
- verify(mockFutureCollector, times(1)).prepare();
- verify(mockFutureCollector, times(1)).discard();
- assertEquals(
- "Expected the number of element in the current bundle to 0",
- 0L,
- bundleManager.getCurrentBundleElementCount());
- assertEquals(
- "Expected pending bundle count to be 0", 0L,
bundleManager.getPendingBundleCount());
- assertFalse("Error didn't reset the bundle as expected.",
bundleManager.isBundleStarted());
Review Comment:
This removed block is already tested in
`testTryStartBundleThrowsExceptionFromTheListener`. The only difference is the
cause of `tryStartBundle` failure. I'm isolating this part to a new and smaller
test.
--
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]
