Bug 181841 - Use promises for basic-gestures.js and 'await' for the corresponding tests
Summary: Use promises for basic-gestures.js and 'await' for the corresponding tests
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Frédéric Wang (:fredw)
URL:
Keywords: InRadar
Depends on: 181798
Blocks: 181681 173833
  Show dependency treegraph
 
Reported: 2018-01-19 00:55 PST by Frédéric Wang (:fredw)
Modified: 2018-01-24 01:46 PST (History)
5 users (show)

See Also:


Attachments
Patch (55.18 KB, patch)
2018-01-19 01:01 PST, Frédéric Wang (:fredw)
megan_gardner: review+
ews-watchlist: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews101 for mac-sierra (2.24 MB, application/zip)
2018-01-19 02:02 PST, EWS Watchlist
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Frédéric Wang (:fredw) 2018-01-19 00:55:52 PST
Use promises for basic-gestures.js and 'await' for the corresponding tests
Comment 1 Frédéric Wang (:fredw) 2018-01-19 01:01:50 PST
Created attachment 331715 [details]
Patch
Comment 2 EWS Watchlist 2018-01-19 02:02:30 PST
Comment on attachment 331715 [details]
Patch

Attachment 331715 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/6132466

New failing tests:
media/modern-media-controls/media-controller/media-controller-fade-controls-when-entering-fullscreen.html
Comment 3 EWS Watchlist 2018-01-19 02:02:31 PST
Created attachment 331717 [details]
Archive of layout-test-results from ews101 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-sierra  Platform: Mac OS X 10.12.6
Comment 4 Frédéric Wang (:fredw) 2018-01-19 02:07:31 PST
The failure on mac seems unrelated as it happened without the patch: https://webkit-queues.webkit.org/patch/331715/mac-ews
Comment 5 Megan Gardner 2018-01-19 08:30:09 PST
Comment on attachment 331715 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=331715&action=review

Looks good other than the few formatting things, and the seemingly unneeded 'done' return.

> LayoutTests/fast/events/touch/ios/long-press-then-drag-left-to-change-selected-text.html:69
> +                output += 'FAIL: failed to reduce selection to a single character by dragging right. Incorrect Selection: ' +     document.getSelection().toString();

extraneous spaces after +
Looks like they were there in the original, but should still be removed.

> LayoutTests/fast/events/touch/ios/long-press-then-drag-to-select-text.html:37
> +                output += 'FAIL: failed to reduce selection to a single character by dragging down. Incorrect Selection: ' +     document.getSelection().toString();

extraneous spaces after +

> LayoutTests/fast/events/touch/ios/long-press-then-drag-up-to-change-selected-text.html:69
> +                output += 'FAIL: failed to reduce selection to a single character by dragging down. Incorrect Selection: ' +     document.getSelection().toString();

Extraneous spaces after +

> LayoutTests/resources/basic-gestures.js:7
> +                    uiController.uiScriptComplete('Done');

Is there a reason that this function now returns 'Done'?
Comment 6 Frédéric Wang (:fredw) 2018-01-19 08:35:25 PST
Comment on attachment 331715 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=331715&action=review

>> LayoutTests/fast/events/touch/ios/long-press-then-drag-left-to-change-selected-text.html:69
>> +                output += 'FAIL: failed to reduce selection to a single character by dragging right. Incorrect Selection: ' +     document.getSelection().toString();
> 
> extraneous spaces after +
> Looks like they were there in the original, but should still be removed.

Right, I'll fix these.

>> LayoutTests/resources/basic-gestures.js:7
>> +                    uiController.uiScriptComplete('Done');
> 
> Is there a reason that this function now returns 'Done'?

mmh, no. I think I initially copied it from ui-helper.js but that should not affect the tests here.
Comment 7 Wenson Hsieh 2018-01-19 09:01:29 PST
Comment on attachment 331715 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=331715&action=review

r=me

> LayoutTests/resources/basic-gestures.js:232
> +                 uiController.uiScriptComplete();

Nit - not a new issue, but the indentation level is off on all of these:

```
uiController.sendEventStream(JSON.stringify(eventStream), function() {});
    uiController.uiScriptComplete();
})();`, resolve);
```
Comment 8 Frédéric Wang (:fredw) 2018-01-19 09:07:09 PST
Just for future reference:  Megan Gardner was actually not a reviewer when I landed this patch. webkit-patch displayed a warning but I could not abort it in time. So the commit is

https://trac.webkit.org/changeset/227201/webkit

and "reviewed by Wenson Hsieh".

(In reply to Wenson Hsieh from comment #7)
> Nit - not a new issue, but the indentation level is off on all of these:
> 
> ```
> uiController.sendEventStream(JSON.stringify(eventStream), function() {});
>     uiController.uiScriptComplete();
> })();`, resolve);
> ```

Right, but the patch already landed so I prefer not to send another one for whitespace-only change.
Comment 9 Radar WebKit Bug Importer 2018-01-19 09:09:13 PST
<rdar://problem/36660038>