Closed Bug 676876 Opened 13 years ago Closed 12 years ago

skip-if in included manifest over-rides skip-if in including manifest

Categories

(Testing :: Mozbase, defect)

x86
Linux
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED WONTFIX

People

(Reporter: gbrown, Unassigned)

References

Details

(Whiteboard: [mozbase])

I tried to skip all services/sync xpcshell tests on android, by changing the entry in testing/xpcshell/xpcshell.ini to: [include:services/sync/tests/unit/xpcshell.ini] skip-if = os == "android" This skipped the sync tests *except* for tests in the sync xpcshell.ini that also had a skip-if statement already. For instance (from services/sync/tests/unit/xpcshell.ini): [test_jpakeclient.js] # Bug 618233: this test produces random failures on Windows 7. skip-if = os == "win" test_jpakeclient.js was *not* skipped on android: ... TEST-INFO | skipping /home/geoff/src/mozilla-central/objdir-droid/_tests/xpcshell/services/sync/tests/unit/test_interval_triggers.js | skip-if: os == "android" TEST-INFO | /home/geoff/src/mozilla-central/objdir-droid/_tests/xpcshell/services/sync/tests/unit/test_jpakeclient.js | running test ... TEST-PASS | /home/geoff/src/mozilla-central/objdir-droid/_tests/xpcshell/services/sync/tests/unit/test_jpakeclient.js | test passed TEST-INFO | skipping /home/geoff/src/mozilla-central/objdir-droid/_tests/xpcshell/services/sync/tests/unit/test_keys.js | skip-if: os == "android" It appears that skip-if directives in an included manifest override the directives of the including manifest, making it more difficult to skip an entire set of tests.
All keys currently override those cascaded down, whether from [include:] or [DEFAULT] sections. Is the desired behaviour: 1. To record that these tests are being skipped? 2. To not include the subfile at all? 3. Doesn't matter -- either 1 or 2 are fine In case 1, I would suggest introducing a notation that a key should dominate locals on cascade. Syntax to be debated (though, e.g. I like @skip-if, e.g.). In case 2, I would suggest restructuring such that inclusion is separated from parsing in a way. There are probably other strategies for this too. In general, the current architecture features a separation of concerns for `manifests of stuff` (ManifestParser) and `test manifests` (TestManifest): https://wiki.mozilla.org/Auto-tools/Projects/ManifestDestiny#Architecture We should probably respect this. Changing the architecture has several implications, e.g. for MozProfile.
From my perspective (as a "manifest modifier", if you will), if I skip-if an [include:], my expectation is that the subfile is not included at all. That may just be the way I read it: "skip this include". BTW, I have worked around my issue by adding conditions to the subfile: [test_jpakeclient.js] # Bug 618233: this test produces random failures on Windows 7. # Bug xxx: all tests in this directory hang on android skip-if = os == "win" || os == "android"
I think the above workaround is probably "correct", as the system stands. I will work on making skip-if for manifests skip the entire include. Note however that there is an asymmetry here, or at least unanswered questions. To phrase one: [include:anothermanifest.ini] fail-if = os == "windows"
Whiteboard: [mozbase]
Component: ManifestParser → Mozbase
Having thought more on it, I don't believe that making skip-if a special case is a good idea. The cascading model used for manifests is pretty simple: the current defaults updated with the (key, value)s of an [include:] section serve as the defaults for the included .ini. Like all other (key, value)s , though, their value may be overridden as well as is the case here. Unless we really want to reconsider that model, I don't think we should special-case `skip-if` or any of the other keys. The code will become more complicated for both human and machine with all the implications that comes therewith. I am sympathetic to the problem and would like to help with a solution, though. I am more than happy to help build tools to help cascade down skip-if and other conditionals if there is an interest. E.g. cascade skip-if=os==win mymanifest.ini # adds skip-if to the [include:] and combines any child skip-if conditions with the os==win check (...if that makes sense.) I'm also more than happy developing diagnostic tools displaying what would be run, etc etc to (and beyond) the heart's desire. I would consider a more generic system of what cascades down and what not, but I do think it opens a bit of a can of worms.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.