Fix #15. Add tests cases using other languages. #16

apipkin · 2013-12-13T21:01:42Z

Fix #15. Add tests cases using other languages using test parameters provided by @lwelti

drewfish · 2013-12-13T21:09:13Z

tests/index.js

+   });
+
+
+   it('en-US', function () {


Should this test be called fr-FR instead?

Yeah this was a copy paste error.

drewfish · 2013-12-13T21:10:33Z

It'd be nice to have some pluralization tests in here as well (such as testing that count:3 matches "few" in some specific languages but "other" in en)

apipkin · 2013-12-13T21:16:42Z

@drewfish yeah i can totally see that, but I'm not familiar with any other languages to know what the best course of a test would be.

lwelti · 2013-12-13T21:21:26Z

i am in the process to give you some plural examples, give me 3mins.

apipkin · 2013-12-13T21:23:30Z

Of course the CLDR stuff will need to be in to test plurals :)

lwelti · 2013-12-13T21:40:05Z

here is a basic plural + gender with French. be aware that the list pattern is created by hand.

en-US: {TRAVELLERS} went to {CITY}.
fr-FR: {TRAVELLERS} {TRAVELLER_COUNT, plural, one {est {GENDER, select, female {allée} other {allé}}} other {sont {GENDER, select, female {allées} other {allés}}}} à {CITY}.

1st
values: TRAVELLERS= "Lucas, Tony and Drew" // Yes a fix list
TRAVELLER_COUNT = 3
GENDER = male

2nd
then you can change the list of TRAVELLERS and change to female

3rd
values: TRAVELLERS="Monica"
TRAVELLER_COUNT = 1
GENDER = female

lwelti · 2013-12-13T21:44:47Z

for few, many let me get some, I am not 100% sure if the one I have in mind is correct, so give me some mins

lwelti · 2013-12-13T22:00:25Z

en-US
text: {COMPANY_COUNT, plural, one {One company} other {# companies}} published new books.

ru-RU
text: {COMPANY_COUNT, plural, one {Одна компания опубликовала} few {# компании опубликовали} many {# компаний опубликовали} other {# компаний опубликовали}} новые книги.

for en-US it is clear the expected result, so i will only paste the one for ru-RU

1st test:
COMPANY_COUNT => 0
expected: 0 компаний опубликовали новые книги.

2nd test
COMPANY_COUNT=>1
expected: Одна компания опубликовала новые книги.

3rd test
COMPANY_COUNT=>2
expected: 2 компании опубликовали новые книги.

4t test
COMPANY_COUNT=>5
expected: 5 компаний опубликовали новые книги.

5t test
COMPANY_COUNT=> 10
expected: 10 компаний опубликовали новые книги.

apipkin · 2013-12-13T22:41:34Z

One of the tests for Russian is now failing, though it should pass when CLDR is in place. I'm leaving it as is to verify CLDR is working later on.

lwelti · 2013-12-13T22:44:10Z

ok so you are not committing the russian one? and which one is failing just to know

apipkin · 2013-12-13T23:30:07Z

No I would commit it, there would just be a failing test.

apipkin · 2013-12-16T22:27:26Z

Is this okay to merge in?

drewfish · 2013-12-16T22:39:45Z

+1

ericf · 2013-12-17T17:32:08Z

Is this okay to merge in?

We should fix this failing test before we merge.

lwelti · 2013-12-17T17:36:33Z

can you confirm why is failing? which test is the one failing?

apipkin · 2013-12-17T20:24:20Z

It's failing because there is no CLDR available yet :)

lwelti · 2013-12-17T20:34:04Z

ok let be more clear :) which specific test is failing? or all of them are failing?

apipkin · 2013-12-17T20:36:32Z

Oh, the pluralized ru-RU https://github.com/yahoo/intl-messageformat/pull/16/files#diff-4663341696a08eef2606d83e7fb282a9R498

lwelti · 2013-12-17T20:40:00Z

then why don't you remove the ru-RU and then after we have the cldr you commit the ru-RU ?

lwelti · 2013-12-17T20:44:21Z

i am actually double checking ru-RU as I have some doubts, so you can remove it for now and then I can double check and when CLDR is there we can test it again.

lwelti · 2013-12-17T20:49:07Z

based on this: https://github.com/papandreou/node-cldr/blob/master/3rdparty/cldr/common/supplemental/plurals.xml#L188

ru seems to have only "one many other" but i know that it has "few" too, and even the twitter implementation has it:

https://github.com/twitter/twitter-cldr-js/blob/master/lib/assets/javascripts/twitter_cldr/ru.js#L150

I definitely need to double check.

drewfish · 2013-12-17T20:54:08Z

Yeah, Russian appears to have "few" as witnessed on this page:
http://unicode.org/repos/cldr-tmp/trunk/diff/supplemental/language_plural_rules.html

drewfish · 2013-12-17T20:56:53Z

BTW it appears that Arabic ("ar") and Welsh ("cy") have the full complement of groups ("zero", "one", "two", "few", "many", "other") so those might be good candidates for testing.

lwelti · 2013-12-17T20:59:05Z

correct, because at the implementation level:
https://raw.github.com/drewfish/intl-messageformat/7a79385bbd4f58eb0c5368c18c87ee55c360cdb3/locale-data/ru.js

it only has: one, many, other.

so remove ru test case for now.

drewfish · 2013-12-17T21:06:22Z

I've filed papandreou/node-cldr#11 on the upstream cldr package.

lwelti · 2013-12-17T21:07:34Z

ok, i found that on the latest version of CLDR they merged few -> other:

http://cldr.unicode.org/index/downloads/cldr-24 then search for: "few (2-4,…) ➞ other (all fractions): Russian"

lwelti · 2013-12-17T21:09:06Z

let me update the test case for ru-RU without "few"

lwelti · 2013-12-17T21:11:10Z

@drewfish here is the latest one (version 24):
http://www.unicode.org/cldr/charts/24/supplemental/language_plural_rules.html

which has one many other

lwelti · 2013-12-17T21:14:23Z

@apipkin feel free to remove "few" from ru-RU test case.

apipkin · 2013-12-17T21:16:54Z

I'm okay with waiting on this until CLDR is merged. I just wasn't sure if this was good to merge in before. I'm quite okay with waiting.

…into tests

apipkin · 2013-12-18T19:29:42Z

  message resolvedOptions
    ✓ empty options

  message creation
    ✓ simple string formatting
    ✓ simple object formatting
    ✓ simple object with post processing tokens
    ✓ complex object formatter
    ✓ complex object formatter with invalid valueName
    ✓ complex object formatter with offset
    ✓ Simple string formatter using a custom formatter for a token
    ✓ Simple string formatter using an inline formatter for a token
    ✓ Simple string formatter using a nonexistent formatter for a token
    ✓ Custom formatters with hidden inheritance
    ✓ broken pattern

  message creation for plurals
    ✓ zero
    ✓ one
    ✓ two
    ✓ few
    ✓ many
    ✓ other

  locale switching
    ✓ en-US simple
    ✓ fr-FR simple
    ✓ en-US complex
    ✓ fr-FR complex

  locale switching with counts
    ✓ en-US
    ✓ ru-RU


  24 passing (10ms)

=============================================================================
Writing coverage object [/Users/apipkin/Projects/intl-messageformat/coverage/coverage.json]
Writing coverage reports at [/Users/apipkin/Projects/intl-messageformat/coverage]
=============================================================================

======================== Coverage summary (IntlMessageFormat) =========================
Statements   : 96.08% (49 / 51) 
Branches     : 87.91% (80 / 91)  
Functions    :  90.91% (10 / 11)   
Lines        : 100% ( 3/3 )
================================================================================

=========================== Coverage summary + Languages ============================
Statements   : 91.89% ( 68/74 )
Branches     : 79.07% ( 102/129 )
Functions    : 92.31% ( 12/13 )
Lines        : 100% ( 3/3 )
================================================================================

lwelti · 2013-12-18T19:31:14Z

yeahhh

Fix formatjs#15. Add tests cases using other languages.

a7f4f5c

drewfish reviewed Dec 13, 2013
View reviewed changes

Add more tests!!

7412435

apipkin added 2 commits December 17, 2013 16:32

Merge branch 'master' of https://github.com/yahoo/intl-messageformat …

61e0e88

…into tests

Completes ru-RU testing and removes erroneous 1 plural option.

f7a4090

Correct test name.

9c7d23a

apipkin merged commit 9c7d23a into formatjs:master Dec 18, 2013

apipkin mentioned this pull request Dec 18, 2013

A test uses '1' as a plural group. #23

Closed

apipkin deleted the tests branch January 16, 2014 16:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix #15. Add tests cases using other languages. #16

Fix #15. Add tests cases using other languages. #16

apipkin commented Dec 13, 2013

drewfish Dec 13, 2013

apipkin Dec 13, 2013

drewfish commented Dec 13, 2013

apipkin commented Dec 13, 2013

lwelti commented Dec 13, 2013

apipkin commented Dec 13, 2013

lwelti commented Dec 13, 2013

lwelti commented Dec 13, 2013

lwelti commented Dec 13, 2013

apipkin commented Dec 13, 2013

lwelti commented Dec 13, 2013

apipkin commented Dec 13, 2013

apipkin commented Dec 16, 2013

drewfish commented Dec 16, 2013

ericf commented Dec 17, 2013

lwelti commented Dec 17, 2013

apipkin commented Dec 17, 2013

lwelti commented Dec 17, 2013

apipkin commented Dec 17, 2013

lwelti commented Dec 17, 2013

lwelti commented Dec 17, 2013

lwelti commented Dec 17, 2013

drewfish commented Dec 17, 2013

drewfish commented Dec 17, 2013

lwelti commented Dec 17, 2013

drewfish commented Dec 17, 2013

lwelti commented Dec 17, 2013

lwelti commented Dec 17, 2013

lwelti commented Dec 17, 2013

lwelti commented Dec 17, 2013

apipkin commented Dec 17, 2013

apipkin commented Dec 18, 2013

lwelti commented Dec 18, 2013

Fix #15. Add tests cases using other languages. #16

Fix #15. Add tests cases using other languages. #16

Conversation

apipkin commented Dec 13, 2013

drewfish Dec 13, 2013

Choose a reason for hiding this comment

apipkin Dec 13, 2013

Choose a reason for hiding this comment

drewfish commented Dec 13, 2013

apipkin commented Dec 13, 2013

lwelti commented Dec 13, 2013

apipkin commented Dec 13, 2013

lwelti commented Dec 13, 2013

lwelti commented Dec 13, 2013

lwelti commented Dec 13, 2013

apipkin commented Dec 13, 2013

lwelti commented Dec 13, 2013

apipkin commented Dec 13, 2013

apipkin commented Dec 16, 2013

drewfish commented Dec 16, 2013

ericf commented Dec 17, 2013

lwelti commented Dec 17, 2013

apipkin commented Dec 17, 2013

lwelti commented Dec 17, 2013

apipkin commented Dec 17, 2013

lwelti commented Dec 17, 2013

lwelti commented Dec 17, 2013

lwelti commented Dec 17, 2013

drewfish commented Dec 17, 2013

drewfish commented Dec 17, 2013

lwelti commented Dec 17, 2013

drewfish commented Dec 17, 2013

lwelti commented Dec 17, 2013

lwelti commented Dec 17, 2013

lwelti commented Dec 17, 2013

lwelti commented Dec 17, 2013

apipkin commented Dec 17, 2013

apipkin commented Dec 18, 2013

lwelti commented Dec 18, 2013