[testdriver] Enable manual interaction #11173
Conversation
jugglinmike
force-pushed
the
testdriver-manual
branch
from
42bb097 to
43bcede
Compare
|
Given the definition of a user initiated event, we don't actually need to rely on the user carrying out the specific action (at least for what we currently have); we should just be able to throw up a button and tell the user to press that and then dispatch synthetic (but user trusted) events from that for what we need. Note however: whatwg/html#1903 and https://bugs.chromium.org/p/chromium/issues/detail?id=404161. |
|
(Basically: that doesn't actually quite work currently, but I think it should, and that would be much easier from a UX POV, given we don't need the user to directly trigger the right sequence of events.) |
|
Call me a nerd, but that discussion is extremely interesting. That said, my reading is that the design is heading in a direction that would not support a user interface like "please click this one button, and we'll take care of the rest." It sounds like we're headed towards a consumption-based model, and that would require a 1-to-1 correspondence between user input and protected API call. |
|
What use cases are there for manual interaction with testdriver APIs other than debugging (which is an important one). |
Beyond test debugging, this patch allows non-developers to run the tests (e.g. via http://w3c-test.org)
Agreed |
|
@jugglinmike then I misunderstood and we should add something more like this :) |
|
There are no owners for this pull request. Please reach out on W3C's irc server (irc.w3.org, port 6665) on channel #testing (web client) to get help with this. Thank you! |
|
One thing that would probably good would be draw attention to the element in question, probably by adding a big |
jugglinmike
force-pushed
the
testdriver-manual
branch
from
43bcede to
f1ae89f
Compare
|
Works for me, @gsnedders. As noted in the comment, I'm recommending we add that decoration even under automation in order to avoid any strange edge cases where the presence of an outline influences the test. This also needed rebasing to accommodate a recent addition to the testharness.js library. The original version of the branch is available here: |
The testdriver.js utility allows tests which require user input to be fully automated. It requires that the browser be under the control of a WebDriver server in order to do this. In test maintenance contexts, this requirement can slow development because a new browsing session must be created for each trial, and debugging tools such as the developer's console cannot be used. This requirement also limits the tests' value to a larger audience when published online. Visitors to sites like w3c-test.org are interested in observing test results experimentally, but although they may be capable of providing the expected input, the hard requirement on WebDriver will cause tests to fail unconditionally. Extend the framework to accept traditional human input if the test is being run without the WebDriver technology. This allows test authors to debug tests and other interested parties to verify results in browsers of their choosing. It does not interfere with fully-automated test execution.
jugglinmike
force-pushed
the
testdriver-manual
branch
from
f1ae89f to
ad92976
Compare
|
Rebased again to resolve conflicts from gh-11512 |
|
On the face of it, this looks good to me. However, it does mean when we introduce a new testdriver function that the manual codepath will run where previously the Promise rejected, and hence it will run to timeout. And timeouts are going to be a problem with relying on manual interaction with the tests, too. |
To support the w3c-test.org/local filesystem use cases, I think that the "fully diff --git a/resources/testdriver.js b/resources/testdriver.js
index 5651bf2..4c3943a 100644
--- a/resources/testdriver.js
+++ b/resources/testdriver.js
@@ -54,6 +54,12 @@
element.style.outline = 'dashed #0366d6 5px';
}
+ function inAutomation() {
+ return Object.keys(manual).some(function(name) {
+ return window.test_driver_internal[name] !== manual[name];
+ });
+ }
+
/**
* @namespace
*/
@@ -154,7 +160,7 @@
}
};
- window.test_driver_internal = {
+ var manual = {
/**
* Waits for a user-initiated click
*
@@ -163,7 +169,13 @@
* @returns {Promise} fulfilled after click occurs
*/
click: function(element, coords) {
- return new Promise(function(resolve) {
+ return new Promise(function(resolve, reject) {
+ if (!inAutomation()) {
+ reject(new Error('Not implemented'));
+
+ return;
+ }
+
element.addEventListener("click", resolve);
});
},
@@ -178,6 +190,12 @@
*/
send_keys: function(element, keys) {
return new Promise(function(resolve, reject) {
+ if (!inAutomation()) {
+ reject(new Error('Not implemented'));
+
+ return;
+ }
+
var seen = "";
function onKeyDown(event) {
@@ -208,4 +226,7 @@
return Promise.reject(new Error("unimplemented"));
}
};
+
+ window.test_driver_internal = Object.create(manual);
+
})();This is a little implicit, though. We could also require that implementers Do either of those strategies seem like a good idea to you?
That's true, but since that is a case where we know a human operator is |
|
@LukeZielinski would be good for you to review these changes too I think. |
This is in preparation for supporting manual interactions on testdriver. By setting this field to true, unimplemented APIs will fail early rather than waiting for user input (and timing out). web-platform-tests/wpt#11173 Change-Id: Ie409b683755358fb7978989bf3894e7b45b05239 Reviewed-on: https://chromium-review.googlesource.com/c/1352571 Commit-Queue: Philip Jägenstedt <[email protected]> Reviewed-by: Philip Jägenstedt <[email protected]> Cr-Commit-Position: refs/heads/master@{#611690}
|
I submitted this patch before we integrated Taskcluster into WPT. This means that Taskcluster ignores pushes to the branch, and that's a problem now that the Taskcluster check is required. I would normally rebase the branch, but since the patch has already been reviewed and approved, I've merged |
|
It looks like this is blocked on existing flakiness. @gsnedders how would you feel about admin-merging this? |
There was a problem hiding this comment.
@jugglinmike see @foolip's outstanding (non-resolved) comments
| @@ -137,27 +137,64 @@ | |||
| } | |||
| }; | |||
|
|
|||
| window.test_driver_internal = { | |||
| var manual = { | |||
Co-Authored-By: jugglinmike <[email protected]>
Co-Authored-By: jugglinmike <[email protected]>
Got it! |
jugglinmike
force-pushed
the
testdriver-manual
branch
from
6faba32 to
8f2bbec
Compare
|
This PR is blocked on the required "Travis CI - Pull Request" check after #14499. In order to trigger it, I will close and reopen this PR. |
|
I'll close and reopen to trigger new CI runs. |
|
Taskcluster failures. Chrome
Firefox
|
|
Changing testdriver.js affects tests that were perhaps already flaky, so that isn't very actionable. Filed web-platform-tests/wpt.fyi#1147 about a problem with the wpt.fyi checks. There is some regression risk here, but I'll merge and then post a comparison between full runs before/after the change. |
Primarily to support manual interaction of testdriver APIs: web-platform-tests/wpt#11173 Also roll idlharness.js by the way (webidl2.js hasn't changed). Change-Id: Ic30677459f9dce2384cffb82b6bf77a141c13495 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1474986 Commit-Queue: Robert Ma <[email protected]> Reviewed-by: Philip Jägenstedt <[email protected]> Cr-Commit-Position: refs/heads/master@{#643197}
The testdriver.js utility allows tests which require user input to be fully automated. It requires that the browser be under the control of a WebDriver server in order to do this. In test maintenance contexts, this requirement can slow development because a new browsing session must be created for each trial, and debugging tools such as the developer's console cannot be used. This requirement also limits the tests' value to a larger audience when published online. Visitors to sites like w3c-test.org are interested in observing test results experimentally, but although they may be capable of providing the expected input, the hard requirement on WebDriver will cause tests to fail unconditionally. Extend the framework to accept traditional human input if the test is being run without the WebDriver technology. This allows test authors to debug tests and other interested parties to verify results in browsers of their choosing. It does not interfere with fully-automated test execution.
The testdriver.js utility allows tests which require user input to be
fully automated. It requires that the browser be under the control of a
WebDriver server in order to do this.
In test maintenance contexts, this requirement can slow development
because a new browsing session must be created for each trial, and
debugging tools such as the developer's console cannot be used.
This requirement also limits the tests' value to a larger audience when
published online. Visitors to sites like w3c-test.org are interested in
observing test results experimentally, but although they may be capable
of providing the expected input, the hard requirement on WebDriver will
cause tests to fail unconditionally.
Extend the framework to accept traditional human input if the test is
being run without the WebDriver technology. This allows test authors to
debug tests and other interested parties to verify results in browsers
of their choosing. It does not interfere with fully-automated test
execution.
@gsnedders @kereliuk I'm interested in going even further with this. We could make testdriver.js-powered tests look a lot like today's manual tests by dynamically inserting instructions for human operators. That's less compelling for test authors (since they presumably know exactly what inputs are needed by their own tests), but it could make a big difference for visitors to http://w3c-test.org.
Of course, any change to the document has the potential to invalidate the test, so it would need to be configurable.
For now, I figured this would be a simple and safe way to make test author's lives a little easier.
I'm excited to start extending tests with testdriver.js. Thanks for making it possible!
This change is