Merge lp:~doanac/lava-android-test/restricted-coremark into lp:lava-android-test

Proposed by Andy Doan on 2012-03-06
Status: Needs review
Proposed branch: lp:~doanac/lava-android-test/restricted-coremark
Merge into: lp:lava-android-test
Diff against target: 76 lines (+67/-0)
2 files modified
lava_android_test/test_definitions/coremark.py (+54/-0)
lava_android_test/test_definitions/download_test.sh (+13/-0)
To merge this branch: bzr merge lp:~doanac/lava-android-test/restricted-coremark
Reviewer Review Type Date Requested Status
Paul Larson (community) 2012-03-06 Abstain on 2018-06-18
Andy Doan (community) Needs Fixing on 2012-03-08
Linaro Validation Team 2012-03-07 Pending
Review via email: mp+96263@code.launchpad.net

Description of the change

Add support for restricted coremark bench

This adds the basic ability to run the coremark benchmark on
Android. Since coremark is a restricted program, the actual
binary is managed separate from this test. The script,
download_test.sh is used by coremark.py to download the proper
binary. Therefore, this test requires install options to be
passed to the install command for example:

   lava-android-test install -o "coremark_url=https://user:pass@1.1.1.1/coremark-linaro4.6" coremark

The above example would download from an HTTP password protected
directory the coremark-linaro4.6 binary.

To post a comment you must log in.
Paul Larson (pwlars) wrote :

I think this looks fine to me, except we should add some documentation for the test (Yongqin has a branch in progress for this right now to add it to the others... it's pretty simple). I think at least the docstring in the test itself explaining what it is, and definitely mentioning that it does not include the coremark binaries which need to be obtained from somewhere else.

I cc'd the rest of the team and would like to get them to weigh in on this as well. The big issue that I'm concerned about I think is that it looks pretty inconvenient to have to create the binaries for coremark, host them somewhere, make sure we only run this in a private job once the ability to do that exists, and create all the jobs pointing to those binaries. I'd love it if lava already had a build step built into it that would accomodate something like this internally, but it doesn't right now. It might be worth considering if we could somehow handle the build part of this through Jenkins in the same style that we do ci testing. Something for you to think about I guess.

In any case, I wouldn't actually recommend running a job that uses this in lava until we land and install the rest of the privacy changes so that you can:
1. specify a private bundle stream to push the results to
2. make sure that nobody sees your user:password who is not in linaro - this will still be in the job log, and even the job json, but if the job is specifically marked private, it won't be visible to anyone outside of linaro.

Andy Doan (doanac) wrote :

On 03/07/2012 10:43 PM, Paul Larson wrote:
> I think this looks fine to me, except we should add some documentation for the test (Yongqin has a branch in progress for this right now to add it to the others... it's pretty simple). I think at least the docstring in the test itself explaining what it is, and definitely mentioning that it does not include the coremark binaries which need to be obtained from somewhere else.

sounds good.

> I cc'd the rest of the team and would like to get them to weigh in on this as well. The big issue that I'm concerned about I think is that it looks pretty inconvenient to have to create the binaries for coremark, host them somewhere, make sure we only run this in a private job once the ability to do that exists, and create all the jobs pointing to those binaries. I'd love it if lava already had a build step built into it that would accomodate something like this internally, but it doesn't right now. It might be worth considering if we could somehow handle the build part of this through Jenkins in the same style that we do ci testing. Something for you to think about I guess.

Its extremely inconvenient but that seems to be the nature of dealing
with these benchmark suites. The changing URL is pretty much a must,
because the actual coremark binary will always be changing.

It could be done from Jenkins. However, I'd have to create special
Android build job just for this, because the code base isn't in a public
git repository which Android requires. So that solution isn't really
trivial either.

Andy Doan (doanac) wrote :

I just discovered this won't work because AOSP builds don't include busybox. I have to come up with a new way for this test to be done.

review: Needs Fixing
Paul Larson (pwlars) :
review: Abstain

Unmerged revisions

141. By Andy Doan on 2012-03-06

Add support for restricted coremark bench

This adds the basic ability to run the coremark benchmark on
Android. Since coremark is a restricted program, the actual
binary is managed separate from this test. The script,
download_test.sh is used by coremark.py to download the proper
binary. Therefore, this test requires install options to be
passed to the install command for example:

 lava-android-test install -o "coremark_url=https://user:pass@1.1.1.1/coremark-linaro4.6" coremark

The above example would download from an HTTP password protected
directory the coremark-linaro4.6 binary.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== added file 'lava_android_test/test_definitions/coremark.py'
2--- lava_android_test/test_definitions/coremark.py 1970-01-01 00:00:00 +0000
3+++ lava_android_test/test_definitions/coremark.py 2012-03-06 23:49:17 +0000
4@@ -0,0 +1,54 @@
5+# Copyright (c) 2012 Linaro
6+
7+# Author: Linaro Validation Team <linaro-dev@lists.linaro.org>
8+#
9+# This file is part of LAVA Android Test.
10+#
11+#
12+# This program is free software: you can redistribute it and/or modify
13+# it under the terms of the GNU General Public License as published by
14+# the Free Software Foundation, either version 3 of the License, or
15+# (at your option) any later version.
16+#
17+# This program is distributed in the hope that it will be useful,
18+# but WITHOUT ANY WARRANTY; without even the implied warranty of
19+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
20+# GNU General Public License for more details.
21+#
22+# You should have received a copy of the GNU General Public License
23+# along with this program. If not, see <http://www.gnu.org/licenses/>.
24+import os
25+import lava_android_test.config
26+import lava_android_test.testdef
27+
28+test_name = 'coremark'
29+
30+config = lava_android_test.config.get_config()
31+inst_dir = '%s/%s' % (config.installdir_android, test_name)
32+
33+options_file = "%s/install_options" % (inst_dir)
34+coremark = '%s/coremark' % (inst_dir)
35+
36+curdir = os.path.realpath(os.path.dirname(__file__))
37+script_host = '%s/download_test.sh' % (curdir)
38+script_target = '%s/download_test.sh' % (inst_dir)
39+
40+INSTALL_STEPS_ADB_PRE = [
41+ 'push %s %s ' % (script_host, script_target),
42+ 'shell chmod 777 %s' % script_target,
43+ 'shell %s coremark %s %s' % (script_target, options_file, coremark),
44+]
45+
46+ADB_SHELL_STEPS = [ '"%s 0x0 0x0 0x66 0 7 1 2000"' % coremark ]
47+PATTERN = "^CoreMark 1.0\s*:\s+(?P<measurement>\d+\.\d+) / (?P<message>.*)"
48+
49+inst = lava_android_test.testdef.AndroidTestInstaller(
50+ steps_adb_pre=INSTALL_STEPS_ADB_PRE)
51+run = lava_android_test.testdef.AndroidTestRunner(
52+ adbshell_steps=ADB_SHELL_STEPS)
53+parser = lava_android_test.testdef.AndroidTestParser(PATTERN,
54+ appendall={'test_case_id':'Iterations/Sec'})
55+testobj = lava_android_test.testdef.AndroidTest(testname=test_name,
56+ installer=inst,
57+ runner=run,
58+ parser=parser)
59
60=== added file 'lava_android_test/test_definitions/download_test.sh'
61--- lava_android_test/test_definitions/download_test.sh 1970-01-01 00:00:00 +0000
62+++ lava_android_test/test_definitions/download_test.sh 2012-03-06 23:49:17 +0000
63@@ -0,0 +1,13 @@
64+#!/system/bin/sh
65+
66+KEY=$1
67+OPTIONS_FILE=$2
68+OUTFILE=$3
69+
70+url=`busybox grep $KEY_url= $OPTIONS_FILE | busybox cut -d= -f2`
71+if [ "a$url" == "a" ] ; then
72+ echo "$KEY install command requires '-o ${KEY}_url=<url>'"
73+ exit 1
74+fi
75+busybox wget -O $OUTFILE $url
76+chmod 755 $OUTFILE

Subscribers

People subscribed via source and target branches