New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 843185 link

Starred by 3 users

Issue metadata

Status: Available
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Feature



Sign in to add a comment

Test suite: add V8 tests (launched with v8/tools/run-tests.py)

Project Member Reported by mmoroz@chromium.org, May 15 2018

Issue description

From an email thread by cbruni@:

From reading the documentation and the test-suite files it seems that the v8 tests aren't run to generate coverage data. Is this correct?

Given that we have our own little test runner in v8/tools/run-tests.py, would it be easy to integrate?

 
Cc: machenb...@chromium.org
Components: Infra>Client>V8
Status: Available (was: Untriaged)
I'd be happy to help here - and deprecate both our gcov coverage and our sanitizer coverage in favor of this. Is it possible to use the coverage on V8 stand-alone? Or is anything from Chromium's src base required? (we depend on src/build and clang already).

Comment 2 by mmoroz@chromium.org, May 15 2018

Let me re-post my reply from the email thread:

Thanks for reaching out! Yes, we don't run any V8 specific tests and yes, it should be possible to integrate V8 tests. However, tests with their own launcher scripts require some extra handling, e.g. right now we're adding layout tests [1], and there are some issues we have to address. I've filed issue 843185 to track this.

It also would be helpful if someone from V8 could try generating coverage report locally for those tests using the script[2] and the docs[3]. But again, separate launcher scripts cause some troubles. If you try generating coverage locally using v8/tools/run-tests.py, you'll have to patch _GetBinaryPath function [4] so that it returns paths of binaries launched from the python scripts, as the binaries are needed for the report generation.

[1]: https://bugs.chromium.org/p/chromium/issues/detail?id=842851
[2]: https://cs.chromium.org/chromium/src/tools/code_coverage/coverage.py
[3]: https://chromium.googlesource.com/chromium/src/+/master/docs/code_coverage.md
[4]: https://cs.chromium.org/chromium/src/tools/code_coverage/coverage.py?l=1556

Comment 3 by mmoroz@chromium.org, May 15 2018

machenbach@, there are two things required from the Chromium repo:

1) use_clang_coverage=true GN flag

2) the script mentioned above
The python script seems to live in src/tools: https://cs.chromium.org/chromium/src/tools/code_coverage/

Any reason why it is there? Could it live in a more stand-alonish place? It doesn't seem to depend on anything in Chromium.

Like this it will be a bit harder to depend on it in e.g. V8 stand-alone. Though a subtreed mirror is also always possible.

Comment 5 by mmoroz@chromium.org, May 15 2018

Two reasons why it's there:

1) that seemed to be a natural place for such a tool :)

2) it needs to download an additional clang package to get llvm tools used for code coverage, https://cs.chromium.org/chromium/src/tools/code_coverage/coverage.py?l=441

Once we get the point 2) resolved, I think we can move it anywhere
We currently don't have a way to support standalone repos. For now, lets try to depend on the rolled version of v8 in chromium repo. If any developer needs it on latest trunk, we can always modify coverage.py to work with standalone repo. but for dashboard, lets stick it to chromium repo only. We need to control the complexity at this point.

Comment 7 by aarya@google.com, May 16 2018

Cc: -machenb...@chromium.org
Owner: machenb...@chromium.org
Status: Assigned (was: Available)
Here are the steps:

1. git clone https://chrome-internal.googlesource.com/chrome/tools/code-coverage
2. add run_v8_tests.sh similar to run_layout_tests.sh (change target to a name with _tests at end, like v8_tests).
3. Add run_v8_tests.sh call in code_coverage_loop.sh
4. Add build target for v8 tests in build_targets.sh (https://cs.chromium.org/chromium/src/tools/code_coverage/test_suite.txt)
similar to chrome, blink_tests (see also exclusion in run_test_targets.sh so that they are not executed directly.
5. run ./scripts/test_all.sh to see test complete with success, verify end-to-end integration

if steps look ok, maybe add a readme for future contributions.

Things might get migrated to infra recipes later, for now, these will run as bash scripts on our bots.
Or if want to save yourself from hassle of c#7.

you need a gn target, that builds other needed targets (like d8, etc) and when it is run, it executes the v8_tests.py with the appropriate arguments. Once you have such gn target, you just need to plug it in https://cs.chromium.org/chromium/src/tools/code_coverage/test_suite.txt and then no code modifications needed.

Comment 9 by mmoroz@chromium.org, May 16 2018

machenbach@, if you follow the steps from c#5, beware that step 5. will do "git clean -f -d" in your Chromium checkout, so don't keep there anything important and untracked :)
Running from within Chromium will make many things harder. We don't run any V8 test from within Chrome infra yet. Or is this not swarmed? For swarming, we'd need some recipe integration of V8 tests in Chromium recipe... a path that I rather don't want to go.

Could you point me to the builder that is running those tests in Chrome's infrastructure?
The testbots in Chrome's infra are in progress right now ( issue 821840 ).
Cc: serg...@chromium.org
machenbach@, sergiyb@, friendly ping. Is there any change you are working on this now or planning to work in Q3?
Owner: ----
Status: Available (was: Assigned)
No cycles now. Not planned for Q3 yet.
Labels: -Type-Bug Type-Feature
Blocking: -759794
Michael, Sergiy, Camillo, I'd strongly recommend to plan some time for this in Q1, as now Shuotao's team is working on moving Code Coverage to Chrome Infra, including its integration with Gerrit and other awesome stuff.

It's probably still worth starting with the steps described in c#7, just to get the tests running on the existing bots and see the reports on the dashboard. Once this works, it can be moved to Chrome Infra altogether with other stuff (e.g. layout tests).

Sign in to add a comment