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

calling/raises/pattern doesn't allow us to match multiple messages with nested exceptions #126

Open
thehcma opened this issue Nov 26, 2019 · 5 comments

Comments

@thehcma
Copy link

thehcma commented Nov 26, 2019

This is more of a feature request rather than a bug.

With Python 3, we can have nested exceptions (e.g., raise RuntimeError("blah") from e, where e is another exception that was just caught).

The full exception message can be retrieve, e.g., using:

def stack_trace_as_string(exception):
      """Returns the full stack trace as a string"""
      return "".join(traceback.format_exception(etype=type(exception), value=exception, tb=exception.__traceback__))

This will then yield something like (stealing the example from here: https://stackoverflow.com/questions/16414744/python-exception-chaining):

Traceback (most recent call last):
  File "t.py", line 2, in <module>
    v = {}['a']
KeyError: 'a'

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "t.py", line 4, in <module>
    raise ValueError('failed') from e
ValueError: failed

As you can see we have 2 messages (from the two exceptions that were raised in a nested fashion).

So using the function I gave above, pyhamcrest could potentially be extend to match/search for both messages.

@brunns
Copy link
Member

brunns commented Nov 27, 2019

Seems reasonable. What do you think an assertion for a nested exception might look like?

@thehcma
Copy link
Author

thehcma commented Nov 27, 2019

My inclination would be to have something like the following (assuming we want to keep close in spirit to what we already have):

assert_that(calling(foo).with_args(bar1, bar2...), raises([exception1, exception2, ...], patterns=[exception1_message, exception2_message]))

The semantics would be that the expected nesting (exception_type, exception_message) would be captured by the two parallel lists - in other words, the nesting stack trace should match these lists in the same order.

Alternatively, something like this would also work (it's a bit of departure to the current syntax, but, imo, a bit safer, since we don't have to keep parallel lists):

assert_that(calling(foo).with_args(bar1, bar2...), raises([(exception1, optional_exception1_message_pattern), (exception2, _optional_exception2_message_pattern]))

Naturally, we could have variations in the sense that we would not need to specify the whole trace - something along the lines of specifying an incomplete stack trace - that is, we could specify the initial frames (e.g., say we have a 5-level nested trace, we could specify it starts with these 2 exceptions and their patterns), the final frames (e.g., say that we expect these last 2 frames), and containment frames (e.g., the following sequence should be present anywhere in the nested frames). This is all extra-credit, but the first proposal, where we specify the whole expected set of frames would already go a long way.

@bittrance
Copy link
Contributor

PR #129 is now merged. Is it possible to use that style to match the inner exception that way?

@thehcma
Copy link
Author

thehcma commented Jan 8, 2020

I believe it could be used.

However, imo, it's suboptimal in the general case, because it doesn't really provide a way to make the nesting/structure associated with a chain of exceptions explicit.

In other words, the nesting includes several exception instances with their associated properties, so one would have to craft a regex to match that instead of simply enumerating what's being sought. In this case, this is akin to comparing two collections directly vs expanding a collection and using a regex to assess their equality. Yes, it works, but it ain't ideal (imho).

@thehcma
Copy link
Author

thehcma commented Jan 8, 2020

Let me clarify a bit and add a disclaimer - i.e., that I fully understood the #129 PR fix correctly.

First, I am changing my opinion, I don't think this change can be used for the purposes of this feature request. :)

While it seems possible that with this change we would be able to inspect the attributes of the outer exception (and, hence, the __cause__ attribute), I can't figure out a way to traverse the causal hierarchy (i.e., assume it's deeper than a single level). Perhaps, there is a way, but I really can't see it.

The change we need in the original feature request is something that would traverse the __cause__ attribute hierarchy in the exception instance and, hence, allow us to assert both the exception type as well as its properties, at each level.

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

No branches or pull requests

3 participants