[RFC,1/1] testcase fail while using cross-test-ssh wrapper
diff mbox series

Message ID 6067f3ff4ac4b7e991405f216e928e62f4780c8b.1561620739.git.han_mao@c-sky.com
State New
Headers show
Series
  • [RFC,1/1] testcase fail while using cross-test-ssh wrapper
Related show

Commit Message

Mao Han June 27, 2019, 7:33 a.m. UTC
I did a regression test yesterday, and found tst-locale-locpath.sh and
tst-rtld-preload.sh failed due to some script problem. The ssh wrapper
is invoked twice on these testcase, seems some of the arguments are
not pass correctly for cross test.

tst-locale-locpath.sh test cmd:
/home/vmh/workspace/buildroot2/buildroot/output/build/glibc-02d8b5ab1c89bcef2627d2b621bfb35b573852c2/scripts/cross-test-ssh.sh --timeoutfactor 6000 10.0.2.17 /home/vmh/workspace/buildroot2/buildroot/output/build/glibc-02d8b5ab1c89bcef2627d2b621bfb35b573852c2/scripts/cross-test-ssh.sh --timeoutfactor 6000 10.0.2.17 env LC_ALL=invalid-locale LOCPATH=does-not-exist /home/vmh/workspace/buildroot2/buildroot/output/build/glibc-02d8b5ab1c89bcef2627d2b621bfb35b573852c2/build/elf/ld.so --library-path /home/vmh/workspace/buildroot2/buildroot/output/build/glibc-02d8b5ab1c89bcef2627d2b621bfb35b573852c2/build/ /home/vmh/workspace/buildroot2/buildroot/output/build/glibc-02d8b5ab1c89bcef2627d2b621bfb35b573852c2/build/locale/locale

Cc: Florian Weimer <fweimer@redhat.com>
Cc: David Newall <glibc@davidnewall.com>
Cc: Vincent Chen<vincentc@andestech.com>
---
 elf/tst-rtld-preload.sh      | 2 +-
 locale/tst-locale-locpath.sh | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

Comments

Florian Weimer July 8, 2019, 12:09 p.m. UTC | #1
* Mao Han:

> I did a regression test yesterday, and found tst-locale-locpath.sh and
> tst-rtld-preload.sh failed due to some script problem. The ssh wrapper
> is invoked twice on these testcase, seems some of the arguments are
> not pass correctly for cross test.
>
> tst-locale-locpath.sh test cmd:
> /home/vmh/workspace/buildroot2/buildroot/output/build/glibc-02d8b5ab1c89bcef2627d2b621bfb35b573852c2/scripts/cross-test-ssh.sh --timeoutfactor 6000 10.0.2.17 /home/vmh/workspace/buildroot2/buildroot/output/build/glibc-02d8b5ab1c89bcef2627d2b621bfb35b573852c2/scripts/cross-test-ssh.sh --timeoutfactor 6000 10.0.2.17 env LC_ALL=invalid-locale LOCPATH=does-not-exist /home/vmh/workspace/buildroot2/buildroot/output/build/glibc-02d8b5ab1c89bcef2627d2b621bfb35b573852c2/build/elf/ld.so --library-path /home/vmh/workspace/buildroot2/buildroot/output/build/glibc-02d8b5ab1c89bcef2627d2b621bfb35b573852c2/build/ /home/vmh/workspace/buildroot2/buildroot/output/build/glibc-02d8b5ab1c89bcef2627d2b621bfb35b573852c2/build/locale/locale
>
> Cc: Florian Weimer <fweimer@redhat.com>
> Cc: David Newall <glibc@davidnewall.com>
> Cc: Vincent Chen<vincentc@andestech.com>
> ---
>  elf/tst-rtld-preload.sh      | 2 +-
>  locale/tst-locale-locpath.sh | 4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/elf/tst-rtld-preload.sh b/elf/tst-rtld-preload.sh
> index f0c0ca1..0fe9e4f 100755
> --- a/elf/tst-rtld-preload.sh
> +++ b/elf/tst-rtld-preload.sh
> @@ -31,7 +31,7 @@ echo "# [${test_wrapper}] [$rtld] [--library-path] [$library_path]" \
>       "[--preload] [$preload] [$test_program]"
>  ${test_wrapper_env} \
>  ${run_program_env} \
> -${test_wrapper} $rtld --library-path "$library_path" \
> +$rtld --library-path "$library_path" \
>    --preload "$preload" $test_program 2>&1 && rc=0 || rc=$?
>  echo "# exit status $rc"
>  
> diff --git a/locale/tst-locale-locpath.sh b/locale/tst-locale-locpath.sh
> index b83de90..1281e9c 100644
> --- a/locale/tst-locale-locpath.sh
> +++ b/locale/tst-locale-locpath.sh
> @@ -20,8 +20,8 @@
>  set -ex
>  
>  common_objpfx=$1
> -test_wrapper_env=$2
> -run_program_env=$3
> +test_wrapper_env=$3
> +run_program_env=
>  
>  LIBPATH="$common_objpfx"

Sorry, this patch doesn't look correct to me.  Why is it appropriate to
ignore evironment variables passed to the test script?

Thanks,
Florian
Mao Han July 9, 2019, 1:30 a.m. UTC | #2
On Mon, Jul 08, 2019 at 02:09:13PM +0200, Florian Weimer wrote:
> * Mao Han:
> >  
> >  common_objpfx=$1
> > -test_wrapper_env=$2
> > -run_program_env=$3
> > +test_wrapper_env=$3
> > +run_program_env=
> >  
> >  LIBPATH="$common_objpfx"
> 
> Sorry, this patch doesn't look correct to me.  Why is it appropriate to
> ignore evironment variables passed to the test script?

I was not intend to ignore the evironment variables. I just posted the problem
I've got and how did I avoid that.
The argument passed by Makefile is:
$(objpfx)tst-locale-locpath.out : tst-locale-locpath.sh $(objpfx)locale
        $(SHELL) $< '$(common-objpfx)' '$(test-wrapper)' '$(test-wrapper-env)'
But the argument taken by the script is:
common_objpfx=$1
test_wrapper_env=$2
run_program_env=$3
The test-wrapper is passed twice and no run_program_env is passed. I don't know
whether the test-wrapper script is unnecessary(last patch) or the argument passed
by Makefile is wrong:

diff --git a/locale/Makefile b/locale/Makefile
index 0ad99ec..d78cf9b 100644
--- a/locale/Makefile
+++ b/locale/Makefile
@@ -113,5 +113,5 @@ lib := locale-programs
 include $(patsubst %,$(..)libof-iterator.mk,$(cpp-srcs-left))
 
 $(objpfx)tst-locale-locpath.out : tst-locale-locpath.sh $(objpfx)locale
-       $(SHELL) $< '$(common-objpfx)' '$(test-wrapper)' '$(test-wrapper-env)' > $@; \
+       $(SHELL) $< '$(common-objpfx)' '$(test-wrapper-env)' '$(run-program-env)' > $@; \
        $(evaluate-test)

Thanks,
Mao Han
Florian Weimer July 9, 2019, 10:16 a.m. UTC | #3
* Mao Han:

> The test-wrapper is passed twice and no run_program_env is passed. I
> don't know whether the test-wrapper script is unnecessary(last patch)
> or the argument passed by Makefile is wrong:
>
> diff --git a/locale/Makefile b/locale/Makefile
> index 0ad99ec..d78cf9b 100644
> --- a/locale/Makefile
> +++ b/locale/Makefile
> @@ -113,5 +113,5 @@ lib := locale-programs
>  include $(patsubst %,$(..)libof-iterator.mk,$(cpp-srcs-left))
>  
>  $(objpfx)tst-locale-locpath.out : tst-locale-locpath.sh $(objpfx)locale
> -       $(SHELL) $< '$(common-objpfx)' '$(test-wrapper)' '$(test-wrapper-env)' > $@; \
> +       $(SHELL) $< '$(common-objpfx)' '$(test-wrapper-env)' '$(run-program-env)' > $@; \
>         $(evaluate-test)

Ah.  Does this patch work for you?  It looks more correct to me, but I
have no way of testing it.

Thanks,
Florian
Mao Han July 9, 2019, 10:50 a.m. UTC | #4
On Tue, Jul 09, 2019 at 12:16:12PM +0200, Florian Weimer wrote:
> * Mao Han:
> 
> > The test-wrapper is passed twice and no run_program_env is passed. I
> > don't know whether the test-wrapper script is unnecessary(last patch)
> > or the argument passed by Makefile is wrong:
> >
> > diff --git a/locale/Makefile b/locale/Makefile
> > index 0ad99ec..d78cf9b 100644
> > --- a/locale/Makefile
> > +++ b/locale/Makefile
> > @@ -113,5 +113,5 @@ lib := locale-programs
> >  include $(patsubst %,$(..)libof-iterator.mk,$(cpp-srcs-left))
> >  
> >  $(objpfx)tst-locale-locpath.out : tst-locale-locpath.sh $(objpfx)locale
> > -       $(SHELL) $< '$(common-objpfx)' '$(test-wrapper)' '$(test-wrapper-env)' > $@; \
> > +       $(SHELL) $< '$(common-objpfx)' '$(test-wrapper-env)' '$(run-program-env)' > $@; \
> >         $(evaluate-test)
> 
> Ah.  Does this patch work for you?  It looks more correct to me, but I
> have no way of testing it.
> 
> Thanks,
> Florian

Yes, I've tested this patch on C-SKY QEMU, it works fine.

Thanks,
Mao Han
Florian Weimer July 9, 2019, 11:59 a.m. UTC | #5
* Mao Han:

> On Tue, Jul 09, 2019 at 12:16:12PM +0200, Florian Weimer wrote:
>> * Mao Han:
>> 
>> > The test-wrapper is passed twice and no run_program_env is passed. I
>> > don't know whether the test-wrapper script is unnecessary(last patch)
>> > or the argument passed by Makefile is wrong:
>> >
>> > diff --git a/locale/Makefile b/locale/Makefile
>> > index 0ad99ec..d78cf9b 100644
>> > --- a/locale/Makefile
>> > +++ b/locale/Makefile
>> > @@ -113,5 +113,5 @@ lib := locale-programs
>> >  include $(patsubst %,$(..)libof-iterator.mk,$(cpp-srcs-left))
>> >  
>> >  $(objpfx)tst-locale-locpath.out : tst-locale-locpath.sh $(objpfx)locale
>> > -       $(SHELL) $< '$(common-objpfx)' '$(test-wrapper)' '$(test-wrapper-env)' > $@; \
>> > +       $(SHELL) $< '$(common-objpfx)' '$(test-wrapper-env)' '$(run-program-env)' > $@; \
>> >         $(evaluate-test)
>> 
>> Ah.  Does this patch work for you?  It looks more correct to me, but I
>> have no way of testing it.
>> 
>> Thanks,
>> Florian
>
> Yes, I've tested this patch on C-SKY QEMU, it works fine.

Great.  Please push this change along with an appropriate ChangeLog
entry.

Thanks,
Florian

Patch
diff mbox series

diff --git a/elf/tst-rtld-preload.sh b/elf/tst-rtld-preload.sh
index f0c0ca1..0fe9e4f 100755
--- a/elf/tst-rtld-preload.sh
+++ b/elf/tst-rtld-preload.sh
@@ -31,7 +31,7 @@  echo "# [${test_wrapper}] [$rtld] [--library-path] [$library_path]" \
      "[--preload] [$preload] [$test_program]"
 ${test_wrapper_env} \
 ${run_program_env} \
-${test_wrapper} $rtld --library-path "$library_path" \
+$rtld --library-path "$library_path" \
   --preload "$preload" $test_program 2>&1 && rc=0 || rc=$?
 echo "# exit status $rc"
 
diff --git a/locale/tst-locale-locpath.sh b/locale/tst-locale-locpath.sh
index b83de90..1281e9c 100644
--- a/locale/tst-locale-locpath.sh
+++ b/locale/tst-locale-locpath.sh
@@ -20,8 +20,8 @@ 
 set -ex
 
 common_objpfx=$1
-test_wrapper_env=$2
-run_program_env=$3
+test_wrapper_env=$3
+run_program_env=
 
 LIBPATH="$common_objpfx"