Some mozmill tests aren't being run currently

RESOLVED FIXED in Thunderbird 17.0

Status

Thunderbird
Testing Infrastructure
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: RyanVM, Assigned: RyanVM)

Tracking

unspecified
Thunderbird 17.0
x86_64
Windows 7
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

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.
Created attachment 615546 [details] [diff] [review]
Add missing directories to Mozmill manifest

Add the missing directories.
Assignee: nobody → ryanvm
Status: NEW → ASSIGNED
Attachment #615546 - Flags: review?(mbanner)
http://build.mozillamessaging.com/tinderboxpushlog/?tree=ThunderbirdTry&rev=cfd7bee9021d

Updated

5 years ago
Component: Mozmill → Testing Infrastructure
Product: Testing → Thunderbird
QA Contact: mozmill → testing-infrastructure
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
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 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.
Attachment #615546 - Flags: review?(mbanner) → review-
Created attachment 616384 [details] [diff] [review]
Patch v2

Looks like this is passing on Try.
http://build.mozillamessaging.com/tinderboxpushlog/?tree=ThunderbirdTry&rev=34f21f8450df
Attachment #615546 - Attachment is obsolete: true
Attachment #616384 - Flags: review?(mbanner)
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.
Attachment #616384 - Flags: review?(mbanner) → review-

Comment 8

5 years ago
http://build.mozillamessaging.com/tinderboxpushlog/?tree=ThunderbirdTry&rev=34f21f8450df

Comment 9

5 years ago
http://build.mozillamessaging.com/tinderboxpushlog/?tree=ThunderbirdTry&rev=
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
Attachment #616384 - Attachment is obsolete: true
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.
Attachment #654147 - Flags: review?(mconley)
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.
Attachment #654147 - Flags: review?(mconley) → review-
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!
Attachment #654147 - Flags: review- → review+
Checked in: https://hg.mozilla.org/comm-central/rev/27d2409937a1
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 17.0
Thanks Ryan for starting this :-)
You need to log in before you can comment on or make changes to this bug.