* [Fuego] Dynamic variables
@ 2018-09-04 10:01 Daniel Sangorrin
2018-09-04 10:01 ` [Fuego] [PATCH 1/3] of: use logdir argument instead of prolog.sh Daniel Sangorrin
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Daniel Sangorrin @ 2018-09-04 10:01 UTC (permalink / raw)
To: fuego
Hello Tim,
These patches add dynamic vars to ftc run-test so that one
can easily test variations of a base spec by overriding its
values. Although prolog.sh contains the values used by the
run, I also store a spec.json file on the log directory
for each run. I think this is cleaner and easier to
read than having to dig in prolog.sh
[PATCH 1/3] of: use logdir argument instead of prolog.sh
[PATCH 2/3] OF: create a spec.json on each run
[PATCH 3/3] ftc: add dynamic-vars to ftc run-test
Thanks,
Daniel
^ permalink raw reply [flat|nested] 9+ messages in thread* [Fuego] [PATCH 1/3] of: use logdir argument instead of prolog.sh 2018-09-04 10:01 [Fuego] Dynamic variables Daniel Sangorrin @ 2018-09-04 10:01 ` Daniel Sangorrin 2018-09-07 18:25 ` Tim.Bird 2018-09-04 10:01 ` [Fuego] [PATCH 2/3] OF: create a spec.json on each run Daniel Sangorrin 2018-09-04 10:01 ` [Fuego] [PATCH 3/3] ftc: add dynamic-vars to ftc run-test Daniel Sangorrin 2 siblings, 1 reply; 9+ messages in thread From: Daniel Sangorrin @ 2018-09-04 10:01 UTC (permalink / raw) To: fuego This patch does not change the behavior of the program, it still generates a prolog.sh in the current job's log directory. It only prepares for the ability to leave other files (such as a dynamically generated spec.json) in the log directory. Signed-off-by: Daniel Sangorrin <daniel.sangorrin@toshiba.co.jp> --- engine/scripts/overlays.sh | 2 +- engine/scripts/ovgen.py | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/engine/scripts/overlays.sh b/engine/scripts/overlays.sh index bbc5d16..9e3eb55 100644 --- a/engine/scripts/overlays.sh +++ b/engine/scripts/overlays.sh @@ -73,7 +73,7 @@ function set_overlay_vars() { rm -f $LOGDIR/prolog.sh - run_python $OF_OVGEN $OF_DEBUG_ARGS --classdir $OF_CLASSDIRS --ovfiles $OF_DISTRIB_FILE $OF_BOARD_FILE $OF_BOARD_VAR_FILE --testdir $TESTDIR --testspec $TESTSPEC --output $LOGDIR/prolog.sh || abort_job "Error while prolog.sh file generation" + run_python $OF_OVGEN $OF_DEBUG_ARGS --classdir $OF_CLASSDIRS --ovfiles $OF_DISTRIB_FILE $OF_BOARD_FILE $OF_BOARD_VAR_FILE --testdir $TESTDIR --testspec $TESTSPEC --logdir $LOGDIR || abort_job "Error while prolog.sh file generation" if [ ! -f "$LOGDIR/prolog.sh" ] then diff --git a/engine/scripts/ovgen.py b/engine/scripts/ovgen.py index 98cdf82..7c1f05f 100755 --- a/engine/scripts/ovgen.py +++ b/engine/scripts/ovgen.py @@ -366,7 +366,8 @@ def parseOverrideFile(overrideFile, layer, ofcls): return classes -def generateProlog(outFilePath, ofcls, classes, testdir, testspec): +def generateProlog(logdir, ofcls, classes, testdir, testspec): + outFilePath = logdir + '/prolog.sh' outfile = open(outFilePath, "w") for ofc in classes: @@ -402,7 +403,6 @@ def generateProlog(outFilePath, ofcls, classes, testdir, testspec): # generateSpec - generate shell output for the spec # ts:TestSpecs - parsed specs for our testdir # testspec: String - name of test spec (e.g. 'default') -# outFilePath - file output descriptor opened -> prolog.sh def generateSpec(ts, fout): debug_print("generating spec %s" % (ts.name)) @@ -470,7 +470,7 @@ def run(test_args=None): parser.add_argument('--ovfiles', nargs='+', metavar='OVFILE', help='list of directories containing .override files', required=True) parser.add_argument('--testdir', help='e.g.: Benchmark.Dhrystone', required=True) parser.add_argument('--testspec', nargs='+', metavar='TESTSPEC', help='testspec', required=True) - parser.add_argument('--output', help='output file name', required=True) + parser.add_argument('--logdir', help='log dir', required=True) parser.add_argument('--debug', help='{1,2,3} debug level (default is no debugging)', type=int, required=False) args = parser.parse_args(args=test_args) @@ -493,10 +493,10 @@ def run(test_args=None): classes = classes + parseOverrideFile(ovf, layers, ofcls) debug_print ("parsed %s override\n------------\n" % (ovf)) - generateProlog(args.output, ofcls, classes, args.testdir, args.testspec[0]) + generateProlog(args.logdir, ofcls, classes, args.testdir, args.testspec[0]) def testrun(): - test_args = "--debug 2 --classdir /fuego-core/engine/overlays/base/ --ovfiles /fuego-ro/boards/qemu-arm.board --testdir Benchmark.Dhrystone --testspec default --output prolog.sh".split() + test_args = "--debug 2 --classdir /fuego-core/engine/overlays/base/ --ovfiles /fuego-ro/boards/qemu-arm.board --testdir Benchmark.Dhrystone --testspec default --output ./".split() run(test_args) run() -- 2.7.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [Fuego] [PATCH 1/3] of: use logdir argument instead of prolog.sh 2018-09-04 10:01 ` [Fuego] [PATCH 1/3] of: use logdir argument instead of prolog.sh Daniel Sangorrin @ 2018-09-07 18:25 ` Tim.Bird 0 siblings, 0 replies; 9+ messages in thread From: Tim.Bird @ 2018-09-07 18:25 UTC (permalink / raw) To: daniel.sangorrin, fuego > -----Original Message----- > From: Daniel Sangorrin > > This patch does not change the behavior of the program, it > still generates a prolog.sh in the current job's log > directory. It only prepares for the ability to leave > other files (such as a dynamically generated spec.json) > in the log directory. > > Signed-off-by: Daniel Sangorrin <daniel.sangorrin@toshiba.co.jp> > --- > engine/scripts/overlays.sh | 2 +- > engine/scripts/ovgen.py | 10 +++++----- > 2 files changed, 6 insertions(+), 6 deletions(-) > > diff --git a/engine/scripts/overlays.sh b/engine/scripts/overlays.sh > index bbc5d16..9e3eb55 100644 > --- a/engine/scripts/overlays.sh > +++ b/engine/scripts/overlays.sh > @@ -73,7 +73,7 @@ function set_overlay_vars() { > > rm -f $LOGDIR/prolog.sh > > - run_python $OF_OVGEN $OF_DEBUG_ARGS --classdir $OF_CLASSDIRS -- > ovfiles $OF_DISTRIB_FILE $OF_BOARD_FILE $OF_BOARD_VAR_FILE --testdir > $TESTDIR --testspec $TESTSPEC --output $LOGDIR/prolog.sh || abort_job > "Error while prolog.sh file generation" > + run_python $OF_OVGEN $OF_DEBUG_ARGS --classdir $OF_CLASSDIRS -- > ovfiles $OF_DISTRIB_FILE $OF_BOARD_FILE $OF_BOARD_VAR_FILE --testdir > $TESTDIR --testspec $TESTSPEC --logdir $LOGDIR || abort_job "Error while > prolog.sh file generation" There are a few issues that I'm thinking about here: 1) this means the name "prolog.sh" is now hardcoded into the overlay generator. I don't know if that's a problem or not. I can't think of any use cases where a user of the overlay generator would want to use a different name. But it was kind of nice that this wasn't part of the protocol between Fuego core and the overlay generator. Now, it is. Implicitly Fuego core expects the overlay generator to produce a specific "magic" filename. Probably, the overlay generator is one of the elements of Fuego least likely to be used by external systems, but still I hesitate to make it less general. On the other hand, it simplifies the code to just use a well-known filename. 2) I'm not a fan of using the argument string "--logdir". I think my preference would be "--outdir", or even "--rundir". This is for the same reason - to keep the overlay generator generic. The artifacts created by the overlay generator have nothing to do with logging, and more to do with processing required during the run. Well, maybe the generated spec is a form of log (collection of information about the run). These are minor nits. It's OK for now. (Perfect is the enemy of good enough!) Maybe it's sufficient if there's a comment above this line, indicating that the overlay generator will populate the logdir with prolog.sh and spec.json. I think I'll just add that. > > if [ ! -f "$LOGDIR/prolog.sh" ] > then > diff --git a/engine/scripts/ovgen.py b/engine/scripts/ovgen.py > index 98cdf82..7c1f05f 100755 > --- a/engine/scripts/ovgen.py > +++ b/engine/scripts/ovgen.py > @@ -366,7 +366,8 @@ def parseOverrideFile(overrideFile, layer, ofcls): > > return classes > > -def generateProlog(outFilePath, ofcls, classes, testdir, testspec): > +def generateProlog(logdir, ofcls, classes, testdir, testspec): > + outFilePath = logdir + '/prolog.sh' > outfile = open(outFilePath, "w") > > for ofc in classes: > @@ -402,7 +403,6 @@ def generateProlog(outFilePath, ofcls, classes, testdir, > testspec): > # generateSpec - generate shell output for the spec > # ts:TestSpecs - parsed specs for our testdir > # testspec: String - name of test spec (e.g. 'default') > -# outFilePath - file output descriptor opened -> prolog.sh > def generateSpec(ts, fout): > debug_print("generating spec %s" % (ts.name)) > > @@ -470,7 +470,7 @@ def run(test_args=None): > parser.add_argument('--ovfiles', nargs='+', metavar='OVFILE', help='list of > directories containing .override files', required=True) > parser.add_argument('--testdir', help='e.g.: Benchmark.Dhrystone', > required=True) > parser.add_argument('--testspec', nargs='+', metavar='TESTSPEC', > help='testspec', required=True) > - parser.add_argument('--output', help='output file name', required=True) > + parser.add_argument('--logdir', help='log dir', required=True) > parser.add_argument('--debug', help='{1,2,3} debug level (default is no > debugging)', type=int, required=False) > > args = parser.parse_args(args=test_args) > @@ -493,10 +493,10 @@ def run(test_args=None): > classes = classes + parseOverrideFile(ovf, layers, ofcls) > debug_print ("parsed %s override\n------------\n" % (ovf)) > > - generateProlog(args.output, ofcls, classes, args.testdir, args.testspec[0]) > + generateProlog(args.logdir, ofcls, classes, args.testdir, args.testspec[0]) > > def testrun(): > - test_args = "--debug 2 --classdir /fuego-core/engine/overlays/base/ -- > ovfiles /fuego-ro/boards/qemu-arm.board --testdir Benchmark.Dhrystone -- > testspec default --output prolog.sh".split() > + test_args = "--debug 2 --classdir /fuego-core/engine/overlays/base/ -- > ovfiles /fuego-ro/boards/qemu-arm.board --testdir Benchmark.Dhrystone -- > testspec default --output ./".split() > run(test_args) > > run() > -- > 2.7.4 Ok - looks good. Applied. -- Tim ^ permalink raw reply [flat|nested] 9+ messages in thread
* [Fuego] [PATCH 2/3] OF: create a spec.json on each run 2018-09-04 10:01 [Fuego] Dynamic variables Daniel Sangorrin 2018-09-04 10:01 ` [Fuego] [PATCH 1/3] of: use logdir argument instead of prolog.sh Daniel Sangorrin @ 2018-09-04 10:01 ` Daniel Sangorrin 2018-09-07 18:31 ` Tim.Bird 2018-09-04 10:01 ` [Fuego] [PATCH 3/3] ftc: add dynamic-vars to ftc run-test Daniel Sangorrin 2 siblings, 1 reply; 9+ messages in thread From: Daniel Sangorrin @ 2018-09-04 10:01 UTC (permalink / raw) To: fuego This patch outputs the spec used in a run into a spec.json file at the run's log folder. Although prolog.sh already contains the variables from the spec, it is useful to have a spec.json file that we can share. Also, this spec.json file could be created by other means such as dynamic variables instead of coming from a semi-permanent source. Signed-off-by: Daniel Sangorrin <daniel.sangorrin@toshiba.co.jp> --- engine/scripts/ovgen.py | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/engine/scripts/ovgen.py b/engine/scripts/ovgen.py index 7c1f05f..ef09e85 100755 --- a/engine/scripts/ovgen.py +++ b/engine/scripts/ovgen.py @@ -397,7 +397,7 @@ def generateProlog(logdir, ofcls, classes, testdir, testspec): file.write(outfile, "\n") - ts = parseSpec(testdir, testspec) + ts = parseSpec(logdir, testdir, testspec) generateSpec(ts, outfile) # generateSpec - generate shell output for the spec @@ -431,9 +431,24 @@ def generateSpec(ts, fout): fout.write(outPattern + "\n") fout.write(outMessage + "\n") -def parseSpec(testdir, testspec): +def parseSpec(logdir, testdir, testspec): # FIXTHIS: get fuego-core from env - specpath = '/fuego-core/engine/tests/%s/spec.json' % (testdir) + specpath = logdir + '/spec.json' + if not os.path.exists(specpath): + # create a spec.json by stripping the test's spec.json + test_specpath = '/fuego-core/engine/tests/%s/spec.json' % (testdir) + with open(test_specpath) as f: + try: + test_spec_data = json.load(f) + except: + error_out("Error parsing spec file %s" % specpath) + for key in test_spec_data['specs'].keys(): + if key != testspec: + del test_spec_data['specs'][key] + print "test spec data" + str(test_spec_data) + with open(specpath, 'w+') as spec_file: + json.dump(test_spec_data, spec_file) + ts = TestSpecs() debug_print("Parsing %s spec file" % (specpath)) -- 2.7.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [Fuego] [PATCH 2/3] OF: create a spec.json on each run 2018-09-04 10:01 ` [Fuego] [PATCH 2/3] OF: create a spec.json on each run Daniel Sangorrin @ 2018-09-07 18:31 ` Tim.Bird 0 siblings, 0 replies; 9+ messages in thread From: Tim.Bird @ 2018-09-07 18:31 UTC (permalink / raw) To: daniel.sangorrin, fuego > -----Original Message----- > From: Daniel Sangorrin > > This patch outputs the spec used in a run into a spec.json > file at the run's log folder. Although prolog.sh already > contains the variables from the spec, it is useful to have > a spec.json file that we can share. Also, this spec.json > file could be created by other means such as dynamic > variables instead of coming from a semi-permanent source. > > Signed-off-by: Daniel Sangorrin <daniel.sangorrin@toshiba.co.jp> > --- > engine/scripts/ovgen.py | 21 ++++++++++++++++++--- > 1 file changed, 18 insertions(+), 3 deletions(-) > > diff --git a/engine/scripts/ovgen.py b/engine/scripts/ovgen.py > index 7c1f05f..ef09e85 100755 > --- a/engine/scripts/ovgen.py > +++ b/engine/scripts/ovgen.py > @@ -397,7 +397,7 @@ def generateProlog(logdir, ofcls, classes, testdir, > testspec): > > file.write(outfile, "\n") > > - ts = parseSpec(testdir, testspec) > + ts = parseSpec(logdir, testdir, testspec) > generateSpec(ts, outfile) > > # generateSpec - generate shell output for the spec > @@ -431,9 +431,24 @@ def generateSpec(ts, fout): > fout.write(outPattern + "\n") > fout.write(outMessage + "\n") > > -def parseSpec(testdir, testspec): > +def parseSpec(logdir, testdir, testspec): > # FIXTHIS: get fuego-core from env > - specpath = '/fuego-core/engine/tests/%s/spec.json' % (testdir) > + specpath = logdir + '/spec.json' > + if not os.path.exists(specpath): > + # create a spec.json by stripping the test's spec.json > + test_specpath = '/fuego-core/engine/tests/%s/spec.json' % (testdir) > + with open(test_specpath) as f: > + try: > + test_spec_data = json.load(f) > + except: > + error_out("Error parsing spec file %s" % specpath) > + for key in test_spec_data['specs'].keys(): > + if key != testspec: > + del test_spec_data['specs'][key] > + print "test spec data" + str(test_spec_data) I'm changing this to a debug_print() > + with open(specpath, 'w+') as spec_file: > + json.dump(test_spec_data, spec_file) > + > ts = TestSpecs() > > debug_print("Parsing %s spec file" % (specpath)) > -- > 2.7.4 OK - looks good. Applied (and adjusted). -- Tim ^ permalink raw reply [flat|nested] 9+ messages in thread
* [Fuego] [PATCH 3/3] ftc: add dynamic-vars to ftc run-test 2018-09-04 10:01 [Fuego] Dynamic variables Daniel Sangorrin 2018-09-04 10:01 ` [Fuego] [PATCH 1/3] of: use logdir argument instead of prolog.sh Daniel Sangorrin 2018-09-04 10:01 ` [Fuego] [PATCH 2/3] OF: create a spec.json on each run Daniel Sangorrin @ 2018-09-04 10:01 ` Daniel Sangorrin 2018-09-07 18:55 ` Tim.Bird 2 siblings, 1 reply; 9+ messages in thread From: Daniel Sangorrin @ 2018-09-04 10:01 UTC (permalink / raw) To: fuego This patch adds the ability to override parameters of a base spec (if not specified, the default spec is used) from ftc run-test. The values are logged into the run's log folder as spec.json. It is useful to modify the parameters given to a test in a quick manner. Signed-off-by: Daniel Sangorrin <daniel.sangorrin@toshiba.co.jp> --- engine/scripts/ftc | 54 +++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 53 insertions(+), 1 deletion(-) diff --git a/engine/scripts/ftc b/engine/scripts/ftc index 2840f5c..875ed59 100755 --- a/engine/scripts/ftc +++ b/engine/scripts/ftc @@ -242,6 +242,7 @@ You can obtain a list of run_ids for runs on the local system with """Usage: ftc run-test -b <board> -t <test> [-s <spec>] [-p <phases>] [-k <kill timeout>] [--rebuild <true|false>] [--reboot <true|false>] [--precleanup <true|false>] [--postcleanup <true|false>] + [--dynamic-vars python_dict] Run the indicated test on the specified board. Use the indicated spec, if one is provided. @@ -251,6 +252,8 @@ rebuild: if true rebuild the test source even if it was already built. reboot: if true reboot the board before the test. precleanup: if true clean the board's test folder before the test. postcleanup: if true clean the board's test folder after the test. +dynamic-vars: allows overriding the variables in a spec from the command line. + Example: ftc run-test -b bbb -t Benchmark.Dhrystone --dynamic-vars "{'LOOPS':'400000000'}" A list of phase characters may be provided, to execute only those phases p = pre_test, c=pre_check, b=build, d=deploy, r=run, @@ -3219,6 +3222,18 @@ def do_run_test(conf, options): else: error_out("Run-test command requires a test name") + if '--dynamic-vars' in options: + try: + import ast + dyn_vars_str = options[options.index('--dynamic-vars') + 1] + dyn_vars = ast.literal_eval(dyn_vars_str) + except IndexError: + error_out('Dynamic vars not provided after --dynamic-vars.') + options.remove(dyn_vars_str) + options.remove('--dynamic-vars') + else: + dyn_vars = None + if '-s' in options: try: spec_name = options[options.index('-s') + 1] @@ -3226,7 +3241,8 @@ def do_run_test(conf, options): error_out('Testspec not provided after -s.') spec_list = get_specs(conf, test_name) if spec_name not in spec_list: - error_out('Unknown spec %s' % spec_name) + if not dyn_vars: + error_out('Unknown spec %s' % spec_name) options.remove(spec_name) options.remove('-s') test_dict["spec"] = spec_name @@ -3405,6 +3421,12 @@ def do_run_test(conf, options): build_number = find_next_build_number_by_log_scan(logs_dir) build_data.build_number = build_number + build_data.test_logdir = conf.FUEGO_RW + '/logs/' + \ + build_data.test_name + '/' + \ + build_data.board_name + '.' + \ + build_data.spec_name + '.' + \ + build_data.build_number + '.' + \ + build_data.build_number os.environ["EXECUTOR_NUMBER"] = "0" os.environ["BUILD_ID"] = build_data.build_number @@ -3437,6 +3459,36 @@ def do_run_test(conf, options): os.environ["Target_PreCleanup"] = build_data.precleanup_flag os.environ["Target_PostCleanup"] = build_data.postcleanup_flag + # prepare a per-run spec.json file + specpath = '%s/engine/tests/%s/spec.json' % (conf.FUEGO_CORE, test_name) + with open(specpath) as f: + try: + test_spec_data = json.load(f) + except: + error_out("Error parsing spec file %s" % specpath) + + for key in test_spec_data['specs'].keys(): + if key != test.spec: + del test_spec_data['specs'][key] + + print "dynvars: " + str(dyn_vars) + if dyn_vars: + if test.spec not in test_spec_data['specs']: + test_spec_data['specs'][test.spec] = dyn_vars + else: + for key in dyn_vars.keys(): + test_spec_data['specs'][test.spec][key] = dyn_vars[key] + # track what variables where modified + test_spec_data['specs'][test.spec]['dyn_vars'] = dyn_vars.keys() + + print "test spec data" + str(test_spec_data) + + # FIXTHIS: use a more pythonic way + os.system("mkdir -p " + build_data.test_logdir) + + with open(build_data.test_logdir + '/spec.json', 'w+') as spec_file: + json.dump(test_spec_data, spec_file) + # cd to buildzone directory saved_cur_dir = os.getcwd() os.chdir(build_data.workspace) -- 2.7.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [Fuego] [PATCH 3/3] ftc: add dynamic-vars to ftc run-test 2018-09-04 10:01 ` [Fuego] [PATCH 3/3] ftc: add dynamic-vars to ftc run-test Daniel Sangorrin @ 2018-09-07 18:55 ` Tim.Bird 2018-10-05 1:43 ` Daniel Sangorrin 0 siblings, 1 reply; 9+ messages in thread From: Tim.Bird @ 2018-09-07 18:55 UTC (permalink / raw) To: daniel.sangorrin, fuego > -----Original Message----- > From: Daniel Sangorrin > > This patch adds the ability to override parameters of a > base spec (if not specified, the default spec is used) > from ftc run-test. The values are logged into the run's > log folder as spec.json. It is useful to modify the > parameters given to a test in a quick manner. > > Signed-off-by: Daniel Sangorrin <daniel.sangorrin@toshiba.co.jp> > --- > engine/scripts/ftc | 54 > +++++++++++++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 53 insertions(+), 1 deletion(-) > > diff --git a/engine/scripts/ftc b/engine/scripts/ftc > index 2840f5c..875ed59 100755 > --- a/engine/scripts/ftc > +++ b/engine/scripts/ftc > @@ -242,6 +242,7 @@ You can obtain a list of run_ids for runs on the local > system with > """Usage: ftc run-test -b <board> -t <test> [-s <spec>] [-p <phases>] > [-k <kill timeout>] [--rebuild <true|false>] [--reboot <true|false>] > [--precleanup <true|false>] [--postcleanup <true|false>] > + [--dynamic-vars python_dict] > Run the indicated test on the specified board. Use the > indicated spec, if one is provided. > > @@ -251,6 +252,8 @@ rebuild: if true rebuild the test source even if it was > already built. > reboot: if true reboot the board before the test. > precleanup: if true clean the board's test folder before the test. > postcleanup: if true clean the board's test folder after the test. > +dynamic-vars: allows overriding the variables in a spec from the command > line. > + Example: ftc run-test -b bbb -t Benchmark.Dhrystone --dynamic-vars > "{'LOOPS':'400000000'}" I'm not a huge fan of the python/json syntax for simple name/value items. However, I see the potential benefit of being able to specify arbitrary data structures here. Would you object to an addition to also support a simplified name=value syntax? The idea would be something like this: --dynamic-vars "LOOPS=400000000,OTHER_THING=foo -bar" Which would be converted to: "{'LOOPS':'400000000','OTHER_THING':'foo -bar'}" The code could use the first character to determine which syntax was being used: '{' for ast, and alpha-numeric for name=value synax. Out of curiosity - do you expect the most common use case is for people to type these on the command line, or have them coded into scripts, or have them generated by some other tool? (e.g. a variation-creating tool, or a bisector tool?) > > A list of phase characters may be provided, to execute only those phases > p = pre_test, c=pre_check, b=build, d=deploy, r=run, > @@ -3219,6 +3222,18 @@ def do_run_test(conf, options): > else: > error_out("Run-test command requires a test name") > > + if '--dynamic-vars' in options: > + try: > + import ast > + dyn_vars_str = options[options.index('--dynamic-vars') + 1] > + dyn_vars = ast.literal_eval(dyn_vars_str) > + except IndexError: > + error_out('Dynamic vars not provided after --dynamic-vars.') > + options.remove(dyn_vars_str) > + options.remove('--dynamic-vars') > + else: > + dyn_vars = None > + > if '-s' in options: > try: > spec_name = options[options.index('-s') + 1] > @@ -3226,7 +3241,8 @@ def do_run_test(conf, options): > error_out('Testspec not provided after -s.') > spec_list = get_specs(conf, test_name) > if spec_name not in spec_list: > - error_out('Unknown spec %s' % spec_name) > + if not dyn_vars: > + error_out('Unknown spec %s' % spec_name) > options.remove(spec_name) > options.remove('-s') > test_dict["spec"] = spec_name > @@ -3405,6 +3421,12 @@ def do_run_test(conf, options): > build_number = find_next_build_number_by_log_scan(logs_dir) > > build_data.build_number = build_number > + build_data.test_logdir = conf.FUEGO_RW + '/logs/' + \ > + build_data.test_name + '/' + \ > + build_data.board_name + '.' + \ > + build_data.spec_name + '.' + \ > + build_data.build_number + '.' + \ > + build_data.build_number > > os.environ["EXECUTOR_NUMBER"] = "0" > os.environ["BUILD_ID"] = build_data.build_number > @@ -3437,6 +3459,36 @@ def do_run_test(conf, options): > os.environ["Target_PreCleanup"] = build_data.precleanup_flag > os.environ["Target_PostCleanup"] = build_data.postcleanup_flag > > + # prepare a per-run spec.json file > + specpath = '%s/engine/tests/%s/spec.json' % (conf.FUEGO_CORE, > test_name) > + with open(specpath) as f: > + try: > + test_spec_data = json.load(f) > + except: > + error_out("Error parsing spec file %s" % specpath) > + > + for key in test_spec_data['specs'].keys(): > + if key != test.spec: > + del test_spec_data['specs'][key] > + > + print "dynvars: " + str(dyn_vars) > + if dyn_vars: > + if test.spec not in test_spec_data['specs']: > + test_spec_data['specs'][test.spec] = dyn_vars > + else: > + for key in dyn_vars.keys(): > + test_spec_data['specs'][test.spec][key] = dyn_vars[key] > + # track what variables where modified > + test_spec_data['specs'][test.spec]['dyn_vars'] = dyn_vars.keys() > + > + print "test spec data" + str(test_spec_data) > + > + # FIXTHIS: use a more pythonic way > + os.system("mkdir -p " + build_data.test_logdir) > + > + with open(build_data.test_logdir + '/spec.json', 'w+') as spec_file: > + json.dump(test_spec_data, spec_file) > + > # cd to buildzone directory > saved_cur_dir = os.getcwd() > os.chdir(build_data.workspace) > -- > 2.7.4 Ok - looks good. Applied. Just to clarify this feature: - if dynamic variables are specified, ftc creates logdir/spec.json with the dynamic variables overriding any values in the specified 'spec', that came from testdir/spec.json - also, the spec data is filtered to just the spec that will be used for the run - the overlay generator will use logdir/spec.json - the list of dynamic vars is noted in the generated spec.json - if no dynamic variables are specified, ftc does nothing, and the overlay generator copies the spec data from testdir/spec.json into logdir/spec.json - the spec data is filtered to just the spec that will be used for the run This seems like a duplication of code. Is this to support backwards compatibility with jobs that still call main.sh, when we do the switchover to calling 'ftc run-test'? If so, thanks. Good job! -- Tim ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Fuego] [PATCH 3/3] ftc: add dynamic-vars to ftc run-test 2018-09-07 18:55 ` Tim.Bird @ 2018-10-05 1:43 ` Daniel Sangorrin 2018-10-06 1:18 ` Tim.Bird 0 siblings, 1 reply; 9+ messages in thread From: Daniel Sangorrin @ 2018-10-05 1:43 UTC (permalink / raw) To: Tim.Bird, fuego Hi Tim, Sorry for the late reply! > -----Original Message----- > From: Tim.Bird@sony.com <Tim.Bird@sony.com> > Sent: Saturday, September 8, 2018 3:56 AM > To: daniel.sangorrin@toshiba.co.jp; fuego@lists.linuxfoundation.org > Subject: RE: [Fuego] [PATCH 3/3] ftc: add dynamic-vars to ftc run-test > > > -----Original Message----- > > From: Daniel Sangorrin > > > > This patch adds the ability to override parameters of a > > base spec (if not specified, the default spec is used) > > from ftc run-test. The values are logged into the run's > > log folder as spec.json. It is useful to modify the > > parameters given to a test in a quick manner. > > > > Signed-off-by: Daniel Sangorrin <daniel.sangorrin@toshiba.co.jp> > > --- > > engine/scripts/ftc | 54 > > > +++++++++++++++++++++++++++++++++++++++++++++++++++ > ++- > > 1 file changed, 53 insertions(+), 1 deletion(-) > > > > diff --git a/engine/scripts/ftc b/engine/scripts/ftc > > index 2840f5c..875ed59 100755 > > --- a/engine/scripts/ftc > > +++ b/engine/scripts/ftc > > @@ -242,6 +242,7 @@ You can obtain a list of run_ids for runs on the local > > system with > > """Usage: ftc run-test -b <board> -t <test> [-s <spec>] [-p <phases>] > > [-k <kill timeout>] [--rebuild <true|false>] [--reboot <true|false>] > > [--precleanup <true|false>] [--postcleanup <true|false>] > > + [--dynamic-vars python_dict] > > Run the indicated test on the specified board. Use the > > indicated spec, if one is provided. > > > > @@ -251,6 +252,8 @@ rebuild: if true rebuild the test source even if it was > > already built. > > reboot: if true reboot the board before the test. > > precleanup: if true clean the board's test folder before the test. > > postcleanup: if true clean the board's test folder after the test. > > +dynamic-vars: allows overriding the variables in a spec from the command > > line. > > + Example: ftc run-test -b bbb -t Benchmark.Dhrystone --dynamic-vars > > "{'LOOPS':'400000000'}" > I'm not a huge fan of the python/json syntax for simple name/value items. > However, I see the potential benefit of being able to specify arbitrary > data structures here. > > Would you object to an addition to also support a simplified name=value syntax? > The idea would be something like this: > --dynamic-vars "LOOPS=400000000,OTHER_THING=foo -bar" > Which would be converted to: > "{'LOOPS':'400000000','OTHER_THING':'foo -bar'}" > > The code could use the first character to determine which syntax was being used: > '{' for ast, and alpha-numeric for name=value synax. The above should work with the patch I just sent. > Out of curiosity - do you expect the most common use case is for people to > type these on the command line, or have them coded into scripts, or have > them generated by some other tool? (e.g. a variation-creating tool, or > a bisector tool?) There are several use cases: - People trying to find the test parameters that best fit their boards. For example, the number of loops appropriated for their board. Once they found them, they can keep them in per-board files. - Developers that want to debug a failure and need to see what happends when they change the parameters. - Periodic CI scripts that run Fuego from the command line - Fuzzy tools that try to find corner cases where the test fails I am interested in the latest use case. I want to provide a fuzzy tool for Fuego tests. > > A list of phase characters may be provided, to execute only those phases > > p = pre_test, c=pre_check, b=build, d=deploy, r=run, > > @@ -3219,6 +3222,18 @@ def do_run_test(conf, options): > > else: > > error_out("Run-test command requires a test name") > > > > + if '--dynamic-vars' in options: > > + try: > > + import ast > > + dyn_vars_str = options[options.index('--dynamic-vars') + 1] > > + dyn_vars = ast.literal_eval(dyn_vars_str) > > + except IndexError: > > + error_out('Dynamic vars not provided after --dynamic-vars.') > > + options.remove(dyn_vars_str) > > + options.remove('--dynamic-vars') > > + else: > > + dyn_vars = None > > + > > if '-s' in options: > > try: > > spec_name = options[options.index('-s') + 1] > > @@ -3226,7 +3241,8 @@ def do_run_test(conf, options): > > error_out('Testspec not provided after -s.') > > spec_list = get_specs(conf, test_name) > > if spec_name not in spec_list: > > - error_out('Unknown spec %s' % spec_name) > > + if not dyn_vars: > > + error_out('Unknown spec %s' % spec_name) > > options.remove(spec_name) > > options.remove('-s') > > test_dict["spec"] = spec_name > > @@ -3405,6 +3421,12 @@ def do_run_test(conf, options): > > build_number = find_next_build_number_by_log_scan(logs_dir) > > > > build_data.build_number = build_number > > + build_data.test_logdir = conf.FUEGO_RW + '/logs/' + \ > > + build_data.test_name + '/' + \ > > + build_data.board_name + '.' + \ > > + build_data.spec_name + '.' + \ > > + build_data.build_number + '.' + \ > > + build_data.build_number > > > > os.environ["EXECUTOR_NUMBER"] = "0" > > os.environ["BUILD_ID"] = build_data.build_number > > @@ -3437,6 +3459,36 @@ def do_run_test(conf, options): > > os.environ["Target_PreCleanup"] = build_data.precleanup_flag > > os.environ["Target_PostCleanup"] = build_data.postcleanup_flag > > > > + # prepare a per-run spec.json file > > + specpath = '%s/engine/tests/%s/spec.json' % (conf.FUEGO_CORE, > > test_name) > > + with open(specpath) as f: > > + try: > > + test_spec_data = json.load(f) > > + except: > > + error_out("Error parsing spec file %s" % specpath) > > + > > + for key in test_spec_data['specs'].keys(): > > + if key != test.spec: > > + del test_spec_data['specs'][key] > > + > > + print "dynvars: " + str(dyn_vars) > > + if dyn_vars: > > + if test.spec not in test_spec_data['specs']: > > + test_spec_data['specs'][test.spec] = dyn_vars > > + else: > > + for key in dyn_vars.keys(): > > + test_spec_data['specs'][test.spec][key] = dyn_vars[key] > > + # track what variables where modified > > + test_spec_data['specs'][test.spec]['dyn_vars'] = dyn_vars.keys() > > + > > + print "test spec data" + str(test_spec_data) > > + > > + # FIXTHIS: use a more pythonic way > > + os.system("mkdir -p " + build_data.test_logdir) > > + > > + with open(build_data.test_logdir + '/spec.json', 'w+') as spec_file: > > + json.dump(test_spec_data, spec_file) > > + > > # cd to buildzone directory > > saved_cur_dir = os.getcwd() > > os.chdir(build_data.workspace) > > -- > > 2.7.4 > > Ok - looks good. Applied. > > Just to clarify this feature: > - if dynamic variables are specified, ftc creates logdir/spec.json with the dynamic > variables overriding any > values in the specified 'spec', that came from testdir/spec.json > - also, the spec data is filtered to just the spec that will be used for the run > - the overlay generator will use logdir/spec.json > - the list of dynamic vars is noted in the generated spec.json > - if no dynamic variables are specified, ftc does nothing, and the overlay generator > copies > the spec data from testdir/spec.json into logdir/spec.json > - the spec data is filtered to just the spec that will be used for the run Yes, that's correct. > This seems like a duplication of code. Is this to support backwards compatibility > with jobs > that still call main.sh, when we do the switchover to calling 'ftc run-test'? Yes that is also correct. I have send a patch that removes the backwards compatibility now that we switched over to calling ftc run-test, but you can turn it into a "Warning, this is deprecated" sort of thing. Probably, that is the wiser choice. > If so, thanks. Good job! > -- Tim Thanks to you! Daniel ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Fuego] [PATCH 3/3] ftc: add dynamic-vars to ftc run-test 2018-10-05 1:43 ` Daniel Sangorrin @ 2018-10-06 1:18 ` Tim.Bird 0 siblings, 0 replies; 9+ messages in thread From: Tim.Bird @ 2018-10-06 1:18 UTC (permalink / raw) To: daniel.sangorrin, fuego > -----Original Message----- > From: Daniel Sangorrin [mailto:daniel.sangorrin@toshiba.co.jp] > > This seems like a duplication of code. Is this to support backwards > compatibility > > with jobs > > that still call main.sh, when we do the switchover to calling 'ftc run-test'? > > Yes that is also correct. I have send a patch that removes the backwards > compatibility now that we switched over to calling ftc run-test, but you can > turn it into a "Warning, this is deprecated" sort of thing. Probably, that is the > wiser choice. Thanks. That's what I did. See my master branch for the warning message. -- Tim ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2018-10-06 1:18 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-09-04 10:01 [Fuego] Dynamic variables Daniel Sangorrin 2018-09-04 10:01 ` [Fuego] [PATCH 1/3] of: use logdir argument instead of prolog.sh Daniel Sangorrin 2018-09-07 18:25 ` Tim.Bird 2018-09-04 10:01 ` [Fuego] [PATCH 2/3] OF: create a spec.json on each run Daniel Sangorrin 2018-09-07 18:31 ` Tim.Bird 2018-09-04 10:01 ` [Fuego] [PATCH 3/3] ftc: add dynamic-vars to ftc run-test Daniel Sangorrin 2018-09-07 18:55 ` Tim.Bird 2018-10-05 1:43 ` Daniel Sangorrin 2018-10-06 1:18 ` Tim.Bird
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.