[Fuego] [PATCH 2/2] ftc: gen_report: Add new functions for getting data to, generate the report
Tim.Bird at sony.com
Tim.Bird at sony.com
Tue Feb 27 19:47:27 UTC 2018
comments inline below.
> -----Original Message-----
> From: Hoang Van Tuyen
>
> Hello Tim,
>
> I am working on gen-report feature (support several other format like
> html, pdf, ...).
>
> The attached patch defines two new functions used for generating data
> for the report.
>
> Could you please check it?
>
> ==========================================================
> =================
>
> For reusability, the two functions have been defined with purpose like
> below:
> gen_header_for_report: Get value for fields in the header_fields
> (test, board,
> report_date, ...).
> gen_data_for_report: Get value for fields in the fields variable
> (test_name,
> spec, timestamp, ...).
>
> This commit also modifies gen_text_report function to compatible with
> defining the two new funtions.
> Also, correct some grammar mistakes.
>
> Signed-off-by: Hoang Van Tuyen <tuyen.hoangvan at toshiba-tsdv.com>
> ---
> engine/scripts/ftc | 78
> ++++++++++++++++++++++++++++++++----------------------
> 1 file changed, 47 insertions(+), 31 deletions(-)
>
> diff --git a/engine/scripts/ftc b/engine/scripts/ftc
> index 44e4da7..a2f5a78 100755
> --- a/engine/scripts/ftc
> +++ b/engine/scripts/ftc
> @@ -71,7 +71,7 @@ def pvar(name):
>
> quiet = 0
> verbose = 0
> -use_statusouput = 1
> +use_statusoutput = 1
Hmmm. This variable assignment was never used by ftc due
to this mis-spelling. Note that the global variable is always
set to 0 in main() anyway. That also makes this definition
superfluous. I'm just going to remove this bogus assignment
completely.
I could give a long story about why this variable was even
introduced, but the logic of its current usage in 'ftc' is a mess.
Thanks for bringing this to my attention.
> server = jenkins.Jenkins('http://localhost:8080/fuego')
>
> # keep configuration file in /fuego-ro/conf area
> @@ -1745,7 +1745,7 @@ class where_class:
> found = False
> try:
> run_value = run.__dict__[self.field_name]
> - found = true
> + found = True
Wow. This is a pernicious bug that resulted in this try statement
ALWAYS raising an exception, resulting in found always being
False, and the code ALWAYS, therefore, reading the run.json
file, even when the field could have been read from the run
object. The logic would all still have worked, but this would
have resulted in big slowdowns processing data.
Great find! Thanks.
It would have been better if the above 2 bugfixes were provided in separate
commits. I'm not real strict about this, because so far we haven't done a lot
of git bisecting of Fuego bugs. But in the future, please isolate unrelated
changes into separate commits (/patches). Thanks.
> except:
> pass
>
> @@ -1849,13 +1849,8 @@ def do_list_runs(conf, options):
>
> sys.exit(0)
>
> -def gen_text_report(run_list, run_map, header_fields, fields):
> - # generate header
> - ddash_line="="*70 + "\n"
> - report = ddash_line
> - title = "Fuego Test Report"
> - report += " **** %s ****\n" % title
> - report_date = time.strftime("%Y-%m-%d_%H:%M:%S")
> +def gen_header_for_report(run_list, run_map, header_fields, fields):
> + header_report = ""
>
> for field in header_fields:
> if field=="report_date":
> @@ -1875,22 +1870,14 @@ def gen_text_report(run_list, run_map,
> header_fields, fields):
> else:
> for value in value_list[1:]:
> val_str += ", "+value
> + header_report += " %-20s: %s\n" % (field, val_str)
This doesn't quite go far enough isolating the field value gathering
from the formatting. This string formatting should be
left as an exercise for gen_text_report. I'll fix that in a subsequent
commit.
>
> - line = " %-20s: %s\n" % (field, val_str)
> - report += line
> - report += ddash_line
> + return header_report
gen_header_for_report should return a list of tuples (field_name, value_str) for the header.
This needs to be refactored a bit, including a name change, IMHO.
I'll do that.
>
> +def gen_data_for_report(run_list, run_map, header_fields, fields):
> + data_to_report = []
> # generate list
>
> - # start with a header line
> - dash_line = "-"*(21*len(fields)+2) + "\n"
> - report += dash_line
> - line = " "
> - for field in fields:
> - line += "%-20s " % field
> - report += line + "\n"
> - report += dash_line
> -
> # now loop over the runs, printing the requested report fields
> # if tguid is a field, we'll print a line for each tguid
> # otherwise, it's a line per test
> @@ -1921,10 +1908,6 @@ def gen_text_report(run_list, run_map,
> header_fields, fields):
> continue
>
> # format each column per row (ie per line)
> - line = " "
> - start_sep = " "
> - mid_sep = " "
> - end_sep = ""
> pos = 0
> for field in fields:
> if field=="tguid":
> @@ -1937,15 +1920,48 @@ def gen_text_report(run_list, run_map,
> header_fields, fields):
> tguid_name = ""
> value = run.get_field(field, tguid_name)
> if pos==0:
> - line += "%-20s%s" % (value, start_sep)
> + data_to_report.append((value,field))
> else:
> - line += "%-20s" % (value)
> if pos<len(fields):
> - line += mid_sep
> - else:
> - line += end_sep
> + data_to_report.append((value,field))
> pos += 1
> - report += line + "\n"
> + return data_to_report
> +
> +def gen_text_report(run_list, run_map, header_fields, fields):
> + #get data for report
> + header_report = gen_header_for_report(run_list, run_map,
> header_fields, fields)
> + data_to_report = gen_data_for_report(run_list, run_map,
> header_fields, fields)
> +
> + # generate header
> + ddash_line="="*70 + "\n"
> + report = ddash_line
> + title = "Fuego Test Report"
> + report += " **** %s ****\n" % title
> + report_date = time.strftime("%Y-%m-%d_%H:%M:%S")
> +
> + report += header_report
> + report += ddash_line
> +
> + # generate list
> +
> + # start with a header line
> + dash_line = "-"*(21*len(fields)+2) + "\n"
> + report += dash_line
> + report += " "
> + for field in fields:
> + report += "%-20s " % field
> + report += "\n" + dash_line
> +
> + # now loop over the runs, printing the requested report fields
> + # format each column per row (ie per line)
> + for i in range(0,len(data_to_report)):
> + if (data_to_report[i][1] == "test_name"):
> + report += " %-20s" % (data_to_report[i][0])
> + else:
> + if (data_to_report[i][1] == "result"):
> + report += " %-20s\n" % (data_to_report[i][0])
> + else:
> + report += " %-20s" % (data_to_report[i][0])
>
> report += dash_line
>
> --
> 2.1.4
OK - I don't think this is the final layout of code to implement this
feature, but it's a start.
I've applied this patch, and will apply a few fixups on top of it to address
the issues I raised.
Thanks,
-- Tim
More information about the Fuego
mailing list