On 8/4/2020 10:42 AM, Jonathan Nieder wrote:
> Derrick Stolee wrote:
>
>> What is seems like you are asking instead is for me to create a tool
>> in the test suite that parses each JSON line, extracts a specific
>> member from that JSON object, reconstructs a command-line invocation
>> from the JSON array, and reports whether that process worked for any
>> line in the event output.
>
> No, that isn't what I'm asking.
>
> I'm asking for this patch series to take the existing "grep" lines
> and put them in a function in test-lib-functions.sh, so that we can
> change them in one place when the trace emitter changes.
Thanks for clarifying, clearing up my misinterpretation of your
request. I'm coming around to this idea.
> [...]
>> If this is to truly be a hard requirement for these tests to move
>> forward,
>
> Yes, from my point of view it really is.
>
> But that "is this truly a hard requirement?" comes up tells me I have
> not done a good job of communicating in this review. A review is
> about participants in the project working together to improve a patch,
> not people making demands at each other.
It's also about a balance. A patch author and a reviewer can have
disagreements, and it is important to discover exactly how hard each
is holding to their opinion.
> [...]
>> If I'm to spend time engineering something more complicated just to
>> check "did this subcommand run with these arguments?" then
>
> I don't see why this is more complicated than what is in patch 1. In
> fact, I think it would be a little more simple.
My interpretation (parsing JSON) _was_ more complicated, hence my
reservations. The helper still requires non-trivial script-fu (for
someone like me who is bad at scripting languages) but centralizing
the grep has value.
Here is my attempt so far:
trace2_subcommand_data () {
command="$1" &&
commas=$(echo $command | sed s/\ /\",\"/g) &&
printf "[\"$commas\"]"
}
test_subcommand_called () {
data=$(trace2_subcommand_data $1) &&
grep $data $2
}
test_subcommand_not_called () {
! test_subcommand_called $1 $2
}
with callers looking like
test_subcommand_called "gc" run-no-auto.txt &&
test_subcommand_not_called "gc --auto" run-auto.txt &&
test_subcommand_called "gc --quiet" run-quiet.txt
Thanks,
-Stolee
