1388478 - Split tests.yml into suite specific configs

Status: RESOLVED FIXED

(Firefox Build System :: Task Configuration, task)

Description

[Andrew Halberstadt [:ahal]]

Joel was pointing out that tests.yml was approaching ~2000 lines and was starting to get just as difficult to modify as the buildbot-configs were.

We should split it up into, e.g:
mochitests.yml
reftests.yml
xpcshell-tests.yml
etc..

Comment 1

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1388478 - Load tests.yml tasks with the transform loader,

This will allow us to use 'jobs', 'jobs-from' and 'job-defaults' with the test
kind.

Review commit: https://reviewboard.mozilla.org/r/190646/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/190646/

Comment 2

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1388478 - Split tests.yml into several smaller suite-specific files,

This is a simple refactor into separate files. No task definitions should be
modified by this patch.

Most tasks are moved into suite-specific .yml files. A handful of miscellaneous
tasks are defined directly in kind.yml under the 'jobs' key.

Review commit: https://reviewboard.mozilla.org/r/190648/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/190648/

Comment 3

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1388478 - Use job-defaults to simplify 'test' configs where appropriate,

This is mostly a refactor, moving shared defaults to the top of the file where
appropriate.

The only change this makes, is modifying a couple of the macosx talos tasks to
use the osx mozharness config. They were previously using the linux config
which was likely a bug.

Review commit: https://reviewboard.mozilla.org/r/190650/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/190650/

Comment 4

[Andrew Halberstadt [:ahal]]

Comment on attachment 8919729 [details]
Bug 1388478 - Use job-defaults to simplify 'test' configs where appropriate,

https://reviewboard.mozilla.org/r/190650/#review195846

::: taskcluster/ci/test/talos.yml
(Diff revision 1)
> -            by-test-platform:
> -                windows.*:
> -                    - talos/windows_config.py
> -                default:
> -                    - talos/linux_config.py

For some reason this task wasn't defining the mac_config.py for osx. This change is going to make the `talos-perf-reftest` tasks on OSX use `talos/mac_config.py`.

Joel, I was assuming the old behaviour was a bug. Do you know if there was any chance this might have been intentional?

There's another task further down that also wasn't defining mac_config.py.

Comment 5

[Andrew Halberstadt [:ahal]]

I tested this change by generating the full task graph from central, then again with this change and doing a diff of the two. Both taskgraphs were identical save for the case mentioned in comment 4.

Comment 6

[Andrew Halberstadt [:ahal]]

The reason I decided to tackle this now was to make working on bug 1391130 a little easier.

Comment 7

[Andrew Halberstadt [:ahal]]

Comment on attachment 8919728 [details]
Bug 1388478 - Split tests.yml into several smaller suite-specific files,

https://reviewboard.mozilla.org/r/190648/#review195886

I don't know how this modified.tasks file got in here :/. It isn't showing up as added in my local clone and I can't figure out how to remove it from the review request (pushing a new change didn't work).

Comment 8

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

Comment on attachment 8919727 [details]
Bug 1388478 - Load tests.yml tasks with the transform loader,

https://reviewboard.mozilla.org/r/190646/#review195898

Comment 9

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

Comment on attachment 8919728 [details]
Bug 1388478 - Split tests.yml into several smaller suite-specific files,

https://reviewboard.mozilla.org/r/190648/#review195900

::: taskcluster/ci/test/kind.yml:33
(Diff revision 1)
> +   - reftest.yml
> +   - talos.yml
> +   - web-platform.yml
> +   - xpcshell.yml
> +
> +jobs:

Could we move these out to misc.yml, or each to their own?  It's weird to factor *some* but not all out..

Comment 10

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

Comment on attachment 8919729 [details]
Bug 1388478 - Use job-defaults to simplify 'test' configs where appropriate,

https://reviewboard.mozilla.org/r/190650/#review195904

That sure is a lot of red :)

Comment 11

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

Comment on attachment 8919728 [details]
Bug 1388478 - Split tests.yml into several smaller suite-specific files,

https://reviewboard.mozilla.org/r/190648/#review195958

there is an empty file: taskcluster/ci/test/modified.tasks, is this needed for something?

::: taskcluster/ci/test/kind.yml:33
(Diff revision 1)
> +   - reftest.yml
> +   - talos.yml
> +   - web-platform.yml
> +   - xpcshell.yml
> +
> +jobs:

I second :dustin's comment.

::: taskcluster/ci/test/mochitest.yml:555
(Diff revision 1)
> +        by-test-platform:
> +            linux64-qr/.*: 1
> +            windows10-64-asan.*: 3
> +            default: default
> +
> +robocop:

this should be in the 'misc' file

Comment 12

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

Comment on attachment 8919729 [details]
Bug 1388478 - Use job-defaults to simplify 'test' configs where appropriate,

https://reviewboard.mozilla.org/r/190650/#review195962

I was just going to say- we should refactor our definitions to remove duplication, DONE

::: taskcluster/ci/test/marionette.yml:52
(Diff revision 1)
>      description: "Marionette headless unittest run"
> -    suite: marionette
>      treeherder-symbol: tc(MnH)
>      max-run-time: 5400
>      instance-size: default
>      chunks: 1

can we remove chunks:1, that seems redundant.

::: taskcluster/ci/test/talos.yml
(Diff revision 1)
> -            by-test-platform:
> -                windows.*:
> -                    - talos/windows_config.py
> -                default:
> -                    - talos/linux_config.py

we didn't have everything scheduled on osx for load reasons, we should turn it on for osx now.

Comment 13

[Andrew Halberstadt [:ahal]]

Comment on attachment 8919728 [details]
Bug 1388478 - Split tests.yml into several smaller suite-specific files,

https://reviewboard.mozilla.org/r/190648/#review195958

Yeah, see my previous review comment. It somehow got pushed accidentally and it's not added in my local clone. I can't seem to make mozreview forget about it :/. Might have to land this on inbound to avoid checking it in.

> I second :dustin's comment.

Ok, I'll drop this issue and use the other issue to track the fix.

Comment 14

[Andrew Halberstadt [:ahal]]

Comment on attachment 8919728 [details]
Bug 1388478 - Split tests.yml into several smaller suite-specific files,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/190648/diff/1-2/

Comment 15

[Andrew Halberstadt [:ahal]]

Comment on attachment 8919729 [details]
Bug 1388478 - Use job-defaults to simplify 'test' configs where appropriate,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/190650/diff/1-2/

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3f413dcc4af5
Load tests.yml tasks with the transform loader, r=dustin
https://hg.mozilla.org/integration/autoland/rev/24d3adf28162
Split tests.yml into several smaller suite-specific files, r=dustin,jmaher
https://hg.mozilla.org/integration/autoland/rev/0316704b960c
Use job-defaults to simplify 'test' configs where appropriate, r=jmaher

Comment 17

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

https://hg.mozilla.org/mozilla-central/rev/3f413dcc4af5
https://hg.mozilla.org/mozilla-central/rev/24d3adf28162
https://hg.mozilla.org/mozilla-central/rev/0316704b960c