Add Makefile targets for running tests on Android.

Add Makefile targets for running tests on Android.

This allows to run tests on the attached Android device using
> make android.check
> make android.debug.check
> make android.release.check
> ANDROID_V8=/data/local/v8 TESTJOBS=-j4 make android.release.check -j10

Tests and binaries are copied to device location specified by the ANDROID_V8
variable and then tests are executed using the 'adb shell' program.

R=jkummerow@chromium.org

Committed: https://code.google.com/p/v8/source/detail?r=11975

[ulan, 2012-06-29 14:30:02]

Please take a look.

[Jakob Kummerow, 2012-07-02 14:25:58]

LGTM with a bunch of comments, mostly nits.

I'm not happy with "android" being an "arch" in the status files, and
essentially aliasing "arm". We'll have support for ia32-Android in the
not-too-distant future, and then that approach probably won't work anymore.
Unfortunately I don't have a good idea how to do it instead. Maybe we should
treat Android as an OS? But that doesn't map well to the existing target syntax
in the Makefile...

https://chromiumcodereview.appspot.com/10696048/diff/3001/Makefile
File Makefile (right):

https://chromiumcodereview.appspot.com/10696048/diff/3001/Makefile#newcode111
Makefile:111: # - "android": cross-compile for Android/ARM (release mode)
Please update this line.

https://chromiumcodereview.appspot.com/10696048/diff/3001/Makefile#newcode204
Makefile:204: @tools/android-sync.sh $(basename $@) $(OUTDIR) \
Does this line start with a <TAB>? Makefile recipes have to! (Same question
below.)

https://chromiumcodereview.appspot.com/10696048/diff/3001/tools/android-run.py
File tools/android-run.py (right):

https://chromiumcodereview.appspot.com/10696048/diff/3001/tools/android-run.p...
tools/android-run.py:40: import string
unused import

https://chromiumcodereview.appspot.com/10696048/diff/3001/tools/android-run.p...
tools/android-run.py:54: args = cmdline,
nit: styleguide says no spaces around = for keyword args

https://chromiumcodereview.appspot.com/10696048/diff/3001/tools/android-run.p...
tools/android-run.py:68: exit_code = exit_code if exit_code != 0 else
Check(output, errors)
shorter:

exit_code = exit_code or Check(output, errors)

https://chromiumcodereview.appspot.com/10696048/diff/3001/tools/android-run.p...
tools/android-run.py:95: script = (" ".join(args) + "\n" +
nit: you don't need '+' at the end of a line to get implicit string
concatenation. The following works just fine:
foo = ("bar"
       "baz")

https://chromiumcodereview.appspot.com/10696048/diff/3001/tools/android-run.p...
tools/android-run.py:102: command =  ("adb push '%s' %s;" +
reformat:

command =  ("adb push '%s' %s;" % (script_file, android_script_file) +
            "adb shell 'sh %s';" % android_script_file +
            "adb shell 'rm %s'" % android_script_file)

https://chromiumcodereview.appspot.com/10696048/diff/3001/tools/android-sync.sh
File tools/android-sync.sh (right):

https://chromiumcodereview.appspot.com/10696048/diff/3001/tools/android-sync....
tools/android-sync.sh:38: ANDROID_V8=$4
How about a check that all arguments are present, and otherwise print usage
instructions and exit?

To save you the trouble of looking it up, here's a snippet:
if [ ${#@} -lt 4 ] ; then
  echo "less than 4 arguments"
fi

https://chromiumcodereview.appspot.com/10696048/diff/3001/tools/android-sync....
tools/android-sync.sh:42: local ANDROID_HASH=`adb shell "md5 $ANDROID_V8/$FILE"`
Please use $(...) instead of `...`

https://chromiumcodereview.appspot.com/10696048/diff/3001/tools/android-sync....
tools/android-sync.sh:43: local HOST_HASH=`md5sum $HOST_V8/$FILE`
again $(...)

And if you don't want this to fail horribly in case $HOST_V8 ever contains
spaces, please put quotes around all variables containing user-defined paths.

https://chromiumcodereview.appspot.com/10696048/diff/3001/tools/android-sync....
tools/android-sync.sh:45: then
nit: you can put the "then" on the "if"'s line (like we do for '{') by inserting
a ';' between them:

if [ condition ] ; then

https://chromiumcodereview.appspot.com/10696048/diff/3001/tools/android-sync....
tools/android-sync.sh:53: for FILE in `find $HOST_V8/$DIR -type f`
again $(...)

https://chromiumcodereview.appspot.com/10696048/diff/3001/tools/android-sync....
tools/android-sync.sh:54: do
nit: again, for consistency with '{', I suggest removing the line break:

for foo ; do

https://chromiumcodereview.appspot.com/10696048/diff/3001/tools/android-sync....
tools/android-sync.sh:57: echo -n "."
How about moving this line into sync_file and removing it everywhere else?

https://chromiumcodereview.appspot.com/10696048/diff/3001/tools/android-sync....
tools/android-sync.sh:69: sync_file $OUTDIR/$ARCH_MODE/process
I don't think the "process" binary is required for anything.

https://chromiumcodereview.appspot.com/10696048/diff/3001/tools/android-sync....
tools/android-sync.sh:71: sync_file $OUTDIR/$ARCH_MODE/shell
"shell" is not required.

https://chromiumcodereview.appspot.com/10696048/diff/3001/tools/android-sync....
tools/android-sync.sh:86: sync_dir test/stress
I don't think test/stress is part of a regular V8 checkout.

[ulan, 2012-07-02 16:18:23]

Thank you for the comments. I uploaded new patch set.

Could you please take another look at android-sync.sh?

> We'll have support for ia32-Android in the not-too-distant future, and then
that approach probably won't work anymore.

As discussed offline, let's land "android" as target until we have to support
ia32-android.

https://chromiumcodereview.appspot.com/10696048/diff/3001/Makefile
File Makefile (right):

https://chromiumcodereview.appspot.com/10696048/diff/3001/Makefile#newcode111
Makefile:111: # - "android": cross-compile for Android/ARM (release mode)
On 2012/07/02 14:25:58, Jakob wrote:
> Please update this line.

Done.

https://chromiumcodereview.appspot.com/10696048/diff/3001/Makefile#newcode204
Makefile:204: @tools/android-sync.sh $(basename $@) $(OUTDIR) \
On 2012/07/02 14:25:58, Jakob wrote:
> Does this line start with a <TAB>? Makefile recipes have to! (Same question
> below.)
Done. Line 205 didn't have TAB, all other lines have.

https://chromiumcodereview.appspot.com/10696048/diff/3001/tools/android-run.py
File tools/android-run.py (right):

https://chromiumcodereview.appspot.com/10696048/diff/3001/tools/android-run.p...
tools/android-run.py:40: import string
On 2012/07/02 14:25:58, Jakob wrote:
> unused import

Done.

https://chromiumcodereview.appspot.com/10696048/diff/3001/tools/android-run.p...
tools/android-run.py:54: args = cmdline,
On 2012/07/02 14:25:58, Jakob wrote:
> nit: styleguide says no spaces around = for keyword args

Done.

https://chromiumcodereview.appspot.com/10696048/diff/3001/tools/android-run.p...
tools/android-run.py:68: exit_code = exit_code if exit_code != 0 else
Check(output, errors)
On 2012/07/02 14:25:58, Jakob wrote:
> shorter:
> 
> exit_code = exit_code or Check(output, errors)

Done.

https://chromiumcodereview.appspot.com/10696048/diff/3001/tools/android-run.p...
tools/android-run.py:95: script = (" ".join(args) + "\n" +
On 2012/07/02 14:25:58, Jakob wrote:
> nit: you don't need '+' at the end of a line to get implicit string
> concatenation. The following works just fine:
> foo = ("bar"
>        "baz")

Done.

https://chromiumcodereview.appspot.com/10696048/diff/3001/tools/android-run.p...
tools/android-run.py:102: command =  ("adb push '%s' %s;" +
On 2012/07/02 14:25:58, Jakob wrote:
> reformat:
> 
> command =  ("adb push '%s' %s;" % (script_file, android_script_file) +
>             "adb shell 'sh %s';" % android_script_file +
>             "adb shell 'rm %s'" % android_script_file)

Done.

https://chromiumcodereview.appspot.com/10696048/diff/3001/tools/android-sync.sh
File tools/android-sync.sh (right):

https://chromiumcodereview.appspot.com/10696048/diff/3001/tools/android-sync....
tools/android-sync.sh:38: ANDROID_V8=$4
On 2012/07/02 14:25:58, Jakob wrote:
> How about a check that all arguments are present, and otherwise print usage
> instructions and exit?
> 
> To save you the trouble of looking it up, here's a snippet:
> if [ ${#@} -lt 4 ] ; then
>   echo "less than 4 arguments"
> fi

Done.

https://chromiumcodereview.appspot.com/10696048/diff/3001/tools/android-sync....
tools/android-sync.sh:42: local ANDROID_HASH=`adb shell "md5 $ANDROID_V8/$FILE"`
On 2012/07/02 14:25:58, Jakob wrote:
> Please use $(...) instead of `...`

Done.

https://chromiumcodereview.appspot.com/10696048/diff/3001/tools/android-sync....
tools/android-sync.sh:43: local HOST_HASH=`md5sum $HOST_V8/$FILE`
On 2012/07/02 14:25:58, Jakob wrote:
> again $(...)
> 
> And if you don't want this to fail horribly in case $HOST_V8 ever contains
> spaces, please put quotes around all variables containing user-defined paths.

Done.

https://chromiumcodereview.appspot.com/10696048/diff/3001/tools/android-sync....
tools/android-sync.sh:45: then
On 2012/07/02 14:25:58, Jakob wrote:
> nit: you can put the "then" on the "if"'s line (like we do for '{') by
inserting
> a ';' between them:
> 
> if [ condition ] ; then

Done.

https://chromiumcodereview.appspot.com/10696048/diff/3001/tools/android-sync....
tools/android-sync.sh:53: for FILE in `find $HOST_V8/$DIR -type f`
On 2012/07/02 14:25:58, Jakob wrote:
> again $(...)

Done.

https://chromiumcodereview.appspot.com/10696048/diff/3001/tools/android-sync....
tools/android-sync.sh:54: do
On 2012/07/02 14:25:58, Jakob wrote:
> nit: again, for consistency with '{', I suggest removing the line break:
> 
> for foo ; do

Done.

https://chromiumcodereview.appspot.com/10696048/diff/3001/tools/android-sync....
tools/android-sync.sh:57: echo -n "."
On 2012/07/02 14:25:58, Jakob wrote:
> How about moving this line into sync_file and removing it everywhere else?

Done.

https://chromiumcodereview.appspot.com/10696048/diff/3001/tools/android-sync....
tools/android-sync.sh:69: sync_file $OUTDIR/$ARCH_MODE/process
On 2012/07/02 14:25:58, Jakob wrote:
> I don't think the "process" binary is required for anything.

Done.

https://chromiumcodereview.appspot.com/10696048/diff/3001/tools/android-sync....
tools/android-sync.sh:71: sync_file $OUTDIR/$ARCH_MODE/shell
On 2012/07/02 14:25:58, Jakob wrote:
> "shell" is not required.

Done.

https://chromiumcodereview.appspot.com/10696048/diff/3001/tools/android-sync....
tools/android-sync.sh:86: sync_dir test/stress
On 2012/07/02 14:25:58, Jakob wrote:
> I don't think test/stress is part of a regular V8 checkout.

Done.

[Jakob Kummerow, 2012-07-02 16:27:49]

LGTM

https://chromiumcodereview.appspot.com/10696048/diff/8001/tools/android-sync.sh
File tools/android-sync.sh (right):

https://chromiumcodereview.appspot.com/10696048/diff/8001/tools/android-sync....
tools/android-sync.sh:59: local RELATIVE_FILE=${FILE:${#HOST_V8}}
quotes please

https://chromiumcodereview.appspot.com/10696048/diff/8001/tools/android-sync....
tools/android-sync.sh:60: sync_file $RELATIVE_FILE
quotes please

https://chromiumcodereview.appspot.com/10696048/diff/8001/tools/android-sync....
tools/android-sync.sh:66: sync_file $OUTDIR/$ARCH_MODE/cctest
quotes please (twice more below)

[ulan, 2012-07-02 16:38:24]

Added quotes, landing.

https://chromiumcodereview.appspot.com/10696048/diff/8001/tools/android-sync.sh
File tools/android-sync.sh (right):

https://chromiumcodereview.appspot.com/10696048/diff/8001/tools/android-sync....
tools/android-sync.sh:59: local RELATIVE_FILE=${FILE:${#HOST_V8}}
On 2012/07/02 16:27:50, Jakob wrote:
> quotes please
Added quotes below.

https://chromiumcodereview.appspot.com/10696048/diff/8001/tools/android-sync....
tools/android-sync.sh:60: sync_file $RELATIVE_FILE
On 2012/07/02 16:27:50, Jakob wrote:
> quotes please

Done.

https://chromiumcodereview.appspot.com/10696048/diff/8001/tools/android-sync....
tools/android-sync.sh:66: sync_file $OUTDIR/$ARCH_MODE/cctest
On 2012/07/02 16:27:50, Jakob wrote:
> quotes please (twice more below)

Done.