Open Bug 1432456 Opened 2 years ago Updated 2 years ago
Custom Marionette harness skip decorators shouldn't trigger set
Up() and tear Down() for skipped tests
As noticed while working on bug 1429759, the custom skip decorators in decorators.py  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  https://searchfox.org/mozilla-central/source/testing/marionette/harness/marionette_harness/marionette_test/decorators.py
2 years ago
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.
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.
OS: Unspecified → All
Priority: -- → P4
Hardware: Unspecified → All
You need to log in before you can comment on or make changes to this bug.