pytest-assume icon indicating copy to clipboard operation
pytest-assume copied to clipboard

Unhandled exception when running from master

Open mbednarski opened this issue 5 years ago • 8 comments

Hello

//For first, thanks for this piece of software - it's really useful for me

I have issue with up-to-date master branch. When I run my tests I receive following error:

pyfuncitem = <Function test_parse_header_assume[data/raw/AuditTrail__20140226_103919.PDF-expected6]>

    @pytest.hookimpl(hookwrapper=True)
    def pytest_pyfunc_call(pyfuncitem):
        """
        Using pyfunc_call to be as 'close' to the actual call of the test as possible.
    
        This is executed immediately after the test itself is called.
    
        Note: I'm not happy with exception handling in here.
        """
        __tracebackhide__ = True
        outcome = None
        try:
            outcome = yield
        finally:
            failed_assumptions = _FAILED_ASSUMPTIONS
            assumption_locals = _ASSUMPTION_LOCALS
            if failed_assumptions:
                failed_count = len(failed_assumptions)
                root_msg = "\n%s Failed Assumptions:\n" % failed_count
                if assumption_locals:
                    assume_data = zip(failed_assumptions, assumption_locals)
                    longrepr = ["{0}\nLocals:\n{1}\n\n".format(assumption, "\n".join(flocals))
                                for assumption, flocals in assume_data]
                else:
                    longrepr = ["\n\n".join(failed_assumptions)]
    
                del _FAILED_ASSUMPTIONS[:]
                del _ASSUMPTION_LOCALS[:]
                if outcome and outcome.excinfo:
                    root_msg = "\nOriginal Failure: \n>> %s\n" % repr(outcome.excinfo[1]) + root_msg
                    raise FailedAssumption(root_msg + "".join(longrepr)).with_traceback(outcome.excinfo[2])
                else:
>                   raise FailedAssumption(root_msg + "".join(longrepr))
E                   pytest_assume.plugin.FailedAssumption: 
E                   1 Failed Assumptions:
E                   tests/test_parse_header.py:119: AssumptionFailure
E                   >>	pytest.assume(parsed.rezept == expected.rezept)

../../tmp/pytest-assume/pytest_assume/plugin.py:92: FailedAssumption

My test fails as expected, however not gracefully. For tag 1.2.2 it works as expected:

tests/test_parse_header.py:119: AssumptionFailure
	pytest.assume(parsed.rezept == expected.rezept)

------------------------------------------------------------
Failed Assumptions: 1

As I was able to investigate, the issue was introduced in 649227cbe93a07a7b95e1dec7c5b49a6e476cf5d

mbednarski avatar Jul 11 '19 09:07 mbednarski

Hi @mbednarski,

This was a part of a change to make pytest-assume play nicer with other plugins. I'll see what I can do about it, because it really isn't pretty (works as expected if there's a failing assert & a failed assumption; but just a failed assumption is ugly).

astraw38 avatar Jul 11 '19 12:07 astraw38

As an update, I pushed a change to the 'traceback' branch that changes the output:

__________________________________ test_func __________________________________

tp = <class 'pytest_assume.plugin.FailedAssumption'>, value = None, tb = None

    def reraise(tp, value, tb=None):
        try:
            if value is None:
                value = tp()
            if value.__traceback__ is not tb:
>               raise value.with_traceback(tb)
E               pytest_assume.plugin.FailedAssumption: 
E               1 Failed Assumptions:
E               
E               test_failing_expect.py:3: AssumptionFailure
E               >>	pytest.assume(1 == 2)

C:\Users\astraw\Projects\pytest-assume\venv\lib\site-packages\six.py:692: FailedAssumption
========================== 1 failed in 0.08 seconds ===========================

It's not ideal to me, because the old method has much better details about where the assumption failure took place, but having the hook be at pytest_func_call severely limits the massaging of the output that I can do. I do think that I can improve it by including more context, but this is a first pass.

astraw38 avatar Jul 11 '19 13:07 astraw38

Thank you for such quick reaction. I think, that new version will work for me.

mbednarski avatar Jul 15 '19 11:07 mbednarski

@astraw38 maybe you can help me figuring out why is it that when we use the context manager version, the exception message points to the test code (as we would like to), whereas when using the assume() call directly it points to six.reraise?

I mean, both approaches in the end go through the same pytest_func_call code and end up with the _reraise function. I don't understand then why they behave differently.

test:

from pytest import assume

a = 1
b = 2

def test_1():
    with assume: assert a == b, "My bad"

def test_2():
    assume(a == b, "My bad")

output:

============================= test session starts =============================
platform win32 -- Python 3.7.3, pytest-5.0.1, py-1.8.0, pluggy-0.12.0
rootdir: D:\WORK\allure-pytest-check-bug
plugins: allure-pytest-2.7.0, assume-2.2.0
collected 2 items

test_check.py FF                                                         [100%]

================================== FAILURES ===================================
___________________________________ test_1 ____________________________________

    def test_1():
>       with assume: assert a == b, "My bad"
E       pytest_assume.plugin.FailedAssumption: 
E       1 Failed Assumptions:
E       
E       test_check.py:9: AssumptionFailure
E       >>	with assume: assert a == b, "My bad"
E       AssertionError: My bad
E       assert 1 == 2

test_check.py:9: FailedAssumption
___________________________________ test_2 ____________________________________

tp = <class 'pytest_assume.plugin.FailedAssumption'>, value = None, tb = None

    def reraise(tp, value, tb=None):
        try:
            if value is None:
                value = tp()
            if value.__traceback__ is not tb:
>               raise value.with_traceback(tb)
E               pytest_assume.plugin.FailedAssumption: 
E               1 Failed Assumptions:
E               
E               test_check.py:12: AssumptionFailure
E               >>	assume(a == b, "My bad")
E               AssertionError: My bad
E               assert False

venv\lib\site-packages\six.py:692: FailedAssumption
========================== 2 failed in 0.12 seconds ===========================

Sup3rGeo avatar Jul 31 '19 10:07 Sup3rGeo

I'm thinking it might be because six doesn't have __tracebackhide__ in it, or the implementation of the reraise function itself. Vendoring that function (& adding __tracebackhide__ & raise exc.with_traceback()) shows the following results instead:

__________________________________ test_func __________________________________

self = <pytest_assume.plugin.AssumeContextManager object at 0x000000000375E438>
expr = False, msg = None

    def __call__(self, expr, msg=""):
        __tracebackhide__ = True
        self._enter_from_call = True
        with self:
            if msg:
                assert expr, msg
            else:
>               assert expr
E               pytest_assume.plugin.FailedAssumption: 
E               1 Failed Assumptions:
E               
E               test_failing_expect.py:3: AssumptionFailure
E               >>	pytest.assume(1==2, None)
E               AssertionError: assert False

C:\Users\astraw\Projects\pytest-assume\pytest_assume\plugin.py:151: FailedAssumption

astraw38 avatar Jul 31 '19 12:07 astraw38

But even with reraise the way it is, why when using the context manager the traceback does not point to reraise?

Sup3rGeo avatar Jul 31 '19 13:07 Sup3rGeo

I believe it's due to pytest's assertion rewriting & exception handling.. When we have a simple test:

def test_one():
    with pytest.assume:
         assert 1 == 2

We get pytest's assertion raised at the end of the test.

A different test:

def test_two():
     with pytest.assume:
           assert 1 == 2
     pytest.assume(1 == 2)

Gets the reraise traceback.

This is relatively easily validated by re-running the 2nd test there swapping the 'last_tb' to the first traceback instead (which changes test_two from showing the reraise to showing pytest's first assertion error).

astraw38 avatar Jul 31 '19 13:07 astraw38

I think the solution is still leveraging pytest's AST assertion rewriting to also rewrite pytest.assume.

Unfortunately, my situation hasn't changed in that I can't devote the time necessary right now to do that, at least not immediately. I might plink away at it though.

astraw38 avatar Jul 31 '19 13:07 astraw38