Add unit instrumentation tests for Chromoting Android code.

Add unit instrumentation tests for Chromoting Android code.

This adds a new GYP target for building Java unittests for Chromoting,
including a simple unittest example.

BUG=322095
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=242060

[frankf, 2013-12-18 00:10:03]

https://codereview.chromium.org/115953003/diff/20001/remoting/android/javates...
File remoting/android/javatests/AndroidManifest.xml (right):

https://codereview.chromium.org/115953003/diff/20001/remoting/android/javates...
remoting/android/javatests/AndroidManifest.xml:17: <uses-sdk
android:minSdkVersion="14" android:targetSdkVersion="18" />
I see android:targetSdkVersion for chromoting app is 14? Why? You should
probably match it here as well.

https://codereview.chromium.org/115953003/diff/20001/remoting/android/javates...
File
remoting/android/javatests/src/org/chromium/chromoting/SwipePinchDetectorTest.java
(right):

https://codereview.chromium.org/115953003/diff/20001/remoting/android/javates...
remoting/android/javatests/src/org/chromium/chromoting/SwipePinchDetectorTest.java:41:
MotionEvent event = MotionEvent.obtain(eventTime, eventTime,
There are utilities for MotionEvent to reduce boiler plate. See TouchCommon.java
If there's something missing, please add it there.

[Lambros, 2013-12-19 02:09:18]

https://codereview.chromium.org/115953003/diff/20001/remoting/android/javates...
File remoting/android/javatests/AndroidManifest.xml (right):

https://codereview.chromium.org/115953003/diff/20001/remoting/android/javates...
remoting/android/javatests/AndroidManifest.xml:17: <uses-sdk
android:minSdkVersion="14" android:targetSdkVersion="18" />
On 2013/12/18 00:10:03, frankf wrote:
> I see android:targetSdkVersion for chromoting app is 14? Why? You should
> probably match it here as well.

Yeah, that's strange. I didn't pick those numbers originally and I don't know
what they mean. For now, I've updated this value to match the app.

I'd be happy to change these numbers for Chromoting if you like, in a separate
CL. If a device isn't new enough to run Chrome, I don't think we care about
supporting it for Chromoting. So I think we can match what Chrome uses.

https://codereview.chromium.org/115953003/diff/20001/remoting/android/javates...
File
remoting/android/javatests/src/org/chromium/chromoting/SwipePinchDetectorTest.java
(right):

https://codereview.chromium.org/115953003/diff/20001/remoting/android/javates...
remoting/android/javatests/src/org/chromium/chromoting/SwipePinchDetectorTest.java:41:
MotionEvent event = MotionEvent.obtain(eventTime, eventTime,
On 2013/12/18 00:10:03, frankf wrote:
> There are utilities for MotionEvent to reduce boiler plate. See
TouchCommon.java
> If there's something missing, please add it there.

The main issue is that TouchCommon.java is under content/ and I don't think we
want remoting/ to depend on anything in there. We don't embed or use Chrome or
ContentShell in any way.

This specific test needs to generate two-finger MotionEvents, and I didn't see
anything in TouchCommon.java or elsewhere that does that. However, we will soon
want to write other tests that could benefit from those existing utilities. So
maybe the proper solution is to separate out the generic utility code and put it
somewhere like ui/ or base/ ?

[frankf, 2013-12-19 02:21:38]

LGTM. You might want to add a remoting OWNER as a reviewer.

https://codereview.chromium.org/115953003/diff/20001/remoting/android/javates...
File remoting/android/javatests/AndroidManifest.xml (right):

https://codereview.chromium.org/115953003/diff/20001/remoting/android/javates...
remoting/android/javatests/AndroidManifest.xml:17: <uses-sdk
android:minSdkVersion="14" android:targetSdkVersion="18" />
I'm not familiar at at with remoting. You have to look at what APIs you're using
and set those accordingly. 

On 2013/12/19 02:09:18, Lambros wrote:
> On 2013/12/18 00:10:03, frankf wrote:
> > I see android:targetSdkVersion for chromoting app is 14? Why? You should
> > probably match it here as well.
> 
> Yeah, that's strange. I didn't pick those numbers originally and I don't know
> what they mean. For now, I've updated this value to match the app.
> 
> I'd be happy to change these numbers for Chromoting if you like, in a separate
> CL. If a device isn't new enough to run Chrome, I don't think we care about
> supporting it for Chromoting. So I think we can match what Chrome uses.
>

https://codereview.chromium.org/115953003/diff/20001/remoting/android/javates...
File
remoting/android/javatests/src/org/chromium/chromoting/SwipePinchDetectorTest.java
(right):

https://codereview.chromium.org/115953003/diff/20001/remoting/android/javates...
remoting/android/javatests/src/org/chromium/chromoting/SwipePinchDetectorTest.java:41:
MotionEvent event = MotionEvent.obtain(eventTime, eventTime,
Yea, org.chromium.base.test.util might be a good place for this.



On 2013/12/19 02:09:18, Lambros wrote:
> On 2013/12/18 00:10:03, frankf wrote:
> > There are utilities for MotionEvent to reduce boiler plate. See
> TouchCommon.java
> > If there's something missing, please add it there.
> 
> The main issue is that TouchCommon.java is under content/ and I don't think we
> want remoting/ to depend on anything in there. We don't embed or use Chrome or
> ContentShell in any way.
> 
> This specific test needs to generate two-finger MotionEvents, and I didn't see
> anything in TouchCommon.java or elsewhere that does that. However, we will
soon
> want to write other tests that could benefit from those existing utilities. So
> maybe the proper solution is to separate out the generic utility code and put
it
> somewhere like ui/ or base/ ?

[Lambros, 2013-12-19 18:39:46]

+sergeyu for OWNERS, please.

[Sergey Ulanov, 2013-12-19 19:45:26]

lgtm

https://codereview.chromium.org/115953003/diff/40001/remoting/android/javates...
File
remoting/android/javatests/src/org/chromium/chromoting/SwipePinchDetectorTest.java
(right):

https://codereview.chromium.org/115953003/diff/40001/remoting/android/javates...
remoting/android/javatests/src/org/chromium/chromoting/SwipePinchDetectorTest.java:5:
package org.chromium.chromoting;
Not related to this CL: I think we should rename it to org.chromium.remoting,
for consistency with src/remoting

https://codereview.chromium.org/115953003/diff/40001/remoting/android/javates...
remoting/android/javatests/src/org/chromium/chromoting/SwipePinchDetectorTest.java:32:
@Feature({"Chromoting"})
RemotingClientUI?

https://codereview.chromium.org/115953003/diff/40001/remoting/android/javates...
remoting/android/javatests/src/org/chromium/chromoting/SwipePinchDetectorTest.java:38:
p0.y = 0;
nit: x and y should be initialized to 0 by default anyway. Don't need these two
lines.

[Lambros, 2013-12-20 02:52:01]

https://codereview.chromium.org/115953003/diff/40001/remoting/android/javates...
File
remoting/android/javatests/src/org/chromium/chromoting/SwipePinchDetectorTest.java
(right):

https://codereview.chromium.org/115953003/diff/40001/remoting/android/javates...
remoting/android/javatests/src/org/chromium/chromoting/SwipePinchDetectorTest.java:32:
@Feature({"Chromoting"})
On 2013/12/19 19:45:27, Sergey Ulanov wrote:
> RemotingClientUI?
Guidance here: http://www.chromium.org/developers/testing/android-tests
suggests feature names should be based on crbug labels.

https://codereview.chromium.org/115953003/diff/40001/remoting/android/javates...
remoting/android/javatests/src/org/chromium/chromoting/SwipePinchDetectorTest.java:38:
p0.y = 0;
On 2013/12/19 19:45:27, Sergey Ulanov wrote:
> nit: x and y should be initialized to 0 by default anyway. Don't need these
two
> lines.

Done. I've kept the p1.y = 0 because it looks really strange without it.

[commit-bot: I haz the power, 2013-12-20 02:54:56]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/lambroslambrou@chromium.org/115953003/...

[commit-bot: I haz the power, 2013-12-20 10:32:26]

Change committed as 242060