Merge into trunk : support-monkeyrunner : Code : LAVA Android Test

Status:	Superseded
Proposed branch:	lp://qastaging/~liuyq0307/lava-android-test/support-monkeyrunner
Merge into:	lp://qastaging/lava-android-test
Diff against target:	428 lines (+278/-10) 7 files modified lava_android_test/commands.py (+165/-3) lava_android_test/main.py (+3/-4) lava_android_test/repository.py (+83/-0) lava_android_test/test_definitions/tjbench.py (+1/-1) lava_android_test/test_definitions/v8.py (+0/-1) lava_android_test/utils.py (+25/-1) setup.py (+1/-0)
To merge this branch:	bzr merge lp://qastaging/~liuyq0307/lava-android-test/support-monkeyrunner
Related bugs:	Link a bug report
Related blueprints:	add support for monkeyrunner (High)

Reviewer	Review Type	Date Requested	Status
Linaro Validation Team		2012-04-25	Pending
Review via email: mp+103452@code.qastaging.launchpad.net

This proposal has been superseded by a proposal from 2012-05-23.

Description of the change

add run-monkeyrunner sub command to make support monkeyrunner test easily.

Revision history for this message

Zygmunt Krynicki (zyga) wrote on 2012-04-25:

#

It's pretty hard to review this as I'm not that fluent in the android parts of lava.

287 === added file 'lava_android_test/repository.py'

We already have lava-vcs which supports git and bzr through single API. Perhaps it would be better to use that instead?

Revision history for this message

Yongqin Liu (liuyq0307) wrote on 2012-04-25:

#

On 25 April 2012 21:17, Zygmunt Krynicki <email address hidden>wrote:

> It's pretty hard to review this as I'm not that fluent in the android
> parts of lava.
>
> 287 === added file 'lava_android_test/repository.py'
>
> We already have lava-vcs which supports git and bzr through single API.
> Perhaps it would be better to use that instead?
>
This component is great.

Can it be used now?
And just one look, it has many files, seems a little complicate.
(Maybe it is easy to be used)

I'd like to change to use it after we merged it to lava-core

Revision history for this message

Zygmunt Krynicki (zyga) wrote on 2012-04-25:

#

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

W dniu 25.04.2012 16:08, Yongqin Liu pisze:
> On 25 April 2012 21:17, Zygmunt Krynicki
> <email address hidden>wrote:
>
>> It's pretty hard to review this as I'm not that fluent in the
>> android parts of lava.
>>
>> 287 === added file 'lava_android_test/repository.py'
>>
>> We already have lava-vcs which supports git and bzr through
>> single API. Perhaps it would be better to use that instead?
>>
> This component is great.
>
> Can it be used now?

Yes

> And just one look, it has many files, seems a little complicate.
> (Maybe it is easy to be used)

It's pretty easy. The abstraction has three objects: IVCS (like git
and bzr), IRemoteBranch which has limited functionality but allows you
to create a local copy / branch / clone and ILocalBranch that gives
you extra stuff that I needed to work on lava. If you need more then
we can easily extend it.

PS: look at lava/vcs/interfaces.py - this is what you can do on both
git and bzr.

from lava.vcs.adapters import get_vcs
git = get_vcs("git")
remote_branch = git.open_remote("git://...")
local_branch = remote_branch.sprout_to_pathname("some-place")
print local_branch.get_revision_id()

> I'd like to change to use it after we merged it to lava-core

Ok

Thanks
ZK

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQIcBAEBAgAGBQJPmDD6AAoJEOvx/vtfL8ZXadQP/0ga6sQg+vojuR6+ZSxzD78t
JtPKaRfWPlKrK7FazNjZLVXl/M390bxR/fWbHP9VMCsf9ZgXXL2t8whA0wk4sxnB
tHeaGjqgc21S2XdwGMbQ/XHI9cdSKvwvzpdOTD0ClqcjCBCOhVzQcylZLGAXapiW
TQo6ZrEvHBJwI62soSxk4xTO1ry5POYy8ZoVreYlhrPuYYE0SH7wnLj/Q/hvodmk
MWV5wfxgsGjeUTykKtNSZ7GQV1bWo8Cq60BG8xrUIlI2QEtcahFL2LsvKU/kV3PG
3Ul3QYj+zNv/TYMgA2qkyK7xYG0Rd0Gk8vxMEfId2nndcXTgo8MCue/vn8eU2Q5d
XdLHN+KzwDT6tY2QnxwYIfYIejUyWIPxBIXVI1DTO7xiJyjNpT/WESugC2MHZdGx
0k6KFXEYiGpze9INgnFp55aanfCDWUd+TY6vW4I754CtTIxOpvmMRIzxghVNjNLT
PNglV3y3nLy22N7+XBf0yKl2n9mh0f8te4DYNtjckKi1/lUyY8F4rDkPdhBkYj/J
b4zfGuw1E50fZ1nc+VDGFD+MS7Odj1Kmu5aRTppRobuOFVlsOSqvd1mkZ0t/WMj9
5cq78RBcXboFQF9beZJowrK6pI+koHzM8n55sicdySUSPhh24KtXHgo2T2TBajyA
3rI4BWQy+2v9tuxRFSw8
=xea1
-----END PGP SIGNATURE-----

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

W dniu 25.04.2012 16:08, Yongqin Liu pisze:
> On 25 April 2012 21:17, Zygmunt Krynicki
> <zygmunt.krynicki@canonical.com>wrote:
> 
>> It's pretty hard to review this as I'm not that fluent in the
>> android parts of lava.
>> 
>> 287     === added file 'lava_android_test/repository.py'
>> 
>> We already have lava-vcs which supports git and bzr through
>> single API. Perhaps it would be better to use that instead?
>> 
> This component is great.
> 
> Can it be used now?

Yes

> And just one look, it has many files, seems a little complicate. 
> (Maybe it is easy to be used)

It's pretty easy. The abstraction has three objects: IVCS (like git
and bzr), IRemoteBranch which has limited functionality but allows you
to create a local copy / branch / clone and ILocalBranch that gives
you extra stuff that I needed to work on lava. If you need more then
we can easily extend it.

PS: look at lava/vcs/interfaces.py - this is what you can do on both
git and bzr.

from lava.vcs.adapters import get_vcs
git = get_vcs("git")
remote_branch = git.open_remote("git://...")
local_branch = remote_branch.sprout_to_pathname("some-place")
print local_branch.get_revision_id()

> I'd like to change to use it after we merged it to lava-core

Ok

Thanks
ZK

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQIcBAEBAgAGBQJPmDD6AAoJEOvx/vtfL8ZXadQP/0ga6sQg+vojuR6+ZSxzD78t
JtPKaRfWPlKrK7FazNjZLVXl/M390bxR/fWbHP9VMCsf9ZgXXL2t8whA0wk4sxnB
tHeaGjqgc21S2XdwGMbQ/XHI9cdSKvwvzpdOTD0ClqcjCBCOhVzQcylZLGAXapiW
TQo6ZrEvHBJwI62soSxk4xTO1ry5POYy8ZoVreYlhrPuYYE0SH7wnLj/Q/hvodmk
MWV5wfxgsGjeUTykKtNSZ7GQV1bWo8Cq60BG8xrUIlI2QEtcahFL2LsvKU/kV3PG
3Ul3QYj+zNv/TYMgA2qkyK7xYG0Rd0Gk8vxMEfId2nndcXTgo8MCue/vn8eU2Q5d
XdLHN+KzwDT6tY2QnxwYIfYIejUyWIPxBIXVI1DTO7xiJyjNpT/WESugC2MHZdGx
0k6KFXEYiGpze9INgnFp55aanfCDWUd+TY6vW4I754CtTIxOpvmMRIzxghVNjNLT
PNglV3y3nLy22N7+XBf0yKl2n9mh0f8te4DYNtjckKi1/lUyY8F4rDkPdhBkYj/J
b4zfGuw1E50fZ1nc+VDGFD+MS7Odj1Kmu5aRTppRobuOFVlsOSqvd1mkZ0t/WMj9
5cq78RBcXboFQF9beZJowrK6pI+koHzM8n55sicdySUSPhh24KtXHgo2T2TBajyA
3rI4BWQy+2v9tuxRFSw8
=xea1
-----END PGP SIGNATURE-----

Revision history for this message

Michael Hudson-Doyle (mwhudson) wrote on 2012-05-21:

#

Download full text (12.6 KiB)

Yongqin Liu <email address hidden> writes:

> Yongqin Liu has proposed merging lp:~liuyq0307/lava-android-test/support-monkeyrunner into lp:lava-android-test.
>
> Requested reviews:
> Linaro Validation Team (linaro-validation)
>
> For more details, see:
> https://code.launchpad.net/~liuyq0307/lava-android-test/support-monkeyrunner/+merge/103452
>
> add run-monkeyrunner sub command to make support monkeyrunner test easily.

It has to be said, this whole approach feels like a way for the android
team to get out of the chore of having to define their tests
properly... but if that's what they want, they can have it I guess.

I have quite a lot of Python coding issues, please see inline comments.

> === modified file 'lava_android_test/commands.py'
> --- lava_android_test/commands.py 2012-04-11 05:58:13 +0000
> +++ lava_android_test/commands.py 2012-04-25 12:02:22 +0000
> @@ -21,16 +21,20 @@
> import urlparse
> import versiontools
>
> +from tempfile import mkdtemp
> +
> from lava_tool.interface import Command as LAVACommand
> from lava_tool.interface import LavaCommandError
> from linaro_dashboard_bundle.io import DocumentIO
>
> from lava_android_test.adb import ADB
> from lava_android_test.config import get_config
> +from lava_android_test.repository import GitRepository
> from lava_android_test.testdef import testloader, AndroidTest
> from lava_android_test.testdef import AndroidTestRunner, \
> AndroidTestInstaller, \
> AndroidTestParser
> +from lava_android_test import utils
>
>
> class Command(LAVACommand):
> @@ -146,6 +150,17 @@
> test_dir = os.path.join(self.config.installdir_android, test_id)
> return self.adb.exists(test_dir)
>
> + def get_device_serial(self):
> + if not self.args.serial:
> + serial_ary = ADB().run_cmd_host('adb get-serialno')[1]
> + serial = serial_ary[0].strip()
> + if not serial or serial == 'unknown':
> + return ''
> + else:
> + return serial
> + else:
> + return self.args.serial
> +
>
> class AndroidTestCommand(AndroidCommand):
> @classmethod
> @@ -435,6 +450,136 @@
> self.say_end(tip_msg)
>
>
> +class run_monkeyrunner(AndroidCommand):
> + """
> + Run the monkeyrunner scripts that stored in the specified git repository

> + program:: lava-android-test run-monkeyrunner -g giturl -r resultfilelist
> + """
> +
> + @classmethod
> + def register_arguments(cls, parser):
> + super(run_monkeyrunner, cls).register_arguments(parser)
> + parser.add_argument("url",
> + help="The repository url of the test scripts")
> + parser.add_argument('-t', '--repo-type',
> + default='git',
> + help=("Specify the type of the repository"))
> + group = parser.add_argument_group("specify the bundle output file")
> + group.add_argument("-o", "--output",
> + default=None,
> + metavar="FILE",
> + he...

Yongqin Liu <yongqin.liu@linaro.org> writes:

> Yongqin Liu has proposed merging lp:~liuyq0307/lava-android-test/support-monkeyrunner into lp:lava-android-test.
>
> Requested reviews:
>   Linaro Validation Team (linaro-validation)
>
> For more details, see:
> https://code.launchpad.net/~liuyq0307/lava-android-test/support-monkeyrunner/+merge/103452
>
> add run-monkeyrunner sub command to make support monkeyrunner test easily.

It has to be said, this whole approach feels like a way for the android
team to get out of the chore of having to define their tests
properly... but if that's what they want, they can have it I guess.

I have quite a lot of Python coding issues, please see inline comments.

> === modified file 'lava_android_test/commands.py'
> --- lava_android_test/commands.py	2012-04-11 05:58:13 +0000
> +++ lava_android_test/commands.py	2012-04-25 12:02:22 +0000
> @@ -21,16 +21,20 @@
>  import urlparse
>  import versiontools
>  
> +from tempfile import mkdtemp
> +
>  from lava_tool.interface import Command as LAVACommand
>  from lava_tool.interface import LavaCommandError
>  from linaro_dashboard_bundle.io import DocumentIO
>  
>  from lava_android_test.adb import ADB
>  from lava_android_test.config import get_config
> +from lava_android_test.repository import GitRepository
>  from lava_android_test.testdef import testloader, AndroidTest
>  from lava_android_test.testdef import AndroidTestRunner, \
>                                        AndroidTestInstaller, \
>                                        AndroidTestParser
> +from lava_android_test import utils
>  
>  
>  class Command(LAVACommand):
> @@ -146,6 +150,17 @@
>          test_dir = os.path.join(self.config.installdir_android, test_id)
>          return self.adb.exists(test_dir)
>  
> +    def get_device_serial(self):
> +        if not self.args.serial:
> +            serial_ary = ADB().run_cmd_host('adb get-serialno')[1]
> +            serial = serial_ary[0].strip()
> +            if not serial or serial == 'unknown':
> +                return ''
> +            else:
> +                return serial
> +        else:
> +            return self.args.serial
> +
>  
>  class AndroidTestCommand(AndroidCommand):
>      @classmethod
> @@ -435,6 +450,136 @@
>          self.say_end(tip_msg)
>  
>  
> +class run_monkeyrunner(AndroidCommand):
> +    """
> +    Run the monkeyrunner scripts that stored in the specified git repository
> +    program:: lava-android-test run-monkeyrunner -g giturl -r resultfilelist
> +    """
> +
> +    @classmethod
> +    def register_arguments(cls, parser):
> +        super(run_monkeyrunner, cls).register_arguments(parser)
> +        parser.add_argument("url",
> +                            help="The repository url of the test scripts")
> +        parser.add_argument('-t', '--repo-type',
> +                            default='git',
> +                            help=("Specify the type of the repository"))
> +        group = parser.add_argument_group("specify the bundle output file")
> +        group.add_argument("-o", "--output",
> +                            default=None,
> +                            metavar="FILE",
> +                            help=("After running the test parse the result"
> +                                 " artefacts, fuse them with the initial"
> +                                 " bundle and finally save the complete bundle"
> +                                 " to the  specified FILE."))
> +
> +    def invoke(self):
> +        config = get_config()
> +        if self.args.serial:
> +            serial = self.args.serial
> +        else:
> +            serial = self.get_device_serial()
> +            if not serial:
> +                raise LavaCommandError("No device attached")
> +        self.adb = ADB(serial)
> +
> +        if self.args.repo_type == 'git':
> +            target_dir = mkdtemp(prefix='git_repo', dir=config.tempdir_host)
> +            os.chmod(target_dir, 0755)
> +            GitRepository(self.args.url).checkout(target_dir)

What happens if repo_type is not git?  It should error I guess.

> +        script_list = utils.find_files(target_dir, '.py')
> +
> +        test_id = self.args.url
> +        if len(test_id) > 40:
> +            test_id = '%s...' % (test_id[:40])
> +        test_id = 'monkeyrunner_%s' % test_id
> +
> +        tip_msg = ("Run monkeyrunner scripts in following url on device(%s):"
> +                       "\n\turl=%s") % (
> +                       self.args.serial,
> +                       self.args.url)
> +
> +        self.say_begin(tip_msg)
> +        bundles = []
> +        for script in script_list:
> +            sub_bundle = {}
> +            from datetime import datetime
> +            starttime = datetime.utcnow()
> +            test_case_id = script.replace('%s/' % target_dir, '')
> +            if len(test_case_id) > 50:
> +                test_case_id = '%s...' % (test_case_id[:50])
> +            try:
> +                sub_bundle = self.run_monkeyrunner_test(script, serial,
> +                                                         test_case_id)
> +                test_result = {"test_case_id": test_case_id,
> +                               "result": 'pass'}
> +                if sub_bundle:
> +                    sub_bundle['test_runs'][0]['test_results'].append(
> +                                                            test_result)
> +            except Exception as strerror:
> +                self.say('Failed to run script(%s) with error:\n%s' % (
> +                                                                script,
> +                                                                strerror))
> +
> +                test_result = {"test_case_id": test_case_id,
> +                               "result": 'fail'}

The next few lines are indented one block too much I think?

> +                from uuid import uuid4

Can you put this import at top level please?

> +                TIMEFORMAT = '%Y-%m-%dT%H:%M:%SZ'
> +                sub_bundle['test_runs'] = [{
> +                                'test_results': [test_result],
> +                                'test_id':
> +                                    'monkeyrunner(%s)' % test_case_id,
> +                                'time_check_performed': False,
> +                                'analyzer_assigned_uuid':
> +                                        str(uuid4()),
> +                                'analyzer_assigned_date':
> +                                    starttime.strftime(TIMEFORMAT)}]
> +            if sub_bundle:
> +                    bundles.append(sub_bundle)
> +
> +        if self.args.output:
> +            output_dir = os.path.dirname(self.args.output)
> +            if output_dir and (not os.path.exists(output_dir)):
> +                os.makedirs(output_dir)
> +            with open(self.args.output, "wt") as stream:
> +                DocumentIO.dump(stream, merge_bundles(bundles))
> +
> +        self.say_end(tip_msg)
> +
> +    def run_monkeyrunner_test(self, script, serial, test_case_id=None):
> +        config = get_config()
> +
> +        inst = AndroidTestInstaller()
> +        run = AndroidTestRunner(steps_host_pre=[
> +                                'monkeyrunner %s %s' % (script, serial)])
> +        parser = AndroidTestParser()
> +        test = AndroidTest(testname='monkeyrunner',
> +                            installer=inst, runner=run, parser=parser)
> +        test.parser.results = {'test_results': []}
> +        test.setadb(self.adb)
> +
> +        if not self.test_installed(test.testname):
> +            test.install()

The need to do this seems a bit strange, given that test.install()
doesn't really seem to do anything?  I guess given the way things work
today it's needed.

> +        bundle = {}
> +        org_png_file_list = utils.find_files(config.tempdir_host,
> +                                             '.%s' % 'png')
> +        result_id = test.run(quiet=self.args.quiet)
> +        if self.args.output:
> +            cur_all_png_list = utils.find_files(config.tempdir_host,
> +                                                '.%s' % 'png')
> +            new_png_list = set(cur_all_png_list).difference(org_png_file_list)
> +            test_id = 'monkeyrunner(%s)' % (test_case_id)
> +            bundle = generate_bundle(self.args.serial,
> +                    result_id, test=test,
> +                    test_id=test_id,
> +                    attachments=list(new_png_list))
> +            utils.delete_files(new_png_list)
> +        return bundle

I can't find the code which will copy the pngs from the device to the
host.  Can you tell me where it is?

> +
>  class parse(AndroidResultsCommand):
>      """
>      Parse the results of previous test that run on the specified device
> @@ -500,19 +645,29 @@
>      return bundle
>  
>  
> -def generate_bundle(serial=None, result_id=None, test=None, test_id=None):
> +def merge_bundles(bundles=[]):
> +    merged_bundles = {"format": "Dashboard Bundle Format 1.2",
> +                      'test_runs': []}
> +    for bundle in bundles:
> +        if bundle['test_runs']:
> +            merged_bundles['test_runs'].append(bundle['test_runs'][0])
> +    return merged_bundles

This only copies one test_run from each bundle.  Maybe that's OK for
now, but it seems like it might bite us in the future.

> +
> +def generate_bundle(serial=None, result_id=None, test=None,
> +                    test_id=None, attachments=[]):
>      if result_id is None:
>          return {}
>      config = get_config()
>      adb = ADB(serial)
>      resultdir = os.path.join(config.resultsdir_android, result_id)
>      if not adb.exists(resultdir):
> -        raise  LavaCommandError("The result (%s) is not existed." % result_id)
> +        raise  Exception("The result (%s) is not existed." % result_id)
>  
>      bundle_text = adb.read_file(os.path.join(resultdir, "testdata.json"))
>      bundle = DocumentIO.loads(bundle_text)[1]
>      test_tmp = None
> -    if bundle['test_runs'][0]['test_id'] == 'custom':
> +    if test:
>          test_tmp = test
>      else:
>          test_tmp = testloader(bundle['test_runs'][0]['test_id'], serial)
> @@ -542,6 +697,7 @@
>              "content":  base64.standard_b64encode(stderr_text)
>          }
>      ]
> +
>      screencap_path = os.path.join(resultdir, 'screencap.png')
>      if adb.exists(screencap_path):
>          tmp_path = os.path.join(config.tempdir_host, 'screencap.png')
> @@ -553,7 +709,17 @@
>              "pathname": 'screencap.png',
>              "mime_type": 'image/png',
>              "content": base64.standard_b64encode(data)})
> +        os.unlink(tmp_path)
>  
> +    for attach in attachments:
> +        if os.path.exists(attach):
> +            with open(attach, 'rb') as stream:
> +                data = stream.read()
> +            if data:
> +                bundle['test_runs'][0]["attachments"].append({
> +                            "pathname": os.path.basename(attach),
> +                            "mime_type": 'image/png',
> +                            "content": base64.standard_b64encode(data)})
>      return bundle
>  
>  
>
> === modified file 'lava_android_test/utils.py'
> --- lava_android_test/utils.py	2012-03-08 11:29:09 +0000
> +++ lava_android_test/utils.py	2012-04-25 12:02:22 +0000
> @@ -1,4 +1,4 @@
> -# Copyright (c) 2010 Linaro
> +# Copyright (c) 2010-2012 Linaro
>  #
>  # This program is free software: you can redistribute it and/or modify
>  # it under the terms of the GNU General Public License as published by
> @@ -152,3 +152,27 @@
>      if rest is None or  rest == '':
>          return host
>      return os.path.basename(rest)
> +
> +
> +def check_command_exist(command):
> +    rc = subprocess.call(["which", command], stdout=open('/dev/null', 'w'))
> +    return rc == 0
> +
> +
> +def find_files(target_dir, ext):
> +    file_list = []
> +    for f in os.listdir(target_dir):
> +        f_path = '%s/%s' % (target_dir, f)
> +        if os.path.isdir(f_path):
> +            file_list.extend(find_files(f_path, ext))
> +        elif f_path.upper().endswith(ext.upper()):
> +            file_list.append(f_path)
> +
> +    file_list.sort()
> +    return file_list

I think using os.walk will make this a lot simpler.

> +def delete_files(flist=[]):
> +    for f in flist:
> +        if os.path.exists(f):
> +            os.unlink(f)

> === modified file 'setup.py'
> --- setup.py	2012-03-07 10:09:01 +0000
> +++ setup.py	2012-04-25 12:02:22 +0000
> @@ -42,6 +42,7 @@
>      uninstall=lava_android_test.commands:uninstall
>      run=lava_android_test.commands:run
>      run-custom=lava_android_test.commands:run_custom
> +    run-monkeyrunner=lava_android_test.commands:run_monkeyrunner
>      parse=lava_android_test.commands:parse
>      parse-custom=lava_android_test.commands:parse_custom
>      show=lava_android_test.commands:show

lp://qastaging/~liuyq0307/lava-android-test/support-monkeyrunner updated on 2012-05-24

152. By Yongqin Liu on 2012-05-23: merge with trunk
153. By Yongqin Liu on 2012-05-23: update according to the comment
154. By Yongqin Liu on 2012-05-24: update the typo of bundle
155. By Yongqin Liu on 2012-05-24: revert project files
156. By Yongqin Liu on 2012-05-24: add return serial for get_device_serial according to review comment
157. By Yongqin Liu on 2012-05-24: modify bundle_format definition bug that using os.environ.get to define
158. By Yongqin Liu on 2012-05-24: modify the auto rewriting error

LAVA Android Test

Merge lp://qastaging/~liuyq0307/lava-android-test/support-monkeyrunner into lp://qastaging/lava-android-test

Commit message

Description of the change

Unmerged revisions

Preview Diff

Subscribers