Issue 2561803002: Cronet smoke tests

kapishnikov

Description was changed from ========== Cronet smoke tests 1. Added test support classes that allow ...

4 years ago (2016-12-08 17:09:25 UTC) #1

Description was changed from

==========
Cronet smoke tests

1. Added test support classes that allow running the smoke tests in
   different environments against different servers. There is one
   support class for Java-only tests and one that requires H2 & QUIC.
   The concrete implementation of support classes is determined at
   runtime by reading the value of |TestSupportImplClass| from the
   Android string resource file.
2. Added two test APKs one for Java only deployment and one to test
   behavior when the native “.so” file is absent.
3. Enabled proguarding for the existing unit tests. The new APKS also
   run proguard in order to check the correctness of proguard configs
   for different deployments.
4. Added new HttpServer for Java-only scenario that does not depend on
   Chromium “base” and “net” modules.
5. Added two additional smoke QUIC & H2 tests to the existing test APK.


To run all smoke tests, execute:
ninja -C out/Debug cronet_java_only_smoke_tests_instrumentation_apk \
cronet_broken_so_file_smoke_tests_instrumentation_apk \
cronet_test_instrumentation_apk -j 240 && \
./out/Debug/bin/run_cronet_java_only_smoke_tests_instrumentation_apk && \
./out/Debug/bin/run_cronet_broken_so_file_smoke_tests_instrumentation_apk && \
./out/Debug/bin/run_cronet_test_instrumentation_apk \
--test-filter=org.chromium.net.smoke.*

BUG=669264
==========

to

==========
Cronet smoke tests

1. Added test support classes that allow running the smoke tests in
   different environments against different servers. There is one
   support class for Java-only tests and one that requires H2 & QUIC.
   The concrete implementation of support classes is determined at
   runtime by reading the value of |TestSupportImplClass| from the
   Android string resource file.
2. Added two test APKs one for Java only deployment and one to test
   behavior when the native “.so” file is absent.
3. Enabled proguarding for the existing unit tests. The new APKS also
   run proguard in order to check the correctness of proguard configs
   for different deployments.
4. Added new HttpServer for Java-only scenario that does not depend on
   Chromium “base” and “net” modules.
5. Added two additional smoke QUIC & H2 tests to the existing test APK.


To run all smoke tests, execute:
ninja -C out/Debug cronet_java_only_smoke_tests_instrumentation_apk \
cronet_broken_so_file_smoke_tests_instrumentation_apk \
cronet_test_instrumentation_apk -j 240 && \
./out/Debug/bin/run_cronet_java_only_smoke_tests_instrumentation_apk && \
./out/Debug/bin/run_cronet_broken_so_file_smoke_tests_instrumentation_apk && \
./out/Debug/bin/run_cronet_test_instrumentation_apk \
--test-filter=org.chromium.net.smoke.*

BUG=669264
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester
==========

kapishnikov

Description was changed from ========== Cronet smoke tests 1. Added test support classes that allow ...

4 years ago (2016-12-08 17:42:14 UTC) #2

Description was changed from

==========
Cronet smoke tests

1. Added test support classes that allow running the smoke tests in
   different environments against different servers. There is one
   support class for Java-only tests and one that requires H2 & QUIC.
   The concrete implementation of support classes is determined at
   runtime by reading the value of |TestSupportImplClass| from the
   Android string resource file.
2. Added two test APKs one for Java only deployment and one to test
   behavior when the native “.so” file is absent.
3. Enabled proguarding for the existing unit tests. The new APKS also
   run proguard in order to check the correctness of proguard configs
   for different deployments.
4. Added new HttpServer for Java-only scenario that does not depend on
   Chromium “base” and “net” modules.
5. Added two additional smoke QUIC & H2 tests to the existing test APK.


To run all smoke tests, execute:
ninja -C out/Debug cronet_java_only_smoke_tests_instrumentation_apk \
cronet_broken_so_file_smoke_tests_instrumentation_apk \
cronet_test_instrumentation_apk -j 240 && \
./out/Debug/bin/run_cronet_java_only_smoke_tests_instrumentation_apk && \
./out/Debug/bin/run_cronet_broken_so_file_smoke_tests_instrumentation_apk && \
./out/Debug/bin/run_cronet_test_instrumentation_apk \
--test-filter=org.chromium.net.smoke.*

BUG=669264
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester
==========

to

==========
Cronet smoke tests

1. Added test support classes that allow running the smoke tests in
   different environments against different servers. There is one
   support class for Java-only tests and one that requires H2 & QUIC.
   The concrete implementation of support classes is determined at
   runtime by reading the value of |TestSupportImplClass| from the
   Android string resource file.
2. Added two test APKs one for Java only deployment and one to test
   behavior when the native “.so” file is absent.
3. Enabled proguarding for the existing unit tests. The new APKS also
   run proguard in order to check the correctness of proguard configs
   for different deployments.
4. Added new HttpServer for Java-only scenario that does not depend on
   Chromium “base” and “net” modules.
5. Added two additional smoke QUIC & H2 tests to the existing test APK.


To run all smoke tests, execute:
ninja -C out/Debug cronet_java_only_smoke_tests_instrumentation_apk \
cronet_broken_so_file_smoke_tests_instrumentation_apk \
cronet_test_instrumentation_apk -j 240 && \
./out/Debug/bin/run_cronet_java_only_smoke_tests_instrumentation_apk && \
./out/Debug/bin/run_cronet_broken_so_file_smoke_tests_instrumentation_apk && \
./out/Debug/bin/run_cronet_test_instrumentation_apk \
--test-filter=org.chromium.net.smoke.*

The output jars that contain the smoke tests:
./out/Debug/lib.java/components/cronet/android/cronet_quic_smoke_test_java.jar
./out/Debug/lib.java/components/cronet/android/cronet_java_only_smoke_test_java.jar
./out/Debug/lib.java/components/cronet/android/cronet_broken_so_file_smoke_test_java.jar

BUG=669264
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester
==========

kapishnikov

kapishnikov@chromium.org changed reviewers: + mef@chromium.org, pauljensen@chromium.org

4 years ago (2016-12-08 17:53:18 UTC) #3

kapishnikov

PTAL! If the change is okay, we need to add the new two APKs to ...

4 years ago (2016-12-08 17:53:19 UTC) #4

xunjieli

Thanks for cc-ing me. Code is self-explanatory, and I agree that a doc is not ...

4 years ago (2016-12-08 18:18:30 UTC) #5

kapishnikov

Description was changed from ========== Cronet smoke tests 1. Added test support classes that allow ...

4 years ago (2016-12-08 19:52:40 UTC) #6

Description was changed from

==========
Cronet smoke tests

1. Added test support classes that allow running the smoke tests in
   different environments against different servers. There is one
   support class for Java-only tests and one that requires H2 & QUIC.
   The concrete implementation of support classes is determined at
   runtime by reading the value of |TestSupportImplClass| from the
   Android string resource file.
2. Added two test APKs one for Java only deployment and one to test
   behavior when the native “.so” file is absent.
3. Enabled proguarding for the existing unit tests. The new APKS also
   run proguard in order to check the correctness of proguard configs
   for different deployments.
4. Added new HttpServer for Java-only scenario that does not depend on
   Chromium “base” and “net” modules.
5. Added two additional smoke QUIC & H2 tests to the existing test APK.


To run all smoke tests, execute:
ninja -C out/Debug cronet_java_only_smoke_tests_instrumentation_apk \
cronet_broken_so_file_smoke_tests_instrumentation_apk \
cronet_test_instrumentation_apk -j 240 && \
./out/Debug/bin/run_cronet_java_only_smoke_tests_instrumentation_apk && \
./out/Debug/bin/run_cronet_broken_so_file_smoke_tests_instrumentation_apk && \
./out/Debug/bin/run_cronet_test_instrumentation_apk \
--test-filter=org.chromium.net.smoke.*

The output jars that contain the smoke tests:
./out/Debug/lib.java/components/cronet/android/cronet_quic_smoke_test_java.jar
./out/Debug/lib.java/components/cronet/android/cronet_java_only_smoke_test_java.jar
./out/Debug/lib.java/components/cronet/android/cronet_broken_so_file_smoke_test_java.jar

BUG=669264
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester
==========

to

==========
Cronet smoke tests

1. Added test support classes that allow running the smoke tests in
   different environments against different servers. There is one
   support class for Java-only tests and one that requires H2 & QUIC.
   The concrete implementation of support classes is determined at
   runtime by reading the value of |TestSupportImplClass| from the
   Android string resource file.
2. Added two test APKs one for Java only deployment and one to test
   behavior when the native “.so” file is absent.
3. Enabled proguarding for the existing unit tests. The new APKS also
   run proguard in order to check the correctness of proguard configs
   for different deployments.
4. Added new HttpServer for Java-only scenario that does not depend on
   Chromium “base” and “net” modules.
5. Added two additional smoke QUIC & H2 tests to the existing test APK.


To run all smoke tests, execute:

ninja -C out/Debug cronet_java_only_smoke_tests_instrumentation_apk \
cronet_broken_native_library_smoke_tests_instrumentation_apk \
cronet_test_instrumentation_apk -j 240 && \
./out/Debug/bin/run_cronet_java_only_smoke_tests_instrumentation_apk && \
./out/Debug/bin/run_cronet_broken_native_library_smoke_tests_instrumentation_apk
&& \
./out/Debug/bin/run_cronet_test_instrumentation_apk \
--test-filter=org.chromium.net.smoke.*

The output jars that contain the smoke tests:
./out/Debug/lib.java/components/cronet/android/cronet_quic_smoke_test_java.jar
./out/Debug/lib.java/components/cronet/android/cronet_java_only_smoke_test_java.jar
./out/Debug/lib.java/components/cronet/android/cronet_broken_native_library_smoke_test_java.jar

BUG=669264
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester
==========

kapishnikov

Helen, thanks for the comments! I have addressed them or added an explanation. https://codereview.chromium.org/2561803002/diff/20001/components/cronet/android/test/smoketests/src/org/chromium/net/smoke/BrokenSoFileTest.java File ...

4 years ago (2016-12-08 19:56:16 UTC) #7

Helen, thanks for the comments! I have addressed them or added an explanation.

https://codereview.chromium.org/2561803002/diff/20001/components/cronet/andro...
File
components/cronet/android/test/smoketests/src/org/chromium/net/smoke/BrokenSoFileTest.java
(right):

https://codereview.chromium.org/2561803002/diff/20001/components/cronet/andro...
components/cronet/android/test/smoketests/src/org/chromium/net/smoke/BrokenSoFileTest.java:16:
public class BrokenSoFileTest extends CronetTestBase {
On 2016/12/08 18:18:29, xunjieli wrote:
> nit: s/So/NativeLibaray

Done, here and everywhere else.

https://codereview.chromium.org/2561803002/diff/20001/components/cronet/andro...
File
components/cronet/android/test/smoketests/src/org/chromium/net/smoke/ChromiumQuicTestSupport.java
(right):

https://codereview.chromium.org/2561803002/diff/20001/components/cronet/andro...
components/cronet/android/test/smoketests/src/org/chromium/net/smoke/ChromiumQuicTestSupport.java:26:
public TestServer createTestServer(Context context, Protocol protocol) {
On 2016/12/08 18:18:29, xunjieli wrote:
> This is identical to that of the parent. Can you omit this?

The parent one throws an exception when QUIC or H2 server is requested. That is
because the parent one cannot have any dependencies on "base" or " net".

https://codereview.chromium.org/2561803002/diff/20001/components/cronet/andro...
components/cronet/android/test/smoketests/src/org/chromium/net/smoke/ChromiumQuicTestSupport.java:40:
public void copyNetLog(File file) {}
On 2016/12/08 18:18:29, xunjieli wrote:
> Why does this do nothing? Could you add a TODO or add this method when it is
> needed?

I renamed the method to processNetLog() and improved TestSupport#processNetLog
documentation.

    /**
     * This method is called at the end of a test run if the netlog is
available. An implementer
     * of {@link TestSupport} can use it to process the result netlog; e.g., to
copy the netlog
     * to a directory where all test logs are collected. This method is optional
and can be no-op.
     *
     * @param file the netlog file.
     */
    void processNetLog(File file);

https://codereview.chromium.org/2561803002/diff/20001/components/cronet/andro...
File
components/cronet/android/test/smoketests/src/org/chromium/net/smoke/NativeCronetTestBase.java
(right):

https://codereview.chromium.org/2561803002/diff/20001/components/cronet/andro...
components/cronet/android/test/smoketests/src/org/chromium/net/smoke/NativeCronetTestBase.java:24:
PathUtils.setPrivateDataDirectorySuffix(PRIVATE_DATA_DIRECTORY_SUFFIX);
On 2016/12/08 18:18:29, xunjieli wrote:
> I don't understand the separation. NativeCronetTestBase.java seems redundant.
> Can you just move ContextUtil.initApplicationContext() and
> PathUtils.setPrivatDataDirectorySuffix() mTestSupport? The JavaOnly one can
have
> no-op passthroughs.

TestSupport contains only methods, which behavior can be different in different
testing environments. I think initing the application context and setting the
private directory should be the same regarding the environment; otherwise, the
implementer of TestSupport for a different environment needs to reimplement the
functionality of these utilities.

ContextUtils & PathUtils depend on Chromium 'base', so they cannot be called
from CronetTestBase since it is used in Java-only tests.

I have improved the documentation of TestSupport interface:
/**
 * Provides support for tests, so they can be run in different environments
against different
 * servers. It contains methods, which behavior can be different in different
testing environments.
 * The concrete implementation of this interface is determined dynamically at
runtime by reading
 * the value of |TestSupportImplClass| from the Android string resource file.
 */

https://codereview.chromium.org/2561803002/diff/20001/components/cronet/andro...
File
components/cronet/android/test/smoketests/src/org/chromium/net/smoke/QuicTest.java
(right):

https://codereview.chromium.org/2561803002/diff/20001/components/cronet/andro...
components/cronet/android/test/smoketests/src/org/chromium/net/smoke/QuicTest.java:27:
public void testHttp2() throws Exception {
On 2016/12/08 18:18:29, xunjieli wrote:
> Should "QuicTest" be renamed or move testHttp2() out to another file?

Done. Created a new H2Test class and move testHttp2() test there.

https://codereview.chromium.org/2561803002/diff/20001/components/cronet/andro...
components/cronet/android/test/smoketests/src/org/chromium/net/smoke/QuicTest.java:30:
try (TestSupport.TestServer server = mTestSupport.createTestServer(getContext(),
H2)) {
On 2016/12/08 18:18:29, xunjieli wrote:
> nit: suggest to do an explicit try and catch so as not to skip all exceptions.
> Could add an enum in mTestSupport to specify what type of TestSupport it is
and
> fail() if the expectation differs. E.g when it's ChromiumTestSupport and we
have
> an exception.

We do not want to catch exceptions. If there is an exception, the test should
fail.

Different environments may instantiate different TestSupport implementations, so
the implementation class is not hardcoded. I have updated the TestSupport
interface documentation.

https://codereview.chromium.org/2561803002/diff/20001/components/cronet/andro...
File
components/cronet/android/test/smoketests/src/org/chromium/net/smoke/TestSupport.java
(right):

https://codereview.chromium.org/2561803002/diff/20001/components/cronet/andro...
components/cronet/android/test/smoketests/src/org/chromium/net/smoke/TestSupport.java:34:
TestServer createTestServer(Context context, Protocol protocol);
On 2016/12/08 18:18:30, xunjieli wrote:
> Can this throw an explicit runtime exception like
UnsupportedOperationException?

Added it to javadoc.

https://codereview.chromium.org/2561803002/diff/20001/components/cronet/andro...
File
components/cronet/android/test/smoketests/src/org/chromium/net/smoke/TestUrlRequestCallback.java
(right):

https://codereview.chromium.org/2561803002/diff/20001/components/cronet/andro...
components/cronet/android/test/smoketests/src/org/chromium/net/smoke/TestUrlRequestCallback.java:31:
// private static final String TAG = "cr_TestUrlRequestCb";
On 2016/12/08 18:18:30, xunjieli wrote:
> nit: commented out code.

Done.

mef

Looks reasonable, although quite big. https://codereview.chromium.org/2561803002/diff/60001/components/cronet/android/BUILD.gn File components/cronet/android/BUILD.gn (right): https://codereview.chromium.org/2561803002/diff/60001/components/cronet/android/BUILD.gn#newcode592 components/cronet/android/BUILD.gn:592: java_only_smoke_test_common_srcs = [ naming ...

4 years ago (2016-12-08 23:17:18 UTC) #8

pauljensen

https://codereview.chromium.org/2561803002/diff/60001/components/cronet/android/test/smoketests/src/org/chromium/net/smoke/TestUrlRequestCallback.java File components/cronet/android/test/smoketests/src/org/chromium/net/smoke/TestUrlRequestCallback.java (right): https://codereview.chromium.org/2561803002/diff/60001/components/cronet/android/test/smoketests/src/org/chromium/net/smoke/TestUrlRequestCallback.java#newcode30 components/cronet/android/test/smoketests/src/org/chromium/net/smoke/TestUrlRequestCallback.java:30: class TestUrlRequestCallback extends UrlRequest.Callback { On 2016/12/08 23:17:18, mef ...

4 years ago (2016-12-15 04:15:49 UTC) #9

kapishnikov

Description was changed from ========== Cronet smoke tests 1. Added test support classes that allow ...

4 years ago (2016-12-19 19:10:21 UTC) #10

Description was changed from

==========
Cronet smoke tests

1. Added test support classes that allow running the smoke tests in
   different environments against different servers. There is one
   support class for Java-only tests and one that requires H2 & QUIC.
   The concrete implementation of support classes is determined at
   runtime by reading the value of |TestSupportImplClass| from the
   Android string resource file.
2. Added two test APKs one for Java only deployment and one to test
   behavior when the native “.so” file is absent.
3. Enabled proguarding for the existing unit tests. The new APKS also
   run proguard in order to check the correctness of proguard configs
   for different deployments.
4. Added new HttpServer for Java-only scenario that does not depend on
   Chromium “base” and “net” modules.
5. Added two additional smoke QUIC & H2 tests to the existing test APK.


To run all smoke tests, execute:

ninja -C out/Debug cronet_java_only_smoke_tests_instrumentation_apk \
cronet_broken_native_library_smoke_tests_instrumentation_apk \
cronet_test_instrumentation_apk -j 240 && \
./out/Debug/bin/run_cronet_java_only_smoke_tests_instrumentation_apk && \
./out/Debug/bin/run_cronet_broken_native_library_smoke_tests_instrumentation_apk
&& \
./out/Debug/bin/run_cronet_test_instrumentation_apk \
--test-filter=org.chromium.net.smoke.*

The output jars that contain the smoke tests:
./out/Debug/lib.java/components/cronet/android/cronet_quic_smoke_test_java.jar
./out/Debug/lib.java/components/cronet/android/cronet_java_only_smoke_test_java.jar
./out/Debug/lib.java/components/cronet/android/cronet_broken_native_library_smoke_test_java.jar

BUG=669264
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester
==========

to

==========
Cronet smoke tests

1. Added test support classes that allow running the smoke tests in
   different environments against different servers. There is one
   support class for Java-only tests and one that requires H2 & QUIC.
   The concrete implementation of support classes is determined at
   runtime by reading the value of |TestSupportImplClass| from the
   Android string resource file.
2. Added two test APKs one for Java only deployment and one to test
   behavior when the native “.so” file is absent.
3. Enabled proguarding for the existing unit tests. The new APKS also
   run proguard in order to check the correctness of proguard configs
   for different deployments.
4. Added new HttpServer for Java-only scenario that does not depend on
   Chromium “base” and “net” modules.
5. Added two additional smoke QUIC & H2 tests to the existing test APK.


To run all smoke tests, execute:

ninja -C out/Debug cronet_smoketests_java_only_instrumentation_apk \
cronet_smoketests_broken_native_library_instrumentation_apk \
cronet_test_instrumentation_apk -j 240 && \
./out/Debug/bin/run_cronet_smoketests_java_only_instrumentation_apk && \
./out/Debug/bin/run_cronet_smoketests_broken_native_library_instrumentation_apk
&& \
./out/Debug/bin/run_cronet_test_instrumentation_apk \
--test-filter=org.chromium.net.smoke.*

The output jars that contain the smoke tests:
./out/Debug/lib.java/components/cronet/android/cronet_smoketests_native_java.jar
./out/Debug/lib.java/components/cronet/android/cronet_smoketests_java_only_java.jar
./out/Debug/lib.java/components/cronet/android/cronet_smoketests_broken_native_library_java.jar


BUG=669264
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester
==========

kapishnikov

https://codereview.chromium.org/2561803002/diff/60001/components/cronet/android/BUILD.gn File components/cronet/android/BUILD.gn (right): https://codereview.chromium.org/2561803002/diff/60001/components/cronet/android/BUILD.gn#newcode592 components/cronet/android/BUILD.gn:592: java_only_smoke_test_common_srcs = [ On 2016/12/08 23:17:18, mef wrote: > ...

4 years ago (2016-12-19 19:31:11 UTC) #11

https://codereview.chromium.org/2561803002/diff/60001/components/cronet/andro...
File components/cronet/android/BUILD.gn (right):

https://codereview.chromium.org/2561803002/diff/60001/components/cronet/andro...
components/cronet/android/BUILD.gn:592: java_only_smoke_test_common_srcs = [
On 2016/12/08 23:17:18, mef wrote:
> naming nit: maybe start all those with cronet_smoketest like
> 'cronet_smoketest_java_only_common_srcs'?

Done.

https://codereview.chromium.org/2561803002/diff/60001/components/cronet/andro...
components/cronet/android/BUILD.gn:596:
"test/smoketests/src/org/chromium/net/smoke/CronetTestBase.java",
On 2016/12/08 23:17:18, mef wrote:
> nit: I'm not sure whether smoke_test or smoketests is better, but would like
> some consistency.

Renamed everywhere to smoketests.

https://codereview.chromium.org/2561803002/diff/60001/components/cronet/andro...
components/cronet/android/BUILD.gn:600: quic_smoke_test_common_srcs =
java_only_smoke_test_common_srcs + [
On 2016/12/08 23:17:18, mef wrote:
> maybe add java_only_smoke_test_common_srcs explicitly to java_files below?

quic_smoke_test_common_srcs depends on java_only_smoke_test_common_srcs, i.e. it
won't compile without it. Somebody may add quic_smoke_test_common_srcs but
forget java_only_smoke_test_common_srcs.

https://codereview.chromium.org/2561803002/diff/60001/components/cronet/andro...
components/cronet/android/BUILD.gn:616: "//third_party/netty4:netty_all_java",
On 2016/12/08 23:17:18, mef wrote:
> hrm, netty4 is for h2, why do we need it for quic?

netty is required to compile ChromiumNativeTestSupport.java

https://codereview.chromium.org/2561803002/diff/60001/components/cronet/andro...
components/cronet/android/BUILD.gn:766:
"//base/android/proguard/chromium_apk.flags",
On 2016/12/08 23:17:18, mef wrote:
> This feels wrong. Why would cronet test depend on chromium apk proguard flags?
> Should this be "$target_gen_dir/cronet_impl_native_proguard.cfg" instead?

Only the test APK (i.e. not the instrumentation APK) should include the proguard
configs that are included by the embedder apps (cronet_impl_native_proguard.cfg
in this particular case). The instrumentation APK should only include configs
that are needed for the testing purpose. In particular, we would like to
preserve Netty server classes, test support classes and the source file / line
number table for easier debugging.

I changed test/smoketests/proguard.cfg to keep SourceFile & LineNumberTable and
removed the dependency on chromium_apk.flags from the Java Only test. However,
the native tests failing without chromium_apk.flags. I will investigate the
reason.

https://codereview.chromium.org/2561803002/diff/60001/components/cronet/andro...
File components/cronet/android/test/smoketests/res/java_only/values/strings.xml
(right):

https://codereview.chromium.org/2561803002/diff/60001/components/cronet/andro...
components/cronet/android/test/smoketests/res/java_only/values/strings.xml:3:
<!-- Copyright 2014 The Chromium Authors. All rights reserved.
On 2016/12/08 23:17:18, mef wrote:
> 2016

Done.

https://codereview.chromium.org/2561803002/diff/60001/components/cronet/andro...
File components/cronet/android/test/smoketests/res/quic/values/strings.xml
(right):

https://codereview.chromium.org/2561803002/diff/60001/components/cronet/andro...
components/cronet/android/test/smoketests/res/quic/values/strings.xml:3: <!--
Copyright 2014 The Chromium Authors. All rights reserved.
On 2016/12/08 23:17:18, mef wrote:
> 2016

Done.

https://codereview.chromium.org/2561803002/diff/60001/components/cronet/andro...
File
components/cronet/android/test/smoketests/src/org/chromium/net/smoke/H2Test.java
(right):

https://codereview.chromium.org/2561803002/diff/60001/components/cronet/andro...
components/cronet/android/test/smoketests/src/org/chromium/net/smoke/H2Test.java:17:
@TargetApi(19)
On 2016/12/08 23:17:18, mef wrote:
> Could you add comment as to why @TargetApi(19) is needed?

It was needed for the 'try' block that auto-closes the test server. Added
setUp() and tearDown() methods instead here and in QuicTest.

https://codereview.chromium.org/2561803002/diff/60001/components/cronet/andro...
components/cronet/android/test/smoketests/src/org/chromium/net/smoke/H2Test.java:24:
try (TestSupport.TestServer server = mTestSupport.createTestServer(getContext(),
H2)) {
On 2016/12/08 23:17:18, mef wrote:
> Can it be TestSupport.Protocol.H2 here?

Done.

https://codereview.chromium.org/2561803002/diff/60001/components/cronet/andro...
File
components/cronet/android/test/smoketests/src/org/chromium/net/smoke/JavaOnlyEngineTest.java
(right):

https://codereview.chromium.org/2561803002/diff/60001/components/cronet/andro...
components/cronet/android/test/smoketests/src/org/chromium/net/smoke/JavaOnlyEngineTest.java:21:
mServer = mTestSupport.createTestServer(getContext(), TestSupport.Protocol.H1);
On 2016/12/08 23:17:18, mef wrote:
> Maybe add comment that java engine supports Http/1.1 only?

Done.

https://codereview.chromium.org/2561803002/diff/60001/components/cronet/andro...
File
components/cronet/android/test/smoketests/src/org/chromium/net/smoke/TestSupport.java
(right):

https://codereview.chromium.org/2561803002/diff/60001/components/cronet/andro...
components/cronet/android/test/smoketests/src/org/chromium/net/smoke/TestSupport.java:24:
H1,
On 2016/12/08 23:17:18, mef wrote:
> Maybe spell out HTTP1 and HTTP2?

Done. Renamed H2TestServer -> Http2TestServer

https://codereview.chromium.org/2561803002/diff/60001/components/cronet/andro...
File
components/cronet/android/test/smoketests/src/org/chromium/net/smoke/TestUrlRequestCallback.java
(right):

https://codereview.chromium.org/2561803002/diff/60001/components/cronet/andro...
components/cronet/android/test/smoketests/src/org/chromium/net/smoke/TestUrlRequestCallback.java:30:
class TestUrlRequestCallback extends UrlRequest.Callback {
On 2016/12/08 23:17:18, mef wrote:
> It is a bit annoying that we have to duplicate TestUrlRequestCallback (and to
> certain extent TestServer(s)) between instrumentation and smoke test. Can we
> factor them out into common 'TestSupport' package?

Agree. I will work on it.

https://codereview.chromium.org/2561803002/diff/60001/components/cronet/andro...
components/cronet/android/test/smoketests/src/org/chromium/net/smoke/TestUrlRequestCallback.java:30:
class TestUrlRequestCallback extends UrlRequest.Callback {
On 2016/12/08 23:17:18, mef wrote:
> It is a bit annoying that we have to duplicate TestUrlRequestCallback (and to
> certain extent TestServer(s)) between instrumentation and smoke test. Can we
> factor them out into common 'TestSupport' package?

I re-implemented TestUrlRequestCallback from scratch adding only functionality
that is needed for the smoke test. It is quite simple now.

kapishnikov

Uploaded a new PS. With the patch, all comments should be currently addressed. https://codereview.chromium.org/2561803002/diff/60001/components/cronet/android/BUILD.gn File ...

4 years ago (2016-12-19 23:33:41 UTC) #12

mef

looks reasonable https://codereview.chromium.org/2561803002/diff/120001/components/cronet/android/BUILD.gn File components/cronet/android/BUILD.gn (right): https://codereview.chromium.org/2561803002/diff/120001/components/cronet/android/BUILD.gn#newcode592 components/cronet/android/BUILD.gn:592: cronet_smoketests_java_only_common_srcs = [ nit: maybe s/java_only/platform_only/ here ...

4 years ago (2016-12-20 20:17:46 UTC) #13

kapishnikov

Description was changed from ========== Cronet smoke tests 1. Added test support classes that allow ...

4 years ago (2016-12-22 19:19:57 UTC) #14

Description was changed from

==========
Cronet smoke tests

1. Added test support classes that allow running the smoke tests in
   different environments against different servers. There is one
   support class for Java-only tests and one that requires H2 & QUIC.
   The concrete implementation of support classes is determined at
   runtime by reading the value of |TestSupportImplClass| from the
   Android string resource file.
2. Added two test APKs one for Java only deployment and one to test
   behavior when the native “.so” file is absent.
3. Enabled proguarding for the existing unit tests. The new APKS also
   run proguard in order to check the correctness of proguard configs
   for different deployments.
4. Added new HttpServer for Java-only scenario that does not depend on
   Chromium “base” and “net” modules.
5. Added two additional smoke QUIC & H2 tests to the existing test APK.


To run all smoke tests, execute:

ninja -C out/Debug cronet_smoketests_java_only_instrumentation_apk \
cronet_smoketests_broken_native_library_instrumentation_apk \
cronet_test_instrumentation_apk -j 240 && \
./out/Debug/bin/run_cronet_smoketests_java_only_instrumentation_apk && \
./out/Debug/bin/run_cronet_smoketests_broken_native_library_instrumentation_apk
&& \
./out/Debug/bin/run_cronet_test_instrumentation_apk \
--test-filter=org.chromium.net.smoke.*

The output jars that contain the smoke tests:
./out/Debug/lib.java/components/cronet/android/cronet_smoketests_native_java.jar
./out/Debug/lib.java/components/cronet/android/cronet_smoketests_java_only_java.jar
./out/Debug/lib.java/components/cronet/android/cronet_smoketests_broken_native_library_java.jar


BUG=669264
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester
==========

to

==========
Cronet smoke tests

1. Added test support classes that allow running the smoke tests in
   different environments against different servers. There is one
   support class for Java-only tests and one that requires H2 & QUIC.
   The concrete implementation of support classes is determined at
   runtime by reading the value of |TestSupportImplClass| from the
   Android string resource file.
2. Added two test APKs one for Java only deployment and one to test
   behavior when the native “.so” file is absent.
3. Enabled proguarding for the existing unit tests. The new APKS also
   run proguard in order to check the correctness of proguard configs
   for different deployments.
4. Added new HttpServer for Java-only scenario that does not depend on
   Chromium “base” and “net” modules.
5. Added two additional smoke QUIC & H2 tests to the existing test APK.


To run all smoke tests, execute:

ninja -C out/Debug cronet_smoketests_platform_only_instrumentation_apk \
cronet_smoketests_missing_native_library_instrumentation_apk \
cronet_test_instrumentation_apk -j 240 && \
./out/Debug/bin/run_cronet_smoketests_platform_only_instrumentation_apk && \
./out/Debug/bin/run_cronet_smoketests_missing_native_library_instrumentation_apk
&& \
./out/Debug/bin/run_cronet_test_instrumentation_apk \
--test-filter=org.chromium.net.smoke.*

The output jars that contain the smoke tests:
./out/Debug/lib.java/components/cronet/android/cronet_smoketests_missing_native_library_java.jar
./out/Debug/lib.java/components/cronet/android/cronet_smoketests_native_java.jar
./out/Debug/lib.java/components/cronet/android/cronet_smoketests_platform_only_java.jar

BUG=669264
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester
==========

kapishnikov

https://codereview.chromium.org/2561803002/diff/120001/components/cronet/android/BUILD.gn File components/cronet/android/BUILD.gn (right): https://codereview.chromium.org/2561803002/diff/120001/components/cronet/android/BUILD.gn#newcode592 components/cronet/android/BUILD.gn:592: cronet_smoketests_java_only_common_srcs = [ On 2016/12/20 20:17:45, mef wrote: > ...

4 years ago (2016-12-22 19:21:13 UTC) #15

mef

lgtm https://codereview.chromium.org/2561803002/diff/120001/components/cronet/android/test/smoketests/src/org/chromium/net/smoke/TestSupport.java File components/cronet/android/test/smoketests/src/org/chromium/net/smoke/TestSupport.java (right): https://codereview.chromium.org/2561803002/diff/120001/components/cronet/android/test/smoketests/src/org/chromium/net/smoke/TestSupport.java#newcode11 components/cronet/android/test/smoketests/src/org/chromium/net/smoke/TestSupport.java:11: import org.chromium.net.ExperimentalCronetEngine; On 2016/12/22 19:21:13, kapishnikov wrote: > ...

4 years ago (2016-12-22 21:35:29 UTC) #16

kapishnikov

https://codereview.chromium.org/2561803002/diff/160001/components/cronet/android/test/smoketests/src/org/chromium/net/smoke/ChromiumNativeTestSupport.java File components/cronet/android/test/smoketests/src/org/chromium/net/smoke/ChromiumNativeTestSupport.java (right): https://codereview.chromium.org/2561803002/diff/160001/components/cronet/android/test/smoketests/src/org/chromium/net/smoke/ChromiumNativeTestSupport.java#newcode36 components/cronet/android/test/smoketests/src/org/chromium/net/smoke/ChromiumNativeTestSupport.java:36: public void addHostResolverRules(JSONObject json) { On 2016/12/22 21:35:29, mef ...

4 years ago (2016-12-22 22:13:31 UTC) #17

https://codereview.chromium.org/2561803002/diff/160001/components/cronet/andr...
File
components/cronet/android/test/smoketests/src/org/chromium/net/smoke/ChromiumNativeTestSupport.java
(right):

https://codereview.chromium.org/2561803002/diff/160001/components/cronet/andr...
components/cronet/android/test/smoketests/src/org/chromium/net/smoke/ChromiumNativeTestSupport.java:36:
public void addHostResolverRules(JSONObject json) {
On 2016/12/22 21:35:29, mef (OOO 1223-0103) wrote:
> nit: maybe s/json/experimentalOptionsJson/ ?

Done.

https://codereview.chromium.org/2561803002/diff/160001/components/cronet/andr...
File
components/cronet/android/test/smoketests/src/org/chromium/net/smoke/NativeCronetTestCase.java
(right):

https://codereview.chromium.org/2561803002/diff/160001/components/cronet/andr...
components/cronet/android/test/smoketests/src/org/chromium/net/smoke/NativeCronetTestCase.java:43:
mCronetEngine.startNetLogToFile(
On 2016/12/22 21:35:29, mef (OOO 1223-0103) wrote:
> Just curious, why do we need netlog in smoke tests?
> I know it is nice, but is it useful?

I think it could be useful in some cases. E.g., if a test failed in a
non-Chromium test environment (i.e. when somebody provided a TestSupport
implementation that makes the tests run against a real QUIC/HTTP2 server), it
could be useful to see the netlogs to identify the problem.

The TestSupport class also provides processNetLog() method that gives access to
the netlog, e.g. it can be used to copy the log to the test result directory.

https://codereview.chromium.org/2561803002/diff/160001/components/cronet/andr...
File
components/cronet/android/test/smoketests/src/org/chromium/net/smoke/SmokeTestRequestCallback.java
(right):

https://codereview.chromium.org/2561803002/diff/160001/components/cronet/andr...
components/cronet/android/test/smoketests/src/org/chromium/net/smoke/SmokeTestRequestCallback.java:57:
mResponseInfo = info;
On 2016/12/22 21:35:29, mef (OOO 1223-0103) wrote:
> nit: don't need to preserve |info| until the final state (here and above).

Done.

https://codereview.chromium.org/2561803002/diff/160001/components/cronet/andr...
components/cronet/android/test/smoketests/src/org/chromium/net/smoke/SmokeTestRequestCallback.java:79:
assertTrue(mState == State.NotSet);
On 2016/12/22 21:35:29, mef (OOO 1223-0103) wrote:
> nit: maybe factor this out from Succeeded/Failed/Canceled into
done(finalState);

Good idea. Done.

kapishnikov

The CQ bit was checked by kapishnikov@chromium.org to run a CQ dry run

4 years ago (2016-12-22 22:14:02 UTC) #18

commit-bot: I haz the power

Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2561803002/180001

4 years ago (2016-12-22 22:14:23 UTC) #19

mef

lgtm https://codereview.chromium.org/2561803002/diff/160001/components/cronet/android/test/smoketests/src/org/chromium/net/smoke/NativeCronetTestCase.java File components/cronet/android/test/smoketests/src/org/chromium/net/smoke/NativeCronetTestCase.java (right): https://codereview.chromium.org/2561803002/diff/160001/components/cronet/android/test/smoketests/src/org/chromium/net/smoke/NativeCronetTestCase.java#newcode43 components/cronet/android/test/smoketests/src/org/chromium/net/smoke/NativeCronetTestCase.java:43: mCronetEngine.startNetLogToFile( On 2016/12/22 22:13:30, kapishnikov wrote: > On ...

4 years ago (2016-12-22 22:17:06 UTC) #20

kapishnikov

The CQ bit was checked by kapishnikov@chromium.org to run a CQ dry run

3 years, 12 months ago (2016-12-27 16:38:04 UTC) #21

commit-bot: I haz the power

Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2561803002/220001

3 years, 12 months ago (2016-12-27 16:38:16 UTC) #22

commit-bot: I haz the power

The CQ bit was unchecked by commit-bot@chromium.org

3 years, 12 months ago (2016-12-27 17:28:24 UTC) #23

commit-bot: I haz the power

Dry run: This issue passed the CQ dry run.

3 years, 12 months ago (2016-12-27 17:28:25 UTC) #24

kapishnikov

The patchset sent to the CQ was uploaded after l-g-t-m from mef@chromium.org Link to the ...

3 years, 12 months ago (2016-12-27 17:40:28 UTC) #26

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2561803002/220001

3 years, 12 months ago (2016-12-27 17:40:36 UTC) #27

commit-bot: I haz the power

CQ is committing da patch. Bot data: {"patchset_id": 220001, "attempt_start_ts": 1482860427863470, "parent_rev": "8f4eba5fb8ce6c6d1a01f347845e2993040ad738", "commit_rev": "0d1d257803ca2e38079a10adc004627d884fef10"}

3 years, 12 months ago (2016-12-27 17:44:51 UTC) #28

commit-bot: I haz the power

Description was changed from ========== Cronet smoke tests 1. Added test support classes that allow ...

3 years, 12 months ago (2016-12-27 17:45:22 UTC) #29

Message was sent while issue was closed.

Description was changed from

==========
Cronet smoke tests

1. Added test support classes that allow running the smoke tests in
   different environments against different servers. There is one
   support class for Java-only tests and one that requires H2 & QUIC.
   The concrete implementation of support classes is determined at
   runtime by reading the value of |TestSupportImplClass| from the
   Android string resource file.
2. Added two test APKs one for Java only deployment and one to test
   behavior when the native “.so” file is absent.
3. Enabled proguarding for the existing unit tests. The new APKS also
   run proguard in order to check the correctness of proguard configs
   for different deployments.
4. Added new HttpServer for Java-only scenario that does not depend on
   Chromium “base” and “net” modules.
5. Added two additional smoke QUIC & H2 tests to the existing test APK.


To run all smoke tests, execute:

ninja -C out/Debug cronet_smoketests_platform_only_instrumentation_apk \
cronet_smoketests_missing_native_library_instrumentation_apk \
cronet_test_instrumentation_apk -j 240 && \
./out/Debug/bin/run_cronet_smoketests_platform_only_instrumentation_apk && \
./out/Debug/bin/run_cronet_smoketests_missing_native_library_instrumentation_apk
&& \
./out/Debug/bin/run_cronet_test_instrumentation_apk \
--test-filter=org.chromium.net.smoke.*

The output jars that contain the smoke tests:
./out/Debug/lib.java/components/cronet/android/cronet_smoketests_missing_native_library_java.jar
./out/Debug/lib.java/components/cronet/android/cronet_smoketests_native_java.jar
./out/Debug/lib.java/components/cronet/android/cronet_smoketests_platform_only_java.jar

BUG=669264
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester
==========

to

==========
Cronet smoke tests

1. Added test support classes that allow running the smoke tests in
   different environments against different servers. There is one
   support class for Java-only tests and one that requires H2 & QUIC.
   The concrete implementation of support classes is determined at
   runtime by reading the value of |TestSupportImplClass| from the
   Android string resource file.
2. Added two test APKs one for Java only deployment and one to test
   behavior when the native “.so” file is absent.
3. Enabled proguarding for the existing unit tests. The new APKS also
   run proguard in order to check the correctness of proguard configs
   for different deployments.
4. Added new HttpServer for Java-only scenario that does not depend on
   Chromium “base” and “net” modules.
5. Added two additional smoke QUIC & H2 tests to the existing test APK.


To run all smoke tests, execute:

ninja -C out/Debug cronet_smoketests_platform_only_instrumentation_apk \
cronet_smoketests_missing_native_library_instrumentation_apk \
cronet_test_instrumentation_apk -j 240 && \
./out/Debug/bin/run_cronet_smoketests_platform_only_instrumentation_apk && \
./out/Debug/bin/run_cronet_smoketests_missing_native_library_instrumentation_apk
&& \
./out/Debug/bin/run_cronet_test_instrumentation_apk \
--test-filter=org.chromium.net.smoke.*

The output jars that contain the smoke tests:
./out/Debug/lib.java/components/cronet/android/cronet_smoketests_missing_native_library_java.jar
./out/Debug/lib.java/components/cronet/android/cronet_smoketests_native_java.jar
./out/Debug/lib.java/components/cronet/android/cronet_smoketests_platform_only_java.jar

BUG=669264
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester

Review-Url: https://codereview.chromium.org/2561803002
==========

commit-bot: I haz the power

Description was changed from ========== Cronet smoke tests 1. Added test support classes that allow ...

3 years, 11 months ago (2017-01-02 15:46:21 UTC) #31

Message was sent while issue was closed.

Description was changed from

==========
Cronet smoke tests

1. Added test support classes that allow running the smoke tests in
   different environments against different servers. There is one
   support class for Java-only tests and one that requires H2 & QUIC.
   The concrete implementation of support classes is determined at
   runtime by reading the value of |TestSupportImplClass| from the
   Android string resource file.
2. Added two test APKs one for Java only deployment and one to test
   behavior when the native “.so” file is absent.
3. Enabled proguarding for the existing unit tests. The new APKS also
   run proguard in order to check the correctness of proguard configs
   for different deployments.
4. Added new HttpServer for Java-only scenario that does not depend on
   Chromium “base” and “net” modules.
5. Added two additional smoke QUIC & H2 tests to the existing test APK.


To run all smoke tests, execute:

ninja -C out/Debug cronet_smoketests_platform_only_instrumentation_apk \
cronet_smoketests_missing_native_library_instrumentation_apk \
cronet_test_instrumentation_apk -j 240 && \
./out/Debug/bin/run_cronet_smoketests_platform_only_instrumentation_apk && \
./out/Debug/bin/run_cronet_smoketests_missing_native_library_instrumentation_apk
&& \
./out/Debug/bin/run_cronet_test_instrumentation_apk \
--test-filter=org.chromium.net.smoke.*

The output jars that contain the smoke tests:
./out/Debug/lib.java/components/cronet/android/cronet_smoketests_missing_native_library_java.jar
./out/Debug/lib.java/components/cronet/android/cronet_smoketests_native_java.jar
./out/Debug/lib.java/components/cronet/android/cronet_smoketests_platform_only_java.jar

BUG=669264
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester

Review-Url: https://codereview.chromium.org/2561803002
==========

to

==========
Cronet smoke tests

1. Added test support classes that allow running the smoke tests in
   different environments against different servers. There is one
   support class for Java-only tests and one that requires H2 & QUIC.
   The concrete implementation of support classes is determined at
   runtime by reading the value of |TestSupportImplClass| from the
   Android string resource file.
2. Added two test APKs one for Java only deployment and one to test
   behavior when the native “.so” file is absent.
3. Enabled proguarding for the existing unit tests. The new APKS also
   run proguard in order to check the correctness of proguard configs
   for different deployments.
4. Added new HttpServer for Java-only scenario that does not depend on
   Chromium “base” and “net” modules.
5. Added two additional smoke QUIC & H2 tests to the existing test APK.

To run all smoke tests, execute:

ninja -C out/Debug cronet_smoketests_platform_only_instrumentation_apk \
cronet_smoketests_missing_native_library_instrumentation_apk \
cronet_test_instrumentation_apk -j 240 && \
./out/Debug/bin/run_cronet_smoketests_platform_only_instrumentation_apk && \
./out/Debug/bin/run_cronet_smoketests_missing_native_library_instrumentation_apk
&& \
./out/Debug/bin/run_cronet_test_instrumentation_apk \
--test-filter=org.chromium.net.smoke.*

The output jars that contain the smoke tests:
./out/Debug/lib.java/components/cronet/android/cronet_smoketests_missing_native_library_java.jar
./out/Debug/lib.java/components/cronet/android/cronet_smoketests_native_java.jar
./out/Debug/lib.java/components/cronet/android/cronet_smoketests_platform_only_java.jar

BUG=669264
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester

Committed: https://crrev.com/7a4cba9f6d608b3347984e3aaa6f7dfd1d9f63b3
Cr-Commit-Position: refs/heads/master@{#440760}
==========

commit-bot: I haz the power

3 years, 11 months ago (2017-01-02 15:46:22 UTC) #32

Message was sent while issue was closed.

Patchset 12 (id:??) landed as
https://crrev.com/7a4cba9f6d608b3347984e3aaa6f7dfd1d9f63b3
Cr-Commit-Position: refs/heads/master@{#440760}

Issue 2561803002: Cronet smoke tests (Closed)

Description

Patch Set 1 #

Patch Set 2 : Rebase #

Patch Set 3 : Addressed Helen's comments #

Patch Set 4 : Renamed ChromiumQuicTestSupport to ChromiumNativeTestSupport #

Patch Set 5 : Addressed some of Misha's comments #

Patch Set 6 : Re-implemented TestUrlRequestCallback. Got rid of API(19) requirement. #

Patch Set 7 : Got rid of //base/android/proguard/chromium_apk.flags proguard config. #

Patch Set 8 : Addressed Misha's comments #

Patch Set 9 : Changed @Smoke to @SmallTest #

Patch Set 10 : Code cleaning #

Patch Set 11 : More cleaning #

Patch Set 12 : Moved to android.support.test.filters & added support for network-security-config #

Messages