[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