Cannot detect x86 type when running marionette-webapi tests for emu-x86-kk on taskcluster

RESOLVED FIXED

Status

Taskcluster
General
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: hsinyi, Assigned: jdai)

Tracking

(Blocks: 1 bug)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

3 years ago
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

3 years ago
Assignee: nobody → jdai
(Reporter)

Updated

3 years ago
Blocks: 1144970
(Assignee)

Comment 1

3 years ago
Created attachment 8604022 [details] [diff] [review]
Marionette-webapi detects the emulator type implicitly.

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 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

3 years ago
Created attachment 8604535 [details] [diff] [review]
Marionette-webapi detects the emulator type implicitly. (V2)

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 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)
Attachment #8604022 - Attachment is obsolete: false
Attachment #8604022 - Flags: review- → review+
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

3 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

3 years ago
Created attachment 8605135 [details] [diff] [review]
Marionette-webapi detects the emulator type implicitly. (V3)

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 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

3 years ago
Hi John,
Do you have any update? Is the patch good for checkin? Thanks!
Flags: needinfo?(jdai)
(Assignee)

Comment 10

3 years ago
Hi Ryan,
Could you help me to checkin? Thanks!
Flags: needinfo?(jdai) → needinfo?(ryanvm)
Keywords: checkin-needed
For future reference, just setting checkin-needed is enough :)
Flags: needinfo?(ryanvm)
I'll be bumping the mozharness revision in m-c soon for another bug anyway. Will close this out when that happens.
Mozharness version bump recently landed on central, this should be live.
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Component: TaskCluster → General
Product: Testing → Taskcluster
Target Milestone: --- → mozilla41
Version: unspecified → Trunk
Resetting Version and Target Milestone that accidentally got changed...
Target Milestone: mozilla41 → ---
Version: Trunk → unspecified

Updated

3 years ago
Blocks: 1212262
You need to log in before you can comment on or make changes to this bug.