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

Implement logging #240

Closed
sadym-chromium opened this issue Aug 18, 2022 · 12 comments
Closed

Implement logging #240

sadym-chromium opened this issue Aug 18, 2022 · 12 comments

Comments

@sadym-chromium
Copy link
Collaborator

WPT tests for logging should pass: https://wpt.fyi/results/webdriver/tests/bidi/log?label=master&label=experimental&aligned&q=webdriver&view=subtest

@whimboo
Copy link

whimboo commented Aug 18, 2022

@sadym-chromium please note that I'm currently removing all the current_session.execute_script calls in the log.entryAdded tests over in https://bugzilla.mozilla.org/show_bug.cgi?id=1784532. Most likely these are blocking you here.

@sadym-chromium
Copy link
Collaborator Author

@whimboo thanks for the heads-up! We'll wait for migration then.

@whimboo
Copy link

whimboo commented Aug 18, 2022

The sync is going to happen via web-platform-tests/wpt#35513

@whimboo
Copy link

whimboo commented Aug 19, 2022

The sync is done and all traces for current_session are gone from the bidi tests. Do these tests work for you now?

@sadym-chromium
Copy link
Collaborator Author

We don't have message caching implemented, so the tests still don't pass on our side. WIP.

@sadym-chromium
Copy link
Collaborator Author

@whimboo another problem is in the fact WPT tests having specific value of the text field, while spec says:

If arg is a primitive ECMAScript value, append ToString(arg) to text. Otherwise append an implementation-defined string to text.

@whimboo
Copy link

whimboo commented Aug 22, 2022

This should actually be related to the discussion on w3c/webdriver-bidi#139. The webdriver tests should most likely check for any_string in the text field?

@juliandescottes
Copy link

Yes for anything non-primitive, we should use any_string instead of expecting a specific text. Primitive log values should still be asserted as-is I think.

@juliandescottes
Copy link

@sadym-chromium do you want to update the tests yourself as part of your work, or should we do it?

@sadym-chromium
Copy link
Collaborator Author

I'll publish PR with that + removing verification of implementation-defined text field.

@sadym-chromium
Copy link
Collaborator Author

@sadym-chromium
Copy link
Collaborator Author

Implemented.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants