[ https://issues.apache.org/jira/browse/YARN-5389?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15435087#comment-15435087 ]
Jason Lowe commented on YARN-5389: ---------------------------------- Thanks for updating the patch! I'm curious why we don't just let the timeout or interrupted exceptions bubble up and fail the test like any other uncaught exception during a unit test. The resulting stacktrace will show us exactly where the timeout or interrupt occurred, so I'm not sure the try/catch clause is worth it. Ah, I see now. The original tests are doing stuff like this: {code} try { dResponse = client.deleteReservation(dRequest); } catch (Exception e) { Assert.fail(e.getMessage()); } {code} That's not good. It suppresses the stack trace of the original exception and instead replaces it with a not-so-useful one. For example if something throws an NullPointerException, this type of code will give us a stacktrace telling us "yep, we got an NPE while running this test, but I'm not going to tell you exactly where the NPE occurred." It's standard practice to have the unit test methods declare the exceptions they can naturally throw (often people just have all the unit test methods declared as throwing Exception) then let the test framework handle diagnostics if the unit test throws during execution. I strongly suggest we remove the Exception catching in the methods and just let the exceptions bubble up to the test framework so we can get useful stacktraces when something fails. Also curious why we're going with 100ms sleeps instead of 10 as I suggested above. Are there concerns of CPU usage? The check every iteration seems very cheap, so it's overall impact should be minimal. Not a must-fix, but it is something that's reused in multiple tests (and likely by additional unit tests in the future) so it'd be nice to eliminate the extra waste. > TestYarnClient#testReservationDelete fails in trunk > --------------------------------------------------- > > Key: YARN-5389 > URL: https://issues.apache.org/jira/browse/YARN-5389 > Project: Hadoop YARN > Issue Type: Test > Reporter: Rohith Sharma K S > Assignee: Sean Po > Labels: test > Attachments: YARN-5389.v1.patch, YARN-5389.v2.patch > > > In build report > [report|https://builds.apache.org/job/PreCommit-YARN-Build/12341/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-client.txt], > below test fails. > {noformat} > Tests run: 28, Failures: 3, Errors: 0, Skipped: 0, Time elapsed: 26.066 sec > <<< FAILURE! - in org.apache.hadoop.yarn.client.api.impl.TestYarnClient > testReservationDelete(org.apache.hadoop.yarn.client.api.impl.TestYarnClient) > Time elapsed: 2.213 sec <<< FAILURE! > java.lang.AssertionError: Exhausted attempts in checking if node capacity was > added to the plan > at org.junit.Assert.fail(Assert.java:88) > at > org.apache.hadoop.yarn.client.api.impl.TestYarnClient.setupMiniYARNCluster(TestYarnClient.java:1222) > at > org.apache.hadoop.yarn.client.api.impl.TestYarnClient.testReservationDelete(TestYarnClient.java:1584) > testListReservationsByInvalidTimeInterval(org.apache.hadoop.yarn.client.api.impl.TestYarnClient) > Time elapsed: 2.215 sec <<< FAILURE! > java.lang.AssertionError: Exhausted attempts in checking if node capacity was > added to the plan > at org.junit.Assert.fail(Assert.java:88) > at > org.apache.hadoop.yarn.client.api.impl.TestYarnClient.setupMiniYARNCluster(TestYarnClient.java:1222) > at > org.apache.hadoop.yarn.client.api.impl.TestYarnClient.testListReservationsByInvalidTimeInterval(TestYarnClient.java:1444) > testListReservationsByTimeIntervalContainingNoReservations(org.apache.hadoop.yarn.client.api.impl.TestYarnClient) > Time elapsed: 2.206 sec <<< FAILURE! > java.lang.AssertionError: Exhausted attempts in checking if node capacity was > added to the plan > at org.junit.Assert.fail(Assert.java:88) > at > org.apache.hadoop.yarn.client.api.impl.TestYarnClient.setupMiniYARNCluster(TestYarnClient.java:1222) > at > org.apache.hadoop.yarn.client.api.impl.TestYarnClient.testListReservationsByTimeIntervalContainingNoReservations(TestYarnClient.java:1494) > {noformat} -- This message was sent by Atlassian JIRA (v6.3.4#6332) --------------------------------------------------------------------- To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org