On Fri, Feb 10, 2017 at 8:37 PM, James Graham <ja...@hoppipolla.co.uk> wrote: >> Should we be requiring people to update the MANIFEST.json whenever >> they touch a file in testing/web-platform/tests (i.e. not just when >> they add/remove files)? > > > This is too much to ask of people; it was effectively required when the > manifest format change first landed and the result was that the associated > lint got hidden on treeherder for being orange too often. Therefore I > changed the lint to only complain if the manifest changes would result in > the wrong tests being run, or tests being run incorrectly.
Ok. That seems reasonable. It does increase the chance of merge conflicts, unfortunately. Even within a given patch series where you're using manifest-update to add/remove/rename files you can often encounter conflicts if you update the test content of an underlying patch and need to regenerate the manifest for each subsequent patch (as I recently discovered) . I suppose not many people are re-organizing web-platform-tests frequently enough for this to be a significant problem though. > The advantage of the checksums in the file is faster incremental updates > with lower complexity. Given that, one possible short term win would be to > make --manifest-update the default for |mach wpt| so that people are more > likely to update it correctly when they are authoring tests. I'm not sure we should do that. Since it touches version-controlled files it is likely to cause trouble as developers jump around their patch queue running mach wpt at various points (e.g. to ensure tests pass at all stages of the patch series--i.e. each changeset is atomic; or to bisect a failing test). For example, you check out a particular revision, run mach wpt, then go to checkout a different revision to run the tests there on only to discover you have changes in your working directory that you need to either commit and then rebase, or discard. >> (Currently when creating a patch queue that adds/removes files from >> web-platform-tests my workflow involves first creating a base patch to >> include all the checksum changes to m-c that haven't been added to >> MANIFEST.json (so they don't show up in later patches in the series >> when I need to run --manifest-update), regenerating that patch >> whenever I rebase, and dropping it when I land the patch queue. >> Probably I'm doing something wrong, however.) > > > Rebasing is a bit of a pain. I think it is possible to configure mercurial > (or git) to just run |mach wpt-manifest-update| whenever there's a conflict > in this file so that it's regenerated to be correct rather than needing > manual work (there are edge cases where this likely won't work, but I think > those are rare enough that it's worth trying). I'm a bit concerned about having hg/git automatically modify these files for the reasons I mentioned above, but maybe it could work. For now, I suspect not too many people are doing significant refactoring of web-platform-tests so maybe this isn't such a big issue. Thanks again, Brian _______________________________________________ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
