[
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: [email protected]
For additional commands, e-mail: [email protected]
