Closed Bug 719982 Opened 12 years ago Closed 12 years ago

Failure in testAddons_RestartlessExtensionWorksAfterRestart | Modal dialog has been found and processed

Categories

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

Product:
Mozilla QA Graveyard
Component:
Mozmill Tests
Type:
defect

Tracking

(firefox14 fixed, firefox15 fixed, firefox16 fixed, firefox17 fixed, firefox-esr10 fixed)

Status:
RESOLVED FIXED
Tracking Flags:
Tracking Status
firefox14 --- fixed
firefox15 --- fixed
firefox16 --- fixed
firefox17 --- fixed
firefox-esr10 --- fixed

People

(Reporter: u279076, Assigned: vladmaniac)

References

(
URL
)

Details

(Whiteboard: [mozmill-test-failure])

Attachments

(8 files, 4 obsolete files)

skip test (aurora, default) [checked-in]
2.57 KB, patch
AlexLakatos
: review+
u279076
: review+
Details | Diff | Splinter Review
patch v1.0 [checked-in]
4.37 KB, patch
davehunt
: review+
Details | Diff | Splinter Review
replaceClickPatch v1.0 [checked-in]
1.22 KB, patch
whimboo
: review+
Details | Diff | Splinter Review
enableExtensionsLogging patch v1.0 [checked-in]
2.81 KB, patch
whimboo
: review+
Details | Diff | Splinter Review
fix patch v1.2
4.39 KB, patch
whimboo
: review+
Details | Diff | Splinter Review
[mozilla-esr10]fix patch v1.0
4.69 KB, patch
whimboo
: review+
Details | Diff | Splinter Review
[mozilla-beta][mozilla-release] fix patch v1.0
4.66 KB, patch
whimboo
: review+
Details | Diff | Splinter Review
[mozilla-aurora] fix patch v1.1
4.67 KB, patch
whimboo
: review+
Details | Diff | Splinter Review 
The following error happens on Aurora and Nightly on Windows only.

TimeoutError("Modal dialog has been found and processed")
Assignee: nobody → remus.pop
Status: NEW → ASSIGNED
Attached patch skip test (aurora, default) (obsolete) — — Splinter Review 
Skips the test.
Attachment #590733 - Flags: review?(alex.lakatos)
Attachment #590733 - Flags: review?(anthony.s.hughes)
Attachment #590733 - Flags: review?(alex.lakatos)
Attachment #590733 - Flags: review+
Comment on attachment 590733 [details] [diff] [review]
skip test (aurora, default)

># Parent  8594ff7dfd94fb0b9f3f00ed3e626abcf961bc47
>Bug 719973 - disable testAddons_RestartlessExtensionWorksAfterRestart due to failure r=alex.lakatos, r=ashughes
Sorry I missed this earlier, but you got the wrong bug number in the commit message.
Attachment #590733 - Flags: review?(anthony.s.hughes)
Attachment #590733 - Flags: review-
Attachment #590733 - Flags: review+
Sorry about that. Here it is with the corrected commit message.
Attachment #590733 - Attachment is obsolete: true
Attachment #590736 - Flags: review?(alex.lakatos)
Attachment #590736 - Flags: review?(anthony.s.hughes)
Attachment #590736 - Flags: review?(alex.lakatos)
Attachment #590736 - Flags: review+
Comment on attachment 590736 [details] [diff] [review]
skip test (aurora, default) [checked-in]

Landed:
http://hg.mozilla.org/qa/mozmill-tests/rev/14d4b55253ea (default)
http://hg.mozilla.org/qa/mozmill-tests/rev/e5e3b347ff20 (mozilla-aurora)

Remus, if you can, please see if you can reproduce the failure on Beta and Release as we may need to disable the test there as well. We don't see failures since these branches don't get run nightly.
Attachment #590736 - Attachment description: skip test (aurora, default) → skip test (aurora, default) [checked-in]
Attachment #590736 - Flags: review?(anthony.s.hughes) → review+
Whiteboard: [mozmill-test-failure] → [mozmill-test-failure][mozmill-test-skipped]
Comment on attachment 625073 [details] [diff] [review]
patch v1.0 [checked-in]

Please use r=dhunt when I'm the reviewer. In this case I've fixed the commit message and landed as:
http://hg.mozilla.org/qa/mozmill-tests/rev/35bc8c5cbb29

When we see this passing in our results we can transplant to the remaining branches.
Attachment #625073 - Flags: review?(dave.hunt) → review+
According to the results it works perfectly. Dave, would you mind to transplant those patches to the other branches?
status1.9.2: --- → unaffected
status-firefox-esr10: --- → unaffected
status-firefox12: --- → affected
status-firefox13: --- → affected
status-firefox14: --- → affected
status-firefox15: --- → fixed
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
OS: Windows 7 → All
Resolution: --- → FIXED
Alex, please make sure to update all the Litmus tests across those branches and c&p their URLs in here. Thanks.
Whiteboard: [mozmill-test-failure][mozmill-test-skipped] → [mozmill-test-failure]
Flags: in-litmus?(alex.lakatos)
I had to backout the patch from default because it causes the installation dialog to stay open on OS X for at least the de locale.

http://hg.mozilla.org/qa/mozmill-tests/rev/425b679eddb5

Alex please check that on any OS X box with a couple of locales.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Status: REOPENED → ASSIGNED
status-firefox15: fixed → affected
We do not have any steps. When I logged into our OS X test machines today they both showed the installation dialog open for this particular test. On 10.7 it were even 3 processes of Firefox which haven't been closed.
Please get this test re-enabled as soon as you can.
Sorry, the last comment was for another bug. Alex, can you please run this test about 100 times across platforms? Just for the en-US locale.
(In reply to Henrik Skupin (:whimboo) from comment #16)
> Sorry, the last comment was for another bug. Alex, can you please run this
> test about 100 times across platforms? Just for the en-US locale.
Started to run them. Should I have them reported to the mozmill-crowd dashboard?
No, it's fine when you do it locally on all platforms. If possible try to fill-up the memory and cause a higher CPU load while running the test. Putting the box under stress should probably easier trigger this problem if is still persists.
Out of 100 runs on each platforms, it failed only once on Ubuntu with "Modal dialog has been found and processed" while the cpu was under load. Rerunning the tests again on Ubuntu 100 times did not fail once. I think's it's safe to land, seeing as the issue you were seeing was on Mac and I can not reproduce that anymore.
This test was mostly failing on Windows. So please also run tests on that platform. See the Mozmill results which version is the best.
URL: http://mozmill-release.blargon7.com/#... → http://mozmill-release.blargon7.com/#...
Ran it 100 time again on XP, Vista and Windows 7. It did not fail.
Ok, so I have pushed this again to default:
http://hg.mozilla.org/qa/mozmill-tests/rev/393c1afcd118

We will have to watch the status of the test in the next couple of days before landing it on aurora too.
Status: ASSIGNED → RESOLVED
Closed: 12 years ago → 12 years ago
status-firefox16: affected → fixed
Resolution: --- → FIXED
It has not failed in the last 4 days.
The patch doesn't apply cleanly. Please update it for the landing on aurora.
You do not need to apply the patch on aurora. When the fix first landed, Dave landed it on all branches, but when you backed it out, you only backed out default.
I was stating it did not fail in my comment in order to be able to close the bug.
Status: RESOLVED → VERIFIED
status-firefox15: affected → fixed
It seems we have an intermittent failure. Failed once since landing:
http://mozmill-ci.blargon7.com/#/functional/report/c67fb1fc2ea8384105b843e32c1056fe
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
The thing is that we never really check here if the install button is present.  have seen cases when httpd failed in loading a local resource and just put a 'bad request' message in the body. Given that this test would fail by waiting for the modal dialog.

I would propose that we exchange the click() call with waitThenClick().

For now lets experiment on the default branch. If we can find the real cause of the problem we probably can also fix all the other cases and branches.
Status: REOPENED → ASSIGNED
status-firefox16: fixed → affected
Alex, please follow up on this failure. I would have expected to get this addressed why earlier. If you don't have time please always find another person.
Here is a patch that replaces click with waitThenClick. But I don't think that's the problem. The same error appears even when the download takes too long. I know there's no actual download cause we are on localhost, but there is still a lag there. But we can certainly test the stability of the test with this patch on default.
The CI dashboard shows 4 failures this week, 2 on Mac on Firefox 16 and 2 on Linux on Firefox 15.
Attachment #641013 - Flags: review?(hskupin)
Comment on attachment 641013 [details] [diff] [review]
replaceClickPatch v1.0 [checked-in]

Well, I have a suspicion and if it is true we will see another (new) failure which will tell us that the installation link does not exist. Looks good and I will land it right away.
Attachment #641013 - Flags: review?(hskupin) → review+
Attachment #625073 - Attachment description: patch v1.0 → patch v1.0 [checked-in]
Comment on attachment 641013 [details] [diff] [review]
replaceClickPatch v1.0 [checked-in]

Landed on default:
http://hg.mozilla.org/qa/mozmill-tests/rev/8b77a8466fb8
Attachment #641013 - Attachment description: replaceClickPatch v1.0 → replaceClickPatch v1.0 [checked-in]
Alex, please follow-up on your assigned bugs regularly. Looks like no difference here. We are still failing.

Vlad, could this be related to the issue you have seen for the discovery pane tests, where the download fails?

I would suggest we disable this test again.
URL: http://mozmill-release.blargon7.com/#... → http://mozmill-ci.blargon7.com/#/func...
status-firefox15: fixed → affected
status-firefox17: --- → affected
Since the (In reply to Henrik Skupin (:whimboo) from comment #32)
> Alex, please follow-up on your assigned bugs regularly. Looks like no
> difference here. We are still failing.
We are still failing with the same error, seems the waitThenClick did not help us get a different error message. It did not hurt the test either, gaining test stability. Do we want to back it out or port on other branches? The patch does not apply cleanly on other branches now.

> 
> I would suggest we disable this test again.
The skip test patch applies cleanly on all branches, you can go ahead and land it.
> 
> Vlad, could this be related to the issue you have seen for the discovery
> pane tests, where the download fails?
> 
> I would suggest we disable this test again.

I know that we are disabling tests when they fail, but it does not make sense to disable this one right now because of the issue I seen. Maybe our errors may help Dave or Blair in their answer. If the failing tests are skipped, we would not have anything to give to the developers, IMHO
Vlad, so please get in touch with both and asking for feedback. I do not want to wait longer for a response on the bug. If nothing has changed by end of this week we will disable this test!
Enable extensions.logging as per Henrik and Vlad's IRC conversation
Attachment #646099 - Flags: review?(hskupin)
Attachment #646099 - Flags: review?(dave.hunt)
Attachment #646099 - Flags: review?(hskupin)
Attachment #646099 - Flags: review?(dave.hunt)
Attachment #646099 - Flags: review+
Comment on attachment 646099 [details] [diff] [review]
enableExtensionsLogging patch v1.0 [checked-in]

http://hg.mozilla.org/qa/mozmill-tests/rev/b7b3a873054d
Attachment #646099 - Attachment description: enableExtensionsLogging patch v1.0 → enableExtensionsLogging patch v1.0 [checked-in]
This failing again 
http://mozmill-ci.blargon7.com/#/functional/report/29fc09ba0c8360d637617903a01e6842

How can we make usage of extension logging pref to see the log?
It looks like that it doesn't get logged to the console:

http://qa-masterblaster.mv.mozilla.com:8080/job/mozilla-central_functional/2487/console

Vlad, please run the testrun_functional.py script on your box and check if you can see the AOM output in the console. If not it looks like an issue with the automation script and needs to be fixed.
Two days without a response here. Vlad, Alex, which updates do you have regarding my last comment?
(In reply to Henrik Skupin (:whimboo) from comment #40)
> Two days without a response here. Vlad, Alex, which updates do you have
> regarding my last comment?

Personally I missed comment 39. Lots of bugmail lately. I'll catch up with this shortly
(In reply to Henrik Skupin (:whimboo) from comment #40)
> Two days without a response here. Vlad, Alex, which updates do you have
> regarding my last comment?

I can see the log when running the functional testrun locally on our box. 
I have ran it on Ubuntu 11.04 x86 

TEST-PASS | /tmp/tmpR9ZVF9.mozmill-tests/tests/functional/restartTests/testAddons_RestartlessExtensionWorksAfterRestart/test1.js | test1.js::testInstallRestartlessExtension
*** LOG addons.xpi: Calling bootstrap method shutdown on restartless-addon@quality.mozilla.org version 3.0
*** LOG addons.xpi: shutdown
*** LOG addons.xpi-utils: shutdown
*** LOG addons.xpi-utils: Database closed
*** LOG addons.xpi: startup
*** LOG addons.xpi: checkForChanges
*** LOG addons.xpi-utils: Opening database
*** LOG addons.xpi: No changes found
*** LOG addons.xpi: Loading bootstrap scope from /tmp/tmpHDPDYy.mozrunner/extensions/restartless-addon@quality.mozilla.org
*** LOG addons.xpi: Calling bootstrap method startup on restartless-addon@quality.mozilla.org version 3.0
TEST-START | /tmp/tmpR9ZVF9.mozmill-tests/tests/functional/restartTests/testAddons_RestartlessExtensionWorksAfterRestart/test2.js | setupModule
Ah, looks like that in our log on Mozmill CI the output has an offset, so yes it's also visible there.

Vlad, given that you have VPN access, please log into MV and check the notification emails from mozmill-ci for such failures. The first link will bring you to the test results log. Just click on console output in the left menu to see the whole output. Then please check for the above addon log entries.
Assignee: alex.lakatos → vlad.mozbugs
Priority: -- → P2
I still miss feedback here. So what are the latest findings?
(In reply to Henrik Skupin (:whimboo) from comment #45)
> I still miss feedback here. So what are the latest findings?

I afraid this is not reproducible on our machines. Several reports are available on the crowd dashboard, none point out this failure. 
Its really hard for me to propose a fix if I cannot see the actual test failing
You should not try to reproduce this failure locally but to analyze the results on the CI instance. That's why I helped you on those steps. I'm waiting on a follow-up for comment 44.
The log is the same when the test fails or passes. I am beginning to think we need to save some time here, meaning: 

* adding extra timeout to waitForDialog(), like we do in all other add-ons tests which are currently reliable and not failing. 
* also, I am going to set the dialog delay to one second, this will save us another four seconds. 
 Judging the fact that we are using the default timeout from waitForDialog at the moment, its a possibility that we are failing because of this cause.
Attached patch fix patch v1.0 (obsolete) — — Splinter Review 
* added extra timeout for extension download - even if the extension is local, under heavy loaded systems download can take some time - this can be easily verified locally 
* set the dialog timer to 1 second, this means aprox 4 sec gain in the waiting of the modal dialog process 

Note: I used this approach for other tests and made them reliable see for e.g. testAddons_changeTheme. Hope it will do the trick here also.
Attachment #652412 - Flags: review?(hskupin)
Attachment #652412 - Flags: review?(dave.hunt)
Comment on attachment 652412 [details] [diff] [review]
fix patch v1.0

>+const PREF_INSTALL_DIALOG = "security.dialog_enable_delay";
>+const INSTALL_DIALOG_DELAY = 1000;
>+const TIMEOUT_DOWNLOAD = 25000;

As you know we separate those different type of constants into separate blocks.

> function setupModule() {
[..]
>+  // Set pref for add-on installation dialog timer 
>+  prefs.preferences.setPref(PREF_INSTALL_DIALOG, INSTALL_DIALOG_DELAY);

Where gets this pref reset?

>+  // Whitelist add the localhost
>+  addons.addToWhiteList(LOCAL_TEST_FOLDER);

nit: kill 'add the' in the comment

Otherwise I think it makes sense.
Attachment #652412 - Flags: review?(hskupin)
Attachment #652412 - Flags: review?(dave.hunt)
Attachment #652412 - Flags: review-
Attached patch fix patch v1.1 (obsolete) — — Splinter Review 
* Clearing the pref now - that was a mistake in the previous patch
* Fixed nits
Attachment #652412 - Attachment is obsolete: true
Attachment #652422 - Flags: review?(hskupin)
Attachment #652422 - Flags: review?(dave.hunt)
Comment on attachment 652422 [details] [diff] [review]
fix patch v1.1

> function teardownModule() {
>   prefs.preferences.clearUserPref("extensions.logging.enabled");
>+  prefs.preferences.clearUserPref("security.dialog_enable_delay");

So lets also please use constants here and at the same time we can disable extension logging which is not necessary anymore.

>   prefs.preferences.clearUserPref("browser.urlbar.trimURLs");

What is that? I can't see that we are setting this pref.
Attachment #652422 - Flags: review?(hskupin)
Attachment #652422 - Flags: review?(dave.hunt)
Attachment #652422 - Flags: review-
 
> >   prefs.preferences.clearUserPref("browser.urlbar.trimURLs");
> 
> What is that? I can't see that we are setting this pref.

I have not set it with any of my patches but it is set in test2.js
function testRestartlessExtensionWorksAfterRestart() {
  // Change pref to show the full url in the location bar
  prefs.preferences.setPref("browser.urlbar.trimURLs", false);
We should probably set it in setupModule IMO
Attached patch fix patch v1.2 — — Splinter Review 
* fixed hard coded strings, declared constants
Attachment #652422 - Attachment is obsolete: true
Attachment #652431 - Flags: review?(hskupin)
Attachment #652431 - Flags: review?(dave.hunt)
Comment on attachment 652431 [details] [diff] [review]
fix patch v1.2

Looks good. Lets cross fingers that this will fix the issue. Thanks Vlad!
Attachment #652431 - Flags: review?(hskupin)
Attachment #652431 - Flags: review?(dave.hunt)
Attachment #652431 - Flags: review+
http://hg.mozilla.org/qa/mozmill-tests/rev/165b3940cbdb

I will re-trigger some tests which were failing today due to this problem. Reports will go to mozmill-crowd. So please check in about 1 hour. If we are good we can backport.
Status: ASSIGNED → RESOLVED
Closed: 12 years ago → 12 years ago
status-firefox-esr10: unaffected → affected
status-firefox12: fixed → ---
status-firefox13: fixed → ---
status-firefox14: fixed → affected
status-firefox17: affected → fixed
Resolution: --- → FIXED
(In reply to Henrik Skupin (:whimboo) from comment #56)
> http://hg.mozilla.org/qa/mozmill-tests/rev/165b3940cbdb
> 
> I will re-trigger some tests which were failing today due to this problem.
> Reports will go to mozmill-crowd. So please check in about 1 hour. If we are
> good we can backport.

Excellent as I am still @office in one hour. I'll make sure I check, thanks Henrik!
The patch will not apply for mozilla-esr10. If we decide to backport, let me know so I can quickly upload the esr patch. Thanks
(In reply to Maniac Vlad Florin (:vladmaniac) from comment #59)
> The patch will not apply for mozilla-esr10. If we decide to backport, let me
> know so I can quickly upload the esr patch. Thanks

Given the flag esr10 is affected. So yes, we have to backport the patch. So please attach it. I will backport the patch right after todays testrun for Nightly builds if all stay green.
* adding the fix patch to correctly apply to mozilla-esr10
Attachment #652696 - Flags: review?(hskupin)
Attachment #652696 - Flags: review?(dave.hunt)
Attachment #652696 - Flags: review?(hskupin)
Attachment #652696 - Flags: review?(dave.hunt)
Attachment #652696 - Flags: review+
I also can't merge this back to aurora. Not tested with beta or release. Please also come up with those patches.
(In reply to Henrik Skupin (:whimboo) from comment #62)
> I also can't merge this back to aurora. Not tested with beta or release.
> Please also come up with those patches.

Right away - it seems like this needs lots of testing
Attached patch [mozilla-aurora] fix patch v1.0 (obsolete) — — Splinter Review 
* this should work for mozilla-aurora
Attachment #652741 - Flags: review?(hskupin)
Attachment #652741 - Flags: review?(dave.hunt)
* patch for mozilla-beta
Attachment #652744 - Flags: review?(hskupin)
Attachment #652744 - Flags: review?(dave.hunt)
We can transplant the beta patch to the release branch. It will apply and work as expected.
Attachment #652741 - Flags: review?(hskupin)
Attachment #652741 - Flags: review?(dave.hunt)
(In reply to Maniac Vlad Florin (:vladmaniac) from comment #67)
> We can transplant the beta patch to the release branch. It will apply and
> work as expected.

Also the aurora patch was wrong - contained an extra empty line somehow. The beta patch applies cleanly for aurora as well and can be transplanted. Sorry for the mess
Comment on attachment 652741 [details] [diff] [review]
[mozilla-aurora] fix patch v1.0

Obsoleting the wrong aurora patch
Attachment #652741 - Attachment is obsolete: true
Attachment #652744 - Attachment description: [mozilla-beta] fix patch v1.0 → [mozilla-aurora][mozilla-beta][mozilla-release] fix patch v1.0
Attachment #652744 - Flags: review?(hskupin)
Attachment #652744 - Flags: review?(dave.hunt)
Attachment #652744 - Flags: review+
Comment on attachment 652744 [details] [diff] [review]
[mozilla-beta][mozilla-release] fix patch v1.0

This still doesn't apply to the aurora branch.
Attachment #652744 - Flags: review+ → review-
Vlad, I want to see an update for this patch. We have to check it in ASAP.
* adding a patch for mozilla-aurora 

Sorry I missed this in by bugmail yesterday
Attachment #653671 - Flags: review?(hskupin)
Attachment #652744 - Attachment description: [mozilla-aurora][mozilla-beta][mozilla-release] fix patch v1.0 → [mozilla-beta][mozilla-release] fix patch v1.0
Comment on attachment 652744 [details] [diff] [review]
[mozilla-beta][mozilla-release] fix patch v1.0

* It seems that we need different for aurora branch, but this still applies and works cleanly for mozilla-beta and release branches, therefore asking again for r
Attachment #652744 - Flags: review- → review?(hskupin)
Attachment #652744 - Flags: review?(dave.hunt)
Attachment #653671 - Flags: review?(dave.hunt)
Attachment #653671 - Flags: review?(hskupin)
Attachment #653671 - Flags: review?(dave.hunt)
Attachment #653671 - Flags: review+
Attachment #652744 - Flags: review?(hskupin)
Attachment #652744 - Flags: review?(dave.hunt)
Attachment #652744 - Flags: review+
Product: Mozilla QA → Mozilla QA Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: