search

web-platform-tests / rfcs

web-platform-tests RFCs
75 stars 63 forks source link

RFC 81: Add testdriver.js support for forcing JS Self-Profiler sampling #81

Closed acomminos closed 1 year ago

acomminos commented 3 years ago

Hey folks,

This RFC helps implement testing of the JS Self-Profiling API by adding a normative mechanism to trigger sampling to testdriver.

Thanks a bunch for taking a look, and please let me know if you have any concerns!

acomminos commented 3 years ago

Friendly ping on this as it's been > 7 days :)

jgraham commented 3 years ago

Looking at the PR I guess this is calling some unspecified blink-internal API in the -vendor.js implementation? If so that's explicitly not enough; the APIs exposed to web platform tests need to be specified in enough detail that multiple interoperable implementations are possible.

foolip commented 3 years ago

This seems like a good fit for https://jgraham.github.io/browser-test/. Given a PR to define this in that spec, and a plan to bring it into the WHATWG, would this be good to go?

jgraham commented 3 years ago

For a PR for that spec to fit the WHATWG guidelines I think there'd have to be interest from two implementors? Is that a bar this would currently meet?

foolip commented 3 years ago

This seems like a good fit for test-only APIs, so I could express interest on Chromium's behalf to have real world test of that model beyond GC.

acomminos commented 3 years ago

Thanks for the feedback folks!

What does the implementation of this look like. testdriver can't specify APIs on its own it can only provide a frontend on either WebDriver-backed APIs or test-only APIs specified somewhere else. I don't see the relevant API surface in the WCIG document you linked.

The intent of test_driver.force_sample is to execute the Take a Sample algorithm in the linked processing model for all active profiling sessions on the page.

I agree it's a bit handwavy, and I'd be happy to specify that last bit of the function formally :)

This seems like a good fit for https://jgraham.github.io/browser-test/.

That sounds good to me! I know the Reporting API defines a method for automation in a separate section -- I'd also be happy to amend the JS Self-Profiling spec to define force_sample in a similar fashion.

For a PR for that spec to fit the WHATWG guidelines I think there'd have to be interest from two implementors? Is that a bar this would currently meet?

We also have positive interest from Edge for the general API, if that helps.

jgraham commented 3 years ago

We also have positive interest from Edge for the general API, if that helps.

Since Edge would presumably share an implementation with Chrome I believe it wouldn't count as seperate from the point of view of the policy.

acomminos commented 2 years ago

Hey folks, I've updated the spec to include a Force Sample WebDriver extension command that defines the target API surface. The RFC has been updated accordingly, plus a usage example has been added.

PTAL -- thanks!

acomminos commented 2 years ago

Friendly ping on this!

acomminos commented 2 years ago

Another ping on this -- this would really be helpful to reduce the flakiness of the js-self-profiling WPT tests.

Ms2ger commented 1 year ago

@acomminos hey - we discussed this in yesterday's meeting and were wondering if you were still interested in moving this forward. If so, what's the implementation status of the webdriver command like?

jgraham commented 1 year ago

So recently we changed policy so that testdriver additions which just reflect a WebDriver endpoint don't require an RFC. So in theory we'd accept a PR here. The only concern might be if the spec isn't on the standards track (e.g. WICG). I don't know if that's the case here, but either way we can discuss that in a PR rather than in an RFC.