[WIP] Adding Mockito support in Android instrumentation tests

[WIP] Adding Mockito support in Android instrumentation tests

This CL upgrades Mockito to 2.7.20 to support instrumentation tests.
Dependencies are also upgraded/added.

Design doc:
https://docs.google.com/a/google.com/document/d/1Vla1McDsX3AXDrvOcIqt4HemFcbWxozaoOaYIed4RUk/edit?usp=sharing

BUG=638318

[Zhiqiang Zhang (Slow), 2017-04-04 18:41:45]

zqzhang@chromium.org changed reviewers:
+ mikecase@chromium.org

[Zhiqiang Zhang (Slow), 2017-04-04 18:41:46]

PTAL this initial patch.

I need to add/update the LICENSE and README files

[Zhiqiang Zhang (Slow), 2017-04-04 18:41:50]

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

[commit-bot: I haz the power, 2017-04-04 18:43:19]

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

[mikecase (-- gone --), 2017-04-04 19:24:13]

You may be asked to split out the things you are adding to third_party/ into
individual CLs. Other than that, this looks good :D

[commit-bot: I haz the power, 2017-04-04 19:51:41]

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

[commit-bot: I haz the power, 2017-04-04 19:51:42]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Zhiqiang Zhang (Slow), 2017-04-04 20:56:37]

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

[commit-bot: I haz the power, 2017-04-04 20:57:27]

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

[Zhiqiang Zhang (Slow), 2017-04-04 21:15:53]

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

[commit-bot: I haz the power, 2017-04-04 21:16:40]

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

[commit-bot: I haz the power, 2017-04-05 02:50:33]

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

[commit-bot: I haz the power, 2017-04-05 02:50:33]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Zhiqiang Zhang (Slow), 2017-04-05 15:00:25]

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

[commit-bot: I haz the power, 2017-04-05 15:00:59]

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

[Zhiqiang Zhang (Slow), 2017-04-05 15:04:07]

Description was changed from

==========
[WIP] Adding Mockito support in Android instrumentation tests

BUG=638318
==========

to

==========
[WIP] Adding Mockito support in Android instrumentation tests

Design doc:
https://docs.google.com/a/google.com/document/d/1Vla1McDsX3AXDrvOcIqt4HemFcbW...

BUG=638318
==========

[Zhiqiang Zhang (Slow), 2017-04-05 15:05:24]

Description was changed from

==========
[WIP] Adding Mockito support in Android instrumentation tests

Design doc:
https://docs.google.com/a/google.com/document/d/1Vla1McDsX3AXDrvOcIqt4HemFcbW...

BUG=638318
==========

to

==========
[WIP] Adding Mockito support in Android instrumentation tests

This CL upgrades Mockito to 2.7.20 to support instrumentation tests.
Dependencies are also upgraded/added.

Design doc:
https://docs.google.com/a/google.com/document/d/1Vla1McDsX3AXDrvOcIqt4HemFcbW...

BUG=638318
==========

[commit-bot: I haz the power, 2017-04-05 15:45:00]

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

[commit-bot: I haz the power, 2017-04-05 15:45:01]

Dry run: Try jobs failed on following builders:
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)

[Zhiqiang Zhang (Slow), 2017-04-05 15:47:25]

zqzhang@chromium.org changed reviewers:
+ agrieve@chromium.org

[Zhiqiang Zhang (Slow), 2017-04-05 15:47:25]

+agrieve@ for a proguard/dex issue on release builds.

The problem is:
I need to keep some classes in the new libraries from removed/obfuscated by
proguard. The consequence is that the dex list is too large that I have enable
multidex for chrome_public_test_apk (added enable_multidex in
chrome/android/BUILD.gn).

Then I got another issue saying "com.android.dex.DexException: Too many classes
in --main-dex-list, main dex capacity exceeded". I checked the main_dex_list and
seems everything is included in it.

Did I do anything wrong?

[Zhiqiang Zhang (Slow), 2017-04-05 16:11:01]

On 2017/04/05 at 15:47:25, Zhiqiang Zhang wrote:
> +agrieve@ for a proguard/dex issue on release builds.
> 
> The problem is:
> I need to keep some classes in the new libraries from removed/obfuscated by
proguard. The consequence is that the dex list is too large that I have enable
multidex for chrome_public_test_apk (added enable_multidex in
chrome/android/BUILD.gn).
> 
> Then I got another issue saying "com.android.dex.DexException: Too many
classes in --main-dex-list, main dex capacity exceeded". I checked the
main_dex_list and seems everything is included in it.
> 
> Did I do anything wrong?

And surprisingly, disabling multidex in debug builds (i.e. proguard off) can
pass.

[Zhiqiang Zhang (Slow), 2017-04-05 16:11:11]

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

[commit-bot: I haz the power, 2017-04-05 16:11:39]

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

[Zhiqiang Zhang (Slow), 2017-04-05 17:30:15]

On 2017/04/05 at 16:11:01, Zhiqiang Zhang wrote:
> On 2017/04/05 at 15:47:25, Zhiqiang Zhang wrote:
> > +agrieve@ for a proguard/dex issue on release builds.
> > 
> > The problem is:
> > I need to keep some classes in the new libraries from removed/obfuscated by
proguard. The consequence is that the dex list is too large that I have enable
multidex for chrome_public_test_apk (added enable_multidex in
chrome/android/BUILD.gn).
> > 
> > Then I got another issue saying "com.android.dex.DexException: Too many
classes in --main-dex-list, main dex capacity exceeded". I checked the
main_dex_list and seems everything is included in it.
> > 
> > Did I do anything wrong?
> 
> And surprisingly, disabling multidex in debug builds (i.e. proguard off) can
pass.

To summarize and clarify:

* Proguard removes some classes however the library look up for them by name, so
we need to keep these classes
* For debug build, everything is fine
* For release build, after adding "-noshrink" proguard flags, I got dex errors
saying exceeding 64K methods (it's weird that I didn't see it in debug builds)
* After enabling multidex for chrome_public_test_apk (either on debug/release
builds), I got error saying the main dex list is too large.

[commit-bot: I haz the power, 2017-04-05 17:33:55]

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

[commit-bot: I haz the power, 2017-04-05 17:33:55]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[agrieve, 2017-04-05 19:43:51]

On 2017/04/05 17:30:15, Zhiqiang Zhang wrote:
> On 2017/04/05 at 16:11:01, Zhiqiang Zhang wrote:
> > On 2017/04/05 at 15:47:25, Zhiqiang Zhang wrote:
> > > +agrieve@ for a proguard/dex issue on release builds.
> > > 
> > > The problem is:
> > > I need to keep some classes in the new libraries from removed/obfuscated
by
> proguard. The consequence is that the dex list is too large that I have enable
> multidex for chrome_public_test_apk (added enable_multidex in
> chrome/android/BUILD.gn).
> > > 
> > > Then I got another issue saying "com.android.dex.DexException: Too many
> classes in --main-dex-list, main dex capacity exceeded". I checked the
> main_dex_list and seems everything is included in it.
> > > 
> > > Did I do anything wrong?
> > 
> > And surprisingly, disabling multidex in debug builds (i.e. proguard off) can
> pass.
> 
> To summarize and clarify:
> 
> * Proguard removes some classes however the library look up for them by name,
so
> we need to keep these classes
I think it would be much better to just add some -keep rules to disable
shrinking on only the classes that are being mocked in this way. Would that make
this all work? Do you have an example of where proguard is removing something
that is needed by the tests?

> * For debug build, everything is fine
> * For release build, after adding "-noshrink" proguard flags, I got dex errors
> saying exceeding 64K methods (it's weird that I didn't see it in debug builds)
> * After enabling multidex for chrome_public_test_apk (either on debug/release
> builds), I got error saying the main dex list is too large.

[Zhiqiang Zhang (Slow), 2017-04-05 22:03:07]

> I think it would be much better to just add some -keep rules to disable
shrinking on only the classes that are being mocked in this way. Would that make
this all work? Do you have an example of where proguard is removing something
that is needed by the tests?

Yes, there are a number of classes that need to be kept, both in org.mockito.*
and net.bytebuddy.*
The issue is that there is a lot of reflection usages in bytebuddy, so I have to
add classes to the list one after another. It is also not easy to find the
needed class as the printed error is not obvious. I'm not sure if I can find a
proper -keep list in the end.

Even if I succeeded, when we add more instrumentation tests, we might discover
more classes that should be kept and have to add them then.

So my preference is to disable shrinking totally for bytebuddy.

[agrieve, 2017-04-06 00:03:13]

On 2017/04/05 22:03:07, Zhiqiang Zhang wrote:
> > I think it would be much better to just add some -keep rules to disable
> shrinking on only the classes that are being mocked in this way. Would that
make
> this all work? Do you have an example of where proguard is removing something
> that is needed by the tests?
> 
> Yes, there are a number of classes that need to be kept, both in org.mockito.*
> and net.bytebuddy.*
> The issue is that there is a lot of reflection usages in bytebuddy, so I have
to
> add classes to the list one after another. It is also not easy to find the
> needed class as the printed error is not obvious. I'm not sure if I can find a
> proper -keep list in the end.
> 
> Even if I succeeded, when we add more instrumentation tests, we might discover
> more classes that should be kept and have to add them then.
> 
> So my preference is to disable shrinking totally for bytebuddy.

Totally happy to have wildcard -keep rules for these third-party libraries. I
had thought you meant you wanted to add "-dontshrink", which would disable
shrinking for *all* classes.

[agrieve, 2017-04-06 00:04:44]

https://codereview.chromium.org/2790063006/diff/120001/third_party/mockito/pr...
File third_party/mockito/proguard.flags (right):

https://codereview.chromium.org/2790063006/diff/120001/third_party/mockito/pr...
third_party/mockito/proguard.flags:3: -dontshrink
Ah, okay, this is what you've done.

These will apply globally. I don't think it's what you want to do. You should
scope these down so they apply only to mockito.

[Zhiqiang Zhang (Slow), 2017-04-06 14:02:43]

Interesting..
I'm checking the number of methods when is_java_debug is true and false before
adding mockito (i.e. on my trunk).

When is_java_debug is false, the number of methods is 64049, while it is 38273
when is_java_debug is false. It surprises me that release builds has more
methods.

Filed https://crbug.com/709007 for tracking the number of methods issue.

It's very easy to exceed to 64K methods limit when adding mockito/bytebuddy,
which needs adds at least 2K methods.

We should probably fix why release builds has more methods first.

Also, I tried using multidex but it seems that it doesn't work very well with
chrome_public_test_apk. It always complain about "too many classes in
--main-dex-list, main dex capacity exceeded"

https://codereview.chromium.org/2790063006/diff/120001/third_party/mockito/pr...
File third_party/mockito/proguard.flags (right):

https://codereview.chromium.org/2790063006/diff/120001/third_party/mockito/pr...
third_party/mockito/proguard.flags:3: -dontshrink
On 2017/04/06 at 00:04:44, agrieve wrote:
> Ah, okay, this is what you've done.
> 
> These will apply globally. I don't think it's what you want to do. You should
scope these down so they apply only to mockito.

OK, I realize that the proguard rule is the combination of all rule files, not
scoped to individual targets.