On Mon, Aug 03, 2020 at 04:01:34PM -0700, Jonathan Nieder wrote: > 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"). 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
