Skip to content
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

Replace general source type with none source type #14081

Merged

Conversation

NavidZ
Copy link
Member

@NavidZ NavidZ commented Nov 15, 2018

Based on the web driver spec the general source type
string should be "none".
https://w3c.github.io/webdriver/#input-source-state

Based on the web driver spec the general source type
string should be "none".
https://w3c.github.io/webdriver/#input-source-state
@NavidZ
Copy link
Member Author

NavidZ commented Nov 15, 2018

FYI @Summerlw

@jgraham I believe we need to use string literal "none" for general actions as per spec.

@NavidZ NavidZ requested a review from foolip November 15, 2018 19:28
@jgraham
Copy link
Contributor

jgraham commented Nov 19, 2018

Please add a test e.g.

Author: James Graham <[email protected]>
Date:   Mon Nov 19 16:31:36 2018 +0000

    Add a test for the pause action

diff --git a/infrastructure/testdriver/actions/pause.html b/infrastructure/testdriver/actions/pause.html
new file mode 100644
index 0000000000..62a631130b
--- /dev/null
+++ b/infrastructure/testdriver/actions/pause.html
@@ -0,0 +1,18 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<title>TestDriver actions: event order</title>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script src="/resources/testdriver.js"></script>
+<script src="/resources/testdriver-actions.js"></script>
+<script src="/resources/testdriver-vendor.js"></script>
+
+<script>
+async_test(t => {
+  let t0 = performance.now();
+  let actions = new test_driver.Actions()
+    .addTick(2000)
+    .send()
+    .then(t.step_func_done(() => assert_greater_than(performance.now() - t0, 2000)));
+})
+</script>

Copy link
Contributor

@jgraham jgraham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a test.

let actions = new test_driver.Actions()
.addTick(2000)
.send()
.then(t.step_func_done(() => assert_greater_than(performance.now() - t0, 2000)));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test will time out instead of fail if the promise is rejected. Could you add a rejection handler or refactor it into a promise_test?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done.

@@ -0,0 +1,18 @@
<!DOCTYPE html>
<meta charset="utf-8">
<title>TestDriver actions: event order</title>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mind updating the title?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure. I will also need to skip the test for Chrome as we don't support it yet in ChromeDriver.

@foolip
Copy link
Member

foolip commented Nov 21, 2018

@jugglinmike are you happy with this PR now with the recent changes?

Copy link
Contributor

@jugglinmike jugglinmike left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am, indeed

@jugglinmike jugglinmike merged commit c6d4a3c into web-platform-tests:master Nov 21, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants