Intermittent test_navigation.py TestBackForwardNavigation.test_no_history_items | AssertionError: 0 != 1

RESOLVED FIXED in Firefox -esr52

Status

Testing
Marionette
RESOLVED FIXED
9 months ago
5 months ago

People

(Reporter: Treeherder Bug Filer, Assigned: whimboo)

Tracking

({intermittent-failure, regression})

Version 3
mozilla55
intermittent-failure, regression
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox-esr52 fixed, firefox53 wontfix, firefox54 fixed, firefox55 fixed)

Details

MozReview Requests

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Reporter)

Description

9 months ago
treeherder
Filed by: archaeopteryx [at] coole-files.de

https://treeherder.mozilla.org/logviewer.html#?job_id=87575401&repo=mozilla-central

https://queue.taskcluster.net/v1/task/XoQQIAinRaSWiXjelNyXtQ/runs/0/artifacts/public/test_info//marionette_errorsummary.log
Somewhat similar to bug 1350598.
See Also: → bug 1350598
Blocks: 1350598
See Also: bug 1350598
Blocks: 1362600
The BaseNavigationTestCase class has the following code in it's `setUp` method:

        # Always use a blank new tab for an empty history
        self.marionette.navigate(self.marionette.absolute_url("windowHandles.html"))
        self.new_tab = self.open_tab(open_with_link)
        self.marionette.switch_to_window(self.new_tab)
        self.assertEqual(self.history_length, 1)

It means that it opens a new window with the target HTML page, but it doesn't seem to wait until the page has been finished loading, and the history item got added. 

Andreas, should `switch_to_window` wait for the page load? At least I cannot find something in the spec. Maybe it's missing? If tests don't actually care about it they could run into race conditions as we see here.
Flags: needinfo?(ato)
(In reply to Henrik Skupin (:whimboo) from comment #2)

> Andreas, should `switch_to_window` wait for the page load? At least I
> cannot find something in the spec. Maybe it's missing? If tests don't
> actually care about it they could run into race conditions as we see
> here.

As far as I’m aware, Selenium has never done this.  It’s never been
the intention that a subsequent command should ensure the document has
finished loading.  A new document may, in theory, begin loading at any
time, and Switch To Window shouldn’t be special cased to wait for said
document to load.

The “open new tab by clicking link” helper function is IMO a
workaround for the fact that WebDriver does not have an API to open a
new window.  If it did have this, it would be the responsibility of
that command to wait for the window to open and the document to finish
navigating before returning.  In any case, it should never be the
responsibility of the next command to finish the work of the last.

In this concrete case we need to put an explicit wait condition in
self.open_tab(open_with_link) so that it doesn’t return before the
window has opened and the document loaded.  This can be tricky because
it doesn’t currently switch to the new window internally.  There is
a possibility we need to change open_tab’s behaviour to implicitly
switch so that it can wait until the document has loaded.

Alternatively we need a separate helper function to run after
self.marionette.switch_to_window(self.new_tab) that waits for
document.readyState to reach "complete".
Flags: needinfo?(ato)
Alright. Lets take the `Wait().until()` approach then.
Assignee: nobody → hskupin
Status: NEW → ASSIGNED
Comment hidden (mozreview-request)
Attachment #8865585 - Flags: review?(ato)

Comment 6

7 months ago
mozreview-review
Comment on attachment 8865585 [details]
Bug 1352132 - Navigation unit tests have to wait for the page loaded in the newly opened tab.

https://reviewboard.mozilla.org/r/137210/#review140282
Attachment #8865585 - Flags: review?(ato) → review+
Attachment #8865585 - Flags: review?(mjzffr)

Comment 7

7 months ago
hg error in cmd: hg rebase -s 5bce2b21f125 -d d19f48b0f205: abort: can't rebase public changeset 5bce2b21f125
(see 'hg help phases' for details)
Comment hidden (mozreview-request)

Comment 9

7 months ago
Pushed by hskupin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0bd894decfae
Navigation unit tests have to wait for the page loaded in the newly opened tab. r=ato

Comment 10

7 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/0bd894decfae
Status: ASSIGNED → RESOLVED
Last Resolved: 7 months ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
This is actually a new failure introduced by my patch on bug 1330348. It would be good to get this test-only patch uplifted to beta, and esr52.
Blocks: 1330348
status-firefox54: --- → affected
status-firefox-esr52: --- → affected
Keywords: regression
Whiteboard: [checkin-needed-beta][checkin-needed-esr52]

Comment 12

7 months ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/6c83cdd103e1
status-firefox54: affected → fixed
Flags: in-testsuite+
Whiteboard: [checkin-needed-beta][checkin-needed-esr52] → [checkin-needed-esr52]

Comment 13

7 months ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-esr52/rev/587f5295339a
status-firefox-esr52: affected → fixed
Whiteboard: [checkin-needed-esr52]
status-firefox53: --- → wontfix
Blocks: 1354091
You need to log in before you can comment on or make changes to this bug.