Mozmill Endurance test for tab switching in Panorama

VERIFIED FIXED

Status

VERIFIED FIXED
8 years ago
7 years ago

People

(Reporter: ashughes, Assigned: ashughes)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [mozmill-endurance][mozmill-panorama])

Attachments

(1 attachment, 3 obsolete attachments)

(Assignee)

Description

8 years ago
Tracking bug for the creation of an endurance test for Panorama tab switching.  

Steps:
1. Open 2 tabs with different pages loaded
*** begin iteration ***
2. Open Panorama view
3. Switch to Tab 2
4. Open Panorama view
5. Switch to Tab 1
*** end iteration ***
(Assignee)

Comment 1

8 years ago
Created attachment 516754 [details] [diff] [review]
Patch v1

First attempt at a test for switching tabs in Panorama.
Attachment #516754 - Flags: review?(dave.hunt)
This patch works when I run it directly with Mozmill but fails with the below trace when I run using the endurance testrun script. No idea why this would be the case.

ERROR | Test Failure: {"exception": {"message": "Expression \"id(\"navigator-toolbox\")\" returned null. Anonymous == false", "lineNumber": 486, "stack": "([object Array],\"id(\\\"navigator-toolbox\\\")\",4,[object Array])@resource://mozmill/modules/elementslib.js:486\n()@resource://mozmill/modules/elementslib.js:501\n([object Object])@resource://mozmill/modules/controller.js:490\n@:0\ntabBrowser_openTab([object Proxy])@resource://mozmill/stdlib/securable-module.js -> file:///var/folders/YW/YW0PWstZHSurB3MGeMrD+E+++TI/-Tmp-/tmp9AU308.mozmill-tests/shared-modules/tabs.js:442\n@:0\ntestSwitchTabs()@resource://mozmill/modules/frame.js -> file:///var/folders/YW/YW0PWstZHSurB3MGeMrD+E+++TI/-Tmp-/tmp9AU308.mozmill-tests/firefox/enduranceTests/testTabView/testSwitchTabs.js:69\n(testSwitchTabs)@resource://mozmill/modules/frame.js:542\n([object Object])@resource://mozmill/modules/frame.js:611\n([object Object])@resource://mozmill/modules/frame.js:654\n(\"/var/folders/YW/YW0PWstZHSurB3MGeMrD+E+++TI/-Tmp-/tmp9AU308.mozmill-tests/firefox/enduranceTests/testTabView/testSwitchTabs.js\")@resource://mozmill/modules/frame.js:491\n(\"/var/folders/YW/YW0PWstZHSurB3MGeMrD+E+++TI/-Tmp-/tmp9AU308.mozmill-tests/firefox/enduranceTests/testTabView/testSwitchTabs.js\")@resource://mozmill/modules/frame.js:666\n((function (filename, invokedFromIDE) {var runner = new Runner(new Collector, invokedFromIDE);runner.runTestFile(filename);runner.end();return true;}),[object Proxy])@resource://jsbridge/modules/server.js:164\n(\"656c2622-4686-11e0-89fd-c42c03350c60\",(function (filename, invokedFromIDE) {var runner = new Runner(new Collector, invokedFromIDE);runner.runTestFile(filename);runner.end();return true;}),[object Proxy])@resource://jsbridge/modules/server.js:168\n@:0\n@resource://jsbridge/modules/server.js:249\n", "fileName": "resource://mozmill/modules/elementslib.js"}}
DEBUG | mozmill.endTest | {"passes": [{"function": "Controller.open()"}, {"function": "controller.waitFor()"}, {"function": "controller.waitForPageLoad()"}], "fails": [{"exception": {"message": "Expression \"id(\"navigator-toolbox\")\" returned null. Anonymous == false", "lineNumber": 486, "stack": "([object Array],\"id(\\\"navigator-toolbox\\\")\",4,[object Array])@resource://mozmill/modules/elementslib.js:486\n()@resource://mozmill/modules/elementslib.js:501\n([object Object])@resource://mozmill/modules/controller.js:490\n@:0\ntabBrowser_openTab([object Proxy])@resource://mozmill/stdlib/securable-module.js -> file:///var/folders/YW/YW0PWstZHSurB3MGeMrD+E+++TI/-Tmp-/tmp9AU308.mozmill-tests/shared-modules/tabs.js:442\n@:0\ntestSwitchTabs()@resource://mozmill/modules/frame.js -> file:///var/folders/YW/YW0PWstZHSurB3MGeMrD+E+++TI/-Tmp-/tmp9AU308.mozmill-tests/firefox/enduranceTests/testTabView/testSwitchTabs.js:69\n(testSwitchTabs)@resource://mozmill/modules/frame.js:542\n([object Object])@resource://mozmill/modules/frame.js:611\n([object Object])@resource://mozmill/modules/frame.js:654\n(\"/var/folders/YW/YW0PWstZHSurB3MGeMrD+E+++TI/-Tmp-/tmp9AU308.mozmill-tests/firefox/enduranceTests/testTabView/testSwitchTabs.js\")@resource://mozmill/modules/frame.js:491\n(\"/var/folders/YW/YW0PWstZHSurB3MGeMrD+E+++TI/-Tmp-/tmp9AU308.mozmill-tests/firefox/enduranceTests/testTabView/testSwitchTabs.js\")@resource://mozmill/modules/frame.js:666\n((function (filename, invokedFromIDE) {var runner = new Runner(new Collector, invokedFromIDE);runner.runTestFile(filename);runner.end();return true;}),[object Proxy])@resource://jsbridge/modules/server.js:164\n(\"656c2622-4686-11e0-89fd-c42c03350c60\",(function (filename, invokedFromIDE) {var runner = new Runner(new Collector, invokedFromIDE);runner.runTestFile(filename);runner.end();return true;}),[object Proxy])@resource://jsbridge/modules/server.js:168\n@:0\n@resource://jsbridge/modules/server.js:249\n", "fileName": "resource://mozmill/modules/elementslib.js"}}], "name": "testSwitchTabs", "filename": "/var/folders/YW/YW0PWstZHSurB3MGeMrD+E+++TI/-Tmp-/tmp9AU308.mozmill-tests/firefox/enduranceTests/testTabView/testSwitchTabs.js", "failed": 1, "passed": 3}
Sounds similar to the other failures.
(Assignee)

Comment 4

8 years ago
(In reply to comment #3)
> Sounds similar to the other failures.

Any suggestions on how to debug this? You seemed to think that there might be a change in the DOM from simply launching the Panorama view.
So it looks like this issue occurs when running the testOpenNewTab.js followed by testSwitchTab.js. I was also able to replicate the issue by simply combining these two tests into one.
(Assignee)

Comment 6

8 years ago
(In reply to comment #5)
> So it looks like this issue occurs when running the testOpenNewTab.js followed
> by testSwitchTab.js. I was also able to replicate the issue by simply combining
> these two tests into one.

Does it happen when testOpen is run before testSwitch, the other way around, or both?
Comment on attachment 516754 [details] [diff] [review]
Patch v1

+
+  tabBrowser.openTab({type: "newTabButton"});
+  

If we use the menu instead of the toolbar then this passes. We either need to fix the issue causing the failure or for the sake of this test just use the menu to open a new tab.
Attachment #516754 - Flags: review?(dave.hunt) → review-
Depends on: 638876
(Assignee)

Comment 8

8 years ago
(In reply to comment #7)
> Comment on attachment 516754 [details] [diff] [review]
> Patch v1
> 
> +
> +  tabBrowser.openTab({type: "newTabButton"});
> +  
> 
> If we use the menu instead of the toolbar then this passes. We either need to
> fix the issue causing the failure or for the sake of this test just use the
> menu to open a new tab.

I'd rather fix the problem in bug 638876 as we are seeing it affect many tests. I think all that needs to happen to trigger this failure is opening and closing Panorama.  I'll focus on fixing that bug first as it has ramifications for all Panorama tests.
(Assignee)

Comment 9

8 years ago
Comment on attachment 516754 [details] [diff] [review]
Patch v1

Putting patch up for re-review. I've tested it and this no longer fails.
Attachment #516754 - Flags: review- → review?(dave.hunt)
Comment on attachment 516754 [details] [diff] [review]
Patch v1

r+ Looks good, and now passes. Results of a testrun including this test can be found here: http://mozmill.blargon7.com/#/endurance/report/b5eb523b2dfa00631bd71a5a3c01589f
Attachment #516754 - Flags: review?(hskupin)
Attachment #516754 - Flags: review?(dave.hunt)
Attachment #516754 - Flags: review+
Comment on attachment 516754 [details] [diff] [review]
Patch v1

>+    if (activeTab.getNode().textContent === allTabs[0].getNode().textContent) {
>+        activeTabView.controller.click(allTabs[1]);
>+    } else {
>+        activeTabView.controller.click(allTabs[0]);
>+    }

Why do you not check for 'activeTab.getNode() === allTabs[0].getNode()'? Your check will fail when the same value is contained in the title.
Attachment #516754 - Flags: review?(hskupin) → review-
(Assignee)

Comment 12

8 years ago
> Why do you not check for 'activeTab.getNode() === allTabs[0].getNode()'? Your
> check will fail when the same value is contained in the title.

This is the only way I could think of to click the tab which is NOT active. Granted, your concern is correct; if both tabs have the same name, this will fail.  However, I think it's safe because we have control over that by using local test pages.
Even when we have control we shouldn't do it. It's simply bad practice and cause problems if a test file gets an update. We shouldn't break tests in such a way, because a failure like that is hard to see.
(Assignee)

Comment 14

8 years ago
(In reply to comment #13)
> Even when we have control we shouldn't do it. It's simply bad practice and
> cause problems if a test file gets an update. We shouldn't break tests in such
> a way, because a failure like that is hard to see.

Ok, then please suggest an alternative.
Everything has been said in comment 11. This code has to work.
(Assignee)

Comment 16

8 years ago
Created attachment 518399 [details] [diff] [review]
Patch v1.1

Revised the IF statement as per your suggestion.
Attachment #516754 - Attachment is obsolete: true
Attachment #518399 - Flags: review?(hskupin)
Comment on attachment 518399 [details] [diff] [review]
Patch v1.1

Please update the locations once the refactoring has been finished.
Attachment #518399 - Flags: review?(hskupin)
(Assignee)

Comment 18

8 years ago
Created attachment 518588 [details] [diff] [review]
Patch v1.2

Incorporates refactored structure.
Attachment #518399 - Attachment is obsolete: true
Attachment #518588 - Flags: review?(hskupin)
Comment on attachment 518588 [details] [diff] [review]
Patch v1.2

>+    var allTabs = activeTabView.getTabs({});

Nit: No need for the curly braces. You can leave those out.

Also you miss to close the Tab View groups in teardownModule. That will block other tests because it will still be present and block the browser.
Attachment #518588 - Flags: review?(hskupin) → review-
(Assignee)

Comment 20

8 years ago
Created attachment 519929 [details] [diff] [review]
Patch v1.3 [checked-in]

Nits addressed.
Attachment #518588 - Attachment is obsolete: true
Attachment #519929 - Flags: review?(hskupin)
Attachment #519929 - Flags: review?(hskupin) → review+
(Assignee)

Comment 21

8 years ago
Comment on attachment 519929 [details] [diff] [review]
Patch v1.3 [checked-in]

Landed:
http://hg.mozilla.org/qa/mozmill-tests/rev/3ad124fc3b76 [default]
http://hg.mozilla.org/qa/mozmill-tests/rev/07f75ec5049a [mozilla2.0]
Attachment #519929 - Attachment description: Patch v1.3 → Patch v1.3 [checked-in]
(Assignee)

Updated

8 years ago
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
(Assignee)

Updated

7 years ago
Status: RESOLVED → VERIFIED
Whiteboard: [mozmill-endurance][mozmill-panorama]
You need to log in before you can comment on or make changes to this bug.