Closed Bug 1411581 Opened 3 years ago Closed 2 years ago

Intermittent browser/components/sessionstore/test/browser_615394-SSWindowState_events_undoCloseTab.js | undefined assertion name - Got about:blank, expected about:rights

Categories

(Firefox :: Session Restore, defect, P2)

defect

Tracking

()

RESOLVED FIXED
Firefox 60
Tracking Status
firefox59 --- fixed
firefox60 --- fixed

People

(Reporter: intermittent-bug-filer, Assigned: mikedeboer)

Details

(Keywords: intermittent-failure, Whiteboard: [stockwell fixed:logic])

Attachments

(1 file)

There are 34 failures in the last 7 days. Most of them are on windows10-64. A spike in failures can be seen starting with 3rd of Jan.

Recent log: https://treeherder.mozilla.org/logviewer.html#?repo=mozilla-inbound&job_id=154384133&lineNumber=16265

15:08:41     INFO -  471 INFO TEST-PASS | browser/components/sessionstore/test/browser_615394-SSWindowState_events_undoCloseTab.js | undefined assertion name -
15:08:41     INFO -  Buffered messages finished
15:08:41    ERROR -  472 INFO TEST-UNEXPECTED-FAIL | browser/components/sessionstore/test/browser_615394-SSWindowState_events_undoCloseTab.js | undefined assertion name - Got about:blank, expected about:rights
15:08:41     INFO -  Stack trace:
15:08:41     INFO -  chrome://mochikit/content/browser-test.js:test_is:1269
Flags: needinfo?(mdeboer)
Whiteboard: [stockwell needswork]
There are 30 failures in the last 7 days. This seems to be affecting mochitests on all platforms and build types.

Any updates for this?
Taking a look.
Assignee: nobody → mdeboer
Status: NEW → ASSIGNED
Flags: needinfo?(mdeboer)
Priority: P5 → P2
Comment on attachment 8943259 [details]
Bug 1411581 - Mochitest 'browser_615394-SSWindowState_events_undoCloseTab.js' leaks windows too frequently; rewrite the test and wait for the browser to load, before finishing up the test.

https://reviewboard.mozilla.org/r/213588/#review220296

Thanks! Always nice to see these older tests get modernized with the good stuff. :)

::: browser/components/sessionstore/test/browser_615394-SSWindowState_events_undoCloseTab.js:29
(Diff revision 2)
>    function onSSWindowStateBusy(aEvent) {
>      busyEventCount++;
>    }
>  
>    function onSSWindowStateReady(aEvent) {
>      reopenedTab = gBrowser.tabs[1];

While you're here, is there any point in setting reopenedTab again here? That kinda weirds me out, since it's already being set via the return value of `undoCloseTab`, and it seems strange to set it again here when... I _think_ `reopenedTab` should already be the same as gBrowser.tabs[1]. So, like, maybe this should be:

```js
function onSSWindowStateReady(aEvent) {
  Assert.equal(gBrowser.tabs.length, 2, "Should only have 2 tabs");
  let lastTab = gBrowser.tabs[1];
  Assert.equal(lastTab, reopenedTab, "Last tab should be the re-opened tab");
  readyEventCount++;
  is(ss.getTabValue(reopenedTab, "foo"), "bar");
}
```
Attachment #8943259 - Flags: review?(mconley) → review+
(In reply to Mike Conley (:mconley) (:⚙️) - PTO until Jan 22 from comment #19)
> While you're here, is there any point in setting reopenedTab again here?
> That kinda weirds me out, since it's already being set via the return value
> of `undoCloseTab`, and it seems strange to set it again here when... I
> _think_ `reopenedTab` should already be the same as gBrowser.tabs[1]. So,
> like, maybe this should be:
> 
> ```js
> function onSSWindowStateReady(aEvent) {
>   Assert.equal(gBrowser.tabs.length, 2, "Should only have 2 tabs");
>   let lastTab = gBrowser.tabs[1];
>   Assert.equal(lastTab, reopenedTab, "Last tab should be the re-opened tab");
>   readyEventCount++;
>   is(ss.getTabValue(reopenedTab, "foo"), "bar");
> }
> ```

It's the same tab, but apparently the 'SSWindowStateReady' event is fired before the `undoCloseTab()` method finishes and returns the tab reference. So what I did instead was remove the tab reference equality check you requested together with the other assertions.
Pushed by mdeboer@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/59ece44fa56e
Mochitest 'browser_615394-SSWindowState_events_undoCloseTab.js' leaks windows too frequently; rewrite the test and wait for the browser to load, before finishing up the test. r=mconley
https://hg.mozilla.org/mozilla-central/rev/59ece44fa56e
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 60
Whiteboard: [stockwell needswork] → [stockwell fixed:logic]
You need to log in before you can comment on or make changes to this bug.