The default bug view has changed. See this FAQ.

Add mozmill tests for the chat toolbar and the central placeholder of the instant messaging UI

RESOLVED FIXED in Thunderbird 18.0

Status

Thunderbird
Instant Messaging
--
enhancement
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: florian, Assigned: florian)

Tracking

Trunk
Thunderbird 18.0
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Description

5 years ago
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.
Attachment #665459 - Flags: feedback?(mconley)
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
Attachment #665459 - Flags: feedback?(mconley) → feedback+
(Assignee)

Comment 2

5 years ago
Created attachment 667981 [details] [diff] [review]
Patch v2

Took into account Mike's review comments.
Attachment #665459 - Attachment is obsolete: true
Attachment #667981 - Flags: review?(mconley)
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
Attachment #667981 - Flags: review?(mconley) → review+
(Assignee)

Comment 4

5 years ago
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.
Summary: Add mozmill tests for the instant messaging UI → Add mozmill tests for the chat toolbar and the central placeholder of the instant messaging UI
(Assignee)

Comment 5

5 years ago
Created attachment 668384 [details] [diff] [review]
Patch v3

This addresses Mike's review comments in comment 3.
(Assignee)

Comment 6

5 years ago
https://hg.mozilla.org/comm-central/rev/134e3483e731
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 18.0
(Assignee)

Updated

5 years ago
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.