threads / patch / 15303

patcht6013: Avoid using tac

Subject: [PATCH] t6013: Avoid using tac

## tl;dr

5 messages between Aug 31, 2008 and Sep 1, 2008. Diffs are folded; open one to read it.

replies: 4people: 4as markdown or json

Brian Gernhardt· Aug 31, 2008, 20:03 UTC · lore

Although tac is useful for testing for reversed output, it is not available everywhere. Replace uses of tac with hand-rolled expected files.

Signed-off-by: Brian Gernhardt <benji@silverinsanity.com>
---
 t/t6013-rev-list-reverse-parents.sh |   20 ++++++++++++++++----
 1 files changed, 16 insertions(+), 4 deletions(-)
Show changes to t/t6013-rev-list-reverse-parents.sh +16 −4
diff --git a/t/t6013-rev-list-reverse-parents.sh b/t/t6013-rev-list-reverse-parents.sh
index d294466..f023fae 100755
--- a/t/t6013-rev-list-reverse-parents.sh
+++ b/t/t6013-rev-list-reverse-parents.sh
@@ -23,17 +23,29 @@ test_expect_success 'set up --reverse example' '
 	commit five
 	'
 
+cat >expected <<\EOF
+86412ffbe95da677fa145e05bcedc75d0d4e49f7
+4931c530d1fde996a29be48a97a056f13ab5e9ce 86412ffbe95da677fa145e05bcedc75d0d4e49f7
+0fcb03eaefb4569dea16195a244d95523cff4934 86412ffbe95da677fa145e05bcedc75d0d4e49f7
+524efe5b12a180964d586b6c47314ea015a98614 4931c530d1fde996a29be48a97a056f13ab5e9ce 0fcb03eaefb4569dea16195a244d95523cff4934
+2ed4a763ad375916b4f0fc34ed08f5843299ded6 524efe5b12a180964d586b6c47314ea015a98614
+EOF
+
 test_expect_success '--reverse --parents --full-history combines correctly' '
-	git rev-list --parents --full-history master -- foo |
-		tac > expected &&
 	git rev-list --reverse --parents --full-history master -- foo \
 		> actual &&
 	test_cmp actual expected
 	'
 
+cat >expected <<\EOF
+-86412ffbe95da677fa145e05bcedc75d0d4e49f7
+4931c530d1fde996a29be48a97a056f13ab5e9ce 86412ffbe95da677fa145e05bcedc75d0d4e49f7
+0fcb03eaefb4569dea16195a244d95523cff4934 86412ffbe95da677fa145e05bcedc75d0d4e49f7
+524efe5b12a180964d586b6c47314ea015a98614 4931c530d1fde996a29be48a97a056f13ab5e9ce 0fcb03eaefb4569dea16195a244d95523cff4934
+2ed4a763ad375916b4f0fc34ed08f5843299ded6 524efe5b12a180964d586b6c47314ea015a98614
+EOF
+
 test_expect_success '--boundary does too' '
-	git rev-list --boundary --parents --full-history master ^root -- foo |
-		tac > expected &&
 	git rev-list --boundary --reverse --parents --full-history \
 		master ^root -- foo > actual &&
 	test_cmp actual expected

-- 
1.6.0.1.207.g020e5
Thomas Rast· Aug 31, 2008, 22:31 UTC · re: Brian Gernhardt · lore

[PATCH] t6013: replace use of 'tac' with equivalent Perl

'tac' is not available everywhere, so substitute the equivalent Perl code 'print reverse <>'. Noticed by Brian Gernhardt.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---

Thanks for pointing this out. However, I tried to avoid hardcoding those results by recommendation of t/README (last paragraph):

  ... If all the test scripts hardcoded the object IDs like
  t0000-basic.sh does, that defeats the purpose of t0000-basic.sh,
  which is to isolate that level of validation in one place.  Your
  test also ends up needing updating when such a change to the
  internal happens, so do _not_ do it and leave the low level of
  validation to t0000-basic.sh.

So I would favour this fix. I think this should be ok because we depend on Perl anyway.

- Thomas
 t/t6013-rev-list-reverse-parents.sh |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
Show changes to t/t6013-rev-list-reverse-parents.sh +2 −2
diff --git a/t/t6013-rev-list-reverse-parents.sh b/t/t6013-rev-list-reverse-parents.sh
index d294466..59fc2f0 100755
--- a/t/t6013-rev-list-reverse-parents.sh
+++ b/t/t6013-rev-list-reverse-parents.sh
@@ -25,7 +25,7 @@ test_expect_success 'set up --reverse example' '
 
 test_expect_success '--reverse --parents --full-history combines correctly' '
 	git rev-list --parents --full-history master -- foo |
-		tac > expected &&
+		perl -e "print reverse <>" > expected &&
 	git rev-list --reverse --parents --full-history master -- foo \
 		> actual &&
 	test_cmp actual expected
@@ -33,7 +33,7 @@ test_expect_success '--reverse --parents --full-history combines correctly' '
 
 test_expect_success '--boundary does too' '
 	git rev-list --boundary --parents --full-history master ^root -- foo |
-		tac > expected &&
+		perl -e "print reverse <>" > expected &&
 	git rev-list --boundary --reverse --parents --full-history \
 		master ^root -- foo > actual &&
 	test_cmp actual expected

-- 
1.6.0.1.282.g3cc57
Andreas Ericsson· Sep 1, 2008, 12:56 UTC · re: Thomas Rast · lore

Re: [PATCH] t6013: replace use of 'tac' with equivalent Perl

Thomas Rast wrote:
Show 19 quoted lines
> 'tac' is not available everywhere, so substitute the equivalent Perl
> code 'print reverse <>'.  Noticed by Brian Gernhardt.
> 
> Signed-off-by: Thomas Rast <trast@student.ethz.ch>
> ---
> 
> Thanks for pointing this out.  However, I tried to avoid hardcoding
> those results by recommendation of t/README (last paragraph):
> 
>   ... If all the test scripts hardcoded the object IDs like
>   t0000-basic.sh does, that defeats the purpose of t0000-basic.sh,
>   which is to isolate that level of validation in one place.  Your
>   test also ends up needing updating when such a change to the
>   internal happens, so do _not_ do it and leave the low level of
>   validation to t0000-basic.sh.
> 
> So I would favour this fix.  I think this should be ok because we
> depend on Perl anyway.
> 
If it isn't, you could always do
	sed '1!G;h;$!d'
or
	sed -n '1!G;h;$p'
instead.
Both of them are very portable indeed.
-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231
Brian Gernhardt· Sep 1, 2008, 16:05 UTC · re: Andreas Ericsson · lore

Re: [PATCH] t6013: replace use of 'tac' with equivalent Perl

On Sep 1, 2008, at 8:56 AM, Andreas Ericsson wrote:
Show 11 quoted lines
> Thomas Rast wrote:
>> So I would favour this fix.  I think this should be ok because we
>> depend on Perl anyway.
>
> If it isn't, you could always do
> 	sed '1!G;h;$!d'
> or
> 	sed -n '1!G;h;$p'
> instead.
>
> Both of them are very portable indeed.

I am reminded of the last time I tried to remove a use of tac from git (rebase -i, to be specific). I was yelled at for trying to use perl, and "sed -ne '1!G;$p;h'" turned out to be not portable enough. Eventually the command was re-written not to need the reversal.

Junio seems to think perl is just fine, judging by next, so I guess I'll just give up trying to second guess what the list wants. :-/

~~ Brian
Johannes Schindelin· Sep 1, 2008, 16:32 UTC · re: Brian Gernhardt · lore

Re: [PATCH] t6013: replace use of 'tac' with equivalent Perl

Hi,
On Mon, 1 Sep 2008, Brian Gernhardt wrote:
> I am reminded of the last time I tried to remove a use of tac from git 
> (rebase -i, to be specific).  I was yelled at for trying to use perl, 
> and "sed -ne '1!G;$p;h'" turned out to be not portable enough.

Please note that using Perl in test scripts is considered less bad than using Perl in a shell script which is part of Git's core.

Ciao, Dscho

← back to recent threads