tillrohrmann commented on a change in pull request #8534: [FLINK-12614][yarn]
Refactor test to not do assertions in @After methods
URL: https://github.com/apache/flink/pull/8534#discussion_r288015417
##########
File path:
flink-yarn-tests/src/test/java/org/apache/flink/yarn/YarnTestBase.java
##########
@@ -212,30 +212,45 @@ public void checkClusterEmpty() {
* Sleep a bit between the tests (we are re-using the YARN cluster for
the tests).
*/
@After
- public void sleep() throws IOException, YarnException {
- Deadline deadline = Deadline.now().plus(Duration.ofSeconds(10));
+ public void shutdownYarnClient() {
+ yarnClient.stop();
+ }
- boolean isAnyJobRunning = yarnClient.getApplications().stream()
- .anyMatch(YarnTestBase::isApplicationRunning);
+ protected void runTest(RunnableWithException test) {
+ Throwable testFailure = null;
+ try {
+ test.run();
+ } catch (Throwable t) {
+ testFailure = t;
+ }
- while (deadline.hasTimeLeft() && isAnyJobRunning) {
- try {
- Thread.sleep(500);
- } catch (InterruptedException e) {
- Assert.fail("Should not happen");
- }
- isAnyJobRunning = yarnClient.getApplications().stream()
+ try {
+ Deadline deadline =
Deadline.now().plus(Duration.ofSeconds(10));
+
+ boolean isAnyJobRunning =
yarnClient.getApplications().stream()
.anyMatch(YarnTestBase::isApplicationRunning);
- }
- if (isAnyJobRunning) {
- final List<String> runningApps =
yarnClient.getApplications().stream()
- .filter(YarnTestBase::isApplicationRunning)
- .map(app -> "App " + app.getApplicationId() + "
is in state " + app.getYarnApplicationState() + '.')
- .collect(Collectors.toList());
- if (!runningApps.isEmpty()) {
- Assert.fail("There is at least one application
on the cluster that is not finished." + runningApps);
+ while (deadline.hasTimeLeft() && isAnyJobRunning) {
+ try {
+ Thread.sleep(500);
+ } catch (InterruptedException e) {
+ Assert.fail("Should not happen");
+ }
+ isAnyJobRunning =
yarnClient.getApplications().stream()
+
.anyMatch(YarnTestBase::isApplicationRunning);
+ }
+
+ if (isAnyJobRunning) {
+ final List<String> runningApps =
yarnClient.getApplications().stream()
+
.filter(YarnTestBase::isApplicationRunning)
+ .map(app -> "App " +
app.getApplicationId() + " is in state " + app.getYarnApplicationState() + '.')
+ .collect(Collectors.toList());
+ if (!runningApps.isEmpty()) {
+ Assert.fail("There is at least one
application on the cluster that is not finished." + runningApps);
+ }
}
+ } catch (Throwable t) {
+ throw new
AssertionError(ExceptionUtils.firstOrSuppressed(t, testFailure));
Review comment:
Where would `t` come from? Would it rely on the fact that a failing test
does not properly shuts down the Yarn application? If yes, then I think this is
a brittle mechanism. As soon as we have a test which terminates the Yarn
application in the failure case, it would break.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
