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

setuponly: remove printing out/err from capman #5960

Closed

Conversation

blueyed
Copy link
Contributor

@blueyed blueyed commented Oct 14, 2019

This was added in 1a5e530b9.
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 #5906

/cc @kvas-it

This was added in pytest-dev@1a5e530b9.
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 blueyed force-pushed the setuponly-no-read_global_capture branch from ed4a11d to fb44ee4 Compare October 15, 2019 00:05
@blueyed blueyed changed the title [WIP/RFC] setuponly: remove printing out/err from capman setuponly: remove printing out/err from capman Oct 15, 2019
@blueyed blueyed added the type: bug problem that needs to be addressed label Oct 15, 2019
@asottile
Copy link
Member

:S I was helping @yoavcaspi make their first contribution

@blueyed
Copy link
Contributor Author

blueyed commented Oct 15, 2019

@asottile
#5906 (comment) ?

That was the wrong direction though.

I've just created a PR already while looking into it.

Can you approve it then maybe?

@asottile
Copy link
Member

next time help them in the right direction then! it's valuable to get people excited about contributing

I don't think this is mergeable yet, there's no test for the bug or change

@blueyed
Copy link
Contributor Author

blueyed commented Oct 15, 2019

next time help them in the right direction then! it's valuable to get people excited about contributing

Sure, but should I have then just posted the diff instead to the issue?
From my experience things here often get stalled, and then it's better to have a fix for things right away, isn't it?

I don't think this is mergeable yet, there's no test for the bug or change

Ok, you can work with @yoavcaspi on it then maybe?
Closing this here for now.

@blueyed blueyed closed this Oct 15, 2019
@blueyed blueyed deleted the setuponly-no-read_global_capture branch October 15, 2019 17:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug problem that needs to be addressed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Crash when using --setup-show and KeyboardInterrupt
2 participants