883281 - Add the ability to skip tests in the manifest based on the platform

Status: RESOLVED FIXED

Description

[Dave Hunt [:davehunt] [he/him] ⌚️UTC]

We would like to be able to skip or fail tests based on the platform they're running on. For example, there may be tests that fail on B2G desktop builds but not on devices. Rather than maintaining separate manifest files, we could take use ManifestDestiny's skip-if/fail-if.

For example, the manifest might look like this:

[test_something_that_fails_on_desktop.py]
fail-if = platform == 'b2gdesktop'

[test_something_that_uses_camera.py]
skip-if = platform == 'b2gdesktop'

Comment 1

[Jonathan Griffin (:jgriffin)]

Attachment: Add ability to skip tests based on device

This is a WIP.  It adds support for skip-if, fail-if for specific devices, e.g.,

skip-if = device == panda

The possible values of device are qemu, desktop, and then any actual device name (panda, unagi, etc).

Skipped tests appear in the log as:

TEST-SKIP | test_name.py | device = desktop

Skipped tests get added to the 'todo' count at the end of the run.

Dave, does this seem like good approach?

Comment 2

[Jonathan Griffin (:jgriffin)]

FYI, I don't think fail-if works yet, but I haven't tried.

Comment 3

[Jonathan Griffin (:jgriffin)]

fail-if doesn't work at all, and will require quite a bit more surgery to support.  In order to avoid stepping on bug 874599, I'm going to split that work off into a separate bug.

Comment 4

[Jonathan Griffin (:jgriffin)]

Attachment: Add ability to skip tests based on device

Pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=388764ab7626

Comment 5

[Jonathan Griffin (:jgriffin)]

Comment on attachment 764367 [details] [diff] [review]
Add ability to skip tests based on device

Review of attachment 764367 [details] [diff] [review]:
-----------------------------------------------------------------

This changes the "old" self.device to self.device_serial, so we can use self.device to refer to the device that Marionette returns in session_capabilities.

This also fixes some bad whitespace in some (mostly unrelated) code (tabs instead of spaces).

Comment 6

[Dave Hunt [:davehunt] [he/him] ⌚️UTC]

Comment on attachment 764367 [details] [diff] [review]
Add ability to skip tests based on device

Review of attachment 764367 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, just a couple of questions.

::: testing/marionette/client/marionette/runtests.py
@@ +611,4 @@
>                          action='store',
>                          help='host:port of running Gecko instance to connect to')
>          self.add_option('--device',
> +                        dest='device_serial',

Where is this value read? I don't see it read anywhere in this patch.

::: testing/marionette/marionette-server.js
@@ +579,4 @@
>            'javascriptEnabled': true,
>            'nativeEvents': false,
>            'platform': Services.appinfo.OS,
> +          'device': qemu == "1" ? "qemu" : (!device ? "desktop" : device),

Is there (do we need?) a way to distinguish between Firefox desktop and B2G desktop?

Comment 7

[Jonathan Griffin (:jgriffin)]

Attachment: Add ability to skip tests based on device

Good catches!  self.device_serial is only used by the update tests, so this fixes the only consumer of that.

Re: needing to detect b2g desktop vs firefox, currently, we have no need for that, but it's something that we certainly may need.  Therefore, I've added an 'app' variable to the manifests:

skip-if = app == B2G && device == desktop (for example)

or:

skip-if = app == Firefox && device == desktop

For the gaia-ui-tests, we don't ever intend to run them on firefox, so just using:

skip-if = device == desktop

would be sufficient, I think.

Comment 8

[Jonathan Griffin (:jgriffin)]

Oops, those should be:

skip-if = app == "B2G" && device == "desktop" (for example)

or:

skip-if = app == "Firefox" && device == "desktop"

(The quotes were missing above.)

Comment 9

[Dave Hunt [:davehunt] [he/him] ⌚️UTC]

Comment on attachment 764399 [details] [diff] [review]
Add ability to skip tests based on device

Review of attachment 764399 [details] [diff] [review]:
-----------------------------------------------------------------

Awesome, thanks! Looking forward to updating the manifest files! :)

Comment 10

[Jonathan Griffin (:jgriffin)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f69a75b67d1f

Comment 11

[Jonathan Griffin (:jgriffin)]

Will file another bug to bump and release to pypi after this lands.

Comment 12

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/f69a75b67d1f

Comment 13

[Jonathan Griffin (:jgriffin)]

http://hg.mozilla.org/releases/mozilla-b2g18/rev/cd86de40cd5c

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18_v1_1_0_hd/rev/cd86de40cd5c