1378161 - browser/base/content/test/general/browser_bookmark_popup.js permafails on cedar (presumably will perma-fails once 56 merges to beta)

Status: RESOLVED FIXED

(Firefox :: Address Bar, defect)

Description

[:Gijs (out for now; he/him)]

https://treeherder.mozilla.org/#/jobs?repo=cedar&selectedJob=111728787

Several failures with messages like this:

07:49:30     INFO - TEST-UNEXPECTED-FAIL | browser/base/content/test/general/browser_bookmark_popup.js | Page is starred after closing - Got false, expected true
07:49:30     INFO - Stack trace:
07:49:30     INFO - chrome://mochikit/content/browser-test.js:test_is:967
07:49:30     INFO - chrome://mochitests/content/browser/browser/base/content/test/general/browser_bookmark_popup.js:test_bookmarks_popup/<:69
07:49:31     INFO - TEST-UNEXPECTED-FAIL | browser/base/content/test/general/browser_bookmark_popup.js | Page should only be starred prior to popupshown if editing bookmark - Got false, expected true
07:49:31     INFO - TEST-UNEXPECTED-FAIL | browser/base/content/test/general/browser_bookmark_popup.js | Page is starred - Got , expected true
07:49:31     INFO - Stack trace:
07:49:31     INFO - chrome://mochikit/content/browser-test.js:test_is:967
07:49:31     INFO - chrome://mochitests/content/browser/browser/base/content/test/general/browser_bookmark_popup.js:test_bookmarks_popup/<:52

I expect I made a mistake updating this test in bug 1352120.

Comment 1

[:Gijs (out for now; he/him)]

(In reply to Marco Bonardo [::mak] from bug 1352120 comment #13)
> Comment on attachment 8880009 [details]
> Bug 1352120 - to fold: test changes,
> 
> https://reviewboard.mozilla.org/r/151346/#review156760
> 
> ::: browser/base/content/test/general/browser_bookmark_popup.js:14
> (Diff revision 2)
> >   */
> >  
> >  let bookmarkPanel = document.getElementById("editBookmarkPanel");
> > -let bookmarkStar = document.getElementById("bookmarks-menu-button");
> > +let bookmarkStar = AppConstants.MOZ_PHOTON_THEME ?
> > +  document.getElementById("star-button") :
> > +  document.getElementById("bookmarks-menu-button");
> 
> Maybe you could use the BookmarkingUI getters here (star or anchor?). The
> test seems to use this just for the starred attribute and a click

Of course, I should have checked this more carefully - BookmarkingUI.star and .anchor both return the inner elements for the pre-photon bookmarks-menu-button, and the 'starred' attribute is only set on the outer element, so now the test fails in non-photon.

Comment 2

[:Gijs (out for now; he/him)]

Attachment: Bug 1378161 - update browser_bookmark_popup to correctly deal with the non-photon case,

Review commit: https://reviewboard.mozilla.org/r/154286/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/154286/

Comment 3

[Marco Bonardo [:mak]]

Comment on attachment 8883397 [details]
Bug 1378161 - update browser_bookmark_popup to correctly deal with the non-photon case,

https://reviewboard.mozilla.org/r/154286/#review159532

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d3964f81fbd4
update browser_bookmark_popup to correctly deal with the non-photon case, r=mak

Comment 5

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/d3964f81fbd4

Comment 6

[Intermittent Failures Robot]

27 failures in 656 pushes (0.041 failures/push) were associated with this bug in the last 7 days.   

Repository breakdown:
* cedar: 27

Platform breakdown:
* windows8-64: 10
* osx-10-10: 7
* windows7-32-vm: 5
* linux64: 5

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1378161&startday=2017-07-03&endday=2017-07-09&tree=all