From 85e75ecc13d9bbb324bc87cb5194e6982486f2d1 Mon Sep 17 00:00:00 2001 From: Rich Gillam <62772518+richgillam@users.noreply.github.com> Date: Tue, 18 Jul 2023 18:42:29 -0700 Subject: [PATCH] ICU-22304 Miscellanous PersonNameFormatter fixes; made ExhaustivePersonNameFormatterTest into a real unit test. --- icu4j/build.xml | 1 - .../impl/personname/PersonNamePattern.java | 2 +- .../com/ibm/icu/text/SimplePersonName.java | 1 - .../ExhaustivePersonNameFormatterTest.java | 106 ++++++++++-------- tools/cldr/build.xml | 10 ++ 5 files changed, 72 insertions(+), 48 deletions(-) diff --git a/icu4j/build.xml b/icu4j/build.xml index 9785c860640e..a9790f2adc2a 100644 --- a/icu4j/build.xml +++ b/icu4j/build.xml @@ -444,7 +444,6 @@ - diff --git a/icu4j/main/classes/core/src/com/ibm/icu/impl/personname/PersonNamePattern.java b/icu4j/main/classes/core/src/com/ibm/icu/impl/personname/PersonNamePattern.java index 14810debe7ed..1f8ab15ee3ca 100644 --- a/icu4j/main/classes/core/src/com/ibm/icu/impl/personname/PersonNamePattern.java +++ b/icu4j/main/classes/core/src/com/ibm/icu/impl/personname/PersonNamePattern.java @@ -361,7 +361,7 @@ public GivenToSurnamePersonName(PersonName underlyingPersonName) { @Override public String toString() { - return "Inverted version os " + underlyingPersonName.toString(); + return "Inverted version of " + underlyingPersonName.toString(); } @Override public Locale getNameLocale() { diff --git a/icu4j/main/classes/core/src/com/ibm/icu/text/SimplePersonName.java b/icu4j/main/classes/core/src/com/ibm/icu/text/SimplePersonName.java index 4939b2199f12..59163667eb24 100644 --- a/icu4j/main/classes/core/src/com/ibm/icu/text/SimplePersonName.java +++ b/icu4j/main/classes/core/src/com/ibm/icu/text/SimplePersonName.java @@ -95,7 +95,6 @@ public SimplePersonName build() { String surnamePrefix = fieldValues.get("surname-prefix"); String surnameCore = fieldValues.get("surname-core"); - StringBuilder sb = new StringBuilder(); if (surnamePrefix != null && surnameCore != null) { fieldValues.put("surname", surnamePrefix + " " + surnameCore); } else if (surnamePrefix != null) { diff --git a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/ExhaustivePersonNameFormatterTest.java b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/ExhaustivePersonNameFormatterTest.java index 8641c5ece32f..ae88a86a5e3a 100644 --- a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/ExhaustivePersonNameFormatterTest.java +++ b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/ExhaustivePersonNameFormatterTest.java @@ -2,19 +2,23 @@ // License & terms of use: http://www.unicode.org/copyright.html package com.ibm.icu.dev.test.format; -import java.io.File; -import java.io.FileInputStream; -import java.io.IOException; -import java.io.InputStreamReader; -import java.io.LineNumberReader; +import java.io.*; +import java.net.URISyntaxException; +import java.net.URL; import java.util.ArrayList; import java.util.Arrays; import java.util.List; import java.util.Locale; +import com.ibm.icu.dev.test.TestFmwk; +import com.ibm.icu.dev.test.TestUtil; import com.ibm.icu.text.PersonName; import com.ibm.icu.text.PersonNameFormatter; import com.ibm.icu.text.SimplePersonName; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; /** * This is a test designed to parse the files generated by GeneratePersonNameTestData.java in @@ -24,66 +28,78 @@ * want to copy all of those over into the ICU tree) and because I thought the test would * take too long to run. */ -public class ExhaustivePersonNameFormatterTest { - public static void main(String[] args) throws IOException { - if (args.length < 1) { - throw new IllegalArgumentException("No data file directory specified!"); - } - - String dataFilePath = args[0]; - File dataFileDir = new File(dataFilePath); +@RunWith(JUnit4.class) +public class ExhaustivePersonNameFormatterTest extends TestFmwk { + private static final String DATA_PATH = TestUtil.DATA_PATH + "cldr/personNameTest/"; + + @Before + public void beforeMethod() { + // Disable this test class except for exhaustive mode. + // To enable exhaustive mode, pass the JVM argument "-DICU.exhaustive=10" + org.junit.Assume.assumeTrue(getExhaustiveness() > 5); + } - if (!dataFileDir.isDirectory()) { - throw new IllegalArgumentException(dataFilePath + " is not a directory!"); - } + @Test + public void TestPersonNames() throws IOException { + InputStream catalogFileStream = TestUtil.class.getResourceAsStream(DATA_PATH + "catalog.txt"); + LineNumberReader catalogFile = new LineNumberReader(new InputStreamReader(catalogFileStream)); int filesWithErrors = 0; int filesWithoutErrors = 0; int skippedFiles = 0; int totalErrors = 0; - for (String filename : dataFileDir.list()) { - File dataFile = new File(dataFileDir, filename); - if (dataFile.isDirectory() || !filename.endsWith(".txt")) { - System.out.println("Skipping " + filename + "..."); - continue; - } - String[] FILENAMES_TO_SKIP = {"gaa.txt", "dsb.txt", "syr.txt", "hsb.txt", "lij.txt"}; - if (Arrays.asList(FILENAMES_TO_SKIP).contains(filename)) { - // extra check to narrow down the files for debugging - System.out.println("Skipping " + filename + "..."); - ++skippedFiles; - continue; - } - int testErrors = runTest(dataFile); - if (testErrors == 0) { - ++filesWithoutErrors; - } else { - ++filesWithErrors; - totalErrors += testErrors; + String filename = null; + do { + filename = catalogFile.readLine(); + if (filename != null) { + if (!filename.endsWith(".txt")) { + logln("Skipping " + filename + "..."); + continue; + } + String[] FILENAMES_TO_SKIP = {"gaa.txt", "dsb.txt", "syr.txt", "hsb.txt", "lij.txt"}; + if (Arrays.asList(FILENAMES_TO_SKIP).contains(filename)) { + // extra check to narrow down the files for debugging + logln("Skipping " + filename + "..."); + ++skippedFiles; + continue; + } + + try { + int testErrors = testIndividualLocale(filename); + if (testErrors == 0) { + ++filesWithoutErrors; + } else { + ++filesWithErrors; + totalErrors += testErrors; + } + } catch (Exception e) { + ++filesWithErrors; + } } - } + } while (filename != null); - System.out.println(); - System.out.println("Files without errors: " + filesWithoutErrors); - System.out.println("Files with errors: " + filesWithErrors); + logln("Files without errors: " + filesWithoutErrors); + logln("Files with errors: " + filesWithErrors); if (skippedFiles > 0) { - System.out.println("Skipped files: " + skippedFiles); + logln("Skipped files: " + skippedFiles); } - System.out.println("Total number of errors: " + totalErrors); + logln("Total number of errors: " + totalErrors); + + assertEquals("Some files had test failures", filesWithErrors, 0); } - private static int runTest(File testFile) throws IOException { - LineNumberReader in = new LineNumberReader(new InputStreamReader(new FileInputStream(testFile))); + private static int testIndividualLocale(String filename) throws IOException { + LineNumberReader in = new LineNumberReader(new InputStreamReader(TestUtil.class.getResourceAsStream(DATA_PATH + filename))); String line = null; - PersonNameTester tester = new PersonNameTester(testFile.getName()); + PersonNameTester tester = new PersonNameTester(filename); do { line = in.readLine(); tester.processLine(line, in.getLineNumber()); } while (line != null); - System.out.println(testFile.getAbsolutePath() + " had " + tester.getErrorCount() + " errors"); + System.out.println(filename + " had " + tester.getErrorCount() + " errors"); return tester.getErrorCount(); } diff --git a/tools/cldr/build.xml b/tools/cldr/build.xml index 29418ea2ea1e..2886f3119649 100644 --- a/tools/cldr/build.xml +++ b/tools/cldr/build.xml @@ -41,6 +41,7 @@ + @@ -49,6 +50,15 @@ + + + + + + + + +