1021898 - skip-if = key == "value" is ignored unless wrapped with parenthesis ()

Status: RESOLVED INVALID

(Testing :: Mochitest, defect)

Description

[Armen [:armenzg]]

I pushed this change to add disabled tests to the test zips and I got tests running on b2g emulator jobs even if skip-if was set to buildapp == "b2g":
https://hg.mozilla.org/try/rev/fd6871af9796
https://tbpl.mozilla.org/?tree=Try&rev=fd6871af9796

This also happened with "toolkit == 'android'".

I assume this is related to runtestb2g.py but I could be wrong.

The workaround was to wrap it with parenthesis (), e.g.:
https://hg.mozilla.org/try/rev/f6edd210fcc3

-skip-if = buildapp == 'b2g' || toolkit == 'android' || e10s # b2g(clipboard undefined) b2g-debug(clipboard undefined) b2g-desktop(clipboard undefined)
+skip-if = (buildapp == 'b2g') || (toolkit == 'android') || e10s # b2g(clipboard undefined) b2g-debug(clipboard undefined) b2g-desktop(clipboard undefined)

Comment 1

[(not currently active) Ted Mielczarek]

We definitely shouldn't land that change, we should figure out what's broken here and fix it.

Comment 2

[Armen [:armenzg]]

ted, do you have any suggestion as to what is a good approach to fix this?
I assume I can write a unit test to help me ensure that it gets fixed.
However, I don't what is the way to do so. If you have any pointers please let me know.

Comment 3

[(not currently active) Ted Mielczarek]

It sounds like the problem is actually in the Mochitest harness itself, not the build system, since the problem is that tests are being run when they shouldn't. Are you still using json manifests for these test types? If so it's possible that there's a bad interaction.

Comment 4

[Armen [:armenzg]]

I think the trick was to:
- at build time, include even disabled tests
- at run time, ignore disabled tests

I believe we deal with this on the desktop side by removing them from the loop [1]
> for test in tests:
>    if 'disabled' in test:
>      del test['disabled']

The runtestsb2g.py does not do such deletion.

I did a push to try [2] by passing a variable disabled=True and overwriting  it with disabled=False when calling it at runtime.

I have to do a bunch of cleanup on the manifests but I should be in better shape by the end of tomorrow.

[1] http://mxr.mozilla.org/mozilla-central/source/testing/mochitest/runtests.py#469
[2] https://tbpl.mozilla.org/?tree=Try&rev=3bb93c4c7f05