Last Comment Bug 746014 - Some mozmill tests aren't being run currently
: Some mozmill tests aren't being run currently
Status: RESOLVED FIXED
:
Product: Thunderbird
Classification: Client Software
Component: Testing Infrastructure (show other bugs)
: unspecified
: x86_64 Windows 7
: -- normal (vote)
: Thunderbird 17.0
Assigned To: Ryan VanderMeulen [:RyanVM]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-04-16 17:07 PDT by Ryan VanderMeulen [:RyanVM]
Modified: 2012-08-27 10:58 PDT (History)
7 users (show)
standard8: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Add missing directories to Mozmill manifest (868 bytes, patch)
2012-04-16 17:10 PDT, Ryan VanderMeulen [:RyanVM]
standard8: review-
Details | Diff | Splinter Review
Patch v2 (1.47 KB, patch)
2012-04-18 17:56 PDT, Ryan VanderMeulen [:RyanVM]
standard8: review-
Details | Diff | Splinter Review
Patch v3 (3.48 KB, patch)
2012-08-22 02:30 PDT, Mark Banner (:standard8)
mconley: review+
Details | Diff | Splinter Review

Description Ryan VanderMeulen [:RyanVM] 2012-04-16 17:07:27 PDT
While creating the follow-up patch for bug 332151, I noticed that there are other directories in the mozmill testsuite which aren't currently being run since they are not included in mozmilltests.list. I've confirmed looking at a log that indeed they aren't being run. I'm assuming that's not on purpose.
Comment 1 Ryan VanderMeulen [:RyanVM] 2012-04-16 17:10:59 PDT
Created attachment 615546 [details] [diff] [review]
Add missing directories to Mozmill manifest

Add the missing directories.
Comment 3 Ryan VanderMeulen [:RyanVM] 2012-04-17 03:26:54 PDT
TEST-UNEXPECTED-FAIL | (runtestlist.py) | Exited with code 1 during directory run
TEST-UNEXPECTED-FAIL | /buildbot/comm-central-linux-opt-unittest-mozmill/build/mozmill/folder-display/test-columns.js | test-columns.js::test_apply_to_folder_and_children
TEST-UNEXPECTED-FAIL | (runtestlist.py) | Exited with code 1 during directory run
Comment 4 Ryan VanderMeulen [:RyanVM] 2012-04-17 03:30:22 PDT
I guess that's a known orange. This one looks new, though. Seems to be happening on most platforms.

TEST-UNEXPECTED-FAIL | (runtestlist.py) | Exited with code 1 during directory run
TEST-UNEXPECTED-FAIL | /builds/slave/comm-central-macosx-opt-unittest-mozmill/build/mozmill/shared-modules/test-newmailaccount-helpers.js | test-newmailaccount-helpers.js::setupModule
TEST-UNEXPECTED-FAIL | (runtestlist.py) | Exited with code 1 during directory run
Comment 5 Mark Banner (:standard8) 2012-04-17 03:38:31 PDT
Comment on attachment 615546 [details] [diff] [review]
Add missing directories to Mozmill manifest

>+bloat

This was an incomplete experiment and shouldn't be added at this time.

>+crypto

This is failing:

IOError: [Errno 2] No such file or directory: '/buildbot/comm-central-linux-opt-unittest-mozmill/build/mozmill/crypto/../../../../mailnews/test/data/db-tinderbox-invalid/cert8.db'

There's too many ../'s on that, probably coming from crypto/wrapper.py. This may need experimentation to check it works locally in-tree and as a packaged test.

>+multiple-identities

This seems to be fine.

>+shared-modules

This is just modules shared by the rest of the tests, there's no actual tests here.
Comment 6 Ryan VanderMeulen [:RyanVM] 2012-04-18 17:56:59 PDT
Created attachment 616384 [details] [diff] [review]
Patch v2

Looks like this is passing on Try.
http://build.mozillamessaging.com/tinderboxpushlog/?tree=ThunderbirdTry&rev=34f21f8450df
Comment 7 Mark Banner (:standard8) 2012-04-19 04:16:56 PDT
Comment on attachment 616384 [details] [diff] [review]
Patch v2

># HG changeset patch
># Parent 6e7dfba5f1afd5c304960ae8ed12b5be37497032
># User Ryan VanderMeulen <ryanvm@gmail.com>
>Bug 746014 - Add missing directories to Mozmill manifest so that the tests are actually run. r=mbanner
>
>diff --git a/mail/test/mozmill/crypto/wrapper.py b/mail/test/mozmill/crypto/wrapper.py
...
> def on_profile_created(profile):
...
>-  data_path = "../../../../mailnews/test/data/db-tinderbox-invalid"
>+  data_path = "../../xpcshell/tests/mailnews/data/db-tinderbox-invalid"

Unfortunately, changing this breaks running mozmill tests in the object directory and fixes it for packaged-tests style that tinderbox uses.

The best thing I can think here is to either see if the directory exists or see if there's a way the proper directory could be passed in through runtest.py.
Comment 10 Mark Banner (:standard8) 2012-08-22 02:30:54 PDT
Created attachment 654147 [details] [diff] [review]
Patch v3

Based on Ryan's patch, but I've added a check to try and account for the different data paths, and also removed the bloat code that we're not currently using.

Try server results appearing here: https://tbpl.mozilla.org/?tree=Thunderbird-Try&rev=5f71545fc27d
Comment 11 Mark Banner (:standard8) 2012-08-22 10:12:44 PDT
Comment on attachment 654147 [details] [diff] [review]
Patch v3

This passed on try server, I think the path exists stuff is the best we can do, and iirc we do it elsewhere in mozmill tests as well.
Comment 12 Mike Conley (:mconley) - (Needinfo me!) 2012-08-27 07:34:04 PDT
Comment on attachment 654147 [details] [diff] [review]
Patch v3

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

Sorry I'm so late in getting to this! Really down to the wire.

I have a couple of suggestions for test_openWindows that will allow us to wipe out some of the sleeps, and take advantage of some helper modules that we have.

Thanks,

-Mike

::: mail/test/mozmill/bloat/test_openWindows.js
@@ -2,5 @@
> -// we fix Bug 458352/Bug 500201 we can't switch bloat tests to mozmill. When
> -// we do, we need to also fix the sleeps to use events/waitFor, more information
> -// in bug 506625.
> -
> -var elementslib = {};

We prefer let over var

@@ -3,5 @@
> -// we do, we need to also fix the sleeps to use events/waitFor, more information
> -// in bug 506625.
> -
> -var elementslib = {};
> -Components.utils.import('resource://mozmill/modules/elementslib.js', elementslib);

I think Cu is available to us in this scope, so this can be shortened to Cu.import("resource://mozmill/modulse/elementslib.js", elementslib);

@@ -5,5 @@
> -
> -var elementslib = {};
> -Components.utils.import('resource://mozmill/modules/elementslib.js', elementslib);
> -var mozmill = {};
> -Components.utils.import('resource://mozmill/modules/mozmill.js', mozmill);

You never use mozmill, so we probably don't need to import it.

@@ -8,5 @@
> -var mozmill = {};
> -Components.utils.import('resource://mozmill/modules/mozmill.js', mozmill);
> -var controller = {};
> -Components.utils.import('resource://mozmill/modules/controller.js', controller);
> -

What I'd suggest instead is to import folder-display-helpers, which gives us mc for free.

@@ -11,5 @@
> -Components.utils.import('resource://mozmill/modules/controller.js', controller);
> -
> -var mc = null;
> -
> -var setupModule = function(module) {

For consistency's sake, I think I'd prefer

function setupModule(module) {
 // ...
}

@@ -13,5 @@
> -var mc = null;
> -
> -var setupModule = function(module) {
> -  mc = mozmill.getMail3PaneController();
> -  controller.sleep(10000);

Why the sleep?

@@ -18,5 @@
> -};
> -
> -var test_start = function() {
> -  // We have to do this manually, MozMill doesn't have a function for non-browser windows.
> -  mc.click(new elementslib.Elem(mc.window.document.getElementById("button-address")));

You can use:

mc.click(mc.e("button-address"));
mc.click(me.e("button-newmsg"));

Though I think I'd prefer if each test opened and closed their own windows internally, as opposed to having each individual test rely on one another.

@@ -21,5 @@
> -  // We have to do this manually, MozMill doesn't have a function for non-browser windows.
> -  mc.click(new elementslib.Elem(mc.window.document.getElementById("button-address")));
> -  mc.click(new elementslib.Elem(mc.window.document.getElementById("button-newmsg")));
> -
> -  mc.sleep(5000);

Why the sleep?

@@ -25,5 @@
> -  mc.sleep(5000);
> -}
> -
> -var test_addressBook = function() {
> -  var abWindow = mozmill.getAddrbkController();

If you import the address book helper module, you can do:

let abController = open_address_book_window();
abController.click(abController.e("menu_close"));

open_address_book_window will take care of opening the window, and waiting for it to load, so you can avoid the sleeps. I think it assumes you've imported folder-display-helpers.

See: http://mxr.mozilla.org/comm-central/source/mail/test/mozmill/shared-modules/test-address-book-helpers.js#138

@@ -30,5 @@
> -  abWindow.click(new elementslib.Elem(abWindow.window.document.getElementById("menu_close")));
> -}
> -
> -var test_compose = function() {
> -  var wm = Components.classes["@mozilla.org/appshell/window-mediator;1"]

Like above, if you import the compose helpers, you can just do:

let cController = open_compose_new_mail();
close_compose_window(cController);

As above, open_compose_new_mail will take care of the waiting for you. I think it assumes you've imported folder-display-helpers.

See: http://mxr.mozilla.org/comm-central/source/mail/test/mozmill/shared-modules/test-compose-helpers.js

@@ -37,5 @@
> -  var composeWindow = wm.getMostRecentWindow("msgcompose");
> -  composeWindow.close();
> -}
> -
> -var test_shutdown = function() {

We shouldn't need this function. The fewer sleeps the better.
Comment 13 Mike Conley (:mconley) - (Needinfo me!) 2012-08-27 10:38:51 PDT
Comment on attachment 654147 [details] [diff] [review]
Patch v3

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

Silly me - all that pink stuff means removed. Good riddance!
Comment 14 Mark Banner (:standard8) 2012-08-27 10:57:51 PDT
Checked in: https://hg.mozilla.org/comm-central/rev/27d2409937a1
Comment 15 Mark Banner (:standard8) 2012-08-27 10:58:16 PDT
Thanks Ryan for starting this :-)

Note You need to log in before you can comment on or make changes to this bug.