500142 - Create mozmill and mozmill-one make targets for build automation to use

Status: RESOLVED FIXED

(Thunderbird :: Build Config, defect)

Description

[Mark Banner (:standard8)]

After the work on bug 458352 to make mail/test/mozmill/runtest.py to work for our different types of mozmill tests, we should extend it to work for running standalone without a build environment (i.e. any random build).

Once we've done that, we should also package it as part of make package-tests, as this will enable tinderboxes to download builds and test, and people who don't have dev builds.

Comment 1

[Mark Banner (:standard8)]

Attachment: WIP

This is a work in progress patch. With it we can build Thunderbird with enable-tests, then go into the objdir and type:

make mozmill

at which point it will run mozmill for each of the directories listed in mail/test/mozmill/mozmilltests.list and eventually end with something like:

INFO | (runtestlist.py) | Directories Run: 2, Passed: 8, Failed: 2

Some comments about the patch so far:

- Removes the mozconfig detection from the existing runtest.py. I did this because make knows where the relevant files are, and when we want to run it where we've packaged the tests separately we need to pass arguments (which will be just 2 of them, one to the binary, one to the test list).

- I haven't added a single-file (or directory) mozmill test run yet. This will probably be something along the lines of make check-one.

- The test output has been reworked to use TEST-PASS and TEST-UNEXPECTED-FAIL. When we get this on tinderboxes, this will be the format we need to get detections of failures.

- There are still a few hard-coded directories and things that I need to clean up.

- There's a core patch as well that is needed for getting make package-tests to work.

Comments welcome.

Comment 2

[Mark Banner (:standard8)]

Attachment: Core Fixes

I'll put the core patch here for now until I've tested it a bit more.

Comment 3

[Mark Banner (:standard8)]

Attachment: WIP v2

Some improvements:

- Should fix all the hard-coded issues so that anyone can run it now.
- Adds a "make SOLO_TEST=bloat mozmill-one" target to run just one test or one set of tests. At the moment this is forced to be a reference in the mail/test/mozmill directory (note that single tests via make package-tests can just use runtest.py directly).

I've moved half the core patch out to bug 503886. I'll need to move the other half out to a different bug as well.

Comment 4

[Mark Banner (:standard8)]

Attachment: WIP v2a

Minor change s/SOLO_FILE/SOLO_TEST/

Comment 5

[Mark Banner (:standard8)]

Attachment: Proposed fix

This patch is the same as the previous but drops the attempts to be able to package the tests as part of package-tests. I took a look at package-tests and got part way through an implementation and then discovered some of our tests rely on srcdir.

I think package-tests isn't important at this stage, but getting a running mozmill is, so I think it is worth moving forward with just this.

I'll request reviews tomorrow once I've done a little more testing.

Comment 6

[Mark Banner (:standard8)]

Comment on attachment 387545 [details] [diff] [review]
Core Fixes

This patch is no longer needed, use the appropriate patch from bug 503886.

Comment 7

[Mark Banner (:standard8)]

Attachment: The fix

This has a couple of minor changes; the addition of folder-display to the directories list (this is now passing nicely on my machine where it wasn't before) and only defining the mozmill and mozmill-one targets on builds with --enable-tests - the same way as xpcshell-tests and other targets are done.

I think it is time to start requesting reviews - I'm requesting two reviews to get perspective from build config and tests. Although the core patch (bug 503886) hasn't bee reviewed yet, I'd like to move this forward and start getting comments so that we can be ready to check this in once the core patch is in place.

I'm also looking at getting a test VM & builder set up (bug 504304) so that we can start seeing how mozmill performs in an automated environment.

Comment 8

[Andrew Sutherland [:asuth] (he/him)]

Attachment: v1 changes to Standard8's patch to make it work on linux for me

Comment 9

[Andrew Sutherland [:asuth] (he/him)]

Comment on attachment 388694 [details] [diff] [review]
The fix

Thanks for being so on top of the mozmill automation!

I had to change the Makefile to make it work for me on linux.  I think the way I did things better approximates how the python code did it an accordingly should ideally work for Windows, but I make no promises.

My changes are in a patch I attached (against your patch), and I also added the search-window subdir since it exists and has folder-display-based tests too.

Two thoughts for future work:

- The code I had added to dump pass/fail should probably actually check that the pass-count is non-zero for each test too.  When you control-C out or otherwise cause the running of tests to be aborted, there will be no pass or fail records for a test, so it ends up passing.  This only happens in exceptional cases that perhaps shouldn't happen in automation, so it's not a biggie.

- It would be neat if we could return the test results via a JSON blob somewhere rather than the current tradition of text-mangling.  I am mainly thinking in terms of having a web UI that is better able to tell us where things went wrong.  But that's just pipe-dreams and tinderpushlog with its magic already gets us more than 80% of the way towards usability...

Comment 10

[Mark Banner (:standard8)]

I've added the two future suggestions to https://wiki.mozilla.org/User:Standard8/Thunderbird_Automated_Tests_Planning#MozMill_and_Leak_.26_bloat_tests.

The core patch has landed on trunk waiting approval to land it on 1.9.1.

Updating summary to reflect what we're actually going to do in this bug.

Comment 11

[Mark Banner (:standard8)]

Attachment: [checked in] Part 1 - initial targets

This is my original fix plus Andrew's changes. One more patch to come in a moment.

Comment 12

[Mark Banner (:standard8)]

Attachment: [checked in] Part 2 - tidy up

This tidies up some things - mozmill requires a --default-profile argument otherwise it can decide to use a default profile from another installation location of the application without telling you and for those who don't have an application installed in one of the special locations, it'll just error out.

Also tidied up the default profile selection code - if a test wants to override it, it should now do the right thing and always end up with the profile in the same place (in the mozilla/_tests/mozmill/mozmillprofile directory) rather than in your source tree.

Comment 13

[Andrew Sutherland [:asuth] (he/him)]

Comment on attachment 389382 [details] [diff] [review]
[checked in] Part 2 - tidy up

good catch on the default profile stuff.  I'm surprised that did not cause major problems for us earlier...

Comment 14

[Philippe M. Chiasson (:gozer)]

Comment on attachment 389381 [details] [diff] [review]
[checked in] Part 1 - initial targets

Except for the MOZ_APP_DISPLAYNAME/APP_NAME logic in https://bugzilla.mozilla.org/attachment.cgi?id=389381&action=diff#a/mail/build.mk_sec1 which feels like duplication and a problem waiting to happen, looks good.

Comment 15

[Mark Banner (:standard8)]

(In reply to comment #14)
> (From update of attachment 389381 [details] [diff] [review])
> Except for the MOZ_APP_DISPLAYNAME/APP_NAME logic in
> https://bugzilla.mozilla.org/attachment.cgi?id=389381&action=diff#a/mail/build.mk_sec1
> which feels like duplication and a problem waiting to happen, looks good.

As discussed on irc, spun this out to bug 506204.

Comment 16

[Mark Banner (:standard8)]

Comment on attachment 389381 [details] [diff] [review]
[checked in] Part 1 - initial targets

Checked in: http://hg.mozilla.org/comm-central/rev/63a765629dc8

Comment 17

[Mark Banner (:standard8)]

Comment on attachment 389382 [details] [diff] [review]
[checked in] Part 2 - tidy up

Checked in: http://hg.mozilla.org/comm-central/rev/5f48e2b67685