Enabling sampling of UMA and crash reports on Android.

chrome/android/javatests/src/org/chromium/chrome/browser/crash/MinidumpUploadCallableTest.java

 // Copyright 2015 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.chrome.browser.crash;
 
 import android.test.suitebuilder.annotation.SmallTest;
 
 import org.chromium.base.annotations.SuppressFBWarnings;
 import org.chromium.base.test.util.Feature;
@@ skipping 95 matching lines @@
     private static class FailHttpURLConnectionFactory implements HttpURLConnectionFactory {
         @Override
         public HttpURLConnection createHttpURLConnection(String url) {
             fail();
             return null;
         }
     }
 
     private static class MockCrashReportingPermissionManager
             implements CrashReportingPermissionManager {
+        private final boolean mIsInSample;
         private final boolean mIsPermitted;
         private final boolean mIsUserPermitted;
         private final boolean mIsCommandLineDisabled;
         private final boolean mIsLimited;
         private final boolean mIsEnabledForTests;
 
-        MockCrashReportingPermissionManager(boolean isPermitted, boolean isUserPermitted,
-                boolean isCommandLineDisabled, boolean isLimited, boolean isEnabledForTests) {
+        MockCrashReportingPermissionManager(boolean isInSample, boolean isPermitted,
+                boolean isUserPermitted, boolean isCommandLineDisabled, boolean isLimited,
+                boolean isEnabledForTests) {
+            mIsInSample = isInSample;

[Alexei Svitkine (slow), 2016/08/18 06:16:39]

Sigh, I really hate this mock class, as it makes it completely not obvious what
each param does during initialization.

I think you can do better by making the members protected and using brace
initializer syntax at the callsite.

e.g.

new MockCrashreportingPermissionManager() {{
  mIsInSample = true;
}};

I think the current state of this code is extremely unreadable - and with this
CL you're incrementally making it worse, so I suggest just making a default ctor
for this and changing the tests to use the above syntax. Given your current
patch is already changing all the call sites, I suggest just cleaning things up
and migrating to the above syntax, so it's easy to tell what each test is doing.

[jwd, 2016/08/23 18:40:03]

Done.

             mIsPermitted = isPermitted;
             mIsUserPermitted = isUserPermitted;
             mIsCommandLineDisabled = isCommandLineDisabled;
             mIsLimited = isLimited;
             mIsEnabledForTests = isEnabledForTests;
         }
 
         @Override
+        public boolean isClientInMetricsSample() {
+            return mIsInSample;
+        }
+
+        @Override
         public boolean isUploadPermitted() {
             return mIsPermitted;
         }
 
         @Override
         public boolean isUmaUploadPermitted() {
             return mIsPermitted;
         }
 
         @Override
@@ skipping 51 matching lines @@
     @Override
     protected void tearDown() throws Exception {
         if (mTestUpload.exists()) mTestUpload.delete();
         super.tearDown();
     }
 
     @SmallTest
     @Feature({"Android-AppBase"})
     public void testCallWhenCurrentlyPermitted() throws Exception {
         CrashReportingPermissionManager testPermManager =
-                new MockCrashReportingPermissionManager(true, true, false, false, false);
+                new MockCrashReportingPermissionManager(true, true, true, false, false, false);
 
         HttpURLConnectionFactory httpURLConnectionFactory = new TestHttpURLConnectionFactory();
 
         MinidumpUploadCallable minidumpUploadCallable =
                 new MockMinidumpUploadCallable(httpURLConnectionFactory, testPermManager);
         assertEquals(MinidumpUploadCallable.UPLOAD_SUCCESS,
                 minidumpUploadCallable.call().intValue());
         assertTrue(mExpectedFileAfterUpload.exists());
         assertValidUploadLogEntry();
     }
 
     @SmallTest
     @Feature({"Android-AppBase"})
     public void testCallNotPermittedByUser() throws Exception {
         CrashReportingPermissionManager testPermManager =
-                new MockCrashReportingPermissionManager(false, false, false, false, false);
+                new MockCrashReportingPermissionManager(true, false, false, false, false, false);
 
         HttpURLConnectionFactory httpURLConnectionFactory = new FailHttpURLConnectionFactory();
 
         MinidumpUploadCallable minidumpUploadCallable =
                 new MockMinidumpUploadCallable(httpURLConnectionFactory, testPermManager);
         assertEquals(MinidumpUploadCallable.UPLOAD_USER_DISABLED,
                 minidumpUploadCallable.call().intValue());
         assertTrue(mExpectedFileAfterUpload.exists());
     }
 
     @SmallTest
     @Feature({"Android-AppBase"})
     public void testCallNotPermittedByCommandLine() throws Exception {
         CrashReportingPermissionManager testPermManager =
-                new MockCrashReportingPermissionManager(true, true, true, false, false);
+                new MockCrashReportingPermissionManager(true, true, true, true, false, false);
 
         HttpURLConnectionFactory httpURLConnectionFactory = new FailHttpURLConnectionFactory();
 
         MinidumpUploadCallable minidumpUploadCallable =
                 new MockMinidumpUploadCallable(httpURLConnectionFactory, testPermManager);
         assertEquals(MinidumpUploadCallable.UPLOAD_COMMANDLINE_DISABLED,
                 minidumpUploadCallable.call().intValue());
         assertFalse(mExpectedFileAfterUpload.exists());
     }
 
     @SmallTest
     @Feature({"Android-AppBase"})
+    public void testCallPermittedButNotInSample() throws Exception {
+        CrashReportingPermissionManager testPermManager =
+                new MockCrashReportingPermissionManager(false, true, true, false, false, false);
+
+        HttpURLConnectionFactory httpURLConnectionFactory = new TestHttpURLConnectionFactory();
+
+        MinidumpUploadCallable minidumpUploadCallable =
+                new MockMinidumpUploadCallable(httpURLConnectionFactory, testPermManager);
+        assertEquals(
+                MinidumpUploadCallable.UPLOAD_FAILURE, minidumpUploadCallable.call().intValue());
+        assertFalse(mExpectedFileAfterUpload.exists());
+    }
+
+    @SmallTest
+    @Feature({"Android-AppBase"})
     public void testCallPermittedButNotUnderCurrentCircumstances() throws Exception {
         CrashReportingPermissionManager testPermManager =
-                new MockCrashReportingPermissionManager(false, true, false, false, false);
+                new MockCrashReportingPermissionManager(true, false, true, false, false, false);
 
         HttpURLConnectionFactory httpURLConnectionFactory = new FailHttpURLConnectionFactory();
 
         MinidumpUploadCallable minidumpUploadCallable =
                 new MockMinidumpUploadCallable(httpURLConnectionFactory, testPermManager);
         assertEquals(MinidumpUploadCallable.UPLOAD_FAILURE,
                 minidumpUploadCallable.call().intValue());
         assertFalse(mExpectedFileAfterUpload.exists());
     }
 
     @SmallTest
     @Feature({"Android-AppBase"})
     public void testCrashUploadConstrainted() throws Exception {
         CrashReportingPermissionManager testPermManager =
-                new MockCrashReportingPermissionManager(true, true, false, true, false);
+                new MockCrashReportingPermissionManager(true, true, true, false, true, false);
 
         HttpURLConnectionFactory httpURLConnectionFactory = new TestHttpURLConnectionFactory();
 
         MinidumpUploadCallable minidumpUploadCallable =
                 new MockMinidumpUploadCallable(httpURLConnectionFactory, testPermManager);
         assertEquals(MinidumpUploadCallable.UPLOAD_FAILURE,
                 minidumpUploadCallable.call().intValue());
         assertFalse(mExpectedFileAfterUpload.exists());
     }
 
     @SmallTest
     @Feature({"Android-AppBase"})
     public void testCrashUploadEnabledForTestsDespiteConstraints() throws Exception {
         CrashReportingPermissionManager testPermManager =
-                new MockCrashReportingPermissionManager(false, false, false, true, true);
+                new MockCrashReportingPermissionManager(true, false, false, false, true, true);
 
         HttpURLConnectionFactory httpURLConnectionFactory = new TestHttpURLConnectionFactory();
 
         MinidumpUploadCallable minidumpUploadCallable =
                 new MockMinidumpUploadCallable(httpURLConnectionFactory, testPermManager);
         assertEquals(MinidumpUploadCallable.UPLOAD_SUCCESS,
                 minidumpUploadCallable.call().intValue());
         assertTrue(mExpectedFileAfterUpload.exists());
         assertValidUploadLogEntry();
     }
@@ skipping 28 matching lines @@
 
         // Sanity check on the time stamp (within an hour).
         // Chances are the write and the check should have less than 1 second in between.
         assertTrue(time <= now);
         assertTrue(time > now - 60 * 60);
 
         String id = lastEntry.substring(seperator + 1, lastEntry.length());
         assertEquals(id, CRASH_ID);
     }
 }