Bug 58509 – Add a sample test case for GTest framework

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

[Dmitry Lomov]

Add a sample test case and a runner for WebKit and GTest framework

[Dmitry Lomov]

Created attachment 89523 [details]
Example test case for GTest in JavaScriptCore

[David Levin]

Comment on attachment 89523 [details]
Example test case for GTest in JavaScriptCore

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

Just a few suggestions.

> Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj:3435
> +			buildSettings = {

Let's try to trim this down.

> Source/JavaScriptCore/wtf/tests/RunAllWtfTests.cpp:35
> +int main(int argc, char **argv) {

The { needs to be on the next line.

> Source/JavaScriptCore/wtf/tests/RunAllWtfTests.cpp:36
> +    std::cout << "Running main() from gtest_main.cc\n";

Avoid stream io for consistency for the rest of WebKit.

> Source/JavaScriptCore/wtf/tests/StringTests.cpp:41
> +TEST_F(CStringTest, TestLength) {

{ on next line.

> Source/ThirdParty/gtest/xcode/gtest.xcodeproj/project.pbxproj:581
> +			developmentRegion = English;

Remove this to keep us the same as upstream.

[Dmitry Lomov]

Created attachment 89654 [details]
Example test case for GTest in JavaScriptCore

Code review feedback addressed

[David Levin]

Comment on attachment 89654 [details]
Example test case for GTest in JavaScriptCore

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

Your change is inverted. Other than that and the minor issue below. It looks fine.

> ChangeLog:194
> [phone]  Adam Barth  <[e-mail]>

Get rid of this change.

[Dmitry Lomov]

Created attachment 89660 [details]
Example test case for GTest in JavaScriptCore

Reversed the diff direction + removed typo in ChangeLog - sorry about that!

[WebKit Review Bot]

Attachment 89660 [details] did not pass style-queue:

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

Source/JavaScriptCore/wtf/tests/RunAllWtfTests.cpp:31:  Found other header before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
Source/JavaScriptCore/wtf/tests/RunAllWtfTests.cpp:33:  Alphabetical sorting problem.  [build/include_order] [4]
Source/JavaScriptCore/wtf/tests/StringTests.cpp:32:  Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
Total errors found: 3 in 6 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Dmitry Lomov]

Created attachment 89664 [details]
Example test case for GTest in JavaScriptCore

Try n+1 - addressing style issues (check-webkit-style still complains about absence of a primary headers)

[David Levin]

Yea!

[WebKit Review Bot]

Attachment 89664 [details] did not pass style-queue:

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

Source/JavaScriptCore/wtf/tests/RunAllWtfTests.cpp:32:  Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
Source/JavaScriptCore/wtf/tests/StringTests.cpp:33:  Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
Total errors found: 2 in 4 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[WebKit Commit Bot]

The commit-queue encountered the following flaky tests while processing attachment 89664 [details]:

http/tests/xmlhttprequest/cross-origin-no-authorization.html bug 33357 (author: [e-mail])
The commit-queue is continuing to process your patch.

[WebKit Commit Bot]

Comment on attachment 89664 [details]
Example test case for GTest in JavaScriptCore

Clearing flags on attachment: 89664

Committed r83974: <http://trac.webkit.org/changeset/83974>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Timothy Hatcher]

What is this and why do we need it?

This GTest stuff causes an error on systems that don't have $(DEVELOPER_SDK_DIR)/MacOSX10.4u.sdk. It should not have an SDK and use the current system.

[Eric Seidel (no email)]

The desire for WebCore unit testing goes waay waay back.  See bug 21010.  That said, I know nothing about this current effort.

[Timothy Hatcher]

Filed bug 58679