Closed Bug 1328298 Opened 4 years ago Closed 3 years ago

Unbreak WPT navigation tests

Categories

(Testing :: Marionette, defect)

Version 3
x86_64
Linux
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED WORKSFORME

People

(Reporter: ato, Assigned: ato)

References

Details

Attachments

(7 files)

After recent updates to wdclient, WPT, geckodriver, and Marionette there are many problems with the WPT navigation tests.
Assignee: nobody → ato
Status: NEW → ASSIGNED
Comment on attachment 8823305 [details]
Bug 1328298 - Rename webdriver.handles to window_handle;

https://reviewboard.mozilla.org/r/101876/#review102866
Attachment #8823305 - Flags: review?(dburns) → review+
Comment on attachment 8823307 [details]
Bug 1328298 - Navigating to malformed URL returns error;

https://reviewboard.mozilla.org/r/101880/#review102868
Attachment #8823307 - Flags: review?(dburns) → review+
Comment on attachment 8823308 [details]
Bug 1328298 - Call correct element retrieval function;

https://reviewboard.mozilla.org/r/101882/#review102870
Attachment #8823308 - Flags: review?(dburns) → review+
Comment on attachment 8823309 [details]
Bug 1328298 - Remove guard as navigating without browsing context is now fixed;

https://reviewboard.mozilla.org/r/101884/#review102872
Attachment #8823309 - Flags: review?(dburns) → review+
Comment on attachment 8823304 [details]
Bug 1328298 - Use default scope for http fixture;

https://reviewboard.mozilla.org/r/101874/#review102268
Attachment #8823304 - Flags: review?(james) → review+
Comment on attachment 8823306 [details]
Bug 1328298 - Set geckodriver log level to info;

https://reviewboard.mozilla.org/r/101878/#review102908

::: testing/web-platform/harness/wptrunner/webdriver_server.py:153
(Diff revision 1)
>                  "--port=%s" % str(self.port)]
>  
>  
>  class GeckoDriverServer(WebDriverServer):
> -    def __init__(self, logger, marionette_port=2828, binary="wires",
> -                 host="127.0.0.1", port=None):
> +    def __init__(self, logger, marionette_port=2828, binary="geckodriver",
> +                 host="127.0.0.1", port=None, log_level="info"):

Are you intending to have a way to set this? I mean it doesn't look wrong, but I don't exactly see how it helps.
Attachment #8823306 - Flags: review?(james)
Comment on attachment 8823306 [details]
Bug 1328298 - Set geckodriver log level to info;

https://reviewboard.mozilla.org/r/101878/#review102908

> Are you intending to have a way to set this? I mean it doesn't look wrong, but I don't exactly see how it helps.

There’s no way to set this yet, but it’s useful for debugging.  If you prefer I can remove the kwarg so it’s not public?
Comment on attachment 8823306 [details]
Bug 1328298 - Set geckodriver log level to info;

https://reviewboard.mozilla.org/r/101878/#review103912

::: testing/web-platform/harness/wptrunner/webdriver_server.py:153
(Diff revision 1)
>                  "--port=%s" % str(self.port)]
>  
>  
>  class GeckoDriverServer(WebDriverServer):
> -    def __init__(self, logger, marionette_port=2828, binary="wires",
> -                 host="127.0.0.1", port=None):
> +    def __init__(self, logger, marionette_port=2828, binary="geckodriver",
> +                 host="127.0.0.1", port=None, log_level="info"):

Yeah, let's do that.
Attachment #8823306 - Flags: review+
Sorry, ato, we never landed this after updating tooltool to geckodriver 0.13. Want to rebase and land?
Flags: needinfo?(ato)
Rebasing caused no conflicts actually, but I found some problems.
Flags: needinfo?(ato)
Comment on attachment 8830734 [details]
Bug 1328298 - Disable WebDriver test for navigation to file://;

https://reviewboard.mozilla.org/r/107468/#review108900

r+wc

::: testing/web-platform/tests/webdriver/navigation.py:70
(Diff revision 1)
>      assert session.url == "about:blank"
>  
>  
>  # TODO(ato): This test requires modification to pass on Windows
> +"""
> +Disabled due to https://bugzilla.mozilla.org/show_bug.cgi?id=1332122

I assume we can't disable an individual test function in navigation.py.ini, which is why you're commenting it out here instead?
Attachment #8830734 - Flags: review?(mjzffr) → review+
Comment on attachment 8830734 [details]
Bug 1328298 - Disable WebDriver test for navigation to file://;

https://reviewboard.mozilla.org/r/107468/#review108900

> I assume we can't disable an individual test function in navigation.py.ini, which is why you're commenting it out here instead?

Yeah, so the metadata in navigation.py.ini defines the expected outcome and doesn’t disable tests.  Because this test times out, I found it better to comment it out since it prevents the subsequent tests from running.
Comment on attachment 8830734 [details]
Bug 1328298 - Disable WebDriver test for navigation to file://;

https://reviewboard.mozilla.org/r/107468/#review108900

> Yeah, so the metadata in navigation.py.ini defines the expected outcome and doesn’t disable tests.  Because this test times out, I found it better to comment it out since it prevents the subsequent tests from running.

maja_zf: Can this issue be resolved?
Marking https://bugzilla.mozilla.org/show_bug.cgi?id=1319237 as a dependency for this bug because it fixes testing/web-platform/tests/webdriver/contexts.py, which fails on try runs with geckodriver 0.13.0 and above.
Depends on: 1319237
Andreas, given that the dependent bug is fixed now, can this patch series be landed?Or is more work necessary?
Flags: needinfo?(ato)
I think this has lagged behind WPT sufficiently that it needs to be reworked from scratch.  This bug can possibly be repurposed to unbreak previously-working Wd tests that were broken in the recent WPT sync.
Flags: needinfo?(ato)
these appear to be working
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.