Tests for a few proposals rely on files in a top-level directory named /gen/:
$ git grep -l '/gen/'
idle-detection/interceptor.https.html
shape-detection/resources/shapedetection-helpers.js
webxr/resources/webxr_util.js
Further references have recently been proposed for tests of the "sms" proposal.
This directory does not exist in WPT's source tree, and to the best of my knowledge, it is not generated by any of WPT's infrastructure.
@Hexcles mentioned that this is generated code in Chromium's infrastructure. In that case, I don't think this can be resolved by checking in the files (doing so would presumably interfere with Chromium's tests, and the files are unlikely to be useful for other implementations).
@Hexcles @LukeZielinski Can we formalize a contract which would allow others to use these tests? Do you think it could be stable enough to support implementation by other browsers?
I brought up the idle-detection issue in #15753.
These are all Mojo stuff. These tests won't work even on Chrome unless 1. generated Mojo JS files are available (this is non-trivial and I'll explain further); 2. a few test flags are turned on (see https://github.com/web-platform-tests/results-collection/pull/582).
Mojo JS stubs are generated from source code at compile time so technically we need the Mojo files generated from the specific revision of Chrome we're testing against, which means we need to publish & download a bundle of files instead of simply checking them into WPT.
More discussions can be found in https://github.com/web-platform-tests/results-collection/issues/81 from last year and cc @foolip .
That's helpful information for getting results from Chromium, but it doesn't seem like it will help others run these tests.
In the issue that Josh created for idle-detection, there's been some discussion about documenting a testing API. That's just the kind of contract I had in mind. It seems essential for the tests to be shared. @jdm would you agree?
I can't find instruction like that for idle-detection, shape-detection, webxr, or sms. @samuelgoto @reillyeon would you mind referencing those docs from the tests?
There's https://github.com/immersive-web/webxr-test-api/blob/master/explainer.md for WebXR.
(From https://chromium-review.googlesource.com/c/chromium/src/+/1070778/15#message-f47cb5752847daa0e0c550d4a033d9fc21103c74)
Are you looking for documentation on how to run these tests or documentation of the API that the tests expect to be provided by a browser implementation?
The latter--something along the lines of the document @Hexcles shared for WebXR
I feel like there is a larger concern here: Google (or anyone working in Chromium) adding tests that rely on internal infra and therefore cannot be run in other browsers. It may be even larger than that: adding tests that are not designed to be run in other browsers. I think this needs to be addressed at that level: given the 2-way sync between Chromium and WPT, I worry this may become a larger problem over time. Should these specific tests even be in the WPT repo?
If not, how do we prevent them from syncing in the future?
if they should be, how do we get them to run in other browsers?
@thejohnjansen the details matter a lot here. Every browser running WPT comes with a bit of implementation-specific infrastructure for automation. A well-written test abstracts over these differences in order to test a feature on all implementations. The reason why this issue was filed was because those tests weren't properly abstracted from the implementation-specific automation they require.
To get these tests to run in other browsers (aside from the browser implementing the API being tested) the engine needs to add its own version of necessary automation hooks.
Thanks for bringing this up again, @thejohnjensen.
given the 2-way sync between Chromium and WPT, I worry this may become a larger problem over time.
You're already correct. Today, there are more references than when this issue was reported:
$ git grep -l '/gen/'
contacts/resources/helpers.js
idle-detection/interceptor.https.html
shape-detection/resources/shapedetection-helpers.js
sms/sms_provider.js
web-nfc/resources/nfc-helpers.js
webxr/resources/webxr_util.js
It's been four months, so I think some action is necessary from WPT. gh-18509 is a patch that causes the project linter to flag these references. Removing the existing tests from WPT directly would likely regress coverage in Chromium, so the patch makes exceptions for those. Probably they should be relocated in the Chromium source tree (and hopefully reintroduced when documentation is available). The new linting rule will at least help us avoid further regression.
Following TPAC discussion today, I think we have a consensus here:
/gen/ stuff) into WPT (these are build artifacts that are specific to Chromium revisions). In order to make MojoJS work with public Chrome, we would need to figure out a way to archive and release MojoJS stubs per Chrome version. (I'm looking into this.)/gen/ URLs that only exist in Chromium infra, which is unfortunate. We should first at least make the fetches conditional and asynchronous so that they don't cause network errors in the upstream. Once 2 is resolved, we will work out a way to make this more transparent.Thanks for pushing for the right thing and taking some actions, @jugglinmike and sorry that I haven't done more -- in case it's not clear, I'd like to tackle this. Here are the concrete and parallel steps I'm taking:
/gen/ would need to be whitelisted in lint.whitelist, of which I'm an owner in Chromium and I'll be vigilant reviewing the changes.@Hexcles Thanks for looking in to this! All those changes sound really positive to me.
I believe the Chromedriver maintainers have recently been working to strengthen the relationship between releases of Chromium and Chromedriver. This is probably a stretch, but I wonder if you could reuse some part of their infrastructure for releasing MojoJS stubs.
@makotoshimazu pointed out a Gecko case that isn't using any internal APIs, but does seem a bit odd:
https://github.com/web-platform-tests/wpt/blob/1abcb7058ecdaabd5fe9c27c90a73795d31d2a0a/service-workers/service-worker/update-after-oneday.https.html#L2-L4
For this to change the time while the test is running, that pref would have to cause something to happen while the test is running, and yet I can't see a call to internals.
@jgraham do you have any insight into what this is, is there an implicit internal API hiding here?
The passive voice of a verb like "enable" makes this statement ambiguous. Another interpretation is that the person/system running the test should enable the preference prior to executing the test and that the preference should remain enabled for the duration of the execution. (Come to think of it, I don't know if "enabled" is a verb in that context.)
This alternative interpretation seems more likely in light of the questions @foolip has asked.
/cc @sideshowbarker because they like this sort of thing.
That is certainly the right interpretation of when to enable the pref, but what I wonder is what the pref does.
I thought that for it to work, the registration age has to be increased while the test is running, but what's the trigger? But.. perhaps it just causes any registration to immediately be considered 24 hours old. That would work and is probably what this does.
So I just noticed this. Yes, that sounds like a thing that ought to be documented as part of a testing API. Can we file a specific issue?
I think one thing is being missed when these things are being discussed: The tests _do not rely on missing infra_ here.
Browsers are expected to implement the WebXR Test API and enable it when running WPT. In Chrome's case, they've put their implementation in some autogenerated code, and for some reason references to that code are in WPT, but those codepaths are skipped for browsers which implement the Test API.
@Manishearth I don't think that's being missed; https://github.com/web-platform-tests/wpt/issues/16455#issuecomment-532040838 sets that out in detail, and has followup actions for @Hexcles to ensure that things continue to work as expected. I'm not sure if those actions are complete.
My comment at https://github.com/web-platform-tests/wpt/issues/16455#issuecomment-601646486 was specifically about the service-worker test identified in https://github.com/web-platform-tests/wpt/issues/16455#issuecomment-532507904 and not related to WebXR.
So I think this bug could either be closed or used to track the work @Hexcles is doing, depending on his preference.
@jgraham It seemed like this issue was misinterpreted in https://github.com/web-platform-tests/wpt/pull/22836 , and I recently had another ping that misunderstood the status quo, hence this comment 馃槃
Oh OK then. The current situation is suboptimal and confusing, just not actualy broken.
To keep folks updated: @Hexcles continues to work on making MojoJS work in upstream WPT. We believe we now have a workable solution that will involve publishing the necessary mojom files alongside Chromium builds, which WPT can then fetch and make available (similar to Chromedriver today). We expect to be MVP-ing this in Q3, with an eye to turning the MVP into the real thing if/when we get sign off :)
To keep folks updated: @Hexcles has completed the majority of the work to make MojoJS work in upstream WPT, and is currently in clean-up + follow-through mode. We expect to close this issue soon.
Alright, I'm happy to announce that we've officially finished the cleanup. Here's the full list of tests that use MojoJS:
All of them use the newly introduced https://github.com/web-platform-tests/wpt/blob/master/resources/test-only-api.js to load Chrome-specific resources, which are also now available (downloaded automatically) with upstream wpt run chrome (#24979 ), which means you can see the results on wpt.fyi. Furthermore, these tests will not fail on other browsers with an obscure network error/timeout (but with a more meaningful assertion failure), and other browsers can fill in their implementations in this new framework (currently the only example is WebXR).
Lastly, we've removed all *.mojom.js from the WPT tree and added a lint.
And huge thanks to our friends from Intel @arskama @rakuco for migrating many device-related tests!
Most helpful comment
And huge thanks to our friends from Intel @arskama @rakuco for migrating many device-related tests!