Report logs only on failures in JUnit and ScalaTest #13721

hubertp · 2025-08-01T20:19:49Z

Pull Request Description

This change simplifies the existing MemoryAppender in order to accommodate for setting up Loggers in both, JUnit and ScalaTest. This change adds support for reporting logs on failures via JUnit's Rules.
Made sure that (in most cases) we use JulHandler as engine's log handler so that logs are appropriately forwarded to slf4j for processing.

The functionality stays as before. The only difference is that we now have logs being reported when a test fails.
Closes #13308.

Important Notes

A ReportLogsOnFailure has already been used in LS but not in runtime-integration-tests. It also didn't work well with logs in engine.

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

All code follows the
Scala,
Java,
TypeScript,
and
Rust
style guides. In case you are using a language not listed above, follow the Rust style guide.

This change simplifies the existing `MemoryAppender` in order to accomodate for setting up Loggers in both, JUnit and ScalaTest. A `ReportLogsOnFailure` has already been used in LS but not in `runtime-integration-tests`. It also didn't work for logs in engine. Junit implements "logs on failure" via rules.

More tests use the "report logs on failure only" feature.

`trace` level seems more useful now that logs are printed only when necessary.

Rather than following all `newBuilder` by `withLogHandler` ones, making the builder use `JulHandler` by default. This change required changes to `ExecCompilerTest` and `HelloWorldCacheTest` to make use of log messages rather than stdout.

So far we have not experienced ordering issues.

hubertp · 2025-08-05T11:10:19Z

build.sbt

        "-Dtck.inlineVerifierInstrument=false",
        "-Dpolyglot.engine.AllowExperimentalOptions=true"
      ),
+      Test / javacOptions += "-implicit:none",


Not related to this PR but reduces warnings noise.

engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/ExecCompilerTest.java

engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/StaticAnalysisTest.java

lib/java/test-utils/src/main/java/org/enso/test/utils/ContextUtils.java

Akirathan

Having another @Rule definition seems like a duplicate on many tests that already have @Rule ContextUtils. Can't the new logging behavior be the default for ContextUtils?

...e/runtime-integration-tests/src/test/java/org/enso/compiler/test/ExecStrictCompilerTest.java

lib/java/test-utils/src/main/java/org/enso/test/utils/ContextUtils.java

hubertp · 2025-08-08T14:10:57Z

I will have another run at all those changed tests and see if I can somehow eliminate the few exceptions that needed manual settings.

Reduced redundant `logHandler` setup in various places.

Akirathan

LGTM in general. Ideally, this behavior should somehow be enforced for all the tests. In a few months, when we all start forgetting to put @Rule ReportLogsOnFailure in our tests, we will be in the same situation and with the same polluted outputs on the CI. Were you able to investigate if it is possible to somehow hook into jUnit or ScalaTest runner?

hubertp · 2025-08-15T12:54:32Z

we will be in the same situation and with the same polluted outputs on the CI.

Quite the contrary. If this rule is not present, no logs would be printed.

Were you able to investigate if it is possible to somehow hook into jUnit or ScalaTest runner?

Hook into? To do what exactly?

Akirathan · 2025-08-15T13:19:19Z

Quite the contrary. If this rule is not present, no logs would be printed.

Can you elaborate? runtime-integration-tests/src/test/resources/application-test.conf specifies that the default appender is MemoryAppender which forwards to console. So if I implement a new test in runtime-integration-tests without any rules, the logs will still be forwarded to the console even if the test is successful, no? And what about tests in other projects? Like project-manager, language-server, etc?

Hook into? To do what exactly?

To do the same thing that you have implemented in the new rule - capture all output and only print it if a test failed.

In non-strict mode errors are being reported as warnings. Should :fingers-crossed: help with CI failures.

hubertp · 2025-08-15T14:50:25Z

Can you elaborate? runtime-integration-tests/src/test/resources/application-test.conf specifies that the default appender is MemoryAppender which forwards to console. So if I implement a new test in runtime-integration-tests without any rules, the logs will still be forwarded to the console even if the test is successful, no?

No. The forwarding is manually requested. If you want to always see the logs you can change the default appender to console.

To do the same thing that you have implemented in the new rule - capture all output and only print it if a test failed.

Let's take it offline as I'm still missing the point.

hubertp · 2025-08-15T17:57:15Z

Can you elaborate? runtime-integration-tests/src/test/resources/application-test.conf specifies that the default appender is MemoryAppender which forwards to console. So if I implement a new test in runtime-integration-tests without any rules, the logs will still be forwarded to the console even if the test is successful, no?

No. The forwarding is manually requested. If you want to always see the logs you can change the default appender to console.

Reading it again, I feel that what you are asking makes sense. I remember that the reason why I didn't do it was because of some logs that could be reported prior to execution of any test cases, in setup.

hubertp added the CI: No changelog needed Do not require a changelog entry for this PR. label Aug 1, 2025

enso-bot bot mentioned this pull request Aug 3, 2025

Collect logs in tests and only print them on failure #13308

Open

hubertp added 4 commits August 4, 2025 17:22

Further transition of JUnit tests

86586d8

More tests use the "report logs on failure only" feature.

Increase default log level for tests

03f43da

`trace` level seems more useful now that logs are printed only when necessary.

DRY when creating context with test log handler

681ad5a

More DRY

25a1f80

Rather than following all `newBuilder` by `withLogHandler` ones, making the builder use `JulHandler` by default. This change required changes to `ExecCompilerTest` and `HelloWorldCacheTest` to make use of log messages rather than stdout.

hubertp force-pushed the wip/hubert/13308-logs-on-failure branch from c636b81 to 25a1f80 Compare August 4, 2025 21:37

hubertp added 4 commits August 5, 2025 11:30

Added appender rule to remaining tests

3ae478a

Use customn log handler by default

780dba2

Move remaining Scala tests to reporting logs on failure

6a2415d

Drop order in Rule

34ec594

So far we have not experienced ordering issues.

hubertp commented Aug 5, 2025

View reviewed changes

hubertp added 4 commits August 5, 2025 15:00

More coverage

212da4c

fmt

f8c2148

fix benchmarks that expect output in stdout

0bb299e

Merge branch 'develop' into wip/hubert/13308-logs-on-failure

37f1b8b

hubertp marked this pull request as ready for review August 6, 2025 09:02

hubertp requested review from 4e6, Akirathan and JaroslavTulach as code owners August 6, 2025 09:02

JaroslavTulach approved these changes Aug 6, 2025

View reviewed changes

Akirathan reviewed Aug 6, 2025

View reviewed changes

...e/runtime-integration-tests/src/test/java/org/enso/compiler/test/ExecStrictCompilerTest.java Show resolved Hide resolved

lib/java/test-utils/src/main/java/org/enso/test/utils/ContextUtils.java Outdated Show resolved Hide resolved

hubertp added 2 commits August 11, 2025 11:26

Merge branch 'develop' into wip/hubert/13308-logs-on-failure

25a5b7f

Avoid concurrent modification errors

c8ed19f

hubertp added the CI: Clean build required CI runners will be cleaned before and after this PR is built. label Aug 11, 2025

Address PR comments

62c6f5d

Reduced redundant `logHandler` setup in various places.

hubertp requested a review from Akirathan August 11, 2025 14:23

Merge branch 'develop' into wip/hubert/13308-logs-on-failure

f43f618

Akirathan approved these changes Aug 12, 2025

View reviewed changes

Merge branch 'develop' into wip/hubert/13308-logs-on-failure

cb9e5de

Fix benchmark runs

5cd4c6e

In non-strict mode errors are being reported as warnings. Should :fingers-crossed: help with CI failures.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Report logs only on failures in JUnit and ScalaTest #13721

Report logs only on failures in JUnit and ScalaTest #13721

Uh oh!

hubertp commented Aug 1, 2025 •

edited

Loading

Uh oh!

hubertp Aug 5, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Akirathan left a comment

Uh oh!

Uh oh!

Uh oh!

hubertp commented Aug 8, 2025

Uh oh!

Akirathan left a comment

Uh oh!

hubertp commented Aug 15, 2025

Uh oh!

Akirathan commented Aug 15, 2025 •

edited by hubertp

Loading

Uh oh!

hubertp commented Aug 15, 2025

Uh oh!

hubertp commented Aug 15, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Report logs only on failures in JUnit and ScalaTest #13721

Are you sure you want to change the base?

Report logs only on failures in JUnit and ScalaTest #13721

Uh oh!

Conversation

hubertp commented Aug 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Description

Important Notes

Checklist

Uh oh!

hubertp Aug 5, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Akirathan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

hubertp commented Aug 8, 2025

Uh oh!

Akirathan left a comment

Choose a reason for hiding this comment

Uh oh!

hubertp commented Aug 15, 2025

Uh oh!

Akirathan commented Aug 15, 2025 • edited by hubertp Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hubertp commented Aug 15, 2025

Uh oh!

hubertp commented Aug 15, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

hubertp commented Aug 1, 2025 •

edited

Loading

Akirathan commented Aug 15, 2025 •

edited by hubertp

Loading