[Fuego] [PATCH 1/3] specs: put links in the spec and don't use the name key

Bird, Timothy Tim.Bird at sony.com
Wed Apr 26 00:43:38 UTC 2017


> -----Original Message-----
> From: Daniel Sangorrin on Wednesday, April 19, 2017 1:32 AM
> 
> This is quite a big change. Until now links that were
> displayed by the "descriptionsetter" plugin were defined
> in the testplans. That doesn't make sense because those links
> depend on the test spec not on the board's testplan. For
> that reason I put all of the links inside the spec files.

I agree that the links should be based on the spec, which might
define different values for links to test materials based on
the variables passed to the test.  However, the changes
just duplicate the same strings into all the spec files. This
is a sign that there should have been a calculated default,
with empty values specifying to use that default.

> Additionally, I have modified the spec file format. Before
> specs where just an array of dicts so to find a specific
> spec you would have to traverse the whole array and compare
> the 'name' property. Now, the spec names are the keys so
> we can quickly access their attributes.
This change is OK.
 
> Signed-off-by: Daniel Sangorrin <daniel.sangorrin at toshiba.co.jp>
> ---
>  engine/scripts/ftc                                 | 40 ++++++++++++------
>  engine/scripts/ovgen.py                            | 20 ++++-----
>  .../Benchmark.Dhrystone/Benchmark.Dhrystone.spec   | 23 +++++-----
>  .../tests/Benchmark.GLMark/Benchmark.GLMark.spec   | 15 ++++---
>  .../tests/Benchmark.IOzone/Benchmark.IOzone.spec   | 26 ++++--------
>  .../Benchmark.Interbench/Benchmark.Interbench.spec | 27 ++++--------
>  engine/tests/Benchmark.Java/Benchmark.Java.spec    | 15 ++++---
>  .../tests/Benchmark.OpenSSL/Benchmark.OpenSSL.spec | 14 +++----
>  .../tests/Benchmark.Stream/Benchmark.Stream.spec   | 14 +++----
>  .../Benchmark.Whetstone/Benchmark.Whetstone.spec   | 16 +++----
>  engine/tests/Benchmark.aim7/Benchmark.aim7.spec    | 10 ++---
>  .../Benchmark.blobsallad/Benchmark.blobsallad.spec | 17 ++++----
>  .../tests/Benchmark.bonnie/Benchmark.bonnie.spec   | 22 ++++------
>  .../Benchmark.cyclictest/Benchmark.cyclictest.spec | 11 +++--
>  .../tests/Benchmark.dbench/Benchmark.dbench.spec   | 23 ++++------
>  .../tests/Benchmark.ebizzy/Benchmark.ebizzy.spec   | 12 +++---
>  engine/tests/Benchmark.ffsb/Benchmark.ffsb.spec    | 23 ++++------
>  engine/tests/Benchmark.fio/Benchmark.fio.spec      | 23 ++++------
>  .../tests/Benchmark.fs_mark/Benchmark.fs_mark.spec | 10 ++---
>  .../Benchmark.fuego_check_plots.spec               | 15 +++----
>  .../tests/Benchmark.gtkperf/Benchmark.gtkperf.spec | 17 ++++----
>  .../Benchmark.hackbench/Benchmark.hackbench.spec   | 12 +++---
>  .../tests/Benchmark.himeno/Benchmark.himeno.spec   | 15 ++++---
>  engine/tests/Benchmark.iperf/Benchmark.iperf.spec  | 12 +++---
>  .../tests/Benchmark.linpack/Benchmark.linpack.spec | 15 ++++---
>  .../Benchmark.lmbench2/Benchmark.lmbench2.spec     | 15 ++++---
>  .../Benchmark.nbench-byte.spec                     | 10 ++---
>  .../Benchmark.nbench_byte.spec                     | 15 ++++---
>  .../tests/Benchmark.netperf/Benchmark.netperf.spec | 17 ++++----
>  .../tests/Benchmark.netpipe/Benchmark.netpipe.spec | 12 +++---
>  .../tests/Benchmark.reboot/Benchmark.reboot.spec   | 15 ++++---
>  .../Benchmark.signaltest/Benchmark.signaltest.spec | 14 +++----
>  .../Benchmark.sysbench/Benchmark.sysbench.spec     | 10 ++---
>  .../Benchmark.tiobench/Benchmark.tiobench.spec     | 19 ++++-----
>  .../tests/Benchmark.x11perf/Benchmark.x11perf.spec | 12 +++---
>  engine/tests/Functional.LTP/Functional.LTP.spec    | 49 +++++++++++--------
> ---
>  .../Functional.OpenSSL/Functional.OpenSSL.spec     | 10 ++---
>  .../Functional.aiostress/Functional.aiostress.spec | 20 ++++-----
>  .../Functional.arch_timer.spec                     | 10 ++---
>  engine/tests/Functional.bc/Functional.bc.spec      | 18 ++++----
>  .../tests/Functional.boost/Functional.boost.spec   | 11 +++--
>  .../tests/Functional.bsdiff/Functional.bsdiff.spec | 10 ++---
>  .../tests/Functional.bzip2/Functional.bzip2.spec   | 11 +++--
>  engine/tests/Functional.cmt/Functional.cmt.spec    | 10 ++---
>  .../Functional.commonAPI_C++.spec                  | 10 ++---
>  .../Functional.commonAPI_Dbus.spec                 | 10 ++---
>  .../Functional.commonAPI_SomeIp.spec               | 10 ++---
>  .../Functional.crashme/Functional.crashme.spec     | 10 ++---
>  .../tests/Functional.croco/Functional.croco.spec   | 10 ++---
>  engine/tests/Functional.curl/Functional.curl.spec  | 11 +++--
>  .../tests/Functional.expat/Functional.expat.spec   | 11 +++--
>  .../Functional.fixesproto.spec                     | 10 ++---
>  .../Functional.fontconfig.spec                     | 11 +++--
>  .../tests/Functional.fsfuzz/Functional.fsfuzz.spec | 10 ++---
>  .../Functional.ft2demos/Functional.ft2demos.spec   | 11 +++--
>  .../Functional.fuego_abort.spec                    | 11 +++--
>  .../Functional.fuego_board_check.spec              | 11 +++--
>  .../Functional.fuego_test_phases.spec              | 11 +++--
>  .../Functional.fuego_transport.spec                | 11 +++--
>  engine/tests/Functional.fuse/Functional.fuse.spec  | 10 ++---
>  .../tests/Functional.giflib/Functional.giflib.spec | 10 ++---
>  engine/tests/Functional.glib/Functional.glib.spec  | 11 +++--
>  .../tests/Functional.glib2/Functional.glib2.spec   | 10 ++---
>  .../tests/Functional.glibc/Functional.glibc.spec   | 10 ++---
>  .../Functional.hciattach/Functional.hciattach.spec | 10 ++---
>  .../Functional.hello_world.spec                    | 16 ++++---
>  .../Functional.imagemagick.spec                    | 10 ++---
>  .../Functional.iptables/Functional.iptables.spec   | 10 ++---
>  .../Functional.iputils/Functional.iputils.spec     | 10 ++---
>  .../Functional.ipv6connect.spec                    | 11 +++--
>  engine/tests/Functional.jpeg/Functional.jpeg.spec  | 11 +++--
>  .../Functional.kernel_build.spec                   | 16 +++----
>  engine/tests/Functional.kmod/Functional.kmod.spec  | 10 ++---
>  .../tests/Functional.libogg/Functional.libogg.spec | 10 ++---
>  .../Functional.libpcap/Functional.libpcap.spec     | 10 ++---
>  .../Functional.librsvg/Functional.librsvg.spec     | 10 ++---
>  .../Functional.libspeex/Functional.libspeex.spec   | 10 ++---
>  .../tests/Functional.libtar/Functional.libtar.spec | 10 ++---
>  .../Functional.libwebsocket.spec                   | 10 ++---
>  .../Functional.linus_stress.spec                   | 10 ++---
>  engine/tests/Functional.lwip/Functional.lwip.spec  | 10 ++---
>  .../Functional.mesa-demos.spec                     | 10 ++---
>  engine/tests/Functional.neon/Functional.neon.spec  | 10 ++---
>  .../Functional.net-tools/Functional.net-tools.spec | 10 ++---
>  .../Functional.netperf/Functional.netperf.spec     | 13 +++---
>  .../Functional.pi_tests/Functional.pi_tests.spec   | 10 ++---
>  .../tests/Functional.pixman/Functional.pixman.spec | 10 ++---
>  engine/tests/Functional.pppd/Functional.pppd.spec  | 10 ++---
>  .../Functional.protobuf/Functional.protobuf.spec   | 10 ++---
>  .../Functional.rmaptest/Functional.rmaptest.spec   | 10 ++---
>  .../tests/Functional.scifab/Functional.scifab.spec | 10 ++---
>  .../Functional.scrashme/Functional.scrashme.spec   | 22 +++++-----
>  .../tests/Functional.sdhi_0/Functional.sdhi_0.spec | 10 ++---
>  .../tests/Functional.stress/Functional.stress.spec | 10 ++---
>  .../Functional.synctest/Functional.synctest.spec   | 23 +++++-----
>  .../tests/Functional.thrift/Functional.thrift.spec | 10 ++---
>  engine/tests/Functional.tiff/Functional.tiff.spec  | 10 ++---
>  .../Functional.vsomeip/Functional.vsomeip.spec     | 10 ++---
>  .../Functional.xorg-macros.spec                    | 10 ++---
>  engine/tests/Functional.zlib/Functional.zlib.spec  | 11 +++--
>  100 files changed, 633 insertions(+), 714 deletions(-)
> 
> diff --git a/engine/scripts/ftc b/engine/scripts/ftc
> index 939043b..98cd7f6 100755
> --- a/engine/scripts/ftc
> +++ b/engine/scripts/ftc
> @@ -788,14 +788,29 @@ def create_job(board, test):
>      else:
>          flot_link = ''
> 
> -    # prepare links for descriptionsetter. Put a link to testlog.txt by default
> +    # prepare links for the descriptionsetter plugin
> +    test_spec_path = '/fuego-core/engine/tests/%s/%s.spec' % (test.name,
> test.name)
>      template_link = '&lt;a
> href=&quot;/fuego/userContent/fuego.logs/%s/%s.%s.${BUILD_NUMBER}.
> ${BUILD_ID}/%%s&quot;&gt;%%s&lt;/a&gt;' % (test.name, board, test.spec)
> -
> -    html_links = template_link % ('testlog.txt', 'log')
> -    failed_links = html_links
> -    if test.extralinks:
> -        for key, value in test.extralinks.iteritems():
> -            html_links = html_links + ' ' + template_link % (str(value), str(key))
> +    with open(test_spec_path, "r") as f:
> +        test_specs_json = json.load(f)
> +        test_spec_json = test_specs_json['specs'][test.spec]
> +
> +    success_links = ''
> +    if 'success_links' in test_specs_json:
> +        for key, value in test_specs_json['success_links'].iteritems():
> +            success_links = success_links + ' ' + template_link % (str(value),
> str(key))
Can't we have an else here and fabricate a default value for the links
(depending on whether the test is a Functional or Benchmark test)?

> +    if 'extra_success_links' in test_spec_json:
> +        for key, value in test_spec_json['extra_success_links'].iteritems():
> +            success_links = success_links + ' ' + template_link % (str(value),
> str(key))
> +
> +    fail_links = ''
> +    if 'fail_links' in test_specs_json:
> +        for key, value in test_specs_json['fail_links'].iteritems():
> +            fail_links = fail_links + ' ' + template_link % (str(value), str(key))
Same here - let's have a default link.  It's currently the same in every spec file.

I'd rather have a default, and override it, than declare the same strings
in every single spec file.

> +
> +    if 'extra_fail_links' in test_spec_json:
> +        for key, value in test_spec_json['extra_fail_links'].iteritems():
> +            fail_links = fail_links + ' ' + template_link % (str(value), str(key))
> 
>      tmp = "/tmp/fuego_tmp_job"
>      fd = open(tmp, "w+")
> @@ -833,8 +848,8 @@ timeout --signal=9 {timeout} /bin/bash
> $FUEGO_CORE/engine/tests/${{TESTDIR}}/${{
>      <hudson.plugins.descriptionsetter.DescriptionSetterPublisher
> plugin="description-setter at 1.10">
>        <regexp></regexp>
>        <regexpForFailed></regexpForFailed>
> -      <description>{html_links}</description>
> -      <descriptionForFailed>{failed_links}</descriptionForFailed>
> +      <description>{success_links}</description>
> +      <descriptionForFailed>{fail_links}</descriptionForFailed>
>        <setForMatrix>false</setForMatrix>
>      </hudson.plugins.descriptionsetter.DescriptionSetterPublisher>
>      </publishers>
> @@ -844,7 +859,7 @@ timeout --signal=9 {timeout} /bin/bash
> $FUEGO_CORE/engine/tests/${{TESTDIR}}/${{
>          precleanup=test.precleanup, postcleanup=test.postcleanup,
>          testdir=test.name, testname=test.base_name,
>          testspec=test.spec, timeout=test.timeout,
> -        flot_link=flot_link, html_links=html_links, failed_links=failed_links))
> +        flot_link=flot_link, success_links=success_links, fail_links=fail_links))
>      fd.close()
> 
>      job_name=board+"."+test.spec+"."+test.name
> @@ -960,7 +975,7 @@ def do_add_jobs(conf, options):
>                  spec = options[options.index('-s') + 1]
>              except IndexError:
>                  error_out('Testspec not provided after -s.')
> -            specnames = [s['name'] for s in get_specs(conf, test_name)]
> +            specnames = get_specs(conf, test_name)
>              if spec not in specnames:
>                  error_out('Unknown spec %s' % spec)
>              options.remove('-s')
> @@ -1225,8 +1240,7 @@ def do_list_specs(conf, options):
>      else:
>          error_out('No testcase name supplied.')
> 
> -    specs = get_specs(conf, testcase)
> -    specnames = [spec['name'] for spec in specs]
> +    specnames = get_specs(conf, testcase).keys()
>      specnames.sort()
> 
>      indent = show_list_title("Testspecs for %s:" % testcase)
> diff --git a/engine/scripts/ovgen.py b/engine/scripts/ovgen.py
> index f3f77e3..b331243 100755
> --- a/engine/scripts/ovgen.py
> +++ b/engine/scripts/ovgen.py
> @@ -404,11 +404,9 @@ def generateProlog(outFilePath, ofcls, classes,
> testdir, testspec):
>  #     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' for '%s'" % (ts.variables['name'],
> ts.name))
> +    debug_print("generating spec %s" % (ts.name))
> 
>      for var in ts.variables:
> -        if var == 'name':
> -            continue
>          varname = "%s_%s" % (ts.name, var)
>          varname = string.replace(varname, ".", "_").upper()
>          value = "%s" % (ts.variables[var])
> @@ -446,17 +444,15 @@ def parseSpec(testdir, testspec):
>          except:
>              raise Exception("Error parsing spec file %s" % specpath)
> 
> -        ts.name = jd["testName"]
> +    ts.name = jd["testName"]
> 
> -        if "fail_case" in jd:
> -            ts.fail_case = jd["fail_case"]
> -            debug_print ("Found fail_case msgs in '%s'" % specpath)
> +    if "fail_case" in jd:
> +        ts.fail_case = jd["fail_case"]
> +        debug_print ("Found fail_case msgs in '%s'" % specpath)
> 
> -        for spec in jd["specs"]:
> -            if spec["name"] == testspec:
> -                ts.variables = spec
> -
> -    if not ts.variables:
> +    if testspec in jd["specs"]:
> +        ts.variables = jd["specs"][testspec]
> +    else:
>          raise Exception("Could not find %s in %s" % (testspec, specpath))
> 
>      return ts
> diff --git a/engine/tests/Benchmark.Dhrystone/Benchmark.Dhrystone.spec
> b/engine/tests/Benchmark.Dhrystone/Benchmark.Dhrystone.spec
> index 794ca68..7ceee2a 100644
> --- a/engine/tests/Benchmark.Dhrystone/Benchmark.Dhrystone.spec
> +++ b/engine/tests/Benchmark.Dhrystone/Benchmark.Dhrystone.spec
> @@ -1,25 +1,22 @@
> -   {
> +{
>      "testName": "Benchmark.Dhrystone",
>      "fail_case": [
>          {
>              "fail_regexp": "Measured time too small to obtain meaningful results",
>              "fail_message": "Measured time too small to obtain meaningful
> results. Please increase LOOPS parameter in Dhrystone test spec."
> -            }
> -        ],
> -    "specs":
> -    [
> -        {
> -            "name":"default",
> +        }
> +    ],
> +    "success_links": {"log": "testlog.txt", "plot": "plot.png"},
> +    "fail_links": {"log": "testlog.txt"},
Let's remove all these, unless they are different from the default.
> +    "specs": {
> +        "default": {
>              "LOOPS":"10000000"
>          },
> -        {
> -            "name":"100M",
> +        "100M": {
>              "LOOPS":"100000000"
>          },
> -        {
> -            "name":"500M",
> +        "500M": {
>              "LOOPS":"500000000"
>          }
> -    ]
> +    }
>  }
> -

Thanks.  Can you please clean up as directed and re-send?

 -- Tim



More information about the Fuego mailing list