git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jonathan Nieder <jrnieder@gmail.com>
To: Josh Steadmon <steadmon@google.com>
Cc: "Johannes Schindelin" <Johannes.Schindelin@gmx.de>,
	"SZEDER Gábor" <szeder.dev@gmail.com>,
	git@vger.kernel.org, gitster@pobox.com, git@jeffhostetler.com,
	avarab@gmail.com, peff@peff.net, jnareb@gmail.com
Subject: Re: [PATCH v3 0/3] Add a JSON Schema for trace2 events
Date: Thu, 1 Aug 2019 18:52:47 -0700	[thread overview]
Message-ID: <20190802015247.GA54514@google.com> (raw)
In-Reply-To: <20190801180829.GP43313@google.com>

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?

> 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.

> 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

  reply	other threads:[~2019-08-02  1:52 UTC|newest]

Thread overview: 47+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-06-11 23:31 [RFC PATCH 0/3] Add a JSON Schema for trace2 events Josh Steadmon
2019-06-11 23:31 ` [RFC PATCH 1/3] trace2: correct trace2 field name documentation Josh Steadmon
2019-06-12 18:00   ` Junio C Hamano
2019-06-12 18:14     ` Josh Steadmon
2019-06-14 15:53   ` Jeff Hostetler
2019-06-11 23:31 ` [RFC PATCH 2/3] trace2: Add a JSON schema for trace2 events Josh Steadmon
2019-06-14 15:59   ` Jeff Hostetler
2019-06-20 17:26     ` Josh Steadmon
2019-06-11 23:31 ` [RFC PATCH 3/3] trace2: add a schema validator " Josh Steadmon
2019-06-12 13:28   ` Ævar Arnfjörð Bjarmason
2019-06-12 16:23     ` Josh Steadmon
2019-06-12 19:18       ` Jeff King
2019-06-20 18:15         ` Josh Steadmon
2019-06-21 11:53       ` Jakub Narebski
2019-06-27 13:57         ` Jeff Hostetler
2019-07-09 23:05 ` [RFC PATCH v2 0/3] Add a JSON Schema " Josh Steadmon
2019-07-09 23:05   ` [RFC PATCH v2 1/3] trace2: Add a JSON schema " Josh Steadmon
2019-07-10 18:32     ` Jakub Narebski
2019-07-24 22:37       ` Josh Steadmon
2019-07-09 23:05   ` [RFC PATCH v2 2/3] trace2: add a schema validator " Josh Steadmon
2019-07-11 13:35     ` Jakub Narebski
2019-07-24 22:47       ` Josh Steadmon
2019-07-09 23:05   ` [RFC PATCH v2 3/3] ci: run trace2 schema validation in the CI suite Josh Steadmon
2019-07-24 23:06 ` [PATCH v3 0/3] Add a JSON Schema for trace2 events Josh Steadmon
2019-07-24 23:06   ` [PATCH v3 1/3] trace2: Add a JSON schema " Josh Steadmon
2019-07-25 16:55     ` Junio C Hamano
2019-07-24 23:06   ` [PATCH v3 2/3] trace2: add a schema validator " Josh Steadmon
2019-07-24 23:06   ` [PATCH v3 3/3] ci: run trace2 schema validation in the CI suite Josh Steadmon
2019-07-25 11:18   ` [PATCH v3 0/3] Add a JSON Schema for trace2 events SZEDER Gábor
2019-07-25 16:14     ` Junio C Hamano
2019-07-26 21:16       ` Josh Steadmon
2019-07-25 23:42   ` SZEDER Gábor
2019-07-26 12:12     ` Johannes Schindelin
2019-07-26 13:53       ` SZEDER Gábor
2019-07-31 11:00         ` Johannes Schindelin
2019-07-26 22:03       ` Josh Steadmon
2019-08-01 18:08         ` Josh Steadmon
2019-08-02  1:52           ` Jonathan Nieder [this message]
2019-08-02 11:56             ` Johannes Schindelin
2019-08-02 16:59               ` Jonathan Nieder
2019-08-02 19:38                 ` SZEDER Gábor
2019-08-02 23:25                   ` Jonathan Nieder
2019-08-03 21:25                     ` Johannes Schindelin
2019-08-02 19:16             ` SZEDER Gábor
2019-08-02 23:06               ` Jonathan Nieder
2019-08-03  7:35                 ` SZEDER Gábor
2019-08-03  7:40                   ` SZEDER Gábor

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190802015247.GA54514@google.com \
    --to=jrnieder@gmail.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=avarab@gmail.com \
    --cc=git@jeffhostetler.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jnareb@gmail.com \
    --cc=peff@peff.net \
    --cc=steadmon@google.com \
    --cc=szeder.dev@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).