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

Issue 190503002: Refactor script compilation / running & use of helper funcs in test-api.cc. (Closed)

Created:
6 years, 9 months ago by marja
Modified:
6 years, 9 months ago
Reviewers:
dcarney
CC:
v8-dev, Sven Panne
Visibility:
Public.

Description

Refactor script compilation / running & use of helper funcs in test-api.cc. The tests were using different kind of constructs for achieving the same thing. This makes refactoring the compilation API more difficult than it should be. cctest.h already contained helpers for compiling and running scripts, but they were not used consistently. For example, all these were used for running scripts: v8::Script::Compile(v8_str("foo"))->Run(); v8::Script::Compile(v8::String::NewFromUtf8(isolate, "foo))->Run(); CompileRun(v8_str("foo")); CompileRun(v8::String::NewFromUtf8(some_way_to_get_isolate(), "foo")); v8::Local<v8::Script> script = any_of_the_above; script->Run(); Most of the tests just want to run a script (which is in const char*) and don't care about how the v8::String is constructed or passed to the compiler API. Using the helpers makes the test more readable and reduces boilerplate code which is unrelated to what the test is testing. R=dcarney@chromium.org BUG= Committed: https://code.google.com/p/v8/source/detail?r=19753

Patch Set 1 #

Patch Set 2 : . #

Patch Set 3 : . #

Patch Set 4 : rebased #

Unified diffs Side-by-side diffs Delta from patch set Stats (+264 lines, -262 lines) Patch
M test/cctest/cctest.h View 1 2 chunks +36 lines, -7 lines 0 comments Download
M test/cctest/test-api.cc View 1 2 3 93 chunks +228 lines, -255 lines 0 comments Download

Messages

Total messages: 5 (0 generated)
marja
svenpanne, ptal
6 years, 9 months ago (2014-03-07 09:43:14 UTC) #1
marja
Reviewer swap (svenpanne -> dcarney), so, dcarney, ptal
6 years, 9 months ago (2014-03-10 10:40:53 UTC) #2
dcarney
lgtm
6 years, 9 months ago (2014-03-10 11:00:30 UTC) #3
marja
thx for reviewing!
6 years, 9 months ago (2014-03-10 11:40:35 UTC) #4
marja
6 years, 9 months ago (2014-03-10 11:59:03 UTC) #5
Message was sent while issue was closed.
Committed patchset #4 manually as r19753 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698