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

fixes #1210 adds stderr write for pytest.exit(msg) call #1655

Closed
wants to merge 1 commit into from

Conversation

doomb0t
Copy link

@doomb0t doomb0t commented Jun 24, 2016

This PR addresses issue #1210 writing the exception message to stderr as well as the class name of the exception.

@The-Compiler
Copy link
Member

Thanks! Could you please take a look at the failing test? I also think this should go to the feature branch instead of master.

excinfo = _pytest._code.ExceptionInfo()
config.hook.pytest_keyboard_interrupt(excinfo=excinfo)
session.exitstatus = EXIT_INTERRUPTED
except pytest.Exit as e:
sys.stderr.write('{0}: {1}\n'.format(type(e).__name__, e.msg))
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the session.exitstatus when pytest.Exit is caught? It looks like it might be 0 and before this change it was EXIT_INTERNALERROR. But I might be wrong... maybe check it in the test?

Copy link
Author

Choose a reason for hiding this comment

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

Good point, I will have the test verify this.

@nicoddemus
Copy link
Member

nicoddemus commented Jun 26, 2016

Thanks @JonathonSonesen! 😁

Closing this for now, as it should target features. Could you please rebase and open a new PR? (GH does not let you change the target branch of a PR).

@nicoddemus nicoddemus closed this Jun 26, 2016
@doomb0t
Copy link
Author

doomb0t commented Jun 27, 2016

Yeah no problem i will get this fixed up!

@@ -90,10 +90,12 @@ def wrap_session(config, doit):
session.exitstatus = doit(config, session) or 0
except pytest.UsageError:
raise
except KeyboardInterrupt:
except KeyboardInterrupt as e:
excinfo = _pytest._code.ExceptionInfo()
Copy link
Contributor

@tomviner tomviner Jun 27, 2016

Choose a reason for hiding this comment

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

Few points:

  • there's no need for as e because the exception data is already in excinfo
  • I'm unsure if this is the right place to do the outputting
  • I tested pytest.exiting from a test, and it now prints the msg twice - I think investigating this and why it doesn't print when pytest.exiting from pytest_configure will point to the right place to do the outputting
  • also when the tests ran, I saw quite a few errors - these will also help point to a good solution

Copy link
Author

Choose a reason for hiding this comment

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

Yes thanks for bringing this up again, I think it may have to do with the init_state logic that we were talking about prior to your departure.

I will do more work on this for sure.

Copy link
Contributor

Choose a reason for hiding this comment

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

@JonathonSonesen cool! Ping me a message if you wanna discuss it.

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.

5 participants