Created attachment 682659 [details] [diff] [review] Use cleanup methods instead of tearDown Using setUp/tearDown causes an issue when there's a failure in setUp as in this scenario tearDown is not called. I believe this is causing tests to exit in an unpredictable state. By using cleanup, the methods are always called, and the tests can provide additional cleanup methods as needed. You can currently see an example of this in my fork of the gaia-ui-tests repo . Unfortunately, this requires Python 2.7, which I understand may be a blocker. If we can find a way around this then I believe it will increase test stability.  https://github.com/davehunt/gaia-ui-tests/commit/92f78a76eeb2ba7850503eb7ca6c49d766f535bb
Attachment #682659 - Flags: feedback?(jgriffin)
Comment on attachment 682659 [details] [diff] [review] Use cleanup methods instead of tearDown Review of attachment 682659 [details] [diff] [review]: ----------------------------------------------------------------- I would love to land this, but unfortunately addCleanup requires Python 2.7, and Marionette is running on buildbot slaves that only have 2.6. There's some work underway to get 2.7 on all the slaves (see bug 724191), but it isn't high priority and won't happen in the short term. Meanwhile, I think we could achieve a similar effect by moving most of the code that's currently in setUp in gaiatest test cases to another function, and call that function from within the test. That is, avoid doing things in setUp which can throw exceptions, in which case tearDown will get called. Another option is to re-implement something like addCleanup() in Marionette, although this would require duplicating some code from Python 2.7's unittest.TestCase.
Attachment #682659 - Flags: feedback?(jgriffin) → feedback-
Dave, want to take a stab at the middle option in comment 1?
I'm not sure that's my preferred option. It would potentially add a lot of duplicated code into the tests. Another option would be to catch any exceptions thrown during setUp to ensure we still run the necessary tearDown.
(In reply to Dave Hunt (:davehunt) from comment #3) > I'm not sure that's my preferred option. It would potentially add a lot of > duplicated code into the tests. Another option would be to catch any > exceptions thrown during setUp to ensure we still run the necessary tearDown. If we do it this way, how to we terminate the test from setUp? I.e., if we self.fail() in setUp, does tearDown still get called?
Given that stability has improved, I would suggest we either close this bug as WONTFIX or mark it as dependant on bug 724191. Thoughts?
+1 for depending on bug 724191 (i'd just mark it, but am less of a stakeholder here)
5 years ago
No longer blocks: 801898
Dave, can you close this one? I think it's long since done!
It's not done, and it's blocked as per the dependency. I don't see this changing any time soon, so let's mark it as WONTFIX.
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.