[test] Deflakes, un-excepts http_close_test.

Clement Skau · commit-bot@chromium.org · commit a2aec5eb0656 · 2019-07-25T08:27:34.000Z
http_close_test was given a test exception in issue 28380. This CL adds Expects to check for the expected number of error on both server and client in testClientCloseWhileSendingRequest. It also moves the server.close() into the server error handler to make sure we don't flakily close the server before all requests are fully sent and received in both ends. This should get rid of: HttpException: Connection closed while receiving data, uri = / #0 _HttpIncoming.listen [...] The modified test has been unflaky across a few hundred test runs locally. A few lints were also fixed in the process such as 'unnecessary new'. Bug: dart-lang/sdk#28380 Change-Id: I3470f70f7965c34f73da71489511e1cb2d1b7e42 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/109964 Commit-Queue: Clement Skau <cskau@google.com> Reviewed-by: Martin Kustermann <kustermann@google.com>
diff --git a/tests/standalone_2/io/http_close_test.dart b/tests/standalone_2/io/http_close_test.dart
@@ -13,7 +13,7 @@ import "dart:io";
 import "dart:typed_data";
 import "dart:math";
 
-void testClientAndServerCloseNoListen(int connections) {
+testClientAndServerCloseNoListen(int connections) {
   HttpServer.bind("127.0.0.1", 0).then((server) {
     int closed = 0;
     server.listen((request) {
@@ -28,7 +28,7 @@ void testClientAndServerCloseNoListen(int connections) {
         }
       });
     });
-    var client = new HttpClient();
+    var client = HttpClient();
     for (int i = 0; i < connections; i++) {
       client
           .get("127.0.0.1", server.port, "/")
@@ -38,10 +38,10 @@ void testClientAndServerCloseNoListen(int connections) {
   });
 }
 
-void testClientCloseServerListen(int connections) {
+testClientCloseServerListen(int connections) {
   HttpServer.bind("127.0.0.1", 0).then((server) {
     int closed = 0;
-    void check() {
+    check() {
       closed++;
       if (closed == connections * 2) {
         Expect.equals(0, server.connectionsInfo().active);
@@ -57,7 +57,7 @@ void testClientCloseServerListen(int connections) {
         request.response.done.then((_) => check());
       });
     });
-    var client = new HttpClient();
+    var client = HttpClient();
     for (int i = 0; i < connections; i++) {
       client
           .get("127.0.0.1", server.port, "/")
@@ -67,15 +67,15 @@ void testClientCloseServerListen(int connections) {
   });
 }
 
-void testClientCloseSendingResponse(int connections) {
-  var buffer = new Uint8List(64 * 1024);
-  var rand = new Random();
+testClientCloseSendingResponse(int connections) {
+  var buffer = Uint8List(64 * 1024);
+  var rand = Random();
   for (int i = 0; i < buffer.length; i++) {
     buffer[i] = rand.nextInt(256);
   }
   HttpServer.bind("127.0.0.1", 0).then((server) {
     int closed = 0;
-    void check() {
+    check() {
       closed++;
       // Wait for both server and client to see the connections as closed.
       if (closed == connections * 2) {
@@ -87,15 +87,15 @@ void testClientCloseSendingResponse(int connections) {
     }
 
     server.listen((request) {
-      var timer = new Timer.periodic(const Duration(milliseconds: 50), (_) {
+      var timer = Timer.periodic(const Duration(milliseconds: 50), (_) {
         request.response.add(buffer);
       });
       request.response.done.catchError((_) {}).whenComplete(() {
         check();
         timer.cancel();
       });
     });
-    var client = new HttpClient();
+    var client = HttpClient();
     for (int i = 0; i < connections; i++) {
       client
           .get("127.0.0.1", server.port, "/")
@@ -104,7 +104,7 @@ void testClientCloseSendingResponse(int connections) {
         // Ensure we don't accept the response until we have send the entire
         // request.
         var subscription = response.listen((_) {});
-        new Timer(const Duration(milliseconds: 20), () {
+        Timer(const Duration(milliseconds: 20), () {
           subscription.cancel();
           check();
         });
@@ -113,30 +113,37 @@ void testClientCloseSendingResponse(int connections) {
   });
 }
 
-void testClientCloseWhileSendingRequest(int connections) {
+// Closing the request early, before sending the full request payload should
+// result in an error on both server and client.
+testClientCloseWhileSendingRequest(int connections) {
   HttpServer.bind("127.0.0.1", 0).then((server) {
-    int errors = 0;
+    int serverErrors = 0;
+    int clientErrors = 0;
     server.listen((request) {
-      request.listen((_) {});
+      request.listen((_) {}, onError: (_) {
+        serverErrors++;
+        if (serverErrors == connections) {
+          server.close();
+        }
+      });
+    }, onDone: () {
+      Expect.equals(connections, clientErrors);
+      Expect.equals(connections, serverErrors);
     });
-    var client = new HttpClient();
-    int closed = 0;
+    var client = HttpClient();
     for (int i = 0; i < connections; i++) {
       client.post("127.0.0.1", server.port, "/").then((request) {
         request.contentLength = 110;
         request.write("0123456789");
         return request.close();
       }).catchError((_) {
-        closed++;
-        if (closed == connections) {
-          server.close();
-        }
+        clientErrors++;
       });
     }
   });
 }
 
-void main() {
+main() {
   testClientAndServerCloseNoListen(10);
   testClientCloseServerListen(10);
   testClientCloseSendingResponse(10);
diff --git a/tests/standalone_2/standalone_2.status b/tests/standalone_2/standalone_2.status
@@ -5,7 +5,6 @@
 # Tests using the multitest feature where failure is expected should *also* be
 # listed in tests/lib/analyzer/analyze_tests.status without the "standalone"
 # prefix.
-io/http_close_test: Pass, RuntimeError # Issue 28380
 io/non_utf8_directory_test: Skip # Issue 33519. Temp files causing bots to go purple.
 io/non_utf8_file_test: Skip # Issue 33519. Temp files causing bots to go purple.
 io/non_utf8_link_test: Skip # Issue 33519. Temp files causing bots to go purple.
diff --git a/tests/standalone_2/standalone_2_kernel.status b/tests/standalone_2/standalone_2_kernel.status
@@ -77,7 +77,6 @@ io/compile_all_test: Pass
 io/compile_all_test: Skip # We do not support --compile-all for precompilation
 io/file_fuzz_test: RuntimeError, Pass
 io/http_client_connect_test: Skip # Flaky.
-io/http_close_test: Crash
 io/http_content_length_test: Skip # Flaky.
 io/http_proxy_advanced_test: Skip # Flaky
 io/http_proxy_test: Skip # Flaky.
@@ -108,9 +107,6 @@ regress_29350_test/none: Pass # Issue 31537
 [ $compiler == dartkp && $runtime == dart_precompiled && $system != windows ]
 io/namespace_test: RuntimeError
 
-[ $compiler == dartkp && $system == android ]
-io/http_close_test: Timeout # Issue 28380
-
 [ $mode == debug && $runtime == vm && ($compiler == dartk || $compiler == dartkb) ]
 io/file_lock_test: Slow, Pass
 io/raw_socket_test: Crash
