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

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Aug 2, 2019, 01:52 UTC
Message-ID: <20190802015247.GA54514@google.com>
In-Reply-To: <20190801180829.GP43313@google.com>

Josh Steadmon wrote:

> On 2019.07.26 15:03, Josh Steadmon wrote:

Show 7 quoted lines

>> [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?

> How would people feel about validating a sample of the "make test"
> output? In the short term we could just use command-line tools to sample
> the trace file; for the long-term, we could add a sampling config option
> for trace2 (I've been considering adding this for other reasons anyway).

I kind of like the idea of a "make smoke" that runs some fast probabilistic tests for interactive use, but for CI runs I prefer the thoroughness of running the full testsuite. So for the problem at hand, I prefer making the test optional and reliable instead of fast and nondeterministic.

> Ideally we would want the sample to be deterministic for any given
> commit, so that we don't end up with flaky tests if changes are made to
> trace2 while neglecting to update the schema.

That would make it hard to bisect to track down bugs, so I don't like it as much.

Show 8 quoted lines

> Since there have been some suggestions to build a standalone test and
> verify its trace output, let me reiterate why I feel it's useful to use
> "make test" instead: I do not feel that I can create a standalone test
> that exercises a wide enough selection of code paths to get sufficient
> coverage of all potential trace2 output. Trying to make a standalone
> test that also anticipates future development is practically impossible.
> Using "make test" means that I can rely on the whole community to
> identify important code paths, both now and in the future.

I agree.

> As always, I am open to other approaches to make sure the schema stays
> up-to-date.

It sounds like the codegen approach would be a nice way to do that, but that doesn't need to block this step in the right direction.

Thanks for your thoughtfulness, Jonathan