Closed Bug 1121107 Opened 9 years ago Closed 9 years ago

Consider patching mozmill to be able to run in the mozharness virtualenv

Categories

(Thunderbird :: Testing Infrastructure, defect)

x86_64
Linux
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 38.0

People

(Reporter: jcranmer, Assigned: jcranmer)

References

Details

Attachments

(4 files)

We have what appears to be at least three bugs on updating mozmill to a newer version, with all of them stalled. Given that there's noise about mozmill being deprecated in favor of marionette, I'm operating under the assumption that updating mozmill isn't a high priority on anyone's list.

The future of our test infrastructure on automation is quite clearly mozharness. As far as I am aware, Thunderbird's unit tests are the only ones not on mozharness (I don't think talos uses mozharness, though), which means we are liable to see hard-to-fix issues without things like bug 1054308. Not to mention, some of my most recent experiments have suggested that mozharness is the best place to put in cool features like code coverage analysis results, so there is a cost to not using mozharness.

When I last investigated bug 1054308, I found that the current setup of mozmill is extremely nontrivial to port to mozharness: nested virtualenv construction fails, and mozmill cannot use the virtualenv due to incompatible versions. If we're not exactly going to update to a newer version of mozmill, then it holds that there would be benefit to patching our in-tree version of mozmill to let it use far newer versions of mozrunner.

On the downside, this does mean that we'd have to maintain the mozmill infrastructure if mozrunner dependencies change, but given what I've said about mozmill earlier, I'm not sure this isn't a price we wouldn't already have to pay.
Depends on: 1126547
We don't actually use ManifestDestiny for anything important inside mozmill, since we don't use manifests at all...
Assignee: nobody → Pidgeot18
Status: NEW → ASSIGNED
Attachment #8556939 - Flags: review?(bugspam.Callek)
... and simplejson has been unneeded for ... years? We haven't needed it since 2.6, and we're on 2.7 nowadays.
Attachment #8556940 - Flags: review?(bugspam.Callek)
Yay, the real meat of the changes...

<https://treeherder.mozilla.org/ui/#/jobs?repo=try-comm-central&revision=324c9a447dba>.

I also took the time to drop the automationutils.py dependency and simplify a bit of the build system. I suspect with a bit more work, I could drop the independent virtualenv altogether. The directory copies can't be moved to moz.build TEST_HARNESS_FILES because... it's broken for comm-central and it's way too annoying to figure out how to fix it (but file copies work!).
Attachment #8556945 - Flags: review?(hskupin)
Attachment #8556945 - Flags: review?(bugspam.Callek)
fyi, im off tomorrow. redirect review if needed
Comment on attachment 8556945 [details] [diff] [review]
Part 3: Heavily patch the mozmill so it can use the regular mozrunner suite

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

I cannot give a review for custom Mozmill changes especially for mozharness. Justin is the way better person to ask for. But I had a quick look over the code and haven't seen something obvious except for the mozrunner deps. See my comment inline.

Justin, if something is unclear to you regarding mozmill, please let me know and I can try to help.

::: mail/test/resources/jsbridge/setup.py
@@ +43,5 @@
>  
>  PACKAGE_NAME = "jsbridge"
>  PACKAGE_VERSION = "2.4.14"
>  
> +requires = ['mozrunner >= 2.5.13']

mozrunner 6.0 includes a major refactoring, so you might want to bump this up to at minimum that version.

::: mail/test/resources/mozmill/setup.py
@@ +62,5 @@
>            mozmill-restart = mozmill:restart_cli
>          """,
>        platforms =['Any'],
>        install_requires = ['jsbridge == 2.4.14',
> +                          'mozrunner >= 2.5.13',

Same here for mozrunner.
Attachment #8556945 - Flags: review?(hskupin) → feedback+
Are they crazy? They want to deprecate everything these days :(
They barely released mozmill 2.0 and it is suddenly bad?

I think we have bugs about upgrading to the new versions of mozmill.
(In reply to :aceman from comment #7)
> Are they crazy? They want to deprecate everything these days :(
> They barely released mozmill 2.0 and it is suddenly bad?

Marionette is the intended replacement--I think marionette is also used for w3c standards test, and since marionette and mozmill are so broadly similar, there's no point maintaining both harnesses.

> I think we have bugs about upgrading to the new versions of mozmill.

We have three bugs on updating to newer versions--1.5.18, 1.5.24, and 2.0. I looked into using 2.1-dev, and the module loading system is different enough to break our code horribly (collector.getModule no longer exists, and I didn't see an obvious solution to that problem), so I didn't bother investigating further.
Attachment #8556939 - Flags: review?(bugspam.Callek) → review+
Comment on attachment 8556940 [details] [diff] [review]
Part 2: Don't use simplejson

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

I would be happier if:

http://mxr.mozilla.org/comm-central/source/mail/test/resources/jsbridge/jsbridge/network.py

was updated too, e.g. remove the try/except on line 46 (to just import json no import `simplejson`)  and no `import json as simplejson` and then s/simplejson/json/ everywhere in the file.

This can be a followup patch
Attachment #8556940 - Flags: review?(bugspam.Callek) → review+
Comment on attachment 8556945 [details] [diff] [review]
Part 3: Heavily patch the mozmill so it can use the regular mozrunner suite

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

I don't know mozmill/mozrunner so I mostly skimmed this as if it is "sane looking" changes. I would love if you found someone who does know mozmill/mozrunner and better info on mozbase than me, to give it a once over as well. But this is comm-* afterall so you may not get that. r+ with that understanding and I'll defer to you on if you think you can coerce another reviewer to give this a pass

::: mail/test/resources/jsbridge/jsbridge/__init__.py
@@ +68,5 @@
>              pass
>          sleep(.25)
>          ttl += .25
>      if ttl == timeout:
> +        print (host, port)

debugging info only?

@@ +111,5 @@
> +                          help="TCP port to run jsbridge on.")
> +
> +    def profile_args(self):
> +        profargs = super(CLI, self).profile_args()
> +        # XXX: this is complicated. defer for now.

would love a bug # here to point to for "when is this addressed"

::: mail/test/resources/jsbridge/setup.py
@@ +43,5 @@
>  
>  PACKAGE_NAME = "jsbridge"
>  PACKAGE_VERSION = "2.4.14"
>  
> +requires = ['mozrunner >= 2.5.13']

whimboo makes a good point here...
Attachment #8556945 - Flags: review?(bugspam.Callek) → review+
(In reply to Justin Wood (:Callek) from comment #10)
> I don't know mozmill/mozrunner so I mostly skimmed this as if it is "sane
> looking" changes. I would love if you found someone who does know
> mozmill/mozrunner and better info on mozbase than me, to give it a once over
> as well. But this is comm-* afterall so you may not get that. r+ with that
> understanding and I'll defer to you on if you think you can coerce another
> reviewer to give this a pass

Whimboo gave some once over in his review.

> would love a bug # here to point to for "when is this addressed"

Bug 1130551.
https://hg.mozilla.org/comm-central/rev/241f8377859e
https://hg.mozilla.org/comm-central/rev/b8f7415c632c
https://hg.mozilla.org/comm-central/rev/e665f5e1b746
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 38.0
Without this patch mozmill didn't work locally under VNC due to undefined 'env'.
Attachment #8560926 - Flags: review?(Pidgeot18)
Attachment #8560926 - Flags: review?(Pidgeot18) → review+
Keywords: checkin-needed
Whiteboard: [check in only the VNC patch]
https://hg.mozilla.org/comm-central/rev/b3e8878f2691
Keywords: checkin-needed
Whiteboard: [check in only the VNC patch]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: