test: logging level #1774
test: logging level #1774
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1774 +/- ##
==========================================
+ Coverage 85.70% 85.74% +0.04%
==========================================
Files 135 135
Lines 6924 6924
==========================================
+ Hits 5934 5937 +3
+ Misses 990 987 -3
Continue to review full report at Codecov.
|
Latency summaryCurrent PR yields:
Breakdown
Backed by latency-tracking. Further commits will update this comment. |
tests/unit/logging/test_logging.py
Outdated
| log(logger) | ||
| assert logger.logger.level == LogVerbosity.from_string('INFO') | ||
|
|
||
| os.environ['JINA_LOG_LEVEL'] = 'SUCCESS' |
There was a problem hiding this comment.
add a try catch mechanism that ensures thr os env is properly deleted but at same time captures the assertion error
There was a problem hiding this comment.
or split the test so that u can use fixtures to guarantee this
There was a problem hiding this comment.
Split the test and use fixtures in the latest commit. Thanks!
There was a problem hiding this comment.
this is not a valid test, as in your test assertion you write the same code as in the logger
if 'JINA_LOG_LEVEL' in os.environ:
verbose_level = LogVerbosity.from_string(os.environ['JINA_LOG_LEVEL'])
self.logger.setLevel(verbose_level.value)This is the same as having the code a=1 and then test it via assert a==1, it is almost meaningless in most cases.
The correct way is to test the emission of the logger, and see if it is suppressed given the current loglevel.
Revised in the latest commit. |
Test the logging level with os variable 'JINA_LOG_LEVEL' setting.
Related to #1744.