874395 - Test failure in /testPopups/testPopupsBlocked.js due to the tab browser element not found

Status: RESOLVED FIXED

(Mozilla QA Graveyard :: Mozmill Tests, defect, P1)

Description

[Daniela Petrovici]

testPopupsBlocked fails on all OS with Australis build - line that makes the test fail:
http://hg.mozilla.org/qa/mozmill-tests/file/05730582186d/lib/tabs.js#l22

TAB_BROWSER lookup expression needs to contain: '/id("content-deck")' element also. The line should be changed to:
const TABS_BROWSER = TABS_VIEW + '/id("content-deck")/id("browser")/id("appcontent")/id("content")';

NOTE: The above line should be changed only if nodeCollector cannot be used instead of Lookup expression

Comment 1

[mario garbi]

The patch for 874393 will introduce new library elements that are needed for this bug, therefore I will set it dependent of that.

Comment 2

[mario garbi]

Attachment: australis_toolbar_2407.patch

Updated all libraries and tests that have Lookup or element issues created by the new Australis nodes. This will also fix bug 874394 and updates the patch landed on Bug 874393 with the changes requested by Henrik.

I had to change the getElement function to handle the button element that had it's class name changed in Australis by introducing a check in each case instead of only checking for Australis once at the start.

Linux:
UX
http://mozmill-crowd.blargon7.com/#/functional/report/180cf2548ef2865af3ae441d61edebbe
http://mozmill-crowd.blargon7.com/#/functional/report/180cf2548ef2865af3ae441d61ee89c7

Nightly
http://mozmill-crowd.blargon7.com/#/functional/report/180cf2548ef2865af3ae441d61ed6800
http://mozmill-crowd.blargon7.com/#/functional/report/180cf2548ef2865af3ae441d61edb7ab

Windows:
UX
http://mozmill-crowd.blargon7.com/#/functional/report/180cf2548ef2865af3ae441d61ed8fcb
http://mozmill-crowd.blargon7.com/#/functional/report/180cf2548ef2865af3ae441d61edb0e7

Nightly
http://mozmill-crowd.blargon7.com/#/functional/report/180cf2548ef2865af3ae441d61f6a78e
http://mozmill-crowd.blargon7.com/#/functional/report/180cf2548ef2865af3ae441d61f72131

Mac:
UX
http://mozmill-crowd.blargon7.com/#/functional/report/180cf2548ef2865af3ae441d61ebba07
http://mozmill-crowd.blargon7.com/#/functional/report/180cf2548ef2865af3ae441d61ee56e2

Nightly
http://mozmill-crowd.blargon7.com/#/functional/report/180cf2548ef2865af3ae441d61ed828a
http://mozmill-crowd.blargon7.com/#/functional/report/180cf2548ef2865af3ae441d61edc379

Comment 3

[mario garbi]

Attachment: australistTabs_2507.patch

Wrong patch, uploaded the right one now.

Comment 4

[Andreea Matei [:AndreeaMatei]]

Comment on attachment 780990 [details] [diff] [review]
australistTabs_2507.patch

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

While this looks good regarding the new updates, we still have some work to do here.
Cosmin or Andrei, could one of you please work on this since Mario is still in PTO? Thanks!

::: lib/toolbars.js
@@ +21,3 @@
>  const URLBAR_INPUTBOX = URLBAR_CONTAINER + '/id("urlbar")' +
>                                             '/anon({"anonid":"textbox-container"})' +
>                                             '/anon({"anonid":"textbox-input-box"})';

This is not fixing bug 874394 with the latest australis build. I see we have another 'urlbar-wrapper' id before 'urlbar' and still something failing with the context menu.

http://mozmill-crowd.blargon7.com/#/functional/report/b3aefd29a5c41bda1cc43616d3820276

::: lib/utils.js
@@ +161,4 @@
>  
> +    switch (aSpec) {
> +      case "search" :
> +        nodeElem = (this.isAustralis()) ? '/id("nav-bar-customizationtarget")' : "";

Not sure if "search" is the right case name here, from what I see "nav-bar-customizationtarget" represents the whole navbar including location bar, navigation buttons, search bar, download and bookmark star buttons.

Comment 5

[Andrei Eftimie]

Attachment: 3.patch

Renamed "search" to "nav-bar-wrapper".

Also fixed the context-manu fail (tracked in bug 899943) by waiting the tab to load properly, by waiting on the disappearance of the "busy" tag.
The fix appears to be working reliably.

Comment 6

[Andrei Eftimie]

And some testruns with the new patch:

Nightly
=======

OSX: http://mozmill-crowd.blargon7.com/#/functional/report/b3aefd29a5c41bda1cc43616d3bcec14
http://mozmill-crowd.blargon7.com/#/functional/report/b3aefd29a5c41bda1cc43616d3bce03e

Windows: http://mozmill-crowd.blargon7.com/#/functional/report/b3aefd29a5c41bda1cc43616d3bebd6d
http://mozmill-crowd.blargon7.com/#/functional/report/b3aefd29a5c41bda1cc43616d3bea83e

UX
==

OSX: http://mozmill-crowd.blargon7.com/#/functional/report/b3aefd29a5c41bda1cc43616d3bc8add
http://mozmill-crowd.blargon7.com/#/functional/report/b3aefd29a5c41bda1cc43616d3bbf879

Windows: http://mozmill-crowd.blargon7.com/#/functional/report/b3aefd29a5c41bda1cc43616d3bf9fa1
http://mozmill-crowd.blargon7.com/#/functional/report/b3aefd29a5c41bda1cc43616d3bf139d

Comment 7

[Andreea Matei [:AndreeaMatei]]

Comment on attachment 783733 [details] [diff] [review]
3.patch

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

One more thing and we're good to land this :)

::: lib/tabs.js
@@ +129,5 @@
>     */
>    set selectedIndex(index) {
> +    var tab = this.getTab(index);
> +    assert.waitFor(function() {
> +      return !!tab.getNode().getAttribute("busy") !== true;

I think we can simplify this and wait for !tab.getNode().hasAttribute("busy"), this is how it's used on tests from mxr.

Comment 8

[Andrei Eftimie]

Attachment: 4.patch

Ok, lets this landed.

It works fine with just checking the availability of the busy attribute.
Searched through the code and it is never set to false.

Comment 9

[Andrei Eftimie]

BTW. passing testrun on UX with the new 'busy' attribute check:
OSX: http://mozmill-crowd.blargon7.com/#/functional/report/b3aefd29a5c41bda1cc43616d3ee23e1
http://mozmill-crowd.blargon7.com/#/functional/report/b3aefd29a5c41bda1cc43616d3ed7df3

Comment 10

[Andreea Matei [:AndreeaMatei]]

Comment on attachment 784357 [details] [diff] [review]
4.patch

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

Looks good:
http://hg.mozilla.org/qa/mozmill-tests/rev/4a76da6bfff0 (default)

Thanks Mario and Andrei :)