Instrument CronetUrlRequestTest#testThrowOrCancelInOnCanceled flaky test

components/cronet/android/test/javatests/src/org/chromium/net/CronetUrlRequestTest.java

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 package org.chromium.net;
 
 import android.os.ConditionVariable;
 import android.os.StrictMode;
 import android.test.MoreAsserts;
 import android.test.suitebuilder.annotation.SmallTest;
@@ skipping 1648 matching lines @@
         callback.setFailure(failureType, failureStep);
         UrlRequest.Builder builder = mTestFramework.mCronetEngine.newUrlRequestBuilder(
                 NativeTestServer.getRedirectURL(), callback, callback.getExecutor());
         UrlRequest urlRequest = builder.build();
         urlRequest.start();
         callback.blockForDone();
         // Wait for all posted tasks to be executed to ensure there is no unhandled exception.
         callback.shutdownExecutorAndWait();
         assertEquals(1, callback.mRedirectCount);
         if (failureType == FailureType.CANCEL_SYNC || failureType == FailureType.CANCEL_ASYNC) {
-            assertEquals(ResponseStep.ON_CANCELED, callback.mResponseStep);
+            assertResponseStepCancelled(callback);
         } else if (failureType == FailureType.THROW_SYNC) {
             assertEquals(ResponseStep.ON_FAILED, callback.mResponseStep);
         }
         assertTrue(urlRequest.isDone());
         assertEquals(expectResponseInfo, callback.mResponseInfo != null);
         assertEquals(expectError, callback.mError != null);
         assertEquals(expectError, callback.mOnErrorCalled);
         assertEquals(failureType == FailureType.CANCEL_SYNC
                         || failureType == FailureType.CANCEL_ASYNC
                         || failureType == FailureType.CANCEL_ASYNC_WITHOUT_PAUSE,
@@ skipping 97 matching lines @@
                 }
             };
             callback.setFailure(type, ResponseStep.ON_CANCELED);
             UrlRequest.Builder builder = mTestFramework.mCronetEngine.newUrlRequestBuilder(
                     NativeTestServer.getEchoBodyURL(), callback, callback.getExecutor());
             UrlRequest urlRequest = builder.build();
             urlRequest.start();
             callback.blockForDone();
             // Wait for all posted tasks to be executed to ensure there is no unhandled exception.
             callback.shutdownExecutorAndWait();
-            assertEquals(ResponseStep.ON_CANCELED, callback.mResponseStep);
+            assertResponseStepCancelled(callback);
             assertTrue(urlRequest.isDone());
             assertNotNull(callback.mResponseInfo);
             assertNull(callback.mError);
             assertTrue(callback.mOnCanceledCalled);
         }
     }
 
     @SmallTest
     @Feature({"Cronet"})
     @OnlyRunNativeCronet // No destroyed callback for tests
@@ skipping 206 matching lines @@
     // as a String. Does not modify byteBuffer's position().
     private String bufferContentsToString(ByteBuffer byteBuffer, int start, int end) {
         // Use a duplicate to avoid modifying byteBuffer.
         ByteBuffer duplicate = byteBuffer.duplicate();
         duplicate.position(start);
         duplicate.limit(end);
         byte[] contents = new byte[duplicate.remaining()];
         duplicate.get(contents);
         return new String(contents);
     }
+
+    private void assertResponseStepCancelled(TestUrlRequestCallback callback) {
+        String message = "Unexpected response state " + callback.mResponseStep;

[Charles, 2016/11/15 19:08:16]

A more idiomatic way to show this would be to say 

if (state) {
  throw new AssertionError(callback.mError)
}

So the callback's error is the cause of the assertion that fails the test. This
way you get message, and stack trace info.

[kapishnikov, 2016/11/15 21:17:01]

Done. The AssertionError with cause was only introduced in JDK 19. Our min SDK
is set to 16. So, I used the plain Error instead.

[Charles, 2016/11/15 21:38:22]

I think you may need to upload a new patchset

[kapishnikov, 2016/11/15 21:49:17]

Because the call to the supertype constructor should be the first statement in
the constructor.

[Charles, 2016/11/15 22:29:33]

That's true, but if you make this logic a private method you can do it -
super(foomethod(param)); is valid java.

+        if (callback.mError != null) {
+            message += message + ". Recorded error: " + callback.mError;
+        }
+        assertEquals(message, ResponseStep.ON_CANCELED, callback.mResponseStep);
+    }
 }