feature: Add OFF log level to StandardEnvironment

[gchaperon]

As discussed in #3901, this PR adds a new logging level OFF(0) and modifies the printing logic in StandardEnvironment so that it is turned off if this new level is set.

I tagged this as a feature following the tag of the issue.
The contributing guidelines state that feature PR should have tests and corresponding documentation. I though this PR is maybe to small for that, but please let me know if you would like tests added and what should they cover.

[slarse]

Thanks for the PR @gchaperon!

No behavioral change is too small for a test case :). See spoon.test.logging.LogTest. It shouldn't be too hard to verify that no message gets logged for any logging level if the level is set to OFF. You can draw inspiration from the testAllLevelsForLogs test case (see how it captures the logging output).

[slarse]

Perhaps this is more clear?

Suggested change

	if (this.level.compareTo(Level.OFF) > 0 && messageLevel.toInt() <= this.level.toInt()) {
	if (this.level != Level.OFF && messageLevel.toInt() <= this.level.toInt()) {

I think it's a bit easier to read as it doesn't require one to go check the declaration order of the Level values. This is probably a matter of preference, though, if you prefer your solution feel free to keep it.

[slarse]

Note that there's a tiny behavioral difference: level != Level.OFF will cause the if-block not to be entered even if the passed level is OFF, whereas your original solution will cause it to be entered (but do nothing as no case in the switch matches)

[gchaperon]

sure, that's better, I didn't know enums could compare like that (that's how little java I know 😅)

[gchaperon]

hi!

I changed the line following your advice and added a test.
I have no idea whatsoever as to where to put the test. I saw there was a class for nonparameterized tests, so I added it there.
Also, since print is private I used (what I think is) the closest proxy for it, which is report. This method receives a Processor but does not use it, so I used whatever Processor implementation was the easiest to instantiate.

Let me know what you think.

[slarse]

I think this looks very good! The placement of the test is correct. The old JUnit4 parameterization is awkward because it parameterizes the entire class, which is why a nested class is needed for the non-parameterized tests.

JUnit5 parameterization is far less confusing (very easy to apply to just a single test).

Anyway, I just had a single comment in that we typically try to put a contract at the top of the test, stating what the test should do. See for example testMavenLauncherLogs (and my comment).

[slarse]

Could you add a contract as a comment at the top of the test method? I.e. a comment stating what the test should do along, like so (ofc replacing my nonsense text):

Suggested change

	@Test
	public void testLoggingOff() {
	final TestLogger logger = TestLoggerFactory.getTestLogger(Launcher.class);
	@Test
	public void testLoggingOff() {
	// contract: The logger should do X given Y and perhaps Z
	final TestLogger logger = TestLoggerFactory.getTestLogger(Launcher.class);

[gchaperon]

aahrg, I saw you had this convention, but then forgot to add one
I just fixed it

[gchaperon]

there, I just added a contract

[slarse]

Just one final thing, there are a couple of redundant imports in LogTest, probably added automatically by your IDE when you were trying stuff out. I missed them before, sorry!

After that I'm satisfied and we ping an integrator :)

[slarse]

These two imports seem redundant.

Suggested change

	import spoon.processing.Processor;
	import spoon.reflect.declaration.CtElement;

[gchaperon]

That is exactly what happened, thanks for the notice.

[slarse]

@gchaperon Looks very good to me!

Ping @monperrus for workflow activation

[slarse]

Oh, and @gchaperon change the title from WIP to review :)

[monperrus]

Thanks a lot @gchaperon for your first contribution in Spoon.

[gchaperon]

also thanks for the guidance and patience!

[slarse]

also thanks for the guidance and patience!

You're very welcome, we're always super happy to help new (or existing, some things are hard!) contributors in completing their PRs :)