Closed Bug 1262662 Opened 8 years ago Closed 8 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
https://hg.mozilla.org/mozilla-central/rev/9d1bc9b90bb7
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 49
You need to log in before you can comment on or make changes to this bug.