Make sure (basic) logging is set up before each test class

[famod]

Alternative, actually much simpler approach to #22821 (which had this irritating flaw: #22821 (comment)).

This makes sure that basic logging is available for all kinds of (Junit) tests, Quarkus or not, right from the start!

This not only achieves the main goal of getting log output from non-Quarkus tests (in whichever order they are executed), but it has also the nice effect that all very early log output is "instant", instead of being delayed to regular log setup. E.g. a log statement from io.quarkus.test.junit.QuarkusTestProfile.getConfigOverrides() appears right away and will be there even if the following log activation is removed:

quarkus/test-framework/junit5/src/main/java/io/quarkus/test/junit/QuarkusTestExtension.java

Lines 309 to 312 in f133a1b

	} catch (Throwable e) {
	if (!InitialConfigurator.DELAYED_HANDLER.isActivated()) {
	activateLogging();
	}

In fact, I think most such calls can be removed with this very early running, global JUnit callback.
I could do that in a follow-up PR which could then also include the renaming from handleFailedStart() to initializeBasicLogging() (as discussed here: #22821 (comment)).

PS: If this is merged, I will also follow up with a PR making RestClientConfigTest more robust and removing that catch from some core tests (see #22821 (comment)). #22921

PPS: There is at least one issue that I believe this PR will fix. I'll look it up later.

[famod]

Not pretty, but saves up to ~50ms total runtime in my not-so-small project.

[quarkus-bot]

This workflow status is outdated as a new workflow run has been triggered.

Failing Jobs - Building 50f9abd

Status	Name	Step	Failures	Logs	Raw logs
✔️	JVM Tests - JDK 11
✖	JVM Tests - JDK 17	Build	Failures	Logs	Raw logs

Full information is available in the Build summary check run.

Failures

⚙️ JVM Tests - JDK 17 #

- Failing: integration-tests/grpc-hibernate 

📦 integration-tests/grpc-hibernate

✖ com.example.grpc.hibernate.BlockingRawTest.shouldAdd line 60 - More details - Source on GitHub

org.awaitility.core.ConditionTimeoutException: Condition with com.example.grpc.hibernate.BlockingRawTest was not fulfilled within 30 seconds.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:164)
	at org.awaitility.core.CallableCondition.await(CallableCondition.java:78)

[geoand]

Very interesting approach! I like it!

I assume you have tested this extensively on your project?

[famod]

I assume you have tested this extensively on your project?

Yes. I didn't test this for days but I tested the scenarios that came to mind:

• all tests in the default order
• single QuarkusTest, also in IDE
• single non-Quarkus test, also in IDE
• continuous testing (all tests, only one QuarkusTest, only one non-Quarkus test)

@stuartwdouglas do you want to have a look as well? Thanks!

[gsmet]

I haven't looked at this patch in details but as for the configuration properties:

• either they are user exposed and they should have the quarkus. prefix (and a dummy config class to avoid the warnings and get them documented);
• or they are internal only and in this case we don't care.

I have no idea in which case you are here but I wanted to remind you this simple rule.

[famod]

@gsmet Can a "dummy config class" be exempt from doc generation?
Because in this case here -D works (JUnit feature), but application.properties doesn't (it's junit-platform.properties instead).

[stuartwdouglas]

LGTM

[stuartwdouglas]

If these are not proper config properties that read from application.properties then IMHO we should not have config classes for them, as it will confuse people when they don't work the way you would expect. I like the current junit. names as it makes it clear that this is managed by junit rather than Quarkus.

That said they should be documented somewhere.

[gsmet]

If these are not proper config properties that read from application.properties then IMHO we should not have config classes for them

Yes I agree.

[famod]

That said they should be documented somewhere.

The config keys for TestClassOrder are really meant for users, which is why I was asking for a release-aware way to link to sources from docs, because I didn't want to repeat myself: #21748 (comment)
If that doesn't work I'll probably have to "duplicate" the keys by hand for the docs (and give it a separate section 🤔).
I think that's a separate PR?

That config key in BasicLoggingEnabler is only meant as a kill switch in case unexpected things happen (well, it's new).
So I wouldn't want to document that too openly TBH.

[famod]

Let's get this in for CR1 to receive some feedback (or at least no negative one).

I'll think about that documentation task (input welcome!).

PS: I'll also go through the issues I think this might be fixing.

[famod]

FTR, I believe this resolves #7696, which has received comparatively many reactions.

I added a prominent comment, asking people to try out 2.7.0.CR1 and giving feedback:
#7696 (comment)

I will close that issue eventually.