Closed Bug 1355180 Opened 7 years ago Closed 7 years ago

Make the default for run-on-projects for tests default to that for the parent build

Categories

(Firefox Build System :: Task Configuration, task)

Product:
Firefox Build System
Component:
Task Configuration
Type:
task
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED
Milestone:
mozilla55

People

(Reporter: dustin, Assigned: dustin)

References

Details

Attachments

(2 files, 3 obsolete files)

Bug 1355180: introduce run-on-projects: built-projects, and use it;
59 bytes, text/x-review-board-request
jmaher
: review+
Details
Bug 1355180: remove special-casing of stylo tests on try;
59 bytes, text/x-review-board-request
dustin
: review+
Details 
So if you have

someplatform:
  run-on-projects: ['oak']

and a test platform

platsomeform:  # mozilla doesn't spell things the same way twice, after all
  build-platform: someplatform
  test-sets:
    - common-tests

then someplatform will be built on all platforms, because most of the common-tests default to `run-on-projects: [all]`.  Thus even though the build isn't selected as a target, the tests are, and they require the build.

If, instead, those tests had something like `run-on-projects: build-platform-projects`, that could be translated (in this case) to [], and the tests wouldn't be targetted, so the build wouldn't either.

:ting, :jmaher, thoughts?
Flags: needinfo?(jmaher)
Flags: needinfo?(janus926)
I like this idea in general.  There are a lot of details and exceptions, but I think this will reduce a lot of confusion going forward.
Flags: needinfo?(jmaher)
Why don't we just skip untargetted build even if the tests are selected by [all]?
Flags: needinfo?(janus926)
See Also: → 1355359
Good question.  Thinking about that question a little, I see we could actually get some nice error messages for users:

build            test           result
-----            ----           ------
[oak]            [oak]          build and test only on oak
[all]            [oak]          build everywhere and test only on oak
[all]            [all]          build and test everywhere
[oak]            built-projects build and test only on oak
[oak]            [all]          error: tests without a build

For a test (in tests.yml), the default would be "built-projects".  However, it would help avoid this situation:

someplatform:
  run-on-projects: ['oak']

and a test

sometest:
  run-on-projects: ['oak', 'jamun']

which might be the first idea of someone interested in running `sometest` on jamun.  Currently, this will cause someplatform to "silently" build on jamun, even though only oak is listed.  With this change, the `./mach taskgraph` invocation would fail with an error saying that test-someplatform/sometest has tests but no build on the jamun project.
Comment on attachment 8859254 [details]
Bug 1355180: introduce run-on-projects: built-projects, and use it;

https://reviewboard.mozilla.org/r/131270/#review133842

this looks good- please ensure docs for taskcluster are up to date :)
Attachment #8859254 - Flags: review?(jmaher) → review+
Was there a particular piece of docs you're thinking of?  The change is documented in the schema (tests.py).
I am thinking of here:
https://dxr.mozilla.org/mozilla-central/source/taskcluster/docs/attributes.rst#22

and I believe there is a fancier site somewhere on the internet with similar information.
Oops, I forgot there was a stylo-specific followon I wanted to push.

I noticed that the stylo *builds* omit try, but the stylo-specific tests do not.  The result is that a push to try with -p all would include the tests, which require the builds -- so the builds would be run anyway.  Things get a little simpler in the test configuration if we just admit this and add "try" to the run-on-projects for the builds.
(In reply to Joel Maher ( :jmaher) from comment #8)
> I am thinking of here:
> https://dxr.mozilla.org/mozilla-central/source/taskcluster/docs/attributes.
> rst#22
> 
> and I believe there is a fancier site somewhere on the internet with similar
> information.

The 'built-projects' never appears as an attribute, so that would be a confusing place to write it down.  It just indicates that the test should copy its run_on_projects attribute from the build it depends on.  I could write it in `kinds.rst`, but it seems *way* more specific than the other content of that file.
Attachment #8859253 - Attachment is obsolete: true
Attachment #8859273 - Flags: review?(kmoir) → review+
Comment on attachment 8859273 [details]
Bug 1355180: remove special-casing of stylo tests on try;

https://reviewboard.mozilla.org/r/131286/#review133970

Kim, for some reason the r+ didn't register in mozreview.  While I have your attention, does this match your understanding of stylo -- that the stylo builds should occur on try with `-p all`?  I've made a try job linked to this review request with just such an option, if you want to take a look.
Comment on attachment 8859273 [details]
Bug 1355180: remove special-casing of stylo tests on try;

https://reviewboard.mozilla.org/r/131286/#review135376
I rebased and re-tested.  Still no diff.
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s 51511e61d599 -d 5d07afcca48a: rebasing 392257:51511e61d599 "Bug 1355180: introduce run-on-projects: built-projects, and use it; r=jmaher"
merging taskcluster/ci/test/tests.yml
warning: conflicts while merging taskcluster/ci/test/tests.yml! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)
Attachment #8859273 - Attachment is obsolete: true
Attachment #8862635 - Attachment is obsolete: true
Comment on attachment 8862642 [details]
Bug 1355180: remove special-casing of stylo tests on try;

https://reviewboard.mozilla.org/r/134492/#review137474

carry forward kim's r+
Attachment #8862642 - Flags: review+
Pushed by dmitchell@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3ff20d246999
introduce run-on-projects: built-projects, and use it; r=jmaher
https://hg.mozilla.org/integration/autoland/rev/3448d57a9be8
remove special-casing of stylo tests on try; r=dustin
Attachment #8862642 - Flags: review?(kmoir)
https://hg.mozilla.org/mozilla-central/rev/3ff20d246999
https://hg.mozilla.org/mozilla-central/rev/3448d57a9be8
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Product: TaskCluster → Firefox Build System
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: