Closing the loop: the changes in bug 1429580 have landed, and so now doing "skip-if(foo) include bar/reftest.list" behaves as one would expect, and skips the include entirely if the foo condition is true.
On Wed, Jan 10, 2018 at 4:01 PM, L. David Baron <[email protected]> wrote: > On Wednesday 2018-01-10 10:49 -0500, Kartikaya Gupta 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. > > I think some of the mistake here is related to how fuzzy was > implemented: > > For a start, fuzzy shouldn't have outranked skip in the list of > statuses. > > But really, fuzzy is sometimes used in different ways: it's > sometimes used as a variation of the status (expected to pass, > expected to fail, etc.) but also sometimes used a variation of the > expected result (==, !=). It's sort of a mess, since there are > different understandings of what it should mean that make more or > less sense for different uses. > > But I'd be fine just getting rid of annotations on 'include' > directives. They're confusing, and there was a substantial need for > them for a short period of time (which was mostly about > transitionally enabling tests on new platforms), but I think it's > over. > > (If we need to keep skip for include directives, maybe we could at > least get rid of the others, since if you go with dholbert's > proposal skip would be processed in an entirely different way from > the others.) > > -David > > -- > 𝄞 L. David Baron http://dbaron.org/ 𝄂 > 𝄢 Mozilla https://www.mozilla.org/ 𝄂 > Before I built a wall I'd ask to know > What I was walling in or walling out, > And to whom I was like to give offense. > - Robert Frost, Mending Wall (1914) _______________________________________________ dev-platform mailing list [email protected] https://lists.mozilla.org/listinfo/dev-platform
