Closed Bug 840022 Opened 7 years ago Closed 6 years ago

Test failure "The forward button has been made visible for the 1 page" in testToolbars/testBackForwardButtons.js

Categories

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

All
macOS
defect

Tracking

(firefox21 wontfix, firefox22 wontfix, firefox23 wontfix, firefox27 fixed, firefox28 fixed, firefox29 fixed, firefox30 ?, firefox-esr24 fixed)

RESOLVED FIXED
Tracking Status
firefox21 --- wontfix
firefox22 --- wontfix
firefox23 --- wontfix
firefox27 --- fixed
firefox28 --- fixed
firefox29 --- fixed
firefox30 --- ?
firefox-esr24 --- fixed

People

(Reporter: AndreeaMatei, Assigned: danisielm)

References

()

Details

(Whiteboard: [mozmill-test-failure])

Attachments

(3 files, 11 obsolete files)

2.04 KB, patch
AndreeaMatei
: review-
Details | Diff | Splinter Review
1.51 KB, patch
AndreeaMatei
: review+
Details | Diff | Splinter Review
1.44 KB, patch
AndreeaMatei
: review+
Details | Diff | Splinter Review
This happened today after Jenkins was restarted, with Nightly fr locale - OS X 10.6.8.
Are the machines too fast now? The forward button appears to not have been visible.
Whiteboard: [mozmill-test-failure]
Hmm, thinking more about the issue in bug 832180 where we're too fast now, could be the same here. The forward button appears after there is a page to be forwarded to, so we either don't click the back button to have the forward button available or we move too fast:
http://hg.mozilla.org/qa/mozmill-tests/file/6692f287b545/tests/functional/testToolbar/testBackForwardButtons.js#l56
Have we been able to replicate this locally? Perhaps we could take one of the nodes offline to attempt to replicate this remotely. There have been no changes to Jenkins or the Mac nodes that should have affected this test. It only appears to be Mac 10.6.8, and is not locale specific.
Whiteboard: [mozmill-test-failure] → [mozmill-test-failure] s=130218 u=failure c=toolbars p=1
Assignee: nobody → mario.garbi
Attached patch patch v1.0Splinter Review
I've added an expect.waitFor in order make sure the transition state is finished before checking in the assert.
Attachment #718344 - Flags: review?(andreea.matei)
Comment on attachment 718344 [details] [diff] [review]
patch v1.0

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

From what I understand, you couldn't reproduced it, but I would suggest to spend more time in doing that, on the machine shown in mozmill-ci reports. There's no guarantee this will work otherwise.
Also, the mini-macs are still slow now, so I'm not sure if this is a "speeding" issue.

::: tests/functional/testToolbar/testBackForwardButtons.js
@@ +52,5 @@
>    }
>  
> +  expect.waitFor(function () {
> +    return transitionFinished;
> +  }, "The transition has been finished");

This is also repeated in the assert from below.
Attachment #718344 - Flags: review?(andreea.matei) → review-
One month without this failure, closing the bug.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → WORKSFORME
Happened again with Firefox 22.0a2 it on Mac OS X 10.6.8 (x86_64):
http://mozmill-ci.blargon7.com/#/functional/report/25ad365ca7bcf4905e9b700b4fd28fc8

Reopening this and will look into it.
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
(In reply to mario garbi from comment #9)
> Reopening this and will look into it.

This bug has a very low frequency and doesn't require to spend time on it. Please care about the existent P1 bugs.
Priority: -- → P5
Why haven't we skipped this test yet on the affected branches? It's bouncing us for days now.
Priority: P5 → P2
Attached patch Skip patch (obsolete) — Splinter Review
Added a skip patch for the failing test until we can come with a fix to the issue.
Attachment #734491 - Flags: review?(andreea.matei)
Comment on attachment 734491 [details] [diff] [review]
Skip patch

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

Given that this is only failing on OS X (right?), we should only skip it on this platform.
Attached patch Skip patch for Mac OS (obsolete) — Splinter Review
Attachment #734497 - Flags: review?(hskupin)
Attachment #734497 - Flags: review?(andreea.matei)
Added a skip patch for Mac only as requested, I should have done so in the first place, thank you Henrik.
Comment on attachment 734497 [details] [diff] [review]
Skip patch for Mac OS

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

::: tests/functional/testToolbar/testBackForwardButtons.js
@@ +63,5 @@
>      controller.waitForElement(element, TIMEOUT);
>    }
>  }
> +
> +if (mozmill.isMac){

Whitespace needed between brackets here.

@@ +65,5 @@
>  }
> +
> +if (mozmill.isMac){
> +  setupModule.__force_skip__ = "Bug 840022 - Test failure The forward button has been made" +
> +                             "visible for the 1 page";

Indentation please and a space in one of the strings so "made visible" don't get attached.

@@ +66,5 @@
> +
> +if (mozmill.isMac){
> +  setupModule.__force_skip__ = "Bug 840022 - Test failure The forward button has been made" +
> +                             "visible for the 1 page";
> +  testBackAndForward.__force_skip__ = "Bug 840022 - Test failure The forward button has been made" +

No need to skip this one as well, as it will be automatically because setup got skipped.
Attachment #734497 - Flags: review?(hskupin)
Attachment #734497 - Flags: review?(andreea.matei)
Attachment #734497 - Flags: review-
Attachment #734491 - Attachment is obsolete: true
Attachment #734491 - Flags: review?(andreea.matei)
Mario, also please edit the commit message, it should have capital letters and be more like "Skip test test.js due to failure.. ". Thanks.
Attached patch Skip patch for Mac OS (obsolete) — Splinter Review
Added an updated version, sorry for those indentation issues, I was in a hurry to add a skip and overlooked them.
Attachment #734497 - Attachment is obsolete: true
Attachment #734517 - Flags: review?(andreea.matei)
Comment on attachment 734517 [details] [diff] [review]
Skip patch for Mac OS

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

I will edit the commit this time, please address all requests next time.

http://hg.mozilla.org/qa/mozmill-tests/rev/b7fe452034a9 (default)
Please check the patch with other affected branches.
Attachment #734517 - Flags: review?(andreea.matei) → review+
The skip patch applies cleanly across all branches.
We attempted to reproduce the bug both on the local system and on the mm-osx-196-3 machine for almost a week now without any luck. I have used the builds that failed and tried to reproduce the conditions best I could.

Will try more this week to reproduce it in order to figure out the problem, if I still cannot get it to fail again I think we should unskip it and watch out for failures.
So lets reenable the test for nightly and we can see how often it fails. It's strange that no failures were visible for esr17. So it might be a regression in some way.
We haven't seen this failure since we unskiped it. We could re-enable it for all branches and see if any fail in the future. I am still unable to reproduce it locally and it seems it doesn't reproduce anymore for the CI either.
Backed out on aurora as well, let's see how this goes. 
http://hg.mozilla.org/qa/mozmill-tests/rev/6f47edec7840 (aurora)
No failures so I re-enabled the test on beta as well:
http://hg.mozilla.org/qa/mozmill-tests/rev/5ff06c2a431a (beta)

Lets give this a few days to check beta, before closing.
It did not reproduce since being unskipped, we could wait a few more days to be sure and close it.
Closing as WFM, please reopen if necessary.
Status: REOPENED → RESOLVED
Closed: 7 years ago7 years ago
Resolution: --- → WORKSFORME
Mario, can you please reproduce on one failing machine if you can't locally? This is still a P2 as it's not failing each time but intermittently. It started again on August 1st, maybe something changed in the browser. 
We might want to skip this again and unskip it only with a fix. The skip patch is not applying anymore.
Priority: P1 → P2
It's still reproducing and I will add a skip patch in a few minutes.
Attached patch skip_0508.patch (obsolete) — Splinter Review
Updated Skip patch until I can come up with a Fix patch. This might be related to the Mac CI machine as I cannot reproduce it locally. I will test this on the failing machine too.
Attachment #734517 - Attachment is obsolete: true
Attachment #785702 - Flags: review?(andreea.matei)
Comment on attachment 785702 [details] [diff] [review]
skip_0508.patch

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

::: tests/functional/testToolbar/manifest.ini
@@ +1,2 @@
>  [testBackForwardButtons.js]
> +disabled = Bug 840022 - Test failure "The forward button has been made visible for the 1 page" in testToolbars/testBackForwardButtons.js

Please remove the test, it's already added above.
Attachment #785702 - Flags: review?(andreea.matei) → review-
Attached patch skip_0508.patch (obsolete) — Splinter Review
Updated the skip patch as requested
Attachment #785702 - Attachment is obsolete: true
Attachment #785709 - Flags: review?(andreea.matei)
Comment on attachment 785709 [details] [diff] [review]
skip_0508.patch

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

Landed:
http://hg.mozilla.org/qa/mozmill-tests/rev/268fa396a2b6 (default)
http://hg.mozilla.org/qa/mozmill-tests/rev/6d9c05b7f35f (aurora)
Attachment #785709 - Flags: review?(andreea.matei) → review+
Why has this test been disabled on OS X in the test but for all platforms in the manifest? This is not clear to me.
Flags: needinfo?(mario.garbi)
Whiteboard: [mozmill-test-failure] s=130218 u=failure c=toolbars p=1 → [mozmill-test-failure][mozmill-test-skipped]
We only had this on OS X so I should have skipped it for OS X only in the manifest too. I will provide a followup skip patch to change that.
Flags: needinfo?(mario.garbi)
Attached patch skip_1908.patch (obsolete) — Splinter Review
Followup skip patch to only skip for mac in the manifest. I didn't knew about skip-if since we don't use it in out manifests so thanks Henrik for noticing this. I will use this from now on for the manifests.
Comment on attachment 792047 [details] [diff] [review]
skip_1908.patch

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

This is way off, please check the patch again cause you've added the skip again when it's supposed to be just a follow up for the manifest. 
Also you need to update the commit message and request review next time :)
Attachment #792047 - Flags: review-
Attached patch skipFollowup.patch (obsolete) — Splinter Review
Sorry about the previous patch, wrong patch uploaded.
Attachment #792047 - Attachment is obsolete: true
Attachment #792225 - Flags: review?(andreea.matei)
Comment on attachment 792225 [details] [diff] [review]
skipFollowup.patch

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

::: tests/functional/testToolbar/manifest.ini
@@ +1,3 @@
>  [testBackForwardButtons.js]
>  disabled = Bug 840022 - Test failure "The forward button has been made visible for the 1 page"
> +skip-if = os == "mac"

This will skip the test regardless of the platform due to the disable line. I remember I've done one skip like this and I used the message after the 'mac'.

Please see: 
https://pypi.python.org/pypi/ManifestDestiny/0.5.6 
and test this with 2.0, running the manifest, to make sure it does what's supposed to.
Also let's use single quotes.
Attachment #792225 - Flags: review?(andreea.matei) → review-
Attached patch skipFollowup_2108.patch (obsolete) — Splinter Review
Correct followup patch, thank you Andreea.
Attachment #792225 - Attachment is obsolete: true
Attachment #793373 - Flags: review?(andreea.matei)
Comment on attachment 793373 [details] [diff] [review]
skipFollowup_2108.patch

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

::: tests/functional/testToolbar/manifest.ini
@@ +1,2 @@
>  [testBackForwardButtons.js]
> +skip-if = os == "mac" = Bug 840022 - Test failure "The forward button has been made visible for the 1 page"

Still missing the single quotes I mentioned.
Attachment #793373 - Flags: review?(andreea.matei) → review-
Attached patch skipFollowup_2508.patch (obsolete) — Splinter Review
Changed to single quotes as requested, I will provide a fix patch as soon as I am able.
Attachment #793373 - Attachment is obsolete: true
Attachment #795325 - Flags: review?(andrei.eftimie)
Attachment #795325 - Flags: review?(andreea.matei)
Comment on attachment 795325 [details] [diff] [review]
skipFollowup_2508.patch

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

Landed followup:
http://hg.mozilla.org/qa/mozmill-tests/rev/6ae7cc15c140 (default)
http://hg.mozilla.org/qa/mozmill-tests/rev/0105774d9354 (aurora)
Attachment #795325 - Flags: review?(andrei.eftimie)
Attachment #795325 - Flags: review?(andreea.matei)
Attachment #795325 - Flags: review+
I have tried to reproduce this on the failing CI machine after removing the skip but I wasn't able to. Could this be related to certain platform issues? In order to be sure that we aren't getting this anymore could we back-out the skip patch for Default?
I tried again today to reproduce it but I am still not able. I took offline the mm-osx-106-2 machine and ran 300 times the test using mozmill -t for the folder testToolbar.
Lets reenable this test once we have mozmill 2.0 running in CI
Mario, when you have a spare machine, please check if this still reproduces with 2.0.3. Thanks.
I have unskipped and tested this on a local Mac machine and the issue doesn't reproduce anymore. I think it would be safe to unskip.

http://mozmill-crowd.blargon7.com/#/functional/reports?branch=All&platform=Mac&from=2014-01-13&to=2014-01-13
Daniel, could you please check if this is safe to unskip and if so prepare a patch? When you have some spare time. I can't backout cause it had follow up patch as well. Thanks!
Assignee: mario.garbi → daniel.gherasim
Status: REOPENED → ASSIGNED
In 105 runs it didn't fail once on latest nightly:

http://mozmill-crowd.blargon7.com/#/functional/reports?branch=30.0&platform=Mac&from=2014-02-06&to=2014-02-06

Unskip patch provided.
Attachment #785709 - Attachment is obsolete: true
Attachment #795325 - Attachment is obsolete: true
Attachment #8371520 - Flags: review?(andreea.matei)
Please be careful about the commit message, it should describe the patch (in this case enabling a test) and have the reviewers added. Thanks

Enabled:
http://hg.mozilla.org/qa/mozmill-tests/rev/4e10acdf95ca (default)
Comment on attachment 8371520 [details] [diff] [review]
unskip_testBackForwardButtons.patch

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

Forgot to change the flag. Let's check the other branches as well if nothing fails on default.
Attachment #8371520 - Flags: review?(andreea.matei) → review+
Attachment #8379609 - Flags: review?(andrei.eftimie)
Attachment #8379609 - Flags: review?(andreea.matei)
There was no need for the new patches, you should've just checked if the nightly patch applies.

Transplanted:
http://hg.mozilla.org/qa/mozmill-tests/rev/dfe91a9223fa (aurora)
http://hg.mozilla.org/qa/mozmill-tests/rev/fc2a7f1eaf51 (beta)

Release and ESR24 are left.
Attachment #8379646 - Attachment is obsolete: true
Attachment #8379646 - Flags: review?(andrei.eftimie)
Attachment #8379646 - Flags: review?(andreea.matei)
Attachment #8379609 - Attachment is obsolete: true
Attachment #8379609 - Flags: review?(andrei.eftimie)
Attachment #8379609 - Flags: review?(andreea.matei)
Previous patch applies cleanly on RELEASE.

REPORT:
http://mozmill-crowd.blargon7.com/#/functional/reports?app=All&branch=27.0&platform=All&from=2014-02-22&to=2014-02-25

--

For ESR24, the previous patch doesn't apply as we have different manifest.ini file.

REPORT
http://mozmill-crowd.blargon7.com/#/functional/reports?app=All&branch=24.3&platform=All&from=2014-02-25&to=2014-02-25
Attachment #8381257 - Flags: review?(andreea.matei)
Comment on attachment 8381257 [details] [diff] [review]
enableTestBackForwardButtons_esr.patch

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

http://hg.mozilla.org/qa/mozmill-tests/rev/cd1037310422 (release)
http://hg.mozilla.org/qa/mozmill-tests/rev/abc280f5d110 (esr24)

Please don't forget to also add the bug no. in the commit message.
Attachment #8381257 - Flags: review?(andreea.matei) → review+
Status: ASSIGNED → RESOLVED
Closed: 7 years ago6 years ago
Resolution: --- → FIXED
Whiteboard: [mozmill-test-failure][mozmill-test-skipped] → [mozmill-test-failure]
Failed twice today with Nightly de and it, both on 10.9.2 os x:
http://mozmill-daily.blargon7.com/#/functional/report/a438ea29b921b2e8124749eda95f10ea
http://mozmill-daily.blargon7.com/#/functional/report/a438ea29b921b2e8124749eda9605175

Daniel, could you check on that machine?
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Hm, not sure if related to that machine anymore. Failed once on 10.7.5, aurora de:
http://mozmill-daily.blargon7.com/#/functional/report/3ed2024184b13bf096824c08080ca4f0
No failure in the last month with this.
Status: REOPENED → RESOLVED
Closed: 6 years ago6 years ago
Resolution: --- → FIXED
I wouldn't mark 30 as fixed since we didn't change anything in particular to address the issue.
Product: Mozilla QA → Mozilla QA Graveyard
You need to log in before you can comment on or make changes to this bug.