-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Fix tests by resetting the log level #5888
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
Fix tests by resetting the log level #5888
Conversation
Could you please post the full pytest output? The CLI is still very much a mess with lots of global state, and the tests which have been written in the past did a lot of monkey-patching while trying to reset state afterwards, but none of them ever did this properly. I've recently rewritten the CLI's logging/console-print setup tests with that in mind. However, I can't reproduce the log level issue. The tests should always run in the same order, determined by this:
Also, this diff should be better, as it doesn't rely on a static value (even though it's the same value as set in diff --git a/tests/cli/test_main_logging.py b/tests/cli/test_main_logging.py
index 3416fd69..f7024787 100644
--- a/tests/cli/test_main_logging.py
+++ b/tests/cli/test_main_logging.py
@@ -33,10 +33,13 @@ def _setup(monkeypatch: pytest.MonkeyPatch, session: Streamlink):
monkeypatch.setattr("streamlink_cli.main.setup_signals", Mock())
monkeypatch.setattr("streamlink_cli.argparser.find_default_player", Mock())
+ level = streamlink_cli.main.logger.root.level
+
try:
yield
finally:
streamlink_cli.main.logger.root.handlers.clear()
+ streamlink_cli.main.logger.root.setLevel(level)
streamlink_cli.main.args = None # type: ignore[assignment]
streamlink_cli.main.console = None # type: ignore[assignment] |
Here are the results: The folder in |
The uploaded file is 404... Please post a GH gist instead. |
Ok, uploaded here: https://gist.github.com/amurzeau/a787142847cdd962c80b96492ae2026e I think I've found the issue, this is because of So nothing will match |
I get no error with |
There's nothing that can be done here to fix this. We can't set the |
Yes, this is related with how Debian handle this.
I can fix the issue by adding this to the pytest command line:
An alternative would be to run tests directly on the root streamlink source directory instead, but I'm not sure this is really better from a Debian package perspective. I'm closing this PR given the issue is caused by packaging scripts, let me know if you me to still reopen it. |
The logger level should still be reset, so I'd appreciate if you could reopen and update the PR with the diff I have posted. Thanks. |
test_main_logging tests are changing the log level. We need to reset it to its default value on teardown.
97733f4
to
85ab2fc
Compare
Thanks! |
While packing the version, I got tests errors, probably because the order of test execution wasn't the same.
test_main_logging
tests are changing the log level by callingstreamlink_cli.main.setup(parser)
. We need to reset it to its default value on teardown.The log level is set by this code in
setup
:Here is a test failure caused by this: