[Cronet] Fix ExperimentalOptionsTest flake

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

 // Copyright 2016 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.support.test.filters.SmallTest;
+import android.support.test.filters.MediumTest;
 
 import org.json.JSONObject;
 
+import org.chromium.base.Log;
 import org.chromium.base.PathUtils;
 import org.chromium.base.test.util.Feature;
 
-import java.io.BufferedReader;
 import java.io.File;
-import java.io.FileReader;
+import java.io.FileInputStream;
+import java.io.FileNotFoundException;
+import java.io.IOException;
 
 /**
  * Tests for experimental options.
  */
 public class ExperimentalOptionsTest extends CronetTestBase {
     private static final String TAG = ExperimentalOptionsTest.class.getSimpleName();
     private CronetTestFramework mTestFramework;
     private ExperimentalCronetEngine.Builder mBuilder;
 
     @Override
     protected void setUp() throws Exception {
         super.setUp();
         mBuilder = new ExperimentalCronetEngine.Builder(getContext());
         CronetTestUtil.setMockCertVerifierForTesting(
                 mBuilder, QuicTestServer.createMockCertVerifier());
         assertTrue(Http2TestServer.startHttp2TestServer(
                 getContext(), QuicTestServer.getServerCert(), QuicTestServer.getServerCertKey()));
     }
 
     @Override
     protected void tearDown() throws Exception {
         assertTrue(Http2TestServer.shutdownHttp2TestServer());
         if (mTestFramework != null && mTestFramework.mCronetEngine != null) {
             mTestFramework.mCronetEngine.shutdown();
         }
         super.tearDown();
     }
 
-    @SmallTest
+    @MediumTest
     @Feature({"Cronet"})
     @OnlyRunNativeCronet
     // Tests that NetLog writes effective experimental options to NetLog.
     public void testNetLog() throws Exception {
         File directory = new File(PathUtils.getDataDirectory());
         File logfile = File.createTempFile("cronet", "json", directory);
         JSONObject hostResolverParams = CronetTestUtil.generateHostResolverRules();
         JSONObject experimentalOptions =
                 new JSONObject().put("HostResolverRules", hostResolverParams);
         mBuilder.setExperimentalOptions(experimentalOptions.toString());
 
         mTestFramework = new CronetTestFramework(null, null, getContext(), mBuilder);
         mTestFramework.mCronetEngine.startNetLogToFile(logfile.getPath(), false);
         String url = Http2TestServer.getEchoMethodUrl();
         TestUrlRequestCallback callback = new TestUrlRequestCallback();
         UrlRequest.Builder builder = mTestFramework.mCronetEngine.newUrlRequestBuilder(
                 url, callback, callback.getExecutor());
         UrlRequest urlRequest = builder.build();
         urlRequest.start();
         callback.blockForDone();
         assertEquals(200, callback.mResponseInfo.getHttpStatusCode());
         assertEquals("GET", callback.mResponseAsString);
         mTestFramework.mCronetEngine.stopNetLog();
-        assertTrue(logfile.exists());
-        assertTrue(logfile.length() != 0);
-        BufferedReader logReader = new BufferedReader(new FileReader(logfile));
-        boolean validFile = false;
-        try {
-            String logLine;
-            while ((logLine = logReader.readLine()) != null) {
-                if (logLine.contains("HostResolverRules")) {
-                    validFile = true;
-                    break;
-                }
-            }
-        } finally {
-            logReader.close();
+        boolean contains = false;
+        for (int i = 0; i < 5; i++) {

[kapishnikov, 2017/04/18 18:50:00]

This loop is a good candidate to be refactored into a separate method like:
assertFileContainsString(logfile, string).

[xunjieli, 2017/04/19 19:02:52]

Done. Good idea!

+            contains = fileContainsString(logfile, "HostResolverRules");
+            if (contains) break;
+            Log.i(TAG, "Still waiting for netlog file.....");
+            Thread.sleep(100);
         }
-        assertTrue(validFile);
         assertTrue(logfile.delete());
-        assertTrue(!logfile.exists());
+        assertFalse(logfile.exists());
+        assertTrue("File content doesn't match", contains);

[kapishnikov, 2017/04/18 18:50:00]

Can we add the file name and the searched string to the error message? I think
it is okay not to delete the file from the device in case of a failure, so we
can open it to investigate the cause. Alternatively, we could dump the content
of the file if it is not too big.

[xunjieli, 2017/04/19 19:02:52]

Done.

     }
 
-    @SmallTest
+    @MediumTest
     @Feature({"Cronet"})
     @OnlyRunNativeCronet
     public void testSetSSLKeyLogFile() throws Exception {
         String url = Http2TestServer.getEchoMethodUrl();
         File dir = new File(PathUtils.getDataDirectory());
         File file = File.createTempFile("ssl_key_log_file", "", dir);
 
         JSONObject experimentalOptions = new JSONObject().put("ssl_key_log_file", file.getPath());
         mBuilder.setExperimentalOptions(experimentalOptions.toString());
         mTestFramework = new CronetTestFramework(null, null, getContext(), mBuilder);
 
         TestUrlRequestCallback callback = new TestUrlRequestCallback();
         UrlRequest.Builder builder = mTestFramework.mCronetEngine.newUrlRequestBuilder(
                 url, callback, callback.getExecutor());
         UrlRequest urlRequest = builder.build();
         urlRequest.start();
         callback.blockForDone();
         assertEquals(200, callback.mResponseInfo.getHttpStatusCode());
         assertEquals("GET", callback.mResponseAsString);
 
-        assertTrue(file.exists());
-        assertTrue(file.length() != 0);
-        BufferedReader logReader = new BufferedReader(new FileReader(file));
-        boolean validFile = false;
+        boolean contains = false;
+        for (int i = 0; i < 5; i++) {
+            contains = fileContainsString(file, "CLIENT_RANDOM");
+            if (contains) break;
+            Log.i(TAG, "Still waiting for ssl key log file.....");
+            Thread.sleep(100);
+        }
+        assertTrue(file.delete());
+        assertFalse(file.exists());
+        assertTrue("File content doesn't match", contains);
+    }
+
+    // Returns whether a file contains a particular string.
+    private boolean fileContainsString(File file, String content) throws IOException {
+        FileInputStream fileInputStream = null;
         try {
-            String logLine;
-            while ((logLine = logReader.readLine()) != null) {
-                if (logLine.contains("CLIENT_RANDOM")) {
-                    validFile = true;
-                    break;
-                }
+            fileInputStream = new FileInputStream(file);
+            byte[] data = new byte[(int) file.length()];
+            fileInputStream.read(data);
+            return new String(data, "UTF-8").contains(content);
+        } catch (FileNotFoundException e) {
+            // Ignored this exception since the file will only be created when updates are
+            // flushed to the disk.
+            Log.i(TAG, "file not found");
+        } finally {
+            if (fileInputStream != null) {
+                fileInputStream.close();
             }
-        } finally {
-            logReader.close();
         }
-        assertTrue(validFile);
-        assertTrue(file.delete());
-        assertTrue(!file.exists());
+        return false;
     }
 }