From 09822cf77cdbe32b03553cd05154100b5f2591d0 Mon Sep 17 00:00:00 2001 From: Tomaž Vajngerl Date: Thu, 19 May 2022 00:11:17 +0900 Subject: comphelper: fix natural string compare function + reorganize tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Natural string compare function doesn't take into account that the string can start with a number and in this case it treats it like a conventional string compare. This change takes this case into account. This change also refactores the tests for NaturalStringSorter class. The previous tet used a mock XBreakIterator and XCollator implementations to test the functionallity. This is not needed as we can just use a real one instead, which makes the test more real as it actually uses a real implementation instead of a mock implementation, which could differ. This change removes the mock XCollator and XBreakIterator implementations and moves the test into a new file - NaturalStringSortTest.cxx The test is also extended with the new use case where the string starts with a number. Change-Id: I32ea055f914c2947e4d979093b32f56170a61102 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/134540 Tested-by: Jenkins Reviewed-by: Tomaž Vajngerl --- comphelper/qa/string/NaturalStringSortTest.cxx | 95 +++++++++++++ comphelper/qa/string/test_string.cxx | 188 ------------------------- 2 files changed, 95 insertions(+), 188 deletions(-) create mode 100644 comphelper/qa/string/NaturalStringSortTest.cxx (limited to 'comphelper/qa') diff --git a/comphelper/qa/string/NaturalStringSortTest.cxx b/comphelper/qa/string/NaturalStringSortTest.cxx new file mode 100644 index 000000000000..bfdcaff6e13c --- /dev/null +++ b/comphelper/qa/string/NaturalStringSortTest.cxx @@ -0,0 +1,95 @@ +/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */ +/* + * This file is part of the LibreOffice project. + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. + * + * This file incorporates work covered by the following license notice: + * + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed + * with this work for additional information regarding copyright + * ownership. The ASF licenses this file to you under the Apache + * License, Version 2.0 (the "License"); you may not use this file + * except in compliance with the License. You may obtain a copy of + * the License at http://www.apache.org/licenses/LICENSE-2.0 . + */ + +#include +#include +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include + +using namespace css; + +namespace +{ +class TestStringNaturalCompare : public test::BootstrapFixtureBase +{ +public: + void testNatural() + { + lang::Locale aLocale; + aLocale.Language = "en"; + aLocale.Country = "US"; + + comphelper::string::NaturalStringSorter aSorter(comphelper::getProcessComponentContext(), + aLocale); + + CPPUNIT_ASSERT_EQUAL(sal_Int32(+0), aSorter.compare("ABC", "ABC")); + CPPUNIT_ASSERT_EQUAL(sal_Int32(+1), aSorter.compare("ABC", "abc")); + CPPUNIT_ASSERT_EQUAL(sal_Int32(-1), aSorter.compare("abc", "ABC")); + CPPUNIT_ASSERT_EQUAL(sal_Int32(+1), aSorter.compare("alongstring", "alongerstring")); + CPPUNIT_ASSERT_EQUAL(sal_Int32(-1), aSorter.compare("alongerstring", "alongstring")); + CPPUNIT_ASSERT_EQUAL(sal_Int32(-1), aSorter.compare("Heading 9", "Heading 10")); + CPPUNIT_ASSERT_EQUAL(sal_Int32(+1), aSorter.compare("Heading 10", "Heading 9")); + CPPUNIT_ASSERT_EQUAL(sal_Int32(-1), aSorter.compare("July, the 4th", "July, the 10th")); + CPPUNIT_ASSERT_EQUAL(sal_Int32(+1), aSorter.compare("July, the 10th", "July, the 4th")); + CPPUNIT_ASSERT_EQUAL(sal_Int32(-1), aSorter.compare("abc08", "abc010")); + CPPUNIT_ASSERT_EQUAL(sal_Int32(+1), aSorter.compare("abc010", "abc08")); + CPPUNIT_ASSERT_EQUAL(sal_Int32(+0), aSorter.compare("apple10apple", "apple10apple")); + + CPPUNIT_ASSERT_EQUAL(sal_Int32(+1), aSorter.compare("KA1", "KA0")); + CPPUNIT_ASSERT_EQUAL(sal_Int32(+0), aSorter.compare("KA1", "KA1")); + CPPUNIT_ASSERT_EQUAL(sal_Int32(-1), aSorter.compare("KA1", "KA2")); + CPPUNIT_ASSERT_EQUAL(sal_Int32(+1), aSorter.compare("KA50", "KA5")); + CPPUNIT_ASSERT_EQUAL(sal_Int32(-1), aSorter.compare("KA50", "KA100")); + + CPPUNIT_ASSERT_EQUAL(sal_Int32(+1), aSorter.compare("1", "0")); + CPPUNIT_ASSERT_EQUAL(sal_Int32(+0), aSorter.compare("1", "1")); + CPPUNIT_ASSERT_EQUAL(sal_Int32(-1), aSorter.compare("1", "2")); + CPPUNIT_ASSERT_EQUAL(sal_Int32(+1), aSorter.compare("11", "1")); + + CPPUNIT_ASSERT_EQUAL(sal_Int32(-1), aSorter.compare("50", "100")); + CPPUNIT_ASSERT_EQUAL(sal_Int32(-1), aSorter.compare("0", "100000")); + + CPPUNIT_ASSERT_EQUAL(sal_Int32(-1), aSorter.compare("0", "A")); + CPPUNIT_ASSERT_EQUAL(sal_Int32(+1), aSorter.compare("A", "0")); + CPPUNIT_ASSERT_EQUAL(sal_Int32(+1), aSorter.compare("A", "99")); + + CPPUNIT_ASSERT_EQUAL(sal_Int32(+1), aSorter.compare("00ABC2", "00ABC1")); + CPPUNIT_ASSERT_EQUAL(sal_Int32(-1), aSorter.compare("00ABC1", "00ABC2")); + CPPUNIT_ASSERT_EQUAL(sal_Int32(+1), aSorter.compare("00ABC11", "00ABC2")); + CPPUNIT_ASSERT_EQUAL(sal_Int32(-1), aSorter.compare("00ABC2", "00ABC11")); + } + + CPPUNIT_TEST_SUITE(TestStringNaturalCompare); + CPPUNIT_TEST(testNatural); + CPPUNIT_TEST_SUITE_END(); +}; + +CPPUNIT_TEST_SUITE_REGISTRATION(TestStringNaturalCompare); +} + +/* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/comphelper/qa/string/test_string.cxx b/comphelper/qa/string/test_string.cxx index 5d3132756ad5..0a9850ed920f 100644 --- a/comphelper/qa/string/test_string.cxx +++ b/comphelper/qa/string/test_string.cxx @@ -35,7 +35,6 @@ namespace { class TestString: public CppUnit::TestFixture { public: - void testNatural(); void testStripStart(); void testStripEnd(); void testStrip(); @@ -48,7 +47,6 @@ public: void testRemoveAny(); CPPUNIT_TEST_SUITE(TestString); - CPPUNIT_TEST(testNatural); CPPUNIT_TEST(testStripStart); CPPUNIT_TEST(testStripEnd); CPPUNIT_TEST(testStrip); @@ -82,192 +80,6 @@ void TestString::testIsdigitAsciiString() CPPUNIT_ASSERT_EQUAL(true, comphelper::string::isdigitAsciiString("")); } -using namespace ::com::sun::star; - -class testCollator : public cppu::WeakImplHelper< i18n::XCollator > -{ -public: - virtual sal_Int32 SAL_CALL compareSubstring( - const OUString& str1, sal_Int32 off1, sal_Int32 len1, - const OUString& str2, sal_Int32 off2, sal_Int32 len2) override - { - return str1.copy(off1, len1).compareTo(str2.subView(off2, len2)); - } - virtual sal_Int32 SAL_CALL compareString( - const OUString& str1, - const OUString& str2) override - { - return str1.compareTo(str2); - } - virtual sal_Int32 SAL_CALL loadDefaultCollator(const lang::Locale&, sal_Int32) override {return 0;} - virtual sal_Int32 SAL_CALL loadCollatorAlgorithm(const OUString&, - const lang::Locale&, sal_Int32) override {return 0;} - virtual void SAL_CALL loadCollatorAlgorithmWithEndUserOption(const OUString&, - const lang::Locale&, const uno::Sequence< sal_Int32 >&) override {} - virtual uno::Sequence< OUString > SAL_CALL listCollatorAlgorithms(const lang::Locale&) override - { - return uno::Sequence< OUString >(); - } - virtual uno::Sequence< sal_Int32 > SAL_CALL listCollatorOptions(const OUString&) override - { - return uno::Sequence< sal_Int32 >(); - } -}; - -#define IS_DIGIT(CHAR) (((CHAR) >= 48) && ((CHAR <= 57))) - -class testBreakIterator : public cppu::WeakImplHelper< i18n::XBreakIterator > -{ -public: - virtual sal_Int32 SAL_CALL nextCharacters( const OUString&, sal_Int32, - const lang::Locale&, sal_Int16, sal_Int32, sal_Int32& ) override {return -1;} - virtual sal_Int32 SAL_CALL previousCharacters( const OUString&, sal_Int32, - const lang::Locale&, sal_Int16, sal_Int32, sal_Int32& ) override {return -1;} - - virtual i18n::Boundary SAL_CALL previousWord( const OUString&, sal_Int32, - const lang::Locale&, sal_Int16) override - { return i18n::Boundary(); } - virtual i18n::Boundary SAL_CALL nextWord( const OUString&, sal_Int32, - const lang::Locale&, sal_Int16) override - { return i18n::Boundary(); } - virtual i18n::Boundary SAL_CALL getWordBoundary( const OUString&, sal_Int32, - const lang::Locale&, sal_Int16, sal_Bool ) override - { return i18n::Boundary(); } - - virtual sal_Bool SAL_CALL isBeginWord( const OUString&, sal_Int32, - const lang::Locale&, sal_Int16 ) override - { return false; } - virtual sal_Bool SAL_CALL isEndWord( const OUString&, sal_Int32, - const lang::Locale& , sal_Int16 ) override - { return false; } - virtual sal_Int16 SAL_CALL getWordType( const OUString&, sal_Int32, - const lang::Locale& ) override - { return 0; } - - virtual sal_Int32 SAL_CALL beginOfSentence( const OUString&, sal_Int32, - const lang::Locale& ) override - { return 0; } - virtual sal_Int32 SAL_CALL endOfSentence( const OUString& rText, sal_Int32, - const lang::Locale& ) override - { return rText.getLength(); } - - virtual i18n::LineBreakResults SAL_CALL getLineBreak( const OUString&, sal_Int32, - const lang::Locale&, sal_Int32, - const i18n::LineBreakHyphenationOptions&, - const i18n::LineBreakUserOptions&) override - { - return i18n::LineBreakResults(); - } - - virtual sal_Int16 SAL_CALL getScriptType( const OUString&, sal_Int32 ) override { return -1; } - virtual sal_Int32 SAL_CALL beginOfScript( const OUString&, sal_Int32, - sal_Int16 ) override { return -1; } - virtual sal_Int32 SAL_CALL endOfScript( const OUString&, sal_Int32, - sal_Int16 ) override { return -1; } - virtual sal_Int32 SAL_CALL previousScript( const OUString&, sal_Int32, - sal_Int16 ) override { return -1; } - virtual sal_Int32 SAL_CALL nextScript( const OUString&, sal_Int32, - sal_Int16 ) override { return -1; } - - virtual sal_Int32 SAL_CALL beginOfCharBlock( const OUString&, sal_Int32, - const lang::Locale&, sal_Int16 ) override { return -1; } - virtual sal_Int32 SAL_CALL endOfCharBlock( const OUString& rText, sal_Int32 nStartPos, - const lang::Locale&, sal_Int16 CharType ) override - { - const sal_Unicode *pStr = rText.getStr()+nStartPos; - for (sal_Int32 nI = nStartPos; nI < rText.getLength(); ++nI) - { - if (CharType == i18n::CharType::DECIMAL_DIGIT_NUMBER && !IS_DIGIT(*pStr)) - return nI; - else if (CharType != i18n::CharType::DECIMAL_DIGIT_NUMBER && IS_DIGIT(*pStr)) - return nI; - ++pStr; - } - return -1; - } - virtual sal_Int32 SAL_CALL previousCharBlock( const OUString&, sal_Int32, - const lang::Locale&, sal_Int16 ) override { return -1; } - virtual sal_Int32 SAL_CALL nextCharBlock( const OUString& rText, sal_Int32 nStartPos, - const lang::Locale&, sal_Int16 CharType ) override - { - const sal_Unicode *pStr = rText.getStr()+nStartPos; - for (sal_Int32 nI = nStartPos; nI < rText.getLength(); ++nI) - { - if (CharType == i18n::CharType::DECIMAL_DIGIT_NUMBER && IS_DIGIT(*pStr)) - return nI; - else if (CharType != i18n::CharType::DECIMAL_DIGIT_NUMBER && !IS_DIGIT(*pStr)) - return nI; - ++pStr; - } - return -1; - } -}; - -void TestString::testNatural() -{ - using namespace comphelper::string; - - uno::Reference< i18n::XCollator > xCollator(new testCollator); - uno::Reference< i18n::XBreakIterator > xBI(new testBreakIterator); - -// --- Some generic tests to ensure we do not alter original behavior -// outside what we want - CPPUNIT_ASSERT_EQUAL( - static_cast(0), compareNatural("ABC", "ABC", xCollator, xBI, lang::Locale()) - ); - // Case sensitivity - CPPUNIT_ASSERT( - compareNatural("ABC", "abc", xCollator, xBI, lang::Locale()) < 0 - ); - // Reverse - CPPUNIT_ASSERT( - compareNatural("abc", "ABC", xCollator, xBI, lang::Locale()) > 0 - ); - // First shorter - CPPUNIT_ASSERT( - compareNatural("alongstring", "alongerstring", xCollator, xBI, lang::Locale()) > 0 - ); - // Second shorter - CPPUNIT_ASSERT( - compareNatural("alongerstring", "alongstring", xCollator, xBI, lang::Locale()) < 0 - ); -// -- Here we go on natural order, each one is followed by classic compare and the reverse comparison - // That's why we originally made the patch - CPPUNIT_ASSERT( - compareNatural("Heading 9", "Heading 10", xCollator, xBI, lang::Locale()) < 0 - ); - // Original behavior - CPPUNIT_ASSERT( - OUString("Heading 9").compareTo(u"Heading 10") > 0 - ); - CPPUNIT_ASSERT( - compareNatural("Heading 10", "Heading 9", xCollator, xBI, lang::Locale()) > 0 - ); - // Harder - CPPUNIT_ASSERT( - compareNatural("July, the 4th", "July, the 10th", xCollator, xBI, lang::Locale()) < 0 - ); - CPPUNIT_ASSERT( - OUString("July, the 4th").compareTo(u"July, the 10th") > 0 - ); - CPPUNIT_ASSERT( - compareNatural("July, the 10th", "July, the 4th", xCollator, xBI, lang::Locale()) > 0 - ); - // Hardest - CPPUNIT_ASSERT( - compareNatural("abc08", "abc010", xCollator, xBI, lang::Locale()) < 0 - ); - CPPUNIT_ASSERT( - OUString("abc08").compareTo(u"abc010") > 0 - ); - CPPUNIT_ASSERT( - compareNatural("abc010", "abc08", xCollator, xBI, lang::Locale()) > 0 - ); - CPPUNIT_ASSERT_EQUAL( - static_cast(0), compareNatural("apple10apple", "apple10apple", xCollator, xBI, lang::Locale()) - ); -} - void TestString::testStripStart() { OString aIn("abc"); -- cgit v1.2.3