From 3671725b58a9768016e141c77424dedb5fd2c55a Mon Sep 17 00:00:00 2001 From: Yorke Lee Date: Wed, 20 Nov 2013 15:02:49 -0800 Subject: Fix Dialer tests * Empty geocode is now " " instead of "-" per UX request * DialpadFragment now throws IllegalArgumentException instead of Log.wtf so that it can be tested * Added contact id column to contactsprovider query * Modified PhoneNumberDisplayHelper to take an instance of PhoneNumberUtilsWrapper so that it can be mocked out Fix label-related tests that were failing due to a change in how we treat empty labels Bug: 9111164 Change-Id: If2244586b9d09fa2839fa0ddfc9f369f9dc66e51 --- res/values/strings.xml | 5 +++-- src/com/android/dialer/PhoneCallDetailsHelper.java | 5 ++--- .../dialer/calllog/PhoneNumberDisplayHelper.java | 12 ++++++++--- .../android/dialer/dialpad/DialpadFragment.java | 10 ++++----- .../android/dialer/PhoneCallDetailsHelperTest.java | 8 ++++--- .../dialer/calllog/CallLogFragmentTest.java | 22 +++++++++++++------ .../dialer/calllog/CallLogListItemHelperTest.java | 5 ++++- .../calllog/TestPhoneNumberUtilsWrapper.java | 2 -- .../dialer/dialpad/DialpadFragmentTest.java | 6 +++++- .../interactions/PhoneNumberInteractionTest.java | 25 +++++++++++----------- 10 files changed, 62 insertions(+), 38 deletions(-) diff --git a/res/values/strings.xml b/res/values/strings.xml index 55cdea4de..9b534da33 100644 --- a/res/values/strings.xml +++ b/res/values/strings.xml @@ -456,8 +456,9 @@ Start voice search - - \u0020 + + Call %s diff --git a/src/com/android/dialer/PhoneCallDetailsHelper.java b/src/com/android/dialer/PhoneCallDetailsHelper.java index af0a5257e..2a4a1425c 100644 --- a/src/com/android/dialer/PhoneCallDetailsHelper.java +++ b/src/com/android/dialer/PhoneCallDetailsHelper.java @@ -63,8 +63,8 @@ public class PhoneCallDetailsHelper { PhoneNumberUtilsWrapper phoneUtils) { mResources = resources; mCallTypeHelper = callTypeHelper; - mPhoneNumberHelper = new PhoneNumberDisplayHelper(resources); mPhoneNumberUtilsWrapper = phoneUtils; + mPhoneNumberHelper = new PhoneNumberDisplayHelper(mPhoneNumberUtilsWrapper, resources); } /** Fills the call details views with content. */ @@ -122,7 +122,7 @@ public class PhoneCallDetailsHelper { nameText = displayNumber; if (TextUtils.isEmpty(details.geocode) || mPhoneNumberUtilsWrapper.isVoicemailNumber(details.number)) { - numberText = mResources.getString(R.string.call_log_empty_gecode); + numberText = mResources.getString(R.string.call_log_empty_geocode); } else { numberText = details.geocode; } @@ -137,7 +137,6 @@ public class PhoneCallDetailsHelper { } views.nameView.setText(nameText); - views.labelView.setText(labelText); views.labelView.setVisibility(TextUtils.isEmpty(labelText) ? View.GONE : View.VISIBLE); } diff --git a/src/com/android/dialer/calllog/PhoneNumberDisplayHelper.java b/src/com/android/dialer/calllog/PhoneNumberDisplayHelper.java index 81f3ca1e5..5d7ce7ea9 100644 --- a/src/com/android/dialer/calllog/PhoneNumberDisplayHelper.java +++ b/src/com/android/dialer/calllog/PhoneNumberDisplayHelper.java @@ -18,8 +18,8 @@ package com.android.dialer.calllog; import android.content.res.Resources; import android.provider.CallLog.Calls; -import android.telephony.PhoneNumberUtils; import android.text.TextUtils; +import android.util.Log; import com.android.dialer.R; @@ -27,10 +27,17 @@ import com.android.dialer.R; * Helper for formatting and managing the display of phone numbers. */ public class PhoneNumberDisplayHelper { + private final PhoneNumberUtilsWrapper mPhoneNumberUtils; private final Resources mResources; public PhoneNumberDisplayHelper(Resources resources) { mResources = resources; + mPhoneNumberUtils = new PhoneNumberUtilsWrapper(); + } + + public PhoneNumberDisplayHelper(PhoneNumberUtilsWrapper phoneNumberUtils, Resources resources) { + mPhoneNumberUtils = phoneNumberUtils; + mResources = resources; } /* package */ CharSequence getDisplayName(CharSequence number, int presentation) { @@ -43,7 +50,7 @@ public class PhoneNumberDisplayHelper { if (presentation == Calls.PRESENTATION_PAYPHONE) { return mResources.getString(R.string.payphone); } - if (new PhoneNumberUtilsWrapper().isVoicemailNumber(number)) { + if (mPhoneNumberUtils.isVoicemailNumber(number)) { return mResources.getString(R.string.voicemail); } if (PhoneNumberUtilsWrapper.isLegacyUnknownNumbers(number)) { @@ -62,7 +69,6 @@ public class PhoneNumberDisplayHelper { int presentation, CharSequence formattedNumber) { final CharSequence displayName = getDisplayName(number, presentation); - if (!TextUtils.isEmpty(displayName)) { return displayName; } diff --git a/src/com/android/dialer/dialpad/DialpadFragment.java b/src/com/android/dialer/dialpad/DialpadFragment.java index 00b8281a7..093cde246 100644 --- a/src/com/android/dialer/dialpad/DialpadFragment.java +++ b/src/com/android/dialer/dialpad/DialpadFragment.java @@ -1560,9 +1560,9 @@ public class DialpadFragment extends Fragment * or Wait character (;). */ private void updateDialString(char newDigit) { - if(newDigit != WAIT && newDigit != PAUSE) { - Log.wtf(TAG, "Not expected for anything other than PAUSE & WAIT"); - return; + if (newDigit != WAIT && newDigit != PAUSE) { + throw new IllegalArgumentException( + "Not expected for anything other than PAUSE & WAIT"); } int selectionStart; @@ -1642,8 +1642,8 @@ public class DialpadFragment extends Fragment /* package */ static boolean canAddDigit(CharSequence digits, int start, int end, char newDigit) { if(newDigit != WAIT && newDigit != PAUSE) { - Log.wtf(TAG, "Should not be called for anything other than PAUSE & WAIT"); - return false; + throw new IllegalArgumentException( + "Should not be called for anything other than PAUSE & WAIT"); } // False if no selection, or selection is reversed (end < start) diff --git a/tests/src/com/android/dialer/PhoneCallDetailsHelperTest.java b/tests/src/com/android/dialer/PhoneCallDetailsHelperTest.java index 6a9817f26..6f5a98658 100644 --- a/tests/src/com/android/dialer/PhoneCallDetailsHelperTest.java +++ b/tests/src/com/android/dialer/PhoneCallDetailsHelperTest.java @@ -51,6 +51,8 @@ public class PhoneCallDetailsHelperTest extends AndroidTestCase { private static final String TEST_COUNTRY_ISO = "US"; /** The geocoded location used in the tests. */ private static final String TEST_GEOCODE = "United States"; + /** Empty geocode label */ + private static final String EMPTY_GEOCODE = ""; /** The object under test. */ private PhoneCallDetailsHelper mHelper; @@ -183,18 +185,18 @@ public class PhoneCallDetailsHelperTest extends AndroidTestCase { public void testSetPhoneCallDetails_NoGeocode() { setPhoneCallDetailsWithNumberAndGeocode("+14125555555", "1-412-555-5555", null); assertNameEquals("1-412-555-5555"); // The phone number is shown as the name. - assertLabelEquals("-"); // The empty geocode is shown as the label. + assertLabelEquals(EMPTY_GEOCODE); // The empty geocode is shown as the label. } public void testSetPhoneCallDetails_EmptyGeocode() { setPhoneCallDetailsWithNumberAndGeocode("+14125555555", "1-412-555-5555", ""); assertNameEquals("1-412-555-5555"); // The phone number is shown as the name. - assertLabelEquals("-"); // The empty geocode is shown as the label. + assertLabelEquals(EMPTY_GEOCODE); // The empty geocode is shown as the label. } public void testSetPhoneCallDetails_NoGeocodeForVoicemail() { setPhoneCallDetailsWithNumberAndGeocode(TEST_VOICEMAIL_NUMBER, "", "United States"); - assertLabelEquals("-"); // The empty geocode is shown as the label. + assertLabelEquals(EMPTY_GEOCODE); // The empty geocode is shown as the label. } public void testSetPhoneCallDetails_Highlighted() { diff --git a/tests/src/com/android/dialer/calllog/CallLogFragmentTest.java b/tests/src/com/android/dialer/calllog/CallLogFragmentTest.java index 7ceec8f08..4ccdaaf33 100644 --- a/tests/src/com/android/dialer/calllog/CallLogFragmentTest.java +++ b/tests/src/com/android/dialer/calllog/CallLogFragmentTest.java @@ -70,6 +70,8 @@ public class CallLogFragmentTest extends ActivityInstrumentationTestCase2