tools: add tap output to cpplint

[jbergstroem]

Implement a crude TAP13 writer for cpplint. Does its job and not much else. Only supports writing TAP output to file, not vs7 or emacs formats.

[silverwind]

Not that I care much, but this wouldn't work on Arch Linux where python is python3.

[jbergstroem]

I don't think python2.4 would work much better :) Just pulled the same convention as what we have in configure and tools/test.py -- would personally have gone with python2 but +1 for consistency.

[silverwind]

Oh, right. I have a python2 and a python2.7. python2 would also have been my choice, but whatever is consistent is fine.

[bnoordhuis]

LGTM with a nit and as long as it doesn't make moving to other tools in the future harder.

[jbergstroem]

My magic ball speaks of clang-format, but we'll see when that happens. Will fix the quotes (will I ever learn).

[jbergstroem]

As for moving into other tools forward, having TAP as a required output format (well, I think we can use junit from jenkins as well) seems reasonable.

cc @nodejs/build.

[joaocgreis]

So this prints one not ok for each problem found, may print more than one per file, but prints nothing if the file is ok. This was not obvious to me at first, maybe add a quick mention to the help output?

It's missing the TAP plan.

LGTM

[thefourtheye]

Any specific reason to use this instead of if _cpplint_state.output_format != 'tap':?

[jbergstroem]

nope

[rmg]

So this prints one not ok for each problem found, may print more than one per file, but prints nothing if the file is ok. This was not obvious to me at first, maybe add a quick mention to the help output?

That's not very useful. It makes it very hard to differentiate between a crash and a successful run if an empty file indicates success.

This should be easy enough to address with the same code that will be added to fix the lack of plan.

[bnoordhuis]

What's the status of this PR? Blocked on @rmg's comment?

[jbergstroem]

Not really. I think we should land and fix as we move forward. This patch will give us visibility (here are the fails) where there currently is pass or fail.

[thefourtheye]

Let's have a CI run and check the result?

[jbergstroem]

@thefourtheye won't be invoked through ci just now.

[thefourtheye]

As per the spec, it should have been version, no?

[jbergstroem]

I think all TAP is case insensitive, but yeah – the spec does use lowercase. I copied above from eslint output. Will update.

[thefourtheye]

this prints one not ok for each problem found, may print more than one per file, but prints nothing if the file is ok

This actually is kind of problematic for the TAP result parser I guess. We don't print the plan and we don't print the current test number as well. So, how will the parser keep track of the current test which failed?

[jbergstroem]

@thefourtheye it won't keep track of tests between runs, but it will report test failures with verbose info each run. lint failures are so rare; my take was that we'd rather get more output than no.

[thefourtheye]

Fair enough. Considering this affects only the cpp linter, LGTM. Do you have plans to extend this to test runner?

[jbergstroem]

@thefourtheye yeah. I've already got a branch i've been running tests on locally (and on my gh) so we can expose this as part of the test runner.

[thefourtheye]

@jbergstroem Awesome!