lint job doesn't show tap result

[refack]

Maybe I'm doing something wrong, but I can't figure out how to get the lint errors...

e.g. https://ci.nodejs.org/job/node-test-linter/9042/console

+ gmake lint-ci
Running JS linter...
./node tools/jslint.js -j 2 -f tap -o test-eslint.tap \
	benchmark doc lib test tools
gmake: *** [Makefile:870: jslint-ci] Error 1
Build step 'Execute shell' marked build as failure
Run condition [Always] enabling perform for step [[Execute a set of scripts]]
[PostBuildScript] - Execution post build scripts.
[PostBuildScript] Build is not success : do not execute script
Run condition [Always] enabling perform for step [[Execute a set of scripts]]
[PostBuildScript] - Execution post build scripts.
[node-test-linter] $ /bin/sh -xe /tmp/hudson6596229026728241521.sh
+ set +x
Notifying upstream projects of job completion
Finished: FAILURE

Sorry to fast fire issues today.

[gibfahn]

You're right, we're passing the output to a TAP file, but not actually running the TAP parser on it. I tried enabling it, but the TAP plugin fails to parse it as it isn't valid TAP.

Looking for TAP results report in workspace using pattern: test-eslint.tap
Saving reports...
org.tap4j.parser.ParserException: Error parsing TAP Stream: Duplicated TAP Header found.
	at org.tap4j.parser.Tap13Parser.parseTapStream(Tap13Parser.java:230)
Caused by: org.tap4j.parser.ParserException: Duplicated TAP Header found.
	at org.tap4j.parser.Tap13Parser.parseLine(Tap13Parser.java:320)
	at org.tap4j.parser.Tap13Parser.parseTapStream(Tap13Parser.java:224)
	... 15 more
TAP parse errors found in the build. Marking build as UNSTABLE

The problem with the generated TAP output is that it's actually multiple TAP files concatenated together, so there are multiple TAP headers.

TAP version 13
1..1
ok 1 - /usr/home/iojs/build/workspace/node-test-linter/benchmark/vm/run-in-context.js

TAP version 13
1..1
ok 1 - /usr/home/iojs/build/workspace/node-test-linter/benchmark/vm/run-in-this-context.js

TAP version 13
1..2
ok 1 - /usr/home/iojs/build/workspace/node-test-linter/benchmark/util/normalize-encoding.js
ok 2 - /usr/home/iojs/build/workspace/node-test-linter/benchmark/v8/get-stats.js

In the short term you can view the file directly (assuming you have the permissions) by going to the workspace.

[gibfahn]

Looks like we can definitely blame @jbergstroem for this 😜

@mscdex nodejs/node#5638 (comment)

@jbergstroem Does the tap output have to all be in one single group of results (e.g. only one TAP header in the output file) or does it not matter?

---

@jbergstroem nodejs/node#5638 (comment)

@mscdex I recall globbing files ("collect *.tap") working just fine.

[gibfahn]

So basically we need to update tools/jslint.js to remove all but the first instance of TAP version 13, I think that should work.

[refack]

I'm on it.

P.S. the workspace will only show that last run, right?

[gibfahn]

P.S. the workspace will only show that last run, right?

Yes, it's overwritten every time.

[gibfahn]

The plan cannot appear in the middle of the output, nor can it appear more than once.

I lied, the plan can't be repeated, I tested it and it still breaks the TAP parsing plugin. The easiest solution is to use Subtests that Jenkins can understand (see the plugin docs):

I've confirmed that this works in a test job:

TAP version 13
  1..9
  ok 1 - /usr/home/iojs/build/workspace/node-test-linter/benchmark/url/legacy-vs-whatwg-url-searchparams-serialize.js
  ok 2 - /usr/home/iojs/build/workspace/node-test-linter/benchmark/url/legacy-vs-whatwg-url-serialize.js
  ok 3 - /usr/home/iojs/build/workspace/node-test-linter/benchmark/url/url-format.js
  ok 4 - /usr/home/iojs/build/workspace/node-test-linter/benchmark/url/url-resolve.js
  ok 5 - /usr/home/iojs/build/workspace/node-test-linter/benchmark/url/url-searchparams-iteration.js
  ok 6 - /usr/home/iojs/build/workspace/node-test-linter/benchmark/url/url-searchparams-read.js
  ok 7 - /usr/home/iojs/build/workspace/node-test-linter/benchmark/url/url-searchparams-sort.js
  ok 8 - /usr/home/iojs/build/workspace/node-test-linter/benchmark/url/usvstring.js
  ok 9 - /usr/home/iojs/build/workspace/node-test-linter/benchmark/url/whatwg-url-idna.js
ok 1 Second set  
  1..4
  ok 1 - /usr/home/iojs/build/workspace/node-test-linter/benchmark/url/whatwg-url-properties.js
  ok 2 - /usr/home/iojs/build/workspace/node-test-linter/benchmark/util/format.js
  ok 3 - /usr/home/iojs/build/workspace/node-test-linter/benchmark/util/inspect-proxy.js
  ok 4 - /usr/home/iojs/build/workspace/node-test-linter/benchmark/util/inspect.js
ok 2 Third set  
  1..1
  ok 1 - /usr/home/iojs/build/workspace/node-test-linter/benchmark/vm/run-in-context.js
ok 3 Fourth set
  1..1
  ok 1 - /usr/home/iojs/build/workspace/node-test-linter/benchmark/vm/run-in-this-context.js
ok 4 Fifth set
  1..2
  ok 1 - /usr/home/iojs/build/workspace/node-test-linter/benchmark/util/normalize-encoding.js
  ok 2 - /usr/home/iojs/build/workspace/node-test-linter/benchmark/v8/get-stats.js
ok 5 sixth set
1..5

[refack]

I started writing the PR... ✍️

[gibfahn]

I changed

gmake lint-ci

to

gmake lint-ci || { cat test-eslint.tap && exit 1; }

, so if the test fails you'll get the test output catted to the console.

It's not the best solution, but I think it solves the problem in the immediate term.

[gibfahn]

New solution (to hopefully avoid #906) and make it easier to see what's going on:

#!/usr/bin/env bash -ex
# If lint-ci fails, print all the interesting lines to the console.
# FreeBSD sed can't handle \s, so use gsed if it exists.
which gsed &>/dev/null && SED=gsed || SED=sed
gmake lint-ci || { 
  cat test-eslint.tap | grep -v '^ok\|^TAP version 13\|^1\.\.' | $SED '/^\s*$/d' && 
  exit 1; }

Filters out the passing tests, the TAP version 13 headers, and the 1..10 lines.

[refack]

grep -v -P "^(ok|1\.\.|TAP|$)" test-eslint.tap

[gibfahn]

Updated comment above, the script got a bit more complicated because:

• /bin/bash doesn't exist
• POSIX sed doesn't support \s for whitespace.
• The linter runs on two boxes, one is test-rackspace-freebsd10-x64-1 which has gsed, the other istest-joyent-freebsd10-x64-2 which doesn't.

So at the moment on the joyent box it doesn't trim whitespace, and on the other rackspace box it does. If we could:

• Work out the POSIX equivalent of sed '/^\s*$/d' is (and test it on the joyent box)
• Install gsed on the joyent box

That would improve the situation.

At the moment the freebsd box trims blank lines, but the joyent one doesn't. It's better than before, but not ideal.

[gibfahn]

This is done for now, better solutions always appreciated.