[Fuego] [PATCH] ftc: improve test class and defaults handling

Daniel Sangorrin daniel.sangorrin at toshiba.co.jp
Wed Jul 25 05:55:37 UTC 2018


Sorry, I missed a line when preparing the patch.
I just resend it.

> -----Original Message-----
> From: fuego-bounces at lists.linuxfoundation.org
> <fuego-bounces at lists.linuxfoundation.org> On Behalf Of Daniel Sangorrin
> Sent: Wednesday, July 25, 2018 2:24 PM
> To: fuego at lists.linuxfoundation.org
> Subject: [Fuego] [PATCH] ftc: improve test class and defaults handling
> 
> Rename plantest_class to test_class because it represents
> any test, not just a test in a testplan.
> 
> Move the test flags default values to the test_class to
> have them shared (see DEFAULT_DEFAULTS).
> 
> Enforce the priority order of test flags (timeout, reboot, etc)
> to satisfy the following:
> commandline flags > per-test flags > default_xxx flags > DEFAULT_DEFAULTS
> 
> Signed-off-by: Daniel Sangorrin <daniel.sangorrin at toshiba.co.jp>
> ---
>  engine/scripts/ftc | 170
> ++++++++++++++++++++++++++++++-----------------------
>  1 file changed, 95 insertions(+), 75 deletions(-)
> 
> diff --git a/engine/scripts/ftc b/engine/scripts/ftc
> index cc1556d..fb07c1d 100755
> --- a/engine/scripts/ftc
> +++ b/engine/scripts/ftc
> @@ -610,17 +610,28 @@ class board_class:
>      # FIXTHIS - board_class should have methods to read board and dist file
> 
> 
> -class plantest_class:
> -    def __init__(self, test_dict, defaults):
> +class test_class:
> +    DEFAULT_DEFAULTS = {
> +        'timeout' : '30m',
> +        'spec'    : 'default',
> +        'reboot'  : 'false',
> +        'rebuild' : 'false',
> +        'precleanup'  : 'true',
> +        'postcleanup' : 'true'
> +    }
> +    def __init__(self, test_dict, test_flags={}):
> +        merged_defaults = dict(self.DEFAULT_DEFAULTS)
> +        for key, value in test_flags.iteritems():
> +            merged_defaults[key] = value
>          self.name = str(test_dict["testName"])
>          self.test_type = self.name.split(".")[0]
>          self.base_name = self.name.split(".")[1]
> -        self.spec = str(test_dict.get("spec", defaults['spec']))
> -        self.timeout = str(test_dict.get("timeout", defaults['timeout']))
> -        self.reboot = str(test_dict.get("reboot", defaults['reboot']))
> -        self.rebuild = str(test_dict.get("rebuild", defaults['rebuild']))
> -        self.precleanup = str(test_dict.get("precleanup", defaults['precleanup']))
> -        self.postcleanup = str(test_dict.get("postcleanup",
> defaults['postcleanup']))
> +        self.spec = str(test_dict.get("spec", merged_defaults['spec']))
> +        self.timeout = str(test_dict.get("timeout", merged_defaults['timeout']))
> +        self.reboot = str(test_dict.get("reboot", merged_defaults['reboot']))
> +        self.rebuild = str(test_dict.get("rebuild", merged_defaults['rebuild']))
> +        self.precleanup = str(test_dict.get("precleanup",
> merged_defaults['precleanup']))
> +        self.postcleanup = str(test_dict.get("postcleanup",
> merged_defaults['postcleanup']))
> 
> 
>  class run_class:
> @@ -1371,42 +1382,54 @@ def create_batch_job(board, testplan, plan_tests):
>          sys.exit(1)
> 
> 
> -# returns a list of plantest_class instances
> -def parse_testplan(testplan, defaults):
> +# parse the testplan and returns a list of test_class instances where
> +# test flags (timeout, reboot, etc) have been merged in this order
> +# commandline flags > per-test flags > default_xxx flags > DEFAULT_DEFAULTS
> +def parse_testplan(testplan, test_dict):
>      abspath = '/fuego-core/engine/overlays/testplans/' + testplan + '.json'
> 
> -    plan_tests = []
>      with open(abspath, "r") as f:
>          plan = json.load(f)
> 
> -    if 'default_timeout' in plan:
> -        defaults['timeout'] = plan['default_timeout']
> +    plan_tests = []
> +    for plan_test_dict in plan['tests']:
> +        # set testplan flags
> +        test_flags = dict()
> +        if 'default_timeout' in plan:
> +            test_flags['timeout'] = plan['default_timeout']
> 
> -    if 'default_spec' in plan:
> -        defaults['spec'] = plan['default_spec']
> +        if 'default_spec' in plan:
> +            test_flags['spec'] = plan['default_spec']
> 
> -    if 'default_reboot' in plan:
> -        defaults['reboot'] = plan['default_reboot']
> +        if 'default_reboot' in plan:
> +            test_flags['reboot'] = plan['default_reboot']
> 
> -    if 'default_rebuild' in plan:
> -        defaults['rebuild'] = plan['default_rebuild']
> +        if 'default_rebuild' in plan:
> +            test_flags['rebuild'] = plan['default_rebuild']
> 
> -    if 'default_precleanup' in plan:
> -        defaults['precleanup'] = plan['default_precleanup']
> +        if 'default_precleanup' in plan:
> +            test_flags['precleanup'] = plan['default_precleanup']
> 
> -    if 'default_postcleanup' in plan:
> -        defaults['postcleanup'] = plan['default_postcleanup']
> +        if 'default_postcleanup' in plan:
> +            test_flags['postcleanup'] = plan['default_postcleanup']
> 
> +        # override with testplan per-test flags
> +        for key, value in plan_test_dict.iteritems():
> +            if key == "testName":
> +                test_dict[key] = value
> +                continue
> +            test_flags[key] = value
> 
> -    for test_dict in plan['tests']:
> -        # test_dict is a dictionary with values for the test
> -        test = plantest_class(test_dict, defaults)
> +        # test_class overrides flags with those in test_dict and applies
> +        # DEFAULT_DEFAULTS for non-specified flags
> +        test = test_class(test_dict, test_flags)
>          plan_tests.append(test)
> 
>      return plan_tests
> 
> 
>  def do_add_jobs(conf, options):
> +    test_dict = {}
>      if '-b' in options:
>          try:
>              board = options[options.index('-b') + 1]
> @@ -1414,16 +1437,15 @@ def do_add_jobs(conf, options):
>              error_out("Board not provided after -b.")
>          options.remove('-b')
>          options.remove(board)
> -
> +        test_dict["board"] = board
>          boards = board.split(",")
>          board_list = get_fuego_boards(conf).keys()
>          for board in boards:
>              if board not in board_list:
> -                raise Exception("Board '%s' not found." % board)
> +                error_out("Board '%s' not found." % board)
>      else:
> -        raise Exception("No board name supplied.")
> +        error_out("No board name supplied.")
> 
> -    rebuild = ''
>      if '--rebuild' in options:
>          try:
>              rebuild = options[options.index('--rebuild') + 1]
> @@ -1431,9 +1453,13 @@ def do_add_jobs(conf, options):
>              error_out('Rebuild option not provided after --rebuild')
>          if rebuild not in ['true', 'false']:
>              error_out("Invalid rebuild option '%s'" % rebuild)
> +        options.remove(rebuild)
> +        options.remove('--rebuild')
> +        test_dict["rebuild"] = rebuild
> 
> -    timeout = '30m'
> -    if '-p' in options:
> +    if '-p' in options and '-t' in options:
> +        error_out('Testplan (-p) and test (-t) options are mutually exclusive.')
> +    elif '-p' in options:
>          try:
>              testplan = options[options.index('-p') + 1]
>          except IndexError:
> @@ -1447,52 +1473,43 @@ def do_add_jobs(conf, options):
>      elif '-t' in options:
>          # FIXTHIS: have add-jobs support wildcards in the test name
>          test_name, options = get_test_arg("Add-jobs", conf, options)
> -        if '-s' in options:
> -            try:
> -                spec = options[options.index('-s') + 1]
> -            except IndexError:
> -                error_out('Testspec not provided after -s.')
> -            specnames = get_specs(conf, test_name)
> -            if spec not in specnames:
> -                error_out('Unknown spec %s' % spec)
> -            options.remove('-s')
> -            options.remove(spec)
> -        else:
> -            spec = 'default'
> -        if '-k' in options:
> -            try:
> -                timeout = options[options.index('-k') + 1]
> -                options.remove('-k')
> -            except IndexError:
> -                error_out('No timeout specified after -k')
> -
> +        test_dict["testName"] = test_name
>      else:
> -        error_out('No testplan or testcase supplied.')
> +        error_out('No testplan (-p) or test (-t) supplied.')
> 
> -    defaults = {
> -        'timeout' : '30m',
> -        'spec'    : 'default',
> -        'reboot'  : 'false',
> -        'rebuild' : 'false',
> -        'precleanup'  : 'true',
> -        'postcleanup' : 'true'
> -    }
> +    if '-s' in options:
> +        if test_name is None:
> +            error_out("-s option requires the test option (-t) to be set")
> +        try:
> +            spec = options[options.index('-s') + 1]
> +        except IndexError:
> +            error_out('Testspec not provided after -s.')
> +        specnames = get_specs(conf, test_name)
> +        if spec not in specnames:
> +            error_out('Unknown spec %s' % spec)
> +        options.remove('-s')
> +        options.remove(spec)
> +        test_dict["spec"] = spec
> +
> +    if '-k' in options:
> +        try:
> +            timeout = options[options.index('-k') + 1]
> +        except IndexError:
> +            error_out('No timeout specified after -k')
> +        if re.match('^\d+[dhms]', timeout) is None:
> +            error_out('%s: Timeout format not supported.' % timeout)
> +        options.remove('-k')
> +        options.remove(timeout)
> +        test_dict["timeout"] = timeout
> 
>      if test_name:
> -        # FIXTHIS - we could parse more parameters for the job here, from the ftc
> command line
> -        # use all defaults, except for the spec
> -        tp_dict = {"testName": test_name, "timeout": timeout, "spec": spec }
> -        if rebuild:
> -            tp_dict["rebuild"] = rebuild
> -        test = plantest_class(tp_dict, defaults)
> +        test = test_class(test_dict)
>          for board in boards:
>              create_job(board, test)
>      else:
> -        plan_tests = parse_testplan(testplan, defaults)
> +        plan_tests = parse_testplan(testplan, test_dict)
>          for board in boards:
>              for test in plan_tests:
> -                if rebuild:
> -                    test.rebuild = rebuild
>                  create_job(board, test)
>              create_batch_job(board, testplan, plan_tests)
>      sys.exit(0)
> @@ -3102,10 +3119,14 @@ def do_run_test(conf, options):
>      board, options = get_board_arg("Run-test", conf, options)
>      board_name = board.name
> 
> -    test_name = None
> +    test_dict = {}
> +
>      if '-t' in options:
>          # FIXTHIS: have run-test support wildcards in the test name
>          test_name, options = get_test_arg("Run-test", conf, options)
> +        test_dict["testName"] = test_name
> +    else:
> +        error_out("Run-test command requires a test name")
> 
>      if '-s' in options:
>          try:
> @@ -3117,8 +3138,7 @@ def do_run_test(conf, options):
>              error_out('Unknown spec %s' % spec_name)
>          options.remove(spec_name)
>          options.remove('-s')
> -    else:
> -        spec_name = 'default'
> +        test_dict["spec"] = spec_name
> 
>      if '-p' in options:
>          phase_map = { "p": "pre_test",
> @@ -3153,7 +3173,7 @@ def do_run_test(conf, options):
>          fuego_caller = "ftc"
>          print "Notice: non-Jenkins test request detected"
> 
> -    print "Running test '%s' on board '%s' using spec '%s'" % (test_name,
> board_name, spec_name)
> +    print "Running test '%s' on board '%s' using spec '%s'" % (test_name,
> board_name, test.spec)
> 
>      # check if there's a Jenkins job that matches this request
>      # Job names have the syntax: <board>.<spec>.<test_name>
> @@ -3181,9 +3201,9 @@ def do_run_test(conf, options):
>      build_data.board_name = board_name
>      build_data.test_name = test_name
>      build_data.testplan_name = "None"
> -    build_data.spec_name = spec_name
> +    build_data.spec_name = test.spec
>      build_data.testdir = test_name
> -    build_data.job_name = job_name
> +    build_data.job_name = "%s.%s.%s" % (board_name, test.spec, test_name)
>      build_data.workspace = conf.FUEGO_RW+"/buildzone"
>      build_data.start_time = long(time.time() * 1000)
> 
> --
> 2.7.4
> 
> _______________________________________________
> Fuego mailing list
> Fuego at lists.linuxfoundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/fuego



More information about the Fuego mailing list