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

RESOLVED FIXED in Thunderbird 3.0b4

Status

RESOLVED FIXED
10 years ago
9 years ago

People

(Reporter: standard8, Assigned: standard8)

Tracking

Trunk
Thunderbird 3.0b4
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 7 obsolete attachments)

(Assignee)

Description

10 years ago
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.
(Assignee)

Comment 1

10 years ago
Created attachment 387543 [details] [diff] [review]
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.
Assignee: nobody → bugzilla
Status: NEW → ASSIGNED
(Assignee)

Comment 2

10 years ago
Created attachment 387545 [details] [diff] [review]
Core Fixes

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

Updated

10 years ago
Depends on: 503886
(Assignee)

Comment 3

10 years ago
Created attachment 388317 [details] [diff] [review]
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.
Attachment #387543 - Attachment is obsolete: true
(Assignee)

Comment 4

10 years ago
Created attachment 388461 [details] [diff] [review]
WIP v2a

Minor change s/SOLO_FILE/SOLO_TEST/
Attachment #388317 - Attachment is obsolete: true
(Assignee)

Comment 5

10 years ago
Created attachment 388549 [details] [diff] [review]
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.
Attachment #388461 - Attachment is obsolete: true
(Assignee)

Updated

10 years ago
Blocks: 504304
(Assignee)

Comment 6

10 years ago
Comment on attachment 387545 [details] [diff] [review]
Core Fixes

This patch is no longer needed, use the appropriate patch from bug 503886.
Attachment #387545 - Attachment is obsolete: true
(Assignee)

Comment 7

10 years ago
Created attachment 388694 [details] [diff] [review]
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.
Attachment #388549 - Attachment is obsolete: true
Attachment #388694 - Flags: review?(gozer)
Attachment #388694 - Flags: review?(bugmail)
Created attachment 388820 [details] [diff] [review]
v1 changes to Standard8's patch to make it work on linux for me
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...
Attachment #388694 - Flags: review?(bugmail) → review+
(Assignee)

Comment 10

10 years ago
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.
Summary: Package mail/test/mozmill as part of make package-tests and make runtest.py work in that situation → Create mozmill and mozmill-one make targets for build automation to use
Target Milestone: --- → Thunderbird 3.0b4
(Assignee)

Comment 11

10 years ago
Created attachment 389381 [details] [diff] [review]
[checked in] Part 1 - initial targets

This is my original fix plus Andrew's changes. One more patch to come in a moment.
Attachment #388694 - Attachment is obsolete: true
Attachment #388820 - Attachment is obsolete: true
Attachment #389381 - Flags: review?(gozer)
Attachment #388694 - Flags: review?(gozer)
(Assignee)

Comment 12

10 years ago
Created attachment 389382 [details] [diff] [review]
[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.
Attachment #389382 - Flags: review?(bugmail)
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...
Attachment #389382 - Flags: review?(bugmail) → review+
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.
Attachment #389381 - Flags: review?(gozer) → review+
(Assignee)

Updated

10 years ago
Blocks: 506202
(Assignee)

Comment 15

10 years ago
(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.
(Assignee)

Comment 16

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

Checked in: http://hg.mozilla.org/comm-central/rev/63a765629dc8
Attachment #389381 - Attachment description: Part 1 - initial targets → [checked in] Part 1 - initial targets
(Assignee)

Comment 17

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

Checked in: http://hg.mozilla.org/comm-central/rev/5f48e2b67685
Attachment #389382 - Attachment description: Part 2 - tidy up → [checked in] Part 2 - tidy up
(Assignee)

Updated

10 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.