957112 - The inclusion of a comment inside of search.xml has caused the engine popup to move inside the DOM tree

Status: RESOLVED FIXED

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

Comment 1

[mario garbi]

Happened today on OSX and Linux with Nightly 29 across multiple testSearch/ tests.

Comment 2

[mario garbi]

http://mozmill-daily.blargon7.com/#/functional/report/40742e0746fd767e6d2fd58659b6f035
http://mozmill-daily.blargon7.com/#/functional/report/40742e0746fd767e6d2fd58659b6d8ce

Comment 3

[mario garbi]

I can reproduce this locally:
http://mozmill-crowd.blargon7.com/#/functional/report/501afab09869849e0d54cead6305d7cb

I am investigating this issue and will come back with more info

Comment 4

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

This failure is breaking all of our search tests across platforms for Firefox 29.0. What was the outcome so far? Why have those tests not been skipped if no solution can be provided in time?

Comment 5

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

This regressed from yesterdays build in Nightly:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=14ac61461f2a&tochange=e7a366c1036c

Stack from a failing test:

"stack": [
          "_byAnonAttrib@resource://mozmill/driver/elementslib.js:378", 
          "_anonByAttrib@resource://mozmill/driver/elementslib.js:426", 
          "Lookup/reduceLookup@resource://mozmill/driver/elementslib.js:516", 
          "Lookup@resource://mozmill/driver/elementslib.js:536", 
          "Lookup@resource://mozmill/driver/mozelement.js:79", 
          "searchBar_getElement@resource://mozmill/stdlib/securable-module.js -> file:///mozilla/code/mozmill-tests/nightly/firefox/lib/search.js:672", 
          "searchBar.prototype.enginesDropDownOpen@resource://mozmill/stdlib/securable-module.js -> file:///mozilla/code/mozmill-tests/nightly/firefox/lib/search.js:460", 
          "searchBar.prototype.enginesDropDownOpen@resource://mozmill/stdlib/securable-module.js -> file:///mozilla/code/mozmill-tests/nightly/firefox/lib/search.js:468", 
          "searchBar_openEngineManager@resource://mozmill/stdlib/securable-module.js -> file:///mozilla/code/mozmill-tests/nightly/firefox/lib/search.js:804", 
          "testRemoveEngine@resource://mozmill/modules/frame.js -> file:///mozilla/code/mozmill-tests/nightly/firefox/tests/functional/testSearch/testRemoveSearchEngine.js:28", 
          "Runner.prototype.execFunction@resource://mozmill/modules/frame.js:752", 
          "Runner.prototype.runTestModule@resource://mozmill/modules/frame.js:706", 
          "Runner.prototype.runTestFile@resource://mozmill/modules/frame.js:690", 
          "runTestFile@resource://mozmill/modules/frame.js:775", 
          "Bridge.prototype._execFunction@resource://jsbridge/modules/Bridge.jsm:140", 
          "Bridge.prototype.execFunction@resource://jsbridge/modules/Bridge.jsm:147", 
          "@resource://jsbridge/modules/Server.jsm:32", 

The problem is that we also iterate in _byAnonAttrib() over Comments now:
https://github.com/mozilla/mozmill/blob/master/mozmill/mozmill/extension/resource/driver/elementslib.js#L378

******** parent [object XULElement]
******** parent [object Comment]

Comment 6

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

The regression range here is:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=a101c9691b15&tochange=93fe56269382

So by checking the changesets bug 601409 seems to be the regressor. Looks like we should be able to fix it easily. And lets hope we can get rid of the lookup code soon for the search library.

Comment 7

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

Attachment: Patch v1

No one else from my team is online at the moment to review. So I hope Gavin its fine when I also ask you. I know you are not familiar with our Mozmill tests but that is an easy one and it has been caused by a change made by yourself.

Comment 8

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

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

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

I haven't tested but I can see that this would work, however I'd prefer a solution that excluded the comment rather than a fragile locator by index.

Comment 9

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

I would love so too, but there is no way right now. As said earlier on this bug we should move to use the nodeCollector instead. And this work should happen ASAP. So I might have to take a stab on bug 879962 in the near future.

Landed on default:
http://hg.mozilla.org/qa/mozmill-tests/rev/64cfed39855a

Comment 10

[mario garbi]

I should have skipped the tests but I was close to finding a fix when I had to leave and I figured I would work on it early this morning. It was a miscalculation on my part and I'm sorry if it caused any problems.

Comment 11

[mario garbi]

We have seen this again on 09.01:
http://mozmill-crowd.blargon7.com/#/functional/report/850f11101678d9fbefeb95e0bd54ee01
http://mozmill-crowd.blargon7.com/#/functional/report/501afab09869849e0d54cead6305d7cb

I have uploaded a patch to replace lookups with nodeCollector which should also fix this in bug 879946.

Comment 12

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

Hm, that's very strange. Does it happen all the time or intermittently?

Comment 13

[mario garbi]

As far as I can tell we had another set of failures on 9th of January, information gathered from the top failures:

http://mozmill-crowd.blargon7.com/#/functional/failure?branch=All&platform=All&from=2014-01-03&to=2014-01-10&test=%2FtestSearch%2FtestGetMoreSearchEngines.js&func=testGetMoreEngines
http://mozmill-crowd.blargon7.com/#/functional/failure?branch=29.0&platform=Linux&from=2014-01-03&to=2014-01-10&test=%2FtestSearch%2FtestOpenSearchAutodiscovery.js&func=testOpenSearchAutodiscovery
http://mozmill-crowd.blargon7.com/#/functional/failure?branch=29.0&platform=Linux&from=2014-01-03&to=2014-01-10&test=%2FtestSearch%2FtestReorderSearchEngines.js&func=testReorderEngines

Hopefully changing the library to use nodeCollector will take care of issues like this.

Comment 14

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

All of them are only happening on Ubuntu 12.04 but not 13.04 or 13.10. Could someone do a quick check if that is reproducible on that platform?

Comment 15

[Andreea Matei [:AndreeaMatei]]

I can reproduce every time on a 12.04 machine, with latest nightly. Tried only by running a test from the testSearch folder, not in testrun. On 13.04 it passes, at least in the 5 times I ran.

Comment 16

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

Would you mind to check what is wrong with the lookup string? What happens if you use 0 again for the index?

Comment 17

[Andreea Matei [:AndreeaMatei]]

Hm, my bad. Now that you mentioned the index, I was using [0] cause my repo was up to date but on aurora branch. It's not failing on 12.04.

Comment 18

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

All those reports as listed above are from mozmill-crowd and not from mozmill-daily. So the results are not from our CI runs but from people most likely running a non-update version of mozmill-tests like Andreea did. So as long as we do not see failures in Jenkins this bug is fixed.