#345: Replace ExitGuard in integration tests #438
Conversation
kaklakariada
changed the title
|
|
||
| import com.exasol.mavenprojectversiongetter.MavenProjectVersionGetter; | ||
|
|
||
| class JarLauncher |
There was a problem hiding this comment.
I think this class deserves a bit of documentation in the header. :-)
product/src/test/java/org/itsallcode/openfasttrace/cli/JarLauncher.java
Outdated
Show resolved
Hide resolved
product/src/test/java/org/itsallcode/openfasttrace/cli/JarLauncher.java
Outdated
Show resolved
Hide resolved
product/src/test/java/org/itsallcode/openfasttrace/cli/JarLauncher.java
Outdated
Show resolved
Hide resolved
| { | ||
| return runnable -> { | ||
| final Thread thread = new Thread(runnable); | ||
| thread.setName(ProcessOutputConsumer.class.getSimpleName()); |
There was a problem hiding this comment.
Wouldn't it make more sense to name the thread after the process being consumed? Or at least to get that processes identification into the name? Thread names are mainly a debugging means as far as I remember.
product/src/test/java/org/itsallcode/openfasttrace/cli/ProcessOutputConsumer.java
Outdated
Show resolved
Hide resolved
| return builder.toString(); | ||
| } | ||
|
|
||
| void streamFinished() |
There was a problem hiding this comment.
Naming an attribute and a method the same is a recipe for confusion.
Please rename the method. Use an imperative. :-)
|
|
||
| private static class ProcessStreamConsumer | ||
| { | ||
| private final CountDownLatch streamFinished = new CountDownLatch(1); |
There was a problem hiding this comment.
| private final CountDownLatch streamFinished = new CountDownLatch(1); | |
| private final CountDownLatch streamFinishedLatch = new CountDownLatch(1); |
|
|
||
| void accept(final String line) | ||
| { | ||
| LOG.fine("%s > %s".formatted(name, line)); |
There was a problem hiding this comment.
Consuming without logging might be a good option, since we are in tests here and don't want to flood the test log.
product/src/test/java/org/itsallcode/openfasttrace/cli/JarLauncher.java
Outdated
Show resolved
Hide resolved
product/src/test/java/org/itsallcode/openfasttrace/cli/JarLauncher.java
Outdated
Show resolved
Hide resolved
product/src/test/java/org/itsallcode/openfasttrace/cli/JarLauncher.java
Outdated
Show resolved
Hide resolved
product/src/test/java/org/itsallcode/openfasttrace/cli/JarLauncher.java
Outdated
Show resolved
Hide resolved
product/src/test/java/org/itsallcode/openfasttrace/cli/TestCliExit.java
Outdated
Show resolved
Hide resolved
product/src/test/java/org/itsallcode/openfasttrace/cli/TestCliStarter.java
Outdated
Show resolved
Hide resolved
|
@redcatbear I implemented your findings. Please check again. |
|
Closes #345.