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

VERIFIED FIXED

Status

Mozilla QA
Mozmill Tests
VERIFIED FIXED
7 years ago
7 years ago

People

(Reporter: Aleksej, Assigned: whimboo)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [mozmill-test-failure][testday-20110715], URL)

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

7 years ago
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.
(Reporter)

Comment 1

7 years ago
Henrik mentioned this Litmus test: https://litmus.mozilla.org/show_test.cgi?id=16468
(Assignee)

Updated

7 years ago
OS: Linux → All
Summary: testRestoreDefaultEngines fails on Linux with "en-US" and "eo" but not "ru" → Failure ''subject.visibleEngine.name == subject.expectedEngine.name' in testRestoreDefaultEngines (testRestoreDefaults.js)
Whiteboard: [testday-20110715] → [mozmill-test-failure][testday-20110715]
(Assignee)

Comment 2

7 years ago
Created attachment 546753 [details] [diff] [review]
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.
Assignee: nobody → hskupin
Status: NEW → ASSIGNED
Attachment #546753 - Flags: review?(gmealer)
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.
Attachment #546753 - Flags: review?(gmealer) → review+
(Assignee)

Comment 4

7 years ago
Created attachment 547044 [details] [diff] [review]
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.
Attachment #546753 - Attachment is obsolete: true
Attachment #547044 - Flags: review?(gmealer)

Comment 5

7 years ago
A Pivotal Tracker story has been created for this Bug: https://www.pivotaltracker.com/story/show/16048707
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. :/
Attachment #547044 - Flags: review?(gmealer) → review+
(Reporter)

Comment 8

7 years ago
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.
(Assignee)

Comment 9

7 years ago
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?
(Reporter)

Comment 10

7 years ago
Fails with 5.0.1 eo, 5.0 en-US,
does not fail with 4.0.1 en-US.
(Assignee)

Comment 11

7 years ago
(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.
(Reporter)

Comment 12

7 years ago
(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).
(Assignee)

Comment 13

7 years ago
(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.
(Assignee)

Updated

7 years ago
Attachment #547044 - Attachment description: Patch v2 → Patch v2 (restoreEngines) [checked in]
(Assignee)

Comment 14

7 years ago
Created attachment 547696 [details] [diff] [review]
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.
Attachment #547696 - Flags: review?(gmealer)
(Reporter)

Comment 15

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

That fixes it.
Comment on attachment 547696 [details] [diff] [review]
Patch v1 (searchSelection)

looks great, r+
Attachment #547696 - Flags: review?(gmealer) → review+
No recent failures -- verified FIXED.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.