Github user StephanEwen commented on the pull request:
https://github.com/apache/flink/pull/977#issuecomment-129829151
Looks much better. Two more comments:
1. It would be good to print the exception in addition to failing the test,
because otherwise the tests only fails and gives no indication why. A typical
pattern is:
```java
try {
// some stuff
} catch (Exception e) {
e.printStackTrace();
Assert.fail(e.getMessage());
}
```
Here, the test prints nothing when working properly, but prints the error
when it fails.
2. `Assert.fail()` does not work when used in spawned threads. The reason
is that JUnit communicates the failure with a special
`AssertionFailedException`, which needs to reach the invoking framework. That
does not happen if the `fail()` method is called in a spawned thread.
Here is how you can do it. It is a bit clumsy, because you need to forward
the exception to the main thread, but it works well:
```java
final AtomicReference<Throwable> error = new AtomicReference<>();
Thread thread = new Thread("server thread") {
@Override
public void run() {
try {
doStuff();
}
catch (Throwable t) {
ref.set(t);
}
}
};
thread.start();
// do some test logic
thread.join();
if (error.get() != null) {
Throwable t = error.get();
t.printStackTrace();
fail("Error in spawned thread: " + t.getMessage());
}
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
