Make cr_cronet.py test automatically run cr_cronet.py install as well.

Update cr_cronet.py to run multiple commands at once.

"test" and "debug" automatically run install, as they required running install before being usable anyways - running them with outdated versions installed did not work.  This makes the standard build then run workflow from other platforms work for cronet as well.

Also add "build-test" and "build-debug" commands, for added convenience.

BUG=none

Committed: https://crrev.com/57abf3fc95678673e074a584e1e3ec31c121cf3c
Cr-Commit-Position: refs/heads/master@{#304934}

[mmenke, 2014-11-19 20:29:08]

mmenke@chromium.org changed reviewers:
+ mef@chromium.org, xunjieli@chromium.org

[mmenke, 2014-11-19 20:29:09]

Every time I start working on cronet after not having had to build/run it for a
couple weeks, I end up spending more time than I care to admit figuring out that
not running install first is why tests are behaving badly.

I don't think there's any case where on will have a local version of
CronetTest.apk that's different from the one currently on their device, and not
want to use the local one - otherwise, CronetTestInstrumentation.apk won't match
CronetTest.apk.

[xunjieli, 2014-11-19 20:38:22]

On 2014/11/19 20:29:09, mmenke wrote:
> Every time I start working on cronet after not having had to build/run it for
a
> couple weeks, I end up spending more time than I care to admit figuring out
that
> not running install first is why tests are behaving badly.
> 
> I don't think there's any case where on will have a local version of
> CronetTest.apk that's different from the one currently on their device, and
not
> want to use the local one - otherwise, CronetTestInstrumentation.apk won't
match
> CronetTest.apk.

I am up for this change, but I wonder if there's a way to detect local working
directory has changed since last build. So our "test" arg can include "build"
before "install & test".

[mmenke, 2014-11-19 20:41:55]

On 2014/11/19 20:38:22, xunjieli wrote:
> On 2014/11/19 20:29:09, mmenke wrote:
> > Every time I start working on cronet after not having had to build/run it
for
> a
> > couple weeks, I end up spending more time than I care to admit figuring out
> that
> > not running install first is why tests are behaving badly.
> > 
> > I don't think there's any case where on will have a local version of
> > CronetTest.apk that's different from the one currently on their device, and
> not
> > want to use the local one - otherwise, CronetTestInstrumentation.apk won't
> match
> > CronetTest.apk.
> 
> I am up for this change, but I wonder if there's a way to detect local working
> directory has changed since last build. So our "test" arg can include "build"
> before "install & test".

I considered making test build, too, but decided not to break current behavior. 
If you're working on a change, may not have your code in a buildable state, for
example, but still want to test.  Could add a new method (build-and-test?) or a
--build argument or something.  I'm open to ideas.

[xunjieli, 2014-11-19 20:44:10]

I like the "build-and-test" idea, and keeping it separate from the normal
"build" and "test".

On Wed, Nov 19, 2014 at 3:41 PM, <mmenke@chromium.org> wrote:

> On 2014/11/19 20:38:22, xunjieli wrote:
>
>> On 2014/11/19 20:29:09, mmenke wrote:
>> > Every time I start working on cronet after not having had to build/run
>> it
>>
> for
>
>> a
>> > couple weeks, I end up spending more time than I care to admit figuring
>> out
>> that
>> > not running install first is why tests are behaving badly.
>> >
>> > I don't think there's any case where on will have a local version of
>> > CronetTest.apk that's different from the one currently on their device,
>> and
>> not
>> > want to use the local one - otherwise, CronetTestInstrumentation.apk
>> won't
>> match
>> > CronetTest.apk.
>>
>
>  I am up for this change, but I wonder if there's a way to detect local
>> working
>> directory has changed since last build. So our "test" arg can include
>> "build"
>> before "install & test".
>>
>
> I considered making test build, too, but decided not to break current
> behavior.
> If you're working on a change, may not have your code in a buildable
> state, for
> example, but still want to test.  Could add a new method (build-and-test?)
> or a
> --build argument or something.  I'm open to ideas.
>
> https://codereview.chromium.org/740783002/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[mef, 2014-11-19 20:52:04]

On 2014/11/19 20:44:10, xunjieli wrote:
> I like the "build-and-test" idea, and keeping it separate from the normal
> "build" and "test".
> 
> On Wed, Nov 19, 2014 at 3:41 PM, <mailto:mmenke@chromium.org> wrote:
> 
> > On 2014/11/19 20:38:22, xunjieli wrote:
> >
> >> On 2014/11/19 20:29:09, mmenke wrote:
> >> > Every time I start working on cronet after not having had to build/run
> >> it
> >>
> > for
> >
> >> a
> >> > couple weeks, I end up spending more time than I care to admit figuring
> >> out
> >> that
> >> > not running install first is why tests are behaving badly.
> >> >
> >> > I don't think there's any case where on will have a local version of
> >> > CronetTest.apk that's different from the one currently on their device,
> >> and
> >> not
> >> > want to use the local one - otherwise, CronetTestInstrumentation.apk
> >> won't
> >> match
> >> > CronetTest.apk.
> >>
> >
> >  I am up for this change, but I wonder if there's a way to detect local
> >> working
> >> directory has changed since last build. So our "test" arg can include
> >> "build"
> >> before "install & test".
> >>
> >
> > I considered making test build, too, but decided not to break current
> > behavior.
> > If you're working on a change, may not have your code in a buildable
> > state, for
> > example, but still want to test.  Could add a new method (build-and-test?)
> > or a
> > --build argument or something.  I'm open to ideas.
> >
> > https://codereview.chromium.org/740783002/
> >
> 
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

I agree, I'd prefer to add a compound option like 'run' or 'bit'
(build-install-test), 
so we have clear atomic option still available if needed.

[mmenke, 2014-11-19 20:54:04]

On 2014/11/19 20:52:04, mef wrote:
> On 2014/11/19 20:44:10, xunjieli wrote:
> > I like the "build-and-test" idea, and keeping it separate from the normal
> > "build" and "test".
> > 
> > On Wed, Nov 19, 2014 at 3:41 PM, <mailto:mmenke@chromium.org> wrote:
> > 
> > > On 2014/11/19 20:38:22, xunjieli wrote:
> > >
> > >> On 2014/11/19 20:29:09, mmenke wrote:
> > >> > Every time I start working on cronet after not having had to build/run
> > >> it
> > >>
> > > for
> > >
> > >> a
> > >> > couple weeks, I end up spending more time than I care to admit figuring
> > >> out
> > >> that
> > >> > not running install first is why tests are behaving badly.
> > >> >
> > >> > I don't think there's any case where on will have a local version of
> > >> > CronetTest.apk that's different from the one currently on their device,
> > >> and
> > >> not
> > >> > want to use the local one - otherwise, CronetTestInstrumentation.apk
> > >> won't
> > >> match
> > >> > CronetTest.apk.
> > >>
> > >
> > >  I am up for this change, but I wonder if there's a way to detect local
> > >> working
> > >> directory has changed since last build. So our "test" arg can include
> > >> "build"
> > >> before "install & test".
> > >>
> > >
> > > I considered making test build, too, but decided not to break current
> > > behavior.
> > > If you're working on a change, may not have your code in a buildable
> > > state, for
> > > example, but still want to test.  Could add a new method (build-and-test?)
> > > or a
> > > --build argument or something.  I'm open to ideas.
> > >
> > > https://codereview.chromium.org/740783002/
> > >
> > 
> > To unsubscribe from this group and stop receiving emails from it, send an
> email
> > to mailto:chromium-reviews+unsubscribe@chromium.org.
> 
> I agree, I'd prefer to add a compound option like 'run' or 'bit'
> (build-install-test), 
> so we have clear atomic option still available if needed.

I oppose having a test option without an install - every single time I've
switched back to working on cronet, I've forgotten the extra step, and wasted
time on it, and I don't know of any use case where test makes sense without
having the current version installed.

[mef, 2014-11-19 20:56:30]

On 2014/11/19 20:54:04, mmenke wrote:
> On 2014/11/19 20:52:04, mef wrote:
> > On 2014/11/19 20:44:10, xunjieli wrote:
> > > I like the "build-and-test" idea, and keeping it separate from the normal
> > > "build" and "test".
> > > 
> > > On Wed, Nov 19, 2014 at 3:41 PM, <mailto:mmenke@chromium.org> wrote:
> > > 
> > > > On 2014/11/19 20:38:22, xunjieli wrote:
> > > >
> > > >> On 2014/11/19 20:29:09, mmenke wrote:
> > > >> > Every time I start working on cronet after not having had to
build/run
> > > >> it
> > > >>
> > > > for
> > > >
> > > >> a
> > > >> > couple weeks, I end up spending more time than I care to admit
figuring
> > > >> out
> > > >> that
> > > >> > not running install first is why tests are behaving badly.
> > > >> >
> > > >> > I don't think there's any case where on will have a local version of
> > > >> > CronetTest.apk that's different from the one currently on their
device,
> > > >> and
> > > >> not
> > > >> > want to use the local one - otherwise, CronetTestInstrumentation.apk
> > > >> won't
> > > >> match
> > > >> > CronetTest.apk.
> > > >>
> > > >
> > > >  I am up for this change, but I wonder if there's a way to detect local
> > > >> working
> > > >> directory has changed since last build. So our "test" arg can include
> > > >> "build"
> > > >> before "install & test".
> > > >>
> > > >
> > > > I considered making test build, too, but decided not to break current
> > > > behavior.
> > > > If you're working on a change, may not have your code in a buildable
> > > > state, for
> > > > example, but still want to test.  Could add a new method
(build-and-test?)
> > > > or a
> > > > --build argument or something.  I'm open to ideas.
> > > >
> > > > https://codereview.chromium.org/740783002/
> > > >
> > > 
> > > To unsubscribe from this group and stop receiving emails from it, send an
> > email
> > > to mailto:chromium-reviews+unsubscribe@chromium.org.
> > 
> > I agree, I'd prefer to add a compound option like 'run' or 'bit'
> > (build-install-test), 
> > so we have clear atomic option still available if needed.
> 
> I oppose having a test option without an install - every single time I've
> switched back to working on cronet, I've forgotten the extra step, and wasted
> time on it, and I don't know of any use case where test makes sense without
> having the current version installed.

Fair enough, sgtm.

[mmenke, 2014-11-19 21:56:59]

I've gone ahead and added build-test and build-debug, and made debug install,
too - it is theoretically possible one would want to debug without install, I
suppose, but I assume that would mess up the local symbols, so probably not.

[mef, 2014-11-19 22:04:47]

lgtm

https://codereview.chromium.org/740783002/diff/20001/components/cronet/tools/...
File components/cronet/tools/cr_cronet.py (right):

https://codereview.chromium.org/740783002/diff/20001/components/cronet/tools/...
components/cronet/tools/cr_cronet.py:29: 
I think python guidelines suggest 2 blank lines between methods.

[mmenke, 2014-11-19 22:06:56]

THanks!

https://codereview.chromium.org/740783002/diff/20001/components/cronet/tools/...
File components/cronet/tools/cr_cronet.py (right):

https://codereview.chromium.org/740783002/diff/20001/components/cronet/tools/...
components/cronet/tools/cr_cronet.py:29: 
On 2014/11/19 22:04:47, mef wrote:
> I think python guidelines suggest 2 blank lines between methods.

Done.

[xunjieli, 2014-11-19 23:16:16]

On 2014/11/19 22:06:56, mmenke wrote:
> THanks!
> 
>
https://codereview.chromium.org/740783002/diff/20001/components/cronet/tools/...
> File components/cronet/tools/cr_cronet.py (right):
> 
>
https://codereview.chromium.org/740783002/diff/20001/components/cronet/tools/...
> components/cronet/tools/cr_cronet.py:29: 
> On 2014/11/19 22:04:47, mef wrote:
> > I think python guidelines suggest 2 blank lines between methods.
> 
> Done.

lgtm! thanks for doing this

[mmenke, 2014-11-19 23:22:02]

On 2014/11/19 23:16:16, xunjieli wrote:
> lgtm! thanks for doing this

No problem, thanks for the suggestions!

[mmenke, 2014-11-19 23:22:05]

The CQ bit was checked by mmenke@chromium.org

[commit-bot: I haz the power, 2014-11-19 23:22:55]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/740783002/40001

[commit-bot: I haz the power, 2014-11-20 00:04:43]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2014-11-20 00:05:22]

Patchset 3 (id:??) landed as
https://crrev.com/57abf3fc95678673e074a584e1e3ec31c121cf3c
Cr-Commit-Position: refs/heads/master@{#304934}