567870 - add waitForClipboard

Status: RESOLVED FIXED

(Testing :: Mochitest, defect)

Description

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

The clipboard can by async (pretty sure it's just linux), and it's a pain to write a polling mechanism for each test that needs it. So we should add one to the framework.

I'm thinking something along the lines of
> waitForClipboard(expectedVal, setupCallback, failCallback, successCallback)
expectedVal = the expected value
setupCallback = callback that would put the expectedVal on the CB
failCallback = callback for when we've hit polling limit & expectedVal isn't on CB
successCallback = callback for when we get the right value

I'm proposing using a setup callback because we really should poll twice, once to put a known value different than expectedVal on the CB, and then for the expected val. That way we ensure that we're not just getting the expectedVal because it was already on the clipboard.

Not sure how flexible we should make this. I'm thinking just text/unicode and kGlobalClipboard since that's the common case.

Comment 1

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

Attachment: Patch v0.1

wrote it & converted my most recent clipboard polling test to use it.

Comment 2

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 447240 [details] [diff] [review]
Patch v0.1

>diff --git a/testing/mochitest/tests/SimpleTest/SimpleTest.js b/testing/mochitest/tests/SimpleTest/SimpleTest.js

>+ * Polls the clipboard waiting for the expected value. A known value different than
>+ * the expected value is put on the clipboard first (and also polled for) so we
>+ * can be sure the value we get isn't just the expected value because it was already
>+ * on the clipboard.

Should probably mention that it only checks the global clipboard, and only text/unicode values.

>+ * @param aSetupFn
>+ *        A function that will be called in order to put the expected value on
>+ *        the clipboard

How about:
"A function responsible for setting the clipboard to the expected value, called after the known value setting succeeds."

>+ * @param aSuccessFn
>+ *        A function that will be called when we successfully get the value on
>+ *        the clipboard

how about "A function called when the expected value is found on the clipboard."?

>+SimpleTest.waitForClipboard = function(aExpectedVal, aSetupFn, aSuccessFn, aFailureFn) {

>+    function wait(expectedVal, successFn, failureFn) {
>+        if (++SimpleTest.waitForClipboard_polls > 50) {
>+            // Log the failure.
>+            SimpleTest.ok(false, "Timed out while polling clipboard for pasted data");
>+            reset();
>+            failureFn();

Seems like it could be valuable to pass the expected string to the callback so that it can differentiate the two failure cases (waiting for the first set vs. second set). At the very least, you should append expectedVal to the "timed out" message.

>+    // First we wait for a known value != aExpectedVal
>+    let preExpectedVal = aExpectedVal + "awesome";

... and use a more unique/explanatory suffix here, like "-waitForClipboard-known-string" or somesuch.

Have you tested that this is actually usable from regular mochitest? (I worry that the enablePrivilege() doesn't end up making it into the wait() given the setTimeout.)

r=me with those considered.

Comment 3

[:Gavin Sharp [email: [e-mail]]]

Should also make the tests mentioned in bug 556061 comment 5 use this (in a followup).

Comment 4

[:Gavin Sharp [email: [e-mail]]]

browser_passwordmgrcopypwd.js, too (bug 566910).

Comment 5

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

Attachment: Patch v0.2 (for check-in)

Now with better comments, and it actually works for mochitests (privs needed to be added, and had to s/let/var/ & expand Cc/Ci since SimpleTest.js gets included in content). Also fixed a pretty big problem where if it didn't have the right value on first poll, it would never get the right value (because wait wasn't called with arguments in setTimeout... oops!).

Comment 6

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

Pushed http://hg.mozilla.org/mozilla-central/rev/5e92d62650cf