Saner test event logging default for CI #1576
Conversation
Generate changelog in
|
|
I've found this kind of logging extremely helpful when a repo has integration tests. Fine to leave it off for unit-tests, but I don't think it's that controversial for integ! |
|
@ferozco The issue that if all your tests pass, we print out no events at all from the test tasks, so even if each test takes 10 secs but you have 60 of them, the test task will not log anything for 10 mins and the build will be killed by circle. I've seen this happen internally on at least 5 different projects, and since circle just Additionally, it's useful for cases when tests do hang and then eventually get killed by gradle to be able to see which test is bad |
|
Definitely more of a problem for integ tests than for unit tests, but I've seen this happen in our gradle nebula tests as well (which use the default |
|
I hear what you're saying perhaps we can find a middle ground? I think printing out every single test case results in a ton of noise and causes CI output to become large (which devtools doesn't like). Can we only print test suites (i.e. classes). Alternatively, we could just recommend the folks with 10m+ tests to configure this themselves |
|
@ferozco just for extra context, here are the repos in foundry already using this: https://sourcegraph.p.b/search?q=testLogging+repo:foundry/&patternType=literal |
|
Thanks for the link @iamdanfox I didn't realize how wide spread it was to configure some tests. I think I still stand by my opinion that this configuration is undesirable inside of unit tests though, so I'd be supportive of @CRogers idea of pushing this into dev-env |
| // there are lots of tests. Don't do this locally to avoid spamming massive amount of info for people running | ||
| // unit tests through the command line | ||
| if ("true".equals(System.getenv("CI"))) { | ||
| task.getTestLogging() |
There was a problem hiding this comment.
@CRogers can we try to not apply this to the ./gradlew test tasks which are normally just unit-tests??
There was a problem hiding this comment.
Yep, I have a commit to do this actually, I was just distracted by testing it on c-v-s that had the silently not running tests problem :p
…ntir/gradle-baseline into callumr/saner-test-logging-defaults
|
Released 3.58.1 |
Before this PR
We've seen numerous times that when projects have long running integration test tasks that take longer than 10 mins they get killed by circle ("context deadline exceeded"). If there are no failing tests we won't print anything in the gradle output, so it's possible to hit this deadline.
After this PR
==COMMIT_MSG==
Print more test logging output to avoid builds with long running tests getting terminated by circle ("context deadline exceeded").
==COMMIT_MSG==
Possible downsides?
Perhaps more noisy, but the failed tests already have junit xml produced in circle, so this should be strictly better.