Summary: | Add a sample test case for GTest framework | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Dmitry Lomov <dslomov> | ||||||||||
Component: | Tools / Tests | Assignee: | Nobody <webkit-unassigned> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | commit-queue, dslomov, eric, levin, timothy, webkit.review.bot | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | PC | ||||||||||||
OS: | OS X 10.5 | ||||||||||||
Bug Depends on: | 58507 | ||||||||||||
Bug Blocks: | 21010 | ||||||||||||
Attachments: |
|
Description
Dmitry Lomov
2011-04-13 20:14:32 PDT
Created attachment 89523 [details]
Example test case for GTest in JavaScriptCore
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. Created attachment 89654 [details]
Example test case for GTest in JavaScriptCore
Code review feedback addressed
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 > +2011-03-28 Adam Barth <abarth@webkit.org> Get rid of this change. Created attachment 89660 [details]
Example test case for GTest in JavaScriptCore
Reversed the diff direction + removed typo in ChangeLog - sorry about that!
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.
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)
Yea! 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.
The commit-queue encountered the following flaky tests while processing attachment 89664 [details]: http/tests/xmlhttprequest/cross-origin-no-authorization.html bug 33357 (author: ap@webkit.org) The commit-queue is continuing to process your patch. 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> All reviewed patches have been landed. Closing bug. 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. The desire for WebCore unit testing goes waay waay back. See bug 21010. That said, I know nothing about this current effort. Filed bug 58679 |