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

Issue 677153004: New test variant for --cache=parse (Closed)

Created:
6 years, 2 months ago by Ian Cullinan
Modified:
5 years, 10 months ago
CC:
v8-dev, mckev
Base URL:
https://chromium.googlesource.com/external/v8.git@bleeding_edge
Project:
v8
Visibility:
Public.

Description

New test variant for --cache=parse This CL updates run-tests.py to add a variant which exercises kProduceCodeCache and kConsumeParserCache (which is how Blink typically uses V8). Also adds a TryCatch to CompileForCachedData() in d8 to avoid printing SyntaxErrors twice (for tests like message/isvar which expect d8 to print a syntax error). Note that this increases the time to run the default tests by about 20%. BUG=v8:3651 LOG=

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+4 lines, -2 lines) Patch
M src/d8.cc View 1 chunk +1 line, -0 lines 0 comments Download
M tools/run-tests.py View 1 chunk +3 lines, -2 lines 0 comments Download

Messages

Total messages: 5 (2 generated)
Ian Cullinan
Hi Michael, PTAL at this patch for improving test coverage of CachedData, necessitated by https://codereview.chromium.org/641283003/ ...
6 years, 2 months ago (2014-10-24 21:57:50 UTC) #2
Jakob Kummerow
I'm opposed to adding more variants. The test suite is too slow as it is. ...
6 years, 1 month ago (2014-10-27 10:16:43 UTC) #4
Ian Cullinan
6 years, 1 month ago (2014-10-27 22:07:15 UTC) #5
On 2014/10/27 10:16:43, Jakob wrote:
> I'm opposed to adding more variants. The test suite is too slow as it is.
> 
> I don't know the overall situation here -- maybe the flag should just be
turned
> on by default? Or always be specified by the test runner?

Good point. Completely understand.

I tried making --cache=parse the default in run-tests.py and it also increased
the test time by about 20%. Doing this on a bot (as suggested by yangguo) makes
sense. Looking at how the new code serializer bot works, it would be messy to
make it run both. What would you think about adding another bot like the code
serializer one, except using --cache=parse instead of --cache=code? I would be
happy to send an infra patch for that.

Since https://codereview.chromium.org/641283003/ is my primary motivation for
getting this coverage, I'm happy to just run the tests with --cache=parse
locally if we can't find a suitable approach.

Cheers,
Ian C

Powered by Google App Engine
This is Rietveld 408576698