Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Gecko Bug 1676679] support virtual authenticator functions in webdriver #41377

Merged
merged 1 commit into from
Sep 1, 2023

Conversation

moz-wptsync-bot
Copy link
Collaborator

Depends on D185198

Differential Revision: https://phabricator.services.mozilla.com/D162624

bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1676679
gecko-commit: af356c621a918b3dbd14ac75390d5915b3cc791b
gecko-reviewers: webdriver-reviewers, jgraham, whimboo

@wpt-pr-bot wpt-pr-bot added infra webdriver wg-testing_browser wptrunner The automated test runner, commonly called through ./wpt run labels Aug 8, 2023
Copy link
Collaborator

@wpt-pr-bot wpt-pr-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The review process for this patch is being conducted in the Firefox project.

@whimboo
Copy link
Contributor

whimboo commented Aug 14, 2023

These failures here exist because no new version of marionette-driver has been released. I did that so lets see if tests are passing now.

@whimboo whimboo closed this Aug 14, 2023
@whimboo whimboo reopened this Aug 14, 2023
@whimboo
Copy link
Contributor

whimboo commented Aug 14, 2023

We will actually have to wait until dependabot created a PR for the marionette client release.

@whimboo
Copy link
Contributor

whimboo commented Aug 22, 2023

The bump of marionette client happened last week via #41457.

As such we can go ahead and get this PR merged. I'll try to push a rebase on top of this change.

Depends on D185198

Differential Revision: https://phabricator.services.mozilla.com/D162624

bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1676679
gecko-commit: d918661c17e3b35d0fdc71e621020e4f2fbf51dd
gecko-reviewers: webdriver-reviewers, jgraham, whimboo
@whimboo
Copy link
Contributor

whimboo commented Aug 22, 2023

Tests for Firefox are passing now. @jgraham, @gsnedders or @DanielRyanSmith could one of you please admin merge? Thanks!

@whimboo
Copy link
Contributor

whimboo commented Sep 1, 2023

Tests for Firefox are passing now. @jgraham, @gsnedders or @DanielRyanSmith could one of you please admin merge? Thanks!

After a week without a reply could some @web-platform-tests/admins please admin merge this PR?

Note that the infra tests are broken because /infrastructure/testdriver/virtual_authenticator.html now unexpectedly pass with these changes. Thanks.

@whimboo whimboo added webauthn and removed printing labels Sep 1, 2023
@DanielRyanSmith DanielRyanSmith merged commit f8eef42 into master Sep 1, 2023
@DanielRyanSmith DanielRyanSmith deleted the gecko/1676679 branch September 1, 2023 07:58
@DanielRyanSmith
Copy link
Contributor

@whimboo Sorry, was OOO last week. Otherwise I'm usually be able to take care of these requests if/when the need arises. 🙂

@whimboo
Copy link
Contributor

whimboo commented Sep 1, 2023

No worries! Thanks a lot for merging!

@gsnedders
Copy link
Member

Note that the infra tests are broken because /infrastructure/testdriver/virtual_authenticator.html now unexpectedly pass with these changes. Thanks.

Could we somewhere better document that the expectations should be kept up-to-date? It would've been better to just update the infrastructure expectations? (c.f. https://github.com/web-platform-tests/wpt/tree/master/infrastructure)

@whimboo
Copy link
Contributor

whimboo commented Sep 1, 2023

I tried to run these infrastructure tests locally and I cannot get them to pass. They still fail with Unhandled rejection with value: "error: Action add_virtual_authenticator not implemented". Not sure why there is a difference between running these tests upstream and locally.

After having a deeper look into the wptrunner code I noticed that we actually missed to implement all the virtual authenticator methods in executormarionette.py. Doing that locally actually make the infrastructure tests work for me as well.

Sorry, that I didn't notice that. I'll have a PR ready soon as a follow-up to get this fixed. Thanks for noticing and reaching out!

Edited: I totally missed to actually pull from upstream before checking that so it is clear why it wasn't working locally. Nevertheless a PR for updating the test expectations has been opened.

Lightning00Blade pushed a commit to Lightning00Blade/wpt that referenced this pull request Dec 11, 2023
…sts#41377)

Depends on D185198

Differential Revision: https://phabricator.services.mozilla.com/D162624

bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1676679
gecko-commit: d918661c17e3b35d0fdc71e621020e4f2fbf51dd
gecko-reviewers: webdriver-reviewers, jgraham, whimboo

Co-authored-by: Dana Keeler <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants