t3302-notes-index-expensive.sh and t3419-rebase-patch-id.sh need time in /usr/bin

t3302-notes-index-expensive.sh and t3419-rebase-patch-id.sh need time in /usr/bin

[Joachim Schmitz]

Hi folks

t3302-notes-index-expensive.sh and t3419-rebase-patch-id.sh need "time " to 
be in "/usr/bin", however on my system it is in "/bin".

Can't this be checked for?

Bye, Jojo 

[PATCH 0/4] Allow different time commands

[Michael J Gruber]

This mini series enables the use of the lazy prereq framework for tests
which need a (non-builtin) time command.

1/4 is a bugfix
2/4 allows lazy prereq test scripts to set variables for later use
3/4 implements the prereq
4/4 uses it

This should solve the problem reported by Joachim Schmitz.

3/4 could be done in various different ways, see the comments there.

Michael J Gruber (4):
  t3419-rebase-patch-id: heed USR_BIN_TIME prereq
  test-lib: allow variable export from lazy prereq tests
  test-lib: provide lazy TIME_COMMAND prereq
  t3302,t3419: use the TIME_COMMAND prereq

 t/t3302-notes-index-expensive.sh | 5 ++---
 t/t3419-rebase-patch-id.sh       | 7 +++----
 t/test-lib-functions.sh          | 6 ++++--
 t/test-lib.sh                    | 7 +++++++
 4 files changed, 16 insertions(+), 9 deletions(-)

-- 
1.8.0.rc2.304.g9f3ac5c

[PATCH 1/4] t3419-rebase-patch-id: heed USR_BIN_TIME prereq

[Michael J Gruber]

t3419 sets the t3419-rebase-patch-id.sh prereq based on the availability
of /usr/bin/time but calls the binary unconditionally (in debug mode).

Make it run the timing only when the prereq is matched.

Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
---
 t/t3419-rebase-patch-id.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t3419-rebase-patch-id.sh b/t/t3419-rebase-patch-id.sh
index e70ac10..52af547 100755
--- a/t/t3419-rebase-patch-id.sh
+++ b/t/t3419-rebase-patch-id.sh
@@ -69,7 +69,7 @@ do_tests()
 		git cherry-pick master >/dev/null 2>&1
 	"
 
-	test_debug "
+	test_have_prereq USR_BIN_TIME && test_debug "
 		run git diff master^\!
 	"
 
@@ -77,7 +77,7 @@ do_tests()
 		echo 'file binary' >.gitattributes
 	"
 
-	test_debug "
+	test_have_prereq USR_BIN_TIME && test_debug "
 		run git format-patch --stdout master &&
 		run git format-patch --stdout --ignore-if-in-upstream master
 	"
-- 
1.8.0.rc2.304.g9f3ac5c

Re: [PATCH 1/4] t3419-rebase-patch-id: heed USR_BIN_TIME prereq

[Junio C Hamano]

Michael J Gruber <git@drmicha.warpmail.net> writes:

> t3419 sets the t3419-rebase-patch-id.sh prereq based on the availability
> of /usr/bin/time but calls the binary unconditionally (in debug mode).
>
> Make it run the timing only when the prereq is matched.
>
> Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>

I do not think we should ship our tests with too many test_debug in
the first place (it is something you as a developer who are trying
to figure out why your change broke tests can insert into tests).
It might be useful to be able to say "sh ./t1234-*.sh -d" and see
debug output that somebody else thought that it might be useful, so
I wouldn't say we should remove all existing test_debug, but at the
same time, if a developer finds existing test_debug does not work
for him (either what the debug output gives him is insufficient for
his needs, or what the debug command uses is not available on his
system), he should be willing to update (and capable of doing so) it
to suit his needs.  Adding prereq in front of test_debug is simply
an act of insanity.

For this particular one, I think this should suffice.  If the shell
does not have "time" built-in, then timeme can be set to even an
empty string, but that is left as an exercise to the readers.

 t/t3419-rebase-patch-id.sh | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git i/t/t3419-rebase-patch-id.sh w/t/t3419-rebase-patch-id.sh
index e70ac10..bf2736a 100755
--- i/t/t3419-rebase-patch-id.sh
+++ w/t/t3419-rebase-patch-id.sh
@@ -6,7 +6,11 @@ test_description='git rebase - test patch id computation'
 
 test_set_prereq NOT_EXPENSIVE
 test -n "$GIT_PATCHID_TIMING_TESTS" && test_set_prereq EXPENSIVE
-test -x /usr/bin/time && test_set_prereq USR_BIN_TIME
+if test -x /usr/bin/time
+	timeme=/usr/bin/time
+else
+	timeme=time
+fi
 
 count()
 {
@@ -35,7 +39,7 @@ scramble()
 run()
 {
 	echo \$ "$@"
-	/usr/bin/time "$@" >/dev/null
+	$timeme "$@" >/dev/null
 }
 
 test_expect_success 'setup' '

[PATCH 2/4] test-lib: allow variable export from lazy prereq tests

[Michael J Gruber]

Currently, lazy prereq tests are run in a subshell which communicates
only the exit code to the outer world.

Run it as a subcommand so that variables can be exported to the test
environment.

Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
---
I don't think this has any adverse side effects, but I'm begging for
another set of eyeballs to have a look. (Test suite passes, of course.)

 t/test-lib-functions.sh | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 8889ba5..e587902 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -246,13 +246,15 @@ test_lazy_prereq () {
 test_run_lazy_prereq_ () {
 	script='
 mkdir -p "$TRASH_DIRECTORY/prereq-test-dir" &&
-(
+{
 	cd "$TRASH_DIRECTORY/prereq-test-dir" &&'"$2"'
-)'
+}'
 	say >&3 "checking prerequisite: $1"
 	say >&3 "$script"
+	orig_pwd="$(pwd)"
 	test_eval_ "$script"
 	eval_ret=$?
+	cd "$orig_pwd"
 	rm -rf "$TRASH_DIRECTORY/prereq-test-dir"
 	if test "$eval_ret" = 0; then
 		say >&3 "prerequisite $1 ok"
-- 
1.8.0.rc2.304.g9f3ac5c

Re: [PATCH 2/4] test-lib: allow variable export from lazy prereq tests

[Junio C Hamano]

Michael J Gruber <git@drmicha.warpmail.net> writes:

> I don't think this has any adverse side effects, but I'm begging for
> another set of eyeballs to have a look. (Test suite passes, of course.)

The lazy prereqs are designed to be used lazily, in any test that he
who wrote lazy-prereq did not anticipate.  It is run inside a subshell
to make it absolutely sure that whatever it does (like use of shell
variables, chdir around) will not be able to affect _any_ calling
context that is not anticipated by who writes lazy prerequisite.

Please don't do this.

>
>  t/test-lib-functions.sh | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
> index 8889ba5..e587902 100644
> --- a/t/test-lib-functions.sh
> +++ b/t/test-lib-functions.sh
> @@ -246,13 +246,15 @@ test_lazy_prereq () {
>  test_run_lazy_prereq_ () {
>  	script='
>  mkdir -p "$TRASH_DIRECTORY/prereq-test-dir" &&
> -(
> +{
>  	cd "$TRASH_DIRECTORY/prereq-test-dir" &&'"$2"'
> -)'
> +}'
>  	say >&3 "checking prerequisite: $1"
>  	say >&3 "$script"
> +	orig_pwd="$(pwd)"
>  	test_eval_ "$script"
>  	eval_ret=$?
> +	cd "$orig_pwd"
>  	rm -rf "$TRASH_DIRECTORY/prereq-test-dir"
>  	if test "$eval_ret" = 0; then
>  		say >&3 "prerequisite $1 ok"

[RFC/PATCH 3/4] test-lib: provide lazy TIME_COMMAND prereq

[Michael J Gruber]

Some test want to use the time command (not the shell builtin) and test
for its availability at /usr/bin/time.

Provide a lazy prereq TIME_COMMAND which tests for /usr/bin/time and
/bin/time. If any is found, set TEST_COMMAND_PATH to the first match.

Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
---
Rather than iterating over 2 or more hardcoded paths, one could use
"test -P time" or allow a make variable TIME_COMMAND_PATH whose
executability is checked by the prereq. I really don't know what's best.

 t/test-lib.sh | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 489bc80..7977c15 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -738,6 +738,13 @@ test_lazy_prereq UTF8_NFD_TO_NFC '
 	esac
 '
 
+test_lazy_prereq TIME_COMMAND '
+	for command in /usr/bin/time /bin/time
+	do
+		test -x "$command" && break
+	done && TIME_COMMAND_PATH="$command"
+'
+
 # When the tests are run as root, permission tests will report that
 # things are writable when they shouldn't be.
 test -w / || test_set_prereq SANITY
-- 
1.8.0.rc2.304.g9f3ac5c

RE: [RFC/PATCH 3/4] test-lib: provide lazy TIME_COMMAND prereq

[Joachim Schmitz]

> From: Michael J Gruber [mailto:git@drmicha.warpmail.net]
> Sent: Tuesday, October 16, 2012 1:40 PM
> To: git@vger.kernel.org
> Cc: Joachim Schmitz; Junio C Hamano
> Subject: [RFC/PATCH 3/4] test-lib: provide lazy TIME_COMMAND prereq
> 
> Some test want to use the time command (not the shell builtin) and test
> for its availability at /usr/bin/time.
> 
> Provide a lazy prereq TIME_COMMAND which tests for /usr/bin/time and
> /bin/time. If any is found, set TEST_COMMAND_PATH to the first match.
> 
> Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
> ---
> Rather than iterating over 2 or more hardcoded paths, one could use
> "test -P time" or allow a make variable TIME_COMMAND_PATH whose

test -P time won't work for me:
test -P: unary operator expected

I do have another one in /usr/local/bin, maybe that could get added too?

> executability is checked by the prereq. I really don't know what's best.
> 
>  t/test-lib.sh | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index 489bc80..7977c15 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -738,6 +738,13 @@ test_lazy_prereq UTF8_NFD_TO_NFC '
>  	esac
>  '
> 
> +test_lazy_prereq TIME_COMMAND '
> +	for command in /usr/bin/time /bin/time
> +	do
> +		test -x "$command" && break
> +	done && TIME_COMMAND_PATH="$command"
> +'
> +
>  # When the tests are run as root, permission tests will report that
>  # things are writable when they shouldn't be.
>  test -w / || test_set_prereq SANITY
> --
> 1.8.0.rc2.304.g9f3ac5c

Re: [RFC/PATCH 3/4] test-lib: provide lazy TIME_COMMAND prereq

[Michael J Gruber]

Joachim Schmitz venit, vidit, dixit 16.10.2012 16:13:
>> From: Michael J Gruber [mailto:git@drmicha.warpmail.net]
>> Sent: Tuesday, October 16, 2012 1:40 PM
>> To: git@vger.kernel.org
>> Cc: Joachim Schmitz; Junio C Hamano
>> Subject: [RFC/PATCH 3/4] test-lib: provide lazy TIME_COMMAND prereq
>>
>> Some test want to use the time command (not the shell builtin) and test
>> for its availability at /usr/bin/time.
>>
>> Provide a lazy prereq TIME_COMMAND which tests for /usr/bin/time and
>> /bin/time. If any is found, set TEST_COMMAND_PATH to the first match.
>>
>> Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
>> ---
>> Rather than iterating over 2 or more hardcoded paths, one could use
>> "test -P time" or allow a make variable TIME_COMMAND_PATH whose
> 
> test -P time won't work for me:
> test -P: unary operator expected
> 
> I do have another one in /usr/local/bin, maybe that could get added too?

Yikes.

If we introduce a make variable TIME_COMMAND_PATH we can even get rid of
2/4 (but have to change Makefile or t/Makefile).

>> executability is checked by the prereq. I really don't know what's best.
>>
>>  t/test-lib.sh | 7 +++++++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/t/test-lib.sh b/t/test-lib.sh
>> index 489bc80..7977c15 100644
>> --- a/t/test-lib.sh
>> +++ b/t/test-lib.sh
>> @@ -738,6 +738,13 @@ test_lazy_prereq UTF8_NFD_TO_NFC '
>>  	esac
>>  '
>>
>> +test_lazy_prereq TIME_COMMAND '
>> +	for command in /usr/bin/time /bin/time
>> +	do
>> +		test -x "$command" && break
>> +	done && TIME_COMMAND_PATH="$command"
>> +'
>> +
>>  # When the tests are run as root, permission tests will report that
>>  # things are writable when they shouldn't be.
>>  test -w / || test_set_prereq SANITY
>> --
>> 1.8.0.rc2.304.g9f3ac5c
> 

RE: [RFC/PATCH 3/4] test-lib: provide lazy TIME_COMMAND prereq

[Joachim Schmitz]

> From: Michael J Gruber [mailto:git@drmicha.warpmail.net]
> Sent: Tuesday, October 16, 2012 5:07 PM
> To: Joachim Schmitz
> Cc: git@vger.kernel.org; 'Junio C Hamano'
> Subject: Re: [RFC/PATCH 3/4] test-lib: provide lazy TIME_COMMAND prereq
> 
> Joachim Schmitz venit, vidit, dixit 16.10.2012 16:13:
> >> From: Michael J Gruber [mailto:git@drmicha.warpmail.net]
> >> Sent: Tuesday, October 16, 2012 1:40 PM
> >> To: git@vger.kernel.org
> >> Cc: Joachim Schmitz; Junio C Hamano
> >> Subject: [RFC/PATCH 3/4] test-lib: provide lazy TIME_COMMAND prereq
> >>
> >> Some test want to use the time command (not the shell builtin) and test
> >> for its availability at /usr/bin/time.
> >>
> >> Provide a lazy prereq TIME_COMMAND which tests for /usr/bin/time and
> >> /bin/time. If any is found, set TEST_COMMAND_PATH to the first match.
> >>
> >> Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
> >> ---
> >> Rather than iterating over 2 or more hardcoded paths, one could use
> >> "test -P time" or allow a make variable TIME_COMMAND_PATH whose
> >
> > test -P time won't work for me:
> > test -P: unary operator expected
> >
> > I do have another one in /usr/local/bin, maybe that could get added too?
> 
> Yikes.
> 
> If we introduce a make variable TIME_COMMAND_PATH we can even get rid of
> 2/4 (but have to change Makefile or t/Makefile).

I don't mind too much. /usr/bin/time and /bin/time should be enough.

[RFC/PATCH 3/4v2] test-lib: provide lazy TIME_COMMAND prereq

[Michael J Gruber]

Some test want to use the time command (not the shell builtin) and test
for its availability at /usr/bin/time.

Provide a lazy prereq TIME_COMMAND which tests for $TEST_COMMAND_PATH,
which can be set from config.mak. It defaults to /usr/bin/time.

Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
---
Here's what the make variable version could look like.
1/4 and 4/4 would stay as is, 2/4 could be dropped.

 t/Makefile    | 5 +++--
 t/test-lib.sh | 4 ++++
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/t/Makefile b/t/Makefile
index 88e289f..52b4039 100644
--- a/t/Makefile
+++ b/t/Makefile
@@ -13,6 +13,7 @@ TAR ?= $(TAR)
 RM ?= rm -f
 PROVE ?= prove
 DEFAULT_TEST_TARGET ?= test
+TIME_COMMAND_PATH ?= /usr/bin/time
 
 # Shell quote;
 SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))
@@ -27,11 +28,11 @@ test: pre-clean $(TEST_LINT)
 	$(MAKE) aggregate-results-and-cleanup
 
 prove: pre-clean $(TEST_LINT)
-	@echo "*** prove ***"; GIT_CONFIG=.git/config $(PROVE) --exec '$(SHELL_PATH_SQ)' $(GIT_PROVE_OPTS) $(T) :: $(GIT_TEST_OPTS)
+	@echo "*** prove ***"; GIT_CONFIG=.git/config TIME_COMMAND_PATH=$(TIME_COMMAND_PATH) $(PROVE) --exec '$(SHELL_PATH_SQ)' $(GIT_PROVE_OPTS) $(T) :: $(GIT_TEST_OPTS)
 	$(MAKE) clean-except-prove-cache
 
 $(T):
-	@echo "*** $@ ***"; GIT_CONFIG=.git/config '$(SHELL_PATH_SQ)' $@ $(GIT_TEST_OPTS)
+	@echo "*** $@ ***"; GIT_CONFIG=.git/config TIME_COMMAND_PATH=$(TIME_COMMAND_PATH) '$(SHELL_PATH_SQ)' $@ $(GIT_TEST_OPTS)
 
 pre-clean:
 	$(RM) -r test-results
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 489bc80..173eb13 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -738,6 +738,10 @@ test_lazy_prereq UTF8_NFD_TO_NFC '
 	esac
 '
 
+test_lazy_prereq TIME_COMMAND '
+	test -x "$TIME_COMMAND_PATH"
+'
+
 # When the tests are run as root, permission tests will report that
 # things are writable when they shouldn't be.
 test -w / || test_set_prereq SANITY
-- 
1.8.0.rc2.304.g9f3ac5c

Re: [RFC/PATCH 3/4v2] test-lib: provide lazy TIME_COMMAND prereq

[Andreas Schwab]

Michael J Gruber <git@drmicha.warpmail.net> writes:

> Some test want to use the time command (not the shell builtin) and test
> for its availability at /usr/bin/time.

An alternative way to suppress the builtin meaning is to quote it, like
\time.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [RFC/PATCH 3/4v2] test-lib: provide lazy TIME_COMMAND prereq

[Stefano Lattarini]

Hi Andreas.  I hope you don't mind my nitpickiness, but ...

On 10/16/2012 06:28 PM, Andreas Schwab wrote:
> Michael J Gruber <git@drmicha.warpmail.net> writes:
> 
>> Some test want to use the time command (not the shell builtin) and test
>> for its availability at /usr/bin/time.
> 
> An alternative way to suppress the builtin meaning is to quote it, like
> \time.
>
... to be 100% precise, this quoting trick works because 'time' is a
shell keyword, rather than a builtin:

    $ type time
    time is a shell keyword

Regards,
  Stefano

Re: [RFC/PATCH 3/4v2] test-lib: provide lazy TIME_COMMAND prereq

[Johannes Sixt]

Am 16.10.2012 17:07, schrieb Michael J Gruber:
> Some test want to use the time command (not the shell builtin) and test
> for its availability at /usr/bin/time.
> 
> Provide a lazy prereq TIME_COMMAND which tests for $TEST_COMMAND_PATH,
> which can be set from config.mak. It defaults to /usr/bin/time.

This avoids the builtin:

	command time $that_command

It works for bash, ksh, zsh, and dash (where the latter doesn't have it
as builtin).

-- Hannes

Re: [RFC/PATCH 3/4v2] test-lib: provide lazy TIME_COMMAND prereq

[Junio C Hamano]

Johannes Sixt <j6t@kdbg.org> writes:

> Am 16.10.2012 17:07, schrieb Michael J Gruber:
>> Some test want to use the time command (not the shell builtin) and test
>> for its availability at /usr/bin/time.
>> 
>> Provide a lazy prereq TIME_COMMAND which tests for $TEST_COMMAND_PATH,
>> which can be set from config.mak. It defaults to /usr/bin/time.
>
> This avoids the builtin:
>
> 	command time $that_command
>
> It works for bash, ksh, zsh, and dash (where the latter doesn't have it
> as builtin).

"command time" works but I think that is because it is not a
built-in ;-)

Here is what I read in bash(1):

       command [-pVv] command [arg ...]  Run command with args
              suppressing the normal shell function lookup. Only
              builtin commands or commands found in the PATH are
              executed.

Taken together with this from "COMMAND EXECUTION":

       If the command name contains no slashes, the shell attempts
       to locate it.  If there exists a shell function by that name,
       that function is invoked as described above in FUNCTIONS.  If
       the name does not match a function, the shell searches for it
       in the list of shell builtins.  If a match is found, that
       builtin is invoked.

       If the name is neither a shell function nor a builtin, and
       contains no slashes, bash searches each element of the PATH
       for a directory containing an executable file by that name.

I suspect "command printf 'a b c\n'" would not use $HOME/bin/printf
even when I have $HOME/bin early in my $PATH (nor /usr/bin/printf
for that matter).

Re: [RFC/PATCH 3/4] test-lib: provide lazy TIME_COMMAND prereq

[Junio C Hamano]

Michael J Gruber <git@drmicha.warpmail.net> writes:

> Some test want to use the time command (not the shell builtin) and test
> for its availability at /usr/bin/time.

As we have t/perf these days, I suspect that we should aim to remove
these uses of /usr/bin/time in the main testsuite instead.

The one in 3419 was "run this command, and while at it run it under
'time'" but it is only inside test_debug and I do not think it gives
anything more than curiosity values.

The one in 3302 is used to compare two runs (one without and one
with notes tree), so it has a little more value than just curiosity,
but its value inside the main test suite is still highly dubious. It
does not have any "under this value the test passes" criteria.

When the performance of having to look up notes tree really matters,
it shouldn't be done inside the main test suite that is designed to
be runnable unattended and the only check the humans do is to see
their "ok/fail" output.

In short, what you would get out of /usr/bin/time simply is not a
good match inside the context of these unit tests.

[PATCH 4/4] t3302,t3419: use the TIME_COMMAND prereq

[Michael J Gruber]

Use the TIME_COMMAND prereq in both tests so that time from several
paths can be used.

Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
---
 t/t3302-notes-index-expensive.sh | 5 ++---
 t/t3419-rebase-patch-id.sh       | 7 +++----
 2 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/t/t3302-notes-index-expensive.sh b/t/t3302-notes-index-expensive.sh
index e35d781..3d9f37e 100755
--- a/t/t3302-notes-index-expensive.sh
+++ b/t/t3302-notes-index-expensive.sh
@@ -9,7 +9,6 @@ test_description='Test commit notes index (expensive!)'
 
 test_set_prereq NOT_EXPENSIVE
 test -n "$GIT_NOTES_TIMING_TESTS" && test_set_prereq EXPENSIVE
-test -x /usr/bin/time && test_set_prereq USR_BIN_TIME
 
 create_repo () {
 	number_of_commits=$1
@@ -96,7 +95,7 @@ time_notes () {
 	for mode in no-notes notes
 	do
 		echo $mode
-		/usr/bin/time "$SHELL_PATH" ../time_notes $mode $1
+		"$TIME_COMMAND_PATH" "$SHELL_PATH" ../time_notes $mode $1
 	done
 }
 
@@ -113,7 +112,7 @@ do_tests () {
 
 	test_expect_success $pr 'notes work' "test_notes $count"
 
-	test_expect_success USR_BIN_TIME,$pr 'notes timing with /usr/bin/time' "time_notes 100"
+	test_expect_success TIME_COMMAND,$pr 'notes timing with a time command' "time_notes 100"
 
 	test_expect_success $pr 'teardown / cd ..' 'cd ..'
 }
diff --git a/t/t3419-rebase-patch-id.sh b/t/t3419-rebase-patch-id.sh
index 52af547..c7d90a6 100755
--- a/t/t3419-rebase-patch-id.sh
+++ b/t/t3419-rebase-patch-id.sh
@@ -6,7 +6,6 @@ test_description='git rebase - test patch id computation'
 
 test_set_prereq NOT_EXPENSIVE
 test -n "$GIT_PATCHID_TIMING_TESTS" && test_set_prereq EXPENSIVE
-test -x /usr/bin/time && test_set_prereq USR_BIN_TIME
 
 count()
 {
@@ -35,7 +34,7 @@ scramble()
 run()
 {
 	echo \$ "$@"
-	/usr/bin/time "$@" >/dev/null
+	"$TIME_COMMAND_PATH" "$@" >/dev/null
 }
 
 test_expect_success 'setup' '
@@ -69,7 +68,7 @@ do_tests()
 		git cherry-pick master >/dev/null 2>&1
 	"
 
-	test_have_prereq USR_BIN_TIME && test_debug "
+	test_have_prereq TIME_COMMAND && test_debug "
 		run git diff master^\!
 	"
 
@@ -77,7 +76,7 @@ do_tests()
 		echo 'file binary' >.gitattributes
 	"
 
-	test_have_prereq USR_BIN_TIME && test_debug "
+	test_have_prereq TIME_COMMAND && test_debug "
 		run git format-patch --stdout master &&
 		run git format-patch --stdout --ignore-if-in-upstream master
 	"
-- 
1.8.0.rc2.304.g9f3ac5c