[Fuego] [PATCH 28/30] logdir: a few changes

Tim.Bird at sony.com Tim.Bird at sony.com
Wed Jun 6 23:28:43 UTC 2018



> -----Original Message-----
> From: Daniel Sangorrin
> 
> logdir is now created by ftc run-test. Also we use logdir
> to put the spec so fix that in ovgen.
> 
> Signed-off-by: Daniel Sangorrin <daniel.sangorrin at toshiba.co.jp>
> ---
>  engine/scripts/overlays.sh | 17 +++++++----------
>  engine/scripts/ovgen.py    | 23 +++++++++++------------
>  2 files changed, 18 insertions(+), 22 deletions(-)
> 
> diff --git a/engine/scripts/overlays.sh b/engine/scripts/overlays.sh
> index 186bd8a..7a66910 100644
> --- a/engine/scripts/overlays.sh
> +++ b/engine/scripts/overlays.sh
> @@ -56,7 +56,7 @@ function set_overlay_vars() {
>      d_value=${d_valtemp%'"'}
>      DISTRIB="${d_value}"
>      set -e
> -
> +
>      if [ ! "$DISTRIB" ]; then
>          # FIXTHIS: automatically discover the best option
>          DISTRIB="nosyslogd.dist"
> @@ -67,17 +67,14 @@ function set_overlay_vars() {
>      fi
>      echo "Using $DISTRIB overlay"
> 
> -    # Create the log directory for this test run here so we can place the
> prolog.sh
> -    export
> LOGDIR="$FUEGO_RW/logs/$TESTDIR/${NODE_NAME}.${TESTSPEC}.${BUIL
> D_NUMBER}.${BUILD_ID}"
> -    mkdir -p $LOGDIR
As stated in previous patch, we shouldn't assume that we're always being
run by ftc, just yet.  I'm not sure whether the pre_test export or this
one come first, but I think we should leave this code for now.

NAK on this hunk.

> -
> -    rm -f $LOGDIR/prolog.sh
> +    if [ -f $LOGDIR/prolog.sh ]; then
> +        abort_job "$LOGDIR directory contains data from a previous test"
> +    fi
> 
> -    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
> -        abort_job "$LOGDIR/prolog.sh not found"
> +    if [ ! -f "$LOGDIR/prolog.sh" ]; then
> +        abort_job "$LOGDIR/prolog.sh was not created by ovgen.py"
>      fi
> 
>      # FIXTHIS: add BUILD_TIMESTAMP and other variables to prolog.sh for --
> replay
> diff --git a/engine/scripts/ovgen.py b/engine/scripts/ovgen.py
> index 98cdf82..7aba86d 100755
> --- a/engine/scripts/ovgen.py
> +++ b/engine/scripts/ovgen.py
> @@ -366,8 +366,8 @@ def parseOverrideFile(overrideFile, layer, ofcls):
> 
>      return classes
> 
> -def generateProlog(outFilePath, ofcls, classes, testdir, testspec):
> -    outfile = open(outFilePath, "w")
> +def generateProlog(logdir, ofcls, classes, testdir, testspec):
> +    outfile = open(logdir + '/prolog.sh', "w")
> 
>      for ofc in classes:
>          # ofc = ofcls[name]
> @@ -379,30 +379,29 @@ def generateProlog(outFilePath, ofcls, classes,
> testdir, testspec):
>          for var in ofc.vars:
>              outStr = "%s=\"%s\"" % (var, ofc.vars[var])
>              outStr = outStr.replace('"', '\"')
> -            debug_print("%s <- %s" % (outFilePath, outStr))
> +            debug_print("%s <- %s" % (logdir, outStr))
>              file.write(outfile, outStr+"\n")
> 
>          for cap in ofc.cap_list:
>              outStr = "CAP_%s=\"yes\"" % (cap)
> -            debug_print("%s <- %s" % (outFilePath, outStr))
> +            debug_print("%s <- %s" % (logdir, outStr))
>              file.write(outfile, outStr+"\n")
> 
>          file.write(outfile, "\n")
> 
>          for func in ofc.funcs:
>              body = ofc.funcs[func]
> -            debug_print("%s <- %s()" % (outFilePath, func))
> +            debug_print("%s <- %s()" % (logdir, func))
>              file.write(outfile, body+"\n")
> 
>          file.write(outfile, "\n")
> 
> -    ts = parseSpec(testdir, testspec)
> +    ts = parseSpec(logdir, testdir, testspec)
>      generateSpec(ts, outfile)
> 
>  # 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))
> 
> @@ -431,9 +430,9 @@ 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'
OK - this seems to be the major reason for this change.

In general, I think that using a generic output path is better
(more decoupled from the rest of Fuego), than using a 
logdir parameter.

Also, it's a bit weird that we have to pass a TESTSPEC variable, when
there is only one spec in the spec.json file (if I'm reading things correctly).

This is required for dynamic vars to work, right?  That is, this looks
like the final change to actually start using the vars that were deposited
in $LOGDIR/spec.json by ftc.

This change also makes the code backwards-incompatible with previously-defined
jobs.  To support backwards-compatibility, can we check for spec.json
in logdir, and if not present fall back to reading spec.json from the test directory?

>      ts = TestSpecs()
> 
>      debug_print("Parsing %s spec file" % (specpath))
> @@ -470,7 +469,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 +492,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()
This should use --logdir instead of --output, to be consistent.

>      run(test_args)
> 
>  run()
> --
> 2.7.4

I did not apply this patch, pending discussion of points above.
 -- Tim


More information about the Fuego mailing list