1272276 - Add a new task to infrastructure to ensure that manifest update doesn't change anything

Status: RESOLVED DUPLICATE of bug 1302796

(Firefox Build System :: Task Configuration, task)

Description

[Xidorn Quan [:xidorn] UTC+10]

It is very annoying when you are adding new web-platform-tests but you find someone edited the manifest manually so that you get unrelated changes when you run --manifest-update.

Can we add one task to the treeherder to ensure that manifest update doesn't change anything after the push? This could be integrated into some existing lint check I suppose.

Comment 1

[Xidorn Quan [:xidorn] UTC+10]

The easiest way is just run "mach web-platform-test --manifest-update some-nonexistent-dir", so that the manifest is updated, but no test is actually one.

Then check the output of "hg status" see whether the manifest file is updated, and if so, mark it as failed.

The total flow takes ~1min in my local machine.

Comment 2

[Xidorn Quan [:xidorn] UTC+10]

It would also be helpful to add layout/reftests/w3c-css/submitted/check-for-references.sh to the same test, which checks that all tests submitted to CSS test repo have proper meta data added. I think dbaron would appreciate that :)

This is very quick, just takes ~2s in my local machine. So checking this shouldn't be a burden to infra, but would remove extra work like bug 1272004 comment 9, bug 548372 comment 142, bug 1228280 comment 110, etc.

Comment 3

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

(In reply to Xidorn Quan [:xidorn] (UTC+10) from comment #2)
> It would also be helpful to add
> layout/reftests/w3c-css/submitted/check-for-references.sh to the same test,
> which checks that all tests submitted to CSS test repo have proper meta data
> added. I think dbaron would appreciate that :)

It would probably be easier to just add this as a feature to the reftest harness.

Comment 4

[Xidorn Quan [:xidorn] UTC+10]

Move to taskcluster. Looks like that is the right component to add new task.

Comment 5

[Xidorn Quan [:xidorn] UTC+10]

(In reply to David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) from comment #3)
> (In reply to Xidorn Quan [:xidorn] (UTC+10) from comment #2)
> > It would also be helpful to add
> > layout/reftests/w3c-css/submitted/check-for-references.sh to the same test,
> > which checks that all tests submitted to CSS test repo have proper meta data
> > added. I think dbaron would appreciate that :)
> 
> It would probably be easier to just add this as a feature to the reftest
> harness.

Probably yes, and that may save even more work as people would almost always run the tests they add, but may not be aware of the lint test at all.

Comment 6

[Dustin J. Mitchell [:dustin] (he/him)]

I don't think this is something the taskcluster team can handle (I don't know what sort of manifest you're talking about, or what metadata the CSS files must have).  So you're welcome to solve the bug in this component, but please don't abandon it here, unassigned :)

Comment 7

[Xidorn Quan [:xidorn] UTC+10]

I would like to help adding that task, but I'm not quite familiar with taskcluster. If I add a new task in the lint group, how can I test that? Does it work if I push to try? Would treeherder display the newly-added task magically? Is there any document about how to add a new task?

Comment 8

[Dustin J. Mitchell [:dustin] (he/him)]

Yep, you can test in try, and the results will show up on treeherder.  There are already some similar tasks defined.  Check under testing/taskcluster/tasks.

There's documentation in http://gecko.readthedocs.io/en/latest/taskcluster/taskcluster/index.html.  The existing .yml files are the "legacy" task definition format.

#taskcluster can help.  I'm at PyCon at the moment or I'd offer myself.

Comment 9

[Xidorn Quan [:xidorn] UTC+10]

So it would be better to have a mach subcommand directly for checking the manifest.

Comment 10

[James Graham [:jgraham]]

The right thing to do here is a a lint to the new mozlint framework that runs the wpt lint and ensures that the manifest file is up to date whenever there are changes in the wpt directory. That already provides the infrastructure for mach, automation, etc.

Comment 11

[Xidorn Quan [:xidorn] UTC+10]

This seems to have been fixed by bug 1302796.