Prevent Test From Hanging With AnimatedLabel Thread

[Kapral67]

The fix for #595 as proposed by #596 looks good to me.

The test proposed in #596 would hang with the AnimatedLabel Thread.

This PR proposes a modification so that now matter if Issue595 test fails or succeeds:

• it never hangs

• indicates test failure via:

  • output to stderr

  • returned exit code

[avl42]

Yes, they are functionally equivalent. And, if ever a logging framework is added, it would be changed to passing the exception to the logger directly, and not stringifying beforehand. (That was originally a thinko on my side, that the logger would then still need the stringified exception.)

In the meantime, anyone could replace the "System.err" part by some other channel opened to some logfile, if he doesn't want exception stack-traces to go in user's face. (if terminal is in raw mode, those exceptions often appear ugly garbled, as the "\n" then is not mapped to a linebreak on terminal.

If we changed it to the compact form, replacing the channel becomes more tedious.

I don't see enough reason to compactify the code towards less flexibility. Afterall the two variants under discussion are equivalent at runtime.

On Thu, Feb 29, 2024 at 9:06 AM Maxwell Kapral @.***> wrote:

@.**** commented on this pull request.

In src/test/java/com/googlecode/lanterna/issue/Issue595.java https://github.com/mabe02/lanterna/pull/597#discussion_r1507167319:

• return;
• }
• final Thread thread = optionalAnimatedLabelThread.get();
• thread.join(SLEEP_MILLIS);
• if (thread.isAlive()) {
• throw new IllegalStateException("AnimatedLabel Thread Waiting");
• }
• } catch (final Throwable e) {
• if (e instanceof InterruptedException) {
• Thread.currentThread()
• .interrupt();
• }
• ++exit_code;
• final StringWriter stringWriter = new StringWriter();
• e.printStackTrace(new PrintWriter(stringWriter));

Unless we resort to a logging framework

StringWriter s = new StringWriter();e.printStackTrace(new PrintWriter(s));System.err.print(s);

Is equivalent to e.printStackTrace() (at least in vanilla JDK implementations) https://github.com/openjdk/jdk/blob/998d0baab0fd051c38d9fd6021628eb863b80554/src/java.base/share/classes/java/lang/Throwable.java#L677-L679

— Reply to this email directly, view it on GitHub https://github.com/mabe02/lanterna/pull/597#discussion_r1507167319, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABIDBMWODDQD6AX4BQCTF3DYV3QRPAVCNFSM6AAAAABDRVJOZ6VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTSMBYGIYTGMBQGQ . You are receiving this because you commented.Message ID: @.***>

[mabe02]

Given that this is a test for reproducing a reported bug, I don't really mind that the code style and some constructs are misaligned with the rest of the library. The code won't be included in the library anyway. I've intentionally avoided introducing or using any logging framework in the library itself since there never really was any strong need for logging anywhere. Rather, errors are thrown out and users should catch exceptions and log them as per their application.