pytest --pdb is no longer working with Flask_testing

[tricosmo]

Hi All

Recently we have problem to run "pytest --pdb" with our flask testcases. The testcases were fine until this change(4eeb475). The case is when we add --pdb option all the case failed with the same reason. "self.app is None", but if no "--pdb" then all tests pass.

By debugging, we found that this pre_setup function
https://github.com/jarus/flask-testing/blob/master/flask_testing/utils.py#L102 is called to setup _self.app.
But since the change, it calls the debug function instead of the Testcase which will skip the whole pre_setup function.

I tried to customise our own debug function to call pre_setup, but failed with further issues(request mocker). So I am wondering if I am doing right, or this is an issue I can address with flask_testing.

For now we just use previous version(3.0.1) of pytest, but we like to be as close as possible to the latest. Any advise will be appreciated. Thanks

Niansheng

[RonnyPfannschmidt]

@mbyt do you have an idea whats missing to have that detail work?

[mbyt]

Hi @niansheng,

Is there a reason why unittests setUpClass and tearDownClass is not used? Note that it is supported also in python2.7. For me it looks like overwriting __call__ achieves something similar.

Basically I would not overwrite __call__ but instead restructure the code as follows:

@classmethod
def setUpClass(cls):
    cls._pre_setup()

@classmethod
def tearDownClass(cls):
    cls._post_teardown()

This is more pythonic / unittest standard. An alternative is to just directly use the pytest machinery. If you stay with unittest, an even more fine grained and better solution might be to use addCleanup in setUpClass which replaces the tearDownClass code.

But probably I misunderstood the flask-testing code.

By the way, using debug is actually documented, I did not recognize that before.

[mbyt]

Sorry, for the noise --- my previous comment is wrong. OK __call__ serves as an addition to setUp and tearDown.

If you overwrite __call__, can you overwrite debug similarly (I did not find a debug in flask-testing/flask_testing/utils:TestCase)?:

    def debug(self):
        self._pre_setup()
        super(TestCase, self).debug()
        self._post_teardown()

Note: In order to enable debugging, I did not put that in a try and except here.

[tricosmo]

Hi @mbyt

Thanks for you reply and the suggestion. We tried this work around, it works fine with it own part. but we have a request mocker so unittests are running in the request mocker context.

class RequestsMocker(TestCase):
    """Provides self.request mock which enables to register URL for request
    library.

    >>> def setUpClass(cls):
    ...     super().setUpClass()
    ...     cls.requests_mocker.register_uri(
                'POST', 'http://localhost:8000/resources',
    ...         json={'id': 'de305d54-75b4-431b-adb2-eb6b9e546014'}
    ...     )

    Highly recommend to use this class in all unit test classes, because it
    intercepts http requests made through requests library and prevents test
    dependencies on external components.
    """

    def run(self, result=None):
        self.requests_mocker = requests_mocker.Mocker()
        with self.requests_mocker:
            return super().run(result=result)

+    def debug(self):
+        self.requests_mocker = requests_mocker.Mocker()
+        with self.requests_mocker:
+            return super().debug()

By overwriting debug, it works. And this workaround is not too bad.
May I ask why the change needed in the first place?

Thanks
Niansheng

[mbyt]

Good to hear that you found a solution! Regarding the motivation for that change:

When debugging standard unittests with pytest --pdb, unittests tearDown and tearDownClass are called before the the post mortem debugger is invoked. These methods might actually change the state of your system (modify or remove variables). This basically means that you are debugging not at the state where the exception occurred. See also #1890 for more info.

[nicoddemus]

Perhaps we should only stop calling tearDown and tearDownClass when an exception actually happens? Currently we only check for --pdb, which means all tests stop getting tearDown and tearDownClass called.

I thought the use case for --pdb was for a single test, not an entire test suite.

[nicoddemus]

I did some debugging and I think that self.debug in pytest is the right thing to do, but mostly by accident it seems.

When calling self.debug instead of self.testcase(result), what happens is that UnitTest doesn't call any internal functions but calls setUp/method/tearDown directly.

From unittest.py:

    def debug(self):
        """Run the test without collecting errors in a TestResult"""
        self.setUp()
        getattr(self, self._testMethodName)()
        self.tearDown()
        while self._cleanups:
            function, args, kwargs = self._cleanups.pop(-1)
            function(*args, **kwargs)

So if getattr(self, self._testMethodName)() raises, it will propagate to the caller, which happens to be item.runtest(). This bypasses unittest's usual run code path which handles skips/unexpectedsuccess handling, adds errors and failures to the result object and never raises.

But bypassing __call__ and run() will potentially break frameworks which override __call__ and run() to do some special handling, but AFAIK that's what debug() is supposed to do.

All in all I'm leaning towards that this is correct behavior, but coming to the conclusion that this shouldn't have gone to master but features instead. If others agree, I think it is fine to pull it back from 3.0.3 and add it to 3.1.0.

[nicoddemus]

All in all I'm leaning towards that this is correct behavior, but coming to the conclusion that this shouldn't have gone to master but features instead. If others agree, I think it is fine to pull it back from 3.0.3 and add it to 3.1.0.

Furthermore we could even add a config.ini option to disable calling .debug() (all the more reason for this to go into features).

[tricosmo]

@mbyt Thanks for the link. Actually I found the simply post morterm debugging issue too. The problem was the db had been dropped instead of GUI when exception happen. Thanks again for that PR, it could also address this problem. I used to add "ipdb" in the code before the exception raises as a workaround.

@nicoddemus Thanks for you comments. But I am not sure what do you mean by "I think it is fine to pull it back from 3.0.3 and add it to 3.1.0."

I usually add --pdb option as default, not just a test method. But for our case it is the same no matter I run a single test method or a test suite.

If the behavior is right, the overwriting the __call__ or run function is not a good practise. Actually I quite agree debug() should skip tearDown and tearDownClass, so any suggestion for framework like flask_testing doing _pre_setup in __call__.

thanks
Niansheng

[mbyt]

Hi @niansheng,

Thanks again for that PR, it could also address this problem. I used to add "ipdb" in the code before the exception raises as a workaround.

Exactly, with #1890 this extra step of putting a "ipdb" before the exception should now be vanished; or in other words, post mortem debugging should work out of the box also in complex scenarios (where different kind of tearDowns act).

so any suggestion for framework like flask_testing doing pre_setup in __call__?

My conclusion is: If people overwrite unittest.TestCase __call__ or run and still want the post-mortem debugging feature, they need to overwrite debug in the same way (this is also true for standard unittest).

[nicoddemus]

Thanks for you comments. But I am not sure what do you mean by "I think it is fine to pull it back from 3.0.3 and add it to 3.1.0."

I meant reverting that change for 3.0.3 release and reintroduce it in 3.1.0, perhaps with some more documentation.

@mbyt perhaps we should add a note to the docs explaining your conclusion?

[tricosmo]

thanks guys. I think this is a good way and right direction. I am sure Flask_testing framework can does something to follow to get that nice post-mortem debugging future.
Cheers