Bug 29008 – Add a test in Qt for https://bugs.webkit.org/show_bug.cgi?id=29005

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Benjamin Poulain]

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

[Benjamin Poulain]

Created attachment 39142 [details]
patch suggestion

[Simon Hausmann]

(In reply to comment #1)
> Created an attachment (id=39142) [details]
> patch suggestion

Did you forget to mark the attachment up for review? :)

[Benjamin Poulain]

(In reply to comment #2)
> Did you forget to mark the attachment up for review? :)

No, I want to fix 29005 first. :)

[WebKit Review Bot]

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

[Adam Barth]

> 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

[Simon Hausmann]

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 ;(

[Benjamin Poulain]

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.

[WebKit Review Bot]

style-queue ran check-webkit-style on attachment 44756 [details] without any errors.

[Simon Hausmann]

Comment on attachment 44756 [details]
Patch

r=me, thanks!

[WebKit Commit Bot]

Comment on attachment 44756 [details]
Patch

Clearing flags on attachment: 44756

Committed r52069: <http://trac.webkit.org/changeset/52069>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.