#4068 Rework mocking of QueryProcessor in DBQueryTestCase
Merged by tkopecek. Opened by mikem.
mikem/koji fix-qp-mock  into  master

Download 4068.patch

The previous only worked for code from kojihub.py. Any hub module that imported QueryProcessor differently would not see the mock. This approach mocks the class in place so that all imports will see it.

While here, I noticed this bit of test code:

    def setUp(self):
        mock.patch.stopall()

It makes no sense to stop mock patches in setUp. At this point, there should be no active patches unless other tests are misbehaving. As it turns out, a great many of our tests were failing to properly clean up. Removing the above stopall broke several tests, which was probably what prompted #2758.

Having stray mocks lingering after a unit test is completed could potentially mask a later unit test, or as seen here break one in a hard to debug way.

Fixes: https://pagure.io/koji/issue/4071
Related: https://pagure.io/koji/issue/2758

:thumbsup:
Maybe changing test_cli.utils.CliTestCase.tearDown and test_hub.utils.DBQueryTestCase.tearDown to do this by default?

Metadata Update from @tkopecek:
- Pull-request tagged with: no_qe

Maybe changing test_cli.utils.CliTestCase.tearDown and test_hub.utils.DBQueryTestCase.tearDown to do this by default?

DBQueryTestCase.tearDown does mock.patch.stopall(). Part of the issue here is that other test cases that did not use this class did not.

CliTestCase doesn't have a setUp or tearDown method and uses decorators or context managers for all its mocking. I suppose we could add a tearDown anyway, but that feels like masking a problem.

I looked, but didn't see a way to declare any sort of global setup or teardown in the unit test framework

You're right though that there could be cli test cases that don't properly clean up. I only checked for / fixed the hub ones.

Commit f5c9ca95 fixes this pull-request

Pull-Request has been merged by tkopecek

Metadata