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

Update test_processor.py #283

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Update test_processor.py #283

wants to merge 1 commit into from

Conversation

pvinci
Copy link
Contributor

@pvinci pvinci commented Nov 16, 2019

Workaround for change in behavior in python3.6
See: https://bugs.python.org/issue29130

Workaround for change in behavior in python3.6
See: https://bugs.python.org/issue29130
@coveralls
Copy link

Coverage Status

Coverage remained the same at 68.114% when pulling a47bb37 on pvinci:patch-1 into ff9b688 on mtreinish:master.

Copy link
Owner

@mtreinish mtreinish left a comment

Choose a reason for hiding this comment

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

I'm not sure about this, based on the bug linked in the commit message it looks like starting in python 3.6 python will exit will exit with 120 if there is an error raised during the closing of the interpreter. The example in that bug was that flush() was called on sys.stdout or sys.stderr. This seems like an actual error that we want to report to the user since it's something wrong with the code. Especially if this is happening during import of a module (which is all that happens during discovery).

Was there a use case or issue you were hitting that required making this change? I'm just trying to understand the reason behind why we would want to do this and maybe we can come up with an alternative solution.

@pvinci
Copy link
Contributor Author

pvinci commented Dec 5, 2019 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants