build: convert V8 test JSON to JUnit XML
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: https://github.com/nodejs/node-v8/issues/236
/cc @nodejs/v8
CI: https://ci.nodejs.org/job/node-test-pull-request/45816/
@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
@kvakil I took the liberty of applying the patch and rebasing your commit on top of main. Let's see how it fares.
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/
@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
@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 🤷 .
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.
@richardlau do you have a suggestion about what we should do now?
@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 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" fiand then replaced
ENABLE_V8_TAP=Truewith"$TAP_OUTPUT_OPTION"in the variables passed tomake.
I typoed junitout 😅 . Also had to update to use the same version of Python that configure picked up.
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?
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 https://github.com/nodejs/node/pull/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).
Here's a run for https://github.com/nodejs/node/pull/44741
https://ci.nodejs.org/job/node-test-commit-v8-linux/4887/
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.
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
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.)
Just trying again: https://ci.nodejs.org/job/node-test-commit-v8-linux/4889/nodes=benchmark-ubuntu1604-intel-64,v8test=v8test/
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.
CI: https://ci.nodejs.org/job/node-test-pull-request/46952/
CI: https://ci.nodejs.org/job/node-test-pull-request/46988/
CI: https://ci.nodejs.org/job/node-test-pull-request/47029/
Landed in 691f8d14274fa76ffaba00c07ed709bc83af144d