Closed
Bug 1162831
Opened 10 years ago
Closed 10 years ago
Cannot detect x86 type when running marionette-webapi tests for emu-x86-kk on taskcluster
Categories
(Taskcluster :: General, defect)
Taskcluster
General
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: hsinyi, Assigned: jdai)
References
(Blocks 1 open bug)
Details
Attachments
(1 file, 2 obsolete files)
3.60 KB,
patch
|
ahal
:
review+
|
Details | Diff | Splinter Review |
With bug 1153774, it correctly detects emulator type when running xpcshell or mochitest on taskcluster but not when running marionette-webapi.
According to the log in the below link, the emulator type being used was still 'arm' even marionette-webapi tests were run on emu-x86-kk.
https://treeherder.allizom.org/#/jobs?repo=try&revision=e33743cfaa22
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → jdai
Reporter | ||
Updated•10 years ago
|
Blocks: emu-x86-kk-tests
Assignee | ||
Comment 1•10 years ago
|
||
Test cluster result:
[1]https://treeherder.allizom.org/#/jobs?repo=try&revision=40947e13daa0
Mozharness detects the emulator 'x86' type log:
[2]https://s3-us-west-2.amazonaws.com/taskcluster-public-artifacts/M0jbld6vTWuEv6DLQmQUXQ/0/public/logs/live_backing.log
Attachment #8604022 -
Flags: review?(ahalberstadt)
Comment 2•10 years ago
|
||
Comment on attachment 8604022 [details] [diff] [review]
Marionette-webapi detects the emulator type implicitly.
Review of attachment 8604022 [details] [diff] [review]:
-----------------------------------------------------------------
Looks good, but please address the comment.
::: scripts/marionette.py
@@ +349,5 @@
> 'xml_output': os.path.join(dirs['abs_work_dir'], 'output.xml'),
> 'html_output': os.path.join(dirs['abs_blob_upload_dir'], 'output.html'),
> 'logcat_dir': dirs['abs_work_dir'],
> + 'emulator': 'x86' if os.path.join(dirs['abs_b2g-distro_dir'], 'out',
> + 'target', 'product', 'generic_x86') else 'arm',
This makes it so --emulator is ignored even if explicitly set. We should either only do this if self.config.get('emulator') is None, or remove --emulator altogether. The latter *may* break some jobs though.
Attachment #8604022 -
Flags: review?(ahalberstadt) → review-
Assignee | ||
Comment 3•10 years ago
|
||
Hi Andrew,
I addressed the review comment #2. Could you take a look again? Thank you.
Test cluster result:
https://treeherder.allizom.org/#/jobs?repo=try&revision=bcfb04b06ccc
Mozharness detects the emulator 'x86' type log:
https://s3-us-west-2.amazonaws.com/taskcluster-public-artifacts/kPGcK7OARum3l4oRXW-JXg/0/public/logs/live_backing.log
Attachment #8604022 -
Attachment is obsolete: true
Attachment #8604535 -
Flags: review?(ahalberstadt)
Comment 4•10 years ago
|
||
Comment on attachment 8604535 [details] [diff] [review]
Marionette-webapi detects the emulator type implicitly. (V2)
Sorry, I wasn't thinking properly in the first review. So this looks good, but it will fail unless you remove the references to --emulator in:
https://dxr.mozilla.org/mozilla-central/source/testing/config/mozharness/marionette.py
That means it will be tricky to land because we need to land this and the m-c patch at the same time. What we should actually do is:
1) Land your original patch and wait for it to merge
2) Remove --emulator from in-tree configs
3) Remove --emulator in the mozharness script
I'll help out.
Attachment #8604535 -
Attachment is obsolete: true
Attachment #8604535 -
Flags: review?(ahalberstadt)
Updated•10 years ago
|
Attachment #8604022 -
Attachment is obsolete: false
Attachment #8604022 -
Flags: review- → review+
Comment 5•10 years ago
|
||
Comment on attachment 8604022 [details] [diff] [review]
Marionette-webapi detects the emulator type implicitly.
Review of attachment 8604022 [details] [diff] [review]:
-----------------------------------------------------------------
::: scripts/marionette.py
@@ +348,5 @@
> 'profile': os.path.join(dirs['abs_gaia_dir'], 'profile'),
> 'xml_output': os.path.join(dirs['abs_work_dir'], 'output.xml'),
> 'html_output': os.path.join(dirs['abs_blob_upload_dir'], 'output.html'),
> 'logcat_dir': dirs['abs_work_dir'],
> + 'emulator': 'x86' if os.path.join(dirs['abs_b2g-distro_dir'], 'out',
Actually looking at this there's a bug here I missed the first time. os.path.join doesn't test for existence, so this will always return True. It needs to be:
os.path.isdir(os.path.join(...))
Attachment #8604022 -
Flags: review+ → review-
Assignee | ||
Comment 6•10 years ago
|
||
(In reply to Andrew Halberstadt [:ahal] from comment #4)
> Comment on attachment 8604535 [details] [diff] [review]
> Marionette-webapi detects the emulator type implicitly. (V2)
>
> Sorry, I wasn't thinking properly in the first review. So this looks good,
> but it will fail unless you remove the references to --emulator in:
> https://dxr.mozilla.org/mozilla-central/source/testing/config/mozharness/
> marionette.py
>
> That means it will be tricky to land because we need to land this and the
> m-c patch at the same time. What we should actually do is:
>
> 1) Land your original patch and wait for it to merge
> 2) Remove --emulator from in-tree configs
> 3) Remove --emulator in the mozharness script
>
> I'll help out.
I did 2) and 3) on test cluster. It gave me "output_timeout 1000" and "must specify --binary, --emulator or --address"[1].
[1]https://s3-us-west-2.amazonaws.com/taskcluster-public-artifacts/2tNCHNVPTbOvHPR7-92Nlg/0/public/logs/live_backing.log
Assignee | ||
Comment 7•10 years ago
|
||
Hi Andrew,
I addressed the review comment #5. Could you take a look again? Thank you.
Test cluster result:
https://treeherder.allizom.org/#/jobs?repo=try&revision=0f12da4f2973
Mozharness detects the emulator 'x86' type log:
https://s3-us-west-2.amazonaws.com/taskcluster-public-artifacts/CcCzcKm1RmOkGIgXp43EZg/0/public/logs/live_backing.log
Attachment #8604022 -
Attachment is obsolete: true
Attachment #8605135 -
Flags: review?(ahalberstadt)
Comment 8•10 years ago
|
||
Comment on attachment 8605135 [details] [diff] [review]
Marionette-webapi detects the emulator type implicitly. (V3)
Review of attachment 8605135 [details] [diff] [review]:
-----------------------------------------------------------------
Great looks good!
Yeah, in order to remove the --emulator parameter from the in-tree configs, this first needs to land on mozharness, then the in-tree pointer needs to get updated to a revision that includes this change. It's a multi-step process. Luckily as soon as this happens the mozharness scripts will be doing what you want. The rest is just follow up cleanup.
Attachment #8605135 -
Flags: review?(ahalberstadt) → review+
Comment 9•10 years ago
|
||
Hi John,
Do you have any update? Is the patch good for checkin? Thanks!
Flags: needinfo?(jdai)
Assignee | ||
Comment 10•10 years ago
|
||
Hi Ryan,
Could you help me to checkin? Thanks!
Flags: needinfo?(jdai) → needinfo?(ryanvm)
Keywords: checkin-needed
Comment 11•10 years ago
|
||
For future reference, just setting checkin-needed is enough :)
Flags: needinfo?(ryanvm)
Comment 12•10 years ago
|
||
Keywords: checkin-needed
Comment 13•10 years ago
|
||
mozharness production tag moved to: https://hg.mozilla.org/build/mozharness/rev/production
Comment 14•10 years ago
|
||
mozharness production tag moved to: https://hg.mozilla.org/build/mozharness/rev/production
Comment 15•10 years ago
|
||
I'll be bumping the mozharness revision in m-c soon for another bug anyway. Will close this out when that happens.
Comment 16•10 years ago
|
||
Mozharness version bump recently landed on central, this should be live.
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Updated•10 years ago
|
Component: TaskCluster → General
Product: Testing → Taskcluster
Target Milestone: --- → mozilla41
Version: unspecified → Trunk
Comment 17•10 years ago
|
||
Resetting Version and Target Milestone that accidentally got changed...
Target Milestone: mozilla41 → ---
Version: Trunk → unspecified
You need to log in
before you can comment on or make changes to this bug.
Description
•