Skip to content

Commit

Permalink
Fix flaky tests due to actual thread switching (#679)
Browse files Browse the repository at this point in the history
  • Loading branch information
@poovamraj
poovamraj authored Aug 1, 2023
1 parent 5a44d52 commit c459978
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 17 deletions.
3 changes: 2 additions & 1 deletion auth0/src/main/java/com/auth0/android/provider/AuthenticationActivity.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import com.auth0.android.authentication.AuthenticationException
import com.auth0.android.callback.RunnableTask
import com.auth0.android.provider.WebAuthProvider.failure
import com.auth0.android.provider.WebAuthProvider.resume
import com.auth0.android.request.internal.CommonThreadSwitcher.Companion.getInstance
import com.google.androidbrowserhelper.trusted.TwaLauncher

public open class AuthenticationActivity : Activity() {
Expand Down Expand Up @@ -73,7 +74,7 @@ public open class AuthenticationActivity : Activity() {
val launchAsTwa: Boolean = extras.getBoolean(EXTRA_LAUNCH_AS_TWA, false)
customTabsController = createCustomTabsController(this, customTabsOptions)
customTabsController!!.bindService()
customTabsController!!.launchUri(authorizeUri!!, launchAsTwa, object : RunnableTask<AuthenticationException> {
customTabsController!!.launchUri(authorizeUri!!, launchAsTwa, getInstance(), object : RunnableTask<AuthenticationException> {
override fun apply(error: AuthenticationException) {
deliverAuthenticationFailure(error)
}
Expand Down
5 changes: 3 additions & 2 deletions auth0/src/main/java/com/auth0/android/provider/CustomTabsController.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import com.auth0.android.authentication.AuthenticationException;
import com.auth0.android.callback.RunnableTask;
import com.auth0.android.request.internal.CommonThreadSwitcher;
import com.auth0.android.request.internal.ThreadSwitcher;
import com.google.androidbrowserhelper.trusted.TwaLauncher;

import java.lang.ref.WeakReference;
Expand Down Expand Up @@ -111,7 +112,7 @@ public void unbindService() {
*
* @param uri the uri to open in a Custom Tab or Browser.
*/
public void launchUri(@NonNull final Uri uri, final boolean launchAsTwa, final RunnableTask<AuthenticationException> failureCallback) {
public void launchUri(@NonNull final Uri uri, final boolean launchAsTwa, ThreadSwitcher threadSwitcher, final RunnableTask<AuthenticationException> failureCallback) {
final Context context = this.context.get();
if (context == null) {
Log.v(TAG, "Custom Tab Context was no longer valid.");
Expand All @@ -137,7 +138,7 @@ public void launchUri(@NonNull final Uri uri, final boolean launchAsTwa, final R
} catch (SecurityException ex) {
AuthenticationException e = new AuthenticationException(
"a0.browser_not_available", "Error launching browser for authentication", ex);
CommonThreadSwitcher.getInstance().mainThread(() -> failureCallback.apply(e));
threadSwitcher.mainThread(() -> failureCallback.apply(e));
}
}).start();
}
Expand Down
9 changes: 5 additions & 4 deletions auth0/src/test/java/com/auth0/android/provider/AuthenticationActivityTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import com.auth0.android.authentication.AuthenticationException
import com.auth0.android.callback.RunnableTask
import com.auth0.android.provider.AuthenticationActivity
import com.auth0.android.provider.CustomTabsOptions
import com.nhaarman.mockitokotlin2.any
import org.hamcrest.CoreMatchers
import org.hamcrest.MatcherAssert
import org.hamcrest.Matchers
Expand Down Expand Up @@ -91,7 +92,7 @@ public class AuthenticationActivityTest {
createActivity(intentCaptor.value)
activityController.create().start().resume()
Mockito.verify(customTabsController).bindService()
Mockito.verify(customTabsController).launchUri(uriCaptor.capture(), launchAsTwaCaptor.capture(), failureCallbackCaptor.capture())
Mockito.verify(customTabsController).launchUri(uriCaptor.capture(), launchAsTwaCaptor.capture(), any(), failureCallbackCaptor.capture())
MatcherAssert.assertThat(uriCaptor.value, Is.`is`(Matchers.notNullValue()))
MatcherAssert.assertThat(uriCaptor.value, Is.`is`(uri))
MatcherAssert.assertThat(activity.deliveredIntent, Is.`is`(Matchers.nullValue()))
Expand Down Expand Up @@ -122,7 +123,7 @@ public class AuthenticationActivityTest {
createActivity(intentCaptor.value)
activityController.create().start().resume()
Mockito.verify(customTabsController).bindService()
Mockito.verify(customTabsController).launchUri(uriCaptor.capture(), launchAsTwaCaptor.capture(), failureCallbackCaptor.capture())
Mockito.verify(customTabsController).launchUri(uriCaptor.capture(), launchAsTwaCaptor.capture(), any(), failureCallbackCaptor.capture())
MatcherAssert.assertThat(uriCaptor.value, Is.`is`(Matchers.notNullValue()))
MatcherAssert.assertThat(uriCaptor.value, Is.`is`(uri))
MatcherAssert.assertThat(activity.deliveredIntent, Is.`is`(Matchers.nullValue()))
Expand Down Expand Up @@ -153,7 +154,7 @@ public class AuthenticationActivityTest {
createActivity(intentCaptor.value)
activityController.create().start().resume()
Mockito.verify(customTabsController).bindService()
Mockito.verify(customTabsController).launchUri(uriCaptor.capture(), launchAsTwaCaptor.capture(), failureCallbackCaptor.capture())
Mockito.verify(customTabsController).launchUri(uriCaptor.capture(), launchAsTwaCaptor.capture(), any(), failureCallbackCaptor.capture())
MatcherAssert.assertThat(uriCaptor.value, Is.`is`(Matchers.notNullValue()))
MatcherAssert.assertThat(uriCaptor.value, Is.`is`(uri))
MatcherAssert.assertThat(launchAsTwaCaptor.value, Is.`is`(Matchers.notNullValue()))
Expand Down Expand Up @@ -183,7 +184,7 @@ public class AuthenticationActivityTest {
createActivity(intentCaptor.value)
activityController.create().start().resume()
Mockito.verify(customTabsController).bindService()
Mockito.verify(customTabsController).launchUri(uriCaptor.capture(), launchAsTwaCaptor.capture(), failureCallbackCaptor.capture())
Mockito.verify(customTabsController).launchUri(uriCaptor.capture(), launchAsTwaCaptor.capture(), any(), failureCallbackCaptor.capture())
MatcherAssert.assertThat(uriCaptor.value, Is.`is`(Matchers.notNullValue()))
MatcherAssert.assertThat(uriCaptor.value, Is.`is`(uri))
MatcherAssert.assertThat(launchAsTwaCaptor.value, Is.`is`(Matchers.notNullValue()))
Expand Down
26 changes: 16 additions & 10 deletions auth0/src/test/java/com/auth0/android/provider/CustomTabsControllerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import org.mockito.ArgumentCaptor;
import org.mockito.Captor;
import org.mockito.Mock;
import org.mockito.Mockito;
import org.mockito.MockitoAnnotations;
import org.powermock.api.mockito.PowerMockito;
import org.robolectric.Robolectric;
Expand Down Expand Up @@ -52,6 +53,7 @@
import static org.mockito.Mockito.when;

import com.auth0.android.authentication.AuthenticationException;
import com.auth0.android.request.internal.ThreadSwitcher;
import com.google.androidbrowserhelper.trusted.TwaLauncher;
import com.google.androidbrowserhelper.trusted.splashscreens.SplashScreenStrategy;

Expand All @@ -75,6 +77,8 @@ public class CustomTabsControllerTest {
@Captor
private ArgumentCaptor<CustomTabsServiceConnection> serviceConnectionCaptor;

private ThreadSwitcher mockThreadSwitcher;

private CustomTabsController controller;


Expand All @@ -83,6 +87,7 @@ public void setUp() {
MockitoAnnotations.openMocks(this);
Activity activity = Robolectric.setupActivity(Activity.class);
context = spy(activity);
mockThreadSwitcher = spy(ThreadSwitcher.class);

//By default, a "compatible" browser is available
BrowserPicker browserPicker = mock(BrowserPicker.class);
Expand Down Expand Up @@ -130,7 +135,7 @@ public void shouldUnbindEvenIfNotBound() throws Exception {
@Test
public void shouldBindAndLaunchUri() throws Exception {
bindService(controller, true);
controller.launchUri(uri, false, null);
controller.launchUri(uri, false, mockThreadSwitcher, null);
connectBoundService();

verify(context, timeout(MAX_TEST_WAIT_TIME_MS)).startActivity(launchIntentCaptor.capture());
Expand All @@ -149,7 +154,7 @@ public void shouldBindAndLaunchUri() throws Exception {
@Test
public void shouldBindAndLaunchUriAsTwa() throws Exception {
bindService(controller, true);
controller.launchUri(uri, true, null);
controller.launchUri(uri, true, mockThreadSwitcher, null);
connectBoundService();
ArgumentCaptor<TrustedWebActivityIntentBuilder> trustedWebActivityIntentBuilderArgumentCaptor
= ArgumentCaptor.forClass(TrustedWebActivityIntentBuilder.class);
Expand Down Expand Up @@ -179,7 +184,7 @@ public void shouldLaunchUriUsingFallbackWhenNoCompatibleBrowserIsAvailable() {
when(browserPicker.getBestBrowserPackage(context.getPackageManager())).thenReturn(null);
CustomTabsOptions ctOptions = CustomTabsOptions.newBuilder().withBrowserPicker(browserPicker).build();
CustomTabsController controller = new CustomTabsController(context, ctOptions, twaLauncher);
controller.launchUri(uri, false, null);
controller.launchUri(uri, false, mockThreadSwitcher, null);

verify(context, timeout(MAX_TEST_WAIT_TIME_MS)).startActivity(launchIntentCaptor.capture());
Intent intent = launchIntentCaptor.getValue();
Expand All @@ -202,7 +207,7 @@ public void shouldBindAndLaunchUriWithCustomization() throws Exception {
CustomTabsController controller = new CustomTabsController(context, ctOptions, twaLauncher);

bindService(controller, true);
controller.launchUri(uri, false, null);
controller.launchUri(uri, false, mockThreadSwitcher, null);
connectBoundService();

verify(context, timeout(MAX_TEST_WAIT_TIME_MS)).startActivity(launchIntentCaptor.capture());
Expand Down Expand Up @@ -231,7 +236,7 @@ public void shouldBindAndLaunchUriWithCustomizationTwa() throws Exception {
CustomTabsController controller = new CustomTabsController(context, ctOptions, twaLauncher);

bindService(controller, true);
controller.launchUri(uri, true, null);
controller.launchUri(uri, true, mockThreadSwitcher, null);
connectBoundService();


Expand Down Expand Up @@ -260,7 +265,7 @@ public void shouldBindAndLaunchUriWithCustomizationTwa() throws Exception {
@Test
public void shouldFailToBindButLaunchUri() {
bindService(controller, false);
controller.launchUri(uri, false, null);
controller.launchUri(uri, false, mockThreadSwitcher, null);

verify(context, timeout(MAX_TEST_WAIT_TIME_MS)).startActivity(launchIntentCaptor.capture());
Intent intent = launchIntentCaptor.getValue();
Expand All @@ -274,7 +279,7 @@ public void shouldFailToBindButLaunchUri() {
public void shouldNotLaunchUriIfContextNoLongerValid() {
bindService(controller, true);
controller.clearContext();
controller.launchUri(uri, false, (ex) -> {});
controller.launchUri(uri, false, mockThreadSwitcher, null);
verify(context, never()).startActivity(any(Intent.class));
}

Expand All @@ -283,7 +288,7 @@ public void shouldLaunchUriWithFallbackIfCustomTabIntentFails() {
doThrow(ActivityNotFoundException.class)
.doNothing()
.when(context).startActivity(any(Intent.class));
controller.launchUri(uri, false, null);
controller.launchUri(uri, false, mockThreadSwitcher, null);

verify(context, timeout(MAX_TEST_WAIT_TIME_MS)).startActivity(launchIntentCaptor.capture());
List<Intent> intents = launchIntentCaptor.getAllValues();
Expand All @@ -306,12 +311,13 @@ public void shouldThrowExceptionIfFailedToLaunchBecauseOfException() {
Exception e = new SecurityException();
doThrow(e)
.when(context).startActivity(any(Intent.class));
controller.launchUri(uri, false, (ex) -> {
controller.launchUri(uri, false, mockThreadSwitcher, (ex) -> {
assertThat(ex, isA(AuthenticationException.class));
assertThat(ex.getCause(), is(eq(e)));
assertThat(ex.getCode(), is("a0.browser_not_available"));
assertThat(ex.getDescription(), is("Error launching browser for authentication"));
assertThat(Looper.myLooper(), is(Looper.getMainLooper()));
verify(mockThreadSwitcher).mainThread(any());
verify(mockThreadSwitcher, Mockito.never()).backgroundThread(any());
});
}

Expand Down

0 comments on commit c459978

Please sign in to comment.