Skip to content

[WIP] Suspend capturing when stdin is being read (optionally) #4996

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

Closed

Conversation

blueyed
Copy link
Contributor

@blueyed blueyed commented Mar 24, 2019

No description provided.

@blueyed blueyed requested review from Sup3rGeo and asottile April 3, 2019 21:53
@asottile
Copy link
Member

asottile commented Apr 3, 2019

I'd rather raise a helpful error message telling the user to use -s instead of magically suspending capture.

The capture code is already some of the more complex parts of pytest and adding more edges seems the wrong direction there

@blueyed
Copy link
Contributor Author

blueyed commented Apr 11, 2019

I'd rather raise a helpful error message telling the user to use -s instead of magically suspending capture.

Good point about a more helpful error message (I've created #5086 for it).

(You should see the code that I have to make this work with runpytest_subprocess to make it a real TTY (using threads to display and capture/forward streams).. ;))

But seriously, passing -s does not wotk with testdir.runpytest (both methods (inline and subprocess)).
What do you suggest here?
I usually end up re-running the tests from the created tempdir directly, but that is rather tedious.

As for runpytest_inprocess, it could maybe just not capture anything if -s is in args (or better have a kwarg for this). This way you could use capture=False to not use capturing for this test, but it would make the following assertions against stdout/stderr fail then - for this to work they could be duplicated maybe (or handled via threads, to be cross platform).

By now it works better to pass through stdin, which could serve as an indicator, but would still need to not capture (or duplicate) stdout/stderr then.

The capture code is already some of the more complex parts of pytest and adding more edges seems the wrong direction there

Agreed in general, but not being able to easily debug pytest's own tests does not help to improve this situation.
And most of this patch is to keep the current behavior - it would be easier if only the new behavior would have been supported in the first place, with maybe an option then to close it explicitly.

(just some quick thoughts - happy to iterate from here)

@asottile
Copy link
Member

But seriously, passing -s does not wotk with testdir.runpytest (both methods (inline and subprocess)).
What do you suggest here?
I usually end up re-running the tests from the created tempdir directly, but that is rather tedious.

Maybe borrow from the subprocess py3 interface and have an input=b'...'?

pytest is currently lacking a good "capsys but for stdin", which (at least in my code) has led to some wacky hacks to patch out input / sys.stdin.read / etc. -- maybe the answer is to expose something like that, maybe even added into the current capturing system

(spitballing)

def test_stdin(capsys):
    capsys.feed_input(b'foo\n')
    assert input() == 'foo'


def test_stdin_not_fed(capsys):
    with pytest.raises(...) as excinfo:
        assert input() == 'foo'
    msg, = excinfo.value.args
    assert msg == 'idk some useful message about input but no provided input'

@nicoddemus
Copy link
Member

Hi @blueyed,

This has been on WIP for a long time now, and has some conflicts. I'm closing this for now to clear up our PR queue a bit, but please do re-open this when you have the time.

Cheers!

@nicoddemus nicoddemus closed this Jun 26, 2019
@blueyed blueyed deleted the capture-resume-on-stdin-read branch September 29, 2019 03:20
@blueyed blueyed restored the capture-resume-on-stdin-read branch September 29, 2019 03:20
@pytest-dev pytest-dev deleted a comment from codecov bot Oct 21, 2019
blueyed added a commit to blueyed/pytest that referenced this pull request Oct 21, 2019
@blueyed blueyed deleted the capture-resume-on-stdin-read branch October 21, 2019 02:45
@blueyed
Copy link
Contributor Author

blueyed commented Oct 21, 2019

@nicoddemus
btw: if you do another round of closing stale PRs it would be helpful to attach some label - so e.g. it is easier to find them again.

@nicoddemus
Copy link
Member

Good idea, done. 👍

@nicoddemus nicoddemus added the PR closed due to inactivity on occasion we do rounds to close up inactive PRs, this label makes it easier to track them label Oct 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR closed due to inactivity on occasion we do rounds to close up inactive PRs, this label makes it easier to track them
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants