671834 - Failure ''subject.visibleEngine.name == subject.expectedEngine.name' in testRestoreDefaultEngines (testRestoreDefaults.js)

Status: VERIFIED FIXED

(Mozilla QA Graveyard :: Mozmill Tests, defect)

Description

[[:Aleksej]]

6.0 (20110713171652)
7.0a1 (20110701030757)
7.0a1 (20110630030749)
6.0a2 (20110701042007)
4.0b13pre (20110322030409)
4.0 (20110318052756)

testRestoreDefaultEngines fails for me (on Debian GNU/Linux) with "en-US" and "eo" but not "ru"; and for somebody else (on SuSE) with "en-US":

  controller.assertJS: Failed for 'subject.visibleEngine.name == subject.expectedEngine.name'

Tried manually with the eo build, the only difference between the lists is which engine is currently active.

Comment 1

[[:Aleksej]]

Henrik mentioned this Litmus test: https://litmus.mozilla.org/show_test.cgi?id=16468

Comment 2

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

Attachment: Patch v1

Clean-up of the test to have better assertion messages. Not sure if this will completely fix this test failures on the mentioned locales but we can get at least better details.

Comment 3

[Geo Mealer [:geo] -- This account is inactive after 2015-07-07]

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

r+, but why did you change the logic from "3 left" to "1 left"? You sure there wasn't some reason it was originally 3?

Not a blocker, but I'm not huge fan of random behavior unless it's somehow recorded in the test results. So we get a failure and then...what? We don't know what order it removed the engines in.

Comment 4

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

Attachment: Patch v2 (restoreEngines) [checked in]

(In reply to comment #3)
> r+, but why did you change the logic from "3 left" to "1 left"? You sure
> there wasn't some reason it was originally 3?

No, I have just deleted half of them. But to prove the restore function we should remove as much as possible. Right now there is a bug which doesn't let us remove the last engine, so we leave the last one.

> Not a blocker, but I'm not huge fan of random behavior unless it's somehow
> recorded in the test results. So we get a failure and then...what? We don't
> know what order it removed the engines in.

I have removed the random deletion order and also replaced the sleep command with a waitFor, which will show the name of the engine if the removal fails.

Comment 5

[u415141]

A Pivotal Tracker story has been created for this Bug: https://www.pivotaltracker.com/story/show/16048707

Comment 6

[Geo Mealer [:geo] -- This account is inactive after 2015-07-07]

Comment on attachment 547044 [details] [diff] [review]
Patch v2 (restoreEngines)  [checked in]

Looks good, and good call about putting the engine in the results. r+

We should talk about how to best do random stuff sometime, as it can find bugs better than deterministic code. If we store a separate test result for each thing removed (for example) it's not bad; then we have a reproduction case.

It's a shame JS doesn't let you seed its random() function. Then it's easy: You pick a random seed (starting value for the generator algorithm) and log the seed that got used. You can make the random numbers come out the same for reproduction by temporarily editing the test to use that seed instead of the random one. 

Randomizing the seed screws up random distribution for real-life stuff, but it's good enough for testing. Alas, unless we write our own generator, no seeding in JS. :/

Comment 7

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

Landed as:
http://hg.mozilla.org/qa/mozmill-tests/rev/5cee08d07913 (default)
http://hg.mozilla.org/qa/mozmill-tests/rev/a7aa0f154032 (aurora)
http://hg.mozilla.org/qa/mozmill-tests/rev/0d6622842886 (beta)
http://hg.mozilla.org/qa/mozmill-tests/rev/3a8bb759eef4 (release)

Aleksej, can you please re-test with the mentioned builds if it is now fixed for you?

Comment 8

[[:Aleksej]]

Still fails (6.0b2rc1 en-US): http://mozmill-crowd.brasstacks.mozilla.com/#/functional/report/8d67634d3d2adcac440871307857d7a2 (message currently not visible due to bug 672954).
Also with Nightly 20110701030757 en-US http://mozmill-crowd.brasstacks.mozilla.com/#/functional/report/8d67634d3d2adcac440871307857b061

But if I run that test alone, it passes (Nightly 20110701030757 eo, 6.0b2c1 en-US).
If I do it manually (6.0b2rc1), it also passes.

Comment 9

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

So the engines are somehow flipped in your case. Not sure why that happens on your system. Can you please test with older builds of Firefox (down to 4.0) if this failure persists on those branches?

Comment 10

[[:Aleksej]]

Fails with 5.0.1 eo, 5.0 en-US,
does not fail with 4.0.1 en-US.

Comment 11

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

(In reply to comment #10)
> Fails with 5.0.1 eo, 5.0 en-US,
> does not fail with 4.0.1 en-US.

Is this repeatable? Lets say for a dozen of test-runs? On IRC you have mentioned NoScript which is globally installed. Per definition those add-ons shouldn't be installed for a Mozmill test-run. While the tests are running, can you verify that in the Add-ons Manager? Then just abort the test-run so we do not send a broken report.

Comment 12

[[:Aleksej]]

(In reply to comment #11)
> (In reply to comment #10)
> Is this repeatable? Lets say for a dozen of test-runs? On IRC you have
> mentioned NoScript which is globally installed. Per definition those add-ons

NoScript was not relevant.

By removing test folders as Henrik suggested on IRC, it was discovered that this test fails when run after functional/testSearch/testSearchSelection.js (same with or without a testrun).

Comment 13

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

(In reply to comment #12)
> By removing test folders as Henrik suggested on IRC, it was discovered that
> this test fails when run after functional/testSearch/testSearchSelection.js
> (same with or without a testrun).

Thanks Aleksej! That was very helpful information. I now can reproduce the failure on OS X. The mixed execution of tests on Linux causes us a bit trouble, but also ensures that our tests will become more stable.

Comment 14

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

Attachment: Patch v1 (searchSelection)

The testSearchSelection.js test module simply has to reset the search engines to default. With that change in place both tests pass.

Comment 15

[[:Aleksej]]

(In reply to comment #14)
> Created attachment 547696 [details] [diff] [review] [review]
> Patch v1 (searchSelection)

That fixes it.

Comment 16

[Geo Mealer [:geo] -- This account is inactive after 2015-07-07]

Comment on attachment 547696 [details] [diff] [review]
Patch v1 (searchSelection)

looks great, r+

Comment 17

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

Landed as:
http://hg.mozilla.org/qa/mozmill-tests/rev/7af9a998c2a4 (default)
http://hg.mozilla.org/qa/mozmill-tests/rev/127b1960d58a (aurora)
http://hg.mozilla.org/qa/mozmill-tests/rev/c0024df729df (beta)
http://hg.mozilla.org/qa/mozmill-tests/rev/a6d58318d7d6 (release)

Comment 18

[u279076]

No recent failures -- verified FIXED.