git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] t6013: Avoid using tac
@ 2008-08-31 20:03 Brian Gernhardt
  2008-08-31 22:31 ` [PATCH] t6013: replace use of 'tac' with equivalent Perl Thomas Rast
  0 siblings, 1 reply; 5+ messages in thread
From: Brian Gernhardt @ 2008-08-31 20:03 UTC (permalink / raw)
  To: Git List; +Cc: Junio C Hamano

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(-)

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

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH] t6013: replace use of 'tac' with equivalent Perl
  2008-08-31 20:03 [PATCH] t6013: Avoid using tac Brian Gernhardt
@ 2008-08-31 22:31 ` Thomas Rast
  2008-09-01 12:56   ` Andreas Ericsson
  0 siblings, 1 reply; 5+ messages in thread
From: Thomas Rast @ 2008-08-31 22:31 UTC (permalink / raw)
  To: git; +Cc: Brian Gernhardt, Junio C Hamano

'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(-)

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

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] t6013: replace use of 'tac' with equivalent Perl
  2008-08-31 22:31 ` [PATCH] t6013: replace use of 'tac' with equivalent Perl Thomas Rast
@ 2008-09-01 12:56   ` Andreas Ericsson
  2008-09-01 16:05     ` Brian Gernhardt
  0 siblings, 1 reply; 5+ messages in thread
From: Andreas Ericsson @ 2008-09-01 12:56 UTC (permalink / raw)
  To: Thomas Rast; +Cc: git, Brian Gernhardt, Junio C Hamano

Thomas Rast wrote:
> '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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] t6013: replace use of 'tac' with equivalent Perl
  2008-09-01 12:56   ` Andreas Ericsson
@ 2008-09-01 16:05     ` Brian Gernhardt
  2008-09-01 16:32       ` Johannes Schindelin
  0 siblings, 1 reply; 5+ messages in thread
From: Brian Gernhardt @ 2008-09-01 16:05 UTC (permalink / raw)
  To: Andreas Ericsson; +Cc: Thomas Rast, git, Junio C Hamano


On Sep 1, 2008, at 8:56 AM, Andreas Ericsson wrote:

> 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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] t6013: replace use of 'tac' with equivalent Perl
  2008-09-01 16:05     ` Brian Gernhardt
@ 2008-09-01 16:32       ` Johannes Schindelin
  0 siblings, 0 replies; 5+ messages in thread
From: Johannes Schindelin @ 2008-09-01 16:32 UTC (permalink / raw)
  To: Brian Gernhardt; +Cc: Andreas Ericsson, Thomas Rast, git, Junio C Hamano

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2008-09-01 16:28 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-08-31 20:03 [PATCH] t6013: Avoid using tac Brian Gernhardt
2008-08-31 22:31 ` [PATCH] t6013: replace use of 'tac' with equivalent Perl Thomas Rast
2008-09-01 12:56   ` Andreas Ericsson
2008-09-01 16:05     ` Brian Gernhardt
2008-09-01 16:32       ` Johannes Schindelin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).