nodejs / i18n

The Node.js Internationalization Working Group – A Community Committee initiative.
MIT License
150 stars 40 forks source link

Add tests for translated content #101

Closed zeke closed 4 years ago

zeke commented 6 years ago

Now what we have translated content flowing into the repo, we should add some basic tests to verify that it has the expected directory and file structure. This will give us more confidence that future PRs from Crowdin can be safely merged.

maddhruv commented 4 years ago

it has the expected directory and file structure

@zeke I didn't get the intention behind these test case, how and which part of the Pull Request/Changes are you going to test in this?

zeke commented 4 years ago

One of the shortcomings of Crowdin's GitHub integration is that it doesn't delete translated files when the source English file is deleted.

For example, if there was an English file en/c.md that was deleted, Crowdin will leave the translated files behind.

$ tree some-project
some-project
├── en
│   ├── a.md
│   └── b.md
├── es
│   ├── a.md
│   ├── b.md
│   └── c.md
└── fr
    ├── a.md
    ├── b.md
    └── c.md

These leftover files can lead to problems down the line.

For the help.github.com website, we have a test that makes sure the English directory and all translations directories are congruent. Here's the code from that: https://gist.github.com/zeke/91dab5f63e592cff6826247f255573e9

nschonni commented 4 years ago

I'd also suggest adding remark-lint along with https://github.com/nodejs/remark-preset-lint-node I started that work, but ran into some failures because the en-US content was behind. I just planning to run it on the content/v12.x as a start, because I don't think the older content is getting fixed in the same way

alexandrtovmach commented 4 years ago
  1. Validation of translated content is no more required, because it's handled by Crowdin now
  2. Deleting translated files if original file was deleted fixed by #269 thanks to @nschonni
  3. Original content linted in main repo I believe, and here's no reason to double lint

I think this issue should be closed, objections?

nschonni commented 4 years ago
1. Validation of translated content is no more required, because it's handled by Crowdin now

I still don't understand how Crowdin validates content. Do you mean that it checks that if the English file is deleted, it will delete the file automatically?

3\. Original content linted in main repo I believe, and here's no reason to double lint

New issues can and have been introduced during translations, like when markdown anchor link texts are changed, which is validated by nodejs remark-lint ruleset

zeke commented 4 years ago

Do you mean that it checks that if the English file is deleted, it will delete the file automatically?

Unfortunately, Crowdin's GitHub integration does not delete localized files when the source file is deleted (or renamed). There needs to be some kind of script to remove these extraneous files.

It appears that https://github.com/nodejs/i18n/pull/269 takes care of resetting English files when script/collect is run, but it doesn't handle extraneous localized files.

alexandrtovmach commented 4 years ago

@Andrulko Any suggestions?

alexandrtovmach commented 4 years ago

I started writing tests to cover this cases #297 and now understand what do you mean: https://github.com/nodejs/i18n/runs/585084630?check_suite_focus=true

I'm going to contact with Crowdin team to find a solution