Merge lp:~liuyq0307/lava-android-test/workload into lp:lava-android-test

Proposed by Yongqin Liu
Status: Merged
Merged at revision: 221
Proposed branch: lp:~liuyq0307/lava-android-test/workload
Merge into: lp:lava-android-test
Diff against target: 226 lines (+182/-5)
3 files modified
lava_android_test/provider.py (+1/-3)
lava_android_test/test_definitions/hostshells/workload.sh (+147/-0)
lava_android_test/testdef.py (+34/-2)
To merge this branch: bzr merge lp:~liuyq0307/lava-android-test/workload
Reviewer Review Type Date Requested Status
Andy Doan (community) Approve
vishal Pending
Review via email: mp+139262@code.launchpad.net

This proposal supersedes a proposal from 2012-12-06.

Description of the change

add test for workload on b.L system
according to the talk on IRC, change to use local git url as default, also add support for specification of git url

To post a comment you must log in.
Revision history for this message
Andy Doan (doanac) wrote : Posted in a previous version of this proposal

Before going too far into this, I wanted to mention my general thought:

This seems to be using ADB to generate workload intensive stuff. I think
this is probably the wrong way to be doing this. I think the bigLITTLE
switcher tests is a better approach where its written entirely for the
target. This seems way better, and will ultimately better fit with our
lava-test-shell efforts.

On 12/06/2012 07:06 AM, Yongqin Liu wrote:
> + git_url="ssh://<email address hidden>/srv/linaro-private.git.linaro.org/people/bhoj/workload-automation.git"

That's not going to work for anyone but you.

Revision history for this message
Yongqin Liu (liuyq0307) wrote : Posted in a previous version of this proposal

> Before going too far into this, I wanted to mention my general thought:
>
> This seems to be using ADB to generate workload intensive stuff. I think
> this is probably the wrong way to be doing this. I think the bigLITTLE
> switcher tests is a better approach where its written entirely for the
> target. This seems way better, and will ultimately better fit with our
> lava-test-shell efforts.

not see the detail implementation of workload test, but it seems to download
the apk of nbench, and install it to device, and do other things.

and for this workload test, are all the steps need to be run on host side
able to be done with lava-test-shell?
  git clone
  update ip in config
  run python workload_setup_dependencies.py
  python workload.py ${config_file} ${outputdir}/

> On 12/06/2012 07:06 AM, Yongqin Liu wrote:
> > + git_url="ssh://<email address hidden>/srv/linaro-
> private.git.linaro.org/people/bhoj/workload-automation.git"
>
> That's not going to work for anyone but you.
Or, what you mean is to rewrite the workload test via lava-test-shell?

Revision history for this message
Andy Doan (doanac) wrote : Posted in a previous version of this proposal

On 12/06/2012 09:02 PM, Yongqin Liu wrote:
>> Before going too far into this, I wanted to mention my general thought:
>>
>> This seems to be using ADB to generate workload intensive stuff. I think
>> this is probably the wrong way to be doing this. I think the bigLITTLE
>> switcher tests is a better approach where its written entirely for the
>> target. This seems way better, and will ultimately better fit with our
>> lava-test-shell efforts.
>
> not see the detail implementation of workload test, but it seems to download
> the apk of nbench, and install it to device, and do other things.

I think this could be a quick patch to lava-test-shell. We have "deps"
support for Ubuntu. We could do similar for Android.

> and for this workload test, are all the steps need to be run on host side
> able to be done with lava-test-shell?
> git clone
> update ip in config
> run python workload_setup_dependencies.py
> python workload.py ${config_file} ${outputdir}/

We don't have to do the exact steps. git-clone is supported. not sure
about the ip update. The setup deps could be done in an equivalent
manner. Instead of thinking "how do we do this on host in python", I'm
saying ask yourself - "how could I do this as a script that ran on the
target"? I don't think it would be that hard.

If you absolutely think I'm crazy, then continue with this approach, but
doing a "workload" test and requiring ADB seems like a bad idea.

>> On 12/06/2012 07:06 AM, Yongqin Liu wrote:
>>> + git_url="ssh://<email address hidden>/srv/linaro-
>> private.git.linaro.org/people/bhoj/workload-automation.git"
>>
>> That's not going to work for anyone but you.
> Or, what you mean is to rewrite the workload test via lava-test-shell?

No - i just meant that the git url you had there will only work for you
with your ssh keys :)

Revision history for this message
vishal (vishalbhoj) wrote : Posted in a previous version of this proposal

On 7 December 2012 22:34, Andy Doan <email address hidden> wrote:

> On 12/06/2012 09:02 PM, Yongqin Liu wrote:
> >> Before going too far into this, I wanted to mention my general thought:
> >>
> >> This seems to be using ADB to generate workload intensive stuff. I think
> >> this is probably the wrong way to be doing this. I think the bigLITTLE
> >> switcher tests is a better approach where its written entirely for the
> >> target. This seems way better, and will ultimately better fit with our
> >> lava-test-shell efforts.
> >
> > not see the detail implementation of workload test, but it seems to
> download
> > the apk of nbench, and install it to device, and do other things.
>
> I think this could be a quick patch to lava-test-shell. We have "deps"
> support for Ubuntu. We could do similar for Android.
>
> > and for this workload test, are all the steps need to be run on host side
> > able to be done with lava-test-shell?
> > git clone
> > update ip in config
> > run python workload_setup_dependencies.py
> > python workload.py ${config_file} ${outputdir}/
>
> We don't have to do the exact steps. git-clone is supported. not sure
> about the ip update. The setup deps could be done in an equivalent
> manner. Instead of thinking "how do we do this on host in python", I'm
> saying ask yourself - "how could I do this as a script that ran on the
> target"? I don't think it would be that hard.
>

I get your point here. But the whole benchmark is written by ARM to run
over adb + they even take control of serial terminal similar to LAVA( we
don't execute that path but rely on LAVA to reboot etc).

>
> If you absolutely think I'm crazy, then continue with this approach, but
> doing a "workload" test and requiring ADB seems like a bad idea.
>

adb is used only to transfer the files before and after the benchmark is
run and not while the benchmark is running.

>
> >> On 12/06/2012 07:06 AM, Yongqin Liu wrote:
> >>> + git_url="ssh://
> <email address hidden>/srv/linaro-
> >> private.git.linaro.org/people/bhoj/workload-automation.git"
> >>
> >> That's not going to work for anyone but you.
> > Or, what you mean is to rewrite the workload test via lava-test-shell?
>
> No - i just meant that the git url you had there will only work for you
> with your ssh keys :)
>
>
> --
>
> https://code.launchpad.net/~liuyq0307/lava-android-test/workload/+merge/138448
> You are requested to review the proposed merge of
> lp:~liuyq0307/lava-android-test/workload into lp:lava-android-test.
>

Revision history for this message
Andy Doan (doanac) wrote : Posted in a previous version of this proposal

On 12/07/2012 11:13 AM, vishal wrote:
> I get your point here. But the whole benchmark is written by ARM to run
> over adb + they even take control of serial terminal similar to LAVA( we
> don't execute that path but rely on LAVA to reboot etc).

So use lava-android-test.

Its a little frustrating though:

* Android guys complained for over a year about "no blackbox testing".

* Benchmark guys complained about all Android results being invalid
because of the same basic thing.

We now have a solution in place, and you guys don't seem to be
interested in using it.

>> >
>> >If you absolutely think I'm crazy, then continue with this approach, but
>> >doing a "workload" test and requiring ADB seems like a bad idea.
>> >
> adb is used only to transfer the files before and after the benchmark is
> run and not while the benchmark is running.

So that could be done in lava-test-shell :)

Revision history for this message
Yongqin Liu (liuyq0307) wrote : Posted in a previous version of this proposal

> On 12/07/2012 11:13 AM, vishal wrote:
> > I get your point here. But the whole benchmark is written by ARM to run
> > over adb + they even take control of serial terminal similar to LAVA( we
> > don't execute that path but rely on LAVA to reboot etc).
>
> So use lava-android-test.
If this means you aggree this branch for now, please approve this MP.
>
> Its a little frustrating though:
>
> * Android guys complained for over a year about "no blackbox testing".
>
what I know about this is for the network test problem which the adb connection that lava-android-test relies on.
> * Benchmark guys complained about all Android results being invalid
> because of the same basic thing.
not know the details, but from the what I can guess it's not a problem lava can resolve,
normally benchmark tools only displayed the result with GUI where command tools are not able to get.
I don't think lava-test-shell can help on this, if it can do, I am glad to know how it implemented.

> We now have a solution in place, and you guys don't seem to be
> interested in using it.
Actually at least I am interesting in it,
but if you ask me to reimplement an existing tool which works well with adb connection,
then I will want to know why, and know how much time it will take to reimplement.

> >> >If you absolutely think I'm crazy, then continue with this approach, but
> >> >doing a "workload" test and requiring ADB seems like a bad idea.
> >> >
> > adb is used only to transfer the files before and after the benchmark is
> > run and not while the benchmark is running.
>
> So that could be done in lava-test-shell :)

Revision history for this message
Andy Doan (doanac) wrote : Posted in a previous version of this proposal

On 12/10/2012 08:18 AM, Yongqin Liu wrote:
>> On 12/07/2012 11:13 AM, vishal wrote:
>>> I get your point here. But the whole benchmark is written by ARM to run
>>> over adb + they even take control of serial terminal similar to LAVA( we
>>> don't execute that path but rely on LAVA to reboot etc).
>>
>> So use lava-android-test.
> If this means you aggree this branch for now, please approve this MP.

yes. it seems fine.

NOTE: we still have to iron out the private git access part of this issue.

Revision history for this message
Andy Doan (doanac) wrote : Posted in a previous version of this proposal

Actually wait a bit. I think your git url still won't work

Revision history for this message
Yongqin Liu (liuyq0307) wrote : Posted in a previous version of this proposal

> Actually wait a bit. I think your git url still won't work
I will change it to use the local git repository tomorrow.

And BTW, I will need you to help to deploy this modification tomorrow then.
So in the design of lava-test-shell, can you make it possible that the deployment isno need when add new test?

Revision history for this message
Andy Doan (doanac) wrote : Posted in a previous version of this proposal

On 12/10/2012 09:32 AM, Yongqin Liu wrote:
>> Actually wait a bit. I think your git url still won't work
> I will change it to use the local git repository tomorrow.
>
> And BTW, I will need you to help to deploy this modification tomorrow then.
> So in the design of lava-test-shell, can you make it possible that the deployment isno need when add new test?

Yep - lava-test-shell allows you to control your own test definitions
(and even keep them in git if you'd like)

Revision history for this message
Andy Doan (doanac) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lava_android_test/provider.py'
--- lava_android_test/provider.py 2012-07-10 06:34:06 +0000
+++ lava_android_test/provider.py 2012-12-11 16:47:25 +0000
@@ -266,15 +266,13 @@
266 test_sh_path = '%s/%s' % (mod.curdir, f_name)266 test_sh_path = '%s/%s' % (mod.curdir, f_name)
267267
268 HOST_SHELL_STEPS = ['bash %s -s $(SERIAL) $(OPTIONS)' % test_sh_path]268 HOST_SHELL_STEPS = ['bash %s -s $(SERIAL) $(OPTIONS)' % test_sh_path]
269 PATTERN = ("^\s*(?P<test_case_id>\S+)="
270 "(?P<result>(pass|fail|ok|ng|true|false|skip|done))\s*$")
271269
272 testobj = self.gen_testobj(270 testobj = self.gen_testobj(
273 testname=test_name,271 testname=test_name,
274 installer=testdef.AndroidTestInstaller(),272 installer=testdef.AndroidTestInstaller(),
275 runner=testdef.AndroidTestRunner(273 runner=testdef.AndroidTestRunner(
276 steps_host_pre=HOST_SHELL_STEPS),274 steps_host_pre=HOST_SHELL_STEPS),
277 parser=testdef.AndroidTestParser(PATTERN),275 parser=testdef.AndroidSimpleTestParser(),
278 adb=ADB(serial))276 adb=ADB(serial))
279 return testobj277 return testobj
280278
281279
=== added file 'lava_android_test/test_definitions/hostshells/workload.sh'
--- lava_android_test/test_definitions/hostshells/workload.sh 1970-01-01 00:00:00 +0000
+++ lava_android_test/test_definitions/hostshells/workload.sh 2012-12-11 16:47:25 +0000
@@ -0,0 +1,147 @@
1#!/bin/bash
2# Copyright (c) 2012 Linaro
3
4# Author: Linaro Validation Team <linaro-dev@lists.linaro.org>
5#
6# This file is part of LAVA Android Test.
7#
8# This program is free software: you can redistribute it and/or modify
9# it under the terms of the GNU General Public License as published by
10# the Free Software Foundation, either version 3 of the License, or
11# (at your option) any later version.
12#
13# This program is distributed in the hope that it will be useful,
14# but WITHOUT ANY WARRANTY; without even the implied warranty of
15# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
16# GNU General Public License for more details.
17#
18# You should have received a copy of the GNU General Public License
19# along with this program. If not, see <http://www.gnu.org/licenses/>.
20
21function parse_argv() {
22 # Parse command line arguments
23 while test -n "$1"; do
24 case "$1" in
25 --serial|-s)
26 SERIAL="$2"
27 if [ -n "${SERIAL}" ]; then
28 shift 2
29 else
30 show_usage
31 exit 1
32 fi
33 ;;
34 --config|-c)
35 CONFIG="$2"
36 if [ -n "${CONFIG}" ]; then
37 shift 2
38 else
39 show_usage
40 exit 1
41 fi
42 ;;
43 --git|-g)
44 GIT_URL="$2"
45 if [ -n "${GIT_URL}" ]; then
46 shift 2
47 else
48 show_usage
49 exit 1
50 fi
51 ;;
52 --help|-h)
53 show_usage
54 exit 1
55 ;;
56 *)
57 if [ -n "${OPTIONS}" ]; then
58 OPTIONS="${OPTIONS} $1"
59 else
60 OPTIONS="$1"
61 fi
62 shift
63 ;;
64 esac
65 done
66}
67
68function show_usage(){
69 # Display the usage line
70 echo "Usage $(basename $0) [--serial|-s <serial>] [--config|-c <config_file>] [--git|g <git-url>] <other-option>"
71 echo "Usage $(basename $0) [--help|-h]"
72}
73
74function parse_output_result(){
75 result_f=${1}
76 if [ ! -f "${1}" ]; then
77 echo "There is no result file output/results.csv"
78 return
79 fi
80
81 file_tmp=${result_f}.tmp
82 sed 's/ /_/g' ${result_f} > ${file_tmp}
83 keys=`head -n 1 ${file_tmp}`
84 values=`tail -n 1 ${file_tmp}`
85 for ((i=1; i<21; i++)); do
86 key=`echo ${keys}|cut -d , -f ${i}|sed 's/\r//'`
87 value=`echo ${values}|cut -d , -f ${i}|sed 's/\r//'`
88
89 echo ${value}|grep -P '^[.\d]+$' &>/dev/null
90 if [ $? -ne 0 ]; then
91 key="${key}_${value}"
92 value="true"
93 fi
94 echo ${key}=${value}
95 done
96 rm -f ${file_tmp}
97}
98
99function main(){
100 local_git="file:///home/bhoj/workload-automation.git"
101 branch="lava"
102 outputdir="outputdir"
103 result="${outputdir}/result.csv"
104
105 parse_argv "$@"
106
107 config_file="config.csv"
108 if [ -n "${CONFIG}" ]; then
109 config_file="${CONFIG}"
110 fi
111
112 if [ -n "${GIT_URL}" ]; then
113 git_url="${GIT_URL}"
114 else
115 git_url="${local_git}"
116 fi
117
118 git clone "${git_url}" -b ${branch}
119 if [ $? -ne 0 ]; then
120 echo "Failed to clone git repository: ${git_url}"
121 exit 1
122 fi
123 ip=`echo ${SERIAL}|sed 's/:5555//'`
124 cd "workload-automation"
125
126 #update the ip address and patch config.csv file
127 sed -i "s/192.168.1.38/${ip}/g" workload_config.py
128 sed -i "s/,1200000//g" "${config_file}" #for config_a15.csv and config.csv
129 sed -i "s/,1000000//g" "${config_file}" #for config_a7.csv
130
131 python workload_setup_dependencies.py
132 if [ $? -ne 0 ]; then
133 echo "Failed to run workload_setup_dependencies.py"
134 exit 1
135 fi
136
137 rm -fr ${outputdir}
138 python workload.py ${config_file} ${outputdir}/
139 if [ $? -ne 0 ]; then
140 echo "Failed to run workload.py config.csv outputdir/"
141 exit 1
142 fi
143 parse_output_result ${result}
144}
145
146main "$@"
147
0148
=== modified file 'lava_android_test/testdef.py'
--- lava_android_test/testdef.py 2012-08-17 07:36:25 +0000
+++ lava_android_test/testdef.py 2012-12-11 16:47:25 +0000
@@ -429,8 +429,8 @@
429 def real_parse(self, result_filename='stdout.log',429 def real_parse(self, result_filename='stdout.log',
430 output_filename='stdout.log', test_name=''):430 output_filename='stdout.log', test_name=''):
431 """Using the pattern to do the real parse operation431 """Using the pattern to do the real parse operation
432 432
433 generate the test_results elements from the result file by parsing 433 generate the test_results elements from the result file by parsing
434 with the pattern specified.434 with the pattern specified.
435 """435 """
436 if not self.pattern:436 if not self.pattern:
@@ -599,6 +599,38 @@
599 self.fixmeasurements()599 self.fixmeasurements()
600 self.fixids()600 self.fixids()
601601
602class AndroidSimpleTestParser(AndroidTestParser):
603
604 def real_parse(self, result_filename='stdout.log',
605 output_filename='stdout.log', test_name=''):
606 self.res_pattern = ("^\s*(?P<test_case_id>.+?)\s*="
607 "\s*(?P<result>(pass|fail|ok|ng|true|false|skip|done))\s*$")
608 self.measurement_pattern = ("^\s*(?P<test_case_id>.+?)\s*="
609 "\s*(?P<measurement>[\.\d]+)\s*$")
610 self.measurement_units_pattern = ("^\s*(?P<test_case_id>.+?)\s*="
611 "\s*(?P<measurement>[\.\d]+)\s+(?P<units>\S+)\s*$")
612
613 res_pat = re.compile(self.res_pattern)
614 measurement_pat = re.compile(self.measurement_pattern)
615 measurement_units_pat = re.compile(self.measurement_units_pattern)
616
617 with open(output_filename, 'r') as stream:
618 for lineno, line in enumerate(stream, 1):
619 match = res_pat.search(line)
620 if not match:
621 match = measurement_pat.search(line)
622 if not match:
623 match = measurement_units_pat.search(line)
624 if not match:
625 continue
626 data = match.groupdict()
627 data["log_filename"] = result_filename
628 data["log_lineno"] = lineno
629 if data.get('result') is None:
630 data['result'] = 'pass'
631
632 self.results['test_results'].append(data)
633
602634
603def _run_steps_host(steps=[], serial=None, option=None, resultsdir=None):635def _run_steps_host(steps=[], serial=None, option=None, resultsdir=None):
604 adb = ADB(serial)636 adb = ADB(serial)

Subscribers

People subscribed via source and target branches