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

Crash when using --setup-show and KeyboardInterrupt #5906

Closed
iwanb opened this issue Oct 2, 2019 · 8 comments · Fixed by #6009
Closed

Crash when using --setup-show and KeyboardInterrupt #5906

iwanb opened this issue Oct 2, 2019 · 8 comments · Fixed by #6009
Assignees

Comments

@iwanb
Copy link
Contributor

@iwanb iwanb commented Oct 2, 2019

When interrupting a test using Ctrl-C (KeyboardInterrupt/SIGINT), the "setuponly" plugin crashes due to some interaction with the capture manager.

Using pytest 5.2.0 (fresh install on Python 3.7 in a virtual env), given a dummy test:

import time
import pytest


@pytest.fixture()
def setup():
    print("Setup")
    yield
    print("Teardown")


def test_something(setup):
    time.sleep(60)

Run it with pytest --setup-show test_teardown.py and abort it while the test is running.

Stacktrace:

Traceback (most recent call last):
  File "venv/bin/pytest", line 10, in <module>
    sys.exit(main())
  File "venv/lib/python3.7/site-packages/_pytest/config/__init__.py", line 90, in main
    return config.hook.pytest_cmdline_main(config=config)
  File "venv/lib/python3.7/site-packages/pluggy/hooks.py", line 289, in __call__
    return self._hookexec(self, self.get_hookimpls(), kwargs)
  File "venv/lib/python3.7/site-packages/pluggy/manager.py", line 87, in _hookexec
    return self._inner_hookexec(hook, methods, kwargs)
  File "venv/lib/python3.7/site-packages/pluggy/manager.py", line 81, in <lambda>
    firstresult=hook.spec.opts.get("firstresult") if hook.spec else False,
  File "venv/lib/python3.7/site-packages/pluggy/callers.py", line 208, in _multicall
    return outcome.get_result()
  File "venv/lib/python3.7/site-packages/pluggy/callers.py", line 80, in get_result
    raise ex[1].with_traceback(ex[2])
  File "venv/lib/python3.7/site-packages/pluggy/callers.py", line 187, in _multicall
    res = hook_impl.function(*args)
  File "venv/lib/python3.7/site-packages/_pytest/main.py", line 228, in pytest_cmdline_main
    return wrap_session(config, _main)
  File "venv/lib/python3.7/site-packages/_pytest/main.py", line 221, in wrap_session
    session=session, exitstatus=session.exitstatus
  File "venv/lib/python3.7/site-packages/pluggy/hooks.py", line 289, in __call__
    return self._hookexec(self, self.get_hookimpls(), kwargs)
  File "venv/lib/python3.7/site-packages/pluggy/manager.py", line 87, in _hookexec
    return self._inner_hookexec(hook, methods, kwargs)
  File "venv/lib/python3.7/site-packages/pluggy/manager.py", line 81, in <lambda>
    firstresult=hook.spec.opts.get("firstresult") if hook.spec else False,
  File "venv/lib/python3.7/site-packages/pluggy/callers.py", line 203, in _multicall
    gen.send(outcome)
  File "venv/lib/python3.7/site-packages/_pytest/terminal.py", line 650, in pytest_sessionfinish
    outcome.get_result()
  File "venv/lib/python3.7/site-packages/pluggy/callers.py", line 80, in get_result
    raise ex[1].with_traceback(ex[2])
  File "venv/lib/python3.7/site-packages/pluggy/callers.py", line 187, in _multicall
    res = hook_impl.function(*args)
  File "venv/lib/python3.7/site-packages/_pytest/runner.py", line 75, in pytest_sessionfinish
    session._setupstate.teardown_all()
  File "venv/lib/python3.7/site-packages/_pytest/runner.py", line 326, in teardown_all
    self._pop_and_teardown()
  File "venv/lib/python3.7/site-packages/_pytest/runner.py", line 299, in _pop_and_teardown
    self._teardown_with_finalization(colitem)
  File "venv/lib/python3.7/site-packages/_pytest/runner.py", line 319, in _teardown_with_finalization
    self._callfinalizers(colitem)
  File "venv/lib/python3.7/site-packages/_pytest/runner.py", line 316, in _callfinalizers
    raise val.with_traceback(tb)
  File "venv/lib/python3.7/site-packages/_pytest/runner.py", line 307, in _callfinalizers
    fin()
  File "venv/lib/python3.7/site-packages/_pytest/fixtures.py", line 867, in finish
    hook.pytest_fixture_post_finalizer(fixturedef=self, request=request)
  File "venv/lib/python3.7/site-packages/pluggy/hooks.py", line 289, in __call__
    return self._hookexec(self, self.get_hookimpls(), kwargs)
  File "venv/lib/python3.7/site-packages/pluggy/manager.py", line 87, in _hookexec
    return self._inner_hookexec(hook, methods, kwargs)
  File "venv/lib/python3.7/site-packages/pluggy/manager.py", line 81, in <lambda>
    firstresult=hook.spec.opts.get("firstresult") if hook.spec else False,
  File "venv/lib/python3.7/site-packages/pluggy/callers.py", line 208, in _multicall
    return outcome.get_result()
  File "venv/lib/python3.7/site-packages/pluggy/callers.py", line 80, in get_result
    raise ex[1].with_traceback(ex[2])
  File "venv/lib/python3.7/site-packages/pluggy/callers.py", line 187, in _multicall
    res = hook_impl.function(*args)
  File "venv/lib/python3.7/site-packages/_pytest/setuponly.py", line 44, in pytest_fixture_post_finalizer
    _show_fixture_action(fixturedef, "TEARDOWN")
  File "venv/lib/python3.7/site-packages/_pytest/setuponly.py", line 54, in _show_fixture_action
    out, err = capman.read_global_capture()
  File "venv/lib/python3.7/site-packages/_pytest/capture.py", line 141, in read_global_capture
    return self._global_capturing.readouterr()
AttributeError: 'NoneType' object has no attribute 'readouterr'

I'm not sure how the capture manager gets into this state where _global_capturing is None, one easy fix would be to make read_global_capture check for None so it can always succeed. Fixing the underlying reason it gets into that state would of course be better.

@iwanb
Copy link
Contributor Author

@iwanb iwanb commented Oct 8, 2019

Can I implement a quick fix by checking for None in read_global_capture? Or should I figure out the reason it's None in the first place?

@blueyed
Copy link
Contributor

@blueyed blueyed commented Oct 8, 2019

Better to figure out the reason.

@yoavcaspi
Copy link
Contributor

@yoavcaspi yoavcaspi commented Oct 14, 2019

Tried to understand the reason global_capturing is None.
stop_global_capturing called twice:

  1. because this callback for KeyboardIntterupt https://github.com/pytest-dev/pytest/blob/master/src/_pytest/capture.py#L233
  2. because the add_cleanup here: https://github.com/pytest-dev/pytest/blob/master/src/_pytest/capture.py#L50

My recommendation is to add a method to CaptureManager
named is_global_capturing_running()
which will look something like this:

    def is_globally_capturing_running(self):
        return self._global_capturing is not None

and then use this method here:
https://github.com/pytest-dev/pytest/blob/master/src/_pytest/setuponly.py#L52
and here:
https://github.com/pytest-dev/pytest/blob/master/src/_pytest/setuponly.py#L75
as far as I understand the check there is in the case we disable CaptureManager so my recommendation is to add this check:

    if capman and capman.is_globally_capturing_running():
        capman.suspend_global_capture()
        out, err = capman.read_global_capture()

@asottile
Copy link
Member

@asottile asottile commented Oct 14, 2019

sg2m -- let's put that into a PR with a test and see how it goes!

@blueyed
Copy link
Contributor

@blueyed blueyed commented Oct 14, 2019

Created #5960 while looking at it.
It's not clear why it needs to be done in the first place, and none of the fast tests failed without it.

btw: for code references it is better to use the stable links (you can press "y" in GitHub's web UI to get those), since master changes in the future.

blueyed added a commit to blueyed/pytest that referenced this issue Oct 15, 2019
This was added in pytest-dev@1a5e530.
The test from there provides the same output with this fix, which makes
sense, since it suspends capturing, reads the output and prints it after
resuming capturing again.  Therefore it can just leave it in the capture
buffer in the first place.

Fixes pytest-dev#5906
@blueyed
Copy link
Contributor

@blueyed blueyed commented Oct 15, 2019

@yoavcaspi
Please see #5960 for a fix.
From what I've understood @asottile would like to help you with getting it ready.

@yoavcaspi
Copy link
Contributor

@yoavcaspi yoavcaspi commented Oct 15, 2019

Hi @blueyed,
I'll work with @asottile on it.

btw, didn't understand your explanation about copying the stable links, where I should click "y"?

@asottile
Copy link
Member

@asottile asottile commented Oct 15, 2019

if you press the [y] key on your keyboard it makes a permanent link to that code page :)

bors bot added a commit to duckinator/bork that referenced this issue Oct 25, 2019
68: Update pytest to 5.2.2 r=duckinator a=pyup-bot


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



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

Bug Fixes
---------

- `5206 &lt;https://github.com/pytest-dev/pytest/issues/5206&gt;`_: Fix ``--nf`` to not forget about known nodeids with partial test selection.


- `5906 &lt;https://github.com/pytest-dev/pytest/issues/5906&gt;`_: Fix crash with ``KeyboardInterrupt`` during ``--setup-show``.


- `5946 &lt;https://github.com/pytest-dev/pytest/issues/5946&gt;`_: Fixed issue when parametrizing fixtures with numpy arrays (and possibly other sequence-like types).


- `6044 &lt;https://github.com/pytest-dev/pytest/issues/6044&gt;`_: Properly ignore ``FileNotFoundError`` exceptions when trying to remove old temporary directories,
  for instance when multiple processes try to remove the same directory (common with ``pytest-xdist``
  for example).
   ```
   
  
</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>
bors bot added a commit to rehandalal/therapist that referenced this issue Oct 29, 2019
98: Update pytest to 5.2.2 r=rehandalal a=pyup-bot


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



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

Bug Fixes
---------

- `5206 &lt;https://github.com/pytest-dev/pytest/issues/5206&gt;`_: Fix ``--nf`` to not forget about known nodeids with partial test selection.


- `5906 &lt;https://github.com/pytest-dev/pytest/issues/5906&gt;`_: Fix crash with ``KeyboardInterrupt`` during ``--setup-show``.


- `5946 &lt;https://github.com/pytest-dev/pytest/issues/5946&gt;`_: Fixed issue when parametrizing fixtures with numpy arrays (and possibly other sequence-like types).


- `6044 &lt;https://github.com/pytest-dev/pytest/issues/6044&gt;`_: Properly ignore ``FileNotFoundError`` exceptions when trying to remove old temporary directories,
  for instance when multiple processes try to remove the same directory (common with ``pytest-xdist``
  for example).
   ```
   
  
</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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

5 participants