635305 - Mozmill Endurance test for tab switching in Panorama

Status: VERIFIED FIXED

(Mozilla QA Graveyard :: Mozmill Tests, defect)

Description

[u279076]

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 ***

Comment 1

[u279076]

Attachment: Patch v1

First attempt at a test for switching tabs in Panorama.

Comment 2

[Dave Hunt [:davehunt] [he/him] ⌚BST]

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}

Comment 3

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

Sounds similar to the other failures.

Comment 4

[u279076]

(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.

Comment 5

[Dave Hunt [:davehunt] [he/him] ⌚BST]

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.

Comment 6

[u279076]

(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 7

[Dave Hunt [:davehunt] [he/him] ⌚BST]

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.

Comment 8

[u279076]

(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.

Comment 9

[u279076]

Comment on attachment 516754 [details] [diff] [review]
Patch v1

Putting patch up for re-review. I've tested it and this no longer fails.

Comment 10

[Dave Hunt [:davehunt] [he/him] ⌚BST]

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

Comment 11

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

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.

Comment 12

[u279076]

> 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.

Comment 13

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

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.

Comment 14

[u279076]

(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.

Comment 15

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

Everything has been said in comment 11. This code has to work.

Comment 16

[u279076]

Attachment: Patch v1.1

Revised the IF statement as per your suggestion.

Comment 17

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

Comment on attachment 518399 [details] [diff] [review]
Patch v1.1

Please update the locations once the refactoring has been finished.

Comment 18

[u279076]

Attachment: Patch v1.2

Incorporates refactored structure.

Comment 19

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

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.

Comment 20

[u279076]

Attachment: Patch v1.3 [checked-in]

Nits addressed.

Comment 21

[u279076]

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]