ctubbsii commented on code in PR #1127: URL: https://github.com/apache/fluo/pull/1127#discussion_r1037376036
########## modules/integration-tests/src/main/java/org/apache/fluo/integration/impl/TransactorIT.java: ########## @@ -111,9 +107,7 @@ public void testFailures() throws Exception { assertNumOpen(0); Assert.assertFalse(checkExists(t1)); - // Test for exception to be called - exception.expect(IllegalStateException.class); - t1.getTransactorID().getLongID(); + Assert.assertThrows(IllegalStateException.class, () -> t1.getTransactorID().getLongID()); Review Comment: This looks fine... it's equivalent. But, one improvement that could be made is: if the expected exception is coming from `.getLongID()`, then the object returned by `t1.getTransactorID()` could be declared on the line prior to the `assertThrows`. If the expected exception is coming from `t1.getTransactorID()`, then `.getLongID()` can be dropped from the assert statement. That just makes it more clear where the exception is expected. -- 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: notifications-unsubscr...@fluo.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org