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

On 8/4/2020 10:42 AM, Jonathan Nieder wrote:

Derrick Stolee wrote:

quoted

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.

[...]

quoted

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.

[...]

quoted

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