Re: [PATCH v2 01/18] maintenance: create basic maintenance runner

On Mon, Aug 03, 2020 at 04:01:34PM -0700, Jonathan Nieder wrote:

Taylor Blau wrote:

quoted

On Mon, Aug 03, 2020 at 10:46:54AM -0700, Jonathan Nieder wrote:

quoted

Derrick Stolee wrote:

quoted

On 7/30/2020 8:30 PM, Jonathan Nieder wrote:

quoted

Derrick Stolee wrote:

quoted

quoted

quoted

quoted

quoted

If there is a better way to ask "Did my command call 'git gc' (with
no arguments|with these arguments)?" then I'm happy to consider it.

My proposal was just to factor this out into a function in
test-lib-functions.sh so it's easy to evolve over time in one place.

This is a valuable suggestion, but this series is already too large
to make such a change in addition to the patches already here.

Hm, it's not clear to me that this would make the series significantly
larger.

I think what Stolee is trying to say is less about a change that would
make the series larger, it's about changing an already-large series.

quoted

And on the contrary, it would make the code less fragile.  I think this
is important.

I'm not sure that I see your argument. What we are really discussing is
whether or not we should have a static struct outside of 'cmd_gc()', or
a zero-initialized frame local struct within 'cmd_gc()'. I fully
understand the arguments in favor of one or the other, but I struggle to
grasp that this is worth our time to debate in this much detail.

Sorry for the lack of clarity.  The proposal Stolee is pushing back on
above is to have a helper in test-lib-functions.sh that tells a test
whether an event it cares about happened in traces (in this example,
invocation of "git gc").

Serves me right for reading too quickly ;).

I consider it important because if we are getting into the habit of
having our tests assume the current exact byte-for-byte trace JSON
output, then that is an assumption that is going to be painful to
unravel.  Factoring out a helper would also make the test easier to
read, but that is less important than "an ounce of prevention is worth
a pound of cure" to me in this example.

I still only partially buy into this, though. I had to deal with some
rather fragile grepping through trace2 JSON in t4216 (? or something,
the log-bloom tests) recently, and found it a little fragile, but not
overly so.

I'd rather just move forward, but I do feel strongly that we discuss
these matters on the list.

Jonathan

Thanks,
Taylor