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

RESOLVED INVALID

Status

RESOLVED INVALID
4 years ago
4 years ago

People

(Reporter: armenzg, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

4 years ago
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)
(Reporter)

Updated

4 years ago
Component: General → Mozbase
QA Contact: hskupin
We definitely shouldn't land that change, we should figure out what's broken here and fix it.
(Reporter)

Comment 2

4 years ago
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.
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.
Component: Mozbase → Mochitest
QA Contact: hskupin
(Reporter)

Comment 4

4 years ago
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
(Reporter)

Updated

4 years ago
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.