Instrument CronetUrlRequestTest#testThrowOrCancelInOnCanceled flaky test

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

Index: components/cronet/android/test/javatests/src/org/chromium/net/CronetUrlRequestTest.java
diff --git a/components/cronet/android/test/javatests/src/org/chromium/net/CronetUrlRequestTest.java b/components/cronet/android/test/javatests/src/org/chromium/net/CronetUrlRequestTest.java
index 694cfebd2e4f92811b71666414cce072878046a1..6ffc7e21c49fee812dfb282bfcd64a0ed1192d25 100644
--- a/components/cronet/android/test/javatests/src/org/chromium/net/CronetUrlRequestTest.java
+++ b/components/cronet/android/test/javatests/src/org/chromium/net/CronetUrlRequestTest.java
@@ -1666,7 +1666,7 @@ public class CronetUrlRequestTest extends CronetTestBase {
         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);
         }
@@ -1784,7 +1784,7 @@ public class CronetUrlRequestTest extends CronetTestBase {
             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);
@@ -2011,4 +2011,12 @@ public class CronetUrlRequestTest extends CronetTestBase {
         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);
+    }
 }