Taylor Blau wrote: > On Mon, Aug 03, 2020 at 10:46:54AM -0700, Jonathan Nieder wrote: >> Derrick Stolee wrote: >>> On 7/30/2020 8:30 PM, Jonathan Nieder wrote: >>>> Derrick Stolee wrote: >>>>> 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. > >> 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"). 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. Jonathan
