Hi To test my recent work on changing OpenOffice to call ICU only through its C API, I tried to get the i18n unit tests in main/qadevOOo working.
-------------------------------------------------------- Porting main/qadevOOo tests to test/ -------------------------------------------------------- I debated whether to get them working in their old test framework, or to port them to our new one in test/, and chose the latter for many reasons. The test/ unit tests are by far our best developed test framework, while others are barely or completely used, never regularly run, break when run in certain ways. It is harder to get subsequent tests to use a different OpenOffice instance, while the test/ tests do so easily. A lot of the code I wrote in main/module/qa tests felt repetitive, and is a poor imitation of the test/ code. And test/ has good reporting, but module tests have almost none. https://wiki.openoffice.org/wiki/Test_Refactor (which I think was written by IBM in the early days of AOO) was definitely in favour of moving qadevOOo tests to test/ (point 8 of the TODO is "Convert the test scripts in qadevooo into testuno"). And I got it working. All 11 of the i18n Java unit test files and 101 test cases they contain are now in test/testuno/source/api/i18n. They've been ported from their old test framework to the new JUnit-based one we use in test/testuno. One i18n unit test I ported from main/qadevOoo helped to find a serious bug in my icu-c-api branch, where a previous call to "delete body" wasn't changed to "ical_close(body)" like it should have been, resulting in memory corruption and a crash of OpenOffice. I quickly fixed this once I found it, but it's a good example of why good unit tests matter. The remaining pool of tests in main/qadevOOo currently contains 710 files and over 2000 test cases, so I only ported the first 1.53%. Note that these tests were never run before. They are supposed to run from main/framework/qa/complex/api_internal, but this was broken ever since AOO began. This is a substantial pool of unit tests that could be ported, which would improve our test coverage considerably. These tests are however in a truly dreadful state. They use a strange legacy custom testing framework instead of JUnit. Instead of using fine-grained assertions for each check, they repeatedly update one boolean variable, and then make a single assertion (with tRes.tested("string", boolean)) at the end of the method. Test methods have to be executed in a certain order (specified by eg. requiredMethod("nextWord()");), a terrible practice that is avoided in modern unit test frameworks, and I've largely eliminated this when porting these i18n tests. Also these tests were divided into a "mod" in main/qadevOOo/java/OOoRunner/src/main/java/mod/ that creates test objects, and then the "ifc" in main/qadevOOo/java/OOoRunner/src/main/java/ifc/ which contains test cases, and how "mod" and "ifc" link is specified through CSV files in main/qadevOOo/java/OOoRunner/src/main/resources/objdsc. Method names prefixed with the underscore ("_") are tests; in JUnit we use @Test instead. But even more serious were the many bugs in these tests that caused test failures which I investigated and fixed: - Fix the XCharacterClassificationTest._getCharacterDirection() test by using 43 (the '+' sign) instead of 47 ('/') when testing for the DirectionProperty.EUROPEAN_NUMBER_SEPARATOR property. - Fix the test failures in XCharacterClassificationTest._getType(). Some of the hardcoded characters no longer had the type they were originally expected to have, probably due to changes in new Unicode versions. Replace them with other characters that have the expected types now. - Fix the test failure in XCharacterClassificationTest._getCharacterType(). The ')' doesn't have type PRINTABLE any longer, so use '*' instead which does have it. - Fix the error and failure in the XCharacterClassificationTest._getScript() unit test. Don't check the surrogate pairs, which getScript() doesn't seem to identify correctly. Also verify the script is in range of our knownscript value range to avoid ArrayIndexOutOfBoundsException. - Fix the test failures in XBreakIteratorTest._isBeginWord() and _isEndWord(). It was failing because the word type differed: it used WordType.ANY_WORD when calling XBreakIterator.isBeginWord() and XBreakIterator.isEndWord(), but used WordType.ANYWORD_IGNOREWHITESPACES instead when determining word boundaries to test in nextWord(). - Test for October properly. Month 10 will fail both of these tests else if (month > 6 && month < 10) { quarter = "Q3"; longQuarter = "3rd quarter"; } else if (month > 10 && month < 13) {quarter = "Q4"; longQuarter = "4th quarter"; } as 10 is neither greater nor lesser than 10. Rewrite the comparison to include 10 properly, and clean it up. There is another related issue I am investigating in https://bz.apache.org/ooo/show_bug.cgi?id=87590 and the test that reproduces it has been @Ignored for now. We need to upgrade to a newer version of Unicode to fix it. I am not sure whether we should update our case folding tables which are only used in a single place, or delete them and call the ICU functions for case folding instead. ------------------------------------------------- The new "api" test type in tests/ ------------------------------------------------- In addition to the previous "bvt", "fvt" and "pvt" tests, there is now a new "api" test type I've created as well, containing these tests ported from main/qadevOOo. --------------------------------------- tests/ now require Java 7 --------------------------------------- It is too painful working with earlier versions, and we should probably require Java 8 even. --------------- In closing --------------- My changes have been pushed to trunk. Thank you Damjan
