Inheriting annotations into included reftest.list files

This will probably come as a surprise to many (as it does to me each
time I rediscover it), but if, in a reftest.list file, you do
something like this (real example from [1]):

skip-if(browserIsRemote) include ogg-video/reftest.list

this may not do what you expect. My expectation, at least, is that the
entire ogg-video/reftest.list file is skipped if the "browserIsRemote"
condition is true.

However, what *actually* happens is that the skip-if expected status
(which is EXPECTED_DEATH [2]) gets "inherited" down into the included
reftest.list, and if there's a higher-valued [3] annotation on a
reftest inside that included list, then that will "win". So in
practice, this means that any reftest inside ogg-video/reftest.list
that has a fuzzy() annotation, or a fuzzy-if(x) annotation where x is
true, will still run.

This seems like a very unexpected result, and looking through some of
the cases where this happens it's not at all clear to me if this was
intentional, or if these tests are just running accidentally because
nobody realized this would happen.

I'm happy to make changes to the reftest manifest parser to remove
this footgun (most likely by disallowing annotations on include
statements) but we would need to go through each instance of this in
the reftest.list files and fix things up so that the tests that are
running are in line with the expectations of whoever added/owns the
tests.

I wanted to open this up for discussion in case people have any
thoughts on it before I move forward and try to clean this up.

[1] https://searchfox.org/mozilla-central/rev/03877052c151a8f062eea177f684a2743cd7b1d5/layout/reftests/reftest.list#275
[2] https://searchfox.org/mozilla-central/rev/03877052c151a8f062eea177f684a2743cd7b1d5/layout/tools/reftest/manifest.jsm#228
[3] https://searchfox.org/mozilla-central/rev/03877052c151a8f062eea177f684a2743cd7b1d5/layout/tools/reftest/globals.jsm#42
0
Kartikaya
1/10/2018 3:49:22 PM
mozilla.dev.platform 6264 articles. 0 followers. Post Follow

0 Replies
27 Views

Similar Articles

[PageSpeed] 56

Reply: