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-18294 Add unit test for inheriting null, and fix two items in xml #4366

Merged
merged 1 commit into from
Feb 12, 2025

Conversation

btangmu
Copy link
Member

@btangmu btangmu commented Feb 12, 2025

-Combine the test with TestCheckCLDR to avoid another cycle through factory.getAvailable

-New method checkNullWithInheritanceMark

-Skip and log known issue for paths not in LANGUAGE/SCRIPT/TERRITORY/VARIANT

CLDR-18294

  • This PR completes the ticket.

ALLOW_MANY_COMMITS=true

-Combine the test with TestCheckCLDR to avoid another cycle through factory.getAvailable

-New method checkNullWithInheritanceMark

-Skip and log known issue for paths not in LANGUAGE/SCRIPT/TERRITORY/VARIANT
@btangmu btangmu self-assigned this Feb 12, 2025
@btangmu btangmu requested review from srl295 and macchiati February 12, 2025 17:01
@@ -476,8 +478,10 @@ public void TestAllLocales() {
LanguageTagParser ltp = new LanguageTagParser();
Set<String> locales = new HashSet<>();
for (String locale : getInclusion() <= 5 ? eightPointLocales : factory.getAvailable()) {
checkNullWithInheritanceMark(locale);
Copy link
Member Author

Choose a reason for hiding this comment

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

Actually I've just realized, this doesn't really avoid adding a cycle through the locales, since the original test waits until after this loop to cycle with checkLocale

Given the hurry to get this ready, I suggest merging this anyway and following up to address the performance

assertNotNull(locale + " " + path, resolved.getStringValue(path));
break;
default:
if (!logKnownIssue("CLDR-18294", "Null with inheritance mark")) {
Copy link
Member

Choose a reason for hiding this comment

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

needs to be a new ticket (48) for after 18294 is already fixed.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll follow up with that; merging now to fix the data

@btangmu btangmu merged commit 02722c8 into unicode-org:main Feb 12, 2025
13 checks passed
@btangmu btangmu deleted the t18294_b branch February 12, 2025 18:07
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.

2 participants