Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1378)

Unified Diff: components/cronet/android/test/javatests/src/org/chromium/net/TestUrlRequestCallback.java

Issue 2292113002: Fix CronetUrlRequestTest#testFailures flake (Closed)
Patch Set: missed one Created 4 years, 4 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « components/cronet/android/test/javatests/src/org/chromium/net/CronetUrlRequestTest.java ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: components/cronet/android/test/javatests/src/org/chromium/net/TestUrlRequestCallback.java
diff --git a/components/cronet/android/test/javatests/src/org/chromium/net/TestUrlRequestCallback.java b/components/cronet/android/test/javatests/src/org/chromium/net/TestUrlRequestCallback.java
index da46d1b9c1858b3f569c2a4d846388181ece8dc3..ffef02d4213b11f3e3b777385e0fd17cbafa5575 100644
--- a/components/cronet/android/test/javatests/src/org/chromium/net/TestUrlRequestCallback.java
+++ b/components/cronet/android/test/javatests/src/org/chromium/net/TestUrlRequestCallback.java
@@ -19,6 +19,7 @@ import java.util.concurrent.Executor;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.ThreadFactory;
+import java.util.concurrent.TimeUnit;
/**
* Callback that tracks information from different callbacks and and has a
@@ -45,6 +46,8 @@ class TestUrlRequestCallback extends UrlRequest.Callback {
// When false, the consumer is responsible for all calls into the request
// that advance it.
private boolean mAutoAdvance = true;
+ // Whether an exception is thrown by maybeThrowCancelOrPause().
+ private boolean mListenerExceptionThrown;
// Conditionally fail on certain steps.
private FailureType mFailureType = FailureType.NONE;
@@ -91,7 +94,9 @@ class TestUrlRequestCallback extends UrlRequest.Callback {
ON_RECEIVED_REDIRECT,
ON_RESPONSE_STARTED,
ON_READ_COMPLETED,
- ON_SUCCEEDED
+ ON_SUCCEEDED,
+ ON_FAILED,
+ ON_CANCELED,
}
public enum FailureType {
@@ -130,6 +135,21 @@ class TestUrlRequestCallback extends UrlRequest.Callback {
mExecutorService.shutdown();
}
+ /**
+ * Shuts down the ExecutorService and waits until it executes all posted
+ * tasks.
+ */
+ public void shutdownExecutorAndWait() {
+ mExecutorService.shutdown();
+ try {
+ // Termination shouldn't take long. Use 5 min which should be more than enough.
+ mExecutorService.awaitTermination(5, TimeUnit.MINUTES);
kapishnikov 2016/08/31 20:35:15 Nit: Also, can we make the waiting interval smalle
xunjieli 2016/08/31 20:44:46 Done.
+ } catch (InterruptedException e) {
+ assertTrue(false);
kapishnikov 2016/08/31 20:13:20 It would be good to add a message to the assert. E
xunjieli 2016/08/31 20:44:46 Done.
+ }
+ assertTrue(mExecutorService.isTerminated());
+ }
+
@Override
public void onRedirectReceived(
UrlRequest request, UrlResponseInfo info, String newLocationUrl) {
@@ -218,7 +238,7 @@ class TestUrlRequestCallback extends UrlRequest.Callback {
assertFalse(mOnErrorCalled);
assertFalse(mOnCanceledCalled);
assertNull(mError);
- if (mFailureType == FailureType.THROW_SYNC) {
+ if (mListenerExceptionThrown) {
assertEquals(UrlRequestError.LISTENER_EXCEPTION_THROWN, error.getErrorCode());
assertEquals(0, error.getCronetInternalErrorCode());
assertEquals("Exception received from UrlRequest.Callback", error.getMessage());
@@ -228,6 +248,7 @@ class TestUrlRequestCallback extends UrlRequest.Callback {
assertFalse(error.immediatelyRetryable());
}
+ mResponseStep = ResponseStep.ON_FAILED;
mOnErrorCalled = true;
mError = error;
openDone();
@@ -243,6 +264,7 @@ class TestUrlRequestCallback extends UrlRequest.Callback {
assertFalse(mOnErrorCalled);
assertNull(mError);
+ mResponseStep = ResponseStep.ON_CANCELED;
mOnCanceledCalled = true;
openDone();
maybeThrowCancelOrPause(request);
@@ -273,6 +295,7 @@ class TestUrlRequestCallback extends UrlRequest.Callback {
* request.
*/
private boolean maybeThrowCancelOrPause(final UrlRequest request) {
+ assertEquals(mExecutorThread, Thread.currentThread());
if (mResponseStep != mFailureStep || mFailureType == FailureType.NONE) {
if (!mAutoAdvance) {
mStepBlock.open();
@@ -282,6 +305,8 @@ class TestUrlRequestCallback extends UrlRequest.Callback {
}
if (mFailureType == FailureType.THROW_SYNC) {
+ assertFalse(mListenerExceptionThrown);
+ mListenerExceptionThrown = true;
throw new IllegalStateException("Listener Exception.");
}
Runnable task = new Runnable() {
« no previous file with comments | « components/cronet/android/test/javatests/src/org/chromium/net/CronetUrlRequestTest.java ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698