790174 - Test failure 'Underlined title matches entered title - got 'communit', expected 'community'' in /testAwesomeBar/testCheckItemHighlight.js

Status: RESOLVED FIXED

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

Description

[Andreea Matei [:AndreeaMatei]]

testCheckItemHighlight.js fails at checking the underlined URL matches the entered URL. 
We have there a controller.assert() which needs to be changed.

Comment 1

[Andreea Matei [:AndreeaMatei]]

Attachment: patch v1

Replaced controller.assert() with controller.waitFor() and did testrun on Ubuntu and Mac OS X 10.7.4.

Comment 2

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

Comment on attachment 660018 [details] [diff] [review]
patch v1

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

::: tests/functional/testAwesomeBar/testCheckItemHighlight.js
@@ +93,4 @@
>  
>    // Check that the underlined URL matches the entered URL
>    underlined.forEach(function (element) {
> +    controller.waitFor(function() {

Do we really need a waitFor() call here? The item is visible for sure so I think using assert is still the right way to go.

Comment 3

[Andreea Matei [:AndreeaMatei]]

Attachment: patch v1.1

Used assert.equal as requested.

Comment 4

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

Comment on attachment 660026 [details] [diff] [review]
patch v1.1

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

::: tests/functional/testAwesomeBar/testCheckItemHighlight.js
@@ +94,5 @@
>  
>    // Check that the underlined URL matches the entered URL
>    underlined.forEach(function (element) {
> +    assert.equal(element.toLowerCase(), LOCAL_TEST_PAGES[0].name,
> +                 "Underlined URL matches entered URL");

Sorry, but I was wrong here. Looking again at the test failure it was really thrown for one of those underlined elements. So we should indeed try with waitFor(). Similar to your last patch but please update the message so that it also contains the aType parameter.

Comment 5

[Andreea Matei [:AndreeaMatei]]

Attachment: patch v1.3

Switched back to waitFor().

Comment 6

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

Comment on attachment 660030 [details] [diff] [review]
patch v1.3

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

::: tests/functional/testAwesomeBar/testCheckItemHighlight.js
@@ +97,3 @@
>        return element.toLowerCase() === LOCAL_TEST_PAGES[0].name;
>      }, "Underlined " + aType + " matches entered " + aType + " - got '" +
>         element.toLowerCase() + "', expected '" + LOCAL_TEST_PAGES[0].name + "'");

As I have mentioned on one of the other bugs lately we do not have a chance to print out the current value. Please adjust that message accordingly.

Comment 7

[Andreea Matei [:AndreeaMatei]]

Attachment: patch v2

Updated the message, leaving aType parameter and the expected value.

Comment 8

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

Comment on attachment 660069 [details] [diff] [review]
patch v2

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

I think that looks fine. I will get it in right now.

Comment 9

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

Landed on default:
http://hg.mozilla.org/qa/mozmill-tests/rev/56c01340f467

If we do not regress anything lets get it backported tomorrow.

Comment 10

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

Sadly, this patch didn't work and now we have lesser information:

http://mozmill-ci.blargon7.com/#/functional/report/671677a5d9d5ca25f3cf5ae1c48686a2

I had to back it out:
http://hg.mozilla.org/qa/mozmill-tests/rev/77e448ec6d20

Comment 11

[Andreea Matei [:AndreeaMatei]]

Attachment: debugging dumps

Dump statements in toolbars.js for the getUnderlinedText method.
The description path was checked and it's accurate.

My return for the values array is "community", all the time.

Comment 12

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

Comment on attachment 660838 [details] [diff] [review]
debugging dumps

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

I think we should add even more debug here. You could dump the type that's passed to getUnderlinedText so we can easily distinguish between the title and url calls. Also, could you add the number of the current iteration to the dump statements - it's currently quite hard to follow and looks like length does not match the number of dump statements. It might also help to use newlines more sparingly, to group dump statements together.

I'd also be curious to see what the following would return at the point of failure, to see if getting the first richlistitem again helps:
dump(locationBar.autoCompleteResults.getUnderlinedText(locationBar.autoCompleteResults.getResult(0), aType) + "\n");

Comment 13

[Andreea Matei [:AndreeaMatei]]

Attachment: debugging dumps

Indeed the length did not match the number of dumps, that is because childNodes has 3 main nodes (0, 1, 2), plus others like item, iterator and length. And the length takes only the 3 principal nodes.
In our for each (node in description.childNodes) we take all the nodes present.
For debugging, I replaced it with a simple for that would go from 0 to description.childNodes.length, so we can see the details on the main nodes and also to dump the current step.

Great idea to try again to get the richlistItem underlined. Tried on a separate function to dump, which would be called in the error message but because of all the functions wrapped there I got undefined. So I have put that into the error message and tested.

Rearranged the new lines for a clearer view.

Hopefully will have more data about the failure with all this.

Comment 14

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

Comment on attachment 661227 [details] [diff] [review]
debugging dumps

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

Landed as:
http://hg.mozilla.org/qa/mozmill-tests/rev/7e5bccbef014 (default)

Let's see what the next failures tell us...

Comment 15

[Andreea Matei [:AndreeaMatei]]

It failed here, both on Mac OS X 10.7.4:
* http://mozmill-ci.blargon7.com/#/functional/report/671677a5d9d5ca25f3cf5ae1c4c18bc4
* http://mozmill-ci.blargon7.com/#/functional/report/671677a5d9d5ca25f3cf5ae1c4c06e8f

It seems it's not retrieving the whole string on the second try either. I am not able to see the dumps results, could you or Henrik please help with that?

Comment 16

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

All the dump() output will be visible in the console on the Jenkins instance. You can't find it in the report. So work together with Vlad here.

Comment 17

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

The dump statements should not have been put into the toolbars module but in the test itself. Given the caching of output there will be no good way to compare the output for a specific test.

I backed this out:
http://hg.mozilla.org/qa/mozmill-tests/rev/afe7826d71ab

All I was able to find was:

Type: title
Nodes length: 3

Iteration number: 0
Node name: #text
innerHTML: undefined

Iteration number: 1
Node name: span
innerHTML: Communit
I got here 

Iteration number: 2
Node name: #text
innerHTML: undefined

Values: Communit

 Type: title
Nodes length: 3

Iteration number: 0
Node name: #text
innerHTML: undefined

Iteration number: 1
Node name: span
innerHTML: Communit
I got here 

Iteration number: 2
Node name: #text
innerHTML: undefined

Values: Communit

Comment 18

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

This should be fixed now by the landing of the patch on bug 792394.