Organize the test transforms into smaller pieces
Categories
(Firefox Build System :: Task Configuration, task, P3)
Tracking
(Not tracked)
People
(Reporter: ahal, Assigned: ahal)
References
Details
Attachments
(10 files)
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review |
In bug 1696041 I had to add a hack or two to allow the 'raptor' transforms to use by-variant
before a by-app
key. In general I think that splitting variants should be done as early as possible since it seems like a very common key to want to use handled_keyed_by
with.
I'd propose to move them to an independent transforms file which loads before the tests.py transforms in general, and before the raptor.py transforms for the raptor case.
Assignee | ||
Comment 1•3 years ago
|
||
This work was mostly done, but I'm going to repurpose this bug to generally re-organize the test transforms better.
Assignee | ||
Updated•3 years ago
|
Assignee | ||
Comment 2•3 years ago
|
||
This includes:
transforms/tests.py -> transforms/test/__init__.py
transforms/raptor.py -> transforms/test/raptor.py
This is a pre-cursor to splitting the file up into multiple smaller files under
the new 'test' transform directory.
Assignee | ||
Comment 3•3 years ago
|
||
In the test transforms we currently have general 'test_description_schema' and
a schema for raptor / browsertime tasks. Prior, the raptor schema was validated
before the test schema, and we were doing a lot of validation twice.
This revision switches the order such that the test schema is first evaluated,
and then the raptor one is. To accomplish this, we create a 'raptor' subconfig
that the 'test_description_schema' completely ignores (and leaves up to the
raptor schema to evaluate).
The benefit of this change is that all test tasks will have a single entry point
when they get into the 'test' transforms. This makes reasoning about their
configuration much easier and lays the ground work to start splitting even more
transforms out into other modules.
Depends on D132068
Assignee | ||
Comment 4•3 years ago
|
||
This ensures validation happens before we handle variants and raptor. It means
we no longer make any meaningful changes to the graph before validating.
Depends on D132069
Assignee | ||
Comment 5•3 years ago
|
||
We'd like to break down the test transforms into smaller well-ordered pieces.
This will allow us to run any transforms defined in files under the
'transforms.test' directory.
Depends on D132070
Assignee | ||
Comment 6•3 years ago
|
||
We have a 3 way circular dependency in the test transforms:
- The 'raptor' transforms depend on 'split_variants'
- The 'get_mobile_project' utility function depends on the 'raptor' transforms
- The 'split_variants' transform depends on the 'get_mobile_project' utility function
The only reason things are working is that in step 3 we don't need the result
of 'get_mobile_project' to be perfect. This revision breaks the circular
dependency by checking for 'android' in the test-platform rather than relying
on 'get_mobile_project == geckoview'.
While this is admittedly not as precise, we only apply the particular variant
that needs this to geckoview tasks in the first place. In other words, this
revision does not cause any additional tasks to be added / removed.
Depends on D132071
Assignee | ||
Comment 8•3 years ago
|
||
This reference was in a new test which wasn't on searchfox yet.
Assignee | ||
Updated•3 years ago
|
Assignee | ||
Comment 10•3 years ago
|
||
Many things need to be set "by variant", so therefore we need to resolve them
early on in the process. By pulling them out into a separate file that
explicitly runs before the "init.py" transforms, we A) make it harder
for someone to accidentally add their transform ahead of them, and B) reduce
some of the clutter in "init.py".
Assignee | ||
Comment 11•3 years ago
|
||
Future commits will tease this apart a bit more, but for now this helps
crystallize the order in which transforms are applied. The flow of the overall
test transforms goes something like this:
- Enter 'transforms/test/init.py'
- Validate all tasks against the test_description_schema
- Run sibling transforms (starting with 'variant.py' and ending with 'main.py' for now)
- Make the job description
As we pull more transforms out of 'main.py' and into their own smaller
transform files, it will be clear that the order in which these smaller files
run is important. Adding new transforms will no longer involve picking some
random spot to insert it.
Depends on D132408
Assignee | ||
Comment 12•3 years ago
|
||
Chunking transforms should always go last since there should never be
configuration that is different between chunks (other than what is set by the
chunking transforms themselves).
Running them last also ensures we don't do extra work as the number of tasks
grows by 10x or more after splitting chunks.
Depends on D132409
Assignee | ||
Comment 13•3 years ago
|
||
This is being split to a new file mainly because the worker definitions take up
a lot of LOC. Getting this out of the main transforms makes it easier to
navigate.
Depends on D132410
Comment 14•3 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/197932d5aecb
https://hg.mozilla.org/mozilla-central/rev/0d0151af2ece
https://hg.mozilla.org/mozilla-central/rev/05210e8d9e8d
https://hg.mozilla.org/mozilla-central/rev/4875751bb32d
https://hg.mozilla.org/mozilla-central/rev/60e50587018c
https://hg.mozilla.org/mozilla-central/rev/92fe337bfdbe
Comment 15•3 years ago
|
||
Updated•3 years ago
|
Comment 16•3 years ago
|
||
Comment 17•3 years ago
|
||
bugherder |
Assignee | ||
Updated•3 years ago
|
Updated•3 years ago
|
Description
•