Skip to content
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

CLDR-9774 Enable test for missing/extra English names for BCP47 keys/types #4364

Conversation

pedberg-icu
Copy link
Contributor

@pedberg-icu pedberg-icu commented Feb 12, 2025

CLDR-9774

  • This PR completes the ticket.

Start using the test to check for missing or extra English display names for relevant BCP47 keys and types. Most of this was already in unittest/TestBCP47.java, but the tests there were not being run until I added them to TestAll() in PR #4358. Then for this PR I just had to switch to using normal error logging, and add some test skips (one marked with a logKnownIssue).

Tested this by temporarily adding some new entries in common/bcp47 files without translations in English and verifying that the test caught them.

ALLOW_MANY_COMMITS=true

@pedberg-icu pedberg-icu self-assigned this Feb 12, 2025
@@ -78,9 +78,6 @@ public static void main(String[] args) {
}

public void TestEnglishKeyTranslations() {
logKnownIssue(
"cldr7631",
"Using just warnings for now, until issues are resolved. Change WARNING/ERROR when removing this.");
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CLDR-7631 was resolved a while ago

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know why my sweep didn't find this. Is this function actually called?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@srl295 Maybe because until a day or so ago the tests in this file were not being run (since they were not included in TestAl())?

@pedberg-icu pedberg-icu force-pushed the CLDR-9774-test-en-names-for-bcp47-keys-values branch from 9516e07 to 22decaf Compare February 12, 2025 01:19
@jira-pull-request-webhook
Copy link

Hooray! The files in the branch are the same across the force-push. 😃

~ Your Friendly Jira-GitHub PR Checker Bot

@@ -78,9 +78,6 @@ public static void main(String[] args) {
}

public void TestEnglishKeyTranslations() {
logKnownIssue(
"cldr7631",
"Using just warnings for now, until issues are resolved. Change WARNING/ERROR when removing this.");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know why my sweep didn't find this. Is this function actually called?

@pedberg-icu pedberg-icu merged commit 73317e5 into unicode-org:main Feb 12, 2025
12 checks passed
@pedberg-icu pedberg-icu deleted the CLDR-9774-test-en-names-for-bcp47-keys-values branch February 12, 2025 18:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants