1200928 - [TaskCluster] Specify the options to use busybox in xpcshell tests

Status: RESOLVED FIXED

(Taskcluster :: Services, defect)

Description

[Edgar Chen [:edgar]]

Please see bug 1192135 comment #4 and bug 1192135 comment #5.

Comment 1

[Edgar Chen [:edgar]]

Attachment: WIP, Patch, v1

With this patch, we can use busybox for xpcshell correctly [1].
It works pretty good on ics. But we have another error on x86-kk/kk, 

> 02:21:45     INFO - Calling ['/home/worker/build/venv/bin/python', 'runtestsb2g.py', '--adbpath=/home/worker/build/emulator/b2g-distro/out/host/linux-x86/bin/adb', '--b2gpath=/home/worker/build/emulator/b2g-distro', '--emulator=x86', '--logdir=/home/worker/build', '--manifest=tests/xpcshell.ini', '--use-device-libs', '--testing-modules-dir=/home/worker/build/tests/modules', '--symbols-path=https://queue.taskcluster.net/v1/task/if789mYST52v_N9Y7Y7AaQ/artifacts/public/build/emulator.crashreporter-symbols.zip', '--busybox=/home/worker/build/busybox', '--total-chunks=20', '--this-chunk=1', '--log-raw=/home/worker/build/blobber_upload_dir/xpcshell_raw.log', '--log-errorsummary=/home/worker/build/blobber_upload_dir/xpcshell_errorsummary.log'] with output_timeout 1000
> 02:22:23     INFO -  pushing /system/bin/busybox
> 02:22:23     INFO -  Traceback (most recent call last):
> 02:22:23     INFO -    File "runtestsb2g.py", line 228, in <module>
> 02:22:23     INFO -      main()
> 02:22:23     INFO -    File "runtestsb2g.py", line 221, in main
> 02:22:23     INFO -      run_remote_xpcshell(parser, options, args, log)
> 02:22:23     INFO -    File "runtestsb2g.py", line 183, in run_remote_xpcshell
> 02:22:23     INFO -      marionette = Marionette(**kwargs)
> 02:22:23     INFO -    File "/home/worker/build/venv/local/lib/python2.7/site-packages/marionette_driver/marionette.py", line 628, in __init__
> 02:22:23     INFO -      self.emulator.install_busybox(busybox=busybox)
> 02:22:23     INFO -    File "/home/worker/build/venv/local/lib/python2.7/site-packages/mozrunner/devices/base.py", line 170, in install_busybox
> 02:22:23     INFO -      self.dm._verifyZip()
> 02:22:23     INFO -    File "/home/worker/build/venv/local/lib/python2.7/site-packages/mozdevice/devicemanagerADB.py", line 721, in _verifyZip
> 02:22:23     INFO -      raise DMError("zip not available")
> 02:22:23     INFO -  mozdevice.devicemanager.DMError: zip not available
> 02:22:29     INFO - Return code: 1

[1] https://treeherder.allizom.org/#/jobs?repo=try&revision=85ae9665a1f7

Comment 2

[Edgar Chen [:edgar]]

Okay, I just found some clues. When running xpcshell locally, the mach system use different version of busybox for arm and x86 [1]. But in Taskcluster, both arm and x86 emulator share the same binary [2], this probably can explain why the busybox doesn't work on x86-kk.

[1] https://dxr.mozilla.org/mozilla-central/source/testing/xpcshell/mach_commands.py#38-41
[2] http://hg.mozilla.org/build/mozharness/file/f5cb8461d40c/configs/b2g/emulator_automation_config.py#l7

Comment 3

[John Dai[:jdai]]

After I use different version of busybox for arm and x86 [1], Taskcluster still has comment #1 issue [2].

[1] https://dxr.mozilla.org/mozilla-central/source/testing/xpcshell/mach_commands.py#38-41
[2] https://treeherder.allizom.org/#/jobs?repo=try&revision=f17b79e039ea

Comment 4

[John Dai[:jdai]]

(In reply to John Dai[:johnz][:jdai] from comment #3)
> After I use different version of busybox for arm and x86 [1], Taskcluster
> still has comment #1 issue [2].
> 
> [1]
> https://dxr.mozilla.org/mozilla-central/source/testing/xpcshell/
> mach_commands.py#38-41
> [2] https://treeherder.allizom.org/#/jobs?repo=try&revision=f17b79e039ea

I think this is right direction, because I saw many of x86 testcases pass.[1]
But I got another error message:

[1] https://treeherder.allizom.org/#/jobs?repo=try&revision=f17b79e039ea

> 10:06:44     INFO - Calling ['/home/worker/build/venv/bin/python', 'runtestsb2g.py', '--adbpath=/home/worker/build/emulator/b2g-distro/out/host/linux-x86/bin/adb', '--b2gpath=/home/worker/build/emulator/b2g-distro', '--emulator=x86', '--logdir=/home/worker/build', '--manifest=tests/xpcshell.ini', '--use-device-libs', '--testing-modules-dir=/home/worker/build/tests/modules', '--symbols-path=https://queue.taskcluster.net/v1/task/qNUgSNmORNiMpg9zSX17cQ/artifacts/public/build/emulator.crashreporter-symbols.zip', '--busybox=/home/worker/build/busybox', '--total-chunks=20', '--this-chunk=11', '--log-raw=/home/worker/build/blobber_upload_dir/xpcshell_raw.log', '--log-errorsummary=/home/worker/build/blobber_upload_dir/xpcshell_errorsummary.log'] with output_timeout 1000
> 10:07:42     INFO -  pushing /system/bin/busybox
> 10:07:42     INFO -  Traceback (most recent call last):
> 10:07:42     INFO -    File "runtestsb2g.py", line 228, in <module>
> 10:07:42     INFO -      main()
> 10:07:42     INFO -    File "runtestsb2g.py", line 221, in main
> 10:07:42     INFO -      run_remote_xpcshell(parser, options, args, log)
> 10:07:42     INFO -    File "runtestsb2g.py", line 183, in run_remote_xpcshell
> 10:07:42     INFO -      marionette = Marionette(**kwargs)
> 10:07:42     INFO -    File "/home/worker/build/venv/local/lib/python2.7/site-packages/marionette_driver/marionette.py", line 628, in __init__
> 10:07:42     INFO -      self.emulator.install_busybox(busybox=busybox)
> 10:07:42     INFO -    File "/home/worker/build/venv/local/lib/python2.7/site-packages/mozrunner/devices/base.py", line 162, in install_busybox
> 10:07:42     INFO -      self.dm.pushFile(busybox, self.app_ctx.remote_busybox, retryLimit=10)
> 10:07:42     INFO -    File "/home/worker/build/venv/local/lib/python2.7/site-packages/mozdevice/devicemanagerADB.py", line 216, in pushFile
> 10:07:42     INFO -      raise DMError("Error pushing file %s -> %s; output: %s" % (localname, destname, proc.output))
> 10:07:42     INFO -  mozdevice.devicemanager.DMError: Error pushing file /home/worker/build/busybox -> /system/bin/busybox; output: ["failed to copy '/home/worker/build/busybox' to '/system/bin/busybox': Read-only file system"]
> 10:07:46     INFO - Return code: 1

Comment 5

[Edgar Chen [:edgar]]

Attachment: WIP, patch, v2

Comment 6

[Edgar Chen [:edgar]]

Attachment: WIP, patch, v3

Comment 7

[Edgar Chen [:edgar]]

Comment on attachment 8674826 [details] [diff] [review]
WIP, patch, v3

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

In order to use busybox on both arm and x86, we need two different binary for them, I extend the option that can accept multiple busybox url and choose the suitable one according to the current emulator type. Hi ahal, I would like to have some feedback from you before providing a formal patch for review. Thank you.

Comment 8

[Andrew Halberstadt [:ahal]]

Comment on attachment 8674826 [details] [diff] [review]
WIP, patch, v3

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

Could we instead create a new config for emulator-x86 that overrides the 'busybox_url' key in 'emulator_automation_config.py'? I sort of suspect we'll want separate configs anyway, as there will probably be other things that are different as well.

::: testing/mozharness/scripts/b2g_emulator_unittest.py
@@ +35,5 @@
>           "default": "browser",
>           "help": "The type of tests to run",
>           }
>      ], [
> +        ["--busybox-urls"],

There isn't really a very good way of specifying a dictionary from the command line. So if someone actually tried to use this, it would fail.

Comment 9

[Edgar Chen [:edgar]]

(In reply to Andrew Halberstadt [:ahal] from comment #8)
> Comment on attachment 8674826 [details] [diff] [review]
> WIP, patch, v3
> 
> Review of attachment 8674826 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Could we instead create a new config for emulator-x86 that overrides the
> 'busybox_url' key in 'emulator_automation_config.py'? I sort of suspect
> we'll want separate configs anyway, as there will probably be other things
> that are different as well.

Thanks for the feedback. Overrides the 'busybox_url' sounds good.
But it seems to me that we won't really need to separate the config, because most of config are shared between emulator-x86 and arm. And since emulator-x86 is only enabled on TC, we could generate busybox_url from taskcluster-graph and override the one in 'emulator_automation_config.py' instead. Will attach WIP patch soon.

Comment 10

[Edgar Chen [:edgar]]

Attachment: MozReview Request: Bug 1200928 - Part 2: [Taskcluster] Use correct busybox binary for x86/arm emulator when running xpcshell tests

Bug 1200928 - Part 1: Iterate over all the things in task.extra.locations and build out the test_parameter

Comment 11

[Edgar Chen [:edgar]]

Attachment: MozReview Request: Bug 1200928 - Part 2: [Taskcluster] Use correct busybox binary for x86/arm emulator when running xpcshell tests

Bug 1200928 - Part 2: [Taskcluster] Use correct busybox binary for x86/arm emulator when running xpcshell tests

Comment 12

[Edgar Chen [:edgar]]

Comment on attachment 8677407 [details]
MozReview Request: Bug 1200928 - Part 2: [Taskcluster] Use correct busybox binary for x86/arm emulator when running xpcshell tests

Bug 1200928 - Part 1: Iterate over all the things in task.extra.locations and build out the test_parameter

Comment 13

[Edgar Chen [:edgar]]

Comment on attachment 8677408 [details]
MozReview Request: Bug 1200928 - Part 2: [Taskcluster] Use correct busybox binary for x86/arm emulator when running xpcshell tests

Bug 1200928 - Part 2: [Taskcluster] Use correct busybox binary for x86/arm emulator when running xpcshell tests

Comment 14

[Edgar Chen [:edgar]]

Comment on attachment 8677407 [details]
MozReview Request: Bug 1200928 - Part 2: [Taskcluster] Use correct busybox binary for x86/arm emulator when running xpcshell tests

Bug 1200928 - Part 1: Iterate over all the things in task.extra.locations and build out the test_parameter

Comment 15

[Edgar Chen [:edgar]]

Comment on attachment 8677408 [details]
MozReview Request: Bug 1200928 - Part 2: [Taskcluster] Use correct busybox binary for x86/arm emulator when running xpcshell tests

Bug 1200928 - Part 2: [Taskcluster] Use correct busybox binary for x86/arm emulator when running xpcshell tests

Comment 16

[Edgar Chen [:edgar]]

(In reply to Edgar Chen [:edgar][:echen] from comment #9)
> (In reply to Andrew Halberstadt [:ahal] from comment #8)
> > Comment on attachment 8674826 [details] [diff] [review]
> > WIP, patch, v3
> > 
> > Review of attachment 8674826 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > Could we instead create a new config for emulator-x86 that overrides the
> > 'busybox_url' key in 'emulator_automation_config.py'? I sort of suspect
> > we'll want separate configs anyway, as there will probably be other things
> > that are different as well.
> 
> Thanks for the feedback. Overrides the 'busybox_url' sounds good.
> But it seems to me that we won't really need to separate the config, because
> most of config are shared between emulator-x86 and arm. And since
> emulator-x86 is only enabled on TC, we could generate busybox_url from
> taskcluster-graph and override the one in 'emulator_automation_config.py'
> instead. Will attach WIP patch soon.

Hi Greg, I would like to see if this changes is okay to you before sending the review request (BTW, I addressed bug 1188330 comment 45 in patch part 1). Thank you.

Comment 17

[Greg Arndt [:garndt]]

https://reviewboard.mozilla.org/r/22937/#review22657

::: testing/taskcluster/mach_commands.py:470
(Diff revision 3)
> +            if 'emulator' in build['build_name']:

I think that perhaps maybe this should just be in the task definition for emulators rather than hacking something into the mach target.  Do you think that might work out?

Comment 18

[Greg Arndt [:garndt]]

https://reviewboard.mozilla.org/r/22935/#review22655

::: testing/taskcluster/mach_commands.py:464
(Diff revision 3)
> -            if 'tests' in build_task['task']['extra']['locations']:
> +            build_parameters['build_url'] = location_urls['build']

it looks like we are not adding mozharness_url back to the build_parameters unless I'm missing it.

Perhaps were would not be an issue with adding all location urls to the build parameters, and then I dont' think you need to iterate over them lower down where the test tasks are rendered.

Comment 19

[Greg Arndt [:garndt]]

Left a couple of comments.

Comment 20

[Edgar Chen [:edgar]]

https://reviewboard.mozilla.org/r/22937/#review22657

> I think that perhaps maybe this should just be in the task definition for emulators rather than hacking something into the mach target.  Do you think that might work out?

I will try add them in task definition. Thank you.

Comment 21

[Edgar Chen [:edgar]]

Comment on attachment 8677407 [details]
MozReview Request: Bug 1200928 - Part 2: [Taskcluster] Use correct busybox binary for x86/arm emulator when running xpcshell tests

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/22935/diff/3-4/

Comment 22

[Edgar Chen [:edgar]]

Comment on attachment 8677408 [details]
MozReview Request: Bug 1200928 - Part 2: [Taskcluster] Use correct busybox binary for x86/arm emulator when running xpcshell tests

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/22937/diff/3-4/

Comment 23

[Edgar Chen [:edgar]]

Hi Greg,
I have addressed the comment #17 and comment #18, could you take a look again?
- Add all location urls to the build parameters.
- Put busybox url in build task definition.
- Try result for your reference: https://treeherder.mozilla.org/#/jobs?repo=try&revision=63668400190f&group_state=expanded&exclusion_profile=false

BTW, according to the policy: "Must not rely on resources from sites whose content we do not control/have no SLA" [1], where should we put the busybox binary for x86, tooltool or somewhere in taskcluster? Could you also kindly help me on this or do you know who can help/answer this?

Thank you.

[1] https://wiki.mozilla.org/Sheriffing/Job_Visibility_Policy#Must_avoid_patterns_known_to_cause_non_deterministic_failures
[2] https://dxr.mozilla.org/mozilla-central/source/testing/mozharness/configs/b2g/emulator_automation_config.py#7

Comment 24

[Greg Arndt [:garndt]]

Thanks, I don't see anything obviously broken with the try push in comment 23.  Looks like the same ol' stuff we usually see.

You're absolutely right, we should not rely on an external resource like that.  That will almost certainly cause some issues at some point.  

I'm not sure how busybox is used or what versions are required, but if we can pin down a version that works for your needs, it should be put on tooltool just like the other busybox binary and referenced that way.

Comment 25

[Edgar Chen [:edgar]]

Attachment: busybox-i686 (1.21.1)

(In reply to Greg Arndt [:garndt] from comment #24)
> I'm not sure how busybox is used or what versions are required, but if we
> can pin down a version that works for your needs, it should be put on
> tooltool just like the other busybox binary and referenced that way.

I attach the latest version (1.21.1) which works for us.
I tried to put it on tooltool [1], but it seems I don't have permission to perform upload. Could you help to upload it and share the link to me.

Thanks a again.

[1] https://api.pub.build.mozilla.org/tooltool/

Comment 26

[Edgar Chen [:edgar]]

ni for comment #25.

Comment 27

[Greg Arndt [:garndt]]

Hrm, I've never uploaded to tooltool either.  Someone in #releng might know what it takes to get it on the pub site.  Let me know if I can help out with that at all.

Comment 28

[Andrew Halberstadt [:ahal]]

Hey Jordan, do you mind helping out getting the x86 busybox binary uploaded to tooltool? I'm not sure what the process is these days either.

We'll also need to add it to the mozharness config for the emu-x86 job like here:
https://dxr.mozilla.org/mozilla-central/source/testing/mozharness/configs/b2g/emulator_automation_config.py#7

Comment 29

[Jordan Lund (:jlund)]

checking the docs, this is slightly more self serve: https://wiki.mozilla.org/ReleaseEngineering/Applications/Tooltool#How_To_Upload_To_Tooltool

from the above docs, what you need to do is file a bug to get permission to create tokens for uploading to tooltool: https://api.pub.build.mozilla.org/tokenauth/ so you can generate a token. Something like: https://bugzilla.mozilla.org/show_bug.cgi?id=1201730

I've done it this time just to speed it up for you.

You can use the following manifest in your mozharness config so your script can tell the tooltool server what and how to download busybox:

[
{
"size": 883644,
"visibility": "public",
"digest": "227d3189be1eb1e5348255ca100168bae8b3c4d3664384ea3c832fc044d168fd594f0b15991d190eceb1fb9f5b3463912104bcd7d4d0dbdd88d3f4b7187ea73d",
"algorithm": "sha512",
"filename": "busybox-i686"
}
]

Comment 30

[Edgar Chen [:edgar]]

Comment on attachment 8677407 [details]
MozReview Request: Bug 1200928 - Part 2: [Taskcluster] Use correct busybox binary for x86/arm emulator when running xpcshell tests

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/22935/diff/4-5/

Comment 31

[Edgar Chen [:edgar]]

Attachment: MozReview Request: Bug 1200928 - Part 1: Iterate over all the things in task.extra.locations and build out the test_parameter

Bug 1200928 - Part 1: Iterate over all the things in task.extra.locations and build out the test_parameter

Comment 32

[Edgar Chen [:edgar]]

Try result: https://treeherder.mozilla.org/#/jobs?repo=try&revision=de344887d1d1&group_state=expanded

Comment 33

[Edgar Chen [:edgar]]

Comment on attachment 8698307 [details]
MozReview Request: Bug 1200928 - Part 1: Iterate over all the things in task.extra.locations and build out the test_parameter

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/27939/diff/1-2/

Comment 34

[Edgar Chen [:edgar]]

Comment on attachment 8677407 [details]
MozReview Request: Bug 1200928 - Part 2: [Taskcluster] Use correct busybox binary for x86/arm emulator when running xpcshell tests

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/22935/diff/5-6/

Comment 35

[Greg Arndt [:garndt]]

Comment on attachment 8698307 [details]
MozReview Request: Bug 1200928 - Part 1: Iterate over all the things in task.extra.locations and build out the test_parameter

https://reviewboard.mozilla.org/r/27939/#review25055

Comment 36

[Greg Arndt [:garndt]]

Comment on attachment 8677407 [details]
MozReview Request: Bug 1200928 - Part 2: [Taskcluster] Use correct busybox binary for x86/arm emulator when running xpcshell tests

https://reviewboard.mozilla.org/r/22935/#review25053

Just a couple of comments that need to be addressed.  At a minimum I think having busybox moved under task.extras.locations is ok, and we can remove that special code in the mach target for just the one url.  Also, consider just putting it directly in the one task that needs it defined.  If we have more that will need busybox url we can always change that decision.

::: testing/taskcluster/tasks/builds/b2g_emulator_base.yml:38
(Diff revision 5)
> +    url:

Hrm, is there a reason why this couldn't just be under task.extra.locations.busybox?

::: testing/taskcluster/tasks/tests/b2g_emulator_xpcshell_chunked.yml:23
(Diff revision 5)
> +        --busybox-url {{busybox_url}}

If this is the only place where busybox_url is used in any of the task definitions, why not just put the URL within this definition directly instead of needing special code for handling task.extra.url in the mach target?  Also, see my other comment about maybe placing it with the other URLs if we continue to keep it defined in the build tasks.

Comment 37

[Edgar Chen [:edgar]]

https://reviewboard.mozilla.org/r/22935/#review25053

> Hrm, is there a reason why this couldn't just be under task.extra.locations.busybox?

We iterate all the item under task.extras.locations to consturct real URL with ARTIFACT_URL [1].
If we put busybox under task.extras.locations, we still need a special code to prevent constructing busybox which is already an url.
So I add `url` to put the pure url address.

[1] https://dxr.mozilla.org/mozilla-central/source/testing/taskcluster/mach_commands.py#426

> If this is the only place where busybox_url is used in any of the task definitions, why not just put the URL within this definition directly instead of needing special code for handling task.extra.url in the mach target?  Also, see my other comment about maybe placing it with the other URLs if we continue to keep it defined in the build tasks.

We need to handle `busybox_url` in mach target is because different arch type (arm or x86) needs different busybox binary and they share the same test definition.

Comment 38

[Greg Arndt [:garndt]]

Ok, thanks for clarifying.  Someting about it seems off, but not enough that I think this should be blocked.  We can always make it better later.  Thanks.

Comment 39

[Pulsebot]

https://hg.mozilla.org/integration/b2g-inbound/rev/8284bea565d7
https://hg.mozilla.org/integration/b2g-inbound/rev/77ab820ce477

Comment 40

[Edgar Chen [:edgar]]

Thanks for the help, Greg, Andrew and Jordan.

Comment 41

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/8284bea565d7
https://hg.mozilla.org/mozilla-central/rev/77ab820ce477

Comment 44

[Pete Moore [:pmoore][:pete]]

Moving closed bugs across to new Bugzilla product "TaskCluster".