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

Status:	Merged
Merged at revision:	158
Proposed branch:	lp://qastaging/~liuyq0307/lava-android-test/support-monkeyrunner
Merge into:	lp://qastaging/lava-android-test
Diff against target:	496 lines (+290/-15) 9 files modified lava_android_test/commands.py (+174/-5) lava_android_test/config.py (+3/-1) 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/testdef.py (+2/-2) lava_android_test/utils.py (+23/-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-05-23	Pending
Review via email: mp+106942@code.qastaging.launchpad.net

Revision history for this message

Zygmunt Krynicki (zyga) wrote on 2012-04-25: Posted in a previous version of this proposal

#

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: Posted in a previous version of this proposal

#

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: Posted in a previous version of this proposal

#

-----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: Posted in a previous version of this proposal

#

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

Revision history for this message

Yongqin Liu (liuyq0307) wrote on 2012-05-23:

#

>> + 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.

The install function will create directory in the target, it is there for store some file.
But in this time, not really use.

Here used to keep the consistency with other tests.

>> + 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?

The png files are generate on the host by the monkeyrunner script, so there is no need to pull them form device.
And it is suggested to generated on the same directory of the monkeyrunner script file

Revision history for this message

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

#

Download full text (13.1 KiB)

Thanks for fixing the things I complained about, however there seem to
be some new, unrelated changes that really belong in another branch...

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/106942
>
> add run-monkeyrunner sub command to make support monkeyrunner test easily.
> --
> https://code.launchpad.net/~liuyq0307/lava-android-test/support-monkeyrunner/+merge/106942
> Your team Linaro Validation Team is requested to review the proposed merge of lp:~liuyq0307/lava-android-test/support-monkeyrunner into lp:lava-android-test.
> === modified file 'lava_android_test/commands.py'
> --- lava_android_test/commands.py 2012-05-08 05:32:19 +0000
> +++ lava_android_test/commands.py 2012-05-23 06:49:24 +0000
> @@ -19,18 +19,23 @@
> import os
> import base64
> import urlparse
> +from uuid import uuid4
> 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 +151,13 @@
> 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 ''

This looks wrong -- I guess a 'return serial' has been dropped?

> def assertDeviceIsConnected(self):
> if not self.adb.isDeviceConnected():
> if self.adb.serial:
> @@ -163,7 +175,7 @@
>
> self.invoke_sub()
>
> - def invoke_sub():
> + def invoke_sub(self):
> raise NotImplementedError
>
>
> @@ -344,7 +356,7 @@
> class run_custom(AndroidCommand):
> """
> Run the command(s) that specified by the -c option in the command line

> - program:: lava-android-test run-custom -c 'command1' -c 'command2' -p 'parse-regex1'
> + program:: lava-android-test run-custom -c 'cm1' -c 'cmd2' -p 'parse-regex1'
> program:: lava-android-test run test-id -s device_serial
> program:: lava-android-test run test-id -s device_serial -o outputfile
> """
> @@ -453,6 +465,139 @@
> self.say_end(tip_msg)
>
>
> +class run_monkeyrunner(AndroidCommand):
> + """
> + Run the monkeyrun...

Thanks for fixing the things I complained about, however there seem to
be some new, unrelated changes that really belong in another branch...

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/106942
>
> add run-monkeyrunner sub command to make support monkeyrunner test easily.
> -- 
> https://code.launchpad.net/~liuyq0307/lava-android-test/support-monkeyrunner/+merge/106942
> Your team Linaro Validation Team is requested to review the proposed merge of lp:~liuyq0307/lava-android-test/support-monkeyrunner into lp:lava-android-test.
> === modified file 'lava_android_test/commands.py'
> --- lava_android_test/commands.py	2012-05-08 05:32:19 +0000
> +++ lava_android_test/commands.py	2012-05-23 06:49:24 +0000
> @@ -19,18 +19,23 @@
>  import os
>  import base64
>  import urlparse
> +from uuid import uuid4
>  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 +151,13 @@
>          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 ''

This looks wrong -- I guess a 'return serial' has been dropped?

>      def assertDeviceIsConnected(self):
>          if not self.adb.isDeviceConnected():
>              if self.adb.serial:
> @@ -163,7 +175,7 @@
>  
>          self.invoke_sub()
>  
> -    def invoke_sub():
> +    def invoke_sub(self):
>          raise NotImplementedError
>  
>  
> @@ -344,7 +356,7 @@
>  class run_custom(AndroidCommand):
>      """
>      Run the command(s) that specified by the -c option in the command line
> -    program:: lava-android-test run-custom -c 'command1' -c 'command2' -p 'parse-regex1'
> +    program:: lava-android-test run-custom -c 'cm1' -c 'cmd2' -p 'parse-regex1'
>      program:: lava-android-test run test-id -s device_serial
>      program:: lava-android-test run test-id -s device_serial -o outputfile
>      """
> @@ -453,6 +465,139 @@
>          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)
> +        else:
> +            raise LavaCommandError("The repository type(%s) is not supported"
> +                                   % self.args.repo_type)
> +
> +        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'}
> +                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)

I find this code a bit confusing, but I guess if it works...

> +        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)
> +
> +        ##By calling the install function, we will create the directory
> +        ##on the target, and the the output file and error file
> +        ##will be pushed there
> +        if not self.test_installed(test.testname):
> +            test.install()
> +
> +        ##The png files here are generated to the host by the monkeyrunner
> +        ##monkeyrunner is run on host, not on the target
> +        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
> +
> +
>  class parse(AndroidResultsCommand):
>      """
>      Parse the results of previous test that run on the specified device
> @@ -476,7 +621,7 @@
>      @classmethod
>      def register_arguments(cls, parser):
>          super(parse_custom, cls).register_arguments(parser)
> -        parser.add_argument('-p', '--parse-regex',
> +        parser.add_argument(' - p', ' - -parse - regex',

What is the purpose of this change?  It doesn't look right.

>                              help=("Specified the regular expression used"
>                                    " for analyzing command output"))
>  
> @@ -518,19 +663,30 @@
>      return bundle
>  
>  
> -def generate_bundle(serial=None, result_id=None, test=None, test_id=None):
> +def merge_bundles(bundles=[]):
> +    config = get_config()
> +    merged_bundles = {"format": config.bundel_format,
> +                      'test_runs': []}
> +    for bundle in bundles:
> +        if bundle['test_runs']:
> +            merged_bundles['test_runs'].append(bundle['test_runs'][0])
> +    return merged_bundles
> +
> +
> +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)
> @@ -560,6 +716,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')
> @@ -571,7 +728,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/config.py'
> --- lava_android_test/config.py	2012-03-08 11:29:09 +0000
> +++ lava_android_test/config.py	2012-05-23 06:49:24 +0000
> @@ -1,4 +1,4 @@
> -# Copyright (c) 2010-2011 Linaro
> +# Copyright (c) 2010-2012 Linaro
>  
>  # Author: Linaro Validation Team <linaro-dev@lists.linaro.org>
>  #
> @@ -33,6 +33,8 @@
>          self.tempdir_android = os.path.join(home, 'temp')
>          self.tempdir_host = os.environ.get('ANDROID_TEST_TEMP_HOST',
>                                              '/tmp/lava-android-test')
> +        self.bundel_format = os.environ.get("Dashboard Bundle Format 1.3")
> +

There are at least three things wrong with this:

1) self.bundel_format is not spelt correctly, it should be bundle_format
2) os.environ.get("Dashboard Bundle Format 1.3") isn't what you mean,
   that attempts to read an environment variable named "Dashboard Bundle
   Format 1.3".  I guess you meant something like
   os.environ.get("ANDROID_BUNDLE_FORMAT", "Dashboard Bundle Format 1.3").
3) this change (which seems to be about bug 1002817) should be in
   another branch anyway.

>  _config = None
>  
>

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

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

Revision history for this message

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

#

206 - parser.add_argument('-p', '--parse-regex',
207 + parser.add_argument(' - p', ' - -parse - regex',

I still don't understand what this is for.

All else looks good, so if you can revert the above, please merge.

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

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

Preview Diff

Subscribers