build: convert V8 test JSON to JUnit XML #44049

kvakil · 2022-07-30T06:16:23Z

This introduces some code to convert from V8's test JSON output to JUnit
XML. We need this because V8's latest refactor of their test runner has
made it difficult to float our JUnit reporter patch on top (see the
referenced issue).

I also think that there needs to be the same changes to vcbuild.bat, but
I don't know how to do/test those yet. I can create a Windows VM and
test it if we decide to go with this approach.

Closes: nodejs/node-v8#236

aduh95 · 2022-07-30T09:59:11Z

/cc @nodejs/v8

Makefile

tools/v8-json-to-junit.py

nodejs-github-bot · 2022-08-03T09:29:39Z

CI: https://ci.nodejs.org/job/node-test-pull-request/45816/

bnoordhuis · 2022-09-15T10:29:03Z

@kvakil Can you try this patch?

diff --git a/tools/test-v8.bat b/tools/test-v8.bat
index d322c31a38d..64265157f00 100644
--- a/tools/test-v8.bat
+++ b/tools/test-v8.bat
@@ -20,18 +20,21 @@ if errorlevel 1 set ERROR_STATUS=1&goto test-v8-exit
 set path=%savedpath%
 
 if not defined test_v8 goto test-v8-intl
-echo running 'python tools\run-tests.py %common_v8_test_options% %v8_test_options% --junitout ./v8-tap.xml'
-call python tools\run-tests.py %common_v8_test_options% %v8_test_options% --junitout ./v8-tap.xml
+echo running 'python tools\run-tests.py %common_v8_test_options% %v8_test_options% --slow-tests-cutoff 1000000 --json-test-results v8-tap.xml'
+call python tools\run-tests.py %common_v8_test_options% %v8_test_options% --slow-tests-cutoff 1000000 --json-test-results v8-tap.xml
+call python ..\..\tools\v8-json-to-junit.py < v8-tap.xml > v8-tap.json
 
 :test-v8-intl
 if not defined test_v8_intl goto test-v8-benchmarks
-echo running 'python tools\run-tests.py %common_v8_test_options% intl --junitout ./v8-intl-tap.xml'
-call python tools\run-tests.py %common_v8_test_options% intl --junitout ./v8-intl-tap.xml
+echo running 'python tools\run-tests.py %common_v8_test_options% intl --slow-tests-cutoff 1000000 --json-test-results v8-intl-tap.xml'
+call python tools\run-tests.py %common_v8_test_options% intl --slow-tests-cutoff 1000000 --json-test-results ./v8-intl-tap.xml
+call python ..\..\tools\v8-json-to-junit.py < v8-intl-tap.xml > v8-intl-tap.json
 
 :test-v8-benchmarks
 if not defined test_v8_benchmarks goto test-v8-exit
-echo running 'python tools\run-tests.py %common_v8_test_options% benchmarks --junitout ./v8-benchmarks-tap.xml'
-call python tools\run-tests.py %common_v8_test_options% benchmarks --junitout ./v8-benchmarks-tap.xml
+echo running 'python tools\run-tests.py %common_v8_test_options% benchmarks --slow-tests-cutoff 1000000 --json-test-results v8-benchmarks-tap.xml'
+call python tools\run-tests.py %common_v8_test_options% benchmarks --slow-tests-cutoff 1000000 --json-test-results ./v8-benchmarks-tap.xml
+call python ..\..\tools\v8-json-to-junit.py < v8-benchmarks-tap.xml > v8-benchmarks-tap.json
 goto test-v8-exit
 
 :test-v8-exit

This introduces some code to convert from V8's test JSON output to JUnit XML. We need this because V8's latest refactor of their test runner has made it difficult to float our JUnit reporter patch on top (see the referenced issue). I also think that there needs to be the same changes to vcbuild.bat, but I don't know how to do test those yet. I can create a Windows VM and test it if we decide to go with this approach. Refs: nodejs/node-v8#236

bnoordhuis · 2022-09-18T09:15:14Z

@kvakil I took the liberty of applying the patch and rebasing your commit on top of main. Let's see how it fares.

nodejs-github-bot · 2022-09-20T08:22:34Z

CI: https://ci.nodejs.org/job/node-test-pull-request/46667/
~~V8: https://ci.nodejs.org/job/node-test-commit-v8-linux/4863/~~
V8: https://ci.nodejs.org/job/node-test-commit-v8-linux/4864/

targos · 2022-09-20T09:21:08Z

@nodejs/build-infra there seems to be an issue with test-nearform_intel-ubuntu1604-x64-1:

11:15:39 [0:00:42] fatal: unable to access 'https://chromium.googlesource.com/chromium/src/third_party/jinja2.git/': Could not resolve host: chromium.googlesource.com

richardlau · 2022-09-20T13:08:03Z

@nodejs/build-infra there seems to be an issue with test-nearform_intel-ubuntu1604-x64-1:
11:15:39 [0:00:42] fatal: unable to access 'https://chromium.googlesource.com/chromium/src/third_party/jinja2.git/': Could not resolve host: chromium.googlesource.com

I'm not entirely sure what's wrong with it. I rebooted it and the "Could not resolve host" error hasn't reappeared but we are getting GnuTLS errors now

13:04:04 [0:05:50] Receiving objects:   0% (1028/124226), 771.77 KiB | 3.00 KiB/s   
13:04:04 [0:07:50] error: RPC failed; curl 56 GnuTLS recv error (-54): Error in the pull function.
13:04:04 [0:07:50] fatal: The remote end hung up unexpectedly
13:04:04 [0:07:50] fatal: early EOF
13:04:04 [0:07:50] fatal: index-pack failed

Although https://ci.nodejs.org/job/node-test-commit-v8-linux/4868/ (rerun of the CI for this PR) passed so the issue isn't consistent 🤷 .

targos · 2022-09-20T13:43:17Z

So, node-test-commit-v8-linux isn't broken. Now I don't know how to try it with ENABLE_CONVERT_V8_JSON_TO_XML.

targos · 2022-09-25T15:42:53Z

@richardlau do you have a suggestion about what we should do now?

richardlau · 2022-09-28T15:55:05Z

@richardlau do you have a suggestion about what we should do now?

I've made some changes to the job:
Added

TAP_OUTPUT_OPTION="ENABLE_CONVERT_V8_JSON_TO_XML=True"
if (./deps/v8/tools/run-tests.py --help | grep -q junittout); then
  TAP_OUTPUT_OPTION="ENABLE_V8_TAP=True"
fi

and then replaced ENABLE_V8_TAP=True with "$TAP_OUTPUT_OPTION" in the variables passed to make.

richardlau · 2022-09-28T16:41:36Z

@richardlau do you have a suggestion about what we should do now?

I've made some changes to the job: Added
TAP_OUTPUT_OPTION="ENABLE_CONVERT_V8_JSON_TO_XML=True"
if (./deps/v8/tools/run-tests.py --help | grep -q junittout); then
  TAP_OUTPUT_OPTION="ENABLE_V8_TAP=True"
fi
and then replaced ENABLE_V8_TAP=True with "$TAP_OUTPUT_OPTION" in the variables passed to make.

I typoed junitout 😅 . Also had to update to use the same version of Python that configure picked up.

targos · 2022-09-29T04:46:57Z

Is https://ci.nodejs.org/job/node-test-commit-v8-linux/4882/ the new run?
It failed with 18:19:57 ERROR: Step ‘Publish JUnit test result report’ failed: No test report files were found. Configuration error?

richardlau · 2022-09-29T11:12:05Z

Is https://ci.nodejs.org/job/node-test-commit-v8-linux/4882/ the new run? It failed with 18:19:57 ERROR: Step ‘Publish JUnit test result report’ failed: No test report files were found. Configuration error?

That run was the one that revealed I needed to amend the job to use the same version of Python that the configure script picked up.
https://ci.nodejs.org/job/node-test-commit-v8-linux/4884/ is the change with v14.x
https://ci.nodejs.org/job/node-test-commit-v8-linux/4885/ with v16.x
https://ci.nodejs.org/job/node-test-commit-v8-linux/4886/ is main

All of the above are without this PR but with the job changes. Since the job changes detect if the V8 test runner supports --junitout it will only use the new converter in this PR with the updated V8 that doesn't carry our junit reporter patch (e.g. in #44741).

I'm not sure what's up with the benchmark machine -- it looks like issues fetching V8 dependencies stretching back over a week (i.e. before any of these changes).

targos · 2022-09-29T12:09:55Z

Here's a run for #44741

https://ci.nodejs.org/job/node-test-commit-v8-linux/4887/

targos · 2022-09-29T16:46:02Z

I'm not sure what's up with the benchmark machine -- it looks like issues fetching V8 dependencies stretching back over a week (i.e. before any of these changes).

Can we clear the workspace and try again? I don't remember how to do that in Jenkins.

richardlau · 2022-09-29T17:26:47Z

I'm not sure what's up with the benchmark machine -- it looks like issues fetching V8 dependencies stretching back over a week (i.e. before any of these changes).

Can we clear the workspace and try again? I don't remember how to do that in Jenkins.

You go to the Workspace link for the job (note not the numbered runs) and then "wipe out workspace" from there, e.g.

https://ci.nodejs.org/job/node-test-commit-v8-linux/ws/
https://ci.nodejs.org/job/node-test-commit-v8-linux/nodes=benchmark-ubuntu1604-intel-64,v8test=v8test/ws/

I've wiped out the workspace for this job and started a new run off main: https://ci.nodejs.org/job/node-test-commit-v8-linux/4888

richardlau · 2022-09-29T19:02:21Z

I've wiped out the workspace for this job and started a new run off main: https://ci.nodejs.org/job/node-test-commit-v8-linux/4888

Nope, doesn't appear to have fixed it 😞 .

18:47:07 [0:04:58] fatal: unable to access 'https://chromium.googlesource.com/chromium/tools/depot_tools.git/': Could not resolve host: chromium.googlesource.com

which is really odd because right at the beginning of the job we had a successful git clone of https://chromium.googlesource.com/chromium/tools/depot_tools.git

(we should probably spin this discussion out to a separate issue as it's not really anything to do with this PR.)

targos · 2022-09-30T08:08:13Z

Just trying again: https://ci.nodejs.org/job/node-test-commit-v8-linux/4889/nodes=benchmark-ubuntu1604-intel-64,v8test=v8test/

targos · 2022-09-30T08:58:42Z

we should probably spin this discussion out to a separate issue as it's not really anything to do with this PR

I agree, this is an infra issue and shouldn't block this PR.

nodejs-github-bot · 2022-09-30T08:59:36Z

CI: https://ci.nodejs.org/job/node-test-pull-request/46952/

nodejs-github-bot · 2022-10-02T16:30:06Z

CI: https://ci.nodejs.org/job/node-test-pull-request/46988/

nodejs-github-bot · 2022-10-03T10:42:55Z

CI: https://ci.nodejs.org/job/node-test-pull-request/47029/

nodejs-github-bot · 2022-10-03T17:45:41Z

Landed in 691f8d1

This introduces some code to convert from V8's test JSON output to JUnit XML. We need this because V8's latest refactor of their test runner has made it difficult to float our JUnit reporter patch on top (see the referenced issue). I also think that there needs to be the same changes to vcbuild.bat, but I don't know how to do test those yet. I can create a Windows VM and test it if we decide to go with this approach. Refs: nodejs/node-v8#236 PR-URL: nodejs#44049 Fixes: nodejs/node-v8#236 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com>

This introduces some code to convert from V8's test JSON output to JUnit XML. We need this because V8's latest refactor of their test runner has made it difficult to float our JUnit reporter patch on top (see the referenced issue). I also think that there needs to be the same changes to vcbuild.bat, but I don't know how to do test those yet. I can create a Windows VM and test it if we decide to go with this approach. Refs: nodejs/node-v8#236 PR-URL: #44049 Fixes: nodejs/node-v8#236 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com>

nodejs-github-bot added build Issues and PRs related to build files or the CI. meta Issues and PRs related to the general management of the project. needs-ci PRs that need a full CI run. tools Issues and PRs related to the tools directory. labels Jul 30, 2022

kvakil mentioned this pull request Jul 30, 2022

Lost JUnit support for V8 tests nodejs/node-v8#236

Closed

bnoordhuis approved these changes Jul 30, 2022

View reviewed changes

Makefile Show resolved Hide resolved

tools/v8-json-to-junit.py Show resolved Hide resolved

bnoordhuis approved these changes Jul 30, 2022

View reviewed changes

bnoordhuis added request-ci Add this label to start a Jenkins CI on a PR. and removed needs-ci PRs that need a full CI run. labels Aug 3, 2022

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 3, 2022

jasnell approved these changes Aug 28, 2022

View reviewed changes

bnoordhuis mentioned this pull request Sep 15, 2022

Update V8 before the release of v19.0.0 #44650

Closed

5 tasks

bnoordhuis force-pushed the v8testjson branch from bbff9b5 to a8bf726 Compare September 18, 2022 09:14

richardlau mentioned this pull request Sep 28, 2022

deps: update V8 to 10.7 #44741

Closed

targos added the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 3, 2022

targos approved these changes Oct 3, 2022

View reviewed changes

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 3, 2022

nodejs-github-bot merged commit 691f8d1 into nodejs:main Oct 3, 2022

github-actions bot mentioned this pull request Oct 4, 2022

CI Reliability 2022-10-04 nodejs/reliability#394

Open

16 tasks

github-actions bot mentioned this pull request Oct 5, 2022

CI Reliability 2022-10-05 nodejs/reliability#395

Open

19 tasks

danielleadams mentioned this pull request Oct 11, 2022

v18.11.0 proposal #44968

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

build: convert V8 test JSON to JUnit XML #44049

build: convert V8 test JSON to JUnit XML #44049

kvakil commented Jul 30, 2022 •

edited by targos

Loading

aduh95 commented Jul 30, 2022

nodejs-github-bot commented Aug 3, 2022

bnoordhuis commented Sep 15, 2022

bnoordhuis commented Sep 18, 2022

nodejs-github-bot commented Sep 20, 2022 •

edited by targos

Loading

targos commented Sep 20, 2022

richardlau commented Sep 20, 2022

targos commented Sep 20, 2022

targos commented Sep 25, 2022

richardlau commented Sep 28, 2022

richardlau commented Sep 28, 2022

targos commented Sep 29, 2022

richardlau commented Sep 29, 2022

targos commented Sep 29, 2022

targos commented Sep 29, 2022

richardlau commented Sep 29, 2022

richardlau commented Sep 29, 2022

targos commented Sep 30, 2022

targos commented Sep 30, 2022

nodejs-github-bot commented Sep 30, 2022

nodejs-github-bot commented Oct 2, 2022

nodejs-github-bot commented Oct 3, 2022

nodejs-github-bot commented Oct 3, 2022

build: convert V8 test JSON to JUnit XML #44049

build: convert V8 test JSON to JUnit XML #44049

Conversation

kvakil commented Jul 30, 2022 • edited by targos Loading

aduh95 commented Jul 30, 2022

nodejs-github-bot commented Aug 3, 2022

bnoordhuis commented Sep 15, 2022

bnoordhuis commented Sep 18, 2022

nodejs-github-bot commented Sep 20, 2022 • edited by targos Loading

targos commented Sep 20, 2022

richardlau commented Sep 20, 2022

targos commented Sep 20, 2022

targos commented Sep 25, 2022

richardlau commented Sep 28, 2022

richardlau commented Sep 28, 2022

targos commented Sep 29, 2022

richardlau commented Sep 29, 2022

targos commented Sep 29, 2022

targos commented Sep 29, 2022

richardlau commented Sep 29, 2022

richardlau commented Sep 29, 2022

targos commented Sep 30, 2022

targos commented Sep 30, 2022

nodejs-github-bot commented Sep 30, 2022

nodejs-github-bot commented Oct 2, 2022

nodejs-github-bot commented Oct 3, 2022

nodejs-github-bot commented Oct 3, 2022

kvakil commented Jul 30, 2022 •

edited by targos

Loading

nodejs-github-bot commented Sep 20, 2022 •

edited by targos

Loading