Merge lp:~liuyq0307/linaro-android-build-tools/add-monkey-l-a-t into lp:linaro-android-build-tools

[Yongqin Liu]

modify for adding monkey test to android build
BP:
    https://blueprints.launchpad.net/linaro-android/+spec/linaro-android-run-monkey-in-lava

modify according review comments:
1. typo of submin_results_command
2. delete of IMAGE_TYPE revelant source
3. fix braces style

[James Westby (james-w)]

Hi,

This looks great, thanks, and there are some good cleanups
here too.

Is the new linaro_android_test_run action available on validation.linaro.org
and able to run both monkey and 0xbench?

If not then we will have to hold off on deploying this until it is.

54 + # Board-specific parameters

Extra space included in the indentation on that line.

122 + submin_results_command = "submit_results"
123 + if image_type == 'android':
124 + submin_results_command = "submit_results_on_host"

Typo in variable name, it should be "submit_results_command"

Thanks,

James

review: Approve

[Paul Sokolovsky (pfalcon)]

+ image_type = os.environ.get("IMAGE_TYPE")

What's being done here? If it calls for adding another build config variable, it's likely superfluous (unless there're reasons why it's not).

Also, braces placement needs fixing, it for sure worse than was before. Rule of thumb is to just follow style of the existing code (and minimize any whitespace changes).

And can you please look thru associated blueprint's whiteboard, it seems to have typos which make it a bit unclear ("Acceptance: Monkey for as long as it can", "Update the meeting to a weekly")

review: Needs Fixing

[Yongqin Liu (liuyq0307)]

thanks for review
> Is the new linaro_android_test_run action available on validation.linaro.org
> and able to run both monkey and 0xbench?
yes, linaro_android_test_run is available on validation.linaro.org, and can run both monkey and 0xbench

> If not then we will have to hold off on deploying this until it is.
>
> 54 + # Board-specific parameters
>
> Extra space included in the indentation on that line.
sorry, delete the extra space

> 122 + submin_results_command = "submit_results"
> 123 + if image_type == 'android':
> 124 + submin_results_command = "submit_results_on_host"
>
> Typo in variable name, it should be "submit_results_command"
sorry, I will fixing it.

[Yongqin Liu (liuyq0307)]

> + image_type = os.environ.get("IMAGE_TYPE")
>
> What's being done here? If it calls for adding another build config variable,
> it's likely superfluous (unless there're reasons why it's not).
sorry, I didn't recognize that this file only be used for android-build.
I will delete the relevant source.

> Also, braces placement needs fixing, it for sure worse than was before. Rule
> of thumb is to just follow style of the existing code (and minimize any
> whitespace changes).
I have modify the braces placement according the policy that minimize any whitespace changes.
BTW, is there any coding style for braces?

> And can you please look thru associated blueprint's whiteboard, it seems to
> have typos which make it a bit unclear ("Acceptance: Monkey for as long as it
> can", "Update the meeting to a weekly")
I have updated the bp, is it ok now.

And is there any environment that I can test this file?
I want to see after the execution of this file, is the result displayed ok.

[lp:~liuyq0307/linaro-android-build-tools/add-monkey-l-a-t]

334. By Yongqin Liu on 2011-09-27

modify for undefined error

335. By Yongqin Liu on 2011-09-27

delete parameter result_disk for command "submit_results_on_host"

336. By Yongqin Liu on 2011-09-27

use the latest url for submit_results_on_host

337. By Yongqin Liu on 2011-09-28

make android.build a string instead of integer for lava-dashboard display

[Paul Sokolovsky (pfalcon)]

On Mon, 26 Sep 2011 08:07:25 -0000
Yongqin Liu <email address hidden> wrote:

> > + image_type = os.environ.get("IMAGE_TYPE")
> >
> > What's being done here? If it calls for adding another build config
> > variable, it's likely superfluous (unless there're reasons why it's
> > not).
> sorry, I didn't recognize that this file only be used for
> android-build. I will delete the relevant source.

Thanks.

>
> > Also, braces placement needs fixing, it for sure worse than was
> > before. Rule of thumb is to just follow style of the existing code
> > (and minimize any whitespace changes).
> I have modify the braces placement according the policy that minimize
> any whitespace changes. BTW, is there any coding style for braces?

Well, we generally follow PEP8, though some grounded deviation may be
used. By general rule is the same as for C: everyone places braces in
different way, but when you submit patch for some code, you're expected
to follow the style of surrounding code. If you really think that
formatting should be changed, it should be separate commit - just
change formatting, no semantic changes.

> > And can you please look thru associated blueprint's whiteboard, it
> > seems to have typos which make it a bit unclear ("Acceptance:
> > Monkey for as long as it can", "Update the meeting to a weekly")
> I have updated the bp, is it ok now.
>
> And is there any environment that I can test this file?
> I want to see after the execution of this file, is the result
> displayed ok.

Thanks for bringing that up, that's how we'd like to handle such
changes for sure - test them on a sandbox before deploying on
production. I've created one at
https://ec2-75-101-195-88.compute-1.amazonaws.com/

Reviewing changes once again:

97 - "server": "http://validation.linaro.org/lava-server/dashboard",
98 + "server": "http://validation.linaro.org/lava-server/RPC2/",

Are you sure this is right? Why exactly such change is needed?

--
Best Regards,
Paul

Linaro.org | Open source software for ARM SoCs
Follow Linaro: http://www.facebook.com/pages/Linaro
http://twitter.com/#!/linaroorg - http://www.linaro.org/linaro-blog

[Yongqin Liu (liuyq0307)]

thanks for review.

Thanks for bringing that up, that's how we'd like to handle such
> changes for sure - test them on a sandbox before deploying on
> production. I've created one at
> https://ec2-75-101-195-88.compute-1.amazonaws.com/
>
Thanks, but until now there hasn't been one build:(
How can i make a build there?

> Reviewing changes once again:
>
> 97 - "server": "http://validation.linaro.org/lava-server/dashboard",
> 98 + "server": "http://validation.linaro.org/lava-server/RPC2/",
>
> Are you sure this is right? Why exactly such change is needed?
>

yes, this is right. the dashoard one is deprecated.
and in the future we will use the new url, see the hlep of api about
lava-server.
http://validation.linaro.org/lava-server/api/help/

Thanks,
Yongqin Liu

[Paul Sokolovsky (pfalcon)]

Thanks for clarification and checking that the sandbox now works. So, please test that it works as expected, and I'll merge it then.

review: Approve

=== modified file 'build-scripts/post-build-lava.py'
--- build-scripts/post-build-lava.py	2011-09-15 15:46:49 +0000
+++ build-scripts/post-build-lava.py	2011-09-28 06:15:29 +0000
@@ -3,6 +3,29 @@
 import json
 import xmlrpclib
 
+def gen_lava_android_test_actions(tests=[]):
+    actions = []
+    if len(tests) == 0:
+        return actions
+    inst_action = {
+                      "command": "lava_android_test_install",
+                      "parameters": {
+                                    "tests": tests
+                                        }
+                        }
+    actions.append(inst_action)
+
+    for test in tests:
+        run_action = {
+                      "command": "lava_android_test_run",
+                      "parameters": {
+                                     "test_name": test
+                                         }
+                        }
+        actions.append(run_action)
+    return actions
+
+
 def main():
     """Script entry point: return some JSON based on calling args.
     We should be called from Jenkins and expect the following to 
@@ -19,8 +42,13 @@
     build_url = os.environ.get("BUILD_URL")
     test_plan = os.environ.get("LAVA_TEST_PLAN")
     if test_plan == None:
-        test_plan = "test_android_0xbench"
-    
+        test_plan = '0xbench, monkey'
+    test_plans = test_plan.split(',')
+    for index in range(len(test_plans)):
+        test_plans[index] = test_plans[index].strip()
+        if test_plans[index] == "test_android_0xbench":
+            test_plans[index] = "0xbench"
+
     # Board-specific parameters
     if target_product == "pandaboard":
         # Board type to test on. Validation does the load balancing.
@@ -35,12 +63,10 @@
         print "Don't know how to test this board. Skip testing."
         return
 
-    config = json.dumps( { "job_name": build_url,
- "image_type": "android",
- "device_type": test_target,
- "timeout": 18000,
- "actions": [
-  {
+
+    android_test_actions = gen_lava_android_test_actions(test_plans)
+    actions = [
+    {
     "command": "deploy_linaro_android_image",
     "parameters":
       {
@@ -53,28 +79,35 @@
       },
     "metadata":
       {
-        "android.name": job_name, 
-        "android.build": build_number, 
+        "android.name": job_name,
+        "android.build": '%s' % build_number,
         "android.url": build_url
-      } 
+      }
   },
   {
     "command": "boot_linaro_android_image"
   },
-  {
-    "command": test_plan
-  },
-  {
-    "command": "submit_results",
+]
+
+    actions.extend(android_test_actions)
+
+    actions.append(
+  {
+    "command": "submit_results_on_host",
     "parameters":
       {
-        "result_disk": "sdcard",
-        "server": "http://validation.linaro.org/lava-server/dashboard",
+        "server": "http://validation.linaro.org/lava-server/RPC2/",
         "stream": test_stream
       }
-  }
- ]
-}, indent=4)
+  })
+
+    config = json.dumps({"job_name": build_url,
+                         "image_type": 'android',
+                         "device_type": test_target,
+                         "timeout": 18000,
+                         "actions":actions
+                            },
+                        indent=4)
 
     print config
     lava_user = os.environ.get("LAVA_USER")