Bug 99168 – Add tests for suggestion picker that test step/min/max attributes

• Status: RESOLVED
• Resolution: FIXED
• Component: Forms

[Keishi Hattori]

Add tests for suggestion picker that test step/min/max attributes

[Keishi Hattori]

Created attachment 168406 [details]
Patch

[Build Bot]

Comment on attachment 168406 [details]
Patch

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

New failing tests:
inspector/profiler/heap-snapshot.html

[Kent Tamura]

Comment on attachment 168406 [details]
Patch

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

> LayoutTests/fast/forms/resources/suggestion-picker-common.js:28
> +    for (var i = 0; i < elements.length; ++i) {
> +        values.push(valueForEntry(elements[i]));
> +    }

nit: { and } are not needed

> LayoutTests/platform/chromium-android/TestExpectations:92
> +platform/chromium/fast/forms/date/date-suggestion-picker-min-max-attribute.html [ WontFix ]
> +platform/chromium/fast/forms/date/date-suggestion-picker-step-attribute.html [ WontFix ]
> +platform/chromium/fast/forms/time/time-suggestion-picker-min-max-attribute.html [ WontFix ]
> +platform/chromium/fast/forms/time/time-suggestion-picker-step-attribute.html [ WontFix ]

We had better put *-suggestion-picker-* tests into their own directory.  platform/chromium/fast/forms/suggestion-picker/ ?

> LayoutTests/platform/chromium/fast/forms/date/date-suggestion-picker-min-max-attribute.html:26
> +description("Tests that key bindings work as expected.");
> +
> +debug('Check that page popup doesn\'t exist at first.');

Usage of ' and " is inconsistent.

> LayoutTests/platform/chromium/fast/forms/date/date-suggestion-picker-min-max-attribute.html:31
> +    openPicker(document.getElementById('date'));
> +    popupWindow.addEventListener("resize", test, false);

nit:
We had better introduce openPicker() with a callback argument. like:

function openPicker(element, callback) {
 ....
  popupOpwnCallback = callback;
  popupWindow.addEventListener("resize", pickerCallbackWrapper, false);
}

function pickerCallbackWrapper() {
    popupWindow.removeEventListener("resize", pickerCallbackWrapper);
    popupOpenCallback();
}

> LayoutTests/platform/chromium/fast/forms/date/date-suggestion-picker-min-max-attribute.html:37
> +var timeoutTimer = setTimeout(function() {
> +    testFailed("Test timed out.");
> +    finishJSTest();
> +}, 10000);

Do not add time out detection in tests.  It will be a source of flakiness.

> LayoutTests/platform/chromium/fast/forms/date/date-suggestion-picker-step-attribute.html:39
> +var timeoutTimer = setTimeout(function() {
> +    testFailed("Test timed out.");
> +    finishJSTest();
> +}, 10000);

ditto.

> LayoutTests/platform/chromium/fast/forms/time/time-suggestion-picker-min-max-attribute.html:34
> +var timeoutTimer = setTimeout(function() {
> +    testFailed("Test timed out.");
> +    finishJSTest();
> +}, 10000);

ditto.

> LayoutTests/platform/chromium/fast/forms/time/time-suggestion-picker-step-attribute.html:36
> +var timeoutTimer = setTimeout(function() {
> +    testFailed("Test timed out.");
> +    finishJSTest();
> +}, 10000);

ditto.

[Keishi Hattori]

Created attachment 168610 [details]
Patch

[Kent Tamura]

Comment on attachment 168610 [details]
Patch

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

> LayoutTests/ChangeLog:9
> +        I am also moving the suggestion picker tests to platform/chromium/fast/forms/suggestion-picker directory.

Please split the patch.

[Keishi Hattori]

Created attachment 168618 [details]
Patch

[WebKit Review Bot]

Attachment 168618 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast..." exit_code: 1
LayoutTests/platform/chromium-android/TestExpectations:72:  Test lacks BUG modifier.  [test/expectations] [5]
Total errors found: 1 in 11 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Kent Tamura]

Comment on attachment 168618 [details]
Patch

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

> LayoutTests/platform/chromium/fast/forms/suggestion-picker/date-suggestion-picker-min-max-attribute.html:35
> +    popupWindow.removeEventListener("resize", test, false);

removeEventListener is unnecessary.

> LayoutTests/platform/chromium/fast/forms/suggestion-picker/date-suggestion-picker-step-attribute.html:33
> +    popupWindow.addEventListener("resize", test1, false);

addEventListener is unnecessary.

> LayoutTests/platform/chromium/fast/forms/suggestion-picker/time-suggestion-picker-min-max-attribute.html:32
> +    popupWindow.removeEventListener("resize", test, false);

removeEventListener is unnecessary.

[Keishi Hattori]

Created attachment 168665 [details]
Patch

[Kent Tamura]

Comment on attachment 168665 [details]
Patch

ok

[WebKit Review Bot]

Attachment 168665 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast..." exit_code: 1
LayoutTests/platform/chromium-android/TestExpectations:72:  Test lacks BUG modifier.  [test/expectations] [5]
Total errors found: 1 in 11 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Keishi Hattori]

Created attachment 168837 [details]
Patch

[WebKit Review Bot]

Comment on attachment 168837 [details]
Patch

Rejecting attachment 168837 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2

Last 500 characters of output:
/mnt/git/webkit-commit-queue/Source/WebKit/chromium'
51>At revision 154708.

________ running '/usr/bin/python tools/clang/scripts/update.py --mac-only' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'

________ running '/usr/bin/python gyp_webkit' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'
Updating webkit projects from gyp files...
LayoutTests/platform/chromium-android/TestExpectations:72:  Test lacks BUG modifier.  [test/expectations] [5]
Total errors found: 1 in 1 files

Full output: http://queues.webkit.org/results/14297834

[Keishi Hattori]

Comment on attachment 168837 [details]
Patch

Clearing flags on attachment: 168837

Committed r131404: <http://trac.webkit.org/changeset/131404>

[Keishi Hattori]

All reviewed patches have been landed.  Closing bug.