diff mbox series

t5703: replace "grep -a" usage by perl

Message ID 20200519005645.GD1999@danh.dev (mailing list archive)
State New, archived
Headers show
Series t5703: replace "grep -a" usage by perl | expand

Commit Message

Đoàn Trần Công Danh May 19, 2020, 12:56 a.m. UTC
On 2020-05-19 09:22:01+1000, Darren Tucker <dtucker@dtucker.net> wrote:
> On Tue, 19 May 2020 at 01:30, Đoàn Trần Công Danh <congdanhqx@gmail.com> wrote:
> [...]
> > > printf: \3: invalid escape
> >
> > Look like HP-UX's printf doesn't understand octal escape.
> 
> The HP-UX one is actually OK with that.  The error is from an old gnu
> coreutils (2.0), and it's complaining because there no leading zero,
> which POSIX says octal escapes have:
> https://pubs.opengroup.org/onlinepubs/9699919799/utilities/printf.html

I think it's better to use HP-UX native tools for the test.
Can you check with this patch applied on top of your tree.
-------------8<------------
From: =?UTF-8?q?=C4=90o=C3=A0n=20Tr=E1=BA=A7n=20C=C3=B4ng=20Danh?=
 <congdanhqx@gmail.com>
Date: Tue, 19 May 2020 07:50:46 +0700
Subject: [PATCH] t5703: replace "grep -a" usage by perl
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

On some platforms likes HP-UX, grep(1) doesn't understand "-a".

Let's switch to perl.

Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>
---

Notes:
    We can also copy-and-paste code from t4019,
    to avoid introduce perl to this test.

 t/t5703-upload-pack-ref-in-want.sh | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

Comments

Darren Tucker May 19, 2020, 2:22 a.m. UTC | #1
On Tue, 19 May 2020 at 10:56, Đoàn Trần Công Danh <congdanhqx@gmail.com> wrote:
> On 2020-05-19 09:22:01+1000, Darren Tucker <dtucker@dtucker.net> wrote:
> > On Tue, 19 May 2020 at 01:30, Đoàn Trần Công Danh <congdanhqx@gmail.com> wrote:
> > [...]
> > > > printf: \3: invalid escape
> > >
> > > Look like HP-UX's printf doesn't understand octal escape.
> >
> > The HP-UX one is actually OK with that.  The error is from an old gnu
> > coreutils (2.0), and it's complaining because there no leading zero,
> > which POSIX says octal escapes have:
> > https://pubs.opengroup.org/onlinepubs/9699919799/utilities/printf.html

I've replaced the printf in question.  You could add leading zeroes in
case anything else strictly conforms to POSIX but it looks like
there's quite a lot of them.

> I think it's better to use HP-UX native tools for the test.
> Can you check with this patch applied on top of your tree.
[...]
> On some platforms likes HP-UX, grep(1) doesn't understand "-a".

nit: "like HP-UX"

Yep that works:

$ PATH=/usr/local/bin:$PATH ./t5703-upload-pack-ref-in-want.sh
ok 1 - setup repository
ok 2 - config controls ref-in-want advertisement
ok 3 - invalid want-ref line
ok 4 - basic want-ref
ok 5 - multiple want-ref lines
ok 6 - mix want and want-ref
ok 7 - want-ref with ref we already have commit for
ok 8 - setup repos for fetching with ref-in-want tests
ok 9 - fetching with exact OID
ok 10 - fetching multiple refs
ok 11 - fetching ref and exact OID
ok 12 - fetching with wildcard that does not match any refs
ok 13 - fetching with wildcard that matches multiple refs
# passed all 13 test(s)
# SKIP skipping test, git built without http support
1..13

Thanks.
Christian Couder May 19, 2020, 7:13 a.m. UTC | #2
On Tue, May 19, 2020 at 3:00 AM Đoàn Trần Công Danh
<congdanhqx@gmail.com> wrote:

> On some platforms likes HP-UX, grep(1) doesn't understand "-a".
>
> Let's switch to perl.

There are some other platforms where Perl is difficult to install.
That's why we have the 'NO_PERL' Makefile knob, and the 'PERL' test
prereq which is used in some test scripts like t0021-conversion.sh,
t2016-checkout-patch.sh, t2071-restore-patch.sh, ...

>     We can also copy-and-paste code from t4019,
>     to avoid introduce perl to this test.

This might be a good idea.

>  t/t5703-upload-pack-ref-in-want.sh | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/t/t5703-upload-pack-ref-in-want.sh b/t/t5703-upload-pack-ref-in-want.sh
> index a34460f7d8..92ad5eeec0 100755
> --- a/t/t5703-upload-pack-ref-in-want.sh
> +++ b/t/t5703-upload-pack-ref-in-want.sh
> @@ -49,15 +49,18 @@ test_expect_success 'setup repository' '
>
>  test_expect_success 'config controls ref-in-want advertisement' '

At least you should add the PERL prereq to the test with something like:

test_expect_success PERL 'config controls ref-in-want advertisement' '

>         test-tool serve-v2 --advertise-capabilities >out &&
> -       ! grep -a ref-in-want out &&
> +       perl -ne "/ref-in-want/ and print" out >out.filter &&
> +       test_must_be_empty out.filter &&

Thanks,
Christian.
Đoàn Trần Công Danh May 19, 2020, 11:39 a.m. UTC | #3
On 2020-05-19 09:13:19+0200, Christian Couder <christian.couder@gmail.com> wrote:
> On Tue, May 19, 2020 at 3:00 AM Đoàn Trần Công Danh
> <congdanhqx@gmail.com> wrote:

> 
> >     We can also copy-and-paste code from t4019,
> >     to avoid introduce perl to this test.
> 
> This might be a good idea.

Probably we need 2 patches, one to libify the current usage in t4019,
one to replace "grep -a" usage in t5703.

Consider this is floating for a long time, I think we can hold this
for the time being, until after 2.27.0 released.

> >  t/t5703-upload-pack-ref-in-want.sh | 9 ++++++---
> >  1 file changed, 6 insertions(+), 3 deletions(-)
> >
> > diff --git a/t/t5703-upload-pack-ref-in-want.sh b/t/t5703-upload-pack-ref-in-want.sh
> > index a34460f7d8..92ad5eeec0 100755
> > --- a/t/t5703-upload-pack-ref-in-want.sh
> > +++ b/t/t5703-upload-pack-ref-in-want.sh
> > @@ -49,15 +49,18 @@ test_expect_success 'setup repository' '
> >
> >  test_expect_success 'config controls ref-in-want advertisement' '
> 
> At least you should add the PERL prereq to the test with something like:
> 
> test_expect_success PERL 'config controls ref-in-want advertisement' '

Well, that's is not t/README says:

 - PERL

   Git wasn't compiled with NO_PERL=YesPlease.

   Even without the PERL prerequisite, tests can assume there is a
   usable perl interpreter at $PERL_PATH, though it need not be
   particularly modern.

And, there're a lot of tests using perl without PERL prereq:

	$ git grep -l '[        ]perl ' 't/t????-*.sh' | wc -l
	55
	$ git grep -l PERL 't/t????-*.sh' | wc -l
	35
Christian Couder May 19, 2020, 2:13 p.m. UTC | #4
On Tue, May 19, 2020 at 1:39 PM Đoàn Trần Công Danh
<congdanhqx@gmail.com> wrote:
>
> On 2020-05-19 09:13:19+0200, Christian Couder <christian.couder@gmail.com> wrote:
> > On Tue, May 19, 2020 at 3:00 AM Đoàn Trần Công Danh
> > <congdanhqx@gmail.com> wrote:

> > >  test_expect_success 'config controls ref-in-want advertisement' '
> >
> > At least you should add the PERL prereq to the test with something like:
> >
> > test_expect_success PERL 'config controls ref-in-want advertisement' '
>
> Well, that's is not t/README says:
>
>  - PERL
>
>    Git wasn't compiled with NO_PERL=YesPlease.
>
>    Even without the PERL prerequisite, tests can assume there is a
>    usable perl interpreter at $PERL_PATH, though it need not be
>    particularly modern.
>
> And, there're a lot of tests using perl without PERL prereq:
>
>         $ git grep -l '[        ]perl ' 't/t????-*.sh' | wc -l
>         55
>         $ git grep -l PERL 't/t????-*.sh' | wc -l
>         35

Ok, sorry about the noise then.
diff mbox series

Patch

diff --git a/t/t5703-upload-pack-ref-in-want.sh b/t/t5703-upload-pack-ref-in-want.sh
index a34460f7d8..92ad5eeec0 100755
--- a/t/t5703-upload-pack-ref-in-want.sh
+++ b/t/t5703-upload-pack-ref-in-want.sh
@@ -49,15 +49,18 @@  test_expect_success 'setup repository' '
 
 test_expect_success 'config controls ref-in-want advertisement' '
 	test-tool serve-v2 --advertise-capabilities >out &&
-	! grep -a ref-in-want out &&
+	perl -ne "/ref-in-want/ and print" out >out.filter &&
+	test_must_be_empty out.filter &&
 
 	git config uploadpack.allowRefInWant false &&
 	test-tool serve-v2 --advertise-capabilities >out &&
-	! grep -a ref-in-want out &&
+	perl -ne "/ref-in-want/ and print" out >out.filter &&
+	test_must_be_empty out.filter &&
 
 	git config uploadpack.allowRefInWant true &&
 	test-tool serve-v2 --advertise-capabilities >out &&
-	grep -a ref-in-want out
+	perl -ne "/ref-in-want/ and print" out >out.filter &&
+	test_file_not_empty out.filter
 '
 
 test_expect_success 'invalid want-ref line' '