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)
signature.asc
Description: PGP signature
_______________________________________________ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
