Another option would be to keep allowing this syntax of "skip-if(x) include some/reftest.list" but actually make it skip the entire include if the condition "x" is true.
On Wed, Jan 10, 2018 at 10:49 AM, Kartikaya Gupta <kgu...@mozilla.com> wrote: > 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 _______________________________________________ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
