-
Notifications
You must be signed in to change notification settings - Fork 3.2k
[No QA] Add test for matching Translations keys #4090
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 2 commits
f31e802
9e0fe2a
6ad65b7
26626ec
b1e7dc2
5551926
158dfb3
72a3280
f9baac6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,9 @@ | ||
const _ = require('underscore'); | ||
const translate = require('../../src/libs/translate'); | ||
const translations = require('../../src/languages/translations'); | ||
const CONFIG = require('../../src/CONFIG'); | ||
const translations = require('../../src/languages/translations'); | ||
|
||
const originalTranslations = _.clone(translations); | ||
translations.default = { | ||
en: { | ||
testKey1: 'English', | ||
|
@@ -52,3 +54,38 @@ describe('translate', () => { | |
expect(translate.translate('en', ['testKeyGroup', 'testFunction'], {testVariable})).toBe(expectedValue); | ||
}); | ||
}); | ||
|
||
describe('Translation Keys', () => { | ||
let activeLanguage; | ||
let path = ''; | ||
function matchKeys(source, target, key) { | ||
path += key ? `${key}.` : ''; | ||
const pathLevel = path; | ||
if (key && !_.has(target, key)) { | ||
console.debug(`🏹 ${path.slice(0, -1)} is missing from ${activeLanguage}.js`); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of a debug log line, can we make it so that the test itself fails? Ideally it would show one failed test per missing key if that's possible, but if that's hard we can have one test fail that prints all missing keys. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Test will fail, if key don't match. This log is just to show what is missing.
I don't think its a good idea to create multiple tests that basically test the same thing.
This can be done. We can log all the missing keys. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ok cool, my main point is that it's better to have the test failure show what keys failed instead of having to look at the logs. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I tried to find way to add the missing key as failure message itself but I don't think its possible AFAIK. So we have to stick with logs. Could you please give me a hint about how to do that if possible? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hmmmm, I think that if you change the expect from |
||
return; | ||
} | ||
const sourceOBJ = key ? source[key] : source; | ||
const targetOBJ = key ? target[key] : target; | ||
if (_.isObject(sourceOBJ) && !_.isFunction(sourceOBJ)) { | ||
return _.every(_.keys(sourceOBJ), (subKey) => { | ||
path = pathLevel; | ||
return matchKeys(sourceOBJ, targetOBJ, subKey); | ||
}); | ||
} | ||
if (key) { | ||
path = path.slice(0, -(key.length - 1)); | ||
} | ||
return true; | ||
} | ||
it('Does each locale has all the keys', () => { | ||
const excludeLanguages = ['en', 'es-ES']; | ||
const languages = _.without(_.keys(originalTranslations.default), ...excludeLanguages); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we make this smarter and instead of having to add There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That was not the original Idea. After Sometime, we can decide to add more Translations. and Two component locale would be completely valid in that case. So to skip regardless of specification will lead to code change. So I think its better, not to make it smarter. It is sufficient to fulfill its purpose. |
||
const parentLanguage = originalTranslations.default.en; | ||
const hasAllKeys = _.every(languages, (ln) => { | ||
activeLanguage = ln; | ||
return matchKeys(parentLanguage, originalTranslations.default[ln]); | ||
}); | ||
expect(hasAllKeys).toBeTruthy(); | ||
}); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this "global"? Can't we pass it as a regular argument to
matchKeys
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is convenient.