[Merge] lp:~tiagosh/telephony-service/use-libphonenumber into lp:telephony-service

Gustavo Pichorim Boiko gustavo.boiko at canonical.com
Fri Aug 7 19:07:38 UTC 2015


Review: Needs Fixing

Just one request, other than that the code looks good!

Diff comments:

> 
> === modified file 'tests/libtelephonyservice/PhoneUtilsTest.cpp'
> --- tests/libtelephonyservice/PhoneUtilsTest.cpp	2015-06-17 19:31:32 +0000
> +++ tests/libtelephonyservice/PhoneUtilsTest.cpp	2015-08-05 15:19:23 +0000
> @@ -62,23 +61,20 @@
>  {
>      QTest::addColumn<QString>("number1");
>      QTest::addColumn<QString>("number2");
> -    QTest::addColumn<bool>("expectedResult");
> +    QTest::addColumn<PhoneUtils::PhoneNumberMatchType>("expectedResult");
>  
> -    QTest::newRow("string equal") << "12345678" << "12345678" << true;
> -    QTest::newRow("number with dash") << "1234-5678" << "12345678" << true;
> -    QTest::newRow("number with area code") << "12312345678" << "12345678" << true;
> -    QTest::newRow("number with extension") << "12345678#123" << "12345678" << false;
> -    QTest::newRow("both numbers with extension") << "(123)12345678#1" << "12345678#1" << true;
> -    QTest::newRow("numbers with different extension") << "1234567#1" << "1234567#2" << false;
> -    QTest::newRow("number with comma") << "33333333,1,1" << "33333333" << true;
> -    QTest::newRow("both numbers with comma") << "22222222,1" << "22222222,2,1" << true;
> -    QTest::newRow("number with semicolon") << "33333333;1" << "33333333" << true;
> -    QTest::newRow("both numbers with semicolon") << "22222222;1" << "22222222;2" << true;
> -    QTest::newRow("short/emergency numbers") << "190" << "190" << true;
> -    QTest::newRow("different numbers") << "12345678" << "1234567" << false;
> -    QTest::newRow("both non phone numbers") << "abcdefg" << "abcdefg" << true;
> -    QTest::newRow("different non phone numbers") << "abcdefg" << "bcdefg" << false;
> -    QTest::newRow("phone number and custom string") << "abc12345678" << "12345678" << false;
> +    QTest::newRow("string equal") << "12345678" << "12345678" << PhoneUtils::NSN_MATCH;
> +    QTest::newRow("number with dash") << "1234-5678" << "12345678" << PhoneUtils::NSN_MATCH;
> +    QTest::newRow("number with area code") << "1231234567" << "1234567" << PhoneUtils::SHORT_NSN_MATCH;
> +    QTest::newRow("number with extension") << "12345678#123" << "12345678" << PhoneUtils::SHORT_NSN_MATCH;
> +    QTest::newRow("both numbers with extension") << "(123)12345678#1" << "12345678#1" << PhoneUtils::SHORT_NSN_MATCH;
> +    QTest::newRow("numbers with different extension") << "1234567#1" << "1234567#2" << PhoneUtils::NO_MATCH;
> +    QTest::newRow("short/emergency numbers") << "190" << "190" << PhoneUtils::EXACT_MATCH;
> +    QTest::newRow("different short/emergency numbers") << "911" << "11" << PhoneUtils::NO_MATCH;
> +    QTest::newRow("different numbers") << "12345678" << "1234567" << PhoneUtils::NO_MATCH;
> +    QTest::newRow("both non phone numbers") << "abcdefg" << "abcdefg" << PhoneUtils::EXACT_MATCH;
> +    QTest::newRow("different non phone numbers") << "abcdefg" << "bcdefg" << PhoneUtils::INVALID_NUMBER;
> +    QTest::newRow("phone number and custom string") << "abc12345678" << "12345678" << PhoneUtils::NSN_MATCH;

Can you just add an entry to cover bug #1462090 (numbers with slashes)?

>      // FIXME: check what other cases we need to test here"
>  }
>  


-- 
https://code.launchpad.net/~tiagosh/telephony-service/use-libphonenumber/+merge/264906
Your team Ubuntu Phablet Team is subscribed to branch lp:telephony-service.



More information about the Ubuntu-reviews mailing list