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

Feature request: Add original exception object to pytest_exception_interact #3626

Open
liiight opened this issue Jun 27, 2018 · 7 comments
Open
Labels
type: enhancement new feature or API change, should be merged into features branch type: feature-branch new feature or API change, should be merged into features branch

Comments

@liiight
Copy link

liiight commented Jun 27, 2018

I'd really like to have a generic exception handler, maybe via a hook. I use a test application and I'd like to raise custom exception and just catch them in one place like so:

def pytest_exception_handler(item, exc):
    if isinstance(exc, KeyError):
        pytest.skip('I knew this was gonna happen!)
    if isinstance(exc, MyException) and 'foo' in item.name:
        raise AnotherGenericException('That foo test is a problem!)

I searched before opening this issue, and couldn't find a way to do this, or even a discussion about this suggestion. If such exist, I apologize.

@pytestbot
Copy link
Contributor

GitMate.io thinks possibly related issues are #56 (Feature: report exceptions in threads as errors), #553 (Handle exceptions in inspect.getsourcelines()), #1165 (Need to specially handle "historic" hooks), #1749 (Feature request: doctest report format as an option), and #3144 (Exit exception).

@pytestbot pytestbot added platform: mac mac platform-specific problem type: enhancement new feature or API change, should be merged into features branch labels Jun 27, 2018
@avadhanij
Copy link

Can this not be achieved using the hook - pytest_exception_interact? I wish the actual exception object was given to this hook though. I would very much like to use isinstance(exc, KeyError) rather than call.excinfo.typename == 'KeyError'

@liiight
Copy link
Author

liiight commented Jun 27, 2018

oh, I apologize, I completely missed that hook. I'll check it out, thanks!

@asottile asottile added type: question general question, might be closed after 2 weeks of inactivity and removed platform: mac mac platform-specific problem type: enhancement new feature or API change, should be merged into features branch labels Jun 27, 2018
@liiight
Copy link
Author

liiight commented Jun 28, 2018

Well, silly me, this does exactly what I wanted, sorry.
However, like stated above, it would be very nice to actually pass the exception object instead of refferencing it via call.excinfo.

You could turn this:

def pytest_exception_interact(node, call, report):
    if call.excinfo.type is HTTPError and call.excinfo.value.response.status_code == 400:
        do_stuff()

To this:

def pytest_exception_interact(node, call, report, exc):
    if isinstace(exc, HTTPError) and exc.response.status_code == 400:
        do_stuff()

Which is arguably a lot nicer and pythonic.

Should I change the issue to reflect that?

On a sort of unrelated note, and I apologize if I'm hijacking (my own) issue, is it possible to skip a test that failed during the setup phase? Using pytest.skip() in the above scenario when the exception occurs in the setup phase results in an INTERNALERROR

@RonnyPfannschmidt
Copy link
Member

@liiight perhaps - the hook could be improved, and we'd like to get rid of callinfo in external apis in future ^^

@liiight liiight changed the title Feature request: A generic exception handling hook Feature request: Add original exception object to pytest_exception_interact Jun 28, 2018
@liiight
Copy link
Author

liiight commented Jun 28, 2018

I updated the title. I'll leave the description as is for prosperity's sake.
I'd like to take a stab into doing this, about time I give something back to this awesome project

@RonnyPfannschmidt RonnyPfannschmidt added type: enhancement new feature or API change, should be merged into features branch type: feature-branch new feature or API change, should be merged into features branch and removed type: question general question, might be closed after 2 weeks of inactivity labels Jun 28, 2018
@liiight
Copy link
Author

liiight commented Jul 5, 2018

Ok, started to take a look into this. As I see it there are at least two approaches to this:

  1. The "ideal" design, which is to change the signature of the hook and remove
def pytest_exception_interact(node, report, exc):
    if isinstace(exc, HTTPError) and exc.response.status_code == 400:
        do_stuff()

The downside here is that it's a breaking change, the upside is that it's the nicest way IMO.

  1. Add the exception to the call object, set a deprecation notice in the docs for callinfo:
def pytest_exception_interact(node, call, report):
    if isinstace(call.exc, HTTPError) and call.exc.response.status_code == 400:
        do_stuff()

I'm probably missing some broader aspect here though, as I'm not exactly sure where else callinfo is used.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement new feature or API change, should be merged into features branch type: feature-branch new feature or API change, should be merged into features branch
Projects
None yet
Development

No branches or pull requests

5 participants