Unbreak WPT navigation tests

RESOLVED WORKSFORME

Status

Testing
Marionette
RESOLVED WORKSFORME
11 months ago
3 months ago

People

(Reporter: ato, Assigned: ato)

Tracking

Version 3
x86_64
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

MozReview Requests

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

Attachments

(7 attachments)

(Assignee)

Description

11 months ago
After recent updates to wdclient, WPT, geckodriver, and Marionette there are many problems with the WPT navigation tests.
(Assignee)

Updated

11 months ago
Assignee: nobody → ato
Status: NEW → ASSIGNED
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 7

11 months ago
mozreview-review
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 8

11 months ago
mozreview-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 9

11 months ago
mozreview-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 10

11 months ago
mozreview-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 11

11 months ago
mozreview-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 12

11 months ago
mozreview-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)
(Assignee)

Comment 13

11 months ago
mozreview-review-reply
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 14

11 months ago
mozreview-review
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+
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Sorry, ato, we never landed this after updating tooltool to geckodriver 0.13. Want to rebase and land?
Flags: needinfo?(ato)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Comment 33

11 months ago
Rebasing caused no conflicts actually, but I found some problems.
Flags: needinfo?(ato)

Comment 34

11 months ago
mozreview-review
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+
(Assignee)

Comment 35

11 months ago
mozreview-review-reply
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.
(Assignee)

Comment 36

10 months ago
mozreview-review-reply
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?
(Assignee)

Comment 37

10 months ago
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
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Andreas, given that the dependent bug is fixed now, can this patch series be landed?Or is more work necessary?
Flags: needinfo?(ato)
(Assignee)

Comment 53

10 months ago
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
Last Resolved: 3 months ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.