Closed
Bug 500142
Opened 16 years ago
Closed 15 years ago
Create mozmill and mozmill-one make targets for build automation to use
Categories
(Thunderbird :: Build Config, defect)
Thunderbird
Build Config
Tracking
(Not tracked)
RESOLVED
FIXED
Thunderbird 3.0b4
People
(Reporter: standard8, Assigned: standard8)
References
Details
Attachments
(2 files, 7 obsolete files)
14.58 KB,
patch
|
gozer
:
review+
|
Details | Diff | Splinter Review |
5.15 KB,
patch
|
asuth
:
review+
|
Details | Diff | Splinter Review |
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•16 years ago
|
||
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•16 years ago
|
||
I'll put the core patch here for now until I've tested it a bit more.
Assignee | ||
Comment 3•16 years ago
|
||
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•16 years ago
|
||
Minor change s/SOLO_FILE/SOLO_TEST/
Attachment #388317 -
Attachment is obsolete: true
Assignee | ||
Comment 5•16 years ago
|
||
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 | ||
Comment 6•15 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•15 years ago
|
||
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)
Comment 8•15 years ago
|
||
Comment 9•15 years ago
|
||
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•15 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•15 years ago
|
||
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•15 years ago
|
||
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 13•15 years ago
|
||
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 14•15 years ago
|
||
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 | ||
Comment 15•15 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•15 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•15 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•15 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•