[MINOR] Reduce logging in MLContextTests#2432
[MINOR] Reduce logging in MLContextTests#2432Baunsgaard wants to merge 2 commits intoapache:mainfrom
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
e-strauss
left a comment
There was a problem hiding this comment.
lgtm, now you can read again the logs in the github actions without downloading them :)
| addTestConfiguration(dir, name); | ||
| getAndLoadTestConfiguration(name); | ||
|
|
||
| //run all mlcontext tests in loglevel trace to improve test coverage |
There was a problem hiding this comment.
how would enabling the tracing, would help with code coverage anyways? is there conditional code on log level?
There was a problem hiding this comment.
It is a bit annoying but people do this if we are not carefull.
In essence, many places use :
if(LOG.isDebug()) {
// some code
}
This means code coverage will be low, unless we explicity enable the logging.
We can circumvent it by having explicit logging tests, instead of just fully enabling logging.
There was a problem hiding this comment.
ahh true, grep gave me these stats:
sysds git:(main) grep -rnF "LOG.is" . | wc -l
401
sysds git:(main) grep -rnF "LOG.isTraceEnabled" . | wc -l
251
sysds git:(main) grep -rnF "LOG.isDebugEnabled" . | wc -l
131
There are some PRs that fail on MLContextTests, and i saw they produce over 80k logging.
Therefore, to test my new setup, here is a PR that reduce this.