Add wtf_unittests target that links unit tests outside DLL

Add wtf_unittests target that links unit tests outside DLL

This moves the wtf unittests out of the webkit_unit_tests target and into
a dedicated gtest executable. This target builds and links much faster since
it's tiny and moves the tests outside the DLL. This target is in all_webkit,
but annoyingly we'll have to configure the bots to run this step.

R=abarth@chromium.org, darin@chromium.org

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=151305

[darin (slow to review), 2013-05-25 22:56:00]

LGTM

https://codereview.chromium.org/16066003/diff/1/Source/wtf/tests/RunAllTests.cpp
File Source/wtf/tests/RunAllTests.cpp (right):

https://codereview.chromium.org/16066003/diff/1/Source/wtf/tests/RunAllTests....
Source/wtf/tests/RunAllTests.cpp:38: return 0.0;
Should there be an assertion that this is never called?  Or, how do we avoid
someone tripping over thinking CurrentTime() actually works?

[darin (slow to review), 2013-05-25 22:56:09]

LGTM

[abarth-chromium, 2013-05-26 00:29:58]

https://codereview.chromium.org/16066003/diff/1/Source/wtf/wtf_tests.gyp
File Source/wtf/wtf_tests.gyp (right):

https://codereview.chromium.org/16066003/diff/1/Source/wtf/wtf_tests.gyp#newc...
Source/wtf/wtf_tests.gyp:58: 'msvs_disabled_warnings': [4127, 4355, 4510, 4512,
4610, 4706, 4068, 4267],
Can we put all this junk into config.gyp?  (Ideally, the cflags_cc settings
too.)

[jamesr, 2013-05-27 23:51:22]

The disabled warning set here is almost completely disjoint with the one in
config.gyp.  I don't like the idea of just unioning the sets - that will make it
harder to get rid of warnings target-by-target.  Tomorrow when I'm in front of
my windows workstation I'll remove the warnings that aren't triggered here.

[abarth-chromium, 2013-05-28 00:04:57]

Thanks.  Yeah, the ideal situation would be just to fix all the warnings.  Now
that we're driving the codebase, that should be possible to do and to maintain.

[jamesr, 2013-05-28 03:42:23]

This rejiggers the gyps a bit - would you mind taking a look, Adam?  The main
change is moving WEBKIT_IMPLEMENTATION out of config.gyp out to targets that are
linked into webkit.dll.  I've also made run_all_tests a separate static_lib so
it's easier to link into other unit test targets.

[abarth-chromium, 2013-05-28 06:28:24]

LGTM

https://codereview.chromium.org/16066003/diff/14001/Source/wtf/tests/RunAllTe...
File Source/wtf/tests/RunAllTests.cpp (right):

https://codereview.chromium.org/16066003/diff/14001/Source/wtf/tests/RunAllTe...
Source/wtf/tests/RunAllTests.cpp:46: WTF::initializeMainThread(0);
It would be better if these calls were combined...

https://codereview.chromium.org/16066003/diff/14001/Source/wtf/wtf_tests.gyp
File Source/wtf/wtf_tests.gyp (right):

https://codereview.chromium.org/16066003/diff/14001/Source/wtf/wtf_tests.gyp#...
Source/wtf/wtf_tests.gyp:35: 'target_name': 'wtf_unittests',
Would you be willing to make a CL so that these sharded unit test executable are
run on the bots (and on the try servers)?

https://codereview.chromium.org/16066003/diff/14001/Source/wtf/wtf_tests.gyp#...
Source/wtf/wtf_tests.gyp:73: '..'
This could be exported by config.

https://codereview.chromium.org/16066003/diff/14001/public/all.gyp
File public/all.gyp (right):

https://codereview.chromium.org/16066003/diff/14001/public/all.gyp#newcode42
public/all.gyp:42: '../Source/wtf/wtf_tests.gyp:wtf_unittests',
Is this the normal convention for Chromium?  I guess we should rename
webkit_unit_tests to foo_unittests at some point?

[jamesr, 2013-05-28 06:55:47]

On 2013/05/28 06:28:24, abarth wrote:
> LGTM
> 
>
https://codereview.chromium.org/16066003/diff/14001/Source/wtf/tests/RunAllTe...
> File Source/wtf/tests/RunAllTests.cpp (right):
> 
>
https://codereview.chromium.org/16066003/diff/14001/Source/wtf/tests/RunAllTe...
> Source/wtf/tests/RunAllTests.cpp:46: WTF::initializeMainThread(0);
> It would be better if these calls were combined...

WTF::initializeMainThread() should go away soon - basically the only thing that
I'm blocked on is not understanding wtf/Functional.h well enough - and then this
line can go as well.

> 
> https://codereview.chromium.org/16066003/diff/14001/Source/wtf/wtf_tests.gyp
> File Source/wtf/wtf_tests.gyp (right):
> 
>
https://codereview.chromium.org/16066003/diff/14001/Source/wtf/wtf_tests.gyp#...
> Source/wtf/wtf_tests.gyp:35: 'target_name': 'wtf_unittests',
> Would you be willing to make a CL so that these sharded unit test executable
are
> run on the bots (and on the try servers)?

Yes - filed https://code.google.com/p/chromium/issues/detail?id=244342 to track
this work.

>
https://codereview.chromium.org/16066003/diff/14001/Source/wtf/wtf_tests.gyp#...
> Source/wtf/wtf_tests.gyp:73: '..'
> This could be exported by config.
> 
> https://codereview.chromium.org/16066003/diff/14001/public/all.gyp
> File public/all.gyp (right):
> 
> https://codereview.chromium.org/16066003/diff/14001/public/all.gyp#newcode42
> public/all.gyp:42: '../Source/wtf/wtf_tests.gyp:wtf_unittests',
> Is this the normal convention for Chromium?  I guess we should rename
> webkit_unit_tests to foo_unittests at some point?

the _unittests convention for unit tests is pretty consistent at least for newer
unit test targets.  There's one unit_tests target (I believe it's the original
unit test target), but the unit tests for component foo are pretty much always
called foo_unittests.

[commit-bot: I haz the power, 2013-05-28 18:40:17]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jamesr@chromium.org/16066003/26001

[commit-bot: I haz the power, 2013-05-28 19:01:41]

Retried try job too often on blink_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=blink_pres...

[commit-bot: I haz the power, 2013-05-28 19:06:14]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jamesr@chromium.org/16066003/26001

[commit-bot: I haz the power, 2013-05-28 19:38:40]

Retried try job too often on blink_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=blink_pres...

[jamesr, 2013-05-28 21:37:11]

Committed patchset #7 manually as r151305 (presubmit successful).