629070 - Mozmill test for Panorama tabgroup naming

Status: RESOLVED FIXED

(Mozilla QA Graveyard :: Mozmill Tests, defect)

Description

[u279076]

Litmus ID:
https://litmus.mozilla.org/show_test.cgi?id=12649

Comment 2

[adam.fishhook+bugzilla]

Attachment: Incomplete test script

Unsure how to check 1)A text entry field should appear.
Otherwise, I'd thought it'd look more or less like the attached file.

Comment 3

[u279076]

(In reply to comment #2)
> Created attachment 512878 [details]
> Incomplete test script
> 
> Unsure how to check 1)A text entry field should appear.
> Otherwise, I'd thought it'd look more or less like the attached file.

Adam, can you change the attachment type to plain-text? It's not viewable in the browser as Javascript, thanks.

Comment 4

[u279076]

(In reply to comment #3)
> (In reply to comment #2)
> > Created attachment 512878 [details]
> > Incomplete test script
> > 
> > Unsure how to check 1)A text entry field should appear.
> > Otherwise, I'd thought it'd look more or less like the attached file.
> 
> Adam, can you change the attachment type to plain-text? It's not viewable in
> the browser as Javascript, thanks.

Nevermind, I did it myself :)

Comment 5

[u279076]

(In reply to comment #2)
> Created attachment 512878 [details]
> Incomplete test script
> 
> Unsure how to check 1)A text entry field should appear.
> Otherwise, I'd thought it'd look more or less like the attached file.

If you can get an instance of the groupTitleBox element and type a name in it, this should be sufficient.

Comment 6

[Aaron Train [:aaronmt]]

Hi Adam,

Did you still want to work on this test? Just checking because it's unassigned.

Comment 7

[Aaron Train [:aaronmt]]

Taking over this bug.

Comment 8

[Aaron Train [:aaronmt]]

Attachment: Patch v1 - automation script for testcase 12649

Simple automation script for testcase 12649 at
https://litmus.mozilla.org/show_test.cgi?id=12649

Comment 9

[Aaron Train [:aaronmt]]

Comment on attachment 516794 [details] [diff] [review]
Patch v1 - automation script for testcase 12649

Switching initial reviewer

Comment 10

[u279076]

Comment on attachment 516794 [details] [diff] [review]
Patch v1 - automation script for testcase 12649

>diff --git a/firefox/testTabView/testTabGroupNaming.js b/firefox/testTabView/testTabGroupNaming.js
>+// Include required modules
>+var tabView = require("../../shared-modules/tabview");
>+const TABGROUP_TITLE = "Mozilla";

nit, please separate "requires" and consts by a newline.

>+
>+function setupModule(module) {
>+  controller = mozmill.getBrowserController();
>+  tabView = new tabView.tabView(controller);
>+}

Please rename "tabView" to "activeTabView" to remove ambiguity with the shared module.

>+  // Check that Tab View has opened 
>+  controller.assert(function () {
>+    return tabView.isOpen;
>+  }, "Tab Groups has opened");
>+

Please refer to the Panorama view as "Tab Groups View" across all code and comments.

Comment 11

[Aaron Train [:aaronmt]]

Attachment: Patch v2 - automation script for testcase 12649

Comment 12

[Aaron Train [:aaronmt]]

Attachment: Patch v2 - automation script for testcase 12649

Comment 13

[u279076]

Comment on attachment 518068 [details] [diff] [review]
Patch v2 - automation script for testcase 12649

>+  // Check that Tab Groups View has opened 
>+  controller.assert(function () {
>+    return activeTabView.isOpen;
>+  }, "Tab Groups View has opened");

I'm not sure if this is redundant since .open() calls waitForOpen(). What do you think? Henrik?

>+  // Check that Tab Groups View has closed
>+  controller.assert(function () {
>+    return !activeTabView.isOpen;
>+  }, "Tab Groups View has closed");
>+}

Could you add a check to enter Tab Groups view and check the title once more (ie. make sure the title remains after switching in and out of Panorama)?

Comment 14

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to comment #13)
> >+  // Check that Tab Groups View has opened 
> >+  controller.assert(function () {
> >+    return activeTabView.isOpen;
> >+  }, "Tab Groups View has opened");
> 
> I'm not sure if this is redundant since .open() calls waitForOpen(). What do
> you think? Henrik?

Exactly. It's not necessary.

Comment 15

[Aaron Train [:aaronmt]]

Attachment: Patch v3 - automation script for testcase 12649

Comment 16

[u279076]

Comment on attachment 518368 [details] [diff] [review]
Patch v3 - automation script for testcase 12649

This looks good to me.  Thanks Aaron.  Over to Henrik for follow-up.

Comment 17

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Comment on attachment 518368 [details] [diff] [review]
Patch v3 - automation script for testcase 12649

Please update the locations once the refactoring has been finished.

Comment 18

[u279076]

Aaron, please submit a new patch for review by Henrik with the new file structure.

Comment 19

[Aaron Train [:aaronmt]]

Attachment: Patch v4 - automation script for testcase 12649

Updated for restructuring changes yesterday. Over to Henrik for review.

Comment 20

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Comment on attachment 518706 [details] [diff] [review]
Patch v4 - automation script for testcase 12649

>+function testTabGroupNaming() {
>+  // Open Tab Groups View (default via keyboard shortcut)
[..]
>+  // Close Tab Groups View (default via keyboard shortcut)
>+  activeTabView.close();
>+
>+  // Open Tab Groups View (default via keyboard shortcut)
>+  activeTabView.open();

Nit: There is no need to call out the default in each test.

Comment 21

[Aaron Train [:aaronmt]]

Attachment: Patch v4.1 [nit addressed][checked-in]

Nit addressed and landed on default and 2.0
http://hg.mozilla.org/qa/mozmill-tests/rev/09a6c4063f88 (default)
http://hg.mozilla.org/qa/mozmill-tests/rev/1ccaf9638b3b (2.0)