Bug 29008

Summary: Add a test in Qt for https://bugs.webkit.org/show_bug.cgi?id=29005
Product: WebKit Reporter: Benjamin Poulain <benjamin>
Component: Tools / TestsAssignee: Benjamin Poulain <benjamin>
Status: RESOLVED FIXED    
Severity: Enhancement CC: abarth, commit-queue, hausmann, webkit.review.bot
Priority: P3 Keywords: Qt
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on: 29005    
Bug Blocks:    
Attachments:
Description Flags
patch suggestion
hausmann: review-
Patch none

Description Benjamin Poulain 2009-09-07 04:33:44 PDT
The patch of https://bugs.webkit.org/show_bug.cgi?id=29005 changes the behavior of the runtime arrays generated by Qt. It would be nice to have a test for it.

This is related to the task 214611 of Qt: http://qt.nokia.com/developer/task-tracker/index_html?method=entry&id=214611
Comment 1 Benjamin Poulain 2009-09-07 04:36:16 PDT
Created attachment 39142 [details]
patch suggestion
Comment 2 Simon Hausmann 2009-11-23 05:58:30 PST
(In reply to comment #1)
> Created an attachment (id=39142) [details]
> patch suggestion

Did you forget to mark the attachment up for review? :)
Comment 3 Benjamin Poulain 2009-11-23 06:05:50 PST
(In reply to comment #2)
> Did you forget to mark the attachment up for review? :)

No, I want to fix 29005 first. :)
Comment 4 WebKit Review Bot 2009-12-11 07:50:41 PST
Attachment 39142 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
WebKit/qt/tests/qwebframe/tst_qwebframe.cpp:2075:  This { should be at the end of the previous line  [whitespace/braces] [4]
WebKit/qt/tests/qwebframe/tst_qwebframe.cpp:2084:  tst_QWebFrame::arrayObjectEnumerable is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebKit/qt/tests/qwebframe/tst_qwebframe.cpp:2091:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
WebKit/qt/tests/qwebframe/tst_qwebframe.cpp:2096:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
WebKit/qt/tests/qwebframe/tst_qwebframe.cpp:2096:  More than one command on the same line  [whitespace/newline] [4]
Total errors found: 5
Comment 5 Adam Barth 2009-12-11 09:17:57 PST
> WebKit/qt/tests/qwebframe/tst_qwebframe.cpp:2084: 
> tst_QWebFrame::arrayObjectEnumerable is incorrectly named. Don't use
> underscores in your identifier names.  [readability/naming] [4]

False positive filed:

https://bugs.webkit.org/show_bug.cgi?id=32436
Comment 6 Simon Hausmann 2009-12-12 16:09:33 PST
Comment on attachment 39142 [details]
patch suggestion


> +class StringListTestObject : public QObject
> +{

The coding style says that this brace should be placed on the preceeding line.

Otherwise the patch looks good! (sorry about the style nitpick ;(
Comment 7 Benjamin Poulain 2009-12-13 07:28:02 PST
Created attachment 44756 [details]
Patch

> The coding style says that this brace should be placed on the preceeding line.
> 
> Otherwise the patch looks good! (sorry about the style nitpick ;(

Thanks for the review...on Saturday! :)

This is the same patch with the curly bracket on the same line as the class name. Only 8 class definitions of the autotests follow this style, out of 34 classes. I'll have a look at that as part of 32216.
Comment 8 WebKit Review Bot 2009-12-13 07:30:34 PST
style-queue ran check-webkit-style on attachment 44756 [details] without any errors.
Comment 9 Simon Hausmann 2009-12-13 14:21:47 PST
Comment on attachment 44756 [details]
Patch

r=me, thanks!
Comment 10 WebKit Commit Bot 2009-12-13 14:35:02 PST
Comment on attachment 44756 [details]
Patch

Clearing flags on attachment: 44756

Committed r52069: <http://trac.webkit.org/changeset/52069>
Comment 11 WebKit Commit Bot 2009-12-13 14:35:10 PST
All reviewed patches have been landed.  Closing bug.