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

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

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

Sourceware - libc-alpha mailing list
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
--
2.25.1

Reply | Threaded
Open this post in threaded view
|

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

Sourceware - libc-alpha mailing list
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 :-)

--
Cheers,
Carlos.

Reply | Threaded
Open this post in threaded view
|

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

Szabolcs Nagy-2
In reply to this post by Sourceware - libc-alpha mailing list
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).

Reply | Threaded
Open this post in threaded view
|

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

Sourceware - libc-alpha mailing list


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).
Reply | Threaded
Open this post in threaded view
|

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

Szabolcs Nagy-2
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.

Reply | Threaded
Open this post in threaded view
|

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

Sourceware - libc-alpha mailing list


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.