fix AWSY taskcluster nits

RESOLVED FIXED in Firefox 54

Status

RESOLVED FIXED
2 years ago
a year ago

People

(Reporter: jmaher, Assigned: pyang)

Tracking

Trunk
mozilla55
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox54 fixed, firefox55 fixed)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

2 years ago
from comment 71 in bug 1272113:
> > 
> > ::: taskcluster/ci/test/tests.yml:1255
> > (Diff revisions 4 - 6)
> > > -        no-read-buildbot-config: true
> > >          config:
> > >              by-test-platform:
> > >                  default:
> > >                      - awsy/linux_config.py
> > >                      - remove_executables.py
> > 
> > I am not sure if we need remove_executables.py here, I thought we added that
> > as a way to work around buildbot scripts that still needed to run.

We added this remove_executables.py so jobs that originally ran in buildbot could run in buildbot and taskcluster.  Since this doesn't run in buildbot, I wonder why we need this?  I don't want to just copy everything else.

> https://dxr.mozilla.org/mozilla-central/source/testing/mozharness/configs/
> remove_executables.py
> I doubt it's relative for buildbot; it seems to avoid download of
> minidump_stackwalk and nodejs.
> But it looks like a workaround since most of test applied this config. 
> Perhaps we can take it out but definitely need someone to provide more
> information.
>  
> > ::: testing/mozharness/scripts/awsy_script.py:34
> > (Diff revisions 4 - 6)
> > >          [["--e10s"],
> > >          {"action": "store_true",
> > >           "dest": "e10s",
> > >           "default": False,
> > >           "help": "Run tests with multiple processes. (Desktop builds only)",
> > >           }]
> > 
> > as a note, if we are only running in e10s, do we need this cli option? 
> > Possibly we do as this is needed to work smoothly with taskcluster.
> 
> Agreed this option confuses people.  We might need to work on all those
> options and make sure e10s is default.

this doesn't answer my question- do we need this option or not?  Since we are running in e10s only I would believe we do not need this option- do let me know if it is needed.



lets get these resolved soon!
(Assignee)

Comment 1

2 years ago
I have finds for this.

remove_executables.py actually purge exec in original config so we can simply take out items in exec and it should works fine.
try result showed in https://treeherder.mozilla.org/#/jobs?repo=try&revision=981199f5283f9b755e1ada99b2e0a403bda040b7

We probably need more explicit definitions for these config and find better ways to fix.

---

For e10s option,
I guess we discussed in https://bugzilla.mozilla.org/show_bug.cgi?id=1272113#c45
, and a try run log https://public-artifacts.taskcluster.net/SWfrtsfYRliqvnfTQyZ-6w/0/public/logs/live_backing.log (search "no such option: --e10s") shows it bumped error when we didn't provide e10s option.
Possible solutions mentioned in previous comments.
(Reporter)

Comment 2

2 years ago
can we default e10s=True?  I am glad we can easily delete the remove_executables.py line :)
(Assignee)

Comment 3

2 years ago
That is what we have in treeherder when using "-u all"; and e10s=True is certainly included in our last patch.
Furthermore, for taking out e10s options, I can give a poc to show how to work.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Comment 6

2 years ago
Comment on attachment 8849900 [details]
Bug 1347678 - enabling on linux-opt/linux64-opt

forgot to fix merge failure in bug1272113 so we actually didn't enable awsy on treeherder.
try result look good:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=51b5b6a08d6bb253e873ae1436cdc8b7ed4eec00
Attachment #8849900 - Flags: review?(jmaher)
(Assignee)

Updated

2 years ago
Assignee: nobody → pyang
(Assignee)

Comment 7

2 years ago
Comment on attachment 8849901 [details]
Bug 1347678 - removing dependency for remove_executables.py

remove_executables.py can be eliminated in tc's config.
try result showed in comment 1
Attachment #8849901 - Flags: review?(jmaher)
(Reporter)

Comment 8

2 years ago
mozreview-review
Comment on attachment 8849900 [details]
Bug 1347678 - enabling on linux-opt/linux64-opt

https://reviewboard.mozilla.org/r/122640/#review124820

just have one question, happy to flip to a r+ if this is expected.

::: taskcluster/ci/test/test-platforms.yml:38
(Diff revision 1)
>      test-sets:
>          -  linux32-tests
>          -  linux32-opt-tests
>          - external-media-tests
>          - external-media-tests-slow
>          - awsy

since we are enabling per push, is there value to get out of keeping this on the nightly scheduler?
Attachment #8849900 - Flags: review?(jmaher) → review-
(Reporter)

Comment 9

2 years ago
mozreview-review
Comment on attachment 8849901 [details]
Bug 1347678 - removing dependency for remove_executables.py

https://reviewboard.mozilla.org/r/122642/#review124824

looks good
Attachment #8849901 - Flags: review?(jmaher) → review+
(Assignee)

Comment 10

2 years ago
(In reply to Joel Maher ( :jmaher) from comment #8)
> Comment on attachment 8849900 [details]
> Bug 1347678 - enabling on linux-opt/linux64-opt
> 
> https://reviewboard.mozilla.org/r/122640/#review124820
> 
> just have one question, happy to flip to a r+ if this is expected.
> 
> ::: taskcluster/ci/test/test-platforms.yml:38
> (Diff revision 1)
> >      test-sets:
> >          -  linux32-tests
> >          -  linux32-opt-tests
> >          - external-media-tests
> >          - external-media-tests-slow
> >          - awsy
> 
> since we are enabling per push, is there value to get out of keeping this on
> the nightly scheduler?

I am not quite sure if it's redundant, and removing awsy in nightly test-set is ok for me.
Joel, if that makes sense I can update the patch.
Flags: needinfo?(jmaher)
(Reporter)

Comment 11

2 years ago
I think just scheduling it per push makes the most sense.  Ensure that is the case for linux32 and linux64.
Flags: needinfo?(jmaher)
Comment hidden (mozreview-request)
(Assignee)

Updated

2 years ago
Attachment #8849901 - Attachment is obsolete: true
Comment hidden (mozreview-request)
(Assignee)

Comment 14

2 years ago
Comment on attachment 8849900 [details]
Bug 1347678 - enabling on linux-opt/linux64-opt

- remove awsy in linux-night test-set
Attachment #8849900 - Flags: review?(jmaher)
(Assignee)

Comment 15

2 years ago
Comment on attachment 8849918 [details]
Bug 1347678 - removing dependency for remove_executables.py

carrying r+
Attachment #8849918 - Flags: review+
(Reporter)

Comment 16

2 years ago
mozreview-review
Comment on attachment 8849900 [details]
Bug 1347678 - enabling on linux-opt/linux64-opt

https://reviewboard.mozilla.org/r/122640/#review124848

thanks for the update
Attachment #8849900 - Flags: review?(jmaher) → review+
(Reporter)

Comment 17

2 years ago
mozreview-review
Comment on attachment 8849918 [details]
Bug 1347678 - removing dependency for remove_executables.py

https://reviewboard.mozilla.org/r/122662/#review124852
Attachment #8849918 - Flags: review+

Comment 18

2 years ago
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 620e90f6b818 -d adc2959fab79: rebasing 383201:620e90f6b818 "Bug 1347678 - enabling on linux-opt/linux64-opt r=jmaher"
merging taskcluster/ci/test/test-platforms.yml
warning: conflicts while merging taskcluster/ci/test/test-platforms.yml! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Comment 21

2 years ago
Comment on attachment 8849900 [details]
Bug 1347678 - enabling on linux-opt/linux64-opt

Commit rebased.
Result looks ok.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=1565644a43a8432b4e2a10bb75e95d723803075d

Carry review result
Attachment #8849900 - Flags: review+
(Assignee)

Comment 22

2 years ago
Comment on attachment 8849918 [details]
Bug 1347678 - removing dependency for remove_executables.py

Commit rebased.
Result looks ok.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=1565644a43a8432b4e2a10bb75e95d723803075d

Carry review result in comment 17
Attachment #8849918 - Flags: review+
(Assignee)

Comment 23

2 years ago
r+ from comment 16 and comment 17
Whiteboard: checkin-needed
(Assignee)

Updated

2 years ago
Keywords: checkin-needed
Whiteboard: checkin-needed

Comment 24

2 years ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/f54f6455a648
enabling on linux-opt/linux64-opt r=jmaher
https://hg.mozilla.org/integration/autoland/rev/78e8bd1ed7a3
removing dependency for remove_executables.py r=jmaher
Keywords: checkin-needed

Comment 25

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/f54f6455a648
https://hg.mozilla.org/mozilla-central/rev/78e8bd1ed7a3
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55

Updated

a year ago
Component: General → AWSY
You need to log in before you can comment on or make changes to this bug.