960358 - [marionette-js-runner] Enable js marionette blacklist manifest in gaia

Status: RESOLVED FIXED

(Testing Graveyard :: JSMarionette, defect)

Description

[Gareth Aye [:gaye] (back from PTO)]

In bug 897805 jugglinmike implemented blacklisting tests via a simple json manifest for marionette-js-runner. We should use this for tests which are failing on TBPL and not Travis so that we can turn them off on TBPL only.

Comment 1

[Gareth Aye [:gaye] (back from PTO)]

Attachment: Link to Github pull-request: https://github.com/mozilla-b2g/marionette-js-runner/pull/29

Hi Mike,

Right now if you pass an invalid filename or something that's not JSON parseable to -M, --manifest in marionette-js-runner we bail. I'd like to change the behavior to run without the applied manifest. This will help me in gaia where I want to default to running without a blacklist, but with the option of passing TEST_MANIFEST=... to disable TBPL only failures (like so https://gist.github.com/gaye/8460608)

Comment 2

[Gareth Aye [:gaye] (back from PTO)]

If it's not clear, passing --manifest="" makes marionette-js-runner@0.4.0 die.

Comment 3

[Gareth Aye [:gaye] (back from PTO)]

Attachment: Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/15434

Comment 4

[Gareth Aye [:gaye] (back from PTO)]

Attachment: Patch to mozharness

Hey Aki,

We'd like to turn on a blacklist manifest for gaia integration tests on tbpl. Would you mind reviewing and pushing this for me?

Thanks!

Comment 5

[Michael Henretty [:mikehenrty][:mhenretty]]

Comment on attachment 8361368 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/15434

Working for me locally, and in travis. Now for tbpl :)

Comment 6

[Aki Sasaki (not active)]

Comment on attachment 8361370 [details]
Patch to mozharness

http://hg.mozilla.org/build/mozharness/rev/1798ff093fb4
Merged to production

Comment 7

[Mike Pennisi [:jugglinmike]]

Comment on attachment 8361211 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/marionette-js-runner/pull/29

Hey Gareth,

I'm hesitant to accept this change. The behavior it introduces is a little bit
confusing--I don't know of any Unix utilities that behave in this way.
Normally, a flag that requires a value is considered invalid when no value is
specified.

Beyond being somewhat unconventional, this behavior would also be technically
incorrect in the event that the program was executed with a inaccessible file
(i.e. if the file was non-existent or non-readable by the current user).

I appreciate you sharing the context, though--it gives me an idea for another
solution to your underlying problem. Instead of conditionally specifying the
*value* of the flag, you can conditionally specify the flag itself (value and
all):

    diff --git a/Makefile b/Makefile
    index 7da5854..3e4d0d7 100644
    --- a/Makefile
    +++ b/Makefile
    @@ -141,6 +141,7 @@ NPM_REGISTRY?=http://registry.npmjs.org
     # Ensure that NPM only logs warnings and errors
     export npm_config_loglevel=warn
     MARIONETTE_RUNNER_HOST?=marionette-b2gdesktop-host
    -TEST_MANIFEST?=""
    +TEST_MANIFEST_FLAG=
    +ifdef TEST_MANIFEST
    +  TEST_MANIFEST_FLAG=--manifest $(TEST_MANIFEST)
    +MOZPERFOUT?=""
     
     GAIA_INSTALL_PARENT?=/system/b2g
    @@ -698,6 +699,7 @@ b2g: node_modules/.bin/mozilla-download
     test-integration: b2g $(PROFILE_FOLDER)
            NPM_REGISTRY=$(NPM_REGISTRY) ./bin/gaia-marionette $(shell find . -path "*$(TEST_INTEGRATION_APP_NAME)/test/marionette/*_test.js") \
                    --host $(MARIONETTE_RUNNER_HOST) \
    -               --manifest $(TEST_MANIFEST) \
    +               $(TEST_MANIFEST_FLAG) \
                    --reporter $(REPORTER)
     
     .PHONY: test-perf

Would that work?