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

Status: RESOLVED FIXED

(Testing :: Marionette Client and Harness, defect)

Description

[Treeherder Bug Filer]

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

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Somewhat similar to bug 1350598.

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+1]]

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.

Comment 3

[Andreas Tolfsen ❲: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".

Comment 4

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Alright. Lets take the `Wait().until()` approach then.

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: Bug 1352132 - Navigation unit tests have to wait for the page loaded in the newly opened tab.

There is a race condition for all navigation tests inside the setUp
method of the BaseNavigationTestCase class. The assert for history
items can already happen if the page hasn't been fully loaded yet.
As such a failure is thrown. To fix this we have to wait for the
page being loaded.

Review commit: https://reviewboard.mozilla.org/r/137210/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/137210/

Comment 6

[Andreas Tolfsen ❲: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

Comment 7

[Mozilla Autoland]

hg error in cmd: hg rebase -s 5bce2b21f125 -d d19f48b0f205: abort: can't rebase public changeset 5bce2b21f125
(see 'hg help phases' for details)

Comment 8

[Henrik Skupin [:whimboo][⌚️UTC+1]]

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

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/137210/diff/1-2/

Comment 9

[Pulsebot]

Pushed by [e-mail]:
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

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/0bd894decfae

Comment 11

[Henrik Skupin [:whimboo][⌚️UTC+1]]

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.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/6c83cdd103e1

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr52/rev/587f5295339a

Comment 14

[BMO Automation]

Moving bug to Testing::Marionette Client and Harness component per bug 1815831.