Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(16)

Issue 138133002: Create our own blink_heap run_all_tests target to avoid piggy-backing on wtf's. (Closed)

Created:
6 years, 11 months ago by wibling-chromium
Modified:
6 years, 11 months ago
CC:
blink-reviews, haraken, Mads Ager (chromium)
Visibility:
Public.

Description

Create our own blink_heap run_all_tests target to avoid piggy-backing on wtf's. We used to use wtf's run_all_tests target to run out blink heap unittests. That causes a problem when we want to initialize our heap since we cannot call Heap::init/shutdown from wtf's RunAllTests since it cannot/should not depend on blink heap. R=ager@chromium.org, erik.corry@gmail.com, haraken@chromium.org, vegorov@chromium.org, zerny@chromium.org BUG= Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=165073

Patch Set 1 #

Total comments: 2

Patch Set 2 : Review feedback #

Unified diffs Side-by-side diffs Delta from patch set Stats (+61 lines, -131 lines) Patch
M Source/core/Init.cpp View 3 chunks +3 lines, -0 lines 0 comments Download
M Source/heap/HeapTest.cpp View 12 chunks +14 lines, -107 lines 0 comments Download
A + Source/heap/RunAllTests.cpp View 1 3 chunks +6 lines, -2 lines 0 comments Download
M Source/heap/blink_heap_tests.gyp View 1 chunk +38 lines, -22 lines 0 comments Download

Messages

Total messages: 9 (0 generated)
wibling-chromium
6 years, 11 months ago (2014-01-14 12:23:18 UTC) #1
haraken
LGTM https://codereview.chromium.org/138133002/diff/1/Source/heap/RunAllTests.cpp File Source/heap/RunAllTests.cpp (right): https://codereview.chromium.org/138133002/diff/1/Source/heap/RunAllTests.cpp#newcode34 Source/heap/RunAllTests.cpp:34: Nit: This empty line is not necessary.
6 years, 11 months ago (2014-01-14 12:26:28 UTC) #2
wibling-chromium
Thanks for the review! https://codereview.chromium.org/138133002/diff/1/Source/heap/RunAllTests.cpp File Source/heap/RunAllTests.cpp (right): https://codereview.chromium.org/138133002/diff/1/Source/heap/RunAllTests.cpp#newcode34 Source/heap/RunAllTests.cpp:34: On 2014/01/14 12:26:28, haraken wrote: ...
6 years, 11 months ago (2014-01-14 12:29:35 UTC) #3
wibling-chromium
6 years, 11 months ago (2014-01-14 12:36:28 UTC) #4
Mads Ager (chromium)
LGTM
6 years, 11 months ago (2014-01-14 12:56:49 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/wibling@chromium.org/138133002/40001
6 years, 11 months ago (2014-01-14 13:53:45 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/wibling@chromium.org/138133002/40001
6 years, 11 months ago (2014-01-14 14:15:29 UTC) #7
commit-bot: I haz the power
Change committed as 165073
6 years, 11 months ago (2014-01-14 15:53:55 UTC) #8
alph
6 years, 11 months ago (2014-01-14 16:07:34 UTC) #9
Message was sent while issue was closed.
A revert of this CL has been created in
https://codereview.chromium.org/138443002/ by alph@chromium.org.

The reason for reverting is: Broke compile..

Powered by Google App Engine
This is Rietveld 408576698