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

Message ID 20190709013036.GA7671@vmh-VirtualBox
State Committed
Headers

Commit Message

毛晗 July 9, 2019, 1:30 a.m. UTC
  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:


Thanks,
Mao Han
  

Comments

Florian Weimer July 9, 2019, 10:16 a.m. UTC | #1
* 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
  
毛晗 July 9, 2019, 10:50 a.m. UTC | #2
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 | #3
* 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 --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)