1426254 - SCHEDULES.inclusive test suites don't work

Status: RESOLVED FIXED

(Firefox Build System :: Task Configuration, task)

Description

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

In bug 1403322, I tried to implement test-verify, jittest, and jsreftest as inclusive test suites, meaning that they don't run by default, but run when files that feed into them change.  However, bug 1410911 shows that this didn't work, and gbrown and ahal correctly identified the cause.

For a file which should trigger test-verify, for example foo/bar.js, SCHEDULES is the full set of exclusive components, plus "test-verify".  This makes sense: test-verify might be affected by that file, but so might any other test suite on any platform.

The test-verify job on, say, android has skip-unless-schedules = ['android', 'test-verify'].  That's the issue: that job doesn't need to run any time a file possibly affecting android is modified (that's an exclusive component, so *most* files might affect android!).

I think we need to think of some test suites as "inclusive" and others as "exclusive".  We can either:
 * be explicit in the tests/*.yml and cross-reference that against whether those suite names appear as inclusive or exclusive components; or
 * allow the component choices to implicitly determine whether a suite is inclusive or exclusive

I'm not sure which would cause less confusion down the road..

Comment 1

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

Attachment: Bug 1426254: only list a single component for "inclusive" test suites;

For example, jittests will be an inclusive test suite; all files which might
affect the suite are tagged with
  SCHEDULES.inclusive += ['jittest']
but those files usually also schedule all of the exclusive components (including
the platform families android, linux, macosx, and windows).  This makes sense:
those files could potentially affect any other test suite on any platform too.

But the jittest job on Android, for example, needs to run only if the jittest
component is scheduled -- it does not need to just because something
Android-related changed.  So its optimization should be {skip-unless-schedules:
['jittest']}, not {skip-unless-schedules: ['jittest', 'android']}.

This fix "figures out" the distinction by looking at what kind of component the
test suite is.  Maybe that is too magic, and we should also have to write
"component: implicit" in the tests/*.yml file.

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

Comment 2

[Andrew Halberstadt [:ahal]]

Comment on attachment 8937879 [details]
Bug 1426254: only list a single component for "inclusive" test suites;

https://reviewboard.mozilla.org/r/208560/#review214456

I think this makes sense. We'll have to do something similar for the lint tasks when we move those over to SCHEDULES.inclusive.

::: taskcluster/taskgraph/transforms/tests.py:993
(Diff revision 1)
> +            if suite in INCLUSIVE_COMPONENTS:
> +                # if this is an "inclusive" test, then all files which might
> +                # cause it to run are annotated with SCHEDULES in moz.build,
> +                # so do not include the platform or any other components here
> +                schedules = [suite]
> +            else:
> +                schedules = [suite, platform_family(test['build-platform'])]

How will this work if we ever want to have more than one inclusive component? Will we ever have more than one?

It would be nice if there were two lists, `inclusive` and `exclusive`, and adding a future schedules component was as easy as appending to the proper list.

This a change we could make when needed though, so feel free to treat it as optional.

Comment 3

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

I don't think a test suite will ever have more than one inclusive component.

The separate lists are on the files, rather than on the jobs.  I think having exclusive/inclusive on both ends would be redundant.

Comment 4

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

Comment on attachment 8937879 [details]
Bug 1426254: only list a single component for "inclusive" test suites;

https://reviewboard.mozilla.org/r/208560/#review214880

::: taskcluster/taskgraph/transforms/tests.py:993
(Diff revision 1)
> +            if suite in INCLUSIVE_COMPONENTS:
> +                # if this is an "inclusive" test, then all files which might
> +                # cause it to run are annotated with SCHEDULES in moz.build,
> +                # so do not include the platform or any other components here
> +                schedules = [suite]
> +            else:
> +                schedules = [suite, platform_family(test['build-platform'])]

I think this whole patch is pretty test-specific.

Lint tasks will be easy (they don't have a platform, so they only have the one inclusive component).  I'm not sure what a test suite with multiple inclusive components would look like.  I guess we would modify that if and when such a thing is invented.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0e5cfb917a48
only list a single component for "inclusive" test suites; r=ahal

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/0e5cfb917a48