Closed Bug 1352132 Opened 3 years ago Closed 3 years ago

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

Categories

(Testing :: Marionette, defect)

Version 3
defect
Not set

Tracking

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

RESOLVED FIXED
mozilla55
Tracking Status
firefox-esr52 --- fixed
firefox53 --- wontfix
firefox54 --- fixed
firefox55 --- fixed

People

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

References

Details

(Keywords: intermittent-failure, regression)

Attachments

(1 file)

Somewhat similar to bug 1350598.
See Also: → 1350598
See Also: 1350598
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
Attachment #8865585 - Flags: review?(ato)
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)
hg error in cmd: hg rebase -s 5bce2b21f125 -d d19f48b0f205: abort: can't rebase public changeset 5bce2b21f125
(see 'hg help phases' for details)
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
https://hg.mozilla.org/mozilla-central/rev/0bd894decfae
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
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
Keywords: regression
Whiteboard: [checkin-needed-beta][checkin-needed-esr52]
https://hg.mozilla.org/releases/mozilla-beta/rev/6c83cdd103e1
Flags: in-testsuite+
Whiteboard: [checkin-needed-beta][checkin-needed-esr52] → [checkin-needed-esr52]
You need to log in before you can comment on or make changes to this bug.