Re: [PATCH v4 6/7] trace.c: print env vars in trace_run_command()

Stefan Beller [off-list ref] writes:

On Tue, Jan 16, 2018 at 2:13 PM, Junio C Hamano [off-list ref] wrote:

quoted

Nguyễn Thái Ngọc Duy  [off-list ref] writes:

quoted

+test_trace() {
+     local expected="$1"

Style: "test_trace () {" is how we start a shell function.
Portability: we do not use "local".

quoted

+     shift
+     GIT_TRACE=1 test-run-command "$@" run-command true 2>&1 >/dev/null | \
+             sed 's/.* run_command: //' >actual &&
+     echo "$expected true" >expected &&
+     test_cmp expected actual

Consistency: everybody else in the test script contrasts "actual" vs
"expect" (they happen to be of the same length ;-); don't say
expectED just to be different.

3491 entries in the t/ directory disagree with your imagination of
consistency. ;)

Heh.  I thought it was clear enough from the context that I was
talking about matching the remainder of the same test script; I do
not have time time to count 'test_cmp expected' in all others in t/
;-)

But I agree that we want to go for consistency, and most likely it's
best to go for 'expect'.

It is unclear if you are volunteering, but do not change things for
the sake of making them consistent.  Not adding new ones and fixing
existing ones when they need to change anyway for some other reason
is the way to do this without unnecessary code churn.