Bug 98866 - Add a test to check a step attribute works for a calendar picker
Summary: Add a test to check a step attribute works for a calendar picker
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Kent Tamura
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-10-09 23:37 PDT by Kent Tamura
Modified: 2012-10-10 00:41 PDT (History)
2 users (show)

See Also:

Attachments
Patch (30.93 KB, patch)
2012-10-09 23:40 PDT, Kent Tamura 		no flags Details | Formatted Diff | Diff
Patch 2 (30.94 KB, patch)
2012-10-09 23:55 PDT, Kent Tamura 		yutak: review+
yutak: commit-queue-
Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Kent Tamura 2012-10-09 23:37:06 PDT 
Add a test to check a step attribute works for a calendar picker
Comment 1 Kent Tamura 2012-10-09 23:40:35 PDT 
Created attachment 167934 [details]
Patch
Comment 2 WebKit Review Bot 2012-10-09 23:44:18 PDT 
Attachment 167934 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/plat..." exit_code: 1
LayoutTests/platform/chromium/TestExpectations:923:  Did not find a test name.  [test/expectations] [5]
LayoutTests/platform/chromium/TestExpectations:924:  Did not find a test name.  [test/expectations] [5]
LayoutTests/platform/chromium/TestExpectations:925:  Did not find a test name.  [test/expectations] [5]
Total errors found: 3 in 6 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Kent Tamura 2012-10-09 23:55:01 PDT 
Created attachment 167939 [details]
Patch 2

Fix a typo.  Style errors are expected.
Comment 4 WebKit Review Bot 2012-10-09 23:58:49 PDT 
Attachment 167939 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/plat..." exit_code: 1
LayoutTests/platform/chromium/TestExpectations:923:  Did not find a test name.  [test/expectations] [5]
LayoutTests/platform/chromium/TestExpectations:924:  Did not find a test name.  [test/expectations] [5]
LayoutTests/platform/chromium/TestExpectations:925:  Did not find a test name.  [test/expectations] [5]
Total errors found: 3 in 6 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 5 Yuta Kitamura 2012-10-10 00:05:24 PDT 
Comment on attachment 167939 [details]
Patch 2

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

Just have one comment. Looks good except for it. Please fix the issue before landing.

> LayoutTests/platform/chromium/fast/forms/calendar-picker/calendar-picker-appearance-step.html:9
> +    setTimeout(function() { testRunner.notifyDone(); }, 0);

I think this line can be simplified to:
    setTimeout(testRunner.notifyDone, 0);
Comment 6 Kent Tamura 2012-10-10 00:31:39 PDT 
Comment on attachment 167939 [details]
Patch 2

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

>> LayoutTests/platform/chromium/fast/forms/calendar-picker/calendar-picker-appearance-step.html:9
>> +    setTimeout(function() { testRunner.notifyDone(); }, 0);
> 
> I think this line can be simplified to:
>     setTimeout(testRunner.notifyDone, 0);

Unfortunately it doesn't work. Probably because testRunner is an NPAPI plugin?
Comment 7 Kent Tamura 2012-10-10 00:33:49 PDT 
Committed r130868: <http://trac.webkit.org/changeset/130868>
Comment 8 Yuta Kitamura 2012-10-10 00:41:34 PDT 
Comment on attachment 167939 [details]
Patch 2

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

>>> LayoutTests/platform/chromium/fast/forms/calendar-picker/calendar-picker-appearance-step.html:9
>>> +    setTimeout(function() { testRunner.notifyDone(); }, 0);
>> 
>> I think this line can be simplified to:
>>     setTimeout(testRunner.notifyDone, 0);
> 
> Unfortunately it doesn't work. Probably because testRunner is an NPAPI plugin?

That's interesting.