ICU-22304 Miscellanous PersonNameFormatter fixes; made ExhaustivePers…

…onNameFormatterTest into a real unit test.
unicode-org · Jul 20, 2023 · 85e75ec · 85e75ec
1 parent 281a2a7
commit 85e75ec
Show file tree

Hide file tree

Showing 5 changed files with 72 additions and 48 deletions.
diff --git a/icu4j/build.xml b/icu4j/build.xml
@@ -444,7 +444,6 @@
         <exclude name="**/*$*.class"/>
         <exclude name="**/data/**"/>
         <exclude name="com/ibm/icu/dev/test/*"/>
-        <exclude name="**/ExhaustivePersonNameFormatterTest*" />
     </patternset>
 
     <!--Class names that will be included/excluded as tests for time zone check-->

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
@@ -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
@@ -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
@@ -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
@@ -41,6 +41,7 @@
         <fileset id="cldrTestData" dir="${cldrDir}/common/testData">
             <!-- Add directories here to control which test data is installed. -->
             <include name="units/**"/> <!-- Used in UnitsTest tests -->
+            <include name="personNameTest/**"/> <!-- Used in ExhaustivePersonNameTest -->
         </fileset>
 
         <copy todir="${testDataDir4C}">
@@ -49,6 +50,15 @@
         <copy todir="${testDataDir4J}">
             <fileset refid="cldrTestData"/>
         </copy>
+
+        <!-- create a catalog file for the cldr/personNameTest directory -->
+        <pathconvert property="personNameTestDirContents" pathsep="${line.separator}">
+            <fileset dir="${cldrDir}/common/testData/personNameTest" includes="**" />
+            <map from="${cldrDir}/common/testData/personNameTest/" to="" />
+        </pathconvert>
+        <echo message="Creating catalog.txt file" />
+        <echo message="${personNameTestDirContents}" file="${testDataDir4C}/personNameTest/catalog.txt" />
+        <echo message="${personNameTestDirContents}" file="${testDataDir4J}/personNameTest/catalog.txt" />
     </target>
 
     <!-- Deletes CLDR test data -->