Closed Bug 1262662 Opened 4 years ago Closed 4 years ago

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


(Firefox :: New Tab Page, defect)

47 Branch
Not set



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


(Reporter: KWierso, Assigned: oyiptong)



(Keywords: intermittent-failure)


(1 file)

Duplicate of this bug: 1262712
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

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:

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:
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
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 49
You need to log in before you can comment on or make changes to this bug.