Closed Bug 1262662 Opened 10 years ago Closed 10 years ago

Intermittent browser_newtabmessages.js | received an expect number of history items - Got 4, expected 5

Categories

(Firefox :: New Tab Page, defect)

47 Branch
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 49
Tracking Status
firefox48 --- fixed
firefox49 --- fixed

People

(Reporter: KWierso, Assigned: oyiptong)

References

Details

(Keywords: intermittent-failure)

Attachments

(1 file)

Olivier, any ideas whats going on here? This is the 4th most frequent orange at the moment.
Flags: needinfo?(oyiptong)
I think so. jkerim is going to be working on this as his first bug with my assistance. It's probably some state not being cleaned up properly between tests.
Flags: needinfo?(oyiptong)
Assignee: nobody → jkerim
See Also: → 1262024
This test is nearly permafail and is the top failure on OrangeFactor if the various different failure modes are added up. This needs a fix ASAP or a disabling in the mean time.
Flags: needinfo?(jkerim)
Ok. I'm taking a look at it instead of :jkerim and will make a patch ASAP
Assignee: jkerim → oyiptong
Flags: needinfo?(jkerim)
@oyiptong, check also line 55. I think there is a yield missing on that `cleanup();`.
Good catch, :marcosc, thanks!
(In reply to Olivier Yiptong [:oyiptong] from comment #15) > Good catch, :marcosc, thanks! Always happy to help :) Ping me if you need me to give anything JS a once over.
will do
This isn't complete yet, but I wanted to push something for mcaceres to review
Attachment #8745036 - Flags: review?(mcaceres) → review+
Comment on attachment 8745036 [details] MozReview Request: Bug 1262662 - clear history on startup for newtab messages tests r?marcosc https://reviewboard.mozilla.org/r/48783/#review45699 I can't see anything now that would be causing the intermittent now. ::: browser/components/newtab/tests/browser/browser_newtabmessages.js:124 (Diff revision 1) > visitDate: timeDaysAgo(daysAgo), > transition: (isTyped) ? TRANSITION_TYPED : TRANSITION_LINK, > }; > } > > yield PlacesTestUtils.clearHistory(); ClearHistory is already called on setup(), do you really want to call it again here? ::: browser/components/newtab/tests/browser/browser_newtabmessages.js:155 (Diff revision 1) > > yield BrowserTestUtils.withNewTab(tabOptions, function*() { > yield placesResponseAck; > let placesChangeAck = new Promise(resolve => { > NewTabWebChannel.once("clearHistoryAck", (_, msg) => { > ok(true, "a change response has been received"); maybe this should move to line 153? So it would be: ```JS yield placesResponseAck; ok(true, "a change response has been received"); ```
Comment on attachment 8745036 [details] MozReview Request: Bug 1262662 - clear history on startup for newtab messages tests r?marcosc Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48783/diff/1-2/
The intermittent still shows up on windows. I'm working on fixing that.
See Also: → 1271177
tracking the windows failures in bug 1271177
See Also: → 1262719
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 49
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: