Add a testing-internal module and functionality to SuppressLogger for…#4130
Conversation
anuraaga
requested review from
bogdandrutu,
carlosalberto,
jack-berg,
jkwatson and
jsuereth
as code owners
anuraaga
force-pushed
the
testing-internal-suppress
branch
from
d8713ba to
ab02fb3
Compare
…nto testing-internal-suppress
Codecov Report
@@ Coverage Diff @@
## main #4130 +/- ##
============================================
+ Coverage 90.14% 90.17% +0.02%
- Complexity 4504 4514 +10
============================================
Files 532 533 +1
Lines 13832 13831 -1
Branches 1322 1321 -1
============================================
+ Hits 12469 12472 +3
+ Misses 931 927 -4
Partials 432 432
Continue to review full report at Codecov.
|
| import org.slf4j.event.Level; | ||
|
|
||
| /** Tests for the {@link LoggingSpanExporter}. */ | ||
| @SuppressLogger(LoggingSpanExporter.class) |
There was a problem hiding this comment.
I'm a bit confused on what is going on here. Whether this annotation is present or not I don't see log output for this when I run ./gradlew :exporters:logging:test.
I thought it might be because the default log level is set to warn, but even after reducing it and removing the @SurpressLogger annotation, I don't see the logs produced by LoggingSpanExporter.
There was a problem hiding this comment.
Thanks for looking into this so deeply! I've never used a logging.properties file before and can't believe that I have to copy in the default values (e.g. ConsoleHandler) when using one... 🙀
buildscripts/test-logging.properties
Outdated
| handlers= java.util.logging.ConsoleHandler | ||
| .level= INFO |
There was a problem hiding this comment.
formatting here is slightly different than the rest of the file. missing a space before the = sign.
| "4.2.0", | ||
| listOf("mockito-core", "mockito-junit-jupiter") | ||
| ) | ||
| ), |
| import org.junit.jupiter.api.extension.BeforeTestExecutionCallback; | ||
| import org.junit.jupiter.api.extension.ExtensionContext; | ||
|
|
||
| public final class LoggerExtension |
There was a problem hiding this comment.
would you mind adding a bit of javadoc on this class to explain what it's for, and why we have it?
jkwatson
left a comment
jkwatson
left a comment
There was a problem hiding this comment.
Just one small ask for some additional internal documentation
3 participants
… a test.
In #4126 I realized that SimpleLogger doesn't support mutating logLevel - I had just had good luck with SetSystemProperty initially that the logger got initialized after the property mutation. I'll still rework that PR to focus on enabling slf4j logging throughout for consistency but for suppression sticking to JUL suppression because JUL does support level mutation.
I also went ahead and moved mockito-extensions into this module as it was a reason I've wanted a testing-internal module for a while now.