Added --merge and --only-missing command line switches. #43
Conversation
|
Also i'm not sure whats going on but when i install my branch manually the |
|
|
||
| it('handles option --merge', function() { | ||
| return emberI18nCsv('to-js', 'tmp/locales', 'test/fixtures/i18n-merge.csv', { merge: true }).then(() => { | ||
| return areDirsEqual('tmp/locales', 'test/fixtures/locales').then(areSame => { |
There was a problem hiding this comment.
It doesn't look like this is testing anything. The beforeEach is copying 'test/fixtures/locales' into tmp, then you are asserting the result of the merge is equal to what's in 'test/fixtures/locales'. So it appears the merge is having no affect.
There was a problem hiding this comment.
I'm testing that the merge does not clobber the translations that are present already but not in the .csv file. This is successful because the .csv is a subset of the tmp/locales. Should i add an additional test that checks to see if additions are properly added?
There was a problem hiding this comment.
Yea, then I think that new test can replace this test, because it will cover the merge protection and the additional properties.
@Kilowhisky It is ignored in https://github.com/kellyselden/ember-i18n-csv/blob/master/.npmignore#L2, since it is babel code it gets compiled to '/dist' |
| ps.on('exit', () => { | ||
| expect(out).to.equal(''); | ||
| expect(err).to.equal(''); | ||
| areDirsEqual('tmp/locales', 'test/fixtures/locales').then(areSame => { |
There was a problem hiding this comment.
I think this needs to check 'test/fixtures/locales-merge' as well.
There was a problem hiding this comment.
Done again. But i'm not exactly sure why the test wasn't failing before. I've never used this test suite before so i'm kinda shooting in the dark here.
There was a problem hiding this comment.
It's because your source on line 61, "test/fixtures/locales", doesn't have any missing keys. You should probably use "test/fixtures/locales-with-missing-keys" as your source to properly test. Then your merge file should have additional keys, but also some missing keys to test that the source keys are preserved.
|
The tests look great now. If you could just squash commits into one, this will be ready to merge. Thanks for all your work! |
|
Think that should do it. |
|
Beautiful. Thank you very much! |
|
Released v0.1.0. |
This is a rebase of #1