Re: [PATCH v3 0/3] Add a JSON Schema for trace2 events

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Aug 2, 2019, 11:56 UTC
Message-ID: <nycvar.QRO.7.76.6.1908021347580.46@tvgsbejvaqbjf.bet>
In-Reply-To: <20190802015247.GA54514@google.com>

Hi Jonathan & Josh,

On Thu, 1 Aug 2019, Jonathan Nieder wrote:

Show 21 quoted lines

> Josh Steadmon wrote:
> > On 2019.07.26 15:03, Josh Steadmon wrote:
>
> >> [ajv-cli] can validate the full 1.7M line trace output in just over a
> >> minute. Moreover, it has helpful output when validation fails. So I
> >> would be happy to re-implement this using ajv-cli.
> >
> > Unfortunately, ajv on Travis is much slower than on my work machine. It
> > still takes over 10 minutes to complete, and is killed by Travis since
> > it doesn't provide any progress indicator while it's running.
>
> Alas.
>
> What do you think of making the validation disabled by default and
> using a parameter (see "Running tests with special setups" in
> t/README) to turn it on?  That way, it should be okay for it to take
> 10 minutes because this would only affect such specialized workers as
> choose to set that parameter, instead of all of them.
>
> Gábor, if we introduce such a parameter, do you think it would make
> sense for us to set up a worker that passes it?

Oh my.

What do we want? To validate the JSON output. Do we want to validate it in a smart way? Yes!

So how about adding a separate test script that activates Trace2, then runs a _few_ selected, very specific commands, starting with a set that Josh thinks representative, validate the generated JSON, and then only add new invocations if we *ever* see violations of the schema, to verify that we don't regress on that front again?

Such a script can't take more than a couple of seconds to run, I am sure. And it would totally strike a sensible balance between benefit and time spent.

Generating JSON output that respects the given schema is *such* a niche problem that I am really against punishing every person who wants to run the test suite by having to sit through 10 minutes, at the same time I do not want this feature to be totally untested by default, either.

We do _not_ have to validate the 10,001st instance of a `git commit`'s corresponding JSON. The 10k invocations before that validated fine, there is little doubt that this one, as well as the following 59,724 invocations won't violate the schema, either.

It's not hard to test smartly. Use a really small, representative sample.

I am usually against trying to be too smart, but in this case using a really small sample (my gut feeling says: at most a dozen) is Just Smart Enough.

Ciao, Johannes