Skip to content

Commit

Permalink
compute pressure: Rewrite rate obfuscation web tests as unit tests
Browse files Browse the repository at this point in the history
These tests have been flaky since they have been rewritten to use
WebDriver (bug 347031400). https://crrev.com/c/5898989 added them to
SlowTests, but that did not work: contrary to what I expected, marking a
test as long in WPT's metadata and adding it to SlowTests are
idempotent, not cumulative operations (i.e. I thought adding a test to
SlowTests would increase the timeout even more, but it's just a no-op if
the test is already marked as long).

However, after investigating it further I believe this is not just a
timeout problem: these tests cannot work anymore by definition. The idea
of these tests is to send a number of updates and verify that:

* Sending a number of updates smaller than the spec-defined threshold to
  trigger the mitigations causes the difference between
  PressureRecord.time in subsequent updates to be smaller than the
  penalty time.

* Sending a number of updates greater than the spec-defined threshold
  does trigger the mitigation and cause update N within a spec-defined
  range to be sent with an implementation-defined delay.

In all cases, we need to send an update to WebDriver and verify that
PressureObserver has received it before sending the next one.
Consequently, the time it takes between sending and receiving updates is
actually outside the test's control: busy bots (such as the mac-rel and
win-rel ones) can simply cause the web test to WebDriver or the
WebDriver to browser process communication to take arbitrarily long,
leading to:

* Measured time between updates that should not trigger a penalty still
  taking longer than allowed because the system took too long to deliver
  the update_virtual_pressure_source() call. This causes the "not
  triggered" test to fail and leads to false positives in the
  "triggered" test.

* Calls to update_virtual_pressure_source() taking too long to be on the
  browser side so that and/or too few updates being sent from the
  browser side while waiting under penalty so that the difference
  between the pre-penalty update's timestamp and the
  update-under-penalty's timestamp ends up being less than the minimum
  penalty time. This makes the "triggered" test never flip the
  |gotPenalty| flag to true.

This was not a problem in the past because the MojoJS-based tests were
restricted to Blink.

The only way to have full control over the timing is to use unit tests
where we can have a mock time source that we move forward as we see fit.

Bug: 370726578, 347031400
Change-Id: I9b21513c7b8e83177e7d7b091607100d0cd21c87
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5909373
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Auto-Submit: Raphael Kubo Da Costa <raphael.kubo.da.costa@intel.com>
Commit-Queue: Reilly Grant <reillyg@chromium.org>
Commit-Queue: Raphael Kubo Da Costa <raphael.kubo.da.costa@intel.com>
Cr-Commit-Position: refs/heads/main@{#1364368}
  • Loading branch information
rakuco authored and chromium-wpt-export-bot committed Oct 4, 2024
1 parent 4e74229 commit 5861fcc
Show file tree
Hide file tree
Showing 3 changed files with 0 additions and 124 deletions.
11 changes: 0 additions & 11 deletions compute-pressure/README.md
Original file line number Diff line number Diff line change
@@ -1,17 +1,6 @@
This directory contains tests for the
[Compute Pressure](https://w3c.github.io/compute-pressure/) specification.

## Notes to implementors
`compute_pressure_rate_obfuscation_mitigation_*`, which test the spec's [rate
obfuscation
sections](https://w3c.github.io/compute-pressure/#rate-obfuscation), need to
send and wait for dozens of updates.

Some tests may need to send over 90 changes and then wait up to 10 seconds for
the next update. At 1 change per second, this would by far exceed WPT's timeout
limits, so implementations need to be able to dispatch updates at a higher rate
than usual for these tests to pass.

## How to write tests
### Tests that only need to run on a window
To test this API, one needs to be able to control the pressure data that will
Expand Down

This file was deleted.

This file was deleted.

0 comments on commit 5861fcc

Please sign in to comment.