:gerard-majax Reporter
	Description • 9 years ago

We need to change the name of the binary for at least Gu: https://treeherder.mozilla.org/#/jobs?repo=try&revision=f66af40f9591

That would be much easier to work on with intree mozharness.

	Wander Lairson Costa
	Comment 1 • 9 years ago

We need to switch tester docker image to in tree mozharness [0]. As tester docker doesn't need the full gecko repo, we can use archiver_client [1] for that.

[0] https://dxr.mozilla.org/mozilla-central/source/testing/docker/tester/bin/entrypoint

[1] http://jordan-lund.ghost.io/mozharness-goes-live-in-the-tree/

	Pete Moore [:pmoore][:pete]
	Comment 2 • 9 years ago

We should also clean up mozilla-taskcluster not to populate the MOZHARNESS_* environment variables in the generated task definition...

	Pete Moore [:pmoore][:pete]
	Comment 3 • 9 years ago

(In reply to Pete Moore [:pmoore][:pete] from comment #2)
> We should also clean up mozilla-taskcluster not to populate the MOZHARNESS_*
> environment variables in the generated task definition...

Ah - this is done in the decision task - my bad. So we should update in-tree taskcluster-graph, i.e.

https://hg.mozilla.org/mozilla-central/file/2ee9895e032c/testing/taskcluster/mach_commands.py#l238

	Pete Moore [:pmoore][:pete]
	Comment 4 • 9 years ago

So to summarize:

1) mach taskcluster-graph generates tasks containing mozharness environment variables
2) https://hg.mozilla.org/try/file/3d0f918b7e4a/testing/docker/tester/bin/entrypoint#l23 checks out mozharness using those values

Instead, the in-tree task definitions should use the in-tree mozharness (under /testing/mozharness) and the mozharness environment variables should be removed, and the entrypoint should be changed not to check out mozharness.

=========

To see this in the code:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=f66af40f9591 
=> https://tools.taskcluster.net/task-inspector/#OUlQhkr_ShWevhSoUL7gig/0
=> https://queue.taskcluster.net/v1/task/OUlQhkr_ShWevhSoUL7gig
=> https://tools.taskcluster.net/task-graph-inspector/#dSO_y8_jRtqmx328IeJrcg/
=> https://queue.taskcluster.net/v1/task/M_D8ClrvSxS3ux2EZnhHYg
=> https://hg.mozilla.org/mozilla-central/file/2ee9895e032c/testing/taskcluster/mach_commands.py#l207
=> https://hg.mozilla.org/mozilla-central/file/2ee9895e032c/testing/taskcluster/mach_commands.py#l265
=> https://hg.mozilla.org/mozilla-central/file/2ee9895e032c/testing/taskcluster/taskcluster_graph/commit_parser.py#l202
=> here it gets a little hazy...
=> https://hg.mozilla.org/mozilla-central/file/2ee9895e032c/testing/taskcluster/tasks/tests/b2g_gaia_unit.yml#l3
=> https://hg.mozilla.org/mozilla-central/file/2ee9895e032c/testing/taskcluster/tasks/test.yml#l39
=> https://hg.mozilla.org/mozilla-central/file/2ee9895e032c/testing/taskcluster/mach_commands.py#l281
=> https://hg.mozilla.org/mozilla-central/file/2ee9895e032c/testing/taskcluster/mach_commands.py#l266
=> https://hg.mozilla.org/mozilla-central/file/2ee9895e032c/testing/taskcluster/mach_commands.py#l44
=> https://hg.mozilla.org/mozilla-central/file/2ee9895e032c/testing/taskcluster/mach_commands.py#l24
=> https://hg.mozilla.org/mozilla-central/file/2ee9895e032c/testing/mozharness/mozharness.json#l2

	Wander Lairson Costa
	Comment 6 • 9 years ago

hrm, I don't see this bug as the same as bug 1185643. Actually I see bug 1185643 more as a meta bug, because there so many pieces to be patched.

See: https://etherpad.mozilla.org/gecko-mozharness-migration

	Jordan Lund (:jlund)
	Comment 7 • 9 years ago

does https://bugzilla.mozilla.org/show_bug.cgi?id=1188330#c4 answer the needinfo? Is there a way I can help here? Thanks wcosta and pmoore for the work thus far.

	Dustin J. Mitchell [:dustin] (he/him)
	Comment 8 • 9 years ago

In bug 1185643 I added a comment to `testing/taskcluster/mach_commands.py` indicating that `load_mozharness_info` and friends should be removed once nothing uses the mozharnses variables.  I *suspect* this is the bug that will finally allow that to be removed.

	Dustin J. Mitchell [:dustin] (he/him)
	Comment 9 • 9 years ago

reopening in agreement with comment 6 and since this isn't done yet :)

	Hsin-Yi Tsai (she/her) [:hsinyi]
	Comment 10 • 9 years ago

Some words about why this is a 2.5+.

Now emulator tests on TC are failed due to the error |The in-tree configuration file '/home/worker/build/tests/config/mozharness/b2g_emulator_config.py' does not exist!It must be added to 'gecko/testing/config/mozharness/b2g_emulator_config.py'. See bug 1035551 for more details.|

According to bug 1181261 comment 26, it points this bug is the key to those test failures. That's why we marked this 2.5+ for B2G.

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

	Jordan Lund (:jlund)
	Comment 11 • 9 years ago

double checking 1181261 isn't involved via https://bugzilla.mozilla.org/show_bug.cgi?id=1181261#c29

	Josh Cheng [:josh]
	Comment 12 • 9 years ago

Hi Jonathan,
Can you find someone who can help here? This bug is the major blocker for emulator on TC.
Thanks.

	Andrew Halberstadt [:ahal]
	Comment 13 • 9 years ago

(In reply to Hsin-Yi Tsai (OOO Sep. 4 ~ Sep. 20) [:hsinyi] from comment #10)
> Some words about why this is a 2.5+.
> 
> Now emulator tests on TC are failed due to the error |The in-tree
> configuration file
> '/home/worker/build/tests/config/mozharness/b2g_emulator_config.py' does not
> exist!It must be added to
> 'gecko/testing/config/mozharness/b2g_emulator_config.py'. See bug 1035551
> for more details.|
> 
> According to bug 1181261 comment 26, it points this bug is the key to those
> test failures. That's why we marked this 2.5+ for B2G.
> 
> [1] https://treeherder.allizom.org/#/jobs?repo=try&revision=6c932d1134af

Sorry, no one ever pasted a bug or failure into bug 1181261, so I didn't realize that emulator-kk tests were busted! While it's true that fixing this bug would also fix your problem, I can just temporarily add the config file back to unblock you guys in the short term.

Thanks for double checking Jordan.

	Andrew Halberstadt [:ahal]
	Comment 14 • 9 years ago

Fix just landed in the other bug. Sorry for the confusion, I misinterpreted the problem from https://bugzilla.mozilla.org/show_bug.cgi?id=1181261#c27 . For future reference, if you have an error, please paste it in the bug :)

	Hsin-Yi Tsai (she/her) [:hsinyi]
	Comment 15 • 9 years ago

(In reply to Andrew Halberstadt [:ahal] from comment #14)
> Fix just landed in the other bug. Sorry for the confusion, I misinterpreted
> the problem from https://bugzilla.mozilla.org/show_bug.cgi?id=1181261#c27 .
> For future reference, if you have an error, please paste it in the bug :)

Thank you Andrew!
B.t.w. do you happen to know which channel we could use to communicate with the release team people that FxOS team is currently working on emu-kk on Treeherder *Staging* and it would be much appreciation that teams could pay attention to that?

	Andrew Halberstadt [:ahal]
	Comment 16 • 9 years ago

The only way to make sure your work isn't broken is to get sheriffs to look at it and back things out that break them. Unfortunately they won't look at things that are already perma-orange. I know it's painful, I've gone through the process of trying to get tests stood up on a new platform many times myself.

But let's take this conversation to e-mail, it isn't related to this bug.

	Edgar Chen [:edgar] Assignee
	Comment 17 • 9 years ago

(In reply to Wander Lairson Costa [:wcosta] from comment #1)
> We need to switch tester docker image to in tree mozharness [0]. As tester
> docker doesn't need the full gecko repo, we can use archiver_client [1] for
> that.
> 
> [0]
> https://dxr.mozilla.org/mozilla-central/source/testing/docker/tester/bin/
> entrypoint
> 
> [1] http://jordan-lund.ghost.io/mozharness-goes-live-in-the-tree/

I have a different idea here. What about,
1. Packaging the mozharness, let's say mozharness.zip, and put it under public/build for each build. (something like what we did for target.*.tests.zip)
2. Then the tester docker can download a mozharness.zip from a specific build.

The advantage is of this approach is:
1. The revision of mozharness will match with the build/test we triggered.
2. Any changes in mozharness can also be secured.

Hi Wander, may I have some feedback from you? I can provide a WIP patch and try result if you are interested.

Thank you.

	Wander Lairson Costa
	Comment 18 • 9 years ago

(In reply to Edgar Chen [:edgar][:echen] from comment #17)
> (In reply to Wander Lairson Costa [:wcosta] from comment #1)
> > We need to switch tester docker image to in tree mozharness [0]. As tester
> > docker doesn't need the full gecko repo, we can use archiver_client [1] for
> > that.
> > 
> > [0]
> > https://dxr.mozilla.org/mozilla-central/source/testing/docker/tester/bin/
> > entrypoint
> > 
> > [1] http://jordan-lund.ghost.io/mozharness-goes-live-in-the-tree/
> 
> I have a different idea here. What about,
> 1. Packaging the mozharness, let's say mozharness.zip, and put it under
> public/build for each build. (something like what we did for
> target.*.tests.zip)
> 2. Then the tester docker can download a mozharness.zip from a specific
> build.
> 
> The advantage is of this approach is:
> 1. The revision of mozharness will match with the build/test we triggered.
> 2. Any changes in mozharness can also be secured.
> 
> Hi Wander, may I have some feedback from you? I can provide a WIP patch and
> try result if you are interested.
> 

I don't have a strong opinion on this, but if archive_client supports checking out with a specific rev, we can achieve the same effect just by putting the revision in the test task.

	Dustin J. Mitchell [:dustin] (he/him)
	Comment 19 • 9 years ago

I prefer Edgar's solution, if possible - especially if that .zip only contains the MH files, and not the hg revision history.  It mirrors the test zip idea nicely, and it looks like it's only about 3MB.

Archiver is basically an interim solution until either hg.m.o supports caching tarballs (archiver is basically a purpose-specific cache in front of hg.m.o, after all) or we extend proxxxy (the generic releng proxy) to handle hg.m.o tarballs or hg.m.o supports subdirectory checkouts (which mercurial itself doesn't support yet, but is working on).  As such, I'd rather avoid introducing dependencies on archiver where possible, as we'll just need to unwind those dependencies later.

	Jordan Lund (:jlund)
	Comment 20 • 9 years ago

> The advantage is of this approach is:
> 1. The revision of mozharness will match with the build/test we triggered.

archiver archives mh from a specific repo+changeset once and uploads it to s3. both builds and tests in buildbot use this archive. iow - you can download by rev

though I hear dustin in not wanting to add more deps to this if an alternative is really easy

	Edgar Chen [:edgar] Assignee
	Comment 22 • 9 years ago

Comment on attachment 8657561 [details] [diff] [review]
WIP, patch, v1

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

Thanks for all your comments.

Here is the WIP patch:
1. Package the mozharness when generating the test zip.
2. Move mozharness.zip to $HOME/artifacts. [1]
3. Generate mozharness url of a specific build and pass it to test task.
4. Test task downloads the mozharness.zip and unzip it. [2]

Here is the try runs for your reference: https://treeherder.allizom.org/#/jobs?repo=try&revision=408bc7975357&group_state=expanded
(I use my own docker hub repository for testing)

I am not sure who should I send feedback? to, so I just send to you all. Hope this won't bother you too much.
Thank you.

[1] You can see mozharness.zip in task inspector: https://tools.taskcluster.net/task-inspector/#_SkSA7fiRQeFqmkL4rkG1w/0
[2] Logs of CPP test for your reference: https://s3-us-west-2.amazonaws.com/taskcluster-public-artifacts/4o2JhH34Tyu_AzwDRYOH2A/0/public/logs/live_backing.log

	Wander Lairson Costa
	Comment 23 • 9 years ago

Comment on attachment 8657561 [details] [diff] [review]
WIP, patch, v1

Adding :garndt for feedback.

	Wander Lairson Costa
	Comment 24 • 9 years ago

Most of the tests failed, but I don't know if it is related to the patch.

	Edgar Chen [:edgar] Assignee
	Comment 25 • 9 years ago

(In reply to Wander Lairson Costa [:wcosta] from comment #24)
> Most of the tests failed, but I don't know if it is related to the patch.

No, it isn't related to this patch, most of the test on taskcluster are still failed even without this patch.

Here is the current situation:
We are trying to stabilize the test in bug 1144970 (mainly focus on emulator-x86-kk). But I realise making some tests green might need to change mozharness as well, like bug 1200928 (I guess there might be more). So I would like to move taskcluster tests to use intree mozharness first. Then we could focus on intree one if any changes needed in mozharness.

Thank you.

	Wander Lairson Costa
	Comment 26 • 9 years ago

Comment on attachment 8657561 [details] [diff] [review]
WIP, patch, v1

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

lgtm, just a remainder we need to check if it is necessary to modify phone tasks too.

::: testing/docker/tester/VERSION
@@ +1,1 @@
> +0.3.7.3

I think the convention is 3 version numbers.

	Jordan Lund (:jlund)
	Comment 27 • 9 years ago

Comment on attachment 8657561 [details] [diff] [review]
WIP, patch, v1

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

looks good but I am not familiar with TC enough to review this :)

	Dustin J. Mitchell [:dustin] (he/him)
	Comment 28 • 9 years ago

Comment on attachment 8657561 [details] [diff] [review]
WIP, patch, v1

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

::: testing/docker/tester/REGISTRY
@@ +1,1 @@
> +edgarchen

Don't forget to push to taskcluster before landing, and change this (I always forget!)

::: testing/docker/tester/bin/entrypoint
@@ +1,3 @@
>  #! /bin/bash -ex
>  
> +test $MOZHARNESS_URL # mozharness url

If this variable isn't set, the only feedback you get in the logs is

```
test
```

Since you're running with -x, it can be helpful to log the things you're testing, similar to how build-linux.sh does.
https://dxr.mozilla.org/mozilla-central/source/testing/taskcluster/scripts/builder/build-linux.sh?offset=0#12
: MOZHARNESS_SCRIPT             ${MOZHARNESS_SCRIPT}

@@ +19,5 @@
>  #   $ docker -v your_mozharness:/home/worker/mozharness ...
>  #
>  if [ ! -d mozharness ]; then
> +  buildbot_step 'Download mozharness' wget -c $MOZHARNESS_URL
> +  buildbot_step 'Unzip mozharness' unzip -q mozharness.zip

Won't this leave the mozharness directory in place in the workspace?  In which case, [ ! -d mozharness ] will be false on the next run, and no new unzip will take place?

	Greg Arndt [:garndt]
	Comment 29 • 9 years ago

Comment on attachment 8657561 [details] [diff] [review]
WIP, patch, v1

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

::: testing/docker/tester/bin/entrypoint
@@ +1,3 @@
>  #! /bin/bash -ex
>  
> +test $MOZHARNESS_URL # mozharness url

From what I see this will only defined for builds that produce the mozharness.zip, which is just he emulator builds altered in this patch.  What about b2g builds/tests that do not have this specified?

@@ +18,5 @@
>  #
>  #   $ docker -v your_mozharness:/home/worker/mozharness ...
>  #
>  if [ ! -d mozharness ]; then
> +  buildbot_step 'Download mozharness' wget -c $MOZHARNESS_URL

Is there any value in having this split across two buildbot steps?

Also, if this directory does not exist, it will try to pull down the mozharness zip for everything that inherits from test.yml.....what if the build (such as b2g desktop) doesn't have this?

@@ +19,5 @@
>  #   $ docker -v your_mozharness:/home/worker/mozharness ...
>  #
>  if [ ! -d mozharness ]; then
> +  buildbot_step 'Download mozharness' wget -c $MOZHARNESS_URL
> +  buildbot_step 'Unzip mozharness' unzip -q mozharness.zip

If this is loaded in the workspace, we probably want to get rid of the mozharness.zip as well so we're not saving that in the cache each time.

	Edgar Chen [:edgar] Assignee
	Comment 30 • 9 years ago

(In reply to Dustin J. Mitchell [:dustin] from comment #28)
> Comment on attachment 8657561 [details] [diff] [review]
> WIP, patch, v1
> 
> Review of attachment 8657561 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: testing/docker/tester/REGISTRY
> @@ +1,1 @@
> > +edgarchen
> 
> Don't forget to push to taskcluster before landing, and change this (I
> always forget!)

Sure, I will use taskcluster in formal patch.
But I don't have permission to push image to taskcluster, may need some help from you guys when landing the patch. :)

> 
> ::: testing/docker/tester/bin/entrypoint
> @@ +1,3 @@
> >  #! /bin/bash -ex
> >  
> > +test $MOZHARNESS_URL # mozharness url
> 
> If this variable isn't set, the only feedback you get in the logs is
> 
> ```
> test
> ```
> 
> Since you're running with -x, it can be helpful to log the things you're
> testing, similar to how build-linux.sh does.
> https://dxr.mozilla.org/mozilla-central/source/testing/taskcluster/scripts/
> builder/build-linux.sh?offset=0#12
> : MOZHARNESS_SCRIPT             ${MOZHARNESS_SCRIPT}

Will do in formal patch. Thank you.

	Edgar Chen [:edgar] Assignee
	Comment 31 • 9 years ago

(In reply to Greg Arndt [:garndt] from comment #29)
> Comment on attachment 8657561 [details] [diff] [review]
> WIP, patch, v1
> 
> Review of attachment 8657561 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: testing/docker/tester/bin/entrypoint
> @@ +1,3 @@
> >  #! /bin/bash -ex
> >  
> > +test $MOZHARNESS_URL # mozharness url
> 
> From what I see this will only defined for builds that produce the
> mozharness.zip, which is just he emulator builds altered in this patch. 
> What about b2g builds/tests that do not have this specified?

I will review all the build/test and do proper modification (and also remove the old mozharnees variables) in formal patch.
Thank you.

> 
> @@ +18,5 @@
> >  #
> >  #   $ docker -v your_mozharness:/home/worker/mozharness ...
> >  #
> >  if [ ! -d mozharness ]; then
> > +  buildbot_step 'Download mozharness' wget -c $MOZHARNESS_URL
> 
> Is there any value in having this split across two buildbot steps?
> 
> Also, if this directory does not exist, it will try to pull down the
> mozharness zip for everything that inherits from test.yml.....what if the
> build (such as b2g desktop) doesn't have this?

Ditto

	Edgar Chen [:edgar] Assignee
	Comment 32 • 9 years ago

Try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=b5357a9da67e

Most test looks good, except all Gaia test on Mulet/B2G_Desktop busted.
Investigating ....

> Traceback (most recent call last):
>   File "./mozharness/scripts/gaia_build_integration.py", line 14, in <module>
>     from mozharness.mozilla.testing.gaia_test import GaiaTest
>   File "/home/worker/mozharness/mozharness/mozilla/testing/gaia_test.py", line 28, in <module>
>     class GaiaTest(TestingMixin, MercurialScript, TransferMixin, GaiaMixin, BlobUploadMixin):
> TypeError: Error when calling the metaclass bases
>     Cannot create a consistent method resolution
> order (MRO) for bases BlobUploadMixin, VirtualenvMixin, object, GaiaMixin, TransferMixin

	Edgar Chen [:edgar] Assignee
	Comment 33 • 9 years ago

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

Another error in Mulet reftests:
> 02:50:05    FATAL - Uncaught exception: Traceback (most recent call last):
> 02:50:05    FATAL -   File "/home/worker/mozharness/mozharness/base/script.py", line 1693, in run
> 02:50:05    FATAL -     self.run_action(action)
> 02:50:05    FATAL -   File "/home/worker/mozharness/mozharness/base/script.py", line 1635, in run_action
> 02:50:05    FATAL -     self._possibly_run_method(method_name, error_if_missing=True)
> 02:50:05    FATAL -   File "/home/worker/mozharness/mozharness/base/script.py", line 1576, in _possibly_run_method
> 02:50:05    FATAL -     return getattr(self, method_name)()
> 02:50:05    FATAL -   File "./mozharness/scripts/mulet_unittest.py", line 73, in run_tests
> 02:50:05    FATAL -     super(MuletUnittest, self).run_tests()
> 02:50:05    FATAL -   File "/home/worker/mozharness/scripts/b2g_desktop_unittest.py", line 197, in run_tests
> 02:50:05    FATAL -     cmd = self._query_abs_base_cmd(suite)
> 02:50:05    FATAL -   File "/home/worker/mozharness/scripts/b2g_desktop_unittest.py", line 156, in _query_abs_base_cmd
> 02:50:05    FATAL -     if suite not in self.config["suite_definitions"]:
> 02:50:05    FATAL - KeyError: 'suite_definitions'

	Edgar Chen [:edgar] Assignee
	Comment 34 • 9 years ago

Bug 1188330 - Use intree mozharness for taskcluster tests

	Edgar Chen [:edgar] Assignee
	Comment 35 • 9 years ago

Comment on attachment 8662321 [details]
MozReview Request: Bug 1188330 - Use intree mozharness for taskcluster tests

Summarize for this formal patch:
1. Download mozharness.zip only when the url is specified (Although test.yml is only inherited by b2g tests which will have mozharness_url defined).
2. Copy mozharness.zip to artifacts in all b2g build script.
3. Move b2g common configuration, test_packages, to b2g_base.yml
4. And also define mozharness in b2g_base.yml since all b2g build will have mozharness.zip.
5. Remove the old mozharnees variables.
6. Fix the busted in Mulet/B2G_Desktop test, comment #32.
7. Fix the busted in Mulet reftest, comment #33.

Try result for your reference: https://treeherder.mozilla.org/#/jobs?repo=try&revision=336e699ff8cf&exclusion_profile=false&group_state=expanded
(I use my own docker hub repository for this try pushing)

	Edgar Chen [:edgar] Assignee
	Comment 36 • 9 years ago

Comment on attachment 8662321 [details]
MozReview Request: Bug 1188330 - Use intree mozharness for taskcluster tests

Bug 1188330 - Use intree mozharness for taskcluster tests

	Edgar Chen [:edgar] Assignee
	Comment 37 • 9 years ago

(In reply to Wander Lairson Costa [:wcosta] from comment #26)
> Comment on attachment 8657561 [details] [diff] [review]
> WIP, patch, v1
> 
> Review of attachment 8657561 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> lgtm, just a remainder we need to check if it is necessary to modify phone
> tasks too.

BTW, phone tasks don't use mozharness, we don't have to do any thing for phone tasks. Thank you.

	Wander Lairson Costa
	Comment 38 • 9 years ago

(In reply to Edgar Chen [:edgar][:echen] from comment #37)
> (In reply to Wander Lairson Costa [:wcosta] from comment #26)
> > Comment on attachment 8657561 [details] [diff] [review]
> > WIP, patch, v1
> > 
> > Review of attachment 8657561 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > lgtm, just a remainder we need to check if it is necessary to modify phone
> > tasks too.
> 
> BTW, phone tasks don't use mozharness, we don't have to do any thing for
> phone tasks. Thank you.

Phone tasks call the testintg/taskcluster/scripts/phone-builder/build-phone*.sh scripts, which call mozharness.

	Greg Arndt [:garndt]
	Comment 39 • 9 years ago

Comment on attachment 8662321 [details]
MozReview Request: Bug 1188330 - Use intree mozharness for taskcluster tests

https://reviewboard.mozilla.org/r/19575/#review17537

This looks good to me.  Just some comments and a suggestion about some cleanup in the mach command either to be done in this bug or a new bug entered to clean it up.

::: testing/docker/tester/bin/entrypoint:15
(Diff revision 1)
> -# used locally in development to test mozharness changes:
> +if [ ! -z "$MOZHARNESS_URL" ]; then

Hrm, is there anytime in a tester when this wouldn't be set?  Would it be safter to ensure that it's set to enforce all tasks to have it before doing anything more?

::: testing/mozharness/configs/b2g/mulet_config.py:13
(Diff revision 1)
> -    "reftest_options": [
> +    "suite_definitions": {

I'm not sure how mozharness handle this, but I assume changing the name of this config option, mozharness knows where to look for things.  I see "suite_definitions" used in various configs.

::: testing/mozharness/mozharness/mozilla/testing/gaia_test.py:28
(Diff revision 1)
> -class GaiaTest(TestingMixin, MercurialScript, TransferMixin, GaiaMixin, BlobUploadMixin):
> +class GaiaTest(MercurialScript, TestingMixin, BlobUploadMixin, GaiaMixin, TransferMixin):

did this change because of method resolution?

::: testing/taskcluster/mach_commands.py:405
(Diff revision 1)
> +            if 'mozharness' in build_task['task']['extra']['locations']:

nit: At this point it might be simpler to just iterate over all the things in task.extra.locations and add them to some dictionary that is then used to build out the test_parmeters...  Doesn't necesasrily have to be changed in this, especially since the current way has been the normal set so far.

location_urls = {}
for location in build_task['task']['extra']['locations']:
  location_urls[location] = ARTIFACT_URL.format(
    build_parameters['build_slugid'],
    build_task['task']['extra']['locations'][location]
 
for location, url in location_urls.iteritems():
  test_parameters['{}_url'.format(location)] = url

	Greg Arndt [:garndt]
	Comment 40 • 9 years ago

Ah, reading the comments, disregard my comments about MRO, suite_definitions, and testing for the mozharness url

	Dustin J. Mitchell [:dustin] (he/him)
	Comment 41 • 9 years ago

Comment on attachment 8662321 [details]
MozReview Request: Bug 1188330 - Use intree mozharness for taskcluster tests

https://reviewboard.mozilla.org/r/19575/#review17551

::: testing/taskcluster/mach_commands.py
(Diff revision 1)
> -            'mozharness_ref':mozharness.get('reference', mozharness['revision']),

I'm happy to see this code going away!

	Edgar Chen [:edgar] Assignee
	Comment 42 • 9 years ago

(In reply to Wander Lairson Costa [:wcosta] from comment #38)
> (In reply to Edgar Chen [:edgar][:echen] from comment #37)
> > (In reply to Wander Lairson Costa [:wcosta] from comment #26)
> > > Comment on attachment 8657561 [details] [diff] [review]
> > > WIP, patch, v1
> > > 
> > > Review of attachment 8657561 [details] [diff] [review]:
> > > -----------------------------------------------------------------
> > > 
> > > lgtm, just a remainder we need to check if it is necessary to modify phone
> > > tasks too.
> > 
> > BTW, phone tasks don't use mozharness, we don't have to do any thing for
> > phone tasks. Thank you.
> 
> Phone tasks call the
> testintg/taskcluster/scripts/phone-builder/build-phone*.sh scripts, which
> call mozharness.

Sorry for didn't make it clear, I meant phone *test* tasks seem not use mozharness. I will double check again.

And you are right, phone build tasks do use mozharness, and most of them has already moved to intree one, except build-dolphin.sh (I will double check this as well) [1]. Since this bug is filed for test tasks, I will file a separated bug for build-dolphin.sh if needed.

Thank you.

[1] https://dxr.mozilla.org/mozilla-central/source/testing/taskcluster/scripts/phone-builder/build-dolphin.sh#10-22

	Wander Lairson Costa
	Comment 43 • 9 years ago

Comment on attachment 8662321 [details]
MozReview Request: Bug 1188330 - Use intree mozharness for taskcluster tests

https://reviewboard.mozilla.org/r/19575/#review17573

::: testing/mozharness/configs/b2g/mulet_config.py:26
(Diff revision 1)
> +    },

I also have no knowledge on this part, so I will just trust other reviewers.

::: testing/testsuite-targets.mk:435
(Diff revision 1)
> +	cd $(abspath $(PKG_STAGE)) && \

I just assume this part is ok, I have no knowledge on it

	Edgar Chen [:edgar] Assignee
	Comment 44 • 9 years ago

(In reply to Edgar Chen [:edgar][:echen] from comment #42)
> And you are right, phone build tasks do use mozharness, and most of them has
> already moved to intree one, except build-dolphin.sh (I will double check
> this as well) [1]. Since this bug is filed for test tasks, I will file a
> separated bug for build-dolphin.sh if needed.

File bug 1205934 for dolphin build.

> 
> Thank you.
> 
> [1]
> https://dxr.mozilla.org/mozilla-central/source/testing/taskcluster/scripts/
> phone-builder/build-dolphin.sh#10-22

	Edgar Chen [:edgar] Assignee
	Comment 45 • 9 years ago

(In reply to Greg Arndt [:garndt] from comment #39)
> Comment on attachment 8662321 [details]
> MozReview Request: Bug 1188330 - Use intree mozharness for taskcluster tests
> 
> https://reviewboard.mozilla.org/r/19575/#review17537
> 
> ::: testing/taskcluster/mach_commands.py:405
> (Diff revision 1)
> > +            if 'mozharness' in build_task['task']['extra']['locations']:
> 
> nit: At this point it might be simpler to just iterate over all the things
> in task.extra.locations and add them to some dictionary that is then used to
> build out the test_parmeters...  Doesn't necesasrily have to be changed in
> this, especially since the current way has been the normal set so far.
> 
> location_urls = {}
> for location in build_task['task']['extra']['locations']:
>   location_urls[location] = ARTIFACT_URL.format(
>     build_parameters['build_slugid'],
>     build_task['task']['extra']['locations'][location]
>  
> for location, url in location_urls.iteritems():
>   test_parameters['{}_url'.format(location)] = url

Will fire a follow-up bug for this. Thank you.

	Edgar Chen [:edgar] Assignee
	Comment 46 • 9 years ago

Hi Wander, I don't have permission to push docker image to taskcluster repository, could you help me to build and push the new docker image based on this patch? Thank you.

	Wander Lairson Costa
	Comment 47 • 9 years ago

(In reply to Edgar Chen [:edgar][:echen] from comment #46)
> Hi Wander, I don't have permission to push docker image to taskcluster
> repository, could you help me to build and push the new docker image based
> on this patch? Thank you.

Redirecting to garndt.

	Wander Lairson Costa
	Comment 48 • 9 years ago

(In reply to Wander Lairson Costa [:wcosta] from comment #47)
> (In reply to Edgar Chen [:edgar][:echen] from comment #46)
> > Hi Wander, I don't have permission to push docker image to taskcluster
> > repository, could you help me to build and push the new docker image based
> > on this patch? Thank you.
> 
> Redirecting to garndt.

Ok, I just pushed the image, could you please test it?

	Edgar Chen [:edgar] Assignee
	Comment 49 • 9 years ago

Comment on attachment 8662321 [details]
MozReview Request: Bug 1188330 - Use intree mozharness for taskcluster tests

Bug 1188330 - Use intree mozharness for taskcluster tests

	Edgar Chen [:edgar] Assignee
	Comment 50 • 9 years ago

(In reply to Wander Lairson Costa [:wcosta] from comment #48)
> (In reply to Wander Lairson Costa [:wcosta] from comment #47)
> > (In reply to Edgar Chen [:edgar][:echen] from comment #46)
> > > Hi Wander, I don't have permission to push docker image to taskcluster
> > > repository, could you help me to build and push the new docker image based
> > > on this patch? Thank you.
> > 
> > Redirecting to garndt.
> 
> Ok, I just pushed the image, could you please test it?

Try result looks good: https://treeherder.mozilla.org/#/jobs?repo=try&revision=180603ed23cc&group_state=expanded&exclusion_profile=false

	Pulsebot
	Comment 51 • 9 years ago

https://hg.mozilla.org/integration/b2g-inbound/rev/7f2c1a5e6771

	Phil Ringnalda (:philor)
	Comment 52 • 9 years ago

https://hg.mozilla.org/mozilla-central/rev/7f2c1a5e6771