Open Bug 1522790 Opened Last year Updated 2 months ago

Marionette executor should use "New Window" command to open the test tab

Categories

(Testing :: web-platform-tests, enhancement)

enhancement
Not set

Tracking

(Not tracked)

People

(Reporter: whimboo, Unassigned)

References

(Depends on 1 open bug)

Details

(Whiteboard: [geckoview:p3])

Attachments

(2 files)

Now that we have the support for the "New Window" command in Marionette, we might want to make use of it.

Depends on: 1523234

Wes, can you add this to your todo list and coordinate with whimboo if you have any questions.

Flags: needinfo?(wkocher)

Note that bug 1523234 isn't needed here because wptrunner expects to have the new tab focused.

No longer depends on: 1523234

All the window.open() calls, which are not only in the py files, but also in various JS modules like:

https://searchfox.org/mozilla-central/source/testing/web-platform/tests/tools/wptrunner/wptrunner/executors/reftest.js

Flags: needinfo?(hskupin)

It's not all, since it can only be used with resources that depend on executormarionette.

I put together a quick patch at https://treeherder.mozilla.org/#/jobs?repo=try&revision=2f2ee65df66a329d7d5acda114a30ddf96a623d7

There are a couple of problems:

  1. The new tab opened in this way appears to show the default nee tab page and not about:blank as required by the spec. This is probably slowing tests down a little.
  2. This depends on an unreleased marionette_driver. If the plan is to not make more releases then we'll need to either avoid using the client methods here or put a copy of the client in [testing/web-platform/tests]tools/third_party for wptrunner to use.

Re 1, I'd suggest setting the new tab and homepage prefs to be about:blank in one of the unittest profiles that wpt depends on.

Depends on: 1530794

Re 2, I will get a new marionette driver package released via bug 1530794.

That last try push looks like a complete fail someone needs to be investigate. I'm happy to fix a possible underlying issue with Marionette.

I just released marionette_driver 2.8.0 to PyPI, which includes support for the new window command. So this unblocks the implementation here.

Depends on: 1533058

Here's jgraham's patch and a patch to set the homepage to about:blank in the wpt testing profile, rebased to current central.

And a try push to see what that looks like: https://treeherder.mozilla.org/#/jobs?repo=try&group_state=expanded&revision=3940a503f1c911c6fb5c8c212e18d8d5fada5a7d

Do I need to do anything to pick up the new marionette_driver now that bug 1530794 is completed? Are there any parts of jgraham's patch that can be dropped now?

You have to make sure to at least bump the dependency for marionette_driver.

Here's a try push with the dependency bumped: https://treeherder.mozilla.org/#/jobs?repo=try&group_state=expanded&selectedJob=234671713&revision=11d24ebc5935bc60a540d1dfee13189ed6ec896f

Looks like everything's still broken, much in the same way it was for jgraham's original try push from comment 5. It's not every single test failing, though. There's one chunk that passed completely, and there are several TEST-OKs in each of the failing chunks. I'm not sure why things are failing. Are they occasionally getting the wrong window or something?

Flags: needinfo?(james)
Flags: needinfo?(hskupin)

Eh I think it was just a silly error; for some reason I thought Create Window switched to the new window, but it turns out it doesn't. So https://treeherder.mozilla.org/#/jobs?repo=try&revision=608fb12a70a9b1ce8451fdacd51fabd114d15818 is a try push with that fixed.

Flags: needinfo?(james)

Looks better. Still seems like there are several failures, and I'm seeing some of the JavascriptException: TypeError: window.__wptrunner_process_next_event is not a functions that we've been seeing in bug 1522790.

Some seem to be known intermittents, some UNEXPECTED-PASSes. Maybe this fixed some of our tests? Maybe we're just on a bad base revision?

(In reply to James Graham [:jgraham] from comment #16)

Eh I think it was just a silly error; for some reason I thought Create Window switched to the new window, but it turns out it doesn't.

Not by default. If you want that behavior you have to specify focus=True. I can see that this was already used in Wes' try push, so what exactly did you change?

Flags: needinfo?(hskupin) → needinfo?(james)

Not by default. If you want that behavior you have to specify focus=True. I can see that this was already used in Wes' try push, so what exactly did you change?

I mean "switched the webdriver active browsing context to"

Still seems like there are several failures, and I'm seeing some of the JavascriptException: TypeError: window.__wptrunner_process_next_event is not a functions that we've been seeing in bug 1522790.

I'm pretty sure there's a race condition that's causing the __process_next_event thing; I'm investigating.

Flags: needinfo?(james)

https://treeherder.mozilla.org/#/jobs?repo=try&author=james%40hoppipolla.co.uk&selectedJob=235205397 is a try push that ports the manual logic to make the runner wait for pending loads over to marionette (we were already using this for Chrome which doesn't support the right pageLoadStragtegy). This looks like it improved things, although there are still quite a few changed results which could be due to focus changes or something; this needs some investigation. That same patch could be a workaround in bug 1501562 so I suggest trying it there and see if it helps. But in any case it's clearly working around a bug in marionette in each case, so it's not the correct long-term solution.

Flags: needinfo?(wkocher)

So on Geckoview, if I don't also have a time.sleep(2) prior to the navigate(), wait_for_load() ends up in an infinite loop. location.href is 'about:blank', and document.readyState is 'complete'.

If I do have the time.sleep(2) up there, wait_for_load() is called once. location.href is the wpt test url, and document.readyState is 'complete'.

Flags: needinfo?(wkocher)
Whiteboard: [geckoview] → [geckoview:p3]
You need to log in before you can comment on or make changes to this bug.