1411343 - add bugzilla component information to testing/web-platform-tests/meta/

Status: RESOLVED FIXED

(Testing :: web-platform-tests, enhancement)

Description

[Joel Maher ( :jmaher ) (UTC -8)]

we have 7830 files in testing/web-platform/ which do not have a bugzilla_component defined, almost all are in the /meta/ folder.

We could map these to the /tests/ equivalent, or consider just marking these .ini files as part of testing/web-platform/

Comment 1

[Joel Maher ( :jmaher ) (UTC -8)]

:jgraham, what would you prefer?

Comment 2

[James Graham [:jgraham]]

I don't think this makes much sense. These files are just metadata files, what's the point of giving them a bugzilla component?  If they are wrong, it will show up as the test being wrong.

So, in principle it would be the same component as the test files, but in practice I think that just adds extra fragility since the moz.build files are so fragile to changes in the source in a way that's easy to miss since it isn't checked with artifact builds. So I would prefer we just ignore these files.

Comment 3

[Joel Maher ( :jmaher ) (UTC -8)]

we upload artifacts of bugzilla component <-> source file mappings:
https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=e1955af99935224decbae66ea14ad6dc3e6ef914&filter-searchStr=bugzilla&selectedJob=139245134

there is components.json, and missing.json, we have 10639 missing files and 7830 are in testing/web-platform.  I don't think we need to fix this specifically, but it makes it hard to know when something new shows up without a component.  When we don't have components we end up disabling tests or letting bugs slip through the cracks because we don't get the bugs to the right people.

Comment 4

[Joel Maher ( :jmaher ) (UTC -8)]

Attachment: proxy all BUGZILLA_COMPONENT info from tests/ -> meta/

this approach seems easier to maintain, but if there is a better way to do this, let me know!

Comment 5

[James Graham [:jgraham]]

Comment on attachment 8921865 [details] [diff] [review]
proxy all BUGZILLA_COMPONENT info from tests/ -> meta/

This will break some code I wrote that automatically removes components where the directory doesn't exist any more. Of course the code is rather fragile, but it is trying to deal with arbitrary python input, so that's perhaps unsurprising.

If you want to do this, I would much prefer if we could put the data in a non-python file (json, yaml, whatever) and open() that during processing. Then the script for updating it could use normal tools and not lightly-documented parts of lib2to3.

Comment 6

[Joel Maher ( :jmaher ) (UTC -8)]

:gps, any further feedback before I implement what :jgraham suggested?

Comment 7

[Gregory Szorc [:gps]]

Comment on attachment 8921865 [details] [diff] [review]
proxy all BUGZILLA_COMPONENT info from tests/ -> meta/

Review of attachment 8921865 [details] [diff] [review]:
-----------------------------------------------------------------

This seems like a reasonable solution! Yay for moz.build files actually being Python :)

Comment 8

[Gregory Szorc [:gps]]

Oh, you don't have access to open() in moz.build files.

Long ago (I'd have to dig up the bug), I wanted to introduce the concept of "data files" to moz.build. This would facilitate storing static data structures for things like lists of directories. That's not implemented though. So the best you can do is create an inline variable (as this patch did) or add a new UPPERCASE variable to the moz.build sandbox holding your special-cased data. That variable could be populated by loading a file because it doesn't reside in the execution sandbox and has full access to Python.

Comment 9

[Joel Maher ( :jmaher ) (UTC -8)]

:jgraham, given the information :gps has shed on opening files from moz.build files, do you have other preferences?  How does this break the situation that was in place before?  while I am eager to land this, I don't want to break a process that currently works

Comment 10

[James Graham [:jgraham]]

The current process basically processes the Python file looking for code like

with Files(<argument>):
   block

and checks whether the argument matches any existing paths. So if you change things so that there isn't a Files block per path that is going to break. Once we are using the "moz.build files are just Python code" feature in a non-trivial way (i.e. anything other than just a series of configuration statements) I expect anything that tries to process the files other than mozbuild itself to break. Since we enforce the rule that Files rules have to match at least one file in the tree, it's important that automatic sync processes are able to adjust these rules when upstream invalidates them.

I think any of the following solutions would be acceptable, in approximately decreasing order of preference:

* Do what GPS suggests and add a mechanism for loading this data into a variable from an data file and process that in the moz.build file rather than adding the rules directly. Working with a non-code file would make consuming and especially updating this data significantly easier.

* Duplicate all the rules for metadata files without changing the format.

* Just ignore the metadata files. They hardly ever have bugs, and are frequently removed. This rule has the notable advantage that we won't get the build breaking every time we fix all the tests in some directory (which I expect to be a common source of developer frustration if we add this particularly since the checks don't get run on artifact builds, and cause the build job to fail in other cases).

Comment 11

[Joel Maher ( :jmaher ) (UTC -8)]

maybe it is simpler to just assign all testing/web-platform/meta/* files to testing::web-platform-tests ?  That would be easy to review, less error prone, and only slightly less accurate (given that you could look at the bug component for the given tests/ files)

Comment 12

[Joel Maher ( :jmaher ) (UTC -8)]

Attachment: map all /meta/* files to testing:web-platform-tests

here is a patch to do the simple option- if we want this please r+ and I can land.

Comment 13

[James Graham [:jgraham]]

Comment on attachment 8922279 [details] [diff] [review]
map all /meta/* files to testing:web-platform-tests

Sure. I don't think this is the best option (I would still rather this data was in a data file rather than in code), but it solves your problem without causing me grief.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e5069413f4d6
add bugzilla component information to testing/web-platform-tests/meta/. r=jgraham

Comment 15

[Joel Maher ( :jmaher ) (UTC -8)]

Attachment: updated patch to account for files not in meta/

I found some issues when pushing to try with my old patch- so if we have a solution for getting data from a file, it will need to be similar to the above.

Comment 16

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/e5069413f4d6