tests: <-- ENGLISH IMPOSED --> removed and gettext() added: why?

classic Classic list List threaded Threaded
5 messages Options
Samuel GOUGEON Samuel GOUGEON
Reply | Threaded
Open this post in threaded view
|

tests: <-- ENGLISH IMPOSED --> removed and gettext() added: why?

Hello,

In some commits just before the release of 6.0.2, and more recently on the master, i noticed that when some tests are tagged with the <-- ENGLISH IMPOSED --> flag and some error messages are tested against their english version, without calling gettext, the reviewer removes the flag and adds gettext().

What's the reason for this? What's the purpose of
the <-- ENGLISH IMPOSED --> flag, then ?

Theoretically, the message in english can be different from the message_id (also in english).
Is it the reason? Or what else?

Thanks for your insight about this topic.

Regards
Samuel


_______________________________________________
dev mailing list
[hidden email]
http://lists.scilab.org/mailman/listinfo/dev
Clément David-3 Clément David-3
Reply | Threaded
Open this post in threaded view
|

Re: tests: <-- ENGLISH IMPOSED --> removed and gettext() added: why?

Hello Samuel,

There should be no real difference, having ENGLISH IMPOSED might have subtle bugs related to the
system locales whereas using gettext() will rely on an extra Scilab function call (so will be less a
"unit test").

IMHO commits having only this kind of diff is useless; there should be something else, could you
point us the related code-review ?

Thanks,

--
Clément


Le vendredi 31 mai 2019 à 18:57 +0200, Samuel Gougeon a écrit :

> Hello,
> In some commits just before the release of 6.0.2, and more recently on the master, i noticed that
> when some tests are tagged with the <-- ENGLISH IMPOSED --> flag and some error messages are
> tested against their english version, without calling gettext, the reviewer removes the flag and
> adds gettext().
>
> What's the reason for this? What's the purpose of the <-- ENGLISH IMPOSED --> flag, then ?
> Theoretically, the message in english can be different from the message_id (also in english).
> Is it the reason? Or what else?
> Thanks for your insight about this topic.
> Regards
> Samuel
>
> _______________________________________________
> dev mailing list
> [hidden email]
> http://lists.scilab.org/mailman/listinfo/dev
_______________________________________________
dev mailing list
[hidden email]
http://lists.scilab.org/mailman/listinfo/dev
Samuel GOUGEON Samuel GOUGEON
Reply | Threaded
Open this post in threaded view
|

Re: tests: <-- ENGLISH IMPOSED --> removed and gettext() added: why?

Hello Clément,

Le 13/06/2019 à 15:02, Clément David a écrit :
> Hello Samuel,
>
> There should be no real difference, having ENGLISH IMPOSED might have subtle bugs related to the
> system locales whereas using gettext() will rely on an extra Scilab function call (so will be less a
> "unit test").
>
> IMHO commits having only this kind of diff is useless; there should be something else, could you
> point us the related code-review ?

Both cases i had in mind were not commited just for this. But this
change was made in addition before merging.
Here is one case :

https://codereview.scilab.org/#/c/20723/6..7

There was another one around the 6.0.2 release, but i failed refinding it.

I agree about the extra function call. But as i wrote before, i got
aware that using gettext() rather than ENGLISH IMPOSED is more robust
for (rare but possible) cases having a translation even for en_US.

Regards
Samuel

_______________________________________________
dev mailing list
[hidden email]
http://lists.scilab.org/mailman/listinfo/dev
Antoine ELIAS-2 Antoine ELIAS-2
Reply | Threaded
Open this post in threaded view
|

Re: tests: <-- ENGLISH IMPOSED --> removed and gettext() added: why?

Hahaha it was me \o/

Seriously, I don't care about localization in test.
A long time ago we had a discussion about localization in tests.
I would have preferred to have some tests about localization mechanism and no localization in the others.
The goal of all tests is not to check localization.
But the final decision was different.

In your commit you have mix localization and no localization, so I update to full localization that's all.

Antoine
Le 14/06/2019 à 00:38, Samuel Gougeon a écrit :
Hello Clément,

Le 13/06/2019 à 15:02, Clément David a écrit :
Hello Samuel,

There should be no real difference, having ENGLISH IMPOSED might have subtle bugs related to the
system locales whereas using gettext() will rely on an extra Scilab function call (so will be less a
"unit test").

IMHO commits having only this kind of diff is useless; there should be something else, could you
point us the related code-review ?

Both cases i had in mind were not commited just for this. But this change was made in addition before merging.
Here is one case :

https://codereview.scilab.org/#/c/20723/6..7

There was another one around the 6.0.2 release, but i failed refinding it.

I agree about the extra function call. But as i wrote before, i got aware that using gettext() rather than ENGLISH IMPOSED is more robust for (rare but possible) cases having a translation even for en_US.

Regards
Samuel

_______________________________________________
dev mailing list
[hidden email]
http://lists.scilab.org/mailman/listinfo/dev


_______________________________________________
dev mailing list
[hidden email]
http://lists.scilab.org/mailman/listinfo/dev
Samuel GOUGEON Samuel GOUGEON
Reply | Threaded
Open this post in threaded view
|

Re: tests: <-- ENGLISH IMPOSED --> removed and gettext() added: why?

Le 14/06/2019 à 12:54, Antoine ELIAS a écrit :
Hahaha it was me \o/

Seriously, I don't care about localization in test.
A long time ago we had a discussion about localization in tests.
I would have preferred to have some tests about localization mechanism and no localization in the others.
The goal of all tests is not to check localization.
But the final decision was different.

In your commit you have mix localization and no localization, so I update to full localization that's all.

Thanks for the piece of history.

I have finally found the other case here:
https://codereview.scilab.org/#/c/20831/2/scilab/modules/dynamic_link/tests/nonreg_tests/bug_7907.tst

OK about the fact that, finally, we don't care.

Regards
Samuel


_______________________________________________
dev mailing list
[hidden email]
http://lists.scilab.org/mailman/listinfo/dev