Bug 75191 - Add unittests for the C++ parser of prepare-ChangeLog
Summary: Add unittests for the C++ parser of prepare-ChangeLog
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: Kentaro Hara
URL:
Keywords:
Depends on:
Blocks: 73531
  Show dependency treegraph
 
Reported: 2011-12-24 09:30 PST by Kentaro Hara
Modified: 2011-12-25 00:25 PST (History)
4 users (show)

See Also:


Attachments
Patch (10.04 KB, patch)
2011-12-24 09:42 PST, Kentaro Hara
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kentaro Hara 2011-12-24 09:30:46 PST
We should add unittests for the C++ parser (i.e. get_function_line_ranges_for_cpp()) of prepare-ChangeLog.
Comment 1 Kentaro Hara 2011-12-24 09:42:16 PST
Created attachment 120495 [details]
Patch
Comment 2 WebKit Review Bot 2011-12-24 09:43:38 PST
Attachment 120495 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Tools/ChangeLog', u'Tools/Scripts/prepare-..." exit_code: 1

Tools/Scripts/webkitperl/prepare-ChangeLog_unittest/resources/cpp_unittests.cpp:29:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Tools/Scripts/webkitperl/prepare-ChangeLog_unittest/resources/cpp_unittests.cpp:73:  Declaration has space between type name and * in char *str  [whitespace/declaration] [3]
Tools/Scripts/webkitperl/prepare-ChangeLog_unittest/resources/cpp_unittests.cpp:82:  Declaration has space between type name and * in char *str  [whitespace/declaration] [3]
Tools/Scripts/webkitperl/prepare-ChangeLog_unittest/resources/cpp_unittests.cpp:160:  This { should be at the end of the previous line  [whitespace/braces] [4]
Tools/Scripts/webkitperl/prepare-ChangeLog_unittest/resources/cpp_unittests.cpp:170:  This { should be at the end of the previous line  [whitespace/braces] [4]
Tools/Scripts/webkitperl/prepare-ChangeLog_unittest/resources/cpp_unittests.cpp:177:  This { should be at the end of the previous line  [whitespace/braces] [4]
Tools/Scripts/webkitperl/prepare-ChangeLog_unittest/resources/cpp_unittests.cpp:184:  This { should be at the end of the previous line  [whitespace/braces] [4]
Tools/Scripts/webkitperl/prepare-ChangeLog_unittest/resources/cpp_unittests.cpp:210:  This { should be at the end of the previous line  [whitespace/braces] [4]
Tools/Scripts/webkitperl/prepare-ChangeLog_unittest/resources/cpp_unittests.cpp:220:  This { should be at the end of the previous line  [whitespace/braces] [4]
Total errors found: 9 in 5 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Ryosuke Niwa 2011-12-24 12:25:21 PST
Comment on attachment 120495 [details]
Patch

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

>> Tools/Scripts/webkitperl/prepare-ChangeLog_unittest/resources/cpp_unittests.cpp:73
>> +char *str = "abcde"
> 
> Declaration has space between type name and * in char *str  [whitespace/declaration] [3]

Is wrong style for pointer intentional? But we aren't really testing string literal parsing, right? So it seems like it's better to follow WebKit style here.

>> Tools/Scripts/webkitperl/prepare-ChangeLog_unittest/resources/cpp_unittests.cpp:160
>> +{
> 
> This { should be at the end of the previous line  [whitespace/braces] [4]

One of these classes should follow WebKit style and put { on the same line as Class1.
Comment 4 Kentaro Hara 2011-12-25 00:24:26 PST
Committed r103669: <http://trac.webkit.org/changeset/103669>
Comment 5 Kentaro Hara 2011-12-25 00:25:28 PST
Comment on attachment 120495 [details]
Patch

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

>>> Tools/Scripts/webkitperl/prepare-ChangeLog_unittest/resources/cpp_unittests.cpp:160
>>> +{
>> 
>> This { should be at the end of the previous line  [whitespace/braces] [4]
> 
> One of these classes should follow WebKit style and put { on the same line as Class1.

Done. Thanks.