Merge lp:~le-chi-thu/linaro-ci/add-testplan-support into lp:linaro-ci

[Le Chi Thu]

Add support to define test plan for lava.

the format is : LAVA_TEST_PLAN=<testcase>(<options>):<timeout>,...
options and timeout is optional.

Examples:
LAVA_TEST_PLAN=ltp,pwrmgmt
LAVA_TEST_PLAN=test_abc(option_abc):100

if the LAVA_TEST_PLAN is not define, the default will be "ltp,pwrmgmt". It mean the current jobs do not need to update.

[Deepti B. Kalakeri (deeptik)]

Thanks for the changes. Please see my comments inline.

On Fri, May 4, 2012 at 7:38 PM, Le Chi Thu <email address hidden> wrote:

> Le Chi Thu has proposed merging
> lp:~le-chi-thu/linaro-ci/add-testplan-support into lp:linaro-ci.
>
> Requested reviews:
> Deepti B. Kalakeri (deeptik)
>
> For more details, see:
>
> https://code.launchpad.net/~le-chi-thu/linaro-ci/add-testplan-support/+merge/104743<https://code.launchpad.net/%7Ele-chi-thu/linaro-ci/add-testplan-support/+merge/104743>
>
> Add support to define test plan for lava.
>
> the format is : LAVA_TEST_PLAN=<testcase>(<options>):<timeout>,...
> options and timeout is optional.
>
> Examples:
> LAVA_TEST_PLAN=ltp,pwrmgmt
> LAVA_TEST_PLAN=test_abc(option_abc):100
>

Could you give me an example of the options_abc ?

>
> if the LAVA_TEST_PLAN is not define, the default will be "ltp,pwrmgmt". It
> mean the current jobs do not need to update.
>

The default test that needs to be run should as well include
boot_linaro_image. Without booting the image how will you be able to run
the tests ?

> --
>
> https://code.launchpad.net/~le-chi-thu/linaro-ci/add-testplan-support/+merge/104743<https://code.launchpad.net/%7Ele-chi-thu/linaro-ci/add-testplan-support/+merge/104743>
> You are requested to review the proposed merge of
> lp:~le-chi-thu/linaro-ci/add-testplan-support into lp:linaro-ci.
>
> === modified file 'get_latest_ci_hwpack'
> --- get_latest_ci_hwpack 2012-04-04 12:14:43 +0000
> +++ get_latest_ci_hwpack 2012-05-04 14:07:20 +0000
> @@ -5,6 +5,7 @@
>
> Usage: ./gentemplate <target> <hwpack_type> <image_type>
> """
> +import re
>
> from find_latest import find_latest_hwpack, find_latest_rootfs,
> find_ci_latest
> import json
> @@ -15,8 +16,7 @@
> job_name = os.getenv("KERNEL_JOB_NAME", "unknown") + '_' + \
> os.getenv("HWPACK_BUILD_DATE", "unknown") + '_daily test'
>
> -
> -
> +testplan = os.getenv("LAVA_TEST_PLAN", "ltp,pwrmgmt")
>
> template = {
> "timeout": 20000,
> @@ -25,36 +25,6 @@
> "command": "deploy_linaro_image"
> },
> {
> - "command": "lava_test_install",
> - "parameters":
> - {
> - "tests": [
> - "ltp",
> - "pwrmgmt"
> - ],
> - "timeout": 4000
> - }
> - },
> - {
> - "command": "boot_linaro_image"
> - },
>

This should not be deleted, as we need to boot the image and then can we
run the tests.

> - {
> - "command": "lava_test_run",
> - "parameters":
> - {
> - "test_name": "ltp",
> - "timeout": 4800
> - }
> - },
> - {
> - "command": "lava_test_run",
> - "parameters":
> - {
> - "test_name": "pwrmgmt",
> - "timeout": 4800
> - }
> - },
> - {
> "command": "submit_results",
> "parameters": {
> "stream": os.getenv("BUNDLE_STREAM_NAME", "/anonymous/plars/"),
> @@ -71,6 +41,9 @@
> hwpack_dir_url=os.getenv("KERNEL_JOB_NAME", "unknown")
> hwpack_url_path = os.path.join(url, hwpack_parent_url)
> hwpack_url_path = os.path.join(hwpack_url_path, hwpack_dir_url)
> +
> +
> +
>

Please delete the blank lines, you can leave a single bla...

[Deepti B. Kalakeri (deeptik)]

review: Needs Fixing

[Le Chi Thu (le-chi-thu)]

ltp take options - see the $OPTIONS macro in the test definition file.
RUNSTEPS = ['cd build && sudo ./runltp $(OPTIONS)']
http://bazaar.launchpad.net/~linaro-validation/lava-test/trunk/view/head:/lava_test/test_definitions/ltp.py

Now the json file is correct generate.

review: Needs Resubmitting

[lp:~le-chi-thu/linaro-ci/add-testplan-support]

62. By Le Chi Thu <email address hidden> <email address hidden> on 2012-05-08

Update after review 1

[Deepti B. Kalakeri (deeptik)]

Thanks for addressing the review, I have few more.

1) Please retain the boot_linaro_image stuff as it was before you made the
changes, since this block does not change, I would like to have it in the
dict itself.

2) I did not see the 80 columns rule being followed, for ex line no: 99
spans to 100 columns.

3) I guess you did not see the following comments in the previous review

(i)
"
>
re.compile("(?P<testcase>\w+)(\((?P<options>[^\)]+)\))?(:(?P<timeout>\d+))?")
>

The regex above should not match first part(:100) of the following
LAVA_TEST_PLAN=":100, test_bcd".
The above regex matches :100 as testcase which is invalid.
"

(ii)

> +
> + if r.group("options"):
> + lava_test_run_command["parameters"]["test_options"] =
> r.group("options")
> +
> + template['actions'].append(lava_test_run_command)
> +
>

It would be good if combined both the for loops into one for loop.

could you please address all of them and resubmit.

On Tue, May 8, 2012 at 5:53 PM, Le Chi Thu <email address hidden> wrote:

> Review: Resubmit
>
> ltp take options - see the $OPTIONS macro in the test definition file.
> RUNSTEPS = ['cd build && sudo ./runltp $(OPTIONS)']
>
> http://bazaar.launchpad.net/~linaro-validation/lava-test/trunk/view/head:/lava_test/test_definitions/ltp.py
>
> Now the json file is correct generate.
> --
>
> https://code.launchpad.net/~le-chi-thu/linaro-ci/add-testplan-support/+merge/104743<https://code.launchpad.net/%7Ele-chi-thu/linaro-ci/add-testplan-support/+merge/104743>
> You are reviewing the proposed merge of
> lp:~le-chi-thu/linaro-ci/add-testplan-support into lp:linaro-ci.
>

--
Thanks and Regards,
Deepti
Infrastructure Team Member, Linaro Platform Teams
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

[Le Chi Thu (le-chi-thu)]

Fixed according to comments.

review: Needs Resubmitting

[lp:~le-chi-thu/linaro-ci/add-testplan-support]

63. By Le Chi Thu <email address hidden> <email address hidden> on 2012-05-09

Updated after review 2

[Deepti B. Kalakeri (deeptik)]

Your changes fail when we pass LAVA_TEST_PLAN="test_abc(option_abc):100, test_bcd":
python ./get_latest_ci_hwpack
Traceback (most recent call last):
  File "./get_latest_ci_hwpack", line 94, in <module>
    lava_test_install_command["parameters"]["tests"].append(r.group("testcase"))
AttributeError: 'NoneType' object has no attribute 'group'

The following changes fix the above problem and includes other changes like aligning to 80 columns rule.
=== modified file 'get_latest_ci_hwpack'
--- get_latest_ci_hwpack 2012-05-09 12:01:28 +0000
+++ get_latest_ci_hwpack 2012-05-14 08:49:26 +0000
@@ -16,7 +16,7 @@
 job_name = os.getenv("KERNEL_JOB_NAME", "unknown") + '_' + \
            os.getenv("HWPACK_BUILD_DATE", "unknown") + '_daily test'

-testplan = os.getenv("LAVA_TEST_PLAN", "ltp,pwrmgmt")
+testplan = os.getenv("LAVA_TEST_PLAN", "ltp, pwrmgmt")

 template = {
   "timeout": 20000,
@@ -66,50 +66,48 @@
 metadata['kernel.git_log_info'] = os.getenv("GIT_LOG", "unknown")
 metadata['hwpack.type'] = hwpacktype
 metadata['hwpack.date'] = hwpackdate
-
-
 rootfsdate, rootfsbuild = rootfs_id.split('.')
 metadata['rootfs.type'] = imagetype
 metadata['rootfs.date'] = rootfsdate
 metadata['rootfs.build'] = rootfsbuild
-
 template['actions'][0]['metadata'] = metadata

 testcases = testplan.split(',')
-
-regex = re.compile("(?P<testcase>^\w+)(\((?P<options>[^\)]+)\))?(:(?P<timeout>\d+))?")
-
-# lava_test_install command
-
+testcases = [tc.strip() for tc in testplan.split(',')]
+regex = re.compile("(?P<testcase>^\w+)(\((?P<options>[^\)]+)\))?"\
+ "(:(?P<timeout>\d+))?")
+
+# Dict for lava_test_install
 lava_test_install_command = {}
 lava_test_install_command["command"] = "lava_test_install"
 lava_test_install_command["parameters"] = { "tests" : [], "timeout" : 4000 }

-# where in the template dict we shall insert test commands
+# The positions in the template dict to insert test install/run commands
 install_cmd_pos = 1
 run_cmd_pos = 2

 for testcase in testcases:
     r = regex.search(testcase)
- lava_test_install_command["parameters"]["tests"].append(r.group("testcase"))
-
- # append lava_test_run commands
- timeout = 4800
- if r.group('timeout'):
- timeout = r.group('timeout')
-
- lava_test_run_command = {}
- lava_test_run_command["command"] = "lava_test_run"
- lava_test_run_command["parameters"] = { "test_name" : r.group('testcase'),
- "timeout" : timeout }
-
- if r.group("options"):
- lava_test_run_command["parameters"]["test_options"] = r.group("options")
-
- template['actions'].insert(run_cmd_pos, lava_test_run_command)
- run_cmd_pos = run_cmd_pos + 1
+ if r.group("testcase") != None:
+ lava_test_install_command["parameters"]["tests"].\
+ append(r.group("testcase"))
+
+ # append lava_test_run commands
+ timeout = 4800
+ if r.group('timeout'):
+ timeout = r.group('timeout')
+
+ lava_test_run_command = {}
+ lava_test_run_command["command"] = "lava_test_run"
+ lava_test_run_command["parameters"] = {"test_name" : r.group('testcase'),
+ ...

review: Needs Fixing

[lp:~le-chi-thu/linaro-ci/add-testplan-support]

64. By Le Chi Thu <email address hidden> <email address hidden> on 2012-05-14

Updated after review 3

[Le Chi Thu (le-chi-thu)]

review: Needs Resubmitting

[Deepti B. Kalakeri (deeptik)]

The changes looks ok. Thanks a lot.

review: Needs Fixing

[Deepti B. Kalakeri (deeptik)]

review: Approve

=== modified file 'get_latest_ci_hwpack'
--- get_latest_ci_hwpack	2012-04-04 12:14:43 +0000
+++ get_latest_ci_hwpack	2012-05-14 10:48:21 +0000
@@ -5,6 +5,7 @@
 
 Usage: ./gentemplate <target> <hwpack_type> <image_type>
 """
+import re
 
 from find_latest import find_latest_hwpack, find_latest_rootfs, find_ci_latest
 import json
@@ -15,8 +16,7 @@
 job_name = os.getenv("KERNEL_JOB_NAME", "unknown") + '_' + \
            os.getenv("HWPACK_BUILD_DATE", "unknown") + '_daily test'
 
-
-
+testplan = os.getenv("LAVA_TEST_PLAN", "ltp, pwrmgmt")
 
 template = {
   "timeout": 20000,
@@ -25,36 +25,9 @@
       "command": "deploy_linaro_image"
     },
     {
-      "command": "lava_test_install",
-      "parameters":
-        {
-            "tests": [
-                "ltp",
-                "pwrmgmt"
-            ],
-            "timeout": 4000
-        }
-    },
-    {
       "command": "boot_linaro_image"
     },
     {
-      "command": "lava_test_run",
-      "parameters":
-        {
-          "test_name": "ltp",
-          "timeout": 4800
-        }
-    },
-    {
-      "command": "lava_test_run",
-      "parameters":
-        {
-          "test_name": "pwrmgmt",
-          "timeout": 4800
-        }
-    },
-    {
       "command": "submit_results",
       "parameters": {
         "stream": os.getenv("BUNDLE_STREAM_NAME", "/anonymous/plars/"),
@@ -63,7 +36,7 @@
     }
   ],
   "job_name": job_name,
-  "device_type": os.getenv("BOARD_TYPE", "unknown") 
+  "device_type": os.getenv("BOARD_TYPE", "unknown")
 }
 
 url=os.getenv("JOB_URL", "http://snapshots.linaro.org/kernel-hwpack/")
@@ -71,7 +44,8 @@
 hwpack_dir_url=os.getenv("KERNEL_JOB_NAME", "unknown")
 hwpack_url_path = os.path.join(url, hwpack_parent_url)
 hwpack_url_path = os.path.join(hwpack_url_path, hwpack_dir_url)
-hwpack_url = find_ci_latest(hwpack_url_path) 
+
+hwpack_url = find_ci_latest(hwpack_url_path)
 rootfs_id, rootfs_url = find_latest_rootfs(imagetype)
 
 deploy_params = { 'hwpack':hwpack_url, 'rootfs':rootfs_url }
@@ -92,13 +66,48 @@
 metadata['kernel.git_log_info'] = os.getenv("GIT_LOG", "unknown")
 metadata['hwpack.type'] = hwpacktype
 metadata['hwpack.date'] = hwpackdate
-
-
 rootfsdate, rootfsbuild = rootfs_id.split('.')
 metadata['rootfs.type'] = imagetype
 metadata['rootfs.date'] = rootfsdate
 metadata['rootfs.build'] = rootfsbuild
-
 template['actions'][0]['metadata'] = metadata
 
+testcases = testplan.split(',')
+testcases = [tc.strip() for tc in testplan.split(',')]
+regex = re.compile("(?P<testcase>^\w+)(\((?P<options>[^\)]+)\))?"\
+                   "(:(?P<timeout>\d+))?")
+
+# Dict for lava_test_install 
+lava_test_install_command = {}
+lava_test_install_command["command"] = "lava_test_install"
+lava_test_install_command["parameters"] = { "tests" : [], "timeout" : 4000 }
+
+# The positions in the template dict to insert test install/run commands
+install_cmd_pos = 1
+run_cmd_pos = 2
+
+for testcase in testcases:
+    r = regex.search(testcase)
+    if r.group("testcase") != None:
+        lava_test_install_command["parameters"]["tests"].\
+                                 append(r.group("testcase"))
+
+        # append lava_test_run commands
+        timeout = 4800
+        if r.group('timeout'):
+         timeout = r.group('timeout')
+
+        lava_test_run_command = {}
+        lava_test_run_command["command"] = "lava_test_run"
+        lava_test_run_command["parameters"] = {"test_name" : r.group('testcase'),
+                                               "timeout" : timeout }
+
+        if r.group("options"):
+            lava_test_run_command["parameters"]["test_options"] = r.group("options")
+
+        template['actions'].insert(run_cmd_pos, lava_test_run_command)
+        run_cmd_pos = run_cmd_pos + 1
+
+# append the lava test install command
+template['actions'].insert(install_cmd_pos, lava_test_install_command)
 print json.dumps(template, indent=2)