1
0
mirror of synced 2025-12-22 19:34:15 -05:00

Improvements in tools to help merge crowdin PRs (#18409)

- add `script/test-render-translation.js` to render all translated content to catch malformed liquid that would cause render errors
- improve test output for `script/fix-translation-errors.js` and `tests/content/lint-files.js`
- make it so `script/reset-translated-file.js` can handle files that have been renamed
This commit is contained in:
Vanessa Yuen
2021-03-26 20:21:45 +01:00
committed by GitHub
parent 3899af0d0d
commit a8d54c9af7
8 changed files with 348 additions and 219 deletions

View File

@@ -4,11 +4,11 @@ const isBrowser = process.env.BROWSER
const isActions = Boolean(process.env.GITHUB_ACTIONS)
const testTranslation = Boolean(process.env.TEST_TRANSLATION)
const reporters = ['default']
let reporters = ['default']
if (testTranslation) {
// only use custom reporter if we are linting translations
reporters.push('<rootDir>/tests/helpers/lint-translation-reporter.js')
reporters = ['<rootDir>/tests/helpers/lint-translation-reporter.js']
} else if (isActions) {
reporters.push('jest-github-actions-reporter')
}

63
package-lock.json generated
View File

@@ -5537,7 +5537,7 @@
},
"agentkeepalive": {
"version": "2.2.0",
"resolved": "http://registry.npmjs.org/agentkeepalive/-/agentkeepalive-2.2.0.tgz",
"resolved": "https://registry.npmjs.org/agentkeepalive/-/agentkeepalive-2.2.0.tgz",
"integrity": "sha1-xdG9SxKQCPEWPyNvhuX66iAm4u8="
},
"aggregate-error": {
@@ -5958,6 +5958,16 @@
"has-ansi": "^2.0.0",
"strip-ansi": "^3.0.0",
"supports-color": "^2.0.0"
},
"dependencies": {
"strip-ansi": {
"version": "3.0.1",
"resolved": "https://registry.npmjs.org/strip-ansi/-/strip-ansi-3.0.1.tgz",
"integrity": "sha1-ajhfuIU9lS1f8F0Oiq+UJ43GPc8=",
"requires": {
"ansi-regex": "^2.0.0"
}
}
}
},
"js-tokens": {
@@ -7083,7 +7093,7 @@
},
"brfs": {
"version": "1.6.1",
"resolved": "http://registry.npmjs.org/brfs/-/brfs-1.6.1.tgz",
"resolved": "https://registry.npmjs.org/brfs/-/brfs-1.6.1.tgz",
"integrity": "sha512-OfZpABRQQf+Xsmju8XE9bDjs+uU4vLREGolP7bDgcpsI17QREyZ4Bl+2KLxxx1kCgA0fAIhKQBaBYh+PEcCqYQ==",
"requires": {
"quote-stream": "^1.0.1",
@@ -11689,6 +11699,23 @@
"has-ansi": "^2.0.0",
"strip-ansi": "^3.0.0",
"supports-color": "^2.0.0"
},
"dependencies": {
"ansi-regex": {
"version": "2.1.1",
"resolved": "https://registry.npmjs.org/ansi-regex/-/ansi-regex-2.1.1.tgz",
"integrity": "sha1-w7M6te42DYbg5ijwRorn7yfWVN8=",
"dev": true
},
"strip-ansi": {
"version": "3.0.1",
"resolved": "https://registry.npmjs.org/strip-ansi/-/strip-ansi-3.0.1.tgz",
"integrity": "sha1-ajhfuIU9lS1f8F0Oiq+UJ43GPc8=",
"dev": true,
"requires": {
"ansi-regex": "^2.0.0"
}
}
}
},
"is-fullwidth-code-point": {
@@ -18386,7 +18413,7 @@
},
"magic-string": {
"version": "0.22.5",
"resolved": "http://registry.npmjs.org/magic-string/-/magic-string-0.22.5.tgz",
"resolved": "https://registry.npmjs.org/magic-string/-/magic-string-0.22.5.tgz",
"integrity": "sha512-oreip9rJZkzvA8Qzk9HFs8fZGF/u7H/gtrE8EN6RjKJ9kh2HlC+yQ2QezifqTZfGyiuAV0dRv5a+y/8gBb1m9w==",
"requires": {
"vlq": "^0.2.2"
@@ -19934,6 +19961,17 @@
"has-ansi": "^2.0.0",
"strip-ansi": "^3.0.0",
"supports-color": "^2.0.0"
},
"dependencies": {
"strip-ansi": {
"version": "3.0.1",
"resolved": "https://registry.npmjs.org/strip-ansi/-/strip-ansi-3.0.1.tgz",
"integrity": "sha1-ajhfuIU9lS1f8F0Oiq+UJ43GPc8=",
"dev": true,
"requires": {
"ansi-regex": "^2.0.0"
}
}
}
},
"commander": {
@@ -22957,11 +22995,20 @@
}
},
"strip-ansi": {
"version": "3.0.1",
"resolved": "https://registry.npmjs.org/strip-ansi/-/strip-ansi-3.0.1.tgz",
"integrity": "sha1-ajhfuIU9lS1f8F0Oiq+UJ43GPc8=",
"version": "6.0.0",
"resolved": "https://registry.npmjs.org/strip-ansi/-/strip-ansi-6.0.0.tgz",
"integrity": "sha512-AuvKTrTfQNYNIctbR1K/YGTR1756GycPsg7b9bdV9Duqur4gv6aKqHXah67Z8ImS7WEz5QVcOtlfW2rZEugt6w==",
"dev": true,
"requires": {
"ansi-regex": "^2.0.0"
"ansi-regex": "^5.0.0"
},
"dependencies": {
"ansi-regex": {
"version": "5.0.0",
"resolved": "https://registry.npmjs.org/ansi-regex/-/ansi-regex-5.0.0.tgz",
"integrity": "sha512-bY6fj56OUQ0hU1KjFNDQuJFezqKdrAyFdIevADiqrWHwSlbmBNMHp5ak2f40Pm8JTFyM2mqxkG6ngkHO11f/lg==",
"dev": true
}
}
},
"strip-bom": {
@@ -23413,7 +23460,7 @@
},
"through": {
"version": "2.3.8",
"resolved": "http://registry.npmjs.org/through/-/through-2.3.8.tgz",
"resolved": "https://registry.npmjs.org/through/-/through-2.3.8.tgz",
"integrity": "sha1-DdTJ/6q8NXlgsbckEV1+Doai4fU="
},
"through2": {

View File

@@ -158,6 +158,7 @@
"replace": "^1.2.0",
"robots-parser": "^2.1.1",
"start-server-and-test": "^1.12.0",
"strip-ansi": "^6.0.0",
"supertest": "^4.0.2",
"url-template": "^2.0.8",
"webpack-dev-middleware": "^3.7.2",

View File

@@ -71,13 +71,14 @@ changedFilesRelPaths.forEach(async (relPath) => {
if (!engResult) return
const { data: engData } = engResult
console.log(chalk.red('fixing errors in ') + chalk.bold(relPath))
console.log(chalk.bold(relPath))
const newData = data
fixableErrors.forEach(({ property }) => {
fixableErrors.forEach(({ property, message }) => {
const correctValue = get(engData, property)
console.log(` [${property}]: ${get(data, property)} -> ${correctValue}`)
console.log(chalk.red(` error message: [${property}] ${message}`))
console.log(` fix property [${property}]: ${get(data, property)} -> ${correctValue}`)
set(newData, property, correctValue)
})

View File

@@ -13,20 +13,8 @@
//
// Examples:
//
// reset a single translated file using a relative path:
// $ script/reset-translated-file.js translations/es-XL/content/actions/index.md
//
// reset a single translated file using a full path:
// $ script/reset-translated-file.js /Users/z/git/github/docs-internal/translations/es-XL/content/actions/index.md
//
// reset all language variants of a single English file (using a relative path):
// $ script/reset-translated-file.js content/actions/index.md
// $ script/reset-translated-file.js data/ui.yml
//
// reset all language variants of a single English file (using a full path):
// $ script/reset-translated-file.js /Users/z/git/github/docs-internal/content/desktop/index.md
// $ script/reset-translated-file.js /Users/z/git/github/docs-internal/data/ui.yml
//
// [end-readme]
const program = require('commander')
@@ -34,42 +22,47 @@ const { execSync } = require('child_process')
const assert = require('assert')
const fs = require('fs')
const path = require('path')
const languages = require('../lib/languages')
const chalk = require('chalk')
program
.description('reset translated files')
.option('-m, --use-main', 'Reset file to the translated file from `main` branch instead of from English source.')
.option('-m, --prefer-main', 'Reset file to the translated file, try using the file from `main` branch first, if not found (usually due to renaming), fall back to English source.')
.parse(process.argv)
const resetToEnglishSource = (translationFilePath) => {
assert(translationFilePath.startsWith('translations/'), 'path argument must be in the format `translations/<lang>/path/to/file`')
assert(fs.existsSync(translationFilePath), `file does not exist: ${translationFilePath}`)
const relativePath = translationFilePath.split(path.sep).slice(2).join(path.sep)
const englishFile = path.join(process.cwd(), relativePath)
assert(fs.existsSync(englishFile), `file does not exist: ${englishFile}`)
// replace file with English source
const englishContent = fs.readFileSync(englishFile, 'utf8')
fs.writeFileSync(translationFilePath, englishContent)
console.log('-> reverted to English: %s', path.relative(process.cwd(), translationFilePath))
}
const [pathArg] = program.args
assert(pathArg, 'first arg must be a target filename')
let languageCode
// Is the arg a fully-qualified path?
let relativePath = fs.existsSync(pathArg)
const relativePath = fs.existsSync(pathArg)
? path.relative(process.cwd(), pathArg)
: pathArg
if (program.useMain) {
execSync(`git checkout main -- ${relativePath}`)
console.log('reverted to file from main branch: %s', relativePath)
} else {
// extract relative path and language code if pathArg is in the format `translations/<lang>/path/to/file`
if (relativePath.startsWith('translations/')) {
languageCode = Object.values(languages).find(language => relativePath.startsWith(language.dir) && language.code !== 'en').code
relativePath = relativePath.split(path.sep).slice(2).join(path.sep)
if (program.preferMain) {
try {
execSync(`git checkout main -- ${relativePath}`, { stdio: 'pipe' })
console.log('-> reverted to file from main branch: %s', relativePath)
} catch (e) {
if (e.message.includes('pathspec')) {
console.warn(chalk.red(`cannot find ${relativePath} in main branch (likely because it was renamed); falling back to English source file.`))
resetToEnglishSource(relativePath)
} else {
console.warn(e.message)
}
const englishFile = path.join(process.cwd(), relativePath)
assert(fs.existsSync(englishFile), `file does not exist: ${englishFile}`)
const englishContent = fs.readFileSync(englishFile, 'utf8')
Object.values(languages).forEach(({ code }) => {
if (code === 'en') return
if (languageCode && languageCode !== code) return
const translatedFile = path.join(process.cwd(), languages[code].dir, relativePath)
fs.writeFileSync(translatedFile, englishContent)
console.log('reverted to English: %s', path.relative(process.cwd(), translatedFile))
})
}
} else {
resetToEnglishSource(relativePath)
}

View File

@@ -0,0 +1,57 @@
const renderContent = require('../lib/render-content')
const loadSiteData = require('../lib/site-data')
const { loadPages } = require('../lib/pages')
const languages = require('../lib/languages')
const path = require('path')
const { execSync } = require('child_process')
const fs = require('fs')
const frontmatter = require('../lib/frontmatter')
const chalk = require('chalk')
const main = async () => {
const siteData = loadSiteData()
const pages = await loadPages()
const contextByLanguage = {}
for (const lang in languages) {
const langObj = languages[lang]
const [crowdinLangCode] = langObj.dir === '' ? 'en' : langObj.dir.split('/').slice(1)
if (!crowdinLangCode) continue
contextByLanguage[crowdinLangCode] = {
site: siteData[langObj.code].site,
currentLanguage: langObj.code,
currentVersion: 'free-pro-team@latest'
}
}
const rootDir = path.join(__dirname, '..')
const changedFilesRelPaths = execSync('git diff --name-only origin/main | egrep "^translations/.*/.+.md$"', { maxBuffer: 1024 * 1024 * 100 })
.toString()
.split('\n')
.filter(path => path !== '' && !path.endsWith('README.md'))
.sort()
console.log(`Found ${changedFilesRelPaths.length} translated files.`)
changedFilesRelPaths.forEach(async (relPath) => {
const fullPath = path.join(rootDir, relPath)
const lang = relPath.split('/')[1]
const context = {
...contextByLanguage[lang],
pages,
page: pages.find(page => page.fullPath === fullPath),
redirects: {}
}
if (!context.page && !relPath.includes('data/reusables')) return
const fileContents = await fs.promises.readFile(fullPath, 'utf8')
const { content } = frontmatter(fileContents)
try {
await renderContent.liquid.parseAndRender(content, context)
} catch (err) {
console.log(chalk.bold(relPath))
console.log(chalk.red(` error message: ${err.message}`))
}
})
}
main()

View File

@@ -202,7 +202,7 @@ if (!process.env.TEST_TRANSLATION) {
console.log('testing translations.')
// get all translated markdown or yaml files by comparing files changed to main branch
const changedFilesRelPaths = execSync('git diff --name-only origin/main | egrep "^translations/.*/.+.(yml|md)$"').toString().split('\n')
const changedFilesRelPaths = execSync('git diff --name-only origin/main | egrep "^translations/.*/.+.(yml|md)$"', { maxBuffer: 1024 * 1024 * 100 }).toString().split('\n')
console.log(`Found ${changedFilesRelPaths.length} translated files.`)
const { mdRelPaths = [], ymlRelPaths = [], releaseNotesRelPaths = [] } = groupBy(changedFilesRelPaths, (path) => {
@@ -244,6 +244,7 @@ function getContent (content) {
}
describe('lint markdown content', () => {
if (mdToLint.length < 1) return
describe.each(mdToLint)(
'%s',
(markdownRelPath, markdownAbsPath) => {
@@ -292,36 +293,6 @@ describe('lint markdown content', () => {
expect(matches.length, errorMessage).toBe(0)
})
test('URLs must not contain a hard-coded language code', async () => {
const matches = links.filter(link => {
return /\/(?:${languageCodes.join('|')})\//.test(link)
})
const errorMessage = formatLinkError(languageLinkErrorText, matches)
expect(matches.length, errorMessage).toBe(0)
})
test('URLs must not contain a hard-coded version number', async () => {
const initialMatches = (content.match(versionLinkRegEx) || [])
// Filter out some very specific false positive matches
const matches = initialMatches.filter(match => {
if (markdownRelPath === 'content/admin/enterprise-management/migrating-from-github-enterprise-1110x-to-2123.md') {
return false
}
return true
})
const errorMessage = formatLinkError(versionLinkErrorText, matches)
expect(matches.length, errorMessage).toBe(0)
})
test('URLs must not contain a hard-coded domain name', async () => {
const matches = (content.match(domainLinkRegex) || [])
const errorMessage = formatLinkError(domainLinkErrorText, matches)
expect(matches.length, errorMessage).toBe(0)
})
test('must not leak Early Access doc URLs', async () => {
// Only execute for docs that are NOT Early Access
if (!isEarlyAccess) {
@@ -348,6 +319,7 @@ describe('lint markdown content', () => {
expect(matches.length, errorMessage).toBe(0)
})
if (!process.env.TEST_TRANSLATION) {
test('does not use old site.data variable syntax', async () => {
const matches = (content.match(oldVariableRegex) || [])
const matchesWithExample = matches.map(match => {
@@ -380,6 +352,37 @@ describe('lint markdown content', () => {
})
})
test('URLs must not contain a hard-coded language code', async () => {
const matches = links.filter(link => {
return /\/(?:${languageCodes.join('|')})\//.test(link)
})
const errorMessage = formatLinkError(languageLinkErrorText, matches)
expect(matches.length, errorMessage).toBe(0)
})
test('URLs must not contain a hard-coded version number', async () => {
const initialMatches = (content.match(versionLinkRegEx) || [])
// Filter out some very specific false positive matches
const matches = initialMatches.filter(match => {
if (markdownRelPath === 'content/admin/enterprise-management/migrating-from-github-enterprise-1110x-to-2123.md') {
return false
}
return true
})
const errorMessage = formatLinkError(versionLinkErrorText, matches)
expect(matches.length, errorMessage).toBe(0)
})
test('URLs must not contain a hard-coded domain name', async () => {
const matches = (content.match(domainLinkRegex) || [])
const errorMessage = formatLinkError(domainLinkErrorText, matches)
expect(matches.length, errorMessage).toBe(0)
})
}
test('contains valid Liquid', async () => {
// If Liquid can't parse the file, it'll throw an error.
// For example, the following is invalid and will fail this test:
@@ -411,6 +414,7 @@ describe('lint markdown content', () => {
})
describe('lint yaml content', () => {
if (ymlToLint.length < 1) return
describe.each(ymlToLint)(
'%s',
(yamlRelPath, yamlAbsPath) => {
@@ -439,54 +443,6 @@ describe('lint yaml content', () => {
expect(matches.length, errorMessage).toBe(0)
})
test('URLs must not contain a hard-coded language code', async () => {
const matches = []
for (const [key, content] of Object.entries(dictionary)) {
const contentStr = getContent(content)
if (!contentStr) continue
const valMatches = (contentStr.match(languageLinkRegex) || [])
if (valMatches.length > 0) {
matches.push(...valMatches.map((match) => `Key "${key}": ${match}`))
}
}
const errorMessage = formatLinkError(languageLinkErrorText, matches)
expect(matches.length, errorMessage).toBe(0)
})
test('URLs must not contain a hard-coded version number', async () => {
const matches = []
for (const [key, content] of Object.entries(dictionary)) {
const contentStr = getContent(content)
if (!contentStr) continue
const valMatches = (contentStr.match(versionLinkRegEx) || [])
if (valMatches.length > 0) {
matches.push(...valMatches.map((match) => `Key "${key}": ${match}`))
}
}
const errorMessage = formatLinkError(versionLinkErrorText, matches)
expect(matches.length, errorMessage).toBe(0)
})
test('URLs must not contain a hard-coded domain name', async () => {
const matches = []
for (const [key, content] of Object.entries(dictionary)) {
const contentStr = getContent(content)
if (!contentStr) continue
const valMatches = (contentStr.match(domainLinkRegex) || [])
if (valMatches.length > 0) {
matches.push(...valMatches.map((match) => `Key "${key}": ${match}`))
}
}
const errorMessage = formatLinkError(domainLinkErrorText, matches)
expect(matches.length, errorMessage).toBe(0)
})
test('must not leak Early Access doc URLs', async () => {
// Only execute for docs that are NOT Early Access
if (!isEarlyAccess) {
@@ -543,6 +499,55 @@ describe('lint yaml content', () => {
expect(matches.length, errorMessage).toBe(0)
})
if (!process.env.TEST_TRANSLATION) {
test('URLs must not contain a hard-coded language code', async () => {
const matches = []
for (const [key, content] of Object.entries(dictionary)) {
const contentStr = getContent(content)
if (!contentStr) continue
const valMatches = (contentStr.match(languageLinkRegex) || [])
if (valMatches.length > 0) {
matches.push(...valMatches.map((match) => `Key "${key}": ${match}`))
}
}
const errorMessage = formatLinkError(languageLinkErrorText, matches)
expect(matches.length, errorMessage).toBe(0)
})
test('URLs must not contain a hard-coded version number', async () => {
const matches = []
for (const [key, content] of Object.entries(dictionary)) {
const contentStr = getContent(content)
if (!contentStr) continue
const valMatches = (contentStr.match(versionLinkRegEx) || [])
if (valMatches.length > 0) {
matches.push(...valMatches.map((match) => `Key "${key}": ${match}`))
}
}
const errorMessage = formatLinkError(versionLinkErrorText, matches)
expect(matches.length, errorMessage).toBe(0)
})
test('URLs must not contain a hard-coded domain name', async () => {
const matches = []
for (const [key, content] of Object.entries(dictionary)) {
const contentStr = getContent(content)
if (!contentStr) continue
const valMatches = (contentStr.match(domainLinkRegex) || [])
if (valMatches.length > 0) {
matches.push(...valMatches.map((match) => `Key "${key}": ${match}`))
}
}
const errorMessage = formatLinkError(domainLinkErrorText, matches)
expect(matches.length, errorMessage).toBe(0)
})
test('does not use old site.data variable syntax', async () => {
const matches = []
@@ -595,11 +600,12 @@ describe('lint yaml content', () => {
expect(matches.length, errorMessage).toBe(0)
})
}
}
)
})
describe('lint release notes', () => {
if (releaseNotesToLint.length > 0) {
if (releaseNotesToLint.length < 1) return
describe.each(releaseNotesToLint)(
'%s',
(yamlRelPath, yamlAbsPath) => {
@@ -615,7 +621,31 @@ describe('lint release notes', () => {
const errorMessage = errors.map(error => `- [${error.property}]: ${error.actual}, ${error.message}`).join('\n')
expect(errors.length, errorMessage).toBe(0)
})
it('contains valid liquid', () => {
const { intro, sections } = dictionary
let toLint = { intro }
for (const key in sections) {
const section = sections[key]
const label = `sections.${key}`
section.forEach((part) => {
if (Array.isArray(part)) {
toLint = { ...toLint, ...{ [label]: section.join('\n') } }
} else {
for (const prop in section) {
toLint = { ...toLint, ...{ [`${label}.${prop}`]: section[prop] } }
}
}
})
}
for (const key in toLint) {
if (!toLint[key]) continue
expect(() => renderContent.liquid.parse(toLint[key]), `${key} contains invalid liquid`)
.not
.toThrow()
}
})
}
)
}
})

View File

@@ -1,8 +1,9 @@
const chalk = require('chalk')
const stripAnsi = require('strip-ansi')
const { groupBy } = require('lodash')
// we don't want to print all the stack traces
const stackTrackRegExp = /^\s+at\s.+/i
const stackTraceRegExp = /^\s+at\s.+/img
class TranslationReporter {
constructor (globalConfig, options) {
@@ -18,7 +19,9 @@ class TranslationReporter {
return {
fileName: ancestorTitles[1],
failedTests: title,
failureMessage: failureMessages.map((message) => message.split('\n').filter(line => !stackTrackRegExp.test(line)).join('\n'))
failureMessage: failureMessages.map((message) => {
return message.split('\n').filter(line => !stackTraceRegExp.test(stripAnsi(line))).join('\n')
})
}
})
return [...fails, ...formattedFails]
@@ -34,9 +37,6 @@ class TranslationReporter {
})
console.groupEnd()
}
console.log(chalk.bold('\nthese files should not be included: '))
console.dir(Object.keys(failuresByFile), { maxArrayLength: null })
}
}