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

Needless message printed with --failed-first and no failed tests #3853

Closed
nedbat opened this Issue Aug 22, 2018 · 15 comments

Comments

Projects
None yet
3 participants
@nedbat
Contributor

nedbat commented Aug 22, 2018

--failed-first always means, run all the tests, but order the recent failure to run first. But if there were no failures, it always prints a message that it is going to run all the tests. This message is not needed.

$ cat test_it.py
def test_it():
    assert 1 == 1

$ pytest
================== test session starts ===================
platform darwin -- Python 2.7.14, pytest-3.7.1, py-1.5.4, pluggy-0.7.1
rootdir: /private/tmp/failedfirst, inifile:
plugins: xdist-1.22.5, forked-0.2
collected 1 item

test_it.py .                                                                                                                                                               [100%]

================ 1 passed in 0.01 seconds ================

$ pytest --failed-first
================== test session starts ===================
platform darwin -- Python 2.7.14, pytest-3.7.1, py-1.5.4, pluggy-0.7.1
rootdir: /private/tmp/failedfirst, inifile:
plugins: xdist-1.22.5, forked-0.2
collected 1 item
run-last-failure: run all (no recorded failures)

test_it.py .                                                                                                                                                               [100%]

================ 1 passed in 0.01 seconds ================

$ pytest -q --failed-first
run-last-failure: run all (no recorded failures)
.                                                                                                                                                                          [100%]
1 passed in 0.01 seconds

@nicoddemus

This comment has been minimized.

Show comment
Hide comment
@nicoddemus

nicoddemus Aug 22, 2018

Member

Hi @nedbat,

I think the idea is to make sure the plugin is actually working... I personally like to see the message to make sure there are actually no failures and now all tests are running.

Member

nicoddemus commented Aug 22, 2018

Hi @nedbat,

I think the idea is to make sure the plugin is actually working... I personally like to see the message to make sure there are actually no failures and now all tests are running.

@nedbat

This comment has been minimized.

Show comment
Hide comment
@nedbat

nedbat Aug 23, 2018

Contributor

But whether there are failures or not, all the tests are run. Shouldn't we let the user assume the plugin is working? And show messages only when they need to know something? Can the plugin follow the -q of the main command line?

Contributor

nedbat commented Aug 23, 2018

But whether there are failures or not, all the tests are run. Shouldn't we let the user assume the plugin is working? And show messages only when they need to know something? Can the plugin follow the -q of the main command line?

@nicoddemus

This comment has been minimized.

Show comment
Hide comment
@nicoddemus

nicoddemus Aug 23, 2018

Member

Oh missed the bit with -q, definitely the plugin should obey -q, seems like an oversight.

When there are no more failures and the user passes --failed-first, you propose to show no message at all?

Member

nicoddemus commented Aug 23, 2018

Oh missed the bit with -q, definitely the plugin should obey -q, seems like an oversight.

When there are no more failures and the user passes --failed-first, you propose to show no message at all?

@nedbat

This comment has been minimized.

Show comment
Hide comment
@nedbat

nedbat Aug 23, 2018

Contributor

Yes :)

Contributor

nedbat commented Aug 23, 2018

Yes :)

@nicoddemus

This comment has been minimized.

Show comment
Hide comment
@nicoddemus

nicoddemus Aug 26, 2018

Member

#3886 deals with handling -q properly, after that gets merged we can then suppress the output when there are no recorded failures on the features branch.

Member

nicoddemus commented Aug 26, 2018

#3886 deals with handling -q properly, after that gets merged we can then suppress the output when there are no recorded failures on the features branch.

@dhirensr

This comment has been minimized.

Show comment
Hide comment
@dhirensr

dhirensr Aug 29, 2018

Member

@nedbat @nicoddemus : I have read the complete issue and also #3886 ! does this ticket say that when --lf ,then the output now is "run-last-failure: run all (no recorded failures)" but instead it should behave exactly as --q and nothing should display?

Member

dhirensr commented Aug 29, 2018

@nedbat @nicoddemus : I have read the complete issue and also #3886 ! does this ticket say that when --lf ,then the output now is "run-last-failure: run all (no recorded failures)" but instead it should behave exactly as --q and nothing should display?

@nicoddemus

This comment has been minimized.

Show comment
Hide comment
@nicoddemus

nicoddemus Aug 29, 2018

Member

@dhirensr exactly! 😁

Member

nicoddemus commented Aug 29, 2018

@dhirensr exactly! 😁

@dhirensr

This comment has been minimized.

Show comment
Hide comment
@dhirensr

dhirensr Aug 29, 2018

Member

@nicoddemus : where do we change the code like do we send mode as empty string or change the print function ? could you also guide me where is the print ignored in case of --quiet ,I am not able to find that piece of code which ignores the print statement and prints nothing!

Member

dhirensr commented Aug 29, 2018

@nicoddemus : where do we change the code like do we send mode as empty string or change the print function ? could you also guide me where is the print ignored in case of --quiet ,I am not able to find that piece of code which ignores the print statement and prints nothing!

@nicoddemus

This comment has been minimized.

Show comment
Hide comment
@nicoddemus

nicoddemus Aug 29, 2018

Member

Sorry, I'm not sure I follow your question... but all we need to do is to not print the message if there are no recorded failures, so this function:

def pytest_report_collectionfinish(self):
if self.active and self.config.getoption("verbose") >= 0:
if not self._previously_failed_count:
mode = "run {} (no recorded failures)".format(
self._no_failures_behavior
)
else:
noun = "failure" if self._previously_failed_count == 1 else "failures"
suffix = " first" if self.config.getoption("failedfirst") else ""
mode = "rerun previous {count} {noun}{suffix}".format(
count=self._previously_failed_count, suffix=suffix, noun=noun
)
return "run-last-failure: %s" % mode

Should be changed to:

    def pytest_report_collectionfinish(self):
        if self.active and self.config.getoption("verbose") >= 0:
            if not self._previously_failed_count:
                return None
            else:
                noun = "failure" if self._previously_failed_count == 1 else "failures"
                suffix = " first" if self.config.getoption("failedfirst") else ""
                mode = "rerun previous {count} {noun}{suffix}".format(
                    count=self._previously_failed_count, suffix=suffix, noun=noun
                )
            return "run-last-failure: %s" % mode

Or even better:

    def pytest_report_collectionfinish(self):
        if self.active and self.config.getoption("verbose") >= 0:
            if not self._previously_failed_count:
                return None

            noun = "failure" if self._previously_failed_count == 1 else "failures"
            suffix = " first" if self.config.getoption("failedfirst") else ""
            mode = "rerun previous {count} {noun}{suffix}".format(
                count=self._previously_failed_count, suffix=suffix, noun=noun
            )
            return "run-last-failure: %s" % mode
Member

nicoddemus commented Aug 29, 2018

Sorry, I'm not sure I follow your question... but all we need to do is to not print the message if there are no recorded failures, so this function:

def pytest_report_collectionfinish(self):
if self.active and self.config.getoption("verbose") >= 0:
if not self._previously_failed_count:
mode = "run {} (no recorded failures)".format(
self._no_failures_behavior
)
else:
noun = "failure" if self._previously_failed_count == 1 else "failures"
suffix = " first" if self.config.getoption("failedfirst") else ""
mode = "rerun previous {count} {noun}{suffix}".format(
count=self._previously_failed_count, suffix=suffix, noun=noun
)
return "run-last-failure: %s" % mode

Should be changed to:

    def pytest_report_collectionfinish(self):
        if self.active and self.config.getoption("verbose") >= 0:
            if not self._previously_failed_count:
                return None
            else:
                noun = "failure" if self._previously_failed_count == 1 else "failures"
                suffix = " first" if self.config.getoption("failedfirst") else ""
                mode = "rerun previous {count} {noun}{suffix}".format(
                    count=self._previously_failed_count, suffix=suffix, noun=noun
                )
            return "run-last-failure: %s" % mode

Or even better:

    def pytest_report_collectionfinish(self):
        if self.active and self.config.getoption("verbose") >= 0:
            if not self._previously_failed_count:
                return None

            noun = "failure" if self._previously_failed_count == 1 else "failures"
            suffix = " first" if self.config.getoption("failedfirst") else ""
            mode = "rerun previous {count} {noun}{suffix}".format(
                count=self._previously_failed_count, suffix=suffix, noun=noun
            )
            return "run-last-failure: %s" % mode
@dhirensr

This comment has been minimized.

Show comment
Hide comment
@dhirensr

dhirensr Aug 29, 2018

Member

@nicoddemus : Oh yeah! that i had already fixed! I just thought I need to remove "platform linux -- Python 3.5.2, pytest-unknown, py-1.6.0, pluggy-0.7.1
rootdir: /home/dhirensr/pytest, inifile: tox.ini
collected 2 items
" also!
I thought like complete terminal should look empty that's the requirement!
I am making a PR with test! I need to mention in the document also right? Thanks a lot for helping man!

Member

dhirensr commented Aug 29, 2018

@nicoddemus : Oh yeah! that i had already fixed! I just thought I need to remove "platform linux -- Python 3.5.2, pytest-unknown, py-1.6.0, pluggy-0.7.1
rootdir: /home/dhirensr/pytest, inifile: tox.ini
collected 2 items
" also!
I thought like complete terminal should look empty that's the requirement!
I am making a PR with test! I need to mention in the document also right? Thanks a lot for helping man!

@nicoddemus

This comment has been minimized.

Show comment
Hide comment
@nicoddemus

nicoddemus Aug 29, 2018

Member

Oh I see, thanks, but no, the idea is to remove only the "run-last-failure: run all (no recorded failures)" message.

When opening the PR, make sure to add a CHANGELOG entry and target the features branch. 👍

Member

nicoddemus commented Aug 29, 2018

Oh I see, thanks, but no, the idea is to remove only the "run-last-failure: run all (no recorded failures)" message.

When opening the PR, make sure to add a CHANGELOG entry and target the features branch. 👍

@dhirensr

This comment has been minimized.

Show comment
Hide comment
@dhirensr

dhirensr Aug 30, 2018

Member

@nicoddemus : PR submitted! #3912
Please review and let me know if any changes required! I changed the code ,added a test and also removed 1 test option which was checking "run all" string.
Thanks a lot for helping! 😄

Member

dhirensr commented Aug 30, 2018

@nicoddemus : PR submitted! #3912
Please review and let me know if any changes required! I changed the code ,added a test and also removed 1 test option which was checking "run all" string.
Thanks a lot for helping! 😄

@nicoddemus

This comment has been minimized.

Show comment
Hide comment
@nicoddemus

nicoddemus Aug 30, 2018

Member

Sure thing, thanks @dhirensr, I will take a look as soon as I get a chance! 👍

Member

nicoddemus commented Aug 30, 2018

Sure thing, thanks @dhirensr, I will take a look as soon as I get a chance! 👍

peterbe added a commit to peterbe/django-peterbecom that referenced this issue Aug 30, 2018

Update pytest to 3.7.4 (#339)
This PR updates [pytest](https://pypi.org/project/pytest) from **3.7.3** to **3.7.4**.



<details>
  <summary>Changelog</summary>
  
  
   ### 3.7.4
   ```
   =========================

Bug Fixes
---------

- `3506 &lt;https://github.com/pytest-dev/pytest/issues/3506&gt;`_: Fix possible infinite recursion when writing ``.pyc`` files.


- `3853 &lt;https://github.com/pytest-dev/pytest/issues/3853&gt;`_: Cache plugin now obeys the ``-q`` flag when ``--last-failed`` and ``--failed-first`` flags are used.


- `3883 &lt;https://github.com/pytest-dev/pytest/issues/3883&gt;`_: Fix bad console output when using ``console_output_style=classic``.


- `3888 &lt;https://github.com/pytest-dev/pytest/issues/3888&gt;`_: Fix macOS specific code using ``capturemanager`` plugin in doctests.



Improved Documentation
----------------------

- `3902 &lt;https://github.com/pytest-dev/pytest/issues/3902&gt;`_: Fix pytest.org links
   ```
   
  
</details>


 

<details>
  <summary>Links</summary>
  
  - PyPI: https://pypi.org/project/pytest
  - Changelog: https://pyup.io/changelogs/pytest/
  - Homepage: https://docs.pytest.org/en/latest/
</details>

cmccandless added a commit to cmccandless/multisite that referenced this issue Aug 30, 2018

Update pytest to 3.7.4 (#10)
This PR updates [pytest](https://pypi.org/project/pytest) from **3.7.2** to **3.7.4**.



<details>
  <summary>Changelog</summary>
  
  
   ### 3.7.4
   ```
   =========================

Bug Fixes
---------

- `3506 &lt;https://github.com/pytest-dev/pytest/issues/3506&gt;`_: Fix possible infinite recursion when writing ``.pyc`` files.


- `3853 &lt;https://github.com/pytest-dev/pytest/issues/3853&gt;`_: Cache plugin now obeys the ``-q`` flag when ``--last-failed`` and ``--failed-first`` flags are used.


- `3883 &lt;https://github.com/pytest-dev/pytest/issues/3883&gt;`_: Fix bad console output when using ``console_output_style=classic``.


- `3888 &lt;https://github.com/pytest-dev/pytest/issues/3888&gt;`_: Fix macOS specific code using ``capturemanager`` plugin in doctests.



Improved Documentation
----------------------

- `3902 &lt;https://github.com/pytest-dev/pytest/issues/3902&gt;`_: Fix pytest.org links
   ```
   
  
  
   ### 3.7.3
   ```
   =========================

Bug Fixes
---------

- `3033 &lt;https://github.com/pytest-dev/pytest/issues/3033&gt;`_: Fixtures during teardown can again use ``capsys`` and ``capfd`` to inspect output captured during tests.


- `3773 &lt;https://github.com/pytest-dev/pytest/issues/3773&gt;`_: Fix collection of tests from ``__init__.py`` files if they match the ``python_files`` configuration option.


- `3796 &lt;https://github.com/pytest-dev/pytest/issues/3796&gt;`_: Fix issue where teardown of fixtures of consecutive sub-packages were executed once, at the end of the outer
  package.


- `3816 &lt;https://github.com/pytest-dev/pytest/issues/3816&gt;`_: Fix bug where ``--show-capture=no`` option would still show logs printed during fixture teardown.


- `3819 &lt;https://github.com/pytest-dev/pytest/issues/3819&gt;`_: Fix ``stdout/stderr`` not getting captured when real-time cli logging is active.


- `3843 &lt;https://github.com/pytest-dev/pytest/issues/3843&gt;`_: Fix collection error when specifying test functions directly in the command line using ``test.py::test`` syntax together with ``--doctest-modules``.


- `3848 &lt;https://github.com/pytest-dev/pytest/issues/3848&gt;`_: Fix bugs where unicode arguments could not be passed to ``testdir.runpytest`` on Python 2.


- `3854 &lt;https://github.com/pytest-dev/pytest/issues/3854&gt;`_: Fix double collection of tests within packages when the filename starts with a capital letter.



Improved Documentation
----------------------

- `3824 &lt;https://github.com/pytest-dev/pytest/issues/3824&gt;`_: Added example for multiple glob pattern matches in ``python_files``.


- `3833 &lt;https://github.com/pytest-dev/pytest/issues/3833&gt;`_: Added missing docs for ``pytester.Testdir``.


- `3870 &lt;https://github.com/pytest-dev/pytest/issues/3870&gt;`_: Correct documentation for setuptools integration.



Trivial/Internal Changes
------------------------

- `3826 &lt;https://github.com/pytest-dev/pytest/issues/3826&gt;`_: Replace broken type annotations with type comments.


- `3845 &lt;https://github.com/pytest-dev/pytest/issues/3845&gt;`_: Remove a reference to issue `568 &lt;https://github.com/pytest-dev/pytest/issues/568&gt;`_ from the documentation, which has since been
  fixed.
   ```
   
  
</details>


 

<details>
  <summary>Links</summary>
  
  - PyPI: https://pypi.org/project/pytest
  - Changelog: https://pyup.io/changelogs/pytest/
  - Homepage: https://docs.pytest.org/en/latest/
</details>

cmccandless added a commit to cmccandless/tools that referenced this issue Aug 30, 2018

Update pytest to 3.7.4 (#27)
This PR updates [pytest](https://pypi.org/project/pytest) from **3.7.3** to **3.7.4**.



<details>
  <summary>Changelog</summary>
  
  
   ### 3.7.4
   ```
   =========================

Bug Fixes
---------

- `3506 &lt;https://github.com/pytest-dev/pytest/issues/3506&gt;`_: Fix possible infinite recursion when writing ``.pyc`` files.


- `3853 &lt;https://github.com/pytest-dev/pytest/issues/3853&gt;`_: Cache plugin now obeys the ``-q`` flag when ``--last-failed`` and ``--failed-first`` flags are used.


- `3883 &lt;https://github.com/pytest-dev/pytest/issues/3883&gt;`_: Fix bad console output when using ``console_output_style=classic``.


- `3888 &lt;https://github.com/pytest-dev/pytest/issues/3888&gt;`_: Fix macOS specific code using ``capturemanager`` plugin in doctests.



Improved Documentation
----------------------

- `3902 &lt;https://github.com/pytest-dev/pytest/issues/3902&gt;`_: Fix pytest.org links
   ```
   
  
</details>


 

<details>
  <summary>Links</summary>
  
  - PyPI: https://pypi.org/project/pytest
  - Changelog: https://pyup.io/changelogs/pytest/
  - Homepage: https://docs.pytest.org/en/latest/
</details>

bors bot added a commit to rehandalal/therapist that referenced this issue Aug 30, 2018

Merge #30
30: Update pytest to 3.7.4 r=rehandalal a=pyup-bot


This PR updates [pytest](https://pypi.org/project/pytest) from **3.7.3** to **3.7.4**.



<details>
  <summary>Changelog</summary>
  
  
   ### 3.7.4
   ```
   =========================

Bug Fixes
---------

- `3506 &lt;https://github.com/pytest-dev/pytest/issues/3506&gt;`_: Fix possible infinite recursion when writing ``.pyc`` files.


- `3853 &lt;https://github.com/pytest-dev/pytest/issues/3853&gt;`_: Cache plugin now obeys the ``-q`` flag when ``--last-failed`` and ``--failed-first`` flags are used.


- `3883 &lt;https://github.com/pytest-dev/pytest/issues/3883&gt;`_: Fix bad console output when using ``console_output_style=classic``.


- `3888 &lt;https://github.com/pytest-dev/pytest/issues/3888&gt;`_: Fix macOS specific code using ``capturemanager`` plugin in doctests.



Improved Documentation
----------------------

- `3902 &lt;https://github.com/pytest-dev/pytest/issues/3902&gt;`_: Fix pytest.org links
   ```
   
  
</details>


 

<details>
  <summary>Links</summary>
  
  - PyPI: https://pypi.org/project/pytest
  - Changelog: https://pyup.io/changelogs/pytest/
  - Homepage: https://docs.pytest.org/en/latest/
</details>



Co-authored-by: pyup-bot <github-bot@pyup.io>

nicoddemus added a commit that referenced this issue Aug 30, 2018

Merge pull request #3912 from dhirensr/needless_message
Needless message printed with --failed-first and no failed tests #3853

@nicoddemus nicoddemus closed this Aug 30, 2018

@nicoddemus

This comment has been minimized.

Show comment
Hide comment
@nicoddemus

nicoddemus Aug 30, 2018

Member

The complete feature will be available in pytest-3.8. Thanks @nedbat!

Member

nicoddemus commented Aug 30, 2018

The complete feature will be available in pytest-3.8. Thanks @nedbat!

@nicoddemus

This comment has been minimized.

Show comment
Hide comment
@nicoddemus

nicoddemus Aug 30, 2018

Member

And thanks @dhirensr for the PR! 🤗

Member

nicoddemus commented Aug 30, 2018

And thanks @dhirensr for the PR! 🤗

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment