Add option to test_runner to specify custom build directory

Add option to test_runner to specify custom build directory

test_runner.py by default assumes that all unit tests apks
are built into CHROME_SRC_DIR/out. If we have custom build
directory other than out, test_runner.py fails to run the
unit tests. User should be able to run the tests from any
build directory

BUG=None

R=bulach,craigdh

Committed: https://crrev.com/3d57cba32d174aa08f18deee79a4f9e7b39b902a
Cr-Commit-Position: refs/heads/master@{#298841}

[r.kasibhatla, 2014-10-08 14:13:37]

PTAL!

[craigdh, 2014-10-08 15:44:55]

craigdh@chromium.org changed reviewers:
+ jbudorick@chromium.org
- bulach@chromium.org, craigdh@chromium.org

[craigdh, 2014-10-08 15:45:26]

On 2014/10/08 14:13:37, r.kasibhatla wrote:
> PTAL!

Re-assigned john as the reviewer.

[jbudorick, 2014-10-08 18:20:57]

Somewhat amusingly, I wrote this patch almost exactly in
https://codereview.chromium.org/144093008/

The comments in that review mostly involved a dislike of using environment
variables. While I agree that it's a bit of a wart in the implementation, I
don't think it's relevant to this review, as replacing our use of environment
variables in the scripts is a larger issue.

The one comment that I want to investigate before approving this is the one
about a possible impact on cr:
https://code.google.com/p/chromium/codesearch#chromium/src/tools/cr/cr/base/p...

https://codereview.chromium.org/639853002/diff/1/build/android/test_runner.py
File build/android/test_runner.py (right):

https://codereview.chromium.org/639853002/diff/1/build/android/test_runner.py...
build/android/test_runner.py:61: help=('Path to the directory under which build
files'
Nit: I'd probably phrase this as "Path to the directory in which which build
files are located."

Also, what does the parenthetical here mean?

[r.kasibhatla, 2014-10-09 04:10:32]

> Somewhat amusingly, I wrote this patch almost exactly in
> https://codereview.chromium.org/144093008/

Sorry I didn't notice your patch with exactly same changes.
It was something I noticed yesterday when running some unittests and figured I
could add the --build-directory option (which has been present in the blink
layout tests scripts from long time).

> The one comment that I want to investigate before approving this is the one
> about a possible impact on cr:
>
https://code.google.com/p/chromium/codesearch#chromium/src/tools/cr/cr/base/p...

What is this cr scripts? How can I run cr script to check impact when setting
CHROMIUM_OUT_DIR?

> https://codereview.chromium.org/639853002/diff/1/build/android/test_runner.py
> File build/android/test_runner.py (right):
>  
> Also, what does the parenthetical here mean?

I couldn't get the question. Where did you observe the phrase/word
parenthetical?

[r.kasibhatla, 2014-10-09 04:22:13]

> The one comment that I want to investigate before approving this is the one
> about a possible impact on cr:
>
https://code.google.com/p/chromium/codesearch#chromium/src/tools/cr/cr/base/p...

Alternatively, if we are concerned about setting the env value CHROMIUM_OUT_DIR
might impact cr bash functions, we can go ahead with something like:

In GetOutDirectory(build_type=None), we can do 2-step check:
1. Check for --build-directory. If present, give it the highest preference and
use it to build out dir path.
2. If not present, check for CHROMIUM_OUT_DIR. If present, use it to build out
dir path.
3. If neither present, use the default 'out' directory in building the path.

WDYT?

[r.kasibhatla, 2014-10-09 06:06:20]

> The one comment that I want to investigate before approving this is the one
> about a possible impact on cr:
>
https://code.google.com/p/chromium/codesearch#chromium/src/tools/cr/cr/base/p...

After checking the cr module (static code analysis), I think setting the environ
value of CHROMIUM_OUT_DIR is safe. We are handling the environ variable
BUILDTYPE also in similar fashion.

Setting explicitly in test_runner.py:
https://code.google.com/p/chromium/codesearch#chromium/src/build/android/pyli...
But we are also referring in cr module:
https://code.google.com/p/chromium/codesearch#chromium/src/tools/cr/cr/base/b...

Let me know your thoughts on it.

[jbudorick, 2014-10-09 09:11:10]

I think you're right about this being safe with cr.

My CL was basically dead in the water anyway.

https://codereview.chromium.org/639853002/diff/20001/build/android/test_runne...
File build/android/test_runner.py (right):

https://codereview.chromium.org/639853002/diff/20001/build/android/test_runne...
build/android/test_runner.py:62: ' located (should not include configuration)'))
"(should not include configuration)" is the parenthetical I was referring to.
I'm not sure what you mean by that.

[r.kasibhatla, 2014-10-09 09:15:14]

On 2014/10/09 09:11:10, jbudorick wrote:
> I think you're right about this being safe with cr.
> 
> My CL was basically dead in the water anyway.
> 
>
https://codereview.chromium.org/639853002/diff/20001/build/android/test_runne...
> File build/android/test_runner.py (right):
> 
>
https://codereview.chromium.org/639853002/diff/20001/build/android/test_runne...
> build/android/test_runner.py:62: ' located (should not include
configuration)'))
> "(should not include configuration)" is the parenthetical I was referring to.
> I'm not sure what you mean by that.

I just copy-pasted --build-directory descrioption from blink layout tests
scripts. :)
It meant the value for --build-directory should be only 'out_kphanee' and not
'out_kphanee/Release'.

I can remove it if you want.

[jbudorick, 2014-10-09 09:19:05]

On 2014/10/09 09:15:14, r.kasibhatla wrote:
> On 2014/10/09 09:11:10, jbudorick wrote:
> > I think you're right about this being safe with cr.
> > 
> > My CL was basically dead in the water anyway.
> > 
> >
>
https://codereview.chromium.org/639853002/diff/20001/build/android/test_runne...
> > File build/android/test_runner.py (right):
> > 
> >
>
https://codereview.chromium.org/639853002/diff/20001/build/android/test_runne...
> > build/android/test_runner.py:62: ' located (should not include
> configuration)'))
> > "(should not include configuration)" is the parenthetical I was referring
to.
> > I'm not sure what you mean by that.
> 
> I just copy-pasted --build-directory descrioption from blink layout tests
> scripts. :)

Ha, good answer.

> It meant the value for --build-directory should be only 'out_kphanee' and not
> 'out_kphanee/Release'.
> 
> I can remove it if you want.

Maybe change "configuration" to "build type", since that's how this script
refers to that elsewhere?

Beyond that, this is fine.

[jbudorick, 2014-10-09 09:19:58]

lgtm w/ aforementioned minor nit

[jbudorick, 2014-10-09 09:20:20]

On 2014/10/09 09:19:58, jbudorick wrote:
> lgtm w/ aforementioned minor nit

Oh, and thanks for the patch!

[r.kasibhatla, 2014-10-09 09:24:27]

The CQ bit was checked by r.kasibhatla@samsung.com

[commit-bot: I haz the power, 2014-10-09 09:26:10]

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

[r.kasibhatla, 2014-10-09 09:27:15]

On 2014/10/09 09:20:20, jbudorick wrote:
> On 2014/10/09 09:19:58, jbudorick wrote:
> > lgtm w/ aforementioned minor nit
> 
> Oh, and thanks for the patch!

Thanks for the review. :)

[commit-bot: I haz the power, 2014-10-09 10:56:07]

Committed patchset #3 (id:40001) as 66fa965cd556355d0e07b54edf46d5726d404108

[commit-bot: I haz the power, 2014-10-09 10:57:04]

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