[check-webkit-style] False positive for tst_QWebFrame

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Adam Barth]

https://bugs.webkit.org/show_bug.cgi?id=29008#c4

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]

I think tst_QWebFrame is a fine name in this test system.

[Adam Barth]

Created attachment 44693 [details]
Patch

[WebKit Review Bot]

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

[Eric Seidel (no email)]

Comment on attachment 44693 [details]
Patch

It wasn't initially clear to me from the ChangeLog or the change that this was related to function names.  Perhaps the changelog text could be improved.

You might also note that tst_ is a required name for the Qt unit testing system (I assume it's required?) because if it's optional it's not a very good name. ;)

[Benjamin Poulain]

Thanks for the patch!

(In reply to comment #3)
> You might also note that tst_ is a required name for the Qt unit testing system
> (I assume it's required?) because if it's optional it's not a very good name.
> ;)

Yep, tst_ is mandatory for the files containing the test suite. The folder of a particular test can also contain utility files with classes/mockup used by the test suite.

[David Levin]

Consider using "startswith" instead of find() == 0.

[Adam Barth]

Created attachment 44702 [details]
Updated with reviewer comments

[Adam Barth]

(In reply to comment #5)
> Consider using "startswith" instead of find() == 0.

Thanks!  I don't actually know python, so suggestions like this are much appreciated.

[WebKit Commit Bot]

Comment on attachment 44702 [details]
Updated with reviewer comments

Clearing flags on attachment: 44702

Committed r52016: <http://trac.webkit.org/changeset/52016>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.