From 50767f4260460d8398a65d5e8454d482b5a7f7bc Mon Sep 17 00:00:00 2001 From: Stefano Duo Date: Fri, 21 Apr 2023 14:06:30 +0100 Subject: [PATCH] Stop checking for QUIC success in CTS tests These CTS tests make use of a non-hermetic server. This means that: * Test failures are not reproducible * Test run are affected by the enviroment they run in (e.g., network configuration of the device running the tests) Ideally we would have a local QUIC test server, but setting that up is non-trivial. So, for the time being (while I come up with a QUIC test server setup), simply enable QUIC and test that things keep working. Bug: 274369142 Change-Id: Icc8b04c1671189b77f7d8995b721fb52afea371d --- .../android/net/http/cts/HttpEngineTest.java | 57 ++++++------------- 1 file changed, 16 insertions(+), 41 deletions(-) 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 ed86854dec..31990fb9bf 100644 --- a/Cronet/tests/cts/src/android/net/http/cts/HttpEngineTest.java +++ b/Cronet/tests/cts/src/android/net/http/cts/HttpEngineTest.java @@ -222,29 +222,16 @@ public class HttpEngineTest { @Test public void testHttpEngine_EnableQuic() throws Exception { + String url = mTestServer.getSuccessUrl(); mEngine = mEngineBuilder.setEnableQuic(true).addQuicHint(HOST, 443, 443).build(); - // The hint doesn't guarantee that QUIC will win the race, just that it will race TCP. - // We send multiple requests to reduce the flakiness of the test. - boolean quicWasUsed = false; - for (int i = 0; i < 5; i++) { - mCallback = new TestUrlRequestCallback(); - UrlRequest.Builder builder = - mEngine.newUrlRequestBuilder(URL, mCallback.getExecutor(), mCallback); - mRequest = builder.build(); - mRequest.start(); + UrlRequest.Builder builder = + mEngine.newUrlRequestBuilder(url, mCallback.getExecutor(), mCallback); + mRequest = builder.build(); + mRequest.start(); - // 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()); - if (quicWasUsed) { - break; - } - } - assertTrue(quicWasUsed); + mCallback.expectCallback(ResponseStep.ON_SUCCEEDED); + UrlResponseInfo info = mCallback.mResponseInfo; + assertOKStatusCode(info); } @Test @@ -379,34 +366,22 @@ public class HttpEngineTest { @Test public void testHttpEngine_SetQuicOptions_RequestSucceedsWithQuic() throws Exception { + String url = mTestServer.getSuccessUrl(); QuicOptions options = new QuicOptions.Builder().build(); mEngine = mEngineBuilder .setEnableQuic(true) .addQuicHint(HOST, 443, 443) .setQuicOptions(options) .build(); + UrlRequest.Builder builder = + mEngine.newUrlRequestBuilder(url, mCallback.getExecutor(), mCallback); + mRequest = builder.build(); + mRequest.start(); - // The hint doesn't guarantee that QUIC will win the race, just that it will race TCP. - // We send multiple requests to reduce the flakiness of the test. - boolean quicWasUsed = false; - for (int i = 0; i < 5; i++) { - mCallback = new TestUrlRequestCallback(); - UrlRequest.Builder builder = - mEngine.newUrlRequestBuilder(URL, mCallback.getExecutor(), mCallback); - mRequest = builder.build(); - mRequest.start(); - mCallback.blockForDone(); + mCallback.expectCallback(ResponseStep.ON_SUCCEEDED); + UrlResponseInfo info = mCallback.mResponseInfo; + assertOKStatusCode(info); - quicWasUsed = isQuic(mCallback.mResponseInfo.getNegotiatedProtocol()); - if (quicWasUsed) { - break; - } - } - - assertTrue(quicWasUsed); - // 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. - assumeOKStatusCode(mCallback.mResponseInfo); } @Test