[Fuego] cyclictest patches

[Fuego] cyclictest patches

[Daniel Sangorrin]

Hi Tim,

I have modified the cyclictest benchmark test.

- cyclictest: update source code tarball

The previous source code was too old and lacked some important
flags. I added a patch to build the test for toolchains based on
old kernels (maybe this should be upstream)

TODO: rt-tests contains other tests that are already in Fuego
and can share the same tarball.
TODO: cyclictest requires some disturbance running in
background.

- [PATCH 1/3] cyclictest: update build function

I have tested the build in x86_64 and an ARM yocto-based toolchain.

- [PATCH 2/3] cyclictest: modify specs

The original spec was hardwiring cyclictest to use 2 threads. Now
the default is using 1 thread per core (with affinity).

- [PATCH 3/3] cyclictest: modify parsing and test measures

This is the biggest change and shows a limitation on using
static definitions (reference.json). The problem is described
on the commit message, but basically it boils down to the 
fact that the amount of test measures may depend on the
hardware (e.g.: number of cores) or spec (e.g.: number of 
threads). We need a mechanism to specify arrays of measures
that share the same 'units' and criteria.

I also had problems understanding chart_config.json.
With the one I have committed I expected that ony "max_latency"
would show up on jenkins. However all of them are displayed.

Cheers,
Daniel

[Fuego] [PATCH 1/3] cyclictest: update build function

[Daniel Sangorrin]

Signed-off-by: Daniel Sangorrin <daniel.sangorrin@toshiba.co.jp>
---
 engine/tests/Benchmark.cyclictest/fuego_test.sh | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/engine/tests/Benchmark.cyclictest/fuego_test.sh b/engine/tests/Benchmark.cyclictest/fuego_test.sh
index 2631a82..efa892c 100755
--- a/engine/tests/Benchmark.cyclictest/fuego_test.sh
+++ b/engine/tests/Benchmark.cyclictest/fuego_test.sh
@@ -1,7 +1,8 @@
-tarball=cyclictest.tar.gz
+tarball=rt-tests-1.0.tar.gz
 
 function test_build {
-    make CC="$CC" AR="$AR" RANLIB="$RANLIB" CXX="$CXX" CPP="$CPP" CXXCPP="$CXXCPP" CFLAGS="$CFLAGS" LDFLAGS="$LDFLAGS"
+	patch -p1 -N -s < $TEST_HOME/0001-Add-scheduling-policies-for-old-kernels.patch
+	make NUMA=0 cyclictest
 }
 
 function test_deploy {
-- 
2.7.4

[Fuego] [PATCH 2/3] cyclictest: modify specs

[Daniel Sangorrin]

cyclictest params can be very flexible so instead of
giving a fixed pattern we use a single variable PARAMS
to hold them.

TODO: make builds parameterizable so that users can
modify the PARAMS from ftc without needing to create
a new spec.

Signed-off-by: Daniel Sangorrin <daniel.sangorrin@toshiba.co.jp>
---
 engine/tests/Benchmark.cyclictest/fuego_test.sh | 3 +--
 engine/tests/Benchmark.cyclictest/spec.json     | 5 ++++-
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/engine/tests/Benchmark.cyclictest/fuego_test.sh b/engine/tests/Benchmark.cyclictest/fuego_test.sh
index efa892c..a32eb6e 100755
--- a/engine/tests/Benchmark.cyclictest/fuego_test.sh
+++ b/engine/tests/Benchmark.cyclictest/fuego_test.sh
@@ -10,8 +10,7 @@ function test_deploy {
 }
 
 function test_run {
-	assert_define BENCHMARK_CYCLICTEST_LOOPS
-	report "cd $BOARD_TESTDIR/fuego.$TESTDIR; ./cyclictest -t 2 -l $BENCHMARK_CYCLICTEST_LOOPS -q"  
+	report "cd $BOARD_TESTDIR/fuego.$TESTDIR; ./cyclictest $BENCHMARK_CYCLICTEST_PARAMS"
 }
 
 
diff --git a/engine/tests/Benchmark.cyclictest/spec.json b/engine/tests/Benchmark.cyclictest/spec.json
index 774c742..a0a3a00 100644
--- a/engine/tests/Benchmark.cyclictest/spec.json
+++ b/engine/tests/Benchmark.cyclictest/spec.json
@@ -2,7 +2,10 @@
     "testName": "Benchmark.cyclictest",
     "specs": {
         "default": {
-            "LOOPS":"10000"
+            "PARAMS": "-S -p 60 -m -D 20 -i 1000 -q"
+        },
+        "twothreads": {
+            "PARAMS": "-t 2 -l 10000 -q"
         }
     }
 }
-- 
2.7.4

[Fuego] [PATCH 3/3] cyclictest: modify parsing and test measures

[Daniel Sangorrin]

cyclictest can have more or less result lines depending on
how many threads are executed.

PROBLEM: Fuego currently does not support arrays of arbitrary
size that can be populated dynamically. The reason
is because we splitted reference.json from parser.py which
could have easily solved that.

For now, I have decided to calculate the global minimum,
average and maximum latency variables for all threads (1
thread per core in the new default spec).

TODO: I tried to write chart_config.json to only display the
maximum values but it didn't work as expected (all measures
are shown).

Signed-off-by: Daniel Sangorrin <daniel.sangorrin@toshiba.co.jp>
---
 .../tests/Benchmark.cyclictest/chart_config.json   |  4 ++-
 engine/tests/Benchmark.cyclictest/criteria.json    | 26 ++++++++++++++++
 engine/tests/Benchmark.cyclictest/parser.py        | 35 ++++++++++------------
 engine/tests/Benchmark.cyclictest/reference.json   | 26 ++++++++++++++++
 engine/tests/Benchmark.cyclictest/reference.log    | 17 -----------
 5 files changed, 71 insertions(+), 37 deletions(-)
 create mode 100644 engine/tests/Benchmark.cyclictest/criteria.json
 create mode 100644 engine/tests/Benchmark.cyclictest/reference.json
 delete mode 100644 engine/tests/Benchmark.cyclictest/reference.log

diff --git a/engine/tests/Benchmark.cyclictest/chart_config.json b/engine/tests/Benchmark.cyclictest/chart_config.json
index 101d5ac..31b0d37 100644
--- a/engine/tests/Benchmark.cyclictest/chart_config.json
+++ b/engine/tests/Benchmark.cyclictest/chart_config.json
@@ -1,3 +1,5 @@
 {
-        "cyclictest":["Thread0", "Thread1"]
+	"chart_type": "measure_plot",
+	"measures": ["latencies.max_latency"],
+	"test_sets": ["default"]
 }
diff --git a/engine/tests/Benchmark.cyclictest/criteria.json b/engine/tests/Benchmark.cyclictest/criteria.json
new file mode 100644
index 0000000..f137d55
--- /dev/null
+++ b/engine/tests/Benchmark.cyclictest/criteria.json
@@ -0,0 +1,26 @@
+{
+    "schema_version":"1.0",
+    "criteria":[
+        {
+            "tguid":"default.latencies.max_latency",
+            "reference":{
+                "value":1000,
+                "operator":"le"
+            }
+        },
+        {
+            "tguid":"default.latencies.min_latency",
+            "reference":{
+                "value":1000,
+                "operator":"le"
+            }
+        },
+        {
+            "tguid":"default.latencies.avg_latency",
+            "reference":{
+                "value":1000,
+                "operator":"le"
+            }
+        }
+    ]
+}
diff --git a/engine/tests/Benchmark.cyclictest/parser.py b/engine/tests/Benchmark.cyclictest/parser.py
index 44425ea..3f4f05d 100755
--- a/engine/tests/Benchmark.cyclictest/parser.py
+++ b/engine/tests/Benchmark.cyclictest/parser.py
@@ -1,26 +1,23 @@
 #!/usr/bin/python
-# See common.py for description of command-line arguments
-
 import os, re, sys
 sys.path.insert(0, os.environ['FUEGO_CORE'] + '/engine/scripts/parser')
 import common as plib
 
-ref_section_pat = "^\[[\w\d_ .]+.[gle]{2}\]"
-cur_search_pat = re.compile("^T:([\s\d]+)(.*)P:(.*)C:(.*)Min:([\s\d]+)Act:([\s\d]+)Avg:([\s\d]+)Max:([\s\d]+)",re.MULTILINE)
-
-res_dict = {}
-cur_dict = {}
-
-pat_result = plib.parse(cur_search_pat)
-if pat_result:
-	cur_dict["Thread0.Min"] = '%d' % int(pat_result[0][4])
-	cur_dict["Thread0.Act"] = '%d' % int(pat_result[0][5])
-	cur_dict["Thread0.Avg"] = '%d' % int(pat_result[0][6])
-	cur_dict["Thread0.Max"] = '%d' % int(pat_result[0][7])
-	cur_dict["Thread1.Min"] = '%d' % int(pat_result[1][4])
-	cur_dict["Thread1.Act"] = '%d' % int(pat_result[1][5])
-	cur_dict["Thread1.Avg"] = '%d' % int(pat_result[1][6])
-	cur_dict["Thread1.Max"] = '%d' % int(pat_result[1][7])
+regex_string = "^T:.*Min:\s+(\d+).*Avg:\s+(\d+) Max:\s+(\d+)"
+measurements = {}
+matches = plib.parse_log(regex_string)
 
+if matches:
+	min_latencies = []
+	avg_latencies = []
+	max_latencies = []
+	for thread in matches:
+		min_latencies.append(float(thread[0]))
+		avg_latencies.append(float(thread[1]))
+		max_latencies.append(float(thread[2]))
+	measurements['default.latencies'] = [
+		{"name": "max_latency", "measure" : max(max_latencies)},
+		{"name": "min_latency", "measure" : min(min_latencies)},
+		{"name": "avg_latency", "measure" : sum(avg_latencies)/len(avg_latencies)}]
 
-sys.exit(plib.process_data(ref_section_pat, cur_dict, 'm', 'usec'))
+sys.exit(plib.process(measurements))
diff --git a/engine/tests/Benchmark.cyclictest/reference.json b/engine/tests/Benchmark.cyclictest/reference.json
new file mode 100644
index 0000000..415a8dd
--- /dev/null
+++ b/engine/tests/Benchmark.cyclictest/reference.json
@@ -0,0 +1,26 @@
+{
+    "test_sets":[
+        {
+            "name":"default",
+            "test_cases":[
+                {
+                    "name":"latencies",
+                    "measurements":[
+                        {
+                            "name":"max_latency",
+                            "unit":"us"
+                        },
+                        {
+                            "name":"min_latency",
+                            "unit":"us"
+                        },
+                        {
+                            "name":"avg_latency",
+                            "unit":"us"
+                        }
+                    ]
+                }
+            ]
+        }
+    ]
+}
diff --git a/engine/tests/Benchmark.cyclictest/reference.log b/engine/tests/Benchmark.cyclictest/reference.log
deleted file mode 100644
index 83a959d..0000000
--- a/engine/tests/Benchmark.cyclictest/reference.log
+++ /dev/null
@@ -1,17 +0,0 @@
-#sdfdsf
-[Thread0.Min|le]
-10000000
-[Thread0.Act|le]
-10000000
-[Thread0.Avg|le]
-10000000
-[Thread0.Max|le]
-10000000
-[Thread1.Min|le]
-10000000
-[Thread1.Act|le]
-10000000
-[Thread1.Avg|le]
-10000000
-[Thread1.Max|le]
-10000000
-- 
2.7.4

Re: [Fuego] cyclictest patches

[Daniel Sangorrin]

> -----Original Message-----
> From: fuego-bounces@lists.linuxfoundation.org [mailto:fuego-bounces@lists.linuxfoundation.org] On Behalf Of Daniel Sangorrin
> I have modified the cyclictest benchmark test.
> 
> - cyclictest: update source code tarball
> 
> The previous source code was too old and lacked some important
> flags. I added a patch to build the test for toolchains based on
> old kernels (maybe this should be upstream)
> 
> TODO: rt-tests contains other tests that are already in Fuego
> and can share the same tarball.
> TODO: cyclictest requires some disturbance running in
> background.

Note: this patch is on my master branch (it was too big for the mailing list).

Regards,
Daniel

Re: [Fuego] cyclictest patches

[Bird, Timothy]

> -----Original Message-----
> From: Daniel Sangorrin on Tuesday, October 31, 2017 9:32 AM
> I have modified the cyclictest benchmark test.

Thanks very much for this work.
 
> - cyclictest: update source code tarball
> 
> The previous source code was too old and lacked some important
> flags. I added a patch to build the test for toolchains based on
> old kernels (maybe this should be upstream)
> 
> TODO: rt-tests contains other tests that are already in Fuego
> and can share the same tarball.
> TODO: cyclictest requires some disturbance running in
> background.
> 
> - [PATCH 1/3] cyclictest: update build function
> 
> I have tested the build in x86_64 and an ARM yocto-based toolchain.
> 
> - [PATCH 2/3] cyclictest: modify specs
> 
> The original spec was hardwiring cyclictest to use 2 threads. Now
> the default is using 1 thread per core (with affinity).
> 
> - [PATCH 3/3] cyclictest: modify parsing and test measures
> 
> This is the biggest change and shows a limitation on using
> static definitions (reference.json). The problem is described
> on the commit message, but basically it boils down to the
> fact that the amount of test measures may depend on the
> hardware (e.g.: number of cores) or spec (e.g.: number of
> threads). We need a mechanism to specify arrays of measures
> that share the same 'units' and criteria.
> 
> I also had problems understanding chart_config.json.
> With the one I have committed I expected that ony "max_latency"
> would show up on jenkins. However all of them are displayed.

The chart_config.json did not get quite all the way to completion
in terms of intended features.  (That's a nice way of saying that I
didn't finish it in time for the 1.2 release.)  Finishing it should be relatively
straightforward, but I really wanted to make the 1.2 release before
I went to ELCE.  I did, just barely, but didn't have time to announce it.
I'm still trying to get documentation ready for the 1.2 release, for 
the announcement.  I'll try to mention the limitations with chart_config.json
in the documentation I work on this week.  Fully finishing the new
charting (with flexible output control) and report generation is on
my 1.3 to-do list.  I'm hoping that 1.3 will have a much shorter development
cycle than 1.2 did.

I'll respond to individual patches with any issues or questions I have.
 -- Tim

Re: [Fuego] cyclictest patches

[Bird, Timothy]

> -----Original Message-----
> From: Daniel Sangorrin on Tuesday, October 31, 2017 9:40 AM
> > -----Original Message-----
> > From: fuego-bounces@lists.linuxfoundation.org [mailto:fuego-
> bounces@lists.linuxfoundation.org] On Behalf Of Daniel Sangorrin
> > I have modified the cyclictest benchmark test.
> >
> > - cyclictest: update source code tarball
> >
> > The previous source code was too old and lacked some important
> > flags. I added a patch to build the test for toolchains based on
> > old kernels (maybe this should be upstream)
> >
> > TODO: rt-tests contains other tests that are already in Fuego
> > and can share the same tarball.
> > TODO: cyclictest requires some disturbance running in
> > background.
> 
> Note: this patch is on my master branch (it was too big for the mailing list).

Is this tarball made on your system, or does it come from some public source?
In general, I'd like to avoid custom-made tarballs in Fuego.  If it's
normal cyclictest source, with some extra custom sauce, I think it would
be better to keep the 2 parts separate, so it's easier to update.

I'll take a look and let you know what I think.
 -- Tim

Re: [Fuego] [PATCH 1/3] cyclictest: update build function

[Bird, Timothy]

> -----Original Message-----
> From: fuego-bounces@lists.linuxfoundation.org [mailto:fuego-
> bounces@lists.linuxfoundation.org] On Behalf Of Daniel Sangorrin
> Sent: Tuesday, October 31, 2017 9:32 AM
> To: fuego@lists.linuxfoundation.org
> Subject: [Fuego] [PATCH 1/3] cyclictest: update build function
> 
> Signed-off-by: Daniel Sangorrin <daniel.sangorrin@toshiba.co.jp>
> ---
>  engine/tests/Benchmark.cyclictest/fuego_test.sh | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/engine/tests/Benchmark.cyclictest/fuego_test.sh
> b/engine/tests/Benchmark.cyclictest/fuego_test.sh
> index 2631a82..efa892c 100755
> --- a/engine/tests/Benchmark.cyclictest/fuego_test.sh
> +++ b/engine/tests/Benchmark.cyclictest/fuego_test.sh
> @@ -1,7 +1,8 @@
> -tarball=cyclictest.tar.gz
> +tarball=rt-tests-1.0.tar.gz
> 
>  function test_build {
> -    make CC="$CC" AR="$AR" RANLIB="$RANLIB" CXX="$CXX" CPP="$CPP"
> CXXCPP="$CXXCPP" CFLAGS="$CFLAGS" LDFLAGS="$LDFLAGS"
> +	patch -p1 -N -s < $TEST_HOME/0001-Add-scheduling-policies-for-old-
> kernels.patch

Where is this patch?  I suspect (I haven't looked yet), that it's inside
the tarball.  But these types of things should be kept outside the
tarball to make it easier to keep track of them separate from upstream
source (and, to make it easier to mainline them, of course).

> +	make NUMA=0 cyclictest
>  }
> 
>  function test_deploy {
> --
> 2.7.4

Re: [Fuego] [PATCH 1/3] cyclictest: update build function

[Bird, Timothy]

> -----Original Message-----
> From:  Daniel Sangorrin on Tuesday, October 31, 2017 9:32 AM
> To: fuego@lists.linuxfoundation.org
> Subject: [Fuego] [PATCH 1/3] cyclictest: update build function
> 
> Signed-off-by: Daniel Sangorrin <daniel.sangorrin@toshiba.co.jp>
> ---
>  engine/tests/Benchmark.cyclictest/fuego_test.sh | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/engine/tests/Benchmark.cyclictest/fuego_test.sh
> b/engine/tests/Benchmark.cyclictest/fuego_test.sh
> index 2631a82..efa892c 100755
> --- a/engine/tests/Benchmark.cyclictest/fuego_test.sh
> +++ b/engine/tests/Benchmark.cyclictest/fuego_test.sh
> @@ -1,7 +1,8 @@
> -tarball=cyclictest.tar.gz
> +tarball=rt-tests-1.0.tar.gz
Where is rt-tests-1.0.tar.gz obtained from?

> 
>  function test_build {
> -    make CC="$CC" AR="$AR" RANLIB="$RANLIB" CXX="$CXX" CPP="$CPP"
> CXXCPP="$CXXCPP" CFLAGS="$CFLAGS" LDFLAGS="$LDFLAGS"
> +	patch -p1 -N -s < $TEST_HOME/0001-Add-scheduling-policies-for-old-
> kernels.patch
> +	make NUMA=0 cyclictest
>  }
> 
>  function test_deploy {
> --
> 2.7.4
> 
> 
> _______________________________________________
> Fuego mailing list
> Fuego@lists.linuxfoundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/fuego

Re: [Fuego] [PATCH 1/3] cyclictest: update build function

[Bird, Timothy]

OK - never mind.  I see that it's outside the tarball.

Thanks!
 -- Tim

> -----Original Message-----
> From: fuego-bounces@lists.linuxfoundation.org [mailto:fuego-
> bounces@lists.linuxfoundation.org] On Behalf Of Bird, Timothy
> Sent: Tuesday, October 31, 2017 6:30 PM
> To: Daniel Sangorrin <daniel.sangorrin@toshiba.co.jp>;
> fuego@lists.linuxfoundation.org
> Subject: Re: [Fuego] [PATCH 1/3] cyclictest: update build function
> 
> 
> 
> > -----Original Message-----
> > From: fuego-bounces@lists.linuxfoundation.org [mailto:fuego-
> > bounces@lists.linuxfoundation.org] On Behalf Of Daniel Sangorrin
> > Sent: Tuesday, October 31, 2017 9:32 AM
> > To: fuego@lists.linuxfoundation.org
> > Subject: [Fuego] [PATCH 1/3] cyclictest: update build function
> >
> > Signed-off-by: Daniel Sangorrin <daniel.sangorrin@toshiba.co.jp>
> > ---
> >  engine/tests/Benchmark.cyclictest/fuego_test.sh | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/engine/tests/Benchmark.cyclictest/fuego_test.sh
> > b/engine/tests/Benchmark.cyclictest/fuego_test.sh
> > index 2631a82..efa892c 100755
> > --- a/engine/tests/Benchmark.cyclictest/fuego_test.sh
> > +++ b/engine/tests/Benchmark.cyclictest/fuego_test.sh
> > @@ -1,7 +1,8 @@
> > -tarball=cyclictest.tar.gz
> > +tarball=rt-tests-1.0.tar.gz
> >
> >  function test_build {
> > -    make CC="$CC" AR="$AR" RANLIB="$RANLIB" CXX="$CXX" CPP="$CPP"
> > CXXCPP="$CXXCPP" CFLAGS="$CFLAGS" LDFLAGS="$LDFLAGS"
> > +	patch -p1 -N -s < $TEST_HOME/0001-Add-scheduling-policies-for-old-
> > kernels.patch
> 
> Where is this patch?  I suspect (I haven't looked yet), that it's inside
> the tarball.  But these types of things should be kept outside the
> tarball to make it easier to keep track of them separate from upstream
> source (and, to make it easier to mainline them, of course).
> 
> > +	make NUMA=0 cyclictest
> >  }
> >
> >  function test_deploy {
> > --
> > 2.7.4
> 
> 
> _______________________________________________
> Fuego mailing list
> Fuego@lists.linuxfoundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/fuego

Re: [Fuego] cyclictest patches

[Bird, Timothy]

> -----Original Message-----
> From: fuego-bounces@lists.linuxfoundation.org [mailto:fuego-
> bounces@lists.linuxfoundation.org] On Behalf Of Bird, Timothy
> Sent: Tuesday, October 31, 2017 6:28 PM
> To: Daniel Sangorrin <daniel.sangorrin@toshiba.co.jp>;
> fuego@lists.linuxfoundation.org
> Subject: Re: [Fuego] cyclictest patches
> 
> 
> 
> > -----Original Message-----
> > From: Daniel Sangorrin on Tuesday, October 31, 2017 9:40 AM
> > > -----Original Message-----
> > > From: fuego-bounces@lists.linuxfoundation.org [mailto:fuego-
> > bounces@lists.linuxfoundation.org] On Behalf Of Daniel Sangorrin
> > > I have modified the cyclictest benchmark test.
> > >
> > > - cyclictest: update source code tarball
> > >
> > > The previous source code was too old and lacked some important
> > > flags. I added a patch to build the test for toolchains based on
> > > old kernels (maybe this should be upstream)
> > >
> > > TODO: rt-tests contains other tests that are already in Fuego
> > > and can share the same tarball.
> > > TODO: cyclictest requires some disturbance running in
> > > background.
> >
> > Note: this patch is on my master branch (it was too big for the mailing list).
> 
> Is this tarball made on your system, or does it come from some public
> source?

Looks like it comes straight from the upstream git repo.
I don't see an online (upstream) source for the tarball.

Is there some reason we're not doing the following?

diff --git a/engine/tests/Benchmark.cyclictest/fuego_test.sh b/engine/tests/Benchmark.cyclictest/fuego_test.sh
index a32eb6e..f7c6350 100755
--- a/engine/tests/Benchmark.cyclictest/fuego_test.sh
+++ b/engine/tests/Benchmark.cyclictest/fuego_test.sh
@@ -1,4 +1,5 @@
-tarball=rt-tests-1.0.tar.gz
+gitrepo=https://git.kernel.org/pub/scm/utils/rt-tests/rt-tests.git
+gitref=stable/v1.0
 
 function test_build {
 	patch -p1 -N -s < $TEST_HOME/0001-Add-scheduling-policies-for-old-kernels.patch

Just checking.  That seemed to work OK for me.  I presume that the gitrepo
mentioned above is the canonical upstream source.
 -- Tim

Re: [Fuego] [PATCH 2/3] cyclictest: modify specs

[Bird, Timothy]

> -----Original Message-----
> From: fuego-bounces@lists.linuxfoundation.org [mailto:fuego-
> bounces@lists.linuxfoundation.org] On Behalf Of Daniel Sangorrin
> Sent: Tuesday, October 31, 2017 9:32 AM
> To: fuego@lists.linuxfoundation.org
> Subject: [Fuego] [PATCH 2/3] cyclictest: modify specs
> 
> cyclictest params can be very flexible so instead of
> giving a fixed pattern we use a single variable PARAMS
> to hold them.
> 
> TODO: make builds parameterizable so that users can
> modify the PARAMS from ftc without needing to create
> a new spec.
> 
> Signed-off-by: Daniel Sangorrin <daniel.sangorrin@toshiba.co.jp>
> ---
>  engine/tests/Benchmark.cyclictest/fuego_test.sh | 3 +--
>  engine/tests/Benchmark.cyclictest/spec.json     | 5 ++++-
>  2 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/engine/tests/Benchmark.cyclictest/fuego_test.sh
> b/engine/tests/Benchmark.cyclictest/fuego_test.sh
> index efa892c..a32eb6e 100755
> --- a/engine/tests/Benchmark.cyclictest/fuego_test.sh
> +++ b/engine/tests/Benchmark.cyclictest/fuego_test.sh
> @@ -10,8 +10,7 @@ function test_deploy {
>  }
> 
>  function test_run {
> -	assert_define BENCHMARK_CYCLICTEST_LOOPS
> -	report "cd $BOARD_TESTDIR/fuego.$TESTDIR; ./cyclictest -t 2 -l
> $BENCHMARK_CYCLICTEST_LOOPS -q"
> +	report "cd $BOARD_TESTDIR/fuego.$TESTDIR; ./cyclictest
> $BENCHMARK_CYCLICTEST_PARAMS"
>  }
> 
> 
> diff --git a/engine/tests/Benchmark.cyclictest/spec.json
> b/engine/tests/Benchmark.cyclictest/spec.json
> index 774c742..a0a3a00 100644
> --- a/engine/tests/Benchmark.cyclictest/spec.json
> +++ b/engine/tests/Benchmark.cyclictest/spec.json
> @@ -2,7 +2,10 @@
>      "testName": "Benchmark.cyclictest",
>      "specs": {
>          "default": {
> -            "LOOPS":"10000"
> +            "PARAMS": "-S -p 60 -m -D 20 -i 1000 -q"
> +        },
> +        "twothreads": {
> +            "PARAMS": "-t 2 -l 10000 -q"

Here is my testlog for a beablebone black, running cyclictest
with spec 'twothreads'.  That is, the result of running
job bbb.twothreads.Benchmark.cyclictest.

# /dev/cpu_dma_latency set to 0us
T: 0 (27542) P: 0 I:1000 C:  10000 Min:     22 Act: -885 Avg:2147483647 Max:    -126
T: 1 (27543) P: 0 I:1500 C:   6690 Min:     23 Act: 3114 Avg:2147483647 Max:    -413

This doesn't look right.  Did it find a bug with the Beaglebone kernel,
or is something wrong with cyclictest, or are the params not right for this
board?

Any ideas?
 -- Tim

Re: [Fuego] cyclictest patches

[Daniel Sangorrin]

> -----Original Message-----
> From: Bird, Timothy [mailto:Tim.Bird@sony.com]
> Sent: Wednesday, November 01, 2017 2:28 AM
> To: Daniel Sangorrin; fuego@lists.linuxfoundation.org
> Subject: RE: [Fuego] cyclictest patches
> 
> 
> 
> > -----Original Message-----
> > From: Daniel Sangorrin on Tuesday, October 31, 2017 9:40 AM
> > > -----Original Message-----
> > > From: fuego-bounces@lists.linuxfoundation.org [mailto:fuego-
> > bounces@lists.linuxfoundation.org] On Behalf Of Daniel Sangorrin
> > > I have modified the cyclictest benchmark test.
> > >
> > > - cyclictest: update source code tarball
> > >
> > > The previous source code was too old and lacked some important
> > > flags. I added a patch to build the test for toolchains based on
> > > old kernels (maybe this should be upstream)
> > >
> > > TODO: rt-tests contains other tests that are already in Fuego
> > > and can share the same tarball.
> > > TODO: cyclictest requires some disturbance running in
> > > background.
> >
> > Note: this patch is on my master branch (it was too big for the mailing list).
> 
> Is this tarball made on your system, or does it come from some public source?
> In general, I'd like to avoid custom-made tarballs in Fuego.  If it's
> normal cyclictest source, with some extra custom sauce, I think it would
> be better to keep the 2 parts separate, so it's easier to update.

Sorry, it comes from https://www.kernel.org/pub/linux/utils/rt-tests/
You can check the integrity with sha256sums.asc.
However, I also prefer using the git version. Let me resend the patches.

Thanks
Daniel


> 
> I'll take a look and let you know what I think.
>  -- Tim

Re: [Fuego] [PATCH 2/3] cyclictest: modify specs

[Daniel Sangorrin]

> -----Original Message-----
> From: Bird, Timothy [mailto:Tim.Bird@sony.com]
> Sent: Wednesday, November 01, 2017 4:56 AM
> To: Daniel Sangorrin; fuego@lists.linuxfoundation.org
> Subject: RE: [Fuego] [PATCH 2/3] cyclictest: modify specs
> 
> 
> 
> > -----Original Message-----
> > From: fuego-bounces@lists.linuxfoundation.org [mailto:fuego-
> > bounces@lists.linuxfoundation.org] On Behalf Of Daniel Sangorrin
> > Sent: Tuesday, October 31, 2017 9:32 AM
> > To: fuego@lists.linuxfoundation.org
> > Subject: [Fuego] [PATCH 2/3] cyclictest: modify specs
> >
> > cyclictest params can be very flexible so instead of
> > giving a fixed pattern we use a single variable PARAMS
> > to hold them.
> >
> > TODO: make builds parameterizable so that users can
> > modify the PARAMS from ftc without needing to create
> > a new spec.
> >
> > Signed-off-by: Daniel Sangorrin <daniel.sangorrin@toshiba.co.jp>
> > ---
> >  engine/tests/Benchmark.cyclictest/fuego_test.sh | 3 +--
> >  engine/tests/Benchmark.cyclictest/spec.json     | 5 ++++-
> >  2 files changed, 5 insertions(+), 3 deletions(-)
> >
> > diff --git a/engine/tests/Benchmark.cyclictest/fuego_test.sh
> > b/engine/tests/Benchmark.cyclictest/fuego_test.sh
> > index efa892c..a32eb6e 100755
> > --- a/engine/tests/Benchmark.cyclictest/fuego_test.sh
> > +++ b/engine/tests/Benchmark.cyclictest/fuego_test.sh
> > @@ -10,8 +10,7 @@ function test_deploy {
> >  }
> >
> >  function test_run {
> > -	assert_define BENCHMARK_CYCLICTEST_LOOPS
> > -	report "cd $BOARD_TESTDIR/fuego.$TESTDIR; ./cyclictest -t 2 -l
> > $BENCHMARK_CYCLICTEST_LOOPS -q"
> > +	report "cd $BOARD_TESTDIR/fuego.$TESTDIR; ./cyclictest
> > $BENCHMARK_CYCLICTEST_PARAMS"
> >  }
> >
> >
> > diff --git a/engine/tests/Benchmark.cyclictest/spec.json
> > b/engine/tests/Benchmark.cyclictest/spec.json
> > index 774c742..a0a3a00 100644
> > --- a/engine/tests/Benchmark.cyclictest/spec.json
> > +++ b/engine/tests/Benchmark.cyclictest/spec.json
> > @@ -2,7 +2,10 @@
> >      "testName": "Benchmark.cyclictest",
> >      "specs": {
> >          "default": {
> > -            "LOOPS":"10000"
> > +            "PARAMS": "-S -p 60 -m -D 20 -i 1000 -q"
> > +        },
> > +        "twothreads": {
> > +            "PARAMS": "-t 2 -l 10000 -q"
> 
> Here is my testlog for a beablebone black, running cyclictest
> with spec 'twothreads'.  That is, the result of running
> job bbb.twothreads.Benchmark.cyclictest.
> 
> # /dev/cpu_dma_latency set to 0us
> T: 0 (27542) P: 0 I:1000 C:  10000 Min:     22 Act: -885 Avg:2147483647 Max:    -126
> T: 1 (27543) P: 0 I:1500 C:   6690 Min:     23 Act: 3114 Avg:2147483647 Max:    -413
> 
> This doesn't look right.  Did it find a bug with the Beaglebone kernel,
> or is something wrong with cyclictest, or are the params not right for this
> board?

This is a bug in cyclictest. It seems a patch was sent after that tarball was released:
http://www.spinics.net/lists/linux-rt-users/msg15372.html

So let's use the git repository as you mentioned. 
# Btw, I think there was a discussion about making a new cyclictest.. Maybe it would be
good to influence in the outputformat?

Thanks,
Daniel

> 
> Any ideas?
>  -- Tim

Re: [Fuego] [PATCH 3/3] cyclictest: modify parsing and test measures

[Bird, Timothy]

> -----Original Message-----
> From: Daniel Sangorrin on Tuesday, October 31, 2017 9:32 AM
> cyclictest can have more or less result lines depending on
> how many threads are executed.
> 
> PROBLEM: Fuego currently does not support arrays of arbitrary
> size that can be populated dynamically. The reason
> is because we splitted reference.json from parser.py which
> could have easily solved that.
> 
> For now, I have decided to calculate the global minimum,
> average and maximum latency variables for all threads (1
> thread per core in the new default spec).
> 
> TODO: I tried to write chart_config.json to only display the
> maximum values but it didn't work as expected (all measures
> are shown).

I worked on this and pushed the results.  It should work as
intended now.

I also add some limited documentation on chart_config.json
at:  http://fuegotest.org/wiki/chart_config.json
 -- Tim
> 
> Signed-off-by: Daniel Sangorrin <daniel.sangorrin@toshiba.co.jp>
> ---
>  .../tests/Benchmark.cyclictest/chart_config.json   |  4 ++-
>  engine/tests/Benchmark.cyclictest/criteria.json    | 26 ++++++++++++++++
>  engine/tests/Benchmark.cyclictest/parser.py        | 35 ++++++++++------------
>  engine/tests/Benchmark.cyclictest/reference.json   | 26
> ++++++++++++++++
>  engine/tests/Benchmark.cyclictest/reference.log    | 17 -----------
>  5 files changed, 71 insertions(+), 37 deletions(-)
>  create mode 100644 engine/tests/Benchmark.cyclictest/criteria.json
>  create mode 100644 engine/tests/Benchmark.cyclictest/reference.json
>  delete mode 100644 engine/tests/Benchmark.cyclictest/reference.log
> 
> diff --git a/engine/tests/Benchmark.cyclictest/chart_config.json
> b/engine/tests/Benchmark.cyclictest/chart_config.json
> index 101d5ac..31b0d37 100644
> --- a/engine/tests/Benchmark.cyclictest/chart_config.json
> +++ b/engine/tests/Benchmark.cyclictest/chart_config.json
> @@ -1,3 +1,5 @@
>  {
> -        "cyclictest":["Thread0", "Thread1"]
> +	"chart_type": "measure_plot",
> +	"measures": ["latencies.max_latency"],
> +	"test_sets": ["default"]
>  }
> diff --git a/engine/tests/Benchmark.cyclictest/criteria.json
> b/engine/tests/Benchmark.cyclictest/criteria.json
> new file mode 100644
> index 0000000..f137d55
> --- /dev/null
> +++ b/engine/tests/Benchmark.cyclictest/criteria.json
> @@ -0,0 +1,26 @@
> +{
> +    "schema_version":"1.0",
> +    "criteria":[
> +        {
> +            "tguid":"default.latencies.max_latency",
> +            "reference":{
> +                "value":1000,
> +                "operator":"le"
> +            }
> +        },
> +        {
> +            "tguid":"default.latencies.min_latency",
> +            "reference":{
> +                "value":1000,
> +                "operator":"le"
> +            }
> +        },
> +        {
> +            "tguid":"default.latencies.avg_latency",
> +            "reference":{
> +                "value":1000,
> +                "operator":"le"
> +            }
> +        }
> +    ]
> +}
> diff --git a/engine/tests/Benchmark.cyclictest/parser.py
> b/engine/tests/Benchmark.cyclictest/parser.py
> index 44425ea..3f4f05d 100755
> --- a/engine/tests/Benchmark.cyclictest/parser.py
> +++ b/engine/tests/Benchmark.cyclictest/parser.py
> @@ -1,26 +1,23 @@
>  #!/usr/bin/python
> -# See common.py for description of command-line arguments
> -
>  import os, re, sys
>  sys.path.insert(0, os.environ['FUEGO_CORE'] + '/engine/scripts/parser')
>  import common as plib
> 
> -ref_section_pat = "^\[[\w\d_ .]+.[gle]{2}\]"
> -cur_search_pat =
> re.compile("^T:([\s\d]+)(.*)P:(.*)C:(.*)Min:([\s\d]+)Act:([\s\d]+)Avg:([\s\d]
> +)Max:([\s\d]+)",re.MULTILINE)
> -
> -res_dict = {}
> -cur_dict = {}
> -
> -pat_result = plib.parse(cur_search_pat)
> -if pat_result:
> -	cur_dict["Thread0.Min"] = '%d' % int(pat_result[0][4])
> -	cur_dict["Thread0.Act"] = '%d' % int(pat_result[0][5])
> -	cur_dict["Thread0.Avg"] = '%d' % int(pat_result[0][6])
> -	cur_dict["Thread0.Max"] = '%d' % int(pat_result[0][7])
> -	cur_dict["Thread1.Min"] = '%d' % int(pat_result[1][4])
> -	cur_dict["Thread1.Act"] = '%d' % int(pat_result[1][5])
> -	cur_dict["Thread1.Avg"] = '%d' % int(pat_result[1][6])
> -	cur_dict["Thread1.Max"] = '%d' % int(pat_result[1][7])
> +regex_string = "^T:.*Min:\s+(\d+).*Avg:\s+(\d+) Max:\s+(\d+)"
> +measurements = {}
> +matches = plib.parse_log(regex_string)
> 
> +if matches:
> +	min_latencies = []
> +	avg_latencies = []
> +	max_latencies = []
> +	for thread in matches:
> +		min_latencies.append(float(thread[0]))
> +		avg_latencies.append(float(thread[1]))
> +		max_latencies.append(float(thread[2]))
> +	measurements['default.latencies'] = [
> +		{"name": "max_latency", "measure" : max(max_latencies)},
> +		{"name": "min_latency", "measure" : min(min_latencies)},
> +		{"name": "avg_latency", "measure" :
> sum(avg_latencies)/len(avg_latencies)}]
> 
> -sys.exit(plib.process_data(ref_section_pat, cur_dict, 'm', 'usec'))
> +sys.exit(plib.process(measurements))
> diff --git a/engine/tests/Benchmark.cyclictest/reference.json
> b/engine/tests/Benchmark.cyclictest/reference.json
> new file mode 100644
> index 0000000..415a8dd
> --- /dev/null
> +++ b/engine/tests/Benchmark.cyclictest/reference.json
> @@ -0,0 +1,26 @@
> +{
> +    "test_sets":[
> +        {
> +            "name":"default",
> +            "test_cases":[
> +                {
> +                    "name":"latencies",
> +                    "measurements":[
> +                        {
> +                            "name":"max_latency",
> +                            "unit":"us"
> +                        },
> +                        {
> +                            "name":"min_latency",
> +                            "unit":"us"
> +                        },
> +                        {
> +                            "name":"avg_latency",
> +                            "unit":"us"
> +                        }
> +                    ]
> +                }
> +            ]
> +        }
> +    ]
> +}
> diff --git a/engine/tests/Benchmark.cyclictest/reference.log
> b/engine/tests/Benchmark.cyclictest/reference.log
> deleted file mode 100644
> index 83a959d..0000000
> --- a/engine/tests/Benchmark.cyclictest/reference.log
> +++ /dev/null
> @@ -1,17 +0,0 @@
> -#sdfdsf
> -[Thread0.Min|le]
> -10000000
> -[Thread0.Act|le]
> -10000000
> -[Thread0.Avg|le]
> -10000000
> -[Thread0.Max|le]
> -10000000
> -[Thread1.Min|le]
> -10000000
> -[Thread1.Act|le]
> -10000000
> -[Thread1.Avg|le]
> -10000000
> -[Thread1.Max|le]
> -10000000
> --
> 2.7.4
> 
> 
> _______________________________________________
> Fuego mailing list
> Fuego@lists.linuxfoundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/fuego

Re: [Fuego] [PATCH 3/3] cyclictest: modify parsing and test measures

[Bird, Timothy]

> -----Original Message-----
> From: fuego-bounces@lists.linuxfoundation.org [mailto:fuego-
> bounces@lists.linuxfoundation.org] On Behalf Of Daniel Sangorrin
> Sent: Tuesday, October 31, 2017 1:32 AM
> To: fuego@lists.linuxfoundation.org
> Subject: [Fuego] [PATCH 3/3] cyclictest: modify parsing and test measures
> 
> cyclictest can have more or less result lines depending on
> how many threads are executed.
> 
> PROBLEM: Fuego currently does not support arrays of arbitrary
> size that can be populated dynamically. The reason
> is because we split reference.json from parser.py which
> could have easily solved that.
> 
> For now, I have decided to calculate the global minimum,
> average and maximum latency variables for all threads (1
> thread per core in the new default spec).

OK - I thought about this, and agree that this is a limitation we need to address.

I've created an issue for it here:
http://fuegotest.org/wiki/Issue_0061

Fuego currently has a pretty hard-coded notion of the test sets, test cases and
measures that will be produced by a test.  But there are clearly cases where
tests will generate results that are dependent on data determined at runtime
(or from other variations, like the spec - in the case of cyclictest, that affect
operation of the test.

I don't have any good answers at the moment, but I do have some ideas for
how to deal with these situations.  Just as a note, Benchmark.cyclictest is
a rather simple test (from Fuego's perspective - as it does not produce a lot
of different, complicated results), so it is a good candidate for experimenting
with solutions to the generic problem of dynamic results.

Thanks for bringing this up.
 -- Tim

Re: [Fuego] [PATCH 2/3] cyclictest: modify specs

[Bird, Timothy]

> -----Original Message-----
> From: fuego-bounces@lists.linuxfoundation.org [mailto:fuego-
> bounces@lists.linuxfoundation.org] On Behalf Of Daniel Sangorrin
> Sent: Tuesday, October 31, 2017 1:32 AM
> To: fuego@lists.linuxfoundation.org
> Subject: [Fuego] [PATCH 2/3] cyclictest: modify specs
> 
> cyclictest params can be very flexible so instead of
> giving a fixed pattern we use a single variable PARAMS
> to hold them.
> 
> TODO: make builds parameterizable so that users can
> modify the PARAMS from ftc without needing to create
> a new spec.

Just a comment on this.  I support this idea, and it's actually
pretty easy to do.  We just need to be careful that this doesn't negatively
affect another of Fuego's goal, which is sharing of 1) methods of invoking
test programs, and 2) ability to compare results from multiple systems.

Test specs are one of the primary ways that Fuego users can share their
experience with each other - by creating canonical sets of build variables
that produce meaningful results for particular test scenarios.

If use of custom parameters for tests becomes widespread, then it becomes
less useful to compare results between different Fuego sites.

I'm not saying we shouldn't add the feature, just be judicious in it's
use, and possibly provide recommendations for when it would be 
appropriate or not.
 -- Tim

Re: [Fuego] [PATCH 2/3] cyclictest: modify specs

[Bird, Timothy]

> -----Original Message-----
> From: Daniel Sangorrin on Tuesday, October 31, 2017 5:29 PM
> > -----Original Message-----
> > From: Bird, Timothy [mailto:Tim.Bird@sony.com]
...
> > Here is my testlog for a beablebone black, running cyclictest
> > with spec 'twothreads'.  That is, the result of running
> > job bbb.twothreads.Benchmark.cyclictest.
> >
> > # /dev/cpu_dma_latency set to 0us
> > T: 0 (27542) P: 0 I:1000 C:  10000 Min:     22 Act: -885 Avg:2147483647 Max:    -
> 126
> > T: 1 (27543) P: 0 I:1500 C:   6690 Min:     23 Act: 3114 Avg:2147483647 Max:    -
> 413
> >
> > This doesn't look right.  Did it find a bug with the Beaglebone kernel,
> > or is something wrong with cyclictest, or are the params not right for this
> > board?
> 
> This is a bug in cyclictest. It seems a patch was sent after that tarball was
> released:
> http://www.spinics.net/lists/linux-rt-users/msg15372.html
> 
> So let's use the git repository as you mentioned.

These results are from the test using the source from the git repository.
So, maybe the patch is not in the stable/v1.0 release, or never made
it in.  The code looks different enough that the patch on the mail list 
doesn't look like it would apply.  Can you check into this?
If we need to, we can carry the patch in Fuego, but we should probably
push the issue again if the patch didn't make it upstream.

> # Btw, I think there was a discussion about making a new cyclictest.. Maybe it
> would be
> good to influence in the outputformat?
Right now I think the output format is OK.  There are some guidelines for test
output that we should probably provide to the authors of these kinds of test
(not just the rt-test developers) for future reference.  This includes
things like making them machine-parsable, and using
markers that can be turned into unique identifiers for the test cases.

I won't personally have time to work with the rt-test upstream, but I think
it would be good if we could influence them and at least report our needs.
 -- Tim

Re: [Fuego] [PATCH 2/3] cyclictest: modify specs

[Daniel Sangorrin]

> -----Original Message-----
> From: Bird, Timothy [mailto:Tim.Bird@sony.com]
> Sent: Thursday, November 02, 2017 2:53 AM
> To: Daniel Sangorrin; fuego@lists.linuxfoundation.org
> Subject: RE: [Fuego] [PATCH 2/3] cyclictest: modify specs
> 
> > -----Original Message-----
> > From: Daniel Sangorrin on Tuesday, October 31, 2017 5:29 PM
> > > -----Original Message-----
> > > From: Bird, Timothy [mailto:Tim.Bird@sony.com]
> ...
> > > Here is my testlog for a beablebone black, running cyclictest
> > > with spec 'twothreads'.  That is, the result of running
> > > job bbb.twothreads.Benchmark.cyclictest.
> > >
> > > # /dev/cpu_dma_latency set to 0us
> > > T: 0 (27542) P: 0 I:1000 C:  10000 Min:     22 Act: -885 Avg:2147483647 Max:    -
> > 126
> > > T: 1 (27543) P: 0 I:1500 C:   6690 Min:     23 Act: 3114 Avg:2147483647 Max:    -
> > 413
> > >
> > > This doesn't look right.  Did it find a bug with the Beaglebone kernel,
> > > or is something wrong with cyclictest, or are the params not right for this
> > > board?
> >
> > This is a bug in cyclictest. It seems a patch was sent after that tarball was
> > released:
> > http://www.spinics.net/lists/linux-rt-users/msg15372.html
> >
> > So let's use the git repository as you mentioned.
> 
> These results are from the test using the source from the git repository.
> So, maybe the patch is not in the stable/v1.0 release, or never made
> it in.  The code looks different enough that the patch on the mail list
> doesn't look like it would apply.  Can you check into this?
> If we need to, we can carry the patch in Fuego, but we should probably
> push the issue again if the patch didn't make it upstream.

Yes, you are right. I was able to reproduce the negative latencies on beaglebone
and noticed they were gone on the latest development branch so I have
upgraded the test to that branch.

> > # Btw, I think there was a discussion about making a new cyclictest.. Maybe it
> > would be
> > good to influence in the outputformat?
> Right now I think the output format is OK.  There are some guidelines for test
> output that we should probably provide to the authors of these kinds of test
> (not just the rt-test developers) for future reference.  This includes
> things like making them machine-parsable, and using
> markers that can be turned into unique identifiers for the test cases.
> 
> I won't personally have time to work with the rt-test upstream, but I think
> it would be good if we could influence them and at least report our needs.

Ok, if the new format was hard to parse I will let them know.

Thanks,
Daniel

Re: [Fuego] [PATCH 2/3] cyclictest: modify specs

[Daniel Sangorrin]

> -----Original Message-----
> From: Bird, Timothy [mailto:Tim.Bird@sony.com]
> Sent: Thursday, November 02, 2017 2:16 AM
> To: Daniel Sangorrin; fuego@lists.linuxfoundation.org
> Subject: RE: [Fuego] [PATCH 2/3] cyclictest: modify specs
> 
> 
> 
> > -----Original Message-----
> > From: fuego-bounces@lists.linuxfoundation.org [mailto:fuego-
> > bounces@lists.linuxfoundation.org] On Behalf Of Daniel Sangorrin
> > Sent: Tuesday, October 31, 2017 1:32 AM
> > To: fuego@lists.linuxfoundation.org
> > Subject: [Fuego] [PATCH 2/3] cyclictest: modify specs
> >
> > cyclictest params can be very flexible so instead of
> > giving a fixed pattern we use a single variable PARAMS
> > to hold them.
> >
> > TODO: make builds parameterizable so that users can
> > modify the PARAMS from ftc without needing to create
> > a new spec.
> 
> Just a comment on this.  I support this idea, and it's actually
> pretty easy to do.  We just need to be careful that this doesn't negatively
> affect another of Fuego's goal, which is sharing of 1) methods of invoking
> test programs, and 2) ability to compare results from multiple systems.
> 
> Test specs are one of the primary ways that Fuego users can share their
> experience with each other - by creating canonical sets of build variables
> that produce meaningful results for particular test scenarios.
> 
> If use of custom parameters for tests becomes widespread, then it becomes
> less useful to compare results between different Fuego sites.
> 
> I'm not saying we shouldn't add the feature, just be judicious in it's
> use, and possibly provide recommendations for when it would be
> appropriate or not.

Yes, I was thinking that ftc could create the spec.json automatically 
on the log folder and then put the corresponding shell environment 
variable to it (actually this can be done in the general case as well). 
Once it's in the log folder, we can share it.

Another thing we could add is "default" parameters so that users
only need to modify those that are important to them and leave the
rest untouched. Maybe that's easy to do by just specifying the base
"default" parameters with -s as now, and then override the variables.

Thanks,
Daniel

Re: [Fuego] [PATCH 2/3] cyclictest: modify specs

[Bird, Timothy]

> -----Original Message-----
> From: Daniel Sangorrin [mailto:daniel.sangorrin@toshiba.co.jp]
> > -----Original Message-----
> > From: Bird, Timothy [mailto:Tim.Bird@sony.com]
> >
> > > -----Original Message-----
> > > From: Daniel Sangorrin on Tuesday, October 31, 2017 1:32 AM
> > > cyclictest params can be very flexible so instead of
> > > giving a fixed pattern we use a single variable PARAMS
> > > to hold them.
> > >
> > > TODO: make builds parameterizable so that users can
> > > modify the PARAMS from ftc without needing to create
> > > a new spec.
> >
> > Just a comment on this.  I support this idea, and it's actually
> > pretty easy to do.  We just need to be careful that this doesn't negatively
> > affect another of Fuego's goal, which is sharing of 1) methods of invoking
> > test programs, and 2) ability to compare results from multiple systems.
> >
> > Test specs are one of the primary ways that Fuego users can share their
> > experience with each other - by creating canonical sets of build variables
> > that produce meaningful results for particular test scenarios.
> >
> > If use of custom parameters for tests becomes widespread, then it
> becomes
> > less useful to compare results between different Fuego sites.
> >
> > I'm not saying we shouldn't add the feature, just be judicious in it's
> > use, and possibly provide recommendations for when it would be
> > appropriate or not.
> 
> Yes, I was thinking that ftc could create the spec.json automatically
> on the log folder and then put the corresponding shell environment
> variable to it (actually this can be done in the general case as well).
> Once it's in the log folder, we can share it.

OK - that's interesting.  I hadn't thought about saving it into
a spec file automatically.  I like that idea.

 -- Tim