Cronet: a framework for providing alternative Cronet implementations

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

Index: components/cronet/android/test/javatests/src/org/chromium/net/CronetEngineBuilderTest.java
diff --git a/components/cronet/android/test/javatests/src/org/chromium/net/CronetEngineBuilderTest.java b/components/cronet/android/test/javatests/src/org/chromium/net/CronetEngineBuilderTest.java
new file mode 100644
index 0000000000000000000000000000000000000000..2d13357db27eaf90029f4506ff291bd8a1a685da
--- /dev/null
+++ b/components/cronet/android/test/javatests/src/org/chromium/net/CronetEngineBuilderTest.java
@@ -0,0 +1,166 @@
+// Copyright 2017 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.content.Context;
+import android.support.test.filters.SmallTest;
+
+import static org.chromium.net.CronetProviders.PROVIDER_NAME_NATIVE;
+import static org.chromium.net.CronetProviders.PROVIDER_NAME_PLATFORM;
+
+import org.chromium.base.test.util.Feature;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+
+/**
+ * Tests {@link CronetEngine.Builder}.
+ */
+public class CronetEngineBuilderTest extends CronetTestBase {
+    /**
+     * Tests the comparison of two strings that contain versions.
+     */
+    @SmallTest
+    @Feature({"Cronet"})
+    @CronetTestBase.OnlyRunNativeCronet
+    public void testVersionComparison() {
+        assertVersionIsHigher("22.44", "22.43.12");
+        assertVersionIsLower("22.43.12", "022.124");
+        assertVersionIsLower("22.99", "22.100");
+        assertVersionIsHigher("22.100", "22.99");
+        assertVersionIsEqual("11.2.33", "11.2.33");
+        assertIllegalArgumentException(null, "1.2.3");
+        assertIllegalArgumentException("1.2.3", null);
+        assertIllegalArgumentException("1.2.3", "1.2.3x");
+    }
+
+    /**
+     * Tests the correct ordering of the providers. The platform provider should be
+     * the last in the list. Other providers should be ordered by placing providers
+     * with the higher version first.
+     */
+    @SmallTest
+    @Feature({"Cronet"})
+    @CronetTestBase.OnlyRunNativeCronet
+    public void testProviderOrdering() {
+        final CronetProvider[] availableProviders = new CronetProvider[] {
+                new FakeProvider(getContext(), PROVIDER_NAME_NATIVE, "99.77", true),
+                new FakeProvider(getContext(), PROVIDER_NAME_PLATFORM, "99.99", true),
+                new FakeProvider(getContext(), "Some other provider", "99.88", true),
+        };
+        int[] expectedPositions = {1, 2, 0};
+
+        CronetProviders providers = new FakeProviders(availableProviders);
+        List<CronetProvider> orderedProviders =
+                CronetEngine.Builder.getEnabledCronetProviders(getContext(), providers);
+
+        // Check the result.
+        for (int i = 0; i < availableProviders.length; i++) {

[mef, 2017/01/19 22:57:46]

Can this just be assertEquals(PROVIDER_NAME_NATIVE,
orderedProviders.get(0).getName()), etc?
Same way as in the next test?

[kapishnikov, 2017/01/20 16:21:54]

Good point. Done.

+            int expectedPosition = expectedPositions[i];
+            assertEquals("i=" + i + ". Provider '" + availableProviders[i]
+                            + "' should be at position " + expectedPosition + ".",
+                    availableProviders[i], orderedProviders.get(expectedPosition));
+        }
+    }
+
+    /**
+     * Tests that the providers that are disabled are not included in the list of available
+     * providers when the provider is selected by the default selection logic.
+     */
+    @SmallTest
+    @Feature({"Cronet"})
+    @CronetTestBase.OnlyRunNativeCronet

[mef, 2017/01/19 22:57:46]

Why is it marked OnlyRunNativeCronet? Would running it twice break?

[kapishnikov, 2017/01/20 16:21:54]

Done.

+    public void testThatDisabledProvidersAreExcluded() {
+        final CronetProvider[] availableProviders = new CronetProvider[] {
+                new FakeProvider(getContext(), PROVIDER_NAME_PLATFORM, "99.99", true),
+                new FakeProvider(getContext(), PROVIDER_NAME_NATIVE, "99.77", true),
+                new FakeProvider(getContext(), "Some other provider", "99.88", false),
+        };
+
+        CronetProviders providers = new FakeProviders(availableProviders);
+        List<CronetProvider> orderedProviders =
+                CronetEngine.Builder.getEnabledCronetProviders(getContext(), providers);
+
+        assertEquals("Unexpected number of providers in the list", 2, orderedProviders.size());
+        assertEquals(PROVIDER_NAME_NATIVE, orderedProviders.get(0).getName());
+        assertEquals(PROVIDER_NAME_PLATFORM, orderedProviders.get(1).getName());
+    }
+
+    private void assertVersionIsHigher(String s1, String s2) {
+        assertEquals(1, CronetEngine.Builder.compareVersions(s1, s2));
+    }
+
+    private void assertVersionIsLower(String s1, String s2) {
+        assertEquals(-1, CronetEngine.Builder.compareVersions(s1, s2));
+    }
+
+    private void assertVersionIsEqual(String s1, String s2) {
+        assertEquals(0, CronetEngine.Builder.compareVersions(s1, s2));
+    }
+
+    private void assertIllegalArgumentException(String s1, String s2) {
+        try {
+            CronetEngine.Builder.compareVersions(s1, s2);
+        } catch (IllegalArgumentException e) {
+            // Do nothing. It is expected.
+            return;
+        }
+        fail("Expected IllegalArgumentException");
+    }
+
+    // TODO(kapishnikov): Replace with a mock when mockito is supported.

[mef, 2017/01/19 22:57:46]

FWIW it seems that mockito is already supported:
https://cs.chromium.org/search/?q=org.mockito.Mockito&sq=package:chromium&typ...
Feel free to do it in followup CL though.

[kapishnikov, 2017/01/20 16:21:54]

The components that use mockito run junit tests (i.e. compared to
instrumentation tests that Cronet has). I tried to include the mockito library
directly but got a message that some of the dependencies do not support android.
We should address it in a follow up CL.

+    private static class FakeProvider extends CronetProvider {
+        private final String mName;
+        private final String mVersion;
+        private final boolean mEnabled;
+
+        protected FakeProvider(Context context, String name, String version, boolean enabled) {
+            super(context);
+            mName = name;
+            mVersion = version;
+            mEnabled = enabled;
+        }
+
+        @Override
+        public CronetEngine.Builder createBuilder() {
+            return new CronetEngine.Builder((ICronetEngineBuilder) null);
+        }
+
+        @Override
+        public String getName() {
+            return mName;
+        }
+
+        @Override
+        public String getVersion() {
+            return mVersion;
+        }
+
+        @Override
+        public boolean isEnabled() {
+            return mEnabled;
+        }
+
+        @Override
+        public String toString() {
+            return mName;
+        }
+    }
+
+    // TODO(kapishnikov): Replace with a mock when mockito is supported.

[mef, 2017/01/19 22:57:46]

unused?

[kapishnikov, 2017/01/20 16:21:54]

The class is used in testProviderOrdering() and
testThatDisabledProvidersAreExcluded().

+    private static class FakeProviders extends CronetProviders {
+        final List<CronetProvider> mList;
+
+        private FakeProviders(CronetProvider[] providers) {
+            mList = new ArrayList<>(Arrays.asList(providers));
+        }
+
+        @Override
+        public List<CronetProvider> getAvailableProviders(Context context) {
+            return mList;
+        }
+    }
+}