Last Comment Bug 794946 - Add mozmill tests for the chat toolbar and the central placeholder of the instant messaging UI
: Add mozmill tests for the chat toolbar and the central placeholder of the ins...
Status: RESOLVED FIXED
:
Product: Thunderbird
Classification: Client Software
Component: Instant Messaging (show other bugs)
: Trunk
: All All
: -- enhancement (vote)
: Thunderbird 18.0
Assigned To: Florian Quèze [:florian] [:flo]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-09-27 08:10 PDT by Florian Quèze [:florian] [:flo]
Modified: 2012-10-05 03:32 PDT (History)
1 user (show)
florian: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Patch (5.58 KB, patch)
2012-09-27 08:10 PDT, Florian Quèze [:florian] [:flo]
mconley: feedback+
Details | Diff | Splinter Review
Patch v2 (4.71 KB, patch)
2012-10-04 08:09 PDT, Florian Quèze [:florian] [:flo]
mconley: review+
Details | Diff | Splinter Review
Patch v3 (4.91 KB, patch)
2012-10-05 03:27 PDT, Florian Quèze [:florian] [:flo]
no flags Details | Diff | Splinter Review

Description Florian Quèze [:florian] [:flo] 2012-09-27 08:10:13 PDT
Created attachment 665459 [details] [diff] [review]
Patch

I started by testing that the chat tab can be opened, that the buttons of the chat toolbar are correctly disabled/enabled, and that the place holder in the middle of the chat tab is correct.

Attaching the WIP to get some feedback before I write more mozmill tests.
Comment 1 Mike Conley (:mconley) - (Needinfo me!) 2012-10-02 12:08:07 PDT
Comment on attachment 665459 [details] [diff] [review]
Patch

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

Hey Florian,

You've got the basic idea, but there's a lot of stuff from folder-display-helpers that you could / should take advantage of. See below.

Other than that, looks good!

::: mail/test/mozmill/im/test-toolbar-buttons.js
@@ +1,5 @@
> +/* This Source Code Form is subject to the terms of the Mozilla Public
> + * License, v. 2.0. If a copy of the MPL was not distributed with this
> + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
> +
> +var mozmill = {}; Components.utils.import('resource://mozmill/modules/mozmill.js', mozmill);

You probably don't need mozmill, elements lib, or frame.

This is probably going to sound horrible to you, but what generally happens is we import the folder-display-helpers module. That gives us most of the powers we need - specifically, we get things like:

* assert_true, assert_false, assert_equals
* Shortcuts for grabbing and manipulating elements, like: mc.click(me.e("button-chat"));
* Access to the 3pane controller, via mc (which you just get for free)
* I think you also get mc.tabmail
Comment 2 Florian Quèze [:florian] [:flo] 2012-10-04 08:09:44 PDT
Created attachment 667981 [details] [diff] [review]
Patch v2

Took into account Mike's review comments.
Comment 3 Mike Conley (:mconley) - (Needinfo me!) 2012-10-04 13:14:12 PDT
Comment on attachment 667981 [details] [diff] [review]
Patch v2

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

Some really simple fix-ups. Assuming this passes locally for you, with the changes I've suggested, r=me.

::: mail/test/mozmill/im/test-toolbar-buttons.js
@@ +12,5 @@
> +function setupModule(module) {
> +  collector.getModule('folder-display-helpers').installInto(module);
> +}
> +
> +function test_toolbar_and_place_holder() {

I'd like a quick documentation block describing what this test is actually testing.

@@ +16,5 @@
> +function test_toolbar_and_place_holder() {
> +  assert_true(mc.tabmail.selectedTab.mode.type != "chat",
> +              "the chat tab shouldn't be selected at startup");
> +  mc.click(mc.eid("button-chat"));
> +  assert_true(mc.tabmail.selectedTab.mode.type == "chat",

You can use assert_equals for these, like:

assert_equals(mc.tabmail.selectedTab.mode.type, "chat",
              "the chat tab should be selected");

@@ +19,5 @@
> +  mc.click(mc.eid("button-chat"));
> +  assert_true(mc.tabmail.selectedTab.mode.type == "chat",
> +              "the chat tab should be selected");
> +
> +  // Check that "No connected account" place holder is correct.

typo: "place holder" -> "placeholder". Also in a few other places in this file.

@@ +20,5 @@
> +  assert_true(mc.tabmail.selectedTab.mode.type == "chat",
> +              "the chat tab should be selected");
> +
> +  // Check that "No connected account" place holder is correct.
> +  assert_true(mc.e("conversationsDeck").selectedPanel.id == "noConvScreen",

assert_equals

@@ +29,5 @@
> +              "the 'No account' place holder is hidden");
> +  assert_true(!mc.e("noConnectedAccountInnerBox").hidden,
> +              "the 'No connected account' place holder is visible");
> +  let chatHandler = mc.window.chatHandler;
> +  assert_true(chatHandler._placeHolderButtonId == "openIMAccountManagerButton",

assert_equals

@@ +31,5 @@
> +              "the 'No connected account' place holder is visible");
> +  let chatHandler = mc.window.chatHandler;
> +  assert_true(chatHandler._placeHolderButtonId == "openIMAccountManagerButton",
> +              "the correct place holder button is visible");
> +  assert_true(mc.window.document.activeElement.id == chatHandler._placeHolderButtonId,

assert_equals

@@ +50,5 @@
> +  // Pretend the account is connected and check how the UI reacts
> +  ircAccount.reportConnected();
> +
> +  // check that add contact and join chat are no longer disabled
> +  assert_true(!mc.e("button-add-buddy").disabled,

instead of assert_true(!x), I'd prefer assert_false(x)

@@ +53,5 @@
> +  // check that add contact and join chat are no longer disabled
> +  assert_true(!mc.e("button-add-buddy").disabled,
> +              "the Add Buddy button is not disabled");
> +  assert_true(!mc.e("button-join-chat").disabled,
> +              "the Join Chat button is not disabled");

assert_false

@@ +56,5 @@
> +  assert_true(!mc.e("button-join-chat").disabled,
> +              "the Join Chat button is not disabled");
> +
> +  // Check that the "No conversations" place holder is correct.
> +  assert_true(!mc.e("noConvInnerBox").hidden,

assert_false

@@ +62,5 @@
> +  assert_true(mc.e("noAccountInnerBox").hidden,
> +              "the 'No account' place holder is hidden");
> +  assert_true(mc.e("noConnectedAccountInnerBox").hidden,
> +              "the 'No connected account' place holder is hidden");
> +  assert_true(!chatHandler._placeHolderButtonId,

assert_false

@@ +79,5 @@
> +  assert_true(mc.e("noConvInnerBox").hidden,
> +              "the 'No conversation' place holder is hidden");
> +  assert_true(mc.e("noAccountInnerBox").hidden,
> +              "the 'No account' place holder is hidden");
> +  assert_true(!mc.e("noConnectedAccountInnerBox").hidden,

assert_false

@@ +81,5 @@
> +  assert_true(mc.e("noAccountInnerBox").hidden,
> +              "the 'No account' place holder is hidden");
> +  assert_true(!mc.e("noConnectedAccountInnerBox").hidden,
> +              "the 'No connected account' place holder is visible");
> +  assert_true(chatHandler._placeHolderButtonId == "openIMAccountManagerButton",

assert_equals
Comment 4 Florian Quèze [:florian] [:flo] 2012-10-05 03:25:59 PDT
Rephrasing the bug summary to match what the test here actually tests. I initially thought I would attach other mozmill tests to this bug, but I now think it will make more sense to file a bug for each test so that we can land each of them as soon as they are ready, and track that correctly.
Comment 5 Florian Quèze [:florian] [:flo] 2012-10-05 03:27:05 PDT
Created attachment 668384 [details] [diff] [review]
Patch v3

This addresses Mike's review comments in comment 3.
Comment 6 Florian Quèze [:florian] [:flo] 2012-10-05 03:30:47 PDT
https://hg.mozilla.org/comm-central/rev/134e3483e731

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