Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 9c747ac6 authored by Hugo Hudson's avatar Hugo Hudson
Browse files

Fix up the unit tests for the CallDetailActivity. 

- We've recently moved lots of code to use AsyncTask to avoid strict mode
  violations.
- Thanks to the new BackgroundTaskService, these weren't being executed,
  and the tests were failing.  But simply executing them is not a fix,
  we want much finer grained control over what executes when, so we
  can assert about different states of the ui.
- This cl introduces the concept of an identifier to go with the submitted
  task, so that you can uniquely identify tasks from the test.

Additionally, on further reflection, adding a new interface BackgroundTask
wasn't necessarily a great idea.  Nor was calling the thing that submits them a
Service - that name is already overloaded to mean something else in Android.

Therefore this cl makes a number of other style changes to the pattern:

- The BackgroundTaskService just becomes an interface AsyncTaskExecutor, with a
  single submit() method, in a very similar fashion to the Executor pattern in
  java.util.concurrent.
- We introduce the AsyncTaskExecutors class, which may be used to create
  AsyncTaskExecutor objects, and also introduces a seam for injecting fake
  executors for testing.
- This cl introduces a FakeAsyncTaskExecutor, which can be used to inspect the
  tasks that have been submitted, as well as being used to execute them in a
  controlled manner between assertions.
- This is now being used to control the flow of voicemail fetching from
  the unit tests, and make sure that the recently implemented logic to
  read has content, move to buffering state, then move to preparing,
  is all working correctly.
- Later this will also be used to exhaustively test all the other
  situations we care about.

Change-Id: Ia75df4996f9a5168db8d9f39560b62ccf4b98b46
parent bb500114

src/com/android/contacts/CallDetailActivity.java

+51 −38
Original line number Diff line number Diff line
@@ -20,9 +20,8 @@ import com.android.contacts.BackScrollManager.ScrollableHeader; 
import com.android.contacts.calllog.CallDetailHistoryAdapter;

import com.android.contacts.calllog.CallTypeHelper;

import com.android.contacts.calllog.PhoneNumberHelper;

import com.android.contacts.util.AbstractBackgroundTask;

import com.android.contacts.util.BackgroundTask;

import com.android.contacts.util.BackgroundTaskService;

import com.android.contacts.util.AsyncTaskExecutor;

import com.android.contacts.util.AsyncTaskExecutors;

import com.android.contacts.voicemail.VoicemailPlaybackFragment;

import com.android.contacts.voicemail.VoicemailStatusHelper;

import com.android.contacts.voicemail.VoicemailStatusHelper.StatusMessage;
@@ -73,6 +72,14 @@ import java.util.List; 
public class CallDetailActivity extends Activity {

    private static final String TAG = "CallDetail";



    /** The enumeration of {@link AsyncTask} objects used in this class. */

    public enum Tasks {

        MARK_VOICEMAIL_READ,

        DELETE_VOICEMAIL_AND_FINISH,

        REMOVE_FROM_CALL_LOG_AND_FINISH,

        UPDATE_PHONE_CALL_DETAILS,

    }



    /** A long array extra containing ids of call log entries to display. */

    public static final String EXTRA_CALL_LOG_IDS = "EXTRA_CALL_LOG_IDS";

    /** If we are started with a voicemail, we'll find the uri to play with this extra. */
@@ -87,7 +94,7 @@ public class CallDetailActivity extends Activity { 
    private ImageView mMainActionView;

    private ImageButton mMainActionPushLayerView;

    private ImageView mContactBackgroundView;

    private BackgroundTaskService mBackgroundTaskService;

    private AsyncTaskExecutor mAsyncTaskExecutor;



    private String mNumber = null;

    private String mDefaultCountryIso;
@@ -161,8 +168,7 @@ public class CallDetailActivity extends Activity { 


        setContentView(R.layout.call_detail);



        mBackgroundTaskService = (BackgroundTaskService) getApplicationContext().getSystemService(

                BackgroundTaskService.BACKGROUND_TASK_SERVICE);

        mAsyncTaskExecutor = AsyncTaskExecutors.createThreadPoolExecutor();

        mInflater = (LayoutInflater) getSystemService(LAYOUT_INFLATER_SERVICE);

        mResources = getResources();
@@ -231,14 +237,15 @@ public class CallDetailActivity extends Activity { 
    }



    private void markVoicemailAsRead(final Uri voicemailUri) {

        mBackgroundTaskService.submit(new AbstractBackgroundTask() {

        mAsyncTaskExecutor.submit(Tasks.MARK_VOICEMAIL_READ, new AsyncTask<Void, Void, Void>() {

            @Override

            public void doInBackground() {

            public Void doInBackground(Void... params) {

                ContentValues values = new ContentValues();

                values.put(Voicemails.IS_READ, true);

                getContentResolver().update(voicemailUri, values, null, null);

                return null;

            }

        }, AsyncTask.THREAD_POOL_EXECUTOR);

        });

    }



    /**
@@ -288,28 +295,27 @@ public class CallDetailActivity extends Activity { 
     * @param callUris URIs into {@link CallLog.Calls} of the calls to be displayed

     */

    private void updateData(final Uri... callUris) {

        mBackgroundTaskService.submit(new BackgroundTask() {

            private PhoneCallDetails[] details;



        class UpdateContactDetailsTask extends AsyncTask<Void, Void, PhoneCallDetails[]> {

            @Override

            public void doInBackground() {

            public PhoneCallDetails[] doInBackground(Void... params) {

                // TODO: All phone calls correspond to the same person, so we can make a single

                // lookup.

                final int numCalls = callUris.length;

                details = new PhoneCallDetails[numCalls];

                PhoneCallDetails[] details = new PhoneCallDetails[numCalls];

                try {

                    for (int index = 0; index < numCalls; ++index) {

                        details[index] = getPhoneCallDetailsForUri(callUris[index]);

                    }

                    return details;

                } catch (IllegalArgumentException e) {

                    // Something went wrong reading in our primary data.

                    Log.w(TAG, "invalid URI starting call details", e);

                    details = null;

                    return null;

                }

            }



            @Override

            public void onPostExecute() {

            public void onPostExecute(PhoneCallDetails[] details) {

                if (details == null) {

                    // Somewhere went wrong: we're going to bail out and show error to users.

                    Toast.makeText(CallDetailActivity.this, R.string.toast_call_detail_error,
@@ -461,7 +467,8 @@ public class CallDetailActivity extends Activity { 
                loadContactPhotos(photoUri);

                findViewById(R.id.call_detail).setVisibility(View.VISIBLE);

            }

        });

        }

        mAsyncTaskExecutor.submit(Tasks.UPDATE_PHONE_CALL_DETAILS, new UpdateContactDetailsTask());

    }



    /** Return the phone call details for a given call log URI. */
@@ -707,31 +714,37 @@ public class CallDetailActivity extends Activity { 
            }

            callIds.append(ContentUris.parseId(callUri));

        }

        mBackgroundTaskService.submit(new BackgroundTask() {

        mAsyncTaskExecutor.submit(Tasks.REMOVE_FROM_CALL_LOG_AND_FINISH,

                new AsyncTask<Void, Void, Void>() {

                    @Override

            public void doInBackground() {

                    public Void doInBackground(Void... params) {

                        getContentResolver().delete(Calls.CONTENT_URI_WITH_VOICEMAIL,

                                Calls._ID + " IN (" + callIds + ")", null);

                        return null;

                    }



                    @Override

            public void onPostExecute() {

                    public void onPostExecute(Void result) {

                        finish();

                    }

                });

    }



    public void onMenuEditNumberBeforeCall(MenuItem menuItem) {

        startActivity(new Intent(Intent.ACTION_DIAL, mPhoneNumberHelper.getCallUri(mNumber)));

    }



    public void onMenuTrashVoicemail(MenuItem menuItem) {

        final Uri voicemailUri = getVoicemailUri();

        mBackgroundTaskService.submit(new BackgroundTask() {

        mAsyncTaskExecutor.submit(Tasks.DELETE_VOICEMAIL_AND_FINISH,

                new AsyncTask<Void, Void, Void>() {

                    @Override

            public void doInBackground() {

                    public Void doInBackground(Void... params) {

                        getContentResolver().delete(voicemailUri, null, null);

                        return null;

                    }

                    @Override

            public void onPostExecute() {

                    public void onPostExecute(Void result) {

                        finish();

                    }

                });

src/com/android/contacts/ContactsApplication.java

+0 −11
Original line number Diff line number Diff line
@@ -16,11 +16,8 @@ 


package com.android.contacts;



import static com.android.contacts.util.BackgroundTaskService.createAsyncTaskBackgroundTaskService;



import com.android.contacts.model.AccountTypeManager;

import com.android.contacts.test.InjectedServices;

import com.android.contacts.util.BackgroundTaskService;

import com.google.common.annotations.VisibleForTesting;



import android.app.Application;
@@ -36,7 +33,6 @@ public final class ContactsApplication extends Application { 
    private static InjectedServices sInjectedServices;

    private AccountTypeManager mAccountTypeManager;

    private ContactPhotoManager mContactPhotoManager;

    private BackgroundTaskService mBackgroundTaskService;



    /**

     * Overrides the system services with mocks for testing.
@@ -97,13 +93,6 @@ public final class ContactsApplication extends Application { 
            return mContactPhotoManager;

        }



        if (BackgroundTaskService.BACKGROUND_TASK_SERVICE.equals(name)) {

            if (mBackgroundTaskService == null) {

                mBackgroundTaskService = createAsyncTaskBackgroundTaskService();

            }

            return mBackgroundTaskService;

        }



        return super.getSystemService(name);

    }

src/com/android/contacts/util/AbstractBackgroundTask.java

deleted100644 → 0
+0 −29
Original line number Diff line number Diff line 
/*

 * Copyright (C) 2011 The Android Open Source Project

 *

 * Licensed 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

 *

 * Unless required by applicable law or agreed to in writing, software

 * distributed under the License is distributed on an "AS IS" BASIS,

 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.

 * See the License for the specific language governing permissions and

 * limitations under the License.

 */



package com.android.contacts.util;



import com.android.contacts.util.BackgroundTask;



/**

 * Base class you can use if you only want to override the {@link #doInBackground()} method.

 */

public abstract class AbstractBackgroundTask implements BackgroundTask {

    @Override

    public void onPostExecute() {

        // No action necessary.

    }

}

src/com/android/contacts/util/BackgroundTask.java→src/com/android/contacts/util/AsyncTaskExecutor.java

+48 −0
Original line number Diff line number Diff line
@@ -16,16 +16,33 @@ 


package com.android.contacts.util;



import android.os.AsyncTask;



import java.util.concurrent.Executor;



/**

 * Interface used to submit {@link AsyncTask} objects to run in the background.

 * <p>

 * This interface has a direct parallel with the {@link Executor} interface. It exists to decouple

 * the mechanics of AsyncTask submission from the description of how that AsyncTask will execute.

 * <p>

 * One immediate benefit of this approach is that testing becomes much easier, since it is easy to

 * introduce a mock or fake AsyncTaskExecutor in unit/integration tests, and thus inspect which

 * tasks have been submitted and control their execution in an orderly manner.

 * <p>

 * Another benefit in due course will be the management of the submitted tasks. An extension to this

 * interface is planned to allow Activities to easily cancel all the submitted tasks that are still

 * pending in the onDestroy() method of the Activity.

 */

public interface AsyncTaskExecutor {

    /**

 * Simple interface to improve the testability of code using AsyncTasks.

     * Executes the given AsyncTask with the default Executor.

     * <p>

 * Provides a trivial replacement for no-arg versions of AsyncTask clients.  We may extend this

 * to add more functionality as we require.

     * This method <b>must only be called from the ui thread</b>.

     * <p>

 * The same memory-visibility guarantees are made here as are made for AsyncTask objects, namely

 * that fields set in {@link #doInBackground()} are visible to {@link #onPostExecute()}.

     * The identifier supplied is any Object that can be used to identify the task later. Most

     * commonly this will be an enum which the tests can also refer to. {@code null} is also

     * accepted, though of course this won't help in identifying the task later.

     */

public interface BackgroundTask {

    public void doInBackground();

    public void onPostExecute();

    <T> AsyncTask<T, ?, ?> submit(Object identifier, AsyncTask<T, ?, ?> task, T... params);

}

src/com/android/contacts/util/AsyncTaskExecutors.java

0 → 100644
+100 −0
Original line number Diff line number Diff line 
/*

 * Copyright (C) 2011 The Android Open Source Project

 *

 * Licensed 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

 *

 * Unless required by applicable law or agreed to in writing, software

 * distributed under the License is distributed on an "AS IS" BASIS,

 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.

 * See the License for the specific language governing permissions and

 * limitations under the License.

 */



package com.android.contacts.util;



import com.android.contacts.test.NeededForTesting;

import com.google.common.base.Preconditions;



import android.os.AsyncTask;

import android.os.Looper;



import java.util.concurrent.Executor;



/**

 * Factory methods for creating AsyncTaskExecutors.

 * <p>

 * All of the factory methods on this class check first to see if you have set a static

 * {@link AsyncTaskExecutorFactory} set through the

 * {@link #setFactoryForTest(AsyncTaskExecutorFactory)} method, and if so delegate to that instead,

 * which is one way of injecting dependencies for testing classes whose construction cannot be

 * controlled such as {@link android.app.Activity}.

 */

public final class AsyncTaskExecutors {

    /**

     * A single instance of the {@link AsyncTaskExecutorFactory}, to which we delegate if it is

     * non-null, for injecting when testing.

     */

    private static AsyncTaskExecutorFactory mInjectedAsyncTaskExecutorFactory = null;



    /**

     * Creates an AsyncTaskExecutor that submits tasks to run with

     * {@link AsyncTask#SERIAL_EXECUTOR}.

     */

    public static AsyncTaskExecutor createAsyncTaskExecutor() {

        synchronized (AsyncTaskExecutors.class) {

            if (mInjectedAsyncTaskExecutorFactory != null) {

                return mInjectedAsyncTaskExecutorFactory.createAsyncTaskExeuctor();

            }

            return new SimpleAsyncTaskExecutor(AsyncTask.SERIAL_EXECUTOR);

        }

    }



    /**

     * Creates an AsyncTaskExecutor that submits tasks to run with

     * {@link AsyncTask#THREAD_POOL_EXECUTOR}.

     */

    public static AsyncTaskExecutor createThreadPoolExecutor() {

        synchronized (AsyncTaskExecutors.class) {

            if (mInjectedAsyncTaskExecutorFactory != null) {

                return mInjectedAsyncTaskExecutorFactory.createAsyncTaskExeuctor();

            }

            return new SimpleAsyncTaskExecutor(AsyncTask.THREAD_POOL_EXECUTOR);

        }

    }



    /** Interface for creating AsyncTaskExecutor objects. */

    public interface AsyncTaskExecutorFactory {

        AsyncTaskExecutor createAsyncTaskExeuctor();

    }



    @NeededForTesting

    public static void setFactoryForTest(AsyncTaskExecutorFactory factory) {

        synchronized (AsyncTaskExecutors.class) {

            mInjectedAsyncTaskExecutorFactory = factory;

        }

    }



    public static void checkCalledFromUiThread() {

        Preconditions.checkState(Thread.currentThread() == Looper.getMainLooper().getThread(),

                "submit method must be called from ui thread, was: " + Thread.currentThread());

    }



    private static class SimpleAsyncTaskExecutor implements AsyncTaskExecutor {

        private final Executor mExecutor;



        public SimpleAsyncTaskExecutor(Executor executor) {

            mExecutor = executor;

        }



        @Override

        public <T> AsyncTask<T, ?, ?> submit(Object identifer, AsyncTask<T, ?, ?> task,

                T... params) {

            checkCalledFromUiThread();

            return task.executeOnExecutor(mExecutor, params);

        }

    }

}