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

[Junio C Hamano <gitster@pobox.com>]

"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:

> ... Note that this
> changes the expectations slightly. The old test (incorrectly) used two
> patterns for the 'grep' invocation, but this performs an OR of the
> patterns, not an AND. This means that as long as one region_enter event
> was logged, the test would succeed, even if it was not due to the
> progress category.

>  	# t0212/parse_events.perl intentionally omits regions and data.
> -	grep -e "region_enter" -e "\"category\":\"progress\"" trace.event &&
> -	grep -e "region_leave" -e "\"category\":\"progress\"" trace.event &&
> +	test_region progress "Working hard" trace.event &&

So we do want "region_enter" and the "category":"progress" on the
same line in the event file, but as long as "category":"progress"
exists, both will pass, regardless of enter/leave.  And ...

> +test_region () {
> +	local expect_exit=0
> +	if test "$1" = "!"
> +	then
> +		expect_exit=1
> +		shift
> +	fi
> +
> +	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 ...

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

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

	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.

> +	then
> +		return 1
> +	fi
> +
> +	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.

> +	exitcode=$?
> +
> +	if test $exitcode != $expect_exit
> +	then
> +		return 1
> +	fi
> +}