Merge pull request #867 from OneSignal/fix_get_nested_tags

Fix Flaky Tests

Author: jkasten2

OneSignalSDK/onesignal/src/main/java/com/onesignal/OneSignalRestClient.java

@@ -66,23 +66,23 @@ public static void put(final String url, final JSONObject jsonBody, final Respon
          public void run() {
             makeRequest(url, "PUT", jsonBody, responseHandler, TIMEOUT, null);
          }
-      }).start();
+      }, "OS_REST_ASYNC_PUT").start();
    }
 
    public static void post(final String url, final JSONObject jsonBody, final ResponseHandler responseHandler) {
       new Thread(new Runnable() {
          public void run() {
             makeRequest(url, "POST", jsonBody, responseHandler, TIMEOUT, null);
          }
-      }).start();
+      }, "OS_REST_ASYNC_POST").start();
    }
 
    public static void get(final String url, final ResponseHandler responseHandler, @NonNull final String cacheKey) {
       new Thread(new Runnable() {
          public void run() {
             makeRequest(url, null, null, responseHandler, GET_TIMEOUT, cacheKey);
          }
-      }).start();
+      }, "OS_REST_ASYNC_GET").start();
    }
 
    public static void getSync(final String url, final ResponseHandler responseHandler, @NonNull String cacheKey) {
@@ -261,7 +261,7 @@ private static Thread callResponseHandlerOnSuccess(final ResponseHandler handler
          public void run() {
             handler.onSuccess(response);
          }
-      });
+      }, "OS_REST_SUCCESS_CALLBACK");
       thread.start();
 
       return thread;
@@ -275,7 +275,7 @@ private static Thread callResponseHandlerOnFailure(final ResponseHandler handler
          public void run() {
             handler.onFailure(statusCode, response, throwable);
          }
-      });
+      }, "OS_REST_FAILURE_CALLBACK");
       thread.start();
 
       return thread;

OneSignalSDK/unittest/src/test/java/com/onesignal/ShadowRoboNotificationManager.java

@@ -31,12 +31,16 @@
 import android.app.Notification;
 import android.app.NotificationChannelGroup;
 import android.app.NotificationManager;
+import android.support.annotation.NonNull;
+import android.support.v4.app.NotificationCompat;
 
 import org.robolectric.annotation.Implements;
 import org.robolectric.shadows.ShadowNotification;
 import org.robolectric.shadows.ShadowNotificationManager;
 
+import java.util.ArrayList;
 import java.util.LinkedHashMap;
+import java.util.List;
 
 import static org.robolectric.Shadows.shadowOf;
 
@@ -69,6 +73,11 @@ public static ShadowNotification getLastShadowNotif() {
    public static int lastNotifId;
 
    public static LinkedHashMap<Integer, PostedNotification> notifications = new LinkedHashMap<>();
+
+   private static ShadowRoboNotificationManager mInstance;
+   ShadowRoboNotificationManager() {
+      mInstance = this;
+   }
 
    @Override
    public void cancelAll() {
@@ -96,6 +105,18 @@ public void notify(String tag, int id, Notification notification) {
       super.notify(tag, id, notification);
    }
 
+   public static @NonNull List<Notification> getNotificationsInGroup(@NonNull String group) {
+      List<Notification> notifications = new ArrayList<>();
+      for (Notification notification : mInstance.getAllNotifications()) {
+         if (NotificationCompat.isGroupSummary(notification))
+            continue;
+         if (!group.equals(notification.getGroup()))
+            continue;
+         notifications.add(notification);
+      }
+      return notifications;
+   }
+
    public static NotificationChannel lastChannel;
    public void createNotificationChannel(NotificationChannel channel) {
       lastChannel = channel;

OneSignalSDK/unittest/src/test/java/com/test/onesignal/GenerateNotificationRunner.java

@@ -94,7 +94,6 @@
 import org.robolectric.annotation.Config;
 import org.robolectric.shadows.ShadowAlertDialog;
 import org.robolectric.shadows.ShadowLog;
-import org.robolectric.shadows.ShadowSystemClock;
 
 import java.lang.reflect.Field;
 import java.math.BigInteger;
@@ -107,6 +106,7 @@
 import static com.onesignal.OneSignalPackagePrivateHelper.NotificationOpenedProcessor_processFromContext;
 import static com.onesignal.OneSignalPackagePrivateHelper.NotificationSummaryManager_updateSummaryNotificationAfterChildRemoved;
 import static com.onesignal.OneSignalPackagePrivateHelper.createInternalPayloadBundle;
+import static com.onesignal.ShadowRoboNotificationManager.getNotificationsInGroup;
 import static com.test.onesignal.RestClientAsserts.assertReportReceivedAtIndex;
 import static com.test.onesignal.RestClientAsserts.assertRestCalls;
 import static com.test.onesignal.TestHelpers.advanceSystemTimeBy;
@@ -327,51 +327,26 @@ public void shouldCancelAllNotificationsPartOfAGroup() throws Exception {
 
    @Test
    @Config(sdk = Build.VERSION_CODES.N)
-   public void testGetMostRecentNotifIdFromGroup() throws Exception {
-      OneSignal.setInFocusDisplaying(OneSignal.OSInFocusDisplayOption.Notification);
-      OneSignal.init(blankActivity, "123456789", "b2f7f966-d8cc-11e4-bed1-df8f05be55ba");
+   public void testFourNotificationsUseProvidedGroup() throws Exception {
+      OneSignal.init(blankActivity.getApplicationContext(), "123456789", "b2f7f966-d8cc-11e4-bed1-df8f05be55ba");
       threadAndTaskWait();
 
       // Add 4 grouped notifications
       postNotificationWithOptionalGroup(4, "test1");
 
-      SQLiteDatabase readableDb = OneSignalDbHelper.getInstance(blankActivity).getReadableDatabase();
-      // Grab the most recent timestamped notification from local DB
-      Integer mostRecentId = OneSignalNotificationManagerPackageHelper.getMostRecentNotifIdFromGroup(readableDb, "test1", false);
-
-      Map<Integer, PostedNotification> postedNotifs = ShadowRoboNotificationManager.notifications;
-      // Iterator is ordered by notification post (0 index is most recent)
-      Iterator<Map.Entry<Integer, PostedNotification>> postedNotifsIterator = postedNotifs.entrySet().iterator();
-      // First notification is the summary so we skip it and move on to the first normal notif
-      postedNotifsIterator.next();
-      Integer expectedId = postedNotifsIterator.next().getKey();
-
-      // Assert our id equals the first real notif id (most recent posted)
-      assertEquals(expectedId, mostRecentId);
+      assertEquals(4, getNotificationsInGroup("test1").size());
    }
 
    @Test
    @Config(sdk = Build.VERSION_CODES.N)
-   public void testGetMostRecentNotifIdFromGroupless() throws Exception {
-      OneSignal.setInFocusDisplaying(OneSignal.OSInFocusDisplayOption.Notification);
-      OneSignal.init(blankActivity, "123456789", "b2f7f966-d8cc-11e4-bed1-df8f05be55ba");
+   public void testFourGrouplessNotificationsUseDefaultGroup() throws Exception {
+      OneSignal.init(blankActivity.getApplicationContext(), "123456789", "b2f7f966-d8cc-11e4-bed1-df8f05be55ba");
       threadAndTaskWait();
 
       // Add 4 groupless notifications
       postNotificationWithOptionalGroup(4, null);
 
-      SQLiteDatabase readableDb = OneSignalDbHelper.getInstance(blankActivity).getReadableDatabase();
-      // Grab the most recent timestamped notification from local DB
-      Integer mostRecentId = OneSignalNotificationManagerPackageHelper.getMostRecentNotifIdFromGroup(readableDb, null, true);
-
-      Map<Integer, PostedNotification> postedNotifs = ShadowRoboNotificationManager.notifications;
-      // Iterator is ordered by notification post (0 index is most recent)
-      Iterator<Map.Entry<Integer, PostedNotification>> postedNotifsIterator = postedNotifs.entrySet().iterator();
-      // Grab first active notification since groupless won't have a summary
-      Integer expectedId = postedNotifsIterator.next().getKey();
-
-      // Assert our id equals the first real notif id (most recent posted)
-      assertEquals(expectedId, mostRecentId);
+      assertEquals(4, getNotificationsInGroup("os_group_undefined").size());
    }
 
     @Test
@@ -526,13 +501,11 @@ public void testGrouplessSummaryKeyReassignmentAtFourOrMoreNotification() throws
       assertEquals(0, groupCount);
    }
 
-    public Bundle postNotificationWithOptionalGroup(int notifCount, String group) {
+    private @Nullable Bundle postNotificationWithOptionalGroup(int notifCount, @Nullable String group) {
        Bundle bundle = null;
        for (int i = 0; i < notifCount; i++) {
           bundle = getBaseNotifBundle("UUID" + i);
-
-          if (group != null)
-             bundle.putString("grp", group);
+          bundle.putString("grp", group);
 
           NotificationBundleProcessor_ProcessFromGCMIntentService(blankActivity, bundle, null);
        }

OneSignalSDK/unittest/src/test/java/com/test/onesignal/MainOneSignalClassRunner.java

@@ -89,6 +89,7 @@
 import org.json.JSONObject;
 import org.junit.After;
 import org.junit.AfterClass;
+import org.junit.Assert;
 import org.junit.Before;
 import org.junit.BeforeClass;
 import org.junit.Test;
@@ -110,6 +111,8 @@
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import java.util.concurrent.ArrayBlockingQueue;
+import java.util.concurrent.BlockingQueue;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.regex.Pattern;
 
@@ -3817,14 +3820,13 @@ public void sendsExternalIdOnEmailPlayers() throws Exception {
 
    @Test
    public void testGetTagsQueuesCallbacks() throws Exception {
+      final BlockingQueue<Boolean> queue = new ArrayBlockingQueue<>(2);
 
       // Allows us to validate that both handlers get executed independently
       class DebugGetTagsHandler implements OneSignal.GetTagsHandler {
-         boolean executed = false;
-
          @Override
          public void tagsAvailable(JSONObject tags) {
-            executed = true;
+            queue.offer(true);
          }
       }
 
@@ -3841,43 +3843,41 @@ public void tagsAvailable(JSONObject tags) {
       OneSignal.getTags(second);
       threadAndTaskWait();
 
-      assertTrue(first.executed);
-      assertTrue(second.executed);
+      assertTrue(queue.take());
+      assertTrue(queue.take());
    }
 
    @Test
    public void testNestedGetTags() throws Exception {
+      final BlockingQueue<Boolean> queue = new ArrayBlockingQueue<>(2);
 
       // Validates that nested getTags calls won't throw a ConcurrentModificationException
       class DebugGetTagsHandler implements OneSignal.GetTagsHandler {
-         boolean executed = false;
-
          @Override
          public void tagsAvailable(JSONObject tags) {
             OneSignal.getTags(new OneSignal.GetTagsHandler() {
                @Override
                public void tagsAvailable(JSONObject tags) {
-                  executed = true;
+                  queue.offer(true);
                }
             });
          }
       }
 
-      DebugGetTagsHandler first = new DebugGetTagsHandler();
-      DebugGetTagsHandler second = new DebugGetTagsHandler();
-
       OneSignalInit();
       threadAndTaskWait();
 
       OneSignal.sendTag("test", "value");
       threadAndTaskWait();
 
+      DebugGetTagsHandler first = new DebugGetTagsHandler();
+      DebugGetTagsHandler second = new DebugGetTagsHandler();
       OneSignal.getTags(first);
       OneSignal.getTags(second);
       threadAndTaskWait();
 
-      assertTrue(first.executed);
-      assertTrue(second.executed);
+      assertTrue(queue.take());
+      assertTrue(queue.take());
    }
 
    /**

OneSignalSDK/unittest/src/test/java/com/test/onesignal/OutcomeEventUnitTests.java

@@ -348,7 +348,6 @@ public void testUniqueOutcomeFailSavedOnDBResetSession() throws Exception {
         controller.sendUniqueOutcomeEvent(OUTCOME_NAME);
         controller.sendUniqueOutcomeEvent(OUTCOME_NAME);
         controller.sendUniqueOutcomeEvent(OUTCOME_NAME);
-
         threadAndTaskWait();
 
         new Thread(new Runnable() {
@@ -367,6 +366,7 @@ public void run() {
 
         controller.sendUniqueOutcomeEvent(OUTCOME_NAME);
         controller.sendUniqueOutcomeEvent(OUTCOME_NAME);
+        threadAndTaskWait();
 
         new Thread(new Runnable() {
             @Override