Skip to content
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 verbosity bug in --collect-only #5391

Merged
merged 1 commit into from Jun 5, 2019
Merged

Conversation

twmr
Copy link
Contributor

@twmr twmr commented Jun 4, 2019

Closes: #5383

  • Target the master branch for bug fixes, documentation updates and trivial changes.
  • Target the features branch for new features and removals/deprecations.
  • Include documentation when adding new features.
  • Include new tests or update existing tests when applicable.

Unless your change is trivial or a small documentation fix (e.g., a typo or reword of a small section) please:

  • Create a new changelog file in the changelog folder, with a name like <ISSUE NUMBER>.<TYPE>.rst. See changelog/README.rst for details.
  • Add yourself to AUTHORS in alphabetical order;

@@ -628,6 +624,10 @@ def pytest_sessionstart(self):
@pytest.hookimpl(hookwrapper=True)
def pytest_runtestloop(self, session):
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit surprised that this hook is called even when --collect-only is used

@codecov
Copy link

codecov bot commented Jun 4, 2019

Codecov Report

Merging #5391 into master will decrease coverage by <.01%.
The diff coverage is 94.44%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5391      +/-   ##
==========================================
- Coverage   95.98%   95.98%   -0.01%     
==========================================
  Files         114      114              
  Lines       25496    25512      +16     
  Branches     2474     2478       +4     
==========================================
+ Hits        24473    24488      +15     
  Misses        718      718              
- Partials      305      306       +1
Impacted Files Coverage Δ
src/_pytest/logging.py 95.85% <100%> (+0.03%) ⬆️
testing/logging/test_reporting.py 99.66% <92.3%> (-0.34%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update be84ba8...577b0df. Read the comment docs.

@blueyed blueyed added the needs backport applied to PRs, indicates that it should be ported to the current bug-fix branch label Jun 4, 2019
@nicoddemus
Copy link
Member

Thanks @thisch for the quick PR!

While it fixes the main problem, using log-cli-level still produces the === section:

λ pytest .tmp\test-foo.py --collect-only -q
test-foo.py::test_post_mortem

no tests ran in 0.01 seconds

 λ pytest .tmp\test-foo.py --collect-only -q --log-cli-level=INFO
test-foo.py::test_post_mortem

=================== no tests ran in 0.01 seconds ====================

@nicoddemus
Copy link
Member

Do we still want to capture log messages during --collect-only? If not this patch (over this PR) fixes this:

diff --git a/src/_pytest/logging.py b/src/_pytest/logging.py
index f8ff0362c..1cffda1c3 100644
--- a/src/_pytest/logging.py
+++ b/src/_pytest/logging.py
@@ -624,16 +624,19 @@ class LoggingPlugin:
     @pytest.hookimpl(hookwrapper=True)
     def pytest_runtestloop(self, session):
         """Runs all collected test items."""
-        if self._log_cli_enabled() and self._config.getoption("verbose") < 1:
-            # setting verbose flag is needed to avoid messy test progress output
-            self._config.option.verbose = 1
-
-        with self.live_logs_context():
-            if self.log_file_handler is not None:
-                with catching_logs(self.log_file_handler, level=self.log_file_level):
+        if session.config.option.collectonly:
+            yield
+        else:
+            if self._log_cli_enabled() and self._config.getoption("verbose") < 1:
+                # setting verbose flag is needed to avoid messy test progress output
+                self._config.option.verbose = 1
+
+            with self.live_logs_context():
+                if self.log_file_handler is not None:
+                    with catching_logs(self.log_file_handler, level=self.log_file_level):
+                        yield  # run all the tests
+                else:
                     yield  # run all the tests
-            else:
-                yield  # run all the tests

@twmr
Copy link
Contributor Author

twmr commented Jun 5, 2019

Do we still want to capture log messages during --collect-only? If not this patch (over this PR) fixes this:

I dont understand your comment. live-logging/capturing of the log messages that are created in the collection phase still works, even with your patch.

Note that I've already applied your patch, thx!

@nicoddemus
Copy link
Member

I dont understand your comment. live-logging/capturing of the log messages that are created in the collection phase still works, even with your patch.

Sorry, I was not very clear: I meant that my patch fixes the output difference from my previous comment (the ==== no tests run in ... ==== footer).

@nicoddemus nicoddemus merged commit 3656885 into pytest-dev:master Jun 5, 2019
nicoddemus added a commit to nicoddemus/pytest that referenced this pull request Jun 5, 2019
Fix verbosity bug in --collect-only
@nicoddemus
Copy link
Member

Backport: #5411

@nicoddemus nicoddemus removed the needs backport applied to PRs, indicates that it should be ported to the current bug-fix branch label Jun 5, 2019
asottile added a commit that referenced this pull request Jun 6, 2019
[4.6] Fix verbosity bug in --collect-only (backport of #5391)
@nicoddemus nicoddemus added the backported PR has been backported to the current bug-fix branch label Jun 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backported PR has been backported to the current bug-fix branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

-q no longer impacts --collect-only (when used with --log-cli-level)
3 participants