All of lore.kernel.org
 help / color / mirror / Atom feed
* [Fuego] [PATCH 1/2] Kernel_build: Update to parse and get test_build time
@ 2022-04-12 11:55 sireesha.nakkala
  2022-04-12 16:33 ` sireesha.nakkala
  2022-04-14 20:51 ` Bird, Tim
  0 siblings, 2 replies; 4+ messages in thread
From: sireesha.nakkala @ 2022-04-12 11:55 UTC (permalink / raw)
  To: tim.bird; +Cc: kazuhiro3.hayashi, dinesh.kumar, sireesha, fuego

From: sireesha <sireesha.nakkala@toshiba-tsip.com>

scripts/parser/common.py: Update reference to 'Consolelog' to parse build time

chart_config.json: Add reference to get test build time

parser.py: Implement to parse test build time from console log

reference.json: Add reference for test build time

test.yaml: Add data files related to kernel build

Signed-off-by: sireesha <sireesha.nakkala@toshiba-tsip.com>
---
 scripts/parser/common.py                      |  1 +
 .../Functional.kernel_build/chart_config.json |  5 +++++
 tests/Functional.kernel_build/parser.py       | 21 +++++++++++++++++++
 tests/Functional.kernel_build/reference.json  | 20 ++++++++++++++++++
 tests/Functional.kernel_build/test.yaml       |  3 +++
 5 files changed, 50 insertions(+)
 create mode 100644 tests/Functional.kernel_build/chart_config.json
 create mode 100644 tests/Functional.kernel_build/parser.py
 create mode 100644 tests/Functional.kernel_build/reference.json

diff --git a/scripts/parser/common.py b/scripts/parser/common.py
index d7dcfbd..ba28cc1 100644
--- a/scripts/parser/common.py
+++ b/scripts/parser/common.py
@@ -114,6 +114,7 @@ for env_var in env_list:
 REF_JSON  = TEST_HOME + '/reference.json'
 CHART_CONFIG_JSON  = TEST_HOME + '/chart_config.json'
 TEST_LOG = '%s/logs/%s/%s.%s.%s.%s/testlog.txt' % (FUEGO_RW, TESTDIR, NODE_NAME, TESTSPEC, BUILD_NUMBER, BUILD_ID)
+CONSOLE_LOG = '%s/logs/%s/%s.%s.%s.%s/consolelog.txt' % (FUEGO_RW, TESTDIR, NODE_NAME, TESTSPEC, BUILD_NUMBER, BUILD_ID)
 RUN_JSON = LOGDIR + '/run.json'
 
 #Here are some pre-packaged regex strings
diff --git a/tests/Functional.kernel_build/chart_config.json b/tests/Functional.kernel_build/chart_config.json
new file mode 100644
index 0000000..a8802de
--- /dev/null
+++ b/tests/Functional.kernel_build/chart_config.json
@@ -0,0 +1,5 @@
+{
+    "chart_type": "measure_plot",
+    "measures": ["default.test_build"]
+}
+
diff --git a/tests/Functional.kernel_build/parser.py b/tests/Functional.kernel_build/parser.py
new file mode 100644
index 0000000..78f35c3
--- /dev/null
+++ b/tests/Functional.kernel_build/parser.py
@@ -0,0 +1,21 @@
+#!/usr/bin/python3
+import os, re, sys
+import common as plib
+   
+cur_search_str = "^(Fuego.test_build.duration=)\ *([\d]{1,8}.[\d]{1,4}).*"
+
+print("Reading current values from " + plib.CONSOLE_LOG)
+cur_file = open(plib.CONSOLE_LOG,'r')
+
+lines = cur_file.readlines()
+cur_file.close()
+
+measurements = {}
+for line in lines:
+    m = re.match(cur_search_str, line)
+    if m:
+        value = m.group(2)
+        measurements['default.kernel_build'] = [{"name": "test_build", "measure" : value}]
+
+sys.exit(plib.process(measurements))
+
diff --git a/tests/Functional.kernel_build/reference.json b/tests/Functional.kernel_build/reference.json
new file mode 100644
index 0000000..9d8adfc
--- /dev/null
+++ b/tests/Functional.kernel_build/reference.json
@@ -0,0 +1,20 @@
+{
+    "test_sets":[
+        {
+            "name":"default",
+            "test_cases":[
+                {
+                    "name":"kernel_build",
+                    "measurements":[
+                        {
+                            "name":"test_build",
+                            "unit":"s"
+                        }
+
+                    ]
+                }
+            ]
+        }
+    ]
+}
+
diff --git a/tests/Functional.kernel_build/test.yaml b/tests/Functional.kernel_build/test.yaml
index c076bd7..304218e 100644
--- a/tests/Functional.kernel_build/test.yaml
+++ b/tests/Functional.kernel_build/test.yaml
@@ -46,3 +46,6 @@ data_files:
     - fuego_test.sh
     - spec.json
     - test.yaml
+    - parser.py
+    - reference.json
+    - chart_config.json
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [Fuego] [PATCH 1/2] Kernel_build: Update to parse and get test_build time
  2022-04-12 11:55 [Fuego] [PATCH 1/2] Kernel_build: Update to parse and get test_build time sireesha.nakkala
@ 2022-04-12 16:33 ` sireesha.nakkala
  2022-04-14 20:51 ` Bird, Tim
  1 sibling, 0 replies; 4+ messages in thread
From: sireesha.nakkala @ 2022-04-12 16:33 UTC (permalink / raw)
  To: sireesha.nakkala, tim.bird; +Cc: kazuhiro3.hayashi, dinesh.kumar, fuego

Dear Tim,

By mistake I have sent patch wrongly, please ignore the title with  [PATCH 1/2].
It is only 1 patch related to Kernel_Build tool.
Kindly review the changes

Thanks & Regards
Sireesha

-----Original Message-----
From: sireesha.nakkala@toshiba-tsip.com <sireesha.nakkala@toshiba-tsip.com> 
Sent: Tuesday, April 12, 2022 5:25 PM
To: tim.bird@sony.com
Cc: nakkala sireesha(TSIP) <sireesha.nakkala@toshiba-tsip.com>; fuego@lists.linuxfoundation.org; dinesh kumar(TSIP) <dinesh.kumar@toshiba-tsip.com>; hayashi kazuhiro(林 和宏 □SWC◯ACT) <kazuhiro3.hayashi@toshiba.co.jp>
Subject: [PATCH 1/2] Kernel_build: Update to parse and get test_build time

From: sireesha <sireesha.nakkala@toshiba-tsip.com>

scripts/parser/common.py: Update reference to 'Consolelog' to parse build time

chart_config.json: Add reference to get test build time

parser.py: Implement to parse test build time from console log

reference.json: Add reference for test build time

test.yaml: Add data files related to kernel build

Signed-off-by: sireesha <sireesha.nakkala@toshiba-tsip.com>
---
 scripts/parser/common.py                      |  1 +
 .../Functional.kernel_build/chart_config.json |  5 +++++
 tests/Functional.kernel_build/parser.py       | 21 +++++++++++++++++++
 tests/Functional.kernel_build/reference.json  | 20 ++++++++++++++++++
 tests/Functional.kernel_build/test.yaml       |  3 +++
 5 files changed, 50 insertions(+)
 create mode 100644 tests/Functional.kernel_build/chart_config.json
 create mode 100644 tests/Functional.kernel_build/parser.py
 create mode 100644 tests/Functional.kernel_build/reference.json

diff --git a/scripts/parser/common.py b/scripts/parser/common.py index d7dcfbd..ba28cc1 100644
--- a/scripts/parser/common.py
+++ b/scripts/parser/common.py
@@ -114,6 +114,7 @@ for env_var in env_list:
 REF_JSON  = TEST_HOME + '/reference.json'
 CHART_CONFIG_JSON  = TEST_HOME + '/chart_config.json'
 TEST_LOG = '%s/logs/%s/%s.%s.%s.%s/testlog.txt' % (FUEGO_RW, TESTDIR, NODE_NAME, TESTSPEC, BUILD_NUMBER, BUILD_ID)
+CONSOLE_LOG = '%s/logs/%s/%s.%s.%s.%s/consolelog.txt' % (FUEGO_RW, 
+TESTDIR, NODE_NAME, TESTSPEC, BUILD_NUMBER, BUILD_ID)
 RUN_JSON = LOGDIR + '/run.json'
 
 #Here are some pre-packaged regex strings diff --git a/tests/Functional.kernel_build/chart_config.json b/tests/Functional.kernel_build/chart_config.json
new file mode 100644
index 0000000..a8802de
--- /dev/null
+++ b/tests/Functional.kernel_build/chart_config.json
@@ -0,0 +1,5 @@
+{
+    "chart_type": "measure_plot",
+    "measures": ["default.test_build"]
+}
+
diff --git a/tests/Functional.kernel_build/parser.py b/tests/Functional.kernel_build/parser.py
new file mode 100644
index 0000000..78f35c3
--- /dev/null
+++ b/tests/Functional.kernel_build/parser.py
@@ -0,0 +1,21 @@
+#!/usr/bin/python3
+import os, re, sys
+import common as plib
+   
+cur_search_str = "^(Fuego.test_build.duration=)\ *([\d]{1,8}.[\d]{1,4}).*"
+
+print("Reading current values from " + plib.CONSOLE_LOG) cur_file = 
+open(plib.CONSOLE_LOG,'r')
+
+lines = cur_file.readlines()
+cur_file.close()
+
+measurements = {}
+for line in lines:
+    m = re.match(cur_search_str, line)
+    if m:
+        value = m.group(2)
+        measurements['default.kernel_build'] = [{"name": "test_build", 
+"measure" : value}]
+
+sys.exit(plib.process(measurements))
+
diff --git a/tests/Functional.kernel_build/reference.json b/tests/Functional.kernel_build/reference.json
new file mode 100644
index 0000000..9d8adfc
--- /dev/null
+++ b/tests/Functional.kernel_build/reference.json
@@ -0,0 +1,20 @@
+{
+    "test_sets":[
+        {
+            "name":"default",
+            "test_cases":[
+                {
+                    "name":"kernel_build",
+                    "measurements":[
+                        {
+                            "name":"test_build",
+                            "unit":"s"
+                        }
+
+                    ]
+                }
+            ]
+        }
+    ]
+}
+
diff --git a/tests/Functional.kernel_build/test.yaml b/tests/Functional.kernel_build/test.yaml
index c076bd7..304218e 100644
--- a/tests/Functional.kernel_build/test.yaml
+++ b/tests/Functional.kernel_build/test.yaml
@@ -46,3 +46,6 @@ data_files:
     - fuego_test.sh
     - spec.json
     - test.yaml
+    - parser.py
+    - reference.json
+    - chart_config.json
--
2.20.1



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [Fuego] [PATCH 1/2] Kernel_build: Update to parse and get test_build time
  2022-04-12 11:55 [Fuego] [PATCH 1/2] Kernel_build: Update to parse and get test_build time sireesha.nakkala
  2022-04-12 16:33 ` sireesha.nakkala
@ 2022-04-14 20:51 ` Bird, Tim
  2022-04-20 11:00   ` sireesha.nakkala
  1 sibling, 1 reply; 4+ messages in thread
From: Bird, Tim @ 2022-04-14 20:51 UTC (permalink / raw)
  To: sireesha.nakkala; +Cc: kazuhiro3.hayashi, dinesh.kumar, fuego

Sireesha,

Before applying this, I have a few questions and comments.
See inline below for some of these, but first...

what is the rationale for wanting to add the build duration as a measure (or metric)
to the run.json file?  Is it to make this an element of the pass criteria for the test
(thus turning this into a kind of benchmark test) or is it just for an extra display
element for this test.  

I don't have a big problem with this, but it is unusual.  Of course, the build_kernel
test itself is a bit unusual.

Normally, a functional test will have one or more testcases that result in PASS/FAIL
and possibly it may have a criteria.json file to indicate that some FAIL results are allowed.
Usually, there are no measures in a functional test's run.json file (and no graphs in
the functional test's output).  Benchmark tests, on the other hand, have one or more
measures in their output, which need to be parsed, and evaluated against some reference
value.

The code, as submitted, adds support for parsing the build duration for the kernel build.
However, the patch does not change the test_processing() function in fuego_test.sh.  So there's
no change of this code into a benchmarking function.  It still passes or fails based on
the result of searching the build log (which, because of the use of 'log_this' in the test_build()
function (during the build phase for this test), becomes part of the test log for the
test.

This test (the current Functional.kernel_build) is meant to check that the code compiles at all,
not to measure how fast it compiles.  And the way the fuego_test.sh is structured,
the test runs the compilation on the Fuego host, not on the board.  So the resulting
metric for the test duration would not normally provide any information about
the speed of the board.  This might be confusing in your case, as I believe Toshiba
runs tests on the 'local' board, where the Fuego host is the same as the target board
(the device under test).

If you want a test that measures how long a board takes to perform a compilation
of the kernel, then this should be structured as a new Benchmark test, and
the actual compilation should be placed in a fuego_test.sh test_run() function,
rather than in the test_build() function.  Arguably, that is how this test should have
been structured also, but when this was written we were experimenting with
using the fuego_test.sh test_deploy() function to provision the kernel image onto
the board, and it was envisioned to possibly to a second testcase inside test_run
to validate that the new kernel booted on the board.  That would have made this
into both a build and a boot test, but that second testcase was never implemented.

In any event, please let me know your reason for doing this work.  If you want a
benchmark of kernel compilation speed on a board, it would be preferable to
write a new Benchmark test structured a bit differently than this.  If you just want
an extra piece of data included in the run.json file and displayed as part
of the Jenkins interface for jobs for this test, then this might be fine.

> -----Original Message-----
> From: sireesha.nakkala@toshiba-tsip.com <sireesha.nakkala@toshiba-tsip.com>
> 
> From: sireesha <sireesha.nakkala@toshiba-tsip.com>
> 
> scripts/parser/common.py: Update reference to 'Consolelog' to parse build time
> 
> chart_config.json: Add reference to get test build time
> 
> parser.py: Implement to parse test build time from console log
> 
> reference.json: Add reference for test build time
> 
> test.yaml: Add data files related to kernel build
> 
> Signed-off-by: sireesha <sireesha.nakkala@toshiba-tsip.com>
> ---
>  scripts/parser/common.py                      |  1 +
>  .../Functional.kernel_build/chart_config.json |  5 +++++
>  tests/Functional.kernel_build/parser.py       | 21 +++++++++++++++++++
>  tests/Functional.kernel_build/reference.json  | 20 ++++++++++++++++++
>  tests/Functional.kernel_build/test.yaml       |  3 +++
>  5 files changed, 50 insertions(+)
>  create mode 100644 tests/Functional.kernel_build/chart_config.json
>  create mode 100644 tests/Functional.kernel_build/parser.py
>  create mode 100644 tests/Functional.kernel_build/reference.json
> 
> diff --git a/scripts/parser/common.py b/scripts/parser/common.py
> index d7dcfbd..ba28cc1 100644
> --- a/scripts/parser/common.py
> +++ b/scripts/parser/common.py
> @@ -114,6 +114,7 @@ for env_var in env_list:
>  REF_JSON  = TEST_HOME + '/reference.json'
>  CHART_CONFIG_JSON  = TEST_HOME + '/chart_config.json'
>  TEST_LOG = '%s/logs/%s/%s.%s.%s.%s/testlog.txt' % (FUEGO_RW, TESTDIR, NODE_NAME, TESTSPEC, BUILD_NUMBER, BUILD_ID)
> +CONSOLE_LOG = '%s/logs/%s/%s.%s.%s.%s/consolelog.txt' % (FUEGO_RW, TESTDIR, NODE_NAME, TESTSPEC, BUILD_NUMBER,
> BUILD_ID)
This seems OK.

>  RUN_JSON = LOGDIR + '/run.json'
> 
>  #Here are some pre-packaged regex strings
> diff --git a/tests/Functional.kernel_build/chart_config.json b/tests/Functional.kernel_build/chart_config.json
> new file mode 100644
> index 0000000..a8802de
> --- /dev/null
> +++ b/tests/Functional.kernel_build/chart_config.json
> @@ -0,0 +1,5 @@
> +{
> +    "chart_type": "measure_plot",
> +    "measures": ["default.test_build"]
I'd rather this was "build_duration", instead of "test_build".

I think this generates confusion between:
 1) the name of the test "kernel_build"
 2) the name of the function "test_build"
 3) the name of the metric or measure "build_duration"

> +}
> +
> diff --git a/tests/Functional.kernel_build/parser.py b/tests/Functional.kernel_build/parser.py
> new file mode 100644
> index 0000000..78f35c3
> --- /dev/null
> +++ b/tests/Functional.kernel_build/parser.py
> @@ -0,0 +1,21 @@
> +#!/usr/bin/python3
> +import os, re, sys
> +import common as plib
> +
> +cur_search_str = "^(Fuego.test_build.duration=)\ *([\d]{1,8}.[\d]{1,4}).*"

The use of periods instead of spaces in the first part of this regex is confusing.
You are trying to match an exact string that Fuego outputs as part of the 
test console log.  This will work, but 'Fuego test_build duration' is more specific
and IMHO accurate, for this search string.  (That is, use a single space instead of
the '.', for this first group.

> +
> +print("Reading current values from " + plib.CONSOLE_LOG)
> +cur_file = open(plib.CONSOLE_LOG,'r')
> +
> +lines = cur_file.readlines()
> +cur_file.close()
> +
> +measurements = {}
> +for line in lines:
> +    m = re.match(cur_search_str, line)
> +    if m:
> +        value = m.group(2)
> +        measurements['default.kernel_build'] = [{"name": "test_build", "measure" : value}]
I'd prefer if the measure was called 'build_duration', instead of 'test_build'.

> +
> +sys.exit(plib.process(measurements))
> +
> diff --git a/tests/Functional.kernel_build/reference.json b/tests/Functional.kernel_build/reference.json
> new file mode 100644
> index 0000000..9d8adfc
> --- /dev/null
> +++ b/tests/Functional.kernel_build/reference.json
> @@ -0,0 +1,20 @@
> +{
> +    "test_sets":[
> +        {
> +            "name":"default",
> +            "test_cases":[
> +                {
> +                    "name":"kernel_build",
> +                    "measurements":[
> +                        {
> +                            "name":"test_build",
I'd prefer if the measure was called 'build_duration', instead of 'test_build'.

> +                            "unit":"s"
> +                        }
> +
> +                    ]
> +                }
> +            ]
> +        }
> +    ]
> +}
> +
> diff --git a/tests/Functional.kernel_build/test.yaml b/tests/Functional.kernel_build/test.yaml
> index c076bd7..304218e 100644
> --- a/tests/Functional.kernel_build/test.yaml
> +++ b/tests/Functional.kernel_build/test.yaml
> @@ -46,3 +46,6 @@ data_files:
>      - fuego_test.sh
>      - spec.json
>      - test.yaml
> +    - parser.py
> +    - reference.json
> +    - chart_config.json
> --
> 2.20.1
> 

This displays a good knowledge of how Fuego works.  Thanks for the contribution.

Please answer the questions above, and we can work out the best way to support
what you want.
 -- Tim


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Fuego] [PATCH 1/2] Kernel_build: Update to parse and get test_build time
  2022-04-14 20:51 ` Bird, Tim
@ 2022-04-20 11:00   ` sireesha.nakkala
  0 siblings, 0 replies; 4+ messages in thread
From: sireesha.nakkala @ 2022-04-20 11:00 UTC (permalink / raw)
  To: Tim.Bird; +Cc: kazuhiro3.hayashi, dinesh.kumar, fuego

Please find below inline comments

> -----Original Message-----
> From: Bird, Tim <Tim.Bird@sony.com>
> Sent: Friday, April 15, 2022 2:21 AM
> To: nakkala sireesha(TSIP) <sireesha.nakkala@toshiba-tsip.com>
> Cc: fuego@lists.linuxfoundation.org; dinesh kumar(TSIP)
> <dinesh.kumar@toshiba-tsip.com>; hayashi kazuhiro(林 和宏 □SWC◯AC
> T) <kazuhiro3.hayashi@toshiba.co.jp>
> Subject: RE: [PATCH 1/2] Kernel_build: Update to parse and get test_build
> time
> 
> Sireesha,
> 
> Before applying this, I have a few questions and comments.
> See inline below for some of these, but first...
> 
> what is the rationale for wanting to add the build duration as a measure (or
> metric) to the run.json file?  Is it to make this an element of the pass criteria
> for the test (thus turning this into a kind of benchmark test) or is it just for an
> extra display element for this test.
> 
> I don't have a big problem with this, but it is unusual.  Of course, the
> build_kernel test itself is a bit unusual.
> 
> Normally, a functional test will have one or more testcases that result in
> PASS/FAIL and possibly it may have a criteria.json file to indicate that some
> FAIL results are allowed.
> Usually, there are no measures in a functional test's run.json file (and no
> graphs in the functional test's output).  Benchmark tests, on the other hand,
> have one or more measures in their output, which need to be parsed, and
> evaluated against some reference value.
> 
> The code, as submitted, adds support for parsing the build duration for the
> kernel build.
> However, the patch does not change the test_processing() function in
> fuego_test.sh.  So there's no change of this code into a benchmarking
> function.  It still passes or fails based on the result of searching the build log
> (which, because of the use of 'log_this' in the test_build() function (during
> the build phase for this test), becomes part of the test log for the test.
> 
> This test (the current Functional.kernel_build) is meant to check that the
> code compiles at all, not to measure how fast it compiles.  And the way the
> fuego_test.sh is structured, the test runs the compilation on the Fuego host,
> not on the board.  So the resulting metric for the test duration would not
> normally provide any information about the speed of the board.  This might
> be confusing in your case, as I believe Toshiba runs tests on the 'local' board,
> where the Fuego host is the same as the target board (the device under test).
> 
> If you want a test that measures how long a board takes to perform a
> compilation of the kernel, then this should be structured as a new
> Benchmark test, and the actual compilation should be placed in a
> fuego_test.sh test_run() function, rather than in the test_build() function.
> Arguably, that is how this test should have been structured also, but when
> this was written we were experimenting with using the fuego_test.sh
> test_deploy() function to provision the kernel image onto the board, and it
> was envisioned to possibly to a second testcase inside test_run to validate
> that the new kernel booted on the board.  That would have made this into
> both a build and a boot test, but that second testcase was never
> implemented.
> 
> In any event, please let me know your reason for doing this work.  If you
> want a benchmark of kernel compilation speed on a board, it would be
> preferable to write a new Benchmark test structured a bit differently than
> this.  If you just want an extra piece of data included in the run.json file and
> displayed as part of the Jenkins interface for jobs for this test, then this might
> be fine.
> 
Thank you for explaining us in detail.

Actually we want to run this on board to measure the Kernel build time,
but in our case both host and board are same so the below changes are worked for us.
To apply this change in upstream I think now it need to change it to Benchmark test.
We are not familiar to create  a new Benchmark test in fuego-core, so currently please ignore this change.

Will come back to you if  we need to create a Benchmark kernel build test.

> > -----Original Message-----
> > From: sireesha.nakkala@toshiba-tsip.com
> > <sireesha.nakkala@toshiba-tsip.com>
> >
> > From: sireesha <sireesha.nakkala@toshiba-tsip.com>
> >
> > scripts/parser/common.py: Update reference to 'Consolelog' to parse
> > build time
> >
> > chart_config.json: Add reference to get test build time
> >
> > parser.py: Implement to parse test build time from console log
> >
> > reference.json: Add reference for test build time
> >
> > test.yaml: Add data files related to kernel build
> >
> > Signed-off-by: sireesha <sireesha.nakkala@toshiba-tsip.com>
> > ---
> >  scripts/parser/common.py                      |  1 +
> >  .../Functional.kernel_build/chart_config.json |  5 +++++
> >  tests/Functional.kernel_build/parser.py       | 21 +++++++++++++++++++
> >  tests/Functional.kernel_build/reference.json  | 20 ++++++++++++++++++
> >  tests/Functional.kernel_build/test.yaml       |  3 +++
> >  5 files changed, 50 insertions(+)
> >  create mode 100644 tests/Functional.kernel_build/chart_config.json
> >  create mode 100644 tests/Functional.kernel_build/parser.py
> >  create mode 100644 tests/Functional.kernel_build/reference.json
> >
> > diff --git a/scripts/parser/common.py b/scripts/parser/common.py index
> > d7dcfbd..ba28cc1 100644
> > --- a/scripts/parser/common.py
> > +++ b/scripts/parser/common.py
> > @@ -114,6 +114,7 @@ for env_var in env_list:
> >  REF_JSON  = TEST_HOME + '/reference.json'
> >  CHART_CONFIG_JSON  = TEST_HOME + '/chart_config.json'
> >  TEST_LOG = '%s/logs/%s/%s.%s.%s.%s/testlog.txt' % (FUEGO_RW,
> TESTDIR,
> > NODE_NAME, TESTSPEC, BUILD_NUMBER, BUILD_ID)
> > +CONSOLE_LOG = '%s/logs/%s/%s.%s.%s.%s/consolelog.txt' %
> (FUEGO_RW,
> > +TESTDIR, NODE_NAME, TESTSPEC, BUILD_NUMBER,
> > BUILD_ID)
> This seems OK.
> 
> >  RUN_JSON = LOGDIR + '/run.json'
> >
> >  #Here are some pre-packaged regex strings diff --git
> > a/tests/Functional.kernel_build/chart_config.json
> > b/tests/Functional.kernel_build/chart_config.json
> > new file mode 100644
> > index 0000000..a8802de
> > --- /dev/null
> > +++ b/tests/Functional.kernel_build/chart_config.json
> > @@ -0,0 +1,5 @@
> > +{
> > +    "chart_type": "measure_plot",
> > +    "measures": ["default.test_build"]
> I'd rather this was "build_duration", instead of "test_build".
> 
> I think this generates confusion between:
>  1) the name of the test "kernel_build"
>  2) the name of the function "test_build"
>  3) the name of the metric or measure "build_duration"
> 
> > +}
> > +
> > diff --git a/tests/Functional.kernel_build/parser.py
> > b/tests/Functional.kernel_build/parser.py
> > new file mode 100644
> > index 0000000..78f35c3
> > --- /dev/null
> > +++ b/tests/Functional.kernel_build/parser.py
> > @@ -0,0 +1,21 @@
> > +#!/usr/bin/python3
> > +import os, re, sys
> > +import common as plib
> > +
> > +cur_search_str = "^(Fuego.test_build.duration=)\ *([\d]{1,8}.[\d]{1,4}).*"
> 
> The use of periods instead of spaces in the first part of this regex is
> confusing.
> You are trying to match an exact string that Fuego outputs as part of the test
> console log.  This will work, but 'Fuego test_build duration' is more specific
> and IMHO accurate, for this search string.  (That is, use a single space instead
> of the '.', for this first group.
> 
> > +
> > +print("Reading current values from " + plib.CONSOLE_LOG) cur_file =
> > +open(plib.CONSOLE_LOG,'r')
> > +
> > +lines = cur_file.readlines()
> > +cur_file.close()
> > +
> > +measurements = {}
> > +for line in lines:
> > +    m = re.match(cur_search_str, line)
> > +    if m:
> > +        value = m.group(2)
> > +        measurements['default.kernel_build'] = [{"name":
> > +"test_build", "measure" : value}]
> I'd prefer if the measure was called 'build_duration', instead of 'test_build'.
> 
> > +
> > +sys.exit(plib.process(measurements))
> > +
> > diff --git a/tests/Functional.kernel_build/reference.json
> > b/tests/Functional.kernel_build/reference.json
> > new file mode 100644
> > index 0000000..9d8adfc
> > --- /dev/null
> > +++ b/tests/Functional.kernel_build/reference.json
> > @@ -0,0 +1,20 @@
> > +{
> > +    "test_sets":[
> > +        {
> > +            "name":"default",
> > +            "test_cases":[
> > +                {
> > +                    "name":"kernel_build",
> > +                    "measurements":[
> > +                        {
> > +                            "name":"test_build",
> I'd prefer if the measure was called 'build_duration', instead of 'test_build'.
> 
> > +                            "unit":"s"
> > +                        }
> > +
> > +                    ]
> > +                }
> > +            ]
> > +        }
> > +    ]
> > +}
> > +
> > diff --git a/tests/Functional.kernel_build/test.yaml
> > b/tests/Functional.kernel_build/test.yaml
> > index c076bd7..304218e 100644
> > --- a/tests/Functional.kernel_build/test.yaml
> > +++ b/tests/Functional.kernel_build/test.yaml
> > @@ -46,3 +46,6 @@ data_files:
> >      - fuego_test.sh
> >      - spec.json
> >      - test.yaml
> > +    - parser.py
> > +    - reference.json
> > +    - chart_config.json
> > --
> > 2.20.1
> >
> 
> This displays a good knowledge of how Fuego works.  Thanks for the
> contribution.
> 
> Please answer the questions above, and we can work out the best way to
> support what you want.
>  -- Tim


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2022-04-20 11:00 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-12 11:55 [Fuego] [PATCH 1/2] Kernel_build: Update to parse and get test_build time sireesha.nakkala
2022-04-12 16:33 ` sireesha.nakkala
2022-04-14 20:51 ` Bird, Tim
2022-04-20 11:00   ` sireesha.nakkala

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.