update according to the review comments

1 files changed, 65 insertions, 54 deletions

diff --git a/build-scripts/post-build-lava.py b/build-scripts/post-build-lava.py
index a5d0876..9741094 100755
--- a/build-scripts/post-build-lava.py
+++ b/build-scripts/post-build-lava.py
@@ -53,10 +53,35 @@ TIMEOUT_SUFFIX = "_TIMEOUT"
 # reboot in between test actions.
 REBOOT_TOKEN = '[system-reboot]'
 
+# CONSTANTS DEFINITION
+DEFAULT_LAVA_SCHEMA = 'https'
+DEFAULT_LAVA_URL = 'validation.linaro.org/lava-server/RPC2/'
+DEFAULT_LAVA_STREAM = '/private/team/linaro/android-daily/'
+DEFAULT_LAVA_DIR = '/var/run/lava/'
+DEFAULT_LAVA_USER_FILE = '%s/lava-user' % DEFAULT_LAVA_DIR
+DEFAULT_LAVA_TOKEN_FILE = '%s/lava-token' % DEFAULT_LAVA_DIR
+DEFAULT_DOWNLOAD_BASE_URL = 'http://snapshots.linaro.org/android/'
+DEFAULT_TIMEOUT_VALUE = 18000
+
+# Environment variables name
+ENV_LAVA_SERVER = 'LAVA_SERVER'
+ENV_LAVA_DEVICE = 'LAVA_DEVICE'
+ENV_LAVA_DEVICE_TYPE = 'LAVA_DEVICE_TYPE'
+ENV_LAVA_TEST_PLAN = 'LAVA_TEST_PLAN'
+ENV_LAVA_TEST_PLAN_SEC_PRE = 'LAVA_TEST_PLAN_SECONDARY_'
+ENV_LAVA_TOKEN_FILE = 'LAVA_TOKEN_FILE'
+ENV_LAVA_USER = 'LAVA_USER'
+# Build url, defined by android-build, e.g.
+# https://android-build.linaro.org/jenkins/job/linaro-android_leb-panda/61/
+ENV_BUILD_URL = 'BUILD_URL'
+ENV_DEFAULT_TIMEOUT = 'DEFAULT_TIMEOUT'
+# Target product name, user defined, e.g. pandaboard
+ENV_TARGET_PRODUCT = 'TARGET_PRODUCT'
+
 
 class LAVADeviceBase(object):
     """
-    Base class for defination of the device type and target in lava job
+    Base class for definition of the device type and target in lava job.
     """
 
     def __init__(self, name=None):
@@ -65,16 +90,20 @@ class LAVADeviceBase(object):
 
 class LAVADeviceType(LAVADeviceBase):
     """
-    Representation the defination of the device type in lava job
+    Representation the definition of the device type in lava job.
     """
 
 
 class LAVADeviceTarget(LAVADeviceBase):
     """
-    Representation the defination of the device target in lava job
+    Representation the definition of the device target in lava job.
     """
 
 
+def get_env_var(name, default=None):
+    return os.environ.get(name, default)
+
+
 def get_lava_device_type_or_target():
     '''
     Here we support the specification by both,
@@ -84,7 +113,7 @@ def get_lava_device_type_or_target():
     devices = []
 
     # allow to submit to a specific device
-    test_device = os.environ.get("LAVA_DEVICE")
+    test_device = get_env_var(ENV_LAVA_DEVICE)
     if test_device:
         targets = test_device.split(',')
         for dev_target in targets:
@@ -93,15 +122,15 @@ def get_lava_device_type_or_target():
                 devices.append(LAVADeviceTarget(name=dev_target))
 
     # allow overload lava device_type by build config
-    test_device_type = os.environ.get("LAVA_DEVICE_TYPE")
+    test_device_type = get_env_var(ENV_LAVA_DEVICE_TYPE)
 
+    target_product = get_env_var(ENV_TARGET_PRODUCT)
     # only use the default device type when neither of
     # LAVA_DEVICE or LAVA_DEVICE_TYPE is specified.
     # or say we will not use the value when any of
     # LAVA_DEVICE or LAVA_DEVICE_TYPE is specified.
     if (not test_device_type) and (not test_device):
-        test_device_type = PRODUCT_MAP[get_target_product()][
-                                            "test_device_type"]
+        test_device_type = PRODUCT_MAP[target_product]["test_device_type"]
     if test_device_type:
         types = test_device_type.split(',')
         for dev_type in types:
@@ -112,39 +141,25 @@ def get_lava_device_type_or_target():
     return devices
 
 
-def get_build_url():
-    # Build url, defined by android-build, e.g.
-    # https://android-build.linaro.org/jenkins/job/linaro-android_leb-panda/61/
-    build_url = os.environ.get("BUILD_URL")
-
-    return build_url
-
-
-def get_target_product():
-    # Target product name, user defined, e.g. pandaboard
-    target_product = os.environ.get("TARGET_PRODUCT")
-    return target_product
-
-
 def get_schema_and_url():
-    lava_server = os.environ.get("LAVA_SERVER")
-    if lava_server == None:
-        schema = 'https'
-        url_no_schema = "validation.linaro.org/lava-server/RPC2/"
+    lava_server = get_env_var(ENV_LAVA_SERVER)
+    if lava_server is None:
+        schema = DEFAULT_LAVA_SCHEMA
+        url_no_schema = DEFAULT_LAVA_URL
     elif lava_server.find('://') >= 0:
         schema = lava_server[:lava_server.find('://')]
         url_no_schema = lava_server[lava_server.find('://') + len('://'):]
     else:
         ## the case that no https and http specified in the url
         ## like: validation.linaro.org/lava-server/RPC2/
-        schema = 'https'
+        schema = DEFAULT_LAVA_SCHEMA
         url_no_schema = lava_server
     return (schema, url_no_schema)
 
 
 def get_plan_list():
-    plan_list = ["LAVA_TEST_PLAN"]
-    sec_plan_prefix = "LAVA_TEST_PLAN_SECONDARY_"
+    plan_list = [ENV_LAVA_TEST_PLAN]
+    sec_plan_prefix = ENV_LAVA_TEST_PLAN_SEC_PRE
 
     for var in os.environ.keys():
         if var.startswith(sec_plan_prefix):
@@ -156,7 +171,8 @@ def get_plan_list():
 
 def check_target_product():
     # Board-specific parameters
-    if get_target_product() not in PRODUCT_MAP:
+    target_product = get_env_var(ENV_TARGET_PRODUCT)
+    if target_product not in PRODUCT_MAP:
         # We don't know how to test this job, so skip testing.
         print "Don't know how to test this board. Skip testing."
         sys.exit(1)
@@ -443,8 +459,9 @@ def gen_deploy_action():
 
     # download base URL, this may differ from job URL if we don't host
     # downloads in Jenkins any more
-    download_url = "http://snapshots.linaro.org/android/%s/%s/" % \
-                         (frontend_job_name, build_number)
+    download_url = "%s/%s/%s/" % (DEFAULT_DOWNLOAD_BASE_URL,
+                                  frontend_job_name,
+                                  build_number)
 
     # Set the file extension based on the type of artifacts
     artifact_type = os.environ.get("MAKE_TARGETS", "tarball")
@@ -465,7 +482,7 @@ def gen_deploy_action():
               "metadata": {
                     "android.name": job_name,
                     "android.build": '%s' % build_number,
-                    "android.url": get_build_url()
+                    "android.url": get_env_var(ENV_BUILD_URL)
                   }
              }
     return action
@@ -491,19 +508,13 @@ def gen_boot_action():
 def gen_install_binaries_action():
     # User can disable the installation of android binaries (doing this will
     # disable hardware acceleration)
-    enable_android_install_binaries = os.environ.get("LAVA_ANDROID_BINARIES",
-                                                     'false')
+    ENV_LAVA_ANDROID_BINARIES = 'LAVA_ANDROID_BINARIES'
+    enable_binaries = get_env_var(ENV_LAVA_ANDROID_BINARIES, 'false')
+
     # Not set, default to False, because this is relevant only for panda
     # from Vishal
-    if enable_android_install_binaries.lower() in ['1', 'true', 'yes']:
-        enable_android_install_binaries = True
-    else:
-        enable_android_install_binaries = False
-
-    if enable_android_install_binaries:
+    if enable_binaries.lower() in ['1', 'true', 'yes']:
         return {"command": "android_install_binaries"}
-    else:
-        return None
 
 
 def gen_submit_action():
@@ -511,13 +522,12 @@ def gen_submit_action():
     (schema, url_no_schema) = get_schema_and_url()
     schema_url = '%s://%s' % (schema, url_no_schema)
 
-    default_stream = '/private/team/linaro/android-daily/'
-
+    target_product = get_env_var(ENV_TARGET_PRODUCT)
     submit_action = {"command": "submit_results_on_host",
                      "parameters": {
                             "server": schema_url,
-                            "stream": PRODUCT_MAP[get_target_product()].get(
-                              "test_stream", default_stream)
+                            "stream": PRODUCT_MAP[target_product].get(
+                              "test_stream", DEFAULT_LAVA_STREAM)
                       }
                     }
 
@@ -532,9 +542,9 @@ def gen_config(actions=[], device=None):
     # Set the default timeout for all test, also used as the timeout value of
     # the total job when it's value bigger than 24 * 60 * 60
     # if this value is not set, then use the 18000 seconds as the default value
-    default_timeout = os.environ.get("DEFAULT_TIMEOUT", 18000)
+    default_timeout = get_env_var(ENV_DEFAULT_TIMEOUT, DEFAULT_TIMEOUT_VALUE)
 
-    config_json = {"job_name": get_build_url(),
+    config_json = {"job_name": get_env_var(ENV_BUILD_URL),
                    "image_type": 'android',
                    "timeout": int(default_timeout),
                    "actions": actions
@@ -552,25 +562,26 @@ def gen_config(actions=[], device=None):
     print config
     return config
 
+
 def submit_job(config=None, plan=None):
     if (not config) or (not plan):
         print "Config or plan is None"
         return
 
     # get the lava token used for submit job
-    lava_token_f = os.environ.get("LAVA_TOKEN_FILE")
+    lava_token_f = get_env_var(ENV_LAVA_TOKEN_FILE)
     if lava_token_f == None:
-        lava_token_f = '/var/run/lava/lava-token'
+        lava_token_f = DEFAULT_LAVA_TOKEN_FILE
     else:
-        lava_token_f = '/var/run/lava/%s' % lava_token_f
+        lava_token_f = '%s/%s' % (DEFAULT_LAVA_DIR, lava_token_f)
 
     with open(lava_token_f) as fd:
         lava_token = fd.read().strip()
 
     # get the lava user used for submit job
-    lava_user = os.environ.get("LAVA_USER")
+    lava_user = get_env_var(ENV_LAVA_USER)
     if lava_user == None:
-        f = open('/var/run/lava/lava-user')
+        f = open(DEFAULT_LAVA_USER_FILE)
         lava_user = f.read().strip()
         f.close()
 
@@ -628,7 +639,7 @@ def main():
         if plan == "LAVA_TEST_PLAN":
             actions.extend(gen_test_actions())
         else:
-            actions.extend(gen_test_plan_actions(os.environ.get(plan)))
+            actions.extend(gen_test_plan_actions(get_env_var(plan)))
         actions.append(gen_submit_action())
 
         devices = get_lava_device_type_or_target()