-
-
Notifications
You must be signed in to change notification settings - Fork 8.4k
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
[dotnet] [bidi] Align Scipt.LocalValue.Map
with spec, enable negative zero
#15395
Conversation
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
PR Code Suggestions ✨Explore these optional code suggestions:
|
Scipt.LocalValue.Map
with specScipt.LocalValue.Map
with spec
On my Firefox your test passes when we |
OK, so in Firefox everything works good without any custom double converters. Chromium cannot handle I posted w3c/webdriver-bidi#887 issue in BiDi spec. I propose to not introduce custom converter at this time, and move forward with ignored unit tests (for chromium only). PS: I was wrong, spec requires
|
Formatting build failure unrelated: --- a/py/test/selenium/webdriver/common/selenium_manager_tests.py
+++ b/py/test/selenium/webdriver/common/selenium_manager_tests.py Test failures not related: //py:common-chrome-bidi-test/selenium/webdriver/common/bidi_tests.py
//py:common-edge-bidi-test/selenium/webdriver/common/bidi_tests.py
//rb/spec/integration/selenium/webdriver:action_builder-chrome-remote
//rb/spec/integration/selenium/webdriver:driver-chrome
//rb/spec/integration/selenium/webdriver:driver-chrome-remote
//rb/spec/integration/selenium/webdriver:driver-firefox-beta
//rb/spec/integration/selenium/webdriver:manager-chrome
//rb/spec/integration/selenium/webdriver:select-chrome
//rb/spec/integration/selenium/webdriver:select-chrome-remote |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good test coverage, thanks!
User description
Fixes using the

Script.LocalValue.Map
type and the related return type. The spec describes it the same asObject
:So this is now a "clone" of

LocalValue.Object
.Additionally, this PR deserializes negative zero from a string, in alignment with the spec:
Motivation and Context
Fixes #15394
Types of changes
Checklist
PR Type
Bug fix, Tests
Description
Fixed
Script.LocalValue.Map
to align with the WebDriver BiDi spec.Updated
RemoteValue.Map
to useIReadOnlyList<IReadOnlyList<RemoteValue>>
.Added comprehensive tests for
LocalValue
andRemoteValue
roundtrip scenarios.Enhanced JSON serialization options to handle named floating-point literals.
Changes walkthrough 📝
Broker.cs
Enhanced JSON serialization options.
dotnet/src/webdriver/BiDi/Communication/Broker.cs
LocalValue.cs
Updated `LocalValue.Map` structure.
dotnet/src/webdriver/BiDi/Modules/Script/LocalValue.cs
Map
to useIEnumerable>
.Object
serialization structure.RemoteValue.cs
Modified `RemoteValue.Map` structure.
dotnet/src/webdriver/BiDi/Modules/Script/RemoteValue.cs
Map
to useIReadOnlyList>
.CallFunctionParameterTest.cs
Added comprehensive tests for `LocalValue` and `RemoteValue`.
dotnet/test/common/BiDi/Script/CallFunctionParameterTest.cs
LocalValue
andRemoteValue
roundtripscenarios.
LocalValue
types.
Map
andObject
structures for compliance with the spec.