1213026 - Skip a test based on the value of a browser pref

Status: RESOLVED FIXED

(Remote Protocol :: Marionette, defect)

Description

[Maja Frydrychowicz :impossibus (was :maja_zf) (needinfo me)]

Often, features being tested are hidden behind a browser pref. So tests might contain code like:

>   if not self.prefs.get_pref('some.thing.enabled'):
>      self.skipTest('some.thing.enabled: false')

It would be convenient to provide this functionality in a general-purpose Python decorator for skipping tests based on a pref.

One concern is the situation where tests are skipped at some point based on a pref, but then the way the pref is used changes (maybe it's removed, say) such that tests that *should not* be skipped *do* get skipped. We can't stop anyone from skipping tests based on a pref, but we could minimize the risk or discourage this type of skipping. Maybe the decorator should only skip if the pref is set (it exists), and raise some more serious exception otherwise?

Comment 1

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

(In reply to Maja Frydrychowicz (:maja_zf) from comment #0)
> >   if not self.prefs.get_pref('some.thing.enabled'):
> >      self.skipTest('some.thing.enabled: false')
>
> It would be convenient to provide this functionality in a general-purpose
> Python decorator for skipping tests based on a pref.

Generally I would support the @unittest.skip like decorator with the if condition, so that those lines do not end up in the test itself. In general this sounds great, given that we had to do a lot of workarounds in Mozmill tests due to new or changed features which have been turned on and off.

> One concern is the situation where tests are skipped at some point based on
> a pref, but then the way the pref is used changes (maybe it's removed, say)
> such that tests that *should not* be skipped *do* get skipped. We can't stop
> anyone from skipping tests based on a pref, but we could minimize the risk
> or discourage this type of skipping. Maybe the decorator should only skip if
> the pref is set (it exists), and raise some more serious exception otherwise?

Yes, that's how it should work. If the pref does not exist, do not skip it. In general the behavior should not change by a pref. A new pref will be used instead. So I wouldn't worry that much about it yet, or do you have an example where this happened?

Comment 2

[Julien Pagès (:parkouss)]

Attachment: 1213026.patch

Hey, I'm interested into working on this one.

So far I have added a skip_unless_browser_pref function in marionette_test.py.

Minimal example:

> from marionette import MarionetteTestCase
> from marionette.marionette_test import skip_unless_browser_pref
> 
> class TestSomething(MarionetteTestCase):
>     @skip_unless_browser_pref("browser.tabs.animate")
>     def test_foo(self):
>         # ...
> 
>     @skip_unless_browser_pref("accessibility.tabfocus",
>                               lambda value: value >= 7):
>     def test_bar(self):
>         # ...

Tested locally, seems to works well.

Comment 3

[Julien Pagès (:parkouss)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/99f004477ccb572ac43b192cae16bd99492d3152
Bug 1213026 - Skip a test based on the value of a browser pref. r=dburns

Comment 4

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/99f004477ccb

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/99f004477ccb

Comment 6

[Carsten Book [:Tomcat]]

removing the b2g 2.5 flag since this commit has been reverted due to an incorrect merge, sorry for the confusion