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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Michael J Gruber <git@xxxxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxxx>

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' '
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]