1516843 - Fix installation of mozmill for local development

Status: RESOLVED FIXED

(Thunderbird :: Build Config, enhancement)

Description

[Geoff Lankow (:darktrojan)]

+++ This bug was initially created as a clone of Bug #1428494 +++

I'm going to change the mozmill installation so that it uses the build virtualenv instead of creating a new one, and also use the build system (instead of make) to sort out the calendar mozmill tests for running. If I figure out how, I'll do something cleverer about the calendar tests.

Comment 1

[Geoff Lankow (:darktrojan)]

Attachment: 1516843-mozmill-virtualenv-1.diff

Comment 2

[Geoff Lankow (:darktrojan)]

Attachment: 1516843-mozmill-build-1.diff

Comment 3

[Philipp Kewisch [:Fallen][☁️📆]]

Comment on attachment 9033681 [details] [diff] [review]
1516843-mozmill-build-1.diff

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

r+wc

::: calendar/test/mozmill/Makefile.in
@@ -35,5 @@
> -mozmill-stage: MOZMILL_STAGE_SHARED=$(MOZMILL_STAGE)/shared-modules
> -mozmill-stage: $(MOZMILL_SHARED)
> -	$(INSTALL) -D $(MOZMILL_STAGE)
> -	$(INSTALL) -D $(MOZMILL_STAGE_SHARED)
> -	$(INSTALL) $(addprefix $(srcdir)/,$(shell cat $(srcdir)/mozmilltests.list)) $(MOZMILL_STAGE)

Do we still need mozmilltests.list with these changes?

::: calendar/test/mozmill/moz.build
@@ +8,5 @@
> +    'cal-recurrence/*',
> +    'eventDialog/*',
> +    'invitations/*',
> +    'recurrenceRotated/*',
> +    'test*.js',

Would it make sense to name these more explicitly? From the other moz.build files I've read I don't often see wildcards.

::: mail/test/mozmill/shared-modules/moz.build
@@ +6,5 @@
> +# Copies the files in this folder to a staging directory in the objdir.
> +# The calendar mozmill test files are run from there and use some files
> +# from this folder.
> +TEST_HARNESS_FILES.mozmill.stage['shared-modules'] += [
> +    'test*.js',

Same here.

Comment 4

[Philipp Kewisch [:Fallen][☁️📆]]

Comment on attachment 9033680 [details] [diff] [review]
1516843-mozmill-virtualenv-1.diff

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

I trust you tested that there is no download on each build with the new virtualenv?

Comment 5

[Geoff Lankow (:darktrojan)]

Attachment: 1516843-mozmill-virtualenv-2.diff

But wait, there's more. We don't need to copy third_party/python/virtualenv into the objdir now that we're not using it.

Comment 6

[Geoff Lankow (:darktrojan)]

(In reply to Philipp Kewisch [:Fallen] [:📆] from comment #3)
> Do we still need mozmilltests.list with these changes?

Yes, it's still used in automation, and for anyone mad enough to run "make mozmill".

> ::: calendar/test/mozmill/moz.build
> @@ +8,5 @@
> > +    'cal-recurrence/*',
> > +    'eventDialog/*',
> > +    'invitations/*',
> > +    'recurrenceRotated/*',
> > +    'test*.js',
> 
> Would it make sense to name these more explicitly? From the other moz.build
> files I've read I don't often see wildcards.

It's definitely a thing that's done, I found a few occurrences in our code and on m-c. I'll expand the test*.js though, because that seems sensible.

> ::: mail/test/mozmill/shared-modules/moz.build
> @@ +6,5 @@
> > +# Copies the files in this folder to a staging directory in the objdir.
> > +# The calendar mozmill test files are run from there and use some files
> > +# from this folder.
> > +TEST_HARNESS_FILES.mozmill.stage['shared-modules'] += [
> > +    'test*.js',
> 
> Same here.

I won't expand this, there's too many and it seems silly. I could list only the ones needed from calendar, but it's nice not to have to check if a file is available before using it.

(In reply to Philipp Kewisch [:Fallen] [:📆] from comment #4)
> Comment on attachment 9033680 [details] [diff] [review]
> 1516843-mozmill-virtualenv-1.diff
> 
> Review of attachment 9033680 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I trust you tested that there is no download on each build with the new
> virtualenv?

There could be downloads for package dependencies, but this was the case anyway. At least now we're checking once at build time, not every time we start a test.

Comment 7

[Philipp Kewisch [:Fallen][☁️📆]]

(In reply to Geoff Lankow (:darktrojan) from comment #6)
> At least now we're checking once at build time, not every time we
> start a test.

Yes, this kind of thing was what I am looking for. We used to be in a position where each incremental build downloaded packages in the tools step, which I want to avoid. Downloading at the start of the full build is fine.

Comment 8

[Joachim Herb]

Just my 0.02$:

I use the mozmill installation/virutalenv included in the test archives of Thunderbird to test Thunderbird with my addon (CompactHeader) installed against my own mozmill tests and some included in the test archive:
https://github.com/jmozmoz/compactheader
https://travis-ci.org/jmozmoz/compactheader
https://ci.appveyor.com/project/jmozmoz/compactheader/branch/master

Will this patch result in a removal of the mozmill infrastructure from the test archives?
(With test archive, I mean one of these: https://ftp.mozilla.org/pub/thunderbird/nightly/latest-comm-central/thunderbird-66.0a1.en-US.linux-i686.common.tests.tar.gz)

As there is no beta channel on AMO/ATN anymore, I think tests are vital. So please make sure, that the test archives/infrastructure can be still used outside the mozilla build servers (even though it looks like I am the only one doing this, but CompactHeader has (still) a lot of users).

Comment 9

[Geoff Lankow (:darktrojan)]

These patches will make no difference unless you're building Thunderbird yourself.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/178da49fe8c8
Copy mozmill test files to the staging directory at build-time (local development only); r=Fallen
https://hg.mozilla.org/comm-central/rev/a58e59146246
Install mozmill packages in build virtualenv (for local development only); r=Fallen

Comment 11

[Geoff Lankow (:darktrojan)]

I have (much) more to do here, but it can go in another bug.