1213798 - All tests running on mozilla-central/*-inbound should be running on try as well

Status: RESOLVED DUPLICATE of bug 1218919

(Taskcluster :: General, defect)

Description

[Edgar Chen [:edgar]]

Before landing a patch, we usually provide a try link to ensure we won't break anything. However, there are couple times that we thought the try result is good to land our code, but the patch was backouted because it broke some build/tests which is running only on mozilla-central/*-inbound, and we are unable to catch it by try runs.

IMO, All build/tests running on mozilla-central/*-inbound should be running on try as well.

Comment 1

[Hsin-Yi Tsai (she/her) [:hsinyi]]

(In reply to Edgar Chen [:edgar][:echen] from comment #0)
> Before landing a patch, we usually provide a try link to ensure we won't
> break anything. However, there are couple times that we thought the try
> result is good to land our code, but the patch was backouted because it
> broke some build/tests which is running only on mozilla-central/*-inbound,
> and we are unable to catch it by try runs.
> 
> IMO, All build/tests running on mozilla-central/*-inbound should be running
> on try as well.

I support this!

Comment 2

[Edgar Chen [:edgar]]

Attachment: WIP, Patch, v1

1. try/job_flags.yml inherits base_jobs.yml (just like mozilla-central/*-inbound).
2. Remove the task which is already included in base_jobs.yml
3. Leave the try specify configuration in try/job_flags.yml

Comment 3

[Edgar Chen [:edgar]]

But I got failure on gecko-decision, https://treeherder.mozilla.org/#/jobs?repo=try&revision=8ce6837e9798.

+ ./mach taskcluster-graph --pushlog-id=91966 --project=try '--message=try: -b do -p all -u all -t all' --owner=[e-mail] --revision-hash=308abf9e0411bb7e56a44ea68fa768addf493eeb --extend-graph
[taskcluster] Graph server error while extending task graph id K7krf_tbSfmHw01i86aKjA : Error: You didn't give the task-graph scopesallowing it define tasks on the queue., {"message":"You didn't give the task-graph scopesallowing it define tasks on the queue.","error":[{"message":"Authorization Failed","error":{"message":"Authorization Failed","error":{"info":"None of the scope-sets was satisfied","scopesets":[["docker-worker:image:taskclusterprivate/tester-device:0.0.7","queue:create-task:aws-provisioner-v1/testdroid-device","docker-worker:cache:tc-vcs","docker-worker:capability:device:phone"]]
...

Looks like something similar to bug 1210181.
Hi Greg, Could you help to take a look? Thank you.

Comment 4

[Greg Arndt [:garndt]]

This is one of the cases why we can't inherit from the same job flags file that integration branches do.  There are jobs that run on the other branches that make use of physical devices in our remote device lab that we currently do not want enabled on try pushes.  The scope error you're seeing is for "docker-worker:capability:device:phone" because we do not allow tasks to be created with that scope on try.

Comment 5

[Edgar Chen [:edgar]]

Attachment: WIP, Patch, v2

Thanks for your comments, Greg. Good to learn.

What about moving those jobs which need physical devices to {mozilla-central|*-inbound}/job_flags.yml, then try can also inherit base_jobs.yml.

Try runs for your reference: https://treeherder.mozilla.org/#/jobs?repo=try&revision=d373b054c1e1&exclusion_profile=false&group_state=expanded

Thank you.

Comment 6

[Greg Arndt [:garndt]]

Comment on attachment 8673038 [details] [diff] [review]
WIP, Patch, v2

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

I am a little hesitant to changing this, but I do see the need to try to ensure things always get enabled on try.  

However, this won't always guarantee that.  It will only guarantee that if a job is added to base_jobs.yml that it will be included on try, but what if a job was added to one of the branch specific job flag files (such as fx-team)? We will run into the same issue of a job possibly not being enabled on a try push.

I think that we should re-frame some of our views when reviewing patches to add jobs and always ask the question about what branches that job should be included on.  Especially when it's try vs. non-try.

We have been through a few rounds of changing where we inherit job flag definitions from, including something very similar to what is being proposed here.  We used to inherit from b2g-inbound, but then changed to inherit from m-c instead because we wanted to enable the physical device tests *only* on b2g-i.  Once we made that change things were fine until a day came where we wanted a specific type of image build only only m-c (I think either OTA or dogfood build).  Then the inheritance was changed to inherit from base_jobs rather than a branch.  At least I think that's what I"m remembering.  Either way, that could (and probably will at some point) happen in this case.

I think if anything, if there is a set of base jobs that should run on all branches including try, then perhaps everything just inherits directly from that file and adds it's own branch specific things in their own job_flags file.  But that again just tries to work around the mistake being made of a job not added to try.


For now this would be a f- because of the things I explained above, but I'm definitely open to trying to find a solution that meets everyone's needs.  If we have owners of each of these platforms and tests explain what definitely should be run on all branches (including try) and what shouldn't, perhaps we can create a base_jobs.yml that is much more complete that all branches inherit from (including try).