Re: [PATCH v2 7/8] test-lib: test_region looks for trace2 regions

On 1/22/2021 2:42 PM, Junio C Hamano wrote:

"Derrick Stolee via GitGitGadget" [off-list ref] writes:

quoted

+	grep -e "\"region_enter\".*\"category\":\"$1\",\"label\":\"$2\"" "$3"

... this makes sure there is enter/category on a line (and
leave/category on a line with another check).  Makes sense.

But...

	test_region '!' '\(unmatching capture)' 'two' 'three'

would try to use an invalid regexp and cause grep to exit with 2,
which would mean ...

quoted

+	exitcode=$?
+
+	if test $exitcode != $expect_exit

... this will not trigger and we return "success" (i.e. "failed as
expected")?

Am I misunderstanding something here? If exitcode is 2, then this
will always trigger and return 1, signaling a failure. That would
propagate to the parent test and cause the test to fail. That seems
like the correct intention, but I'm not 100% confident about that.

	Clarification.  The point is *NOT* that the grep pattern is
	not robust against funnies in $1 and $2---after all, these
	strings are under our control.  The point is what should
	happen when "grep" exits with an error when asked to ensure
	that there is no region detected.

I'll be more robust to these in the next version. We'll expect
exit code equal to zero or _not_ equal to zero, depending on the
presence of '!'. This has the downside of returning success for
bad input strings when '!' is specified.

Basically, the approach I'm taking for v3 is here:

	if [test $expect_exit = 1] && [test $exitcode = 0]
	then
		return 1
	elif [test $expect_exit = 0] && [test $exitcode != 0]
	then
		return 1
	fi

quoted

+	grep -e "\"region_leave\".*\"category\":\"$1\",\"label\":\"$2\"" "$3"

The same comment on "what about an error from grep" applies to this
one.

It might be easier to read to avoid having to say too many
backslash-quoted double quotes:

	grep -e	'"region_leave".*"category":"'"$1"'","label":"'"$2"\" "$3"

This comment applies to the earlier "grep", too.

Thanks. This does look a bit cleaner.

-Stolee