Bug 21010

Summary: WebKit needs C++ Unit tests
Product: WebKit Reporter: Eric Seidel (no email) <eric>
Component: Tools / TestsAssignee: Nobody <webkit-unassigned>
Status: NEW ---    
Severity: Normal CC: cmarcelo, darin, ggaren, levin, mbelshe, mjs, mrowe, playmobil, yutak, yuzo, zwarich
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Macintosh   
OS: OS X 10.5   
Bug Depends on: 58507, 58509    
Bug Blocks: 55853    
Description Flags
First stab @ unit tests
First stab @ unit tests none

Description Eric Seidel (no email) 2008-09-22 16:16:44 PDT
WebKit needs C++ Unit tests

Maybe Google's gtest framework?  That's what Chrome uses.
Comment 1 Eric Seidel (no email) 2008-09-22 16:17:20 PDT
Created attachment 23679 [details]
First stab @ unit tests

 WebCore/WebCore.base.exp                  |    6 +
 WebCore/WebCore.xcodeproj/project.pbxproj |  145 ++++++++++++++++++++++++++++-
 2 files changed, 150 insertions(+), 1 deletions(-)
Comment 2 Eric Seidel (no email) 2008-09-22 16:18:54 PDT
Comment on attachment 23679 [details]
First stab @ unit tests

This is just a hack, was not meant for review.
Comment 3 Eric Seidel (no email) 2008-09-22 16:19:31 PDT
Created attachment 23680 [details]
First stab @ unit tests

 WebCore/WebCore.base.exp                           |    6 +
 WebCore/WebCore.xcodeproj/project.pbxproj          |  145 +++++++++++++++++++-
 .../platform/text/RegularExpressionUnitTests.cpp   |   85 ++++++++++++
 3 files changed, 235 insertions(+), 1 deletions(-)
Comment 4 Mark Rowe (bdash) 2008-09-22 16:23:46 PDT
Comment on attachment 23679 [details]
First stab @ unit tests

All of the configuration settings below XCBuildConfiguration should live in an .xcconfig file within WebCore/Configurations/, rather than inline in the project file.  While doing this, you'll probably notice that you appear to have some oddly duplicated entries in FRAMEWORK_SEARCH_PATH, as well as having some settings specified that probably shouldn't be (GCC_ENABLE_FIX_AND_CONTINUE, GCC_ENABLE_CPP_RTTI).

Why do you need to export WTF-related symbols from WebCore?  These symbols are already exported from JavaScriptCore, so this seems unnecessary.

I'd personally prefer to see unit tests that don't have a dependency on an external framework, so that they can work out of the box without extra work on behalf of the developer.
Comment 5 Cameron Zwarich (cpst) 2008-09-23 04:38:05 PDT
This patch has some possible licensing issues. Is the LICENSE file included in our tree? Shouldn't we just put the BSD license in the source files?
Comment 6 Eric Seidel (no email) 2009-03-27 11:43:14 PDT
I've been thinking about this further.  My current work refactoring work in editing would be helped greatly by being able to unit test the classes I'm re-working.

Thinking through general design issues:

1.  Unit tests should work in Debug builds, but need not work in Release builds.  (This allows us to depend on debug-only functionality).

2.  I don't think we want to re-invent the wheel.  I'm not familiar with all c++ testing frameworks, but there seem to be numerous (CPlusTest ships w/ OSX, gunit is what Google uses for everything, I know there are others).  The simple functionality is easy, but there are lots of bells and whistles w/ other's approaches.

3.  Unit tests should be built as part of the normal process (or even if the tests aren't built as part of a normal build, building the tests should not require a special build of WebCore/JSC, etc.)

To satisfy goal 3, I can see two solutions:
1.  If unit test code is built outside of WebCore then WebCore Debug builds will need to expose some extra symbols for the unit tests.
2.  If unit tests are built inside WebCore, a single symbol (runTests()) needs to be exported, and all the unit test code will need to be guarded in !NDEBUG #ifdefs to prevent it from getting built into a Release/Production binary.  This approach could directly conflict with goal 2 above (not re-inventing the wheel), as this would require linking in a bunch of 3rd-party code into WebCore debug builds which could be undesirable.

I'd be interested to hear other's thoughts.