search

nodejs / node-v8

Experimental Node.js mirror on V8 lkgr :sparkles::turtle::rocket::sparkles:
418 stars 71 forks source link

Broken V8 CI #100

Closed targos closed 3 years ago

targos commented 5 years ago

https://ci.nodejs.org/job/node-test-commit-v8-linux/2081/nodes=benchmark,v8test=v8test/console

Error:

12:22:20 deps/v8/tools/run-tests.py --gn --arch=x64 \
12:22:20         --mode=release --progress=dots --timeout=120 \
12:22:20                mjsunit cctest debugger inspector message preparser \
12:22:20          --junitout /home/iojs/build/workspace/node-test-commit-v8-linux/nodes/benchmark/v8test/v8test/v8-tap.xml
12:22:20 Usage: run-tests.py [options] [tests]
12:22:20 
12:22:20 run-tests.py: error: no such option: --junitout
targos commented 5 years ago

Support for junit output was removed in https://github.com/v8/v8/commit/bd019bdb725cebaa34327634d73936cd7003d17c

refack commented 5 years ago

Who do we talk to to get this back. JUnit is the lingua franca of Jenkins...

@nodejs/v8-update

hashseed commented 5 years ago

I wasn't aware this was used. Is there no alternative? It's hard to maintain features we don't test upstream.

refack commented 5 years ago

Ironically I wanted to port it to node-core's test.py, since ATM we output TAP and transform it to JUnit...

From what I see in that file you also dropped TAP so the only machine readable output format is your proprietary JSON? https://github.com/v8/v8/blob/master/tools/testrunner/testproc/progress.py#L249

An alternative could be to write a transformer for that, but IMHO restoring those dropped 95 lines would be the lowest hanging fruit. (For which you get CI test coverage from us ;)

refack commented 5 years ago

No expected resolution?

hashseed commented 5 years ago

@tmrts we should consider readding junit support.

tmrts commented 5 years ago

@hashseed agreed, I'll readd it this week.

Thanks for the details @targos @refack!

refack commented 5 years ago

@tmrts, thanks for your consideration!

refack commented 5 years ago

ping? any news of this?

refack commented 5 years ago

BTW you can rename this xUnit as the format has been adopted by multiple tools for several runtimes - https://en.wikipedia.org/wiki/XUnit#Test_result_formatter

mmarchini commented 4 years ago

Is there anything left to be done here?

targos commented 4 years ago

Either someone tries to upstream the revert: https://github.com/nodejs/node-v8/commit/c58cd35aa191b746fe2c242696da13ba718d1995#diff-f93464a48a2b9281080b555fac1081b1852154552800cbc6a2440ad1e3629033 Or we accept to float it forever and close this issue.

mhdawson commented 4 years ago

As I understand it the only impact is that we don't get the test results in a format that we'd like. We still run/get results and have coped for 1 1/2 years. I figure we just close. @targos make sense to you?

cclauss commented 3 years ago

% flake8 . --count --select=E9,F63,F7,F82 --show-source --statistics

./v8/tools/dev/update-compile-commands.py:55:25: F821 undefined name 'Error'
    if code != 0: raise Error("gn gen failed")
                        ^
./v8/tools/clusterfuzz/js_fuzzer/tools/minimize.py:26:1: F631 assertion is always true, perhaps remove parentheses?
assert(len(sys.argv) > 1, 'Need to specify minimizer path.')
^
1     F631 assertion is always true, perhaps remove parentheses?
1     F821 undefined name 'Error'
2

Fixes needed:

    if code != 0: raise Exception("gn gen failed")
...and...
assert len(sys.argv) > 1, 'Need to specify minimizer path.'
targos commented 3 years ago

@cclauss That's unrelated. This issue was about a patch that we have to float on top of V8 to keep support for junit test output.