Add more comments and Target for trapping test failures

Author: allisonbm92

smoke-tests/README.md

@@ -16,12 +16,13 @@ is a work in progress, and the following list shows what is complete:
 
 Tests on devices usually involve at least two threads: the main thread and the
 testing thread. Either thread should be sufficient for testing, but most users
-will likely invoke Firebase methods from the main thread. As a result, this
-framework provides a simple solution to easily share state between threads. Most
-tests will consist of two methods. Both may be present in the same file for
-simplicity and ease of understanding. One method will be executed by the test
-runner on the testing thread, while the other, is invoked via a lambda on the
-main thread:
+will likely invoke Firebase methods from the main thread. Integration tests
+already verify the use of the testing thread, so it is important to gain
+coverage from the main thread as well. Therefore, this framework provides a
+simple solution to easily share state between threads. Most tests will consist
+of two methods. Both may be present in the same file for simplicity and ease of
+understanding. One method will be executed by the test runner on the testing
+thread, while the other, is invoked via a lambda on the main thread:
 
 ```java
 @Test

smoke-tests/src/androidTest/java/com/google/firebase/testing/common/AbstractChannel.java

@@ -14,9 +14,11 @@
 
 package com.google.firebase.testing.common;
 
-import java.util.concurrent.CountDownLatch;
+import com.google.android.gms.tasks.TaskCompletionSource;
+import com.google.android.gms.tasks.Tasks;
+import java.util.concurrent.ExecutionException;
 import java.util.concurrent.TimeUnit;
-import java.util.concurrent.locks.ReentrantLock;
+import java.util.concurrent.TimeoutException;
 
 /**
  * An abstract channel for sending test results across threads.
@@ -27,11 +29,21 @@
  */
 public abstract class AbstractChannel<T> {
 
-  private final CountDownLatch latch = new CountDownLatch(1);
-  private final ReentrantLock lock = new ReentrantLock();
+  private final TaskCompletionSource<T> implementation = new TaskCompletionSource<>();
 
-  private Exception error = null;
-  private T value = null;
+  /** Runs the test target on the main thread, trapping any exception into the channel. */
+  protected static <U extends AbstractChannel<R>, R> U runTarget(Target<U> target, U channel) {
+    MainThread.run(
+        () -> {
+          try {
+            target.run(channel);
+          } catch (Exception ex) {
+            channel.fail(ex);
+          }
+        });
+
+    return channel;
+  }
 
   /**
    * Sends a failure back to the testing thread.
@@ -42,19 +54,10 @@ public abstract class AbstractChannel<T> {
    * send only one value through the channel. This method is safe to invoke from any thread.
    */
   protected void fail(Exception err) {
-    // This is explicitly synchronized in case multiple threads are trying to send values.
-    try {
-      lock.lock();
-
-      if (error == null) {
-        error = err;
-      } else {
-        error.addSuppressed(err);
-      }
+    boolean isSet = implementation.trySetException(err);
 
-      latch.countDown();
-    } finally {
-      lock.unlock();
+    if (!isSet) {
+      implementation.getTask().getException().addSuppressed(err);
     }
   }
 
@@ -66,23 +69,7 @@ protected void fail(Exception err) {
    * any thread.
    */
   protected void succeed(T val) {
-    // This is explicitly synchronized in case multiple threads are trying to send values.
-    try {
-      lock.lock();
-
-      if (latch.getCount() == 0) {
-        // Only a single, successful value is supported. This throws the exception on both the
-        // testing thread and the main thread.
-        IllegalStateException error = new IllegalStateException("Result already completed");
-        fail(error);
-        throw error;
-      }
-
-      value = val;
-      latch.countDown();
-    } finally {
-      lock.unlock();
-    }
+    implementation.setResult(val);
   }
 
   /** Waits 30 seconds to receive the successful value. */
@@ -98,17 +85,13 @@ public T waitForSuccess() throws InterruptedException {
    * thread.
    */
   public T waitForSuccess(long duration, TimeUnit unit) throws InterruptedException {
-    boolean completed = latch.await(duration, unit);
-
-    if (!completed) {
+    try {
+      return Tasks.await(implementation.getTask(), duration, unit);
+    } catch (ExecutionException ex) {
+      throw new AssertionError("Test completed with errors", ex.getCause());
+    } catch (TimeoutException ex) {
       String message = String.format("Test did not complete within %s %s", duration, unit);
-      throw new AssertionError(message, error);
+      throw new AssertionError(message);
     }
-
-    if (error != null) {
-      throw new AssertionError("Test completed with errors", error);
-    }
-
-    return value;
   }
 }

smoke-tests/src/androidTest/java/com/google/firebase/testing/common/MainThread.java

@@ -29,7 +29,7 @@ public final class MainThread {
   private MainThread() {}
 
   /** Runs the {@link Runnable} on the main thread. */
-  public static void run(Runnable r) throws InterruptedException {
+  public static void run(Runnable r) {
     if (handler == null) {
       handler = new Handler(Looper.getMainLooper());
     }

smoke-tests/src/androidTest/java/com/google/firebase/testing/common/Target.java

@@ -0,0 +1,26 @@
+// Copyright 2018 Google LLC
+//
+// 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.google.firebase.testing.common;
+
+/**
+ * A test target.
+ *
+ * <p>This interface is similar to {@link Runnable}, but this interface's {@link #run} method takes
+ * an instance of {@link T} as input. This is intended to be a channel.
+ */
+public interface Target<T> {
+
+  void run(T channel);
+}

smoke-tests/src/androidTest/java/com/google/firebase/testing/common/TaskChannel.java

@@ -25,6 +25,11 @@
  */
 public class TaskChannel<T> extends AbstractChannel<T> {
 
+  /** Runs the test on the main thread and returns the corresponding channel. */
+  public static <U> TaskChannel<U> runWithTaskChannel(Target<TaskChannel<U>> test) {
+    return AbstractChannel.runTarget(test, new TaskChannel<>());
+  }
+
   /**
    * Sends the outcome of a task over the channel.
    *

smoke-tests/src/androidTestDatabase/java/com/google/firebase/testing/database/DatabaseChannel.java

@@ -18,6 +18,8 @@
 import com.google.firebase.database.DatabaseError;
 import com.google.firebase.database.Query;
 import com.google.firebase.database.ValueEventListener;
+import com.google.firebase.testing.common.AbstractChannel;
+import com.google.firebase.testing.common.Target;
 import com.google.firebase.testing.common.TaskChannel;
 
 /**
@@ -29,6 +31,11 @@
  */
 public class DatabaseChannel extends TaskChannel<DataSnapshot> {
 
+  /** Runs the test on the main thread and returns the corresponding channel. */
+  public static DatabaseChannel runWithDatabaseChannel(Target<DatabaseChannel> test) {
+    return AbstractChannel.runTarget(test, new DatabaseChannel());
+  }
+
   /**
    * Adds a listener to the query that sends the results back to the testing thread.
    *

smoke-tests/src/androidTestDatabase/java/com/google/firebase/testing/database/DatabaseTest.java

@@ -21,7 +21,6 @@
 import com.google.firebase.database.DataSnapshot;
 import com.google.firebase.database.DatabaseReference;
 import com.google.firebase.database.FirebaseDatabase;
-import com.google.firebase.testing.common.MainThread;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 
@@ -30,9 +29,7 @@ public final class DatabaseTest {
 
   @Test
   public void listenForUpdate() throws Exception {
-    DatabaseChannel channel = new DatabaseChannel();
-
-    MainThread.run(() -> test_ListenForUpdate(channel));
+    DatabaseChannel channel = DatabaseChannel.runWithDatabaseChannel(c -> test_ListenForUpdate(c));
 
     DataSnapshot snapshot = channel.waitForSuccess();
     assertThat(snapshot.child("location").getValue()).isEqualTo("Google SVL");

smoke-tests/src/androidTestFirestore/java/com/google/firebase/testing/firestore/FirestoreChannel.java

@@ -16,6 +16,8 @@
 
 import com.google.firebase.firestore.DocumentReference;
 import com.google.firebase.firestore.DocumentSnapshot;
+import com.google.firebase.testing.common.AbstractChannel;
+import com.google.firebase.testing.common.Target;
 import com.google.firebase.testing.common.TaskChannel;
 
 /**
@@ -27,6 +29,11 @@
  */
 public final class FirestoreChannel extends TaskChannel<DocumentSnapshot> {
 
+  /** Runs the test on the main thread and returns the corresponding channel. */
+  public static FirestoreChannel runWithFirestoreChannel(Target<FirestoreChannel> test) {
+    return AbstractChannel.runTarget(test, new FirestoreChannel());
+  }
+
   /**
    * Adds a listener to the document that sends the results back to the testing thread.
    *

smoke-tests/src/androidTestFirestore/java/com/google/firebase/testing/firestore/FirestoreTest.java

@@ -22,7 +22,6 @@
 import com.google.firebase.firestore.DocumentSnapshot;
 import com.google.firebase.firestore.FirebaseFirestore;
 import com.google.firebase.firestore.FirebaseFirestoreSettings;
-import com.google.firebase.testing.common.MainThread;
 import java.util.HashMap;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -32,9 +31,8 @@ public final class FirestoreTest {
 
   @Test
   public void listenForUpdate() throws Exception {
-    FirestoreChannel channel = new FirestoreChannel();
-
-    MainThread.run(() -> test_ListenForUpdate(channel));
+    FirestoreChannel channel =
+        FirestoreChannel.runWithFirestoreChannel(c -> test_ListenForUpdate(c));
 
     DocumentSnapshot snapshot = channel.waitForSuccess();
     assertThat(snapshot.getString("location")).isEqualTo("Google SVL");

smoke-tests/src/androidTestStorage/java/com/google/firebase/testing/storage/StorageTest.java

@@ -22,7 +22,6 @@
 import com.google.firebase.auth.FirebaseAuth;
 import com.google.firebase.storage.FirebaseStorage;
 import com.google.firebase.storage.StorageReference;
-import com.google.firebase.testing.common.MainThread;
 import com.google.firebase.testing.common.TaskChannel;
 import java.nio.charset.StandardCharsets;
 import org.junit.Test;
@@ -33,9 +32,7 @@ public final class StorageTest {
 
   @Test
   public void getSet() throws Exception {
-    TaskChannel<byte[]> channel = new TaskChannel<>();
-
-    MainThread.run(() -> test_GetSet(channel));
+    TaskChannel channel = TaskChannel.runWithTaskChannel(c -> test_GetSet(c));
 
     byte[] bytes = channel.waitForSuccess();
     String text = new String(bytes, StandardCharsets.UTF_8);