Fix the iOS performance test to correctly detect failed requests

Without the change the test considers a requests failed only if it timed out. It doesn't take into consideration the response staus code. BUG=706515 Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet Change-Id: Ibe938c0486547cbe64ce2545fa016b71c28ff1d3 Reviewed-on: https://chromium-review.googlesource.com/862665 Commit-Queue: Andrei Kapishnikov <kapishnikov@chromium.org> Reviewed-by: Misha Efimov <mef@chromium.org> Cr-Commit-Position: refs/heads/master@{#530195}
Denger-Network · Jan 18, 2018 · 6d5eb3c · 6d5eb3c
1 parent d5dd606
commit 6d5eb3c
Show file tree

Hide file tree

Showing 4 changed files with 35 additions and 12 deletions.
diff --git a/components/cronet/ios/test/cronet_performance_test.mm b/components/cronet/ios/test/cronet_performance_test.mm
@@ -209,11 +209,13 @@ void TearDown() override {
       [task cancel];
     }
 
+    success = success && IsResponseSuccessful(task);
+
     NSTimeInterval elapsed = -[start timeIntervalSinceNow];
 
     // Do not tolerate failures on internal server.
     if (!kUseExternalUrl) {
-      CHECK(success && ![delegate_ errorPerTask][task]);
+      CHECK(success);
     }
 
     if (kUseExternalUrl && success && !first_log) {

diff --git a/components/cronet/ios/test/cronet_pkp_test.mm b/components/cronet/ios/test/cronet_pkp_test.mm
@@ -67,10 +67,10 @@ void sendRequestAndAssertResult(NSURL* url, bool expected_success) {
         [url_session_ dataTaskWithURL:request_url_];
     StartDataTaskAndWaitForCompletion(dataTask);
     if (expected_success) {
-      ASSERT_TRUE(IsResponseSuccessful());
+      ASSERT_TRUE(IsResponseSuccessful(dataTask));
     } else {
-      ASSERT_FALSE(IsResponseSuccessful());
-      ASSERT_FALSE(IsResponseCanceled());
+      ASSERT_FALSE(IsResponseSuccessful(dataTask));
+      ASSERT_FALSE(IsResponseCanceled(dataTask));
     }
   }
 

diff --git a/components/cronet/ios/test/cronet_test_base.h b/components/cronet/ios/test/cronet_test_base.h
@@ -53,6 +53,11 @@ typedef void (^BlockType)(void);
 // Contains metrics data.
 @property(readonly) NSURLSessionTaskMetrics* taskMetrics NS_AVAILABLE_IOS(10.0);
 
+// Contains NSHTTPURLResponses for the tasks.
+@property(readonly)
+    NSMutableDictionary<NSURLSessionDataTask*, NSHTTPURLResponse*>*
+        responsePerTask;
+
 // Resets the delegate, so it can be used again for another request.
 - (void)reset;
 
@@ -100,8 +105,8 @@ class CronetTestBase : public ::testing::Test {
   void PostBlockToNetworkThread(const base::Location& from_here,
                                 BlockType block);
 
-  ::testing::AssertionResult IsResponseSuccessful();
-  ::testing::AssertionResult IsResponseCanceled();
+  ::testing::AssertionResult IsResponseSuccessful(NSURLSessionDataTask* task);
+  ::testing::AssertionResult IsResponseCanceled(NSURLSessionDataTask* task);
 
   TestDelegate* delegate_;
 

diff --git a/components/cronet/ios/test/cronet_test_base.mm b/components/cronet/ios/test/cronet_test_base.mm
@@ -31,6 +31,7 @@ @implementation TestDelegate {
 @synthesize totalBytesReceivedPerTask = _totalBytesReceivedPerTask;
 @synthesize expectedContentLengthPerTask = _expectedContentLengthPerTask;
 @synthesize taskMetrics = _taskMetrics;
+@synthesize responsePerTask = _responsePerTask;
 
 - (id)init {
   if (self = [super init]) {
@@ -45,6 +46,7 @@ - (void)reset {
   _totalBytesReceivedPerTask = [NSMutableDictionary dictionaryWithCapacity:0];
   _expectedContentLengthPerTask =
       [NSMutableDictionary dictionaryWithCapacity:0];
+  _responsePerTask = [NSMutableDictionary dictionaryWithCapacity:0];
   _taskMetrics = nil;
 }
 
@@ -162,6 +164,7 @@ - (void)URLSession:(NSURLSession*)session
                            completionHandler {
   _expectedContentLengthPerTask[dataTask] =
       [NSNumber numberWithInt:[response expectedContentLength]];
+  _responsePerTask[dataTask] = static_cast<NSHTTPURLResponse*>(response);
   completionHandler(NSURLSessionResponseAllow);
 }
 
@@ -217,22 +220,35 @@ - (void)URLSession:(NSURLSession*)session
   return [delegate_ waitForDone:task withTimeout:deadline_ns];
 }
 
-::testing::AssertionResult CronetTestBase::IsResponseSuccessful() {
-  if ([delegate_ error]) {
+::testing::AssertionResult CronetTestBase::IsResponseSuccessful(
+    NSURLSessionDataTask* task) {
+  if ([delegate_ errorPerTask][task]) {
     return ::testing::AssertionFailure() << "error in response: " <<
            [[[delegate_ error] description]
                cStringUsingEncoding:NSUTF8StringEncoding];
   }
+
+  if (![delegate_ responsePerTask][task]) {
+    return ::testing::AssertionFailure() << " no response has been received";
+  }
+
+  NSInteger statusCode = [delegate_ responsePerTask][task].statusCode;
+  if (statusCode < 200 || statusCode > 299) {
+    return ::testing::AssertionFailure()
+           << " the response code was " << statusCode;
+  }
+
   return ::testing::AssertionSuccess() << "no errors in response";
 }
 
-::testing::AssertionResult CronetTestBase::IsResponseCanceled() {
-  if ([delegate_ error] && [[delegate_ error] code] == NSURLErrorCancelled)
+::testing::AssertionResult CronetTestBase::IsResponseCanceled(
+    NSURLSessionDataTask* task) {
+  NSError* error = [delegate_ errorPerTask][task];
+  if (error && [error code] == NSURLErrorCancelled)
     return ::testing::AssertionSuccess() << "the response is canceled";
   return ::testing::AssertionFailure() << "the response is not canceled."
                                        << " The response error is " <<
-         [[[delegate_ error] description]
-             cStringUsingEncoding:NSUTF8StringEncoding];
+         [[error description] cStringUsingEncoding:NSUTF8StringEncoding];
 }
 
 std::unique_ptr<net::MockCertVerifier> CronetTestBase::CreateMockCertVerifier(