Restructure test_setup.py according to review

test/integrationtests/voight_kampff/test_setup.py

@@ -15,14 +15,12 @@
 import argparse
 from argparse import RawTextHelpFormatter
 from glob import glob
-from os.path import join, dirname, expanduser, basename
-from pathlib import Path
+from os.path import join, dirname, expanduser, basename, exists
 from random import shuffle
 import shutil
 import sys
 
 import yaml
-
 from msm import MycroftSkillsManager
 from msm.exceptions import MsmException
 
@@ -54,7 +52,7 @@ def copy_step_files(source, destination):
         shutil.copyfile(f, join(destination, basename(f)))
 
 
-def load_config(config, args):
+def apply_config(config, args):
     """Load config and add to unset arguments."""
     with open(expanduser(config)) as f:
         conf_dict = yaml.safe_load(f)
@@ -65,47 +63,57 @@ def load_config(config, args):
         args.extra_skills = conf_dict['extra_skills']
     if not args.platform and 'platform' in conf_dict:
         args.platform = conf_dict['platform']
-    return
 
 
-def main(cmdline_args):
-    """Parse arguments and run environment setup."""
+def create_argument_parser():
+    """Create the argument parser for the command line options.
+
+    Returns: ArgumentParser
+    """
+    class TestSkillAction(argparse.Action):
+        def __call__(self, parser, args, values, option_string=None):
+            args.test_skills = values.replace(',', ' ').split()
+
+    class ExtraSkillAction(argparse.Action):
+        def __call__(self, parser, args, values, option_string=None):
+            args.extra_skills = values.replace(',', ' ').split()
+
     platforms = list(MycroftSkillsManager.SKILL_GROUPS)
     parser = argparse.ArgumentParser(formatter_class=RawTextHelpFormatter)
-    parser.add_argument('-p', '--platform', choices=platforms)
+    parser.add_argument('-p', '--platform', choices=platforms,
+                        default='mycroft_mark_1')
     parser.add_argument('-t', '--test-skills', default=[],
+                        action=TestSkillAction,
                         help=('Comma-separated list of skills to test.\n'
                               'Ex: "mycroft-weather, mycroft-stock"'))
     parser.add_argument('-s', '--extra-skills', default=[],
+                        action=ExtraSkillAction,
                         help=('Comma-separated list of extra skills to '
                               'install.\n'
                               'Ex: "cocktails, laugh"'))
-    parser.add_argument('-r', '--random-skills', default=0,
+    parser.add_argument('-r', '--random-skills', default=0, type=int,
                         help='Number of random skills to install.')
     parser.add_argument('-d', '--skills-dir')
     parser.add_argument('-c', '--config',
                         help='Path to test configuration file.')
-
-    args = parser.parse_args(cmdline_args)
-    if args.test_skills:
-        args.test_skills = args.test_skills.replace(',', ' ').split()
-    if args.extra_skills:
-        args.extra_skills = args.extra_skills.replace(',', ' ').split()
-
-    if args.config:
-        load_config(args.config, args)
-
-    if args.platform is None:
-        args.platform = "mycroft_mark_1"
-
-    msm = MycroftSkillsManager(args.platform, args.skills_dir)
-    run_setup(msm, args.test_skills, args.extra_skills, args.random_skills)
+    return parser
 
 
-def run_setup(msm, test_skills, extra_skills, num_random_skills):
-    """Install needed skills and collect feature files for the test."""
-    skills = [msm.find_skill(s) for s in test_skills + extra_skills]
-    # Install test skills
+def get_random_skills(msm, num_random_skills):
+    """Install random skills from uninstalled skill list."""
+    random_skills = [s for s in msm.all_skills if not s.is_local]
+    shuffle(random_skills)  # Make them random
+    return [s.name for s in random_skills[:num_random_skills]]
+
+
+def install_or_upgrade_skills(msm, skills):
+    """Install needed skills if uninstalled, otherwise try to update.
+
+    Arguments:
+        msm: msm instance to use for the operations
+        skills: list of skills
+    """
+    skills = [msm.find_skill(s) for s in skills]
     for s in skills:
         if not s.is_local:
             print('Installing {}'.format(s))
@@ -116,15 +124,22 @@ def run_setup(msm, test_skills, extra_skills, num_random_skills):
             except MsmException:
                 pass
 
-    # collect feature files
-    for skill_name in test_skills:
+
+def collect_test_cases(msm, skills):
+    """Collect feature files and step files for each skill.
+
+    Arguments:
+        msm: msm instance to use for the operations
+        skills: list of skills
+    """
+    for skill_name in skills:
         skill = msm.find_skill(skill_name)
         behave_dir = join(skill.path, 'test', 'behave')
-        if Path(behave_dir).exists():
+        if exists(behave_dir):
             copy_feature_files(behave_dir, FEATURE_DIR)
 
             step_dir = join(behave_dir, 'steps')
-            if Path().exists():
+            if exists(step_dir):
                 copy_step_files(step_dir, join(FEATURE_DIR, 'steps'))
         else:
             # Generate feature file if no data exists yet
@@ -135,17 +150,6 @@ def run_setup(msm, test_skills, extra_skills, num_random_skills):
             with open(join(FEATURE_DIR, skill_name + '.feature'), 'w') as f:
                 f.write(feature)
 
-    # Install random skills from uninstalled skill list
-    random_skills = [s for s in msm.all_skills if s not in msm.local_skills]
-    shuffle(random_skills)  # Make them random
-    random_skills = random_skills[:num_random_skills]
-    for s in random_skills:
-        msm.install(s)
-
-    print_install_report(msm.platform, test_skills,
-                         extra_skills + [s.name for s in random_skills])
-    return
-
 
 def print_install_report(platform, test_skills, extra_skills):
     """Print in nice format."""
@@ -159,5 +163,43 @@ def print_install_report(platform, test_skills, extra_skills):
     print('----------------------------')
 
 
+def get_arguments(cmdline_args):
+    """Get arguments for test setup.
+
+    Parses the commandline and if specified applies configuration file.
+
+    Arguments:
+        cmdline_args (list): argv like list of arguments
+
+    Returns:
+        Argument parser NameSpace
+    """
+    parser = create_argument_parser()
+    args = parser.parse_args(cmdline_args)
+    return args
+
+
+def main(cmdline_args):
+    """Parse arguments and run test environment setup.
+
+    This installs and/or upgrades any skills needed for the tests and
+    collects the feature and step files for the skills.
+    """
+    args = get_arguments(cmdline_args)
+    if args.config:
+        apply_config(args.config, args)
+
+    msm = MycroftSkillsManager(args.platform, args.skills_dir)
+
+    random_skills = get_random_skills(msm, args.random_skills)
+    all_skills = args.test_skills + args.extra_skills + random_skills
+
+    install_or_upgrade_skills(msm, all_skills)
+    collect_test_cases(msm, args.test_skills)
+
+    print_install_report(msm.platform, args.test_skills,
+                         args.extra_skills + random_skills)
+
+
 if __name__ == '__main__':
     main(sys.argv[1:])