[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@@ Coverage Diff @@
## main #2432 +/- ##
============================================
- Coverage 71.55% 71.39% -0.16%
+ Complexity 47461 47368 -93
============================================
Files 1539 1539
Lines 182631 182631
Branches 35919 35919
============================================
- Hits 130677 130397 -280
- Misses 41944 42184 +240
- Partials 10010 10050 +40 ☔ 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.