Always cancel requests during tearDown
Assertions can terminate a test prior to that test's blockForDone call.
In this case we will try to shutdown the engine with an outstanding
request, in which case the method will throw, effectively failing the
test.
To avoid that, store a reference by the request created by the test and
always cancel it (before calling engine.shutdown()).
Also, instead of asserting the success of the requests, assume it. This
is required because these tests are not hermetic and the request might
fail due to reasons unrelated to the code being tested.
Bug: 267618889
Test: m && atest CtsNetHttpTestCasess
Change-Id: Ia1e30c99303f05342a2a73562e85d4ab2486c0d7
diff --git a/Cronet/tests/cts/src/android/net/http/cts/HttpEngineTest.java b/Cronet/tests/cts/src/android/net/http/cts/HttpEngineTest.java
index 6d27b43..8663a67 100644
--- a/Cronet/tests/cts/src/android/net/http/cts/HttpEngineTest.java
+++ b/Cronet/tests/cts/src/android/net/http/cts/HttpEngineTest.java
@@ -47,6 +47,7 @@
private HttpEngine.Builder mEngineBuilder;
private TestUrlRequestCallback mCallback;
+ private UrlRequest mRequest;
private HttpEngine mEngine;
@Before
@@ -59,6 +60,10 @@
@After
public void tearDown() throws Exception {
+ if (mRequest != null) {
+ mRequest.cancel();
+ mCallback.blockForDone();
+ }
if (mEngine != null) {
mEngine.shutdown();
}
@@ -73,9 +78,12 @@
mEngine = mEngineBuilder.build();
UrlRequest.Builder builder =
mEngine.newUrlRequestBuilder(URL, mCallback, mCallback.getExecutor());
- builder.build().start();
+ mRequest = builder.build();
+ mRequest.start();
- mCallback.expectCallback(ResponseStep.ON_SUCCEEDED);
+ // This tests uses a non-hermetic server. Instead of asserting, assume the next callback.
+ // This way, if the request were to fail, the test would just be skipped instead of failing.
+ mCallback.assumeCallback(ResponseStep.ON_SUCCEEDED);
UrlResponseInfo info = mCallback.mResponseInfo;
assertOKStatusCode(info);
assertEquals("h2", info.getNegotiatedProtocol());
@@ -86,9 +94,12 @@
mEngine = mEngineBuilder.setEnableHttp2(false).build();
UrlRequest.Builder builder =
mEngine.newUrlRequestBuilder(URL, mCallback, mCallback.getExecutor());
- builder.build().start();
+ mRequest = builder.build();
+ mRequest.start();
- mCallback.expectCallback(ResponseStep.ON_SUCCEEDED);
+ // This tests uses a non-hermetic server. Instead of asserting, assume the next callback.
+ // This way, if the request were to fail, the test would just be skipped instead of failing.
+ mCallback.assumeCallback(ResponseStep.ON_SUCCEEDED);
UrlResponseInfo info = mCallback.mResponseInfo;
assertOKStatusCode(info);
assertEquals("http/1.1", info.getNegotiatedProtocol());
@@ -104,9 +115,13 @@
mCallback = new TestUrlRequestCallback();
UrlRequest.Builder builder =
mEngine.newUrlRequestBuilder(URL, mCallback, mCallback.getExecutor());
- builder.build().start();
+ mRequest = builder.build();
+ mRequest.start();
- mCallback.expectCallback(ResponseStep.ON_SUCCEEDED);
+ // This tests uses a non-hermetic server. Instead of asserting, assume the next
+ // callback. This way, if the request were to fail, the test would just be skipped
+ // instead of failing.
+ mCallback.assumeCallback(ResponseStep.ON_SUCCEEDED);
UrlResponseInfo info = mCallback.mResponseInfo;
assumeOKStatusCode(info);
quicWasUsed = isQuic(info.getNegotiatedProtocol());
diff --git a/Cronet/tests/cts/src/android/net/http/cts/util/TestUrlRequestCallback.java b/Cronet/tests/cts/src/android/net/http/cts/util/TestUrlRequestCallback.java
index 0b9e90f..fc4912c 100644
--- a/Cronet/tests/cts/src/android/net/http/cts/util/TestUrlRequestCallback.java
+++ b/Cronet/tests/cts/src/android/net/http/cts/util/TestUrlRequestCallback.java
@@ -16,6 +16,7 @@
package android.net.http.cts.util;
+import static org.hamcrest.Matchers.equalTo;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
@@ -23,6 +24,8 @@
import static org.junit.Assert.assertSame;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
+import static org.junit.Assume.assumeThat;
+import static org.junit.Assume.assumeTrue;
import android.net.http.CallbackException;
import android.net.http.HttpException;
@@ -232,6 +235,19 @@
}
/**
+ * Waits for a terminal callback to complete execution before skipping the test if the
+ * callback is not the expected one
+ *
+ * @param expectedStep the expected callback step
+ */
+ public void assumeCallback(ResponseStep expectedStep) {
+ if (isTerminalCallback(expectedStep)) {
+ assumeTrue("Did not receive terminal callback before timeout", blockForDone());
+ }
+ assumeThat(expectedStep, equalTo(mResponseStep));
+ }
+
+ /**
* Blocks the calling thread until one of the final states has been called.
* This is called before the callback has finished executed.
*/