node icon indicating copy to clipboard operation
node copied to clipboard

build: convert V8 test JSON to JUnit XML

Open kvakil opened this issue 3 years ago • 8 comments

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

kvakil avatar Jul 30 '22 06:07 kvakil

/cc @nodejs/v8

aduh95 avatar Jul 30 '22 09:07 aduh95

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

nodejs-github-bot avatar Aug 03 '22 09:08 nodejs-github-bot

@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

bnoordhuis avatar Sep 15 '22 10:09 bnoordhuis

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

bnoordhuis avatar Sep 18 '22 09:09 bnoordhuis

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-github-bot avatar Sep 20 '22 08:09 nodejs-github-bot

@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

targos avatar Sep 20 '22 09:09 targos

@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 🤷 .

richardlau avatar Sep 20 '22 13:09 richardlau

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 avatar Sep 20 '22 13:09 targos

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

targos avatar Sep 25 '22 15:09 targos

@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 avatar Sep 28 '22 15:09 richardlau

@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.

richardlau avatar Sep 28 '22 16:09 richardlau

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?

targos avatar Sep 29 '22 04:09 targos

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).

richardlau avatar Sep 29 '22 11:09 richardlau

Here's a run for https://github.com/nodejs/node/pull/44741

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

targos avatar Sep 29 '22 12:09 targos

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.

targos avatar Sep 29 '22 16:09 targos

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 avatar Sep 29 '22 17:09 richardlau

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.)

richardlau avatar Sep 29 '22 19:09 richardlau

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

targos avatar Sep 30 '22 08:09 targos

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.

targos avatar Sep 30 '22 08:09 targos

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

nodejs-github-bot avatar Sep 30 '22 08:09 nodejs-github-bot

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

nodejs-github-bot avatar Oct 02 '22 16:10 nodejs-github-bot

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

nodejs-github-bot avatar Oct 03 '22 10:10 nodejs-github-bot

Landed in 691f8d14274fa76ffaba00c07ed709bc83af144d

nodejs-github-bot avatar Oct 03 '22 17:10 nodejs-github-bot