1432456 - Custom Marionette harness skip decorators shouldn't trigger setUp() and tearDown() for skipped tests

Status: NEW

(Testing :: Marionette Client and Harness, defect, P5)

Description

[Henrik Skupin [:whimboo][⌚️UTC+1]]

As noticed while working on bug 1429759, the custom skip decorators in decorators.py [1] do not stop the test runner from executing `setUp()` and `tearDown()`. As result inappropriate code is getting executed, which basically shouldn't be run at all (eg. restarts of Firefox).

Code:

    def setUp(self):
        print "*** running setUp"
        [..]

    def tearDown(self):
        print "** running tearDown"
        [..]

    @skip_if_desktop("don't run!")
    def test(self):
        print "** test"


Output:
> TEST-START | _a/test_minimized.py TestMinimizedTestCase.test
> *** running setUp
> TEST-SKIP | _a/test_minimized.py TestMinimizedTestCase.test | took 136ms
> ** running tearDown

In contrast the `skip` decorator works just fine.

Output:
> TEST-START | _a/test_minimized.py TestMinimizedTestCase.test
> TEST-SKIP | _a/test_minimized.py TestMinimizedTestCase.test | took 1ms


[1] https://searchfox.org/mozilla-central/source/testing/marionette/harness/marionette_harness/marionette_test/decorators.py

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Ok, so there is actually no way around that without messing up with the testcase classes in the harness.

Given that we have code to run in the custom skip decorators which need access to an existing Marionette instance, we clearly have to run the setUp() method of the MarionetteTestCase class. And as such we also run the setUp method of the testcase class.

We could think about to move the instance creation already to the setUpClass() method, but not sure which implications it would have without digging too much into this problem.

For now we should make sure that setUp methods don't run code which should be avoided in case of the custom skip decorator declares the method to be skipped.

Comment 2

[Andreas Tolfsen ❲:ato❳]

I’m inclined to say we should not fix this bug and instead focus
future efforts on porting the entire test suite to pytest.  I think
that would be a vastly better use of our time than hacking on
unittest.