-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
sensors: Convert existing Generic Sensor web tests to test_driver #41410
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
wpt-pr-bot
approved these changes
Aug 10, 2023
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.
The review process for this patch is being conducted in the Chromium project.
533942a
to
a1e7367
Compare
d9e0bab
to
61b02c3
Compare
27adc3b
to
af81e95
Compare
a1ff5ed
to
ad6e4bc
Compare
All Generic Sensor web tests are now using virtual sensors and their implementation in WPT's testdriver.js and ChromeDriver/content_shell. These tests no longer depend on generic_sensor_mocks.js (or Mojo mocks in general), which has been kept only for the Device Orientation tests at this point, and should be fully interoperable now. This also exercises the code in //content and //services. Compared to the JS mocks-based implementation, one of the biggest changes is that there is no equivalent to a MockSensor that takes an array of readings and periodically rotates and reports them to consumers. This was an implementation detail that was kind of hard to justify in spec terms, and the interaction between the JS timer and the C++ code in services could lead to races between shared buffer values and expected values (bug 1073865 is probably related, for example). We now follow the new Automation section in the Generic Sensor spec and explicitly require reading updates to be done via calls to test_driver.update_virtual_sensor(). These calls might not result in a change in readings if the sensor is not active or if the threshold/rounding checks fail, so the tests are careful to check that a reading has gone through when necessary. Furthermore, given how the code in //services is implemented it is not possible to assert that a call to update_virtual_sensor() will resolve _before_ a "reading" event is emitted, so some Promise.all() calls are required. The iframe tests have been essentially rewritten for clarity and to test the right things: * iframe_sensor_handler.html is used only by the cross-origin test. The same-origin one simply creates the sensor in the test file directly. * send_message_to_iframe() does not take a |reply| argument anymore. Instead, it propagate the replies to the callers so they can do whatever they need and we do not need to come up with values like "success" in replies. * The cross-origin and same-origin tests follow a workflow that is easier to follow and mostly relies on timestamps that are part of the public API to determine whether readings are being provided or not depending on frame focus. Now that the //content code is being exercised, the Ambient Light Sensor and Magnetometer web tests need the GenericSensorExtraClasses feature to be enabled, which is done by the newly-created generic-sensor-extra-classes virtual test suite. Finally, the referenceFrame test in generic-sensor-tests.js has been disabled and a copy was added to wpt_internal, as there is no cross-platform way to set an orientation angle and in wpt_internal it can be set via Internals' setMockScreenOrientation(). Bug: 1278377, 1471996 Change-Id: Ie556c6d7bcfc0b2b84abc5f0770f6b3120ec81a2
ad6e4bc
to
0d15c5b
Compare
past
approved these changes
Oct 26, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
All Generic Sensor web tests are now using virtual sensors and their
implementation in WPT's testdriver.js and ChromeDriver/content_shell.
These tests no longer depend on generic_sensor_mocks.js (or Mojo mocks
in general), which has been kept only for the Device Orientation tests
at this point, and should be fully interoperable now. This also
exercises the code in //content and //services.
Compared to the JS mocks-based implementation, one of the biggest
changes is that there is no equivalent to a MockSensor that takes an
array of readings and periodically rotates and reports them to
consumers. This was an implementation detail that was kind of hard to
justify in spec terms, and the interaction between the JS timer and the
C++ code in services could lead to races between shared buffer values
and expected values (bug 1073865 is probably related, for example).
We now follow the new Automation section in the Generic Sensor spec and
explicitly require reading updates to be done via calls to
test_driver.update_virtual_sensor(). These calls might not result in a
change in readings if the sensor is not active or if the
threshold/rounding checks fail, so the tests are careful to check that a
reading has gone through when necessary. Furthermore, given how the code
in //services is implemented it is not possible to assert that a call to
update_virtual_sensor() will resolve before a "reading" event is
emitted, so some Promise.all() calls are required.
The iframe tests have been essentially rewritten for clarity and to test
the right things:
same-origin one simply creates the sensor in the test file directly.
Instead, it propagate the replies to the callers so they can do
whatever they need and we do not need to come up with values like
"success" in replies.
easier to follow and mostly relies on timestamps that are part of the
public API to determine whether readings are being provided or not
depending on frame focus.
Now that the //content code is being exercised, the Ambient Light Sensor
and Magnetometer web tests need the GenericSensorExtraClasses feature to
be enabled, which is done by the newly-created
generic-sensor-extra-classes virtual test suite.
Finally, the referenceFrame test in generic-sensor-tests.js has been
disabled and a copy was added to wpt_internal, as there is no
cross-platform way to set an orientation angle and in wpt_internal it
can be set via Internals' setMockScreenOrientation().
Bug: 1278377, 1471996
Change-Id: Ie556c6d7bcfc0b2b84abc5f0770f6b3120ec81a2
Reviewed-on: https://chromium-review.googlesource.com/4770867
WPT-Export-Revision: 3fbc35c1d2fc47d3c40119e1f0a99ae7bf33a525