Last Comment Bug 805953 - create a shared function to determine account tree row to click for account manager test files, instead of each test hardcoding its own assumptions
: create a shared function to determine account tree row to click for account m...
Status: RESOLVED FIXED
:
Product: Thunderbird
Classification: Client Software
Component: Testing Infrastructure (show other bugs)
: Trunk
: All All
: -- normal (vote)
: Thunderbird 19.0
Assigned To: :aceman
:
:
Mentors:
Depends on: 749200
Blocks: 525024 817390
  Show dependency treegraph
 
Reported: 2012-10-26 13:39 PDT by :aceman
Modified: 2012-12-02 06:50 PST (History)
1 user (show)
ryanvm: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch (14.66 KB, patch)
2012-10-28 08:17 PDT, :aceman
mconley: review+
Details | Diff | Splinter Review

Description :aceman 2012-10-26 13:39:59 PDT
As seen in bug 749200, some tests in mail/test/mozmill/account do not properly determine which row to click in the account manager tree. They use hardcoded values or depend on account order or have assumptions that may not be met after future changes.

In this bug I propose to move the function test-archive-options.js::get_account_tree_row() into mail/test/mozmill/shared-modules/test-account-manager-helpers.js and make use of it in all of the mail/test/mozmill/account tests. They already import this module. While they do not fail now, they may break anytime in the future. So I propose to make them more robust beforehand.
Comment 1 :aceman 2012-10-28 08:17:24 PDT
Created attachment 675957 [details] [diff] [review]
patch

Preliminary patch to show what this intends to do.
Comment 2 Mike Conley (:mconley) 2012-11-07 18:17:50 PST
Comment on attachment 675957 [details] [diff] [review]
patch

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

This looks good - glad to see us chucking out repetitive code. So, am I to understand that all of our Mozmill tests automatically have access to Services and MailServices? When did that happen?

::: mail/test/mozmill/account/test-ab-whitelist.js
@@ +29,5 @@
>    amh.installInto(module);
>  
>    let server = MailServices.accounts
>                             .FindServer("tinderbox", "tinderbox", "pop3");
> +  gAccount = MailServices.accounts.FindAccountForServer(server);

So, MailServices is already available in this scope, and we don't need to re-import it? When did that happen?
Comment 4 Mike Conley (:mconley) 2012-11-09 12:11:36 PST
Comment on attachment 675957 [details] [diff] [review]
patch

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

Yep, looks good. Thanks aceman!
Comment 5 Ryan VanderMeulen [:RyanVM] 2012-11-09 12:53:47 PST
https://hg.mozilla.org/comm-central/rev/9aa8346533dd

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