685854 - Failure in /testSearch/testOpenSearchAutodiscovery.js | controller.assertJS: Failed for 'subject.installableEngines.length == 1'

Status: RESOLVED WONTFIX

(Mozilla QA Graveyard :: Mozmill Tests, defect)

Description

[Remus Pop (:RemusPop)]

TEST: testOpenSearchAutodiscovery.js::testOpenSearchAutodiscovery
ERROR: controller.assertJS: Failed for 'subject.installableEngines.length == 1' 
WHEN: 2011-09-09
FIRST: 2011-08-26
BRANCHES: default, aurora, beta, release

Comment 1

[Remus Pop (:RemusPop)]

Attachment: patch 1st assertJS

Changed the first assertJS.

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

Comment on attachment 559460 [details] [diff] [review]
patch 1st assertJS

Please always replace all existent assertJS calls when you on it already.

Comment 3

[u279076]

Comment on attachment 559460 [details] [diff] [review]
patch 1st assertJS

Just a reminder, if a patch is ready for review between 7am and 7pm PDT, please use me as the reviewer; this will allow Henrik to work on his projects. Henrik should only be used for review when I am not available.

>-  controller.assertJS("subject.installableEngines.length == 1",
>-                      {installableEngines: addEngines});
>+  expect.equal(addEngines.length, 1, "Autodiscovered OpenSearch Engines");
> 

I think this should be an assert instead of an expect. We can't install an engine if one is not discovered.

Comment 4

[Remus Pop (:RemusPop)]

Attachment: patch assertions

Patched both assertions to reflect the new assertions API.

Comment 5

[Remus Pop (:RemusPop)]

Attachment: patch assertions

r? Anthony

Comment 6

[u279076]

Comment on attachment 559470 [details] [diff] [review]
patch assertions

>   var addEngines = searchBar.installableEngines;
>-  controller.assertJS("subject.installableEngines.length == 1",
>-                      {installableEngines: addEngines});
>+  expect.equal(addEngines.length, 1, "Autodiscovered OpenSearch Engines");

I think this should be an assert not an expect since we can't do the rest of the test if an Open Search engine is not found.

>-  controller.assertJS("subject.placeholder == subject.engineName", {
>-    placeholder: inputField.getNode().placeholder,
>-    engineName: SEARCH_ENGINE.name
>-  });
>+  expect.equal(inputField.getNode().placeholder, SEARCH_ENGINE.name, "Selected search engine");

This one is fine because there is no code following which depends on a TRUE state.

Comment 7

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

(In reply to Anthony Hughes, Mozilla QA (irc: ashughes) from comment #6)
> >-  controller.assertJS("subject.placeholder == subject.engineName", {
> >-    placeholder: inputField.getNode().placeholder,
> >-    engineName: SEARCH_ENGINE.name
> >-  });
> >+  expect.equal(inputField.getNode().placeholder, SEARCH_ENGINE.name, "Selected search engine");
> 
> This one is fine because there is no code following which depends on a TRUE
> state.

Whereby the message should be "New engine is selected, and its name displayed as placeholder".

Comment 8

[Remus Pop (:RemusPop)]

Attachment: patch assertions v2 [checked-in]

Used assert for the first assertion.

Comment 9

[u279076]

Comment on attachment 559844 [details] [diff] [review]
patch assertions v2 [checked-in]

This looks fine to me. I'll have to check it in later since I'm not near my laptop right now.

Comment 10

[u279076]

Comment on attachment 559844 [details] [diff] [review]
patch assertions v2 [checked-in]

Landed:
http://hg.mozilla.org/qa/mozmill-tests/rev/cb23146d1058 (default)
http://hg.mozilla.org/qa/mozmill-tests/rev/09f058d30379 (mozilla-aurora)
http://hg.mozilla.org/qa/mozmill-tests/rev/ddfde89ed542 (mozilla-beta)
http://hg.mozilla.org/qa/mozmill-tests/rev/44939626c107 (mozilla-release)

Comment 11

[u279076]

Please mark VERIFIED if this failure is no longer present in tomorrow's results; reopen otherwise.

Thanks.

Comment 12

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

This patch does not fix the failure. It's just for getting more information about the currently happening failure in installing the search engine. Reopening the bug.

Comment 13

[u279076]

(In reply to Henrik Skupin (:whimboo) from comment #12)
> This patch does not fix the failure. It's just for getting more information
> about the currently happening failure in installing the search engine.
> Reopening the bug.

My bad, thanks.

Comment 14

[Remus Pop (:RemusPop)]

Attachment: skip test

Skips the test.

Comment 15

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

So do we have any new information here since the last check-in happened?  See my comment 12.

Comment 16

[Remus Pop (:RemusPop)]

No more info Henrik. It seems it cannot find the search engine in the drop-down popup.
I would really like to see this reproduced and exactly see what happens.

Comment 17

[Alex Lakatos[:AlexLakatos]]

Comment on attachment 563349 [details] [diff] [review]
skip test

We usually add a comment before skipping the test. Use:

>// XXX: Bug 685854 - Failure in /testSearch/testOpenSearchAutodiscovery.js |
>// controller.assertJS: Failed for 'subject.installableEngines.length == 1'
>+setupModule.__force_skip__ = "Bug 685854 - Failure in /testSearch/testOpenSearchAutodiscovery.js " +
>+                             "| controller.assertJS: Failed for 'subject.installableEngines.length == 1'";
>+teardownModule.__force_skip__ = "Bug 685854 - Failure in /testSearch/testOpenSearchAutodiscovery.js " +
>+                                "| controller.assertJS: Failed for 'subject.installableEngines.length == 1'";

Comment 18

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

(In reply to Remus Pop (:RemusPop) from comment #16)
> No more info Henrik. It seems it cannot find the search engine in the
> drop-down popup.
> I would really like to see this reproduced and exactly see what happens.

Are we using the referenced search engine in a search test which gets executed ahead? Probably we do not correctly clean-up the installed engines. Can you please check?

Comment 19

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

(In reply to Alex Lakatos from comment #17)
> We usually add a comment before skipping the test. Use:

I don't think we have to. All the information is already included in the skip lines.

Comment 20

[Remus Pop (:RemusPop)]

> Are we using the referenced search engine in a search test which gets
> executed ahead? Probably we do not correctly clean-up the installed engines.
> Can you please check?

This is the only test in which we use that engine. Keep in mind that this happens only on win2k.

Comment 21

[Alex Lakatos[:AlexLakatos]]

(In reply to Henrik Skupin (:whimboo) from comment #19)
> I don't think we have to. All the information is already included in the
> skip lines.

In our repo, except conditional skips, all have a comment, even though it's the same as the skip message. If we want to be consistent, we either should add one here or remove the others in a separate bug.

Comment 22

[Remus Pop (:RemusPop)]

Attachment: skip test v2 [checked-in]

Comment 23

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

(In reply to Remus Pop (:RemusPop) from comment #20)
> This is the only test in which we use that engine. Keep in mind that this
> happens only on win2k.

Anthony, can you please do a quick test for this test and the whole folder, just to check if it is easily reproducable? Would be great.

Comment 24

[Remus Pop (:RemusPop)]

http://mozmill-crowd.brasstacks.mozilla.com/#/functional/report/18fd2c24fc56c33feb3c8a50f4ed5458
http://mozmill-crowd.brasstacks.mozilla.com/#/functional/report/18fd2c24fc56c33feb3c8a50f4ed6917
http://mozmill-crowd.brasstacks.mozilla.com/#/functional/report/18fd2c24fc56c33feb3c8a50f4ed9a82

I have set up a win2k machine to run the functional testrun. I could not reproduce this issue. Above are the results with 0 fails for FF Beta (8).

I will try to run the functional testrun 100 times to see if anything shows up.

Comment 25

[u279076]

I was able to reproduce this by running the testSearch folder on qa-set in the Win2k VM. Running the test alone does not reproduce the failure.

The minimized failure case is an interaction between testGetMoreSearchEngines.js and the hand-off to testOpenSearchAutoDiscovery.js. Maybe a garbage collector issue.

Remus, can you provide a patch which refactors testGetMoreSearchEngines.js to bring it up to our standards for new tests? Attach it to this bug and I will apply it locally to the Win2K VM on qa-set.

Thanks

Comment 26

[u279076]

In the meantime, I'm going to set this test as skipped so the failures clear up.

Comment 27

[u279076]

Comment on attachment 563402 [details] [diff] [review]
skip test v2 [checked-in]

Landed:
http://hg.mozilla.org/qa/mozmill-tests/rev/091cf87ba64b (default)
http://hg.mozilla.org/qa/mozmill-tests/rev/582183ab4263 (mozilla-aurora)
http://hg.mozilla.org/qa/mozmill-tests/rev/ae9390e43ae5 (mozilla-beta)
http://hg.mozilla.org/qa/mozmill-tests/rev/ccf333570cb2 (mozilla-release)

Comment 28

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

Would be good to know if Remus can reproduce it in the exact same situation on his VM. If that's the case it would make it much easier to fix. Thanks Anthony, that was kinda helpful!

Comment 29

[Remus Pop (:RemusPop)]

I ran the tests on a real win2k machine.
What I've got so far is visible on brasstacks for crowd. Just look after Build ID: 20110928060149. I ran the functional test_run 200 times.

Here is a summary of what I observed: testOpenSearchAutodiscovery.js failed 41 times almost one after another, which is approximately the first 41 test runs. After that it wouldn't fail any more.

What I want to do next, given Anthony's comment: find a way to always reproduce this issue in order to fix it, meaning I will run only with 2 files in the testSearch folder.

Anthony, I will make a patch if this always reproduces for you.

Comment 30

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

Remus, even if the failure is only happening occasionally, you should put more debug information into the test. Add sleep calls to slow down the test for visibility checks or use dump statements to log values directly to the console. In any case a reproducible failure would be better at all. Thanks for the initially investigation.

Comment 31

[Remus Pop (:RemusPop)]

Attachment: screenshot of fail

I found the problem but I need some help with what will be the following steps in investigating the root cause.
So as you can see the page that gives us a search engine cannot be displayed.
I ran the testSearch folder with only 2 tests inside: testGetMoreSearchEngines and testOpenSearchAutodiscovery. It does not fail all the time.

Notice: I could not use dump() to output on the console.

Comment 32

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

That's interesting and reminds me on bug 672884. Do we close any tab before we open this page, e.g. closeAllTabs()? That could be the reason here. It would be great if you could combine those two test files into a single test, which would make the investigation and a possible fix easier.

Comment 33

[Remus Pop (:RemusPop)]

AFAIK we will drop support for Win 2K. So I've unassigned myself until new resolution.

Comment 34

[u279076]

Yes, Firefox 12 is the last release Windows 2000 will be supported. Considering that this only happens on Windows 2000, I think we should try re-enabling this test on default and aurora.and then let it ride the train.

Remus, can you take care of this?

Comment 35

[Remus Pop (:RemusPop)]

Attachment: enable v1 (default,aurora) [checked-in]

Enables the test.

Comment 36

[u279076]

Comment on attachment 616964 [details] [diff] [review]
enable v1 (default,aurora) [checked-in]

Looks good to me. I'll land this now.

NOTE: I still expect to see failures until bug 747375 is resolved.

Comment 37

[u279076]

Comment on attachment 616964 [details] [diff] [review]
enable v1 (default,aurora) [checked-in]

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

Patch fails to apply cleanly to mozilla-aurora, please update.

Comment 38

[Remus Pop (:RemusPop)]

Very strange. The patch was last refreshed in mozilla-aurora branch. It applies cleanly on a new repo on my side.

Comment 39

[u279076]

Comment on attachment 616964 [details] [diff] [review]
enable v1 (default,aurora) [checked-in]

Yeah, that was strange. It applied cleanly via hg-transplant.

Landed:
http://hg.mozilla.org/qa/mozmill-tests/rev/bfe7df26371b (mozilla-aurora)

Comment 40

[u279076]

Resolving WONTFIX given that we won't fix the failure (due to Win2k de-support) and the test has been enabled only for branches where Firefox has Win2k de-support.