From 01b22e5d78f40b8cd6389678bd93a2f727472db9 Mon Sep 17 00:00:00 2001 From: Hugo van der Merwe <17109322+hugovdm@users.noreply.github.com> Date: Fri, 6 Mar 2020 15:54:03 +0100 Subject: [PATCH 1/7] Temporarily disable unitPreferencesTest.txt tests with unsupported identifiers. --- .../testdata/units/unitPreferencesTest.txt | 36 +++++++++---------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/icu4c/source/test/testdata/units/unitPreferencesTest.txt b/icu4c/source/test/testdata/units/unitPreferencesTest.txt index 49ee7c74ff6..52673f99e0e 100644 --- a/icu4c/source/test/testdata/units/unitPreferencesTest.txt +++ b/icu4c/source/test/testdata/units/unitPreferencesTest.txt @@ -53,21 +53,21 @@ area; land; GB; 1738883619 / 390625; 4451.54206464; square-meter; 11 / area; land; GB; 316160658 / 78125; 4046.8564224; square-meter; 1; 1.0; acre area; land; GB; 1422722961 / 390625; 3642.17078016; square-meter; 9 / 10; 0.9; acre -concentration; blood-glucose; AG; 662435483600000000000000; 6.624354836E23; item-per-cubic-meter; 11 / 10; 1.1; millimole-per-liter -concentration; blood-glucose; AG; 602214076000000000000000; 6.02214076E23; item-per-cubic-meter; 1; 1.0; millimole-per-liter -concentration; blood-glucose; AG; 541992668400000000000000; 5.419926684E23; item-per-cubic-meter; 9 / 10; 0.9; millimole-per-liter +#WIP(ILLEGAL_ARG)#concentration; blood-glucose; AG; 662435483600000000000000; 6.624354836E23; item-per-cubic-meter; 11 / 10; 1.1; millimole-per-liter +#WIP(ILLEGAL_ARG)#concentration; blood-glucose; AG; 602214076000000000000000; 6.02214076E23; item-per-cubic-meter; 1; 1.0; millimole-per-liter +#WIP(ILLEGAL_ARG)#concentration; blood-glucose; AG; 541992668400000000000000; 5.419926684E23; item-per-cubic-meter; 9 / 10; 0.9; millimole-per-liter -concentration; default; 001; 11 / 10; 1.1; item-per-cubic-meter; 11 / 10; 1.1; item-per-cubic-meter -concentration; default; 001; 1; 1.0; item-per-cubic-meter; 1; 1.0; item-per-cubic-meter -concentration; default; 001; 9 / 10; 0.9; item-per-cubic-meter; 9 / 10; 0.9; item-per-cubic-meter +#WIP(ILLEGAL_ARG)#concentration; default; 001; 11 / 10; 1.1; item-per-cubic-meter; 11 / 10; 1.1; item-per-cubic-meter +#WIP(ILLEGAL_ARG)#concentration; default; 001; 1; 1.0; item-per-cubic-meter; 1; 1.0; item-per-cubic-meter +#WIP(ILLEGAL_ARG)#concentration; default; 001; 9 / 10; 0.9; item-per-cubic-meter; 9 / 10; 0.9; item-per-cubic-meter -consumption; default; 001; 11 / 1000000000; 1.1E-8; cubic-meter-per-meter; 11 / 10; 1.1; liter-per-100-kilometer -consumption; default; 001; 1 / 100000000; 1.0E-8; cubic-meter-per-meter; 1; 1.0; liter-per-100-kilometer -consumption; default; 001; 9 / 1000000000; 9.0E-9; cubic-meter-per-meter; 9 / 10; 0.9; liter-per-100-kilometer +#WIP(ILLEGAL_ARG)#consumption; default; 001; 11 / 1000000000; 1.1E-8; cubic-meter-per-meter; 11 / 10; 1.1; liter-per-100-kilometer +#WIP(ILLEGAL_ARG)#consumption; default; 001; 1 / 100000000; 1.0E-8; cubic-meter-per-meter; 1; 1.0; liter-per-100-kilometer +#WIP(ILLEGAL_ARG)#consumption; default; 001; 9 / 1000000000; 9.0E-9; cubic-meter-per-meter; 9 / 10; 0.9; liter-per-100-kilometer -consumption; vehicle-fuel; 001; 11 / 1000000000; 1.1E-8; cubic-meter-per-meter; 11 / 10; 1.1; liter-per-100-kilometer -consumption; vehicle-fuel; 001; 1 / 100000000; 1.0E-8; cubic-meter-per-meter; 1; 1.0; liter-per-100-kilometer -consumption; vehicle-fuel; 001; 9 / 1000000000; 9.0E-9; cubic-meter-per-meter; 9 / 10; 0.9; liter-per-100-kilometer +#WIP(ILLEGAL_ARG)#consumption; vehicle-fuel; 001; 11 / 1000000000; 1.1E-8; cubic-meter-per-meter; 11 / 10; 1.1; liter-per-100-kilometer +#WIP(ILLEGAL_ARG)#consumption; vehicle-fuel; 001; 1 / 100000000; 1.0E-8; cubic-meter-per-meter; 1; 1.0; liter-per-100-kilometer +#WIP(ILLEGAL_ARG)#consumption; vehicle-fuel; 001; 9 / 1000000000; 9.0E-9; cubic-meter-per-meter; 9 / 10; 0.9; liter-per-100-kilometer consumption; vehicle-fuel; BR; 11 / 10000000; 1.1E-6; cubic-meter-per-meter; 11 / 10; 1.1; liter-per-kilometer consumption; vehicle-fuel; BR; 1 / 1000000; 1.0E-6; cubic-meter-per-meter; 1; 1.0; liter-per-kilometer @@ -311,17 +311,17 @@ pressure; baromtrc; 001; 110; 110.0; kilogram-per-meter-square-second pressure; baromtrc; 001; 100; 100.0; kilogram-per-meter-square-second; 1; 1.0; hectopascal pressure; baromtrc; 001; 90; 90.0; kilogram-per-meter-square-second; 9 / 10; 0.9; hectopascal -pressure; baromtrc; IN; 37250279 / 10000; 3725.0279; kilogram-per-meter-square-second; 11 / 10; 1.1; inch-ofhg -pressure; baromtrc; IN; 3386389 / 1000; 3386.389; kilogram-per-meter-square-second; 1; 1.0; inch-ofhg -pressure; baromtrc; IN; 30477501 / 10000; 3047.7501; kilogram-per-meter-square-second; 9 / 10; 0.9; inch-ofhg +#WIP(ILLEGAL_ARG)#pressure; baromtrc; IN; 37250279 / 10000; 3725.0279; kilogram-per-meter-square-second; 11 / 10; 1.1; inch-ofhg +#WIP(ILLEGAL_ARG)#pressure; baromtrc; IN; 3386389 / 1000; 3386.389; kilogram-per-meter-square-second; 1; 1.0; inch-ofhg +#WIP(ILLEGAL_ARG)#pressure; baromtrc; IN; 30477501 / 10000; 3047.7501; kilogram-per-meter-square-second; 9 / 10; 0.9; inch-ofhg pressure; baromtrc; BR; 110; 110.0; kilogram-per-meter-square-second; 11 / 10; 1.1; millibar pressure; baromtrc; BR; 100; 100.0; kilogram-per-meter-square-second; 1; 1.0; millibar pressure; baromtrc; BR; 90; 90.0; kilogram-per-meter-square-second; 9 / 10; 0.9; millibar -pressure; baromtrc; MX; 44583 / 3040; 14.66546052631579; kilogram-per-meter-square-second; 11 / 10; 1.1; millimeter-ofhg -pressure; baromtrc; MX; 4053 / 304; 13.33223684210526; kilogram-per-meter-square-second; 1; 1.0; millimeter-ofhg -pressure; baromtrc; MX; 36477 / 3040; 11.99901315789474; kilogram-per-meter-square-second; 9 / 10; 0.9; millimeter-ofhg +#WIP(ILLEGAL_ARG)#pressure; baromtrc; MX; 44583 / 3040; 14.66546052631579; kilogram-per-meter-square-second; 11 / 10; 1.1; millimeter-ofhg +#WIP(ILLEGAL_ARG)#pressure; baromtrc; MX; 4053 / 304; 13.33223684210526; kilogram-per-meter-square-second; 1; 1.0; millimeter-ofhg +#WIP(ILLEGAL_ARG)#pressure; baromtrc; MX; 36477 / 3040; 11.99901315789474; kilogram-per-meter-square-second; 9 / 10; 0.9; millimeter-ofhg pressure; default; 001; 1100000; 1100000.0; kilogram-per-meter-square-second; 11 / 10; 1.1; megapascal pressure; default; 001; 1000000; 1000000.0; kilogram-per-meter-square-second; 1; 1.0; megapascal From 3ef88f403cb33abded304d3a17450125104e9b88 Mon Sep 17 00:00:00 2001 From: Hugo van der Merwe <17109322+hugovdm@users.noreply.github.com> Date: Thu, 12 Mar 2020 18:39:42 +0100 Subject: [PATCH 2/7] Read unitPreferencesTest.txt, preparing to test thresholds. --- icu4c/source/test/intltest/unitstest.cpp | 103 +++++++++++++++++++++-- 1 file changed, 97 insertions(+), 6 deletions(-) diff --git a/icu4c/source/test/intltest/unitstest.cpp b/icu4c/source/test/intltest/unitstest.cpp index f55ce5b59ca..c8cdfdc6ed4 100644 --- a/icu4c/source/test/intltest/unitstest.cpp +++ b/icu4c/source/test/intltest/unitstest.cpp @@ -7,12 +7,15 @@ #include "charstr.h" #include "intltest.h" +#include "number_decimalquantity.h" #include "unicode/ctest.h" #include "unicode/measunit.h" #include "unicode/unistr.h" #include "unicode/unum.h" #include "uparse.h" +using icu::number::impl::DecimalQuantity; + class UnitsTest : public IntlTest { public: UnitsTest() {} @@ -20,6 +23,7 @@ class UnitsTest : public IntlTest { void runIndexedTest(int32_t index, UBool exec, const char *&name, char *par = NULL); void testConversions(); + void testPreferences(); void testBasic(); void testSiPrefixes(); void testMass(); @@ -30,9 +34,12 @@ class UnitsTest : public IntlTest { extern IntlTest *createUnitsTest() { return new UnitsTest(); } void UnitsTest::runIndexedTest(int32_t index, UBool exec, const char *&name, char * /*par*/) { - if (exec) { logln("TestSuite UnitsTest: "); } + if (exec) { + logln("TestSuite UnitsTest: "); + } TESTCASE_AUTO_BEGIN; TESTCASE_AUTO(testConversions); + TESTCASE_AUTO(testPreferences); TESTCASE_AUTO(testBasic); TESTCASE_AUTO(testSiPrefixes); TESTCASE_AUTO(testMass); @@ -184,8 +191,7 @@ StringPiece trimField(char *(&field)[2]) { * WIP(hugovdm): deals with a single data-driven unit test for unit conversions. * This is a UParseLineFn as required by u_parseDelimitedFile. */ -static void U_CALLCONV unitsTestDataLineFn(void *context, char *fields[][2], int32_t fieldCount, - UErrorCode *pErrorCode) { +void unitsTestDataLineFn(void *context, char *fields[][2], int32_t fieldCount, UErrorCode *pErrorCode) { (void)fieldCount; // unused UParseLineFn variable IcuTestErrorCode status(*(UnitsTest *)context, "unitsTestDatalineFn"); @@ -211,7 +217,8 @@ static void U_CALLCONV unitsTestDataLineFn(void *context, char *fields[][2], int // Possible after merging in younies/tryingdouble: // UnitConverter converter(sourceUnit, targetUnit, *pErrorCode); // double got = converter.convert(1000, *pErrorCode); - // ((UnitsTest*)context)->assertEqualsNear(quantity.data(), expected, got, 0.0001); + // ((UnitsTest*)context)->assertEqualsNear(quantity.data(), expected, got, + // 0.0001); // // In the meantime, printing to stderr. fprintf(stderr, @@ -242,10 +249,94 @@ void UnitsTest::testConversions() { CharString path(sourceTestDataPath, errorCode); path.appendPathPart("units", errorCode); - path.appendPathPart("unitsTest.txt", errorCode); + path.appendPathPart(filename, errorCode); u_parseDelimitedFile(path.data(), ';', fields, kNumFields, unitsTestDataLineFn, this, errorCode); - if (errorCode.errIfFailureAndReset("error parsing %s: %s\n", filename, u_errorName(errorCode))) { + if (errorCode.errIfFailureAndReset("error parsing %s: %s\n", path.data(), u_errorName(errorCode))) { + return; + } +} + +/** + * WIP(hugovdm): deals with a single data-driven unit test for unit preferences. + * This is a UParseLineFn as required by u_parseDelimitedFile. + */ +void unitPreferencesTestDataLineFn(void *context, char *fields[][2], int32_t fieldCount, + UErrorCode *pErrorCode) { + (void)fieldCount; // unused UParseLineFn variable + IcuTestErrorCode status(*(UnitsTest *)context, "unitPreferencesTestDatalineFn"); + + StringPiece quantity = trimField(fields[0]); + StringPiece usage = trimField(fields[1]); + StringPiece region = trimField(fields[2]); + StringPiece inputR = trimField(fields[3]); + StringPiece inputD = trimField(fields[4]); + StringPiece inputUnit = trimField(fields[5]); + StringPiece outputR = trimField(fields[6]); + StringPiece outputD = trimField(fields[7]); + StringPiece outputUnit = trimField(fields[8]); + + DecimalQuantity dqOutputD; + dqOutputD.setToDecNumber(outputD, status); + if (status.errIfFailureAndReset("parsing decimal quantity: \"%.*s\"", outputD.length(), + outputD.data())) { + return; + } + double expectedOutput = dqOutputD.toDouble(); + + MeasureUnit input = MeasureUnit::forIdentifier(inputUnit, status); + if (status.errIfFailureAndReset("forIdentifier(\"%.*s\")", inputUnit.length(), inputUnit.data())) { + return; + } + + MeasureUnit output = MeasureUnit::forIdentifier(outputUnit, status); + if (status.errIfFailureAndReset("forIdentifier(\"%.*s\")", outputUnit.length(), outputUnit.data())) { + return; + } + + // WIP(hugovdm): hook this up to actual tests. + // + // Possible after merging in younies/tryingdouble: + // UnitConverter converter(sourceUnit, targetUnit, *pErrorCode); + // double got = converter.convert(1000, *pErrorCode); + // ((UnitsTest*)context)->assertEqualsNear(quantity.data(), expected, got, + // 0.0001); + // + // In the meantime, printing to stderr. + fprintf(stderr, + "Quantity (Category): \"%.*s\", Usage: \"%.*s\", Region: \"%.*s\", " + "Input: %.*s %.*s (%.*s), Output: %.*s %.*s (%.*s) - Expected: %f\n", + quantity.length(), quantity.data(), usage.length(), usage.data(), region.length(), + region.data(), inputD.length(), inputD.data(), inputUnit.length(), inputUnit.data(), + inputR.length(), inputR.data(), outputD.length(), outputD.data(), outputUnit.length(), + outputUnit.data(), outputR.length(), outputR.data(), expectedOutput); +} + +/** + * Runs data-driven unit tests for unit preferences. + */ +void UnitsTest::testPreferences() { + const char *filename = "unitPreferencesTest.txt"; + const int32_t kNumFields = 9; + char *fields[kNumFields][2]; + + IcuTestErrorCode errorCode(*this, "UnitsTest::testPreferences"); + const char *sourceTestDataPath = getSourceTestData(errorCode); + if (errorCode.errIfFailureAndReset("unable to find the source/test/testdata " + "folder (getSourceTestData())")) { + return; + } + + CharString path(sourceTestDataPath, errorCode); + path.appendPathPart("units", errorCode); + path.appendPathPart(filename, errorCode); + + // WIP(hugovdm): we need to replace u_parseDelimitedFile with something + // custom, because not all lines in unitPreferencesTest.txt have the same + // number of fields. + u_parseDelimitedFile(path.data(), ';', fields, kNumFields, unitPreferencesTestDataLineFn, this, + errorCode); + if (errorCode.errIfFailureAndReset("error parsing %s: %s\n", path.data(), u_errorName(errorCode))) { return; } } From c8d1960895728daa00bf93bb75a857f6cd410a38 Mon Sep 17 00:00:00 2001 From: Hugo van der Merwe <17109322+hugovdm@users.noreply.github.com> Date: Thu, 12 Mar 2020 18:36:27 +0100 Subject: [PATCH 3/7] Clone parsePreferencesTests from u_parseDelimitedFile (w/ minor mods) --- icu4c/source/test/intltest/unitstest.cpp | 114 +++++++++++++++++++++-- 1 file changed, 105 insertions(+), 9 deletions(-) diff --git a/icu4c/source/test/intltest/unitstest.cpp b/icu4c/source/test/intltest/unitstest.cpp index c8cdfdc6ed4..962f8251b7b 100644 --- a/icu4c/source/test/intltest/unitstest.cpp +++ b/icu4c/source/test/intltest/unitstest.cpp @@ -6,6 +6,7 @@ #if !UCONFIG_NO_FORMATTING #include "charstr.h" +#include "filestrm.h" #include "intltest.h" #include "number_decimalquantity.h" #include "unicode/ctest.h" @@ -217,8 +218,7 @@ void unitsTestDataLineFn(void *context, char *fields[][2], int32_t fieldCount, U // Possible after merging in younies/tryingdouble: // UnitConverter converter(sourceUnit, targetUnit, *pErrorCode); // double got = converter.convert(1000, *pErrorCode); - // ((UnitsTest*)context)->assertEqualsNear(quantity.data(), expected, got, - // 0.0001); + // ((UnitsTest*)context)->assertEqualsNear(quantity.data(), expected, got, 0.0001); // // In the meantime, printing to stderr. fprintf(stderr, @@ -299,8 +299,7 @@ void unitPreferencesTestDataLineFn(void *context, char *fields[][2], int32_t fie // Possible after merging in younies/tryingdouble: // UnitConverter converter(sourceUnit, targetUnit, *pErrorCode); // double got = converter.convert(1000, *pErrorCode); - // ((UnitsTest*)context)->assertEqualsNear(quantity.data(), expected, got, - // 0.0001); + // ((UnitsTest*)context)->assertEqualsNear(quantity.data(), expected, got, 0.0001); // // In the meantime, printing to stderr. fprintf(stderr, @@ -312,6 +311,106 @@ void unitPreferencesTestDataLineFn(void *context, char *fields[][2], int32_t fie outputUnit.data(), outputR.length(), outputR.data(), expectedOutput); } +// WIP(hugovdm): we need to replace u_parseDelimitedFile with something +// custom, because not all lines in unitPreferencesTest.txt have the same +// number of fields. +void parsePreferencesTests(const char *filename, char delimiter, char *fields[][2], int32_t fieldCount, + UParseLineFn *lineFn, void *context, UErrorCode *pErrorCode) { + FileStream *file; + char line[10000]; + char *start, *limit; + int32_t i, length; + + if (U_FAILURE(*pErrorCode)) { return; } + + if (fields == NULL || lineFn == NULL || fieldCount <= 0) { + *pErrorCode = U_ILLEGAL_ARGUMENT_ERROR; + return; + } + + if (filename == NULL || *filename == 0 || (*filename == '-' && filename[1] == 0)) { + filename = NULL; + file = T_FileStream_stdin(); + } else { + file = T_FileStream_open(filename, "r"); + } + if (file == NULL) { + *pErrorCode = U_FILE_ACCESS_ERROR; + return; + } + + while (T_FileStream_readLine(file, line, sizeof(line)) != NULL) { + /* remove trailing newline characters */ + length = (int32_t)(u_rtrim(line) - line); + + // FYI: uparse.cpp had this code, which we're skipping here: + // /* + // * detect a line with # @missing: + // * start parsing after that, or else from the beginning of the line + // * set the default warning for @missing lines + // */ + // start = (char *)getMissingLimit(line); + // if (start == line) { + // *pErrorCode = U_ZERO_ERROR; + // } else { + // *pErrorCode = U_USING_DEFAULT_WARNING; + // } + start = line; + *pErrorCode = U_ZERO_ERROR; + + /* skip this line if it is empty or a comment */ + if (*start == 0 || *start == '#') { continue; } + + /* remove in-line comments */ + limit = uprv_strchr(start, '#'); + if (limit != NULL) { + /* get white space before the pound sign */ + while (limit > start && U_IS_INV_WHITESPACE(*(limit - 1))) { + --limit; + } + + /* truncate the line */ + *limit = 0; + } + + /* skip lines with only whitespace */ + if (u_skipWhitespace(start)[0] == 0) { continue; } + + /* for each field, call the corresponding field function */ + for (i = 0; i < fieldCount; ++i) { + /* set the limit pointer of this field */ + limit = start; + while (*limit != delimiter && *limit != 0) { + ++limit; + } + + /* set the field start and limit in the fields array */ + fields[i][0] = start; + fields[i][1] = limit; + + /* set start to the beginning of the next field, if any */ + start = limit; + if (*start != 0) { + ++start; + } else if (i + 1 < fieldCount) { + *pErrorCode = U_PARSE_ERROR; + limit = line + length; + i = fieldCount; + break; + } + } + + /* too few fields? */ + if (U_FAILURE(*pErrorCode)) { break; } + + /* call the field function */ + lineFn(context, fields, fieldCount, pErrorCode); + if (U_FAILURE(*pErrorCode)) { break; } + } + + if (filename != NULL) { T_FileStream_close(file); } +} + /** * Runs data-driven unit tests for unit preferences. */ @@ -331,11 +430,8 @@ void UnitsTest::testPreferences() { path.appendPathPart("units", errorCode); path.appendPathPart(filename, errorCode); - // WIP(hugovdm): we need to replace u_parseDelimitedFile with something - // custom, because not all lines in unitPreferencesTest.txt have the same - // number of fields. - u_parseDelimitedFile(path.data(), ';', fields, kNumFields, unitPreferencesTestDataLineFn, this, - errorCode); + parsePreferencesTests(path.data(), ';', fields, kNumFields, unitPreferencesTestDataLineFn, this, + errorCode); if (errorCode.errIfFailureAndReset("error parsing %s: %s\n", path.data(), u_errorName(errorCode))) { return; } From cd7b0cc58dd463dd67a3cb4451794911389c42c5 Mon Sep 17 00:00:00 2001 From: Hugo van der Merwe <17109322+hugovdm@users.noreply.github.com> Date: Thu, 12 Mar 2020 18:34:25 +0100 Subject: [PATCH 4/7] Make compound parsing work. --- icu4c/source/test/intltest/unitstest.cpp | 183 +++++++++++++++++------ 1 file changed, 134 insertions(+), 49 deletions(-) diff --git a/icu4c/source/test/intltest/unitstest.cpp b/icu4c/source/test/intltest/unitstest.cpp index 962f8251b7b..488fac39230 100644 --- a/icu4c/source/test/intltest/unitstest.cpp +++ b/icu4c/source/test/intltest/unitstest.cpp @@ -173,8 +173,9 @@ void UnitsTest::testArea() { } /** - * Returns a StringPiece pointing at the given field with space prefixes and - * postfixes trimmed off. + * Trims whitespace (spaces only) off of the specified string. + * @param field is two pointers pointing at the start and end of the string. + * @return A StringPiece with initial and final space characters trimmed off. */ StringPiece trimField(char *(&field)[2]) { char *start = field[0]; @@ -257,42 +258,138 @@ void UnitsTest::testConversions() { } } +/** + * This class represents the output fields from unitPreferencesTest.txt. Please + * see the documentation at the top of that file for details. + * + * For "mixed units" output, there are more (repeated) output fields. The last + * output unit has the expected output specified as both a rational fraction and + * a decimal fraction. This class ignores rational fractions, and expects to + * find a decimal fraction for each output unit. + */ +class ExpectedOutput { + private: + // Counts number of units in the output. When this is more than one, we have + // "mixed units" in the expected output. + int _compoundCount = 0; + + // Counts how many fields were skipped: we expect to skip only one per + // output unit type (the rational fraction). + int _skippedFields = 0; + + // The expected output units: more than one for "mixed units". + MeasureUnit _measureUnits[3]; + + // The amounts of each of the output units. + double _amounts[3]; + + public: + /** + * Parse an expected output field from the test data file. + * @param output may be a string representation of an integer, a rational + * fraction, a decimal fraction, or it may be a unit identifier. Whitespace + * should already be trimmed. This function ignores rational fractions, + * saving only decimal fractions and their unit identifiers. + * @return true if the field was successfully parsed, false if parsing + * failed. + */ + UBool parseOutputField(StringPiece output) { + DecimalQuantity dqOutputD; + UErrorCode errorCode = U_ZERO_ERROR; + + dqOutputD.setToDecNumber(output, errorCode); + if (U_SUCCESS(errorCode)) { + _amounts[_compoundCount] = dqOutputD.toDouble(); + return true; + } else if (errorCode == U_DECIMAL_NUMBER_SYNTAX_ERROR) { + errorCode = U_ZERO_ERROR; + } else { + return false; + } + + _measureUnits[_compoundCount] = MeasureUnit::forIdentifier(output, errorCode); + if (U_SUCCESS(errorCode)) { + _compoundCount++; + _skippedFields = 0; + return true; + } + _skippedFields++; + if (_skippedFields < 2) { + return true; + } else { + return false; + } + } + + /** + * Produces an output string for debug purposes. + * @param dbgOutput the buffer to which to write the output string. + * @param dbgOutputSize the size of the buffer. + * @returns a null-terminated string containing each amount and unit. + */ + void toDebugStringN(char *dbgOutput, int dbgOutputSize) { + char *next = dbgOutput; + char *end = dbgOutput + dbgOutputSize; + for (int i = 0; i < _compoundCount; i++) { + int printlen = + snprintf(next, dbgOutputSize, "%f %s ", _amounts[i], _measureUnits[i].getIdentifier()); + next += printlen; + dbgOutputSize -= printlen; + if (dbgOutputSize <= 0) { break; } + } + // Terminate string with a null. + if (next >= end) { + dbgOutput[dbgOutputSize - 1] = 0; + } else { + *next = 0; + } + } +}; + /** * WIP(hugovdm): deals with a single data-driven unit test for unit preferences. * This is a UParseLineFn as required by u_parseDelimitedFile. */ void unitPreferencesTestDataLineFn(void *context, char *fields[][2], int32_t fieldCount, UErrorCode *pErrorCode) { - (void)fieldCount; // unused UParseLineFn variable + if (U_FAILURE(*pErrorCode)) return; + UnitsTest *intltest = (UnitsTest *)context; IcuTestErrorCode status(*(UnitsTest *)context, "unitPreferencesTestDatalineFn"); + if (!intltest->assertTrue(u"unitPreferencesTestDataLineFn expects 9 fields for simple and 11 " + u"fields for compound. Other field counts not yet supported. ", + fieldCount == 9 || fieldCount == 11)) { + return; + } + StringPiece quantity = trimField(fields[0]); StringPiece usage = trimField(fields[1]); StringPiece region = trimField(fields[2]); StringPiece inputR = trimField(fields[3]); StringPiece inputD = trimField(fields[4]); StringPiece inputUnit = trimField(fields[5]); - StringPiece outputR = trimField(fields[6]); - StringPiece outputD = trimField(fields[7]); - StringPiece outputUnit = trimField(fields[8]); + ExpectedOutput output; + for (int i = 6; i < fieldCount; i++) { + output.parseOutputField(trimField(fields[i])); + } - DecimalQuantity dqOutputD; - dqOutputD.setToDecNumber(outputD, status); - if (status.errIfFailureAndReset("parsing decimal quantity: \"%.*s\"", outputD.length(), - outputD.data())) { + DecimalQuantity dqInputD; + dqInputD.setToDecNumber(inputD, status); + if (status.errIfFailureAndReset("parsing decimal quantity: \"%.*s\"", inputD.length(), + inputD.data())) { + *pErrorCode = U_PARSE_ERROR; return; } - double expectedOutput = dqOutputD.toDouble(); + double inputAmount = dqInputD.toDouble(); - MeasureUnit input = MeasureUnit::forIdentifier(inputUnit, status); + MeasureUnit inputMeasureUnit = MeasureUnit::forIdentifier(inputUnit, status); if (status.errIfFailureAndReset("forIdentifier(\"%.*s\")", inputUnit.length(), inputUnit.data())) { + *pErrorCode = U_PARSE_ERROR; return; } - MeasureUnit output = MeasureUnit::forIdentifier(outputUnit, status); - if (status.errIfFailureAndReset("forIdentifier(\"%.*s\")", outputUnit.length(), outputUnit.data())) { - return; - } + char debugCompoundOutput[100] = "TEST"; + output.toDebugStringN(debugCompoundOutput, sizeof(debugCompoundOutput)); // WIP(hugovdm): hook this up to actual tests. // @@ -304,18 +401,22 @@ void unitPreferencesTestDataLineFn(void *context, char *fields[][2], int32_t fie // In the meantime, printing to stderr. fprintf(stderr, "Quantity (Category): \"%.*s\", Usage: \"%.*s\", Region: \"%.*s\", " - "Input: %.*s %.*s (%.*s), Output: %.*s %.*s (%.*s) - Expected: %f\n", + "Input: \"%f %s\", Expected Output: %s\n", quantity.length(), quantity.data(), usage.length(), usage.data(), region.length(), - region.data(), inputD.length(), inputD.data(), inputUnit.length(), inputUnit.data(), - inputR.length(), inputR.data(), outputD.length(), outputD.data(), outputUnit.length(), - outputUnit.data(), outputR.length(), outputR.data(), expectedOutput); + region.data(), inputAmount, inputMeasureUnit.getIdentifier(), debugCompoundOutput); } -// WIP(hugovdm): we need to replace u_parseDelimitedFile with something -// custom, because not all lines in unitPreferencesTest.txt have the same -// number of fields. -void parsePreferencesTests(const char *filename, char delimiter, char *fields[][2], int32_t fieldCount, - UParseLineFn *lineFn, void *context, UErrorCode *pErrorCode) { +/** + * Parses the format used by unitPreferencesTest.txt, calling lineFn for each + * line. + * + * This is a modified version of u_parseDelimitedFile, customised for + * unitPreferencesTest.txt, due to it having a variable number of fields per + * line. + */ +void parsePreferencesTests(const char *filename, char delimiter, char *fields[][2], + int32_t maxFieldCount, UParseLineFn *lineFn, void *context, + UErrorCode *pErrorCode) { FileStream *file; char line[10000]; char *start, *limit; @@ -323,7 +424,7 @@ void parsePreferencesTests(const char *filename, char delimiter, char *fields[][ if (U_FAILURE(*pErrorCode)) { return; } - if (fields == NULL || lineFn == NULL || fieldCount <= 0) { + if (fields == NULL || lineFn == NULL || maxFieldCount <= 0) { *pErrorCode = U_ILLEGAL_ARGUMENT_ERROR; return; } @@ -343,18 +444,6 @@ void parsePreferencesTests(const char *filename, char delimiter, char *fields[][ /* remove trailing newline characters */ length = (int32_t)(u_rtrim(line) - line); - // FYI: uparse.cpp had this code, which we're skipping here: - // /* - // * detect a line with # @missing: - // * start parsing after that, or else from the beginning of the line - // * set the default warning for @missing lines - // */ - // start = (char *)getMissingLimit(line); - // if (start == line) { - // *pErrorCode = U_ZERO_ERROR; - // } else { - // *pErrorCode = U_USING_DEFAULT_WARNING; - // } start = line; *pErrorCode = U_ZERO_ERROR; @@ -377,7 +466,7 @@ void parsePreferencesTests(const char *filename, char delimiter, char *fields[][ if (u_skipWhitespace(start)[0] == 0) { continue; } /* for each field, call the corresponding field function */ - for (i = 0; i < fieldCount; ++i) { + for (i = 0; i < maxFieldCount; ++i) { /* set the limit pointer of this field */ limit = start; while (*limit != delimiter && *limit != 0) { @@ -392,16 +481,12 @@ void parsePreferencesTests(const char *filename, char delimiter, char *fields[][ start = limit; if (*start != 0) { ++start; - } else if (i + 1 < fieldCount) { - *pErrorCode = U_PARSE_ERROR; - limit = line + length; - i = fieldCount; + } else { break; } } - - /* too few fields? */ - if (U_FAILURE(*pErrorCode)) { break; } + if (i == maxFieldCount) { *pErrorCode = U_PARSE_ERROR; } + int fieldCount = i + 1; /* call the field function */ lineFn(context, fields, fieldCount, pErrorCode); @@ -416,8 +501,8 @@ void parsePreferencesTests(const char *filename, char delimiter, char *fields[][ */ void UnitsTest::testPreferences() { const char *filename = "unitPreferencesTest.txt"; - const int32_t kNumFields = 9; - char *fields[kNumFields][2]; + const int32_t maxFields = 11; + char *fields[maxFields][2]; IcuTestErrorCode errorCode(*this, "UnitsTest::testPreferences"); const char *sourceTestDataPath = getSourceTestData(errorCode); @@ -430,7 +515,7 @@ void UnitsTest::testPreferences() { path.appendPathPart("units", errorCode); path.appendPathPart(filename, errorCode); - parsePreferencesTests(path.data(), ';', fields, kNumFields, unitPreferencesTestDataLineFn, this, + parsePreferencesTests(path.data(), ';', fields, maxFields, unitPreferencesTestDataLineFn, this, errorCode); if (errorCode.errIfFailureAndReset("error parsing %s: %s\n", path.data(), u_errorName(errorCode))) { return; From 92f57fcdb3130aea9d09bd534e679a44203bf029 Mon Sep 17 00:00:00 2001 From: Hugo van der Merwe <17109322+hugovdm@users.noreply.github.com> Date: Mon, 16 Mar 2020 10:59:43 +0100 Subject: [PATCH 5/7] ICU-Style: use UErrorCode& parameter instead of returning UBool. --- icu4c/source/test/intltest/unitstest.cpp | 27 +++++++++++++++++------- 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/icu4c/source/test/intltest/unitstest.cpp b/icu4c/source/test/intltest/unitstest.cpp index 488fac39230..4a7844311de 100644 --- a/icu4c/source/test/intltest/unitstest.cpp +++ b/icu4c/source/test/intltest/unitstest.cpp @@ -286,6 +286,7 @@ class ExpectedOutput { public: /** * Parse an expected output field from the test data file. + * * @param output may be a string representation of an integer, a rational * fraction, a decimal fraction, or it may be a unit identifier. Whitespace * should already be trimmed. This function ignores rational fractions, @@ -293,31 +294,38 @@ class ExpectedOutput { * @return true if the field was successfully parsed, false if parsing * failed. */ - UBool parseOutputField(StringPiece output) { + void parseOutputField(StringPiece output, UErrorCode &errorCode) { + if (U_FAILURE(errorCode)) return; DecimalQuantity dqOutputD; - UErrorCode errorCode = U_ZERO_ERROR; dqOutputD.setToDecNumber(output, errorCode); if (U_SUCCESS(errorCode)) { _amounts[_compoundCount] = dqOutputD.toDouble(); - return true; + return; } else if (errorCode == U_DECIMAL_NUMBER_SYNTAX_ERROR) { + // Not a decimal fraction, it might be a rational fraction or a unit + // identifier: continue. errorCode = U_ZERO_ERROR; } else { - return false; + // Unexpected error, so we propagate it. + return; } _measureUnits[_compoundCount] = MeasureUnit::forIdentifier(output, errorCode); if (U_SUCCESS(errorCode)) { _compoundCount++; _skippedFields = 0; - return true; + return; } _skippedFields++; if (_skippedFields < 2) { - return true; + // We are happy skipping one field per output unit: we want to skip + // rational fraction fiels like "11 / 10". + errorCode = U_ZERO_ERROR; + return; } else { - return false; + // Propagate the error. + return; } } @@ -370,7 +378,10 @@ void unitPreferencesTestDataLineFn(void *context, char *fields[][2], int32_t fie StringPiece inputUnit = trimField(fields[5]); ExpectedOutput output; for (int i = 6; i < fieldCount; i++) { - output.parseOutputField(trimField(fields[i])); + output.parseOutputField(trimField(fields[i]), status); + } + if (status.errIfFailureAndReset("parsing unitPreferencesTestData.txt test case: %s", fields[0][0])) { + return; } DecimalQuantity dqInputD; From f07472cef07e48263bf4d6881251acf3cf060bdb Mon Sep 17 00:00:00 2001 From: Hugo van der Merwe <17109322+hugovdm@users.noreply.github.com> Date: Mon, 16 Mar 2020 11:33:56 +0100 Subject: [PATCH 6/7] Use a toDebugString that returns a std::string for simplicity. --- icu4c/source/test/intltest/unitstest.cpp | 30 +++++++----------------- 1 file changed, 9 insertions(+), 21 deletions(-) diff --git a/icu4c/source/test/intltest/unitstest.cpp b/icu4c/source/test/intltest/unitstest.cpp index 4a7844311de..dba5498524c 100644 --- a/icu4c/source/test/intltest/unitstest.cpp +++ b/icu4c/source/test/intltest/unitstest.cpp @@ -331,26 +331,16 @@ class ExpectedOutput { /** * Produces an output string for debug purposes. - * @param dbgOutput the buffer to which to write the output string. - * @param dbgOutputSize the size of the buffer. - * @returns a null-terminated string containing each amount and unit. */ - void toDebugStringN(char *dbgOutput, int dbgOutputSize) { - char *next = dbgOutput; - char *end = dbgOutput + dbgOutputSize; + std::string toDebugString() { + std::string result; for (int i = 0; i < _compoundCount; i++) { - int printlen = - snprintf(next, dbgOutputSize, "%f %s ", _amounts[i], _measureUnits[i].getIdentifier()); - next += printlen; - dbgOutputSize -= printlen; - if (dbgOutputSize <= 0) { break; } - } - // Terminate string with a null. - if (next >= end) { - dbgOutput[dbgOutputSize - 1] = 0; - } else { - *next = 0; + result += std::to_string(_amounts[i]); + result += " "; + result += _measureUnits[i].getIdentifier(); + result += " "; } + return result; } }; @@ -399,9 +389,6 @@ void unitPreferencesTestDataLineFn(void *context, char *fields[][2], int32_t fie return; } - char debugCompoundOutput[100] = "TEST"; - output.toDebugStringN(debugCompoundOutput, sizeof(debugCompoundOutput)); - // WIP(hugovdm): hook this up to actual tests. // // Possible after merging in younies/tryingdouble: @@ -414,7 +401,8 @@ void unitPreferencesTestDataLineFn(void *context, char *fields[][2], int32_t fie "Quantity (Category): \"%.*s\", Usage: \"%.*s\", Region: \"%.*s\", " "Input: \"%f %s\", Expected Output: %s\n", quantity.length(), quantity.data(), usage.length(), usage.data(), region.length(), - region.data(), inputAmount, inputMeasureUnit.getIdentifier(), debugCompoundOutput); + region.data(), inputAmount, inputMeasureUnit.getIdentifier(), + output.toDebugString().c_str()); } /** From a3a41c822c5aa49ad523dfb154dc27f0005fa6b2 Mon Sep 17 00:00:00 2001 From: Hugo van der Merwe <17109322+hugovdm@users.noreply.github.com> Date: Mon, 16 Mar 2020 12:00:57 +0100 Subject: [PATCH 7/7] Remove two unused variables. --- icu4c/source/test/intltest/unitstest.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/icu4c/source/test/intltest/unitstest.cpp b/icu4c/source/test/intltest/unitstest.cpp index dba5498524c..41d761132fa 100644 --- a/icu4c/source/test/intltest/unitstest.cpp +++ b/icu4c/source/test/intltest/unitstest.cpp @@ -363,7 +363,7 @@ void unitPreferencesTestDataLineFn(void *context, char *fields[][2], int32_t fie StringPiece quantity = trimField(fields[0]); StringPiece usage = trimField(fields[1]); StringPiece region = trimField(fields[2]); - StringPiece inputR = trimField(fields[3]); + // Unused // StringPiece inputR = trimField(fields[3]); StringPiece inputD = trimField(fields[4]); StringPiece inputUnit = trimField(fields[5]); ExpectedOutput output; @@ -419,7 +419,7 @@ void parsePreferencesTests(const char *filename, char delimiter, char *fields[][ FileStream *file; char line[10000]; char *start, *limit; - int32_t i, length; + int32_t i; if (U_FAILURE(*pErrorCode)) { return; } @@ -441,7 +441,7 @@ void parsePreferencesTests(const char *filename, char delimiter, char *fields[][ while (T_FileStream_readLine(file, line, sizeof(line)) != NULL) { /* remove trailing newline characters */ - length = (int32_t)(u_rtrim(line) - line); + u_rtrim(line); start = line; *pErrorCode = U_ZERO_ERROR;