findFailure fails to find the correct failure in a more complicated case

[Peaker]

The problem is that findFailure uses the fact that the caller of the frame in which except: was used is "throwExceptionIntoGenerator" but that doesn't necessarily mean that we are actually catching *that*. We may be catching something that was re-raised from a catcher of that exception.

I think a solution would be to verify that it was indeed a yield that raised it, and not only that throwExceptionIntoGenerator is the caller frame.

I attached a patch with an extra test in test_failure's generator_tests to demonstrate the problem.

[Peaker]

A test to demonstrate a problem with findFailure

[Peaker]

I created a branch findfailure-2803 for this ticket, so ignore the patch file.

[peaker]

(In [21923]) Naming conventions, spacing conventions, and a docstring. refs #2803

[Peaker]

Hairy situation requires hairy fixes!

I think its ready for review...

[therve]

• _yieldOpcode is unnecessary, opcode.opmap["YIELD_VALUE"] exists in python 2.3. I suggest making the value a private class variable of Failure (but with documentation).
• several whitespaces missing around = operator in tests
• instead of self.assertEqual(Failure().type, IndexError), I prefer self.assertIsInstance(Failure().value, IndexError)
• in the docstrings of test_ambiguousFailureInGenerator and test_ambiguousFailureFromGenerator, please use L{Failure._findFailure} instead of findFailure
• trailing whitespace in failure.py

Thanks!

[Peaker]

Replying to therve:

Thanks for the review.

* _yieldOpcode is unnecessary, opcode.opmap["YIELD_VALUE"] exists in python 2.3. I suggest making the value a private class variable of Failure (but with documentation).

Ah, didn't know that. Done.

* several whitespaces missing around = operator in tests

Fixed.

* instead of self.assertEqual(Failure().type, IndexError), I prefer self.assertIsInstance(Failure().value, IndexError)

Done.

* in the docstrings of test_ambiguousFailureInGenerator and test_ambiguousFailureFromGenerator, please use L{Failure._findFailure} instead of findFailure

Done.

* trailing whitespace in failure.py

That wasn't mine, but fixed anyhow.

[therve]

OK please merge.

[therve]

(In [21945]) Merge findfailure-2803

Author: peaker Reviewer: therve Fixes #2803

Fix a corner case in Failure._findFailure where throwExceptionIntoGenerator is in the caller frame but not the real exception.