Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(Java): disable verbose unit test logging #948

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 15 additions & 12 deletions AwsCryptographicMaterialProviders/runtimes/java/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -224,18 +224,21 @@ tasks.test {
// This will show System.out.println statements
testLogging.showStandardStreams = true

testLogging {
lifecycle {
events = mutableSetOf(org.gradle.api.tasks.testing.logging.TestLogEvent.FAILED, org.gradle.api.tasks.testing.logging.TestLogEvent.PASSED, org.gradle.api.tasks.testing.logging.TestLogEvent.SKIPPED)
exceptionFormat = org.gradle.api.tasks.testing.logging.TestExceptionFormat.FULL
showExceptions = true
showCauses = true
showStackTraces = true
showStandardStreams = true
}
info.events = lifecycle.events
info.exceptionFormat = lifecycle.exceptionFormat
}
// Disable Verbose test logging to make the LocalCMCTests less flaky.
// It is hard to log 300k messages to a terminal,
// particularly if 10 threads are concurrently writing to the terminal.
// testLogging {
// lifecycle {
// events = mutableSetOf(org.gradle.api.tasks.testing.logging.TestLogEvent.FAILED, org.gradle.api.tasks.testing.logging.TestLogEvent.PASSED, org.gradle.api.tasks.testing.logging.TestLogEvent.SKIPPED)
// exceptionFormat = org.gradle.api.tasks.testing.logging.TestExceptionFormat.FULL
// showExceptions = true
// showCauses = true
// showStackTraces = true
// showStandardStreams = true
// }
// info.events = lifecycle.events
// info.exceptionFormat = lifecycle.exceptionFormat
// }
Comment on lines +227 to +241
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not just remove this instead of commenting?

Copy link
Contributor Author

@texastony texastony Nov 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because if a test fails,
it is very helpful to know what it is.
Which this lifecycle logging helps with.

Locally, the logs are not that helpful,
as TestNG will write a test report HTML.

But that report is not available from GitHub CI.
Only the logs are.
Hence, it is nice to leave this comment block in case someone needs it in the future.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then maybe use a flag or disable the params that control this? Maybe this can help: https://docs.gradle.org/current/javadoc/org/gradle/api/tasks/testing/logging/TestExceptionFormat.html#SHORT

I'm afraid that by commenting it out, we are creating a precedence that it's okay to comment out code for "future" use case. And have seen this in our code base. If nothing else, we can remove this and add link to the gradle doc.


// See https://github.com/gradle/kotlin-dsl/issues/836
addTestListener(object : TestListener {
Expand Down
Loading