[COMMITTED] string: Move tst-strsignal tst-strerror to tests-container

Message ID 20200708132614.2800896-1-adhemerval.zanella@linaro.org
State Committed
Commit c1e63c7214aaef99039068da384a0ab3abc176f2
Headers
Series [COMMITTED] string: Move tst-strsignal tst-strerror to tests-container |

Commit Message

Adhemerval Zanella Netto July 8, 2020, 1:26 p.m. UTC
  Both tests require libc.mo translation files which might not be
installed on the system.

Checked on x86_64-linux-gnu.
---
 string/Makefile | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
  

Comments

Carlos O'Donell July 8, 2020, 4:05 p.m. UTC | #1
On 7/8/20 9:26 AM, Adhemerval Zanella via Libc-alpha wrote:
> Both tests require libc.mo translation files which might not be
> installed on the system.
> 
> Checked on x86_64-linux-gnu.
> ---
>  string/Makefile | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/string/Makefile b/string/Makefile
> index f8d3104e16..206c9b103c 100644
> --- a/string/Makefile
> +++ b/string/Makefile
> @@ -63,7 +63,9 @@ tests		:= tester inl-tester noinl-tester testcopy test-ffs	\
>  		   tst-strtok_r bug-strcoll2 tst-cmp tst-xbzero-opt	\
>  		   test-endian-types test-endian-file-scope		\
>  		   test-endian-sign-conversion tst-memmove-overflow	\
> -		   tst-strsignal tst-strerror test-sig_np
> +		   test-sig_np
> +
> +tests-container += tst-strsignal tst-strerror
>  
>  # This test allocates a lot of memory and can run for a long time.
>  xtests = tst-strcoll-overflow
> 

Thanks. This is a *good* reason to put them in tests-container :-)
  
Szabolcs Nagy July 9, 2020, 10 a.m. UTC | #2
The 07/08/2020 10:26, Adhemerval Zanella via Libc-alpha wrote:
> Both tests require libc.mo translation files which might not be
> installed on the system.
> 
> Checked on x86_64-linux-gnu.
> ---
>  string/Makefile | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/string/Makefile b/string/Makefile
> index f8d3104e16..206c9b103c 100644
> --- a/string/Makefile
> +++ b/string/Makefile
> @@ -63,7 +63,9 @@ tests		:= tester inl-tester noinl-tester testcopy test-ffs	\
>  		   tst-strtok_r bug-strcoll2 tst-cmp tst-xbzero-opt	\
>  		   test-endian-types test-endian-file-scope		\
>  		   test-endian-sign-conversion tst-memmove-overflow	\
> -		   tst-strsignal tst-strerror test-sig_np
> +		   test-sig_np
> +
> +tests-container += tst-strsignal tst-strerror

these still fail for me.

i have no libc.mo files in the testroot.root or anywhere.

i assume this is because i have

$ grep MSGFMT config.make
MSGFMT = :

i can install gettext if this is expected (but i think
failing with UNSUPPORTED would be nicer).
  
Adhemerval Zanella Netto July 9, 2020, 12:38 p.m. UTC | #3
On 09/07/2020 07:00, Szabolcs Nagy wrote:
> The 07/08/2020 10:26, Adhemerval Zanella via Libc-alpha wrote:
>> Both tests require libc.mo translation files which might not be
>> installed on the system.
>>
>> Checked on x86_64-linux-gnu.
>> ---
>>  string/Makefile | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/string/Makefile b/string/Makefile
>> index f8d3104e16..206c9b103c 100644
>> --- a/string/Makefile
>> +++ b/string/Makefile
>> @@ -63,7 +63,9 @@ tests		:= tester inl-tester noinl-tester testcopy test-ffs	\
>>  		   tst-strtok_r bug-strcoll2 tst-cmp tst-xbzero-opt	\
>>  		   test-endian-types test-endian-file-scope		\
>>  		   test-endian-sign-conversion tst-memmove-overflow	\
>> -		   tst-strsignal tst-strerror test-sig_np
>> +		   test-sig_np
>> +
>> +tests-container += tst-strsignal tst-strerror
> 
> these still fail for me.
> 
> i have no libc.mo files in the testroot.root or anywhere.
> 
> i assume this is because i have
> 
> $ grep MSGFMT config.make
> MSGFMT = :
> 
> i can install gettext if this is expected (but i think
> failing with UNSUPPORTED would be nicer).

I think it is easier to just disable the test in such case, since it is
not straightforward to check if the translation as loaded.  Maybe:

---
diff --git a/string/Makefile b/string/Makefile
index 206c9b103c..6d4f88ef36 100644
--- a/string/Makefile
+++ b/string/Makefile
@@ -65,7 +65,10 @@ tests                := tester inl-tester noinl-tester testcopy test-ffs     \
                   test-endian-sign-conversion tst-memmove-overflow     \
                   test-sig_np
 
+# Both tests requires the .mo translation files generated by msgfmt.
+ifneq ($(MSGFMT),:)
 tests-container += tst-strsignal tst-strerror
+endif
 
 # This test allocates a lot of memory and can run for a long time.
 xtests = tst-strcoll-overflow
---

Another option was to set a config.h to indicate this test is unsupported
(I don't have a strong opinion here).
  
Szabolcs Nagy July 9, 2020, 2:42 p.m. UTC | #4
The 07/09/2020 09:38, Adhemerval Zanella wrote:
> On 09/07/2020 07:00, Szabolcs Nagy wrote:
> > The 07/08/2020 10:26, Adhemerval Zanella via Libc-alpha wrote:
> >> Both tests require libc.mo translation files which might not be
> >> installed on the system.
> >>
> >> Checked on x86_64-linux-gnu.
> >> ---
> >>  string/Makefile | 4 +++-
> >>  1 file changed, 3 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/string/Makefile b/string/Makefile
> >> index f8d3104e16..206c9b103c 100644
> >> --- a/string/Makefile
> >> +++ b/string/Makefile
> >> @@ -63,7 +63,9 @@ tests		:= tester inl-tester noinl-tester testcopy test-ffs	\
> >>  		   tst-strtok_r bug-strcoll2 tst-cmp tst-xbzero-opt	\
> >>  		   test-endian-types test-endian-file-scope		\
> >>  		   test-endian-sign-conversion tst-memmove-overflow	\
> >> -		   tst-strsignal tst-strerror test-sig_np
> >> +		   test-sig_np
> >> +
> >> +tests-container += tst-strsignal tst-strerror
> > 
> > these still fail for me.
> > 
> > i have no libc.mo files in the testroot.root or anywhere.
> > 
> > i assume this is because i have
> > 
> > $ grep MSGFMT config.make
> > MSGFMT = :
> > 
> > i can install gettext if this is expected (but i think
> > failing with UNSUPPORTED would be nicer).
> 
> I think it is easier to just disable the test in such case, since it is
> not straightforward to check if the translation as loaded.  Maybe:
> 
> ---
> diff --git a/string/Makefile b/string/Makefile
> index 206c9b103c..6d4f88ef36 100644
> --- a/string/Makefile
> +++ b/string/Makefile
> @@ -65,7 +65,10 @@ tests                := tester inl-tester noinl-tester testcopy test-ffs     \
>                    test-endian-sign-conversion tst-memmove-overflow     \
>                    test-sig_np
>  
> +# Both tests requires the .mo translation files generated by msgfmt.
> +ifneq ($(MSGFMT),:)
>  tests-container += tst-strsignal tst-strerror
> +endif
>  
>  # This test allocates a lot of memory and can run for a long time.
>  xtests = tst-strcoll-overflow
> ---
> 
> Another option was to set a config.h to indicate this test is unsupported
> (I don't have a strong opinion here).

hm i'm not sure what's best, i'm fine with the
makefile patch, but it seems even after i
install gettext the test fails e.g. if i run

LANGUAGE=hu make t=string/tst-strsignal test

i see hungarian translations in the failure, so the
LANGUAGE env var has precedence over setlocale?

i think this may need some more env var setting.
  
Adhemerval Zanella Netto July 9, 2020, 4:22 p.m. UTC | #5
On 09/07/2020 11:42, Szabolcs Nagy wrote:
> The 07/09/2020 09:38, Adhemerval Zanella wrote:
>> On 09/07/2020 07:00, Szabolcs Nagy wrote:
>>> The 07/08/2020 10:26, Adhemerval Zanella via Libc-alpha wrote:
>>>> Both tests require libc.mo translation files which might not be
>>>> installed on the system.
>>>>
>>>> Checked on x86_64-linux-gnu.
>>>> ---
>>>>  string/Makefile | 4 +++-
>>>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/string/Makefile b/string/Makefile
>>>> index f8d3104e16..206c9b103c 100644
>>>> --- a/string/Makefile
>>>> +++ b/string/Makefile
>>>> @@ -63,7 +63,9 @@ tests		:= tester inl-tester noinl-tester testcopy test-ffs	\
>>>>  		   tst-strtok_r bug-strcoll2 tst-cmp tst-xbzero-opt	\
>>>>  		   test-endian-types test-endian-file-scope		\
>>>>  		   test-endian-sign-conversion tst-memmove-overflow	\
>>>> -		   tst-strsignal tst-strerror test-sig_np
>>>> +		   test-sig_np
>>>> +
>>>> +tests-container += tst-strsignal tst-strerror
>>>
>>> these still fail for me.
>>>
>>> i have no libc.mo files in the testroot.root or anywhere.
>>>
>>> i assume this is because i have
>>>
>>> $ grep MSGFMT config.make
>>> MSGFMT = :
>>>
>>> i can install gettext if this is expected (but i think
>>> failing with UNSUPPORTED would be nicer).
>>
>> I think it is easier to just disable the test in such case, since it is
>> not straightforward to check if the translation as loaded.  Maybe:
>>
>> ---
>> diff --git a/string/Makefile b/string/Makefile
>> index 206c9b103c..6d4f88ef36 100644
>> --- a/string/Makefile
>> +++ b/string/Makefile
>> @@ -65,7 +65,10 @@ tests                := tester inl-tester noinl-tester testcopy test-ffs     \
>>                    test-endian-sign-conversion tst-memmove-overflow     \
>>                    test-sig_np
>>  
>> +# Both tests requires the .mo translation files generated by msgfmt.
>> +ifneq ($(MSGFMT),:)
>>  tests-container += tst-strsignal tst-strerror
>> +endif
>>  
>>  # This test allocates a lot of memory and can run for a long time.
>>  xtests = tst-strcoll-overflow
>> ---
>>
>> Another option was to set a config.h to indicate this test is unsupported
>> (I don't have a strong opinion here).
> 
> hm i'm not sure what's best, i'm fine with the
> makefile patch, but it seems even after i
> install gettext the test fails e.g. if i run
> 
> LANGUAGE=hu make t=string/tst-strsignal test
> 
> i see hungarian translations in the failure, so the
> LANGUAGE env var has precedence over setlocale?
> 
> i think this may need some more env var setting.

The 'make t=<test> test; does not run really run the testcase inside a
container, so system environment will affect its output different than
when running with test-container where it restrict the environment 
variables.

And it seems that LANGUAGE takes precedence over setlocale for loading
the translation catalogue, and some tests do explicitly unset it to 
avoid such issues (intl/tst-codeset.c, intl/tst-gettext{2,3,4,5,6}.c, 
intl/tst-translit.c).

Although not strictly necessary I think we should also make it explict
on tst-strsignal and tst-strerror as well.

I will send a patch to address both issue, thanks for checking on it.
  

Patch

diff --git a/string/Makefile b/string/Makefile
index f8d3104e16..206c9b103c 100644
--- a/string/Makefile
+++ b/string/Makefile
@@ -63,7 +63,9 @@  tests		:= tester inl-tester noinl-tester testcopy test-ffs	\
 		   tst-strtok_r bug-strcoll2 tst-cmp tst-xbzero-opt	\
 		   test-endian-types test-endian-file-scope		\
 		   test-endian-sign-conversion tst-memmove-overflow	\
-		   tst-strsignal tst-strerror test-sig_np
+		   test-sig_np
+
+tests-container += tst-strsignal tst-strerror
 
 # This test allocates a lot of memory and can run for a long time.
 xtests = tst-strcoll-overflow