<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Wed, May 30, 2018 at 1:34 PM, Vijay Kumar Banerjee <span dir="ltr"><<a href="mailto:vijaykumar9597@gmail.com" target="_blank">vijaykumar9597@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><div class="h5">On 30 May 2018 at 23:29, Joel Sherrill <span dir="ltr"><<a href="mailto:joel@rtems.org" target="_blank">joel@rtems.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><div><div class="m_7284862638170494635h5">On Wed, May 30, 2018 at 12:54 PM, Vijay Kumar Banerjee <span dir="ltr"><<a href="mailto:vijaykumar9597@gmail.com" target="_blank">vijaykumar9597@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span>On 30 May 2018 at 22:51, Gedare Bloom <span dir="ltr"><<a href="mailto:gedare@rtems.org" target="_blank">gedare@rtems.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Please provide your name in your commits (git config --<a href="http://user.name" rel="noreferrer" target="_blank">user.name</a> "My<br>
Name") that you submit.<br>
<br></blockquote></span><div>OK Noted :) </div><span><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
The first line of this commit, and therefore the email subject, is<br>
overly vague. Provide a slightly more specific description. <br></blockquote><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span>
On Wed, May 30, 2018 at 1:00 PM, thelunatic <<a href="mailto:vijaykumar9597@gmail.com" target="_blank">vijaykumar9597@gmail.com</a>> wrote:<br>
> + Add script to run covoar and generate an html report from<br>
> the output generated from covoar<br>
> + Add symbol-sets ini file for library addresses of the symbol-sets<br>
> + tester/rt/test : Add options for running coverage<br>
><br></span></blockquote><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span><br>
</span>I'd rather see a narrative paragraph than this list of + bullet items.<br>
Are all of these changes required to run the report? Should they be<br>
broken into smaller commits that are logically related but separately<br>
reviewable and commited?<br>
<span><br></span></blockquote></span><div>OK, I will write in a descriptive paragraph.</div><div>These changes are all needed to run coverage.</div><span><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span>
> Co-author : Cillian O'Donnel <<a href="mailto:cpodonnell8@gmail.com" target="_blank">cpodonnell8@gmail.com</a>><br>
</span>I don't know what Co-Author should mean. I would prefer to receive<br>
separate commits/patches for contributions made by different people if<br>
that is possible.<br>
<span><br></span></blockquote></span><div>Plese refer below... </div><span><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span>
> ---<br>
> tester/rt/coverage.py | 380 ++++++++++++++++++++++++++<br>
> tester/rt/test.py | 36 ++-<br>
> tester/rtems/testing/bsps/leon<wbr>3-qemu-cov.ini | 3 +-<br>
> tester/rtems/testing/coverage/<wbr>symbol-sets.ini | 36 +++<br>
> tester/rtems/testing/qemu.cfg | 4 +-<br>
> 5 files changed, 447 insertions(+), 12 deletions(-)<br>
> create mode 100644 tester/rt/coverage.py<br>
> create mode 100644 tester/rtems/testing/coverage/<wbr>symbol-sets.ini<br>
><br>
> diff --git a/tester/rt/coverage.py b/tester/rt/coverage.py<br>
> new file mode 100644<br>
> index 0000000..38dcce6<br>
> --- /dev/null<br>
> +++ b/tester/rt/coverage.py<br>
> @@ -0,0 +1,380 @@<br>
> +#<br>
> +# RTEMS Tools Project (<a href="http://www.rtems.org/" rel="noreferrer" target="_blank">http://www.rtems.org/</a>)<br>
> +# Copyright 2014 Krzysztof Miesowicz (<a href="mailto:krzysztof.miesowicz@gmail.com" target="_blank">krzysztof.miesowicz@gmail.com</a><wbr>)<br>
<br>
</span>Is this Krzysztof's code? if so, it should be added as a commit with<br>
him as the --author="" field of git-commit option.<br>
<div><div class="m_7284862638170494635m_727216205359767578m_7921129128105952456m_6803257989446512162h5"><br></div></div></blockquote></span><div>Actually this script has undergone a lot of updates.</div><div>It doesn't even work the same way it used to. I am uncertain</div><div>about the portions of the code that are written by him and still in </div><div>the script. Basically I left the copyright notice untouched and</div><div>let it be there because I am unsure of what to include there.</div><div>Same is true in case of Cillian. I don't really know how much </div><div>of Code is authored by him. </div><div>It surely isn't the proper way to add him as the co-author in </div><div>the log but that seemed like the only way to include him.</div></div></div></div></blockquote><div><br></div></div></div><div>OK. I was afraid of it being technically impossible to separate out the work</div><div>for revision control purposes. </div><div><br></div><div>Just make sure credit due is given.</div><div><div class="m_7284862638170494635h5"><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><div class="m_7284862638170494635m_727216205359767578h5"><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div class="m_7284862638170494635m_727216205359767578m_7921129128105952456m_6803257989446512162h5">
> +# All rights reserved.<br>
> +#<br>
> +# This file is part of the RTEMS Tools package in 'rtems-tools'.<br>
> +#<br>
> +# Redistribution and use in source and binary forms, with or without<br>
> +# modification, are permitted provided that the following conditions are met:<br>
> +#<br>
> +# 1. Redistributions of source code must retain the above copyright notice,<br>
> +# this list of conditions and the following disclaimer.<br>
> +#<br>
> +# 2. Redistributions in binary form must reproduce the above copyright notice,<br>
> +# this list of conditions and the following disclaimer in the documentation<br>
> +# and/or other materials provided with the distribution.<br>
> +#<br>
> +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS 'AS IS'<br>
> +# AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE<br>
> +# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE<br>
> +# ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE<br>
> +# LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR<br>
> +# CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF<br>
> +# SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS<br>
> +# INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN<br>
> +# CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)<br>
> +# ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE<br>
> +# POSSIBILITY OF SUCH DAMAGE.<br>
> +#<br>
> +<br>
> +from rtemstoolkit import error<br>
> +from rtemstoolkit import path<br>
> +from rtemstoolkit import log<br>
> +from rtemstoolkit import execute<br>
> +from rtemstoolkit import macros<br>
> +<br>
> +from datetime import datetime<br>
> +<br>
> +from . import options<br>
> +<br>
> +import shutil<br>
> +import os<br>
> +<br>
> +try:<br>
> + import configparser<br>
> +except:<br>
> + import ConfigParser as configparser<br>
> +<br>
> +class summary:<br>
> + def __init__(self, p_summary_dir):<br>
> + self.summary_file_path = path.join(p_summary_dir, 'summary.txt')<br>
> + self.index_file_path = path.join(p_summary_dir, 'index.html')<br>
> + self.bytes_analyzed = 0<br>
> + self.bytes_not_executed = 0<br>
> + self.percentage_executed = 0.0<br>
> + self.percentage_not_executed = 100.0<br>
> + self.ranges_uncovered = 0<br>
> + self.branches_uncovered = 0<br>
> + self.branches_total = 0<br>
> + self.branches_always_taken = 0<br>
> + self.branches_never_taken = 0<br>
> + self.percentage_branches_cover<wbr>ed = 0.0<br>
> + self.is_failure = False<br>
> +<br>
> + def parse(self):<br>
> + if(not path.exists(self.summary_file_<wbr>path)):<br>
> + log.notice('summary file %s does not exist!' % (self.summary_file_path))<br>
> + self.is_failure = True<br>
> + return<br>
> +<br>
> + with open(self.summary_file_path,'r<wbr>') as summary_file:<br>
> + self.bytes_analyzed = self._get_next_with_colon(summ<wbr>ary_file)<br>
> + self.bytes_not_executed = self._get_next_with_colon(summ<wbr>ary_file)<br>
> + self.percentage_executed = self._get_next_with_colon(summ<wbr>ary_file)<br>
> + self.percentage_not_executed = self._get_next_with_colon(summ<wbr>ary_file)<br>
> + self.ranges_uncovered = self._get_next_with_colon(summ<wbr>ary_file)<br>
> + self.branches_total = self._get_next_with_colon(summ<wbr>ary_file)<br>
> + self.branches_uncovered = self._get_next_with_colon(summ<wbr>ary_file)<br>
> + self.branches_always_taken = self._get_next_without_colon(s<wbr>ummary_file)<br>
> + self.branches_never_taken = self._get_next_without_colon(s<wbr>ummary_file)<br>
> + if len(self.branches_uncovered) > 0 and len(self.branches_total) > 0:<br>
> + self.percentage_branches_cover<wbr>ed = \<br>
> + 1 - (float(self.branches_uncovered<wbr>) / float(self.branches_total))<br>
> + else:<br>
> + self.percentage_branches_cover<wbr>ed = 0.0<br>
> + return<br>
> +<br>
> + def _get_next_with_colon(self, summary_file):<br>
> + line = summary_file.readline()<br>
> + if ':' in line:<br>
> + return line.split(':')[1].strip()<br>
> + else:<br>
> + return ''<br>
> +<br>
> + def _get_next_without_colon(self, summary_file):<br>
> + line = summary_file.readline()<br>
> + return line.strip().split(' ')[0]<br>
> +<br>
> +class report_gen_html:<br>
> + def __init__(self, p_symbol_sets_list, build_dir, rtdir):<br>
> + self.symbol_sets_list = ['score']<br>
> + self.build_dir = build_dir<br>
> + self.partial_reports_files = list(["index.html", "summary.txt"])<br>
> + self.number_of_columns = 1<br>
> + self.covoar_src_path = path.join(rtdir, 'covoar')<br>
> +<br>
> + def _find_partial_reports(self):<br>
> + partial_reports = {}<br>
> + for symbol_set in self.symbol_sets_list:<br>
> + set_summary = summary(path.join(self.build_d<wbr>ir, "coverage",<br>
> + symbol_set))<br>
> + set_summary.parse()<br>
> + partial_reports[symbol_set] = set_summary<br>
> + return partial_reports<br>
> +<br>
> + def _prepare_head_section(self):<br>
> + head_section = '''<br>
> + <head><br>
> + <title>RTEMS coverage report</title><br>
> + <style type="text/css"><br>
> + progress[value] {<br>
> + -webkit-appearance: none;<br>
> + appearance: none;<br>
> +<br>
> + width: 150px;<br>
> + height: 15px;<br>
> + }<br>
> + </style><br>
> + </head>'''<br>
> + return head_section<br>
> +<br>
> + def _prepare_index_content(self, partial_reports):<br>
> + header = "<h1> RTEMS coverage analysis report </h1>"<br>
> + header += "<h3>Coverage reports by symbols sets:</h3>"<br>
> + table = "<table>"<br>
> + table += self._header_row()<br>
> + for symbol_set in partial_reports:<br>
> + table += self._row(symbol_set, partial_reports[symbol_set])<br>
> + table += "</table> </br>"<br>
> + timestamp = "Analysis performed on " + datetime.now().ctime()<br>
> + return "<body>\n" + header + table + timestamp + "\n</body>"<br>
> +<br>
> + def _row(self, symbol_set, summary):<br>
> + row = "<tr>"<br>
> + row += "<td>" + symbol_set + "</td>"<br>
> + if summary.is_failure:<br>
> + row += ' <td colspan="' + str(self.number_of_columns-1) \<br>
> + + '" style="background-color:red">F<wbr>AILURE</td>'<br>
> + else:<br>
> + row += " <td>" + self._link(summary.index_file_<wbr>path,"Index") \<br>
> + + "</td>"<br>
> + row += " <td>" + self._link(summary.summary_fil<wbr>e_path,"Summary") \<br>
> + + "</td>"<br>
> + row += " <td>" + summary.bytes_analyzed + "</td>"<br>
> + row += " <td>" + summary.bytes_not_executed + "</td>"<br>
> + row += " <td>" + summary.ranges_uncovered + "</td>"<br>
> + row += " <td>" + summary.percentage_executed + "%</td>"<br>
> + row += " <td>" + summary.percentage_not_execute<wbr>d + "%</td>"<br>
> + row += ' <td><progress value="' + summary.percentage_executed \<br>
> + + '" max="100"></progress></td>'<br>
> + row += " <td>" + summary.branches_uncovered + "</td>"<br>
> + row += " <td>" + summary.branches_total + "</td>"<br>
> + row += " <td> {:.3%} </td>".format(summary.percenta<wbr>ge_branches_covered)<br>
> + row += ' <td><progress value="{:.3}" max="100"></progress></td>'.fo<wbr>rmat(100*summary.percentage_br<wbr>anches_covered)<br>
> + row += "</tr>\n"<br>
> + return row<br>
> +<br>
> + def _header_row(self):<br>
> + row = "<tr>"<br>
> + row += "<th> Symbols set name </th>"<br>
> + row += "<th> Index file </th>"<br>
> + row += "<th> Summary file </th>"<br>
> + row += "<th> Bytes analyzed </th>"<br>
> + row += "<th> Bytes not executed </th>"<br>
> + row += "<th> Uncovered ranges </th>"<br>
> + row += "<th> Percentage covered </th>"<br>
> + row += "<th> Percentage uncovered </th>"<br>
> + row += "<th> Instruction coverage </th>"<br>
> + row += "<th> Branches uncovered </th>"<br>
> + row += "<th> Branches total </th>"<br>
> + row += "<th> Branches covered percentage </th>"<br>
> + row += "<th> Branches coverage </th>"<br>
> + row += "</tr>\n"<br>
> + self.number_of_columns = row.count('<th>')<br>
> + return row<br>
> +<br>
> + def _link(self, address, text):<br>
> + return '<a href="' + address + '">' + text + '</a>'<br>
> +<br>
> + def _create_index_file(self, head_section, content):<br>
> + with open(path.join(self.build_dir,<wbr>"report.html"),'w') as f:<br>
> + f.write(head_section)<br>
> + f.write(content)<br>
> +<br>
> + def generate(self):<br>
> + partial_reports = self._find_partial_reports()<br>
> + head_section = self._prepare_head_section()<br>
> + index_content = self._prepare_index_content(pa<wbr>rtial_reports)<br>
> + self._create_index_file(head_s<wbr>ection,index_content)<br>
> +<br>
> + def add_covoar_src_path(self):<br>
> + table_js_path = path.join(self.covoar_src_path<wbr>, 'table.js')<br>
> + covoar_css_path = path.join(self.covoar_src_path<wbr>, 'covoar.css')<br>
> + for symbol_set in self.symbol_sets_list:<br>
> + symbol_set_dir = path.join(self.build_dir, "coverage", symbol_set)<br>
> + html_files = os.listdir(symbol_set_dir)<br>
> + for html_file in html_files:<br>
> + html_file = path.join(symbol_set_dir, html_file)<br>
> + if path.exists(html_file) and 'html' in html_file:<br>
> + with open(html_file, 'r') as f:<br>
> + file_data = f.read()<br>
> + file_data = file_data.replace('table.js', table_js_path)<br>
> + file_data = file_data.replace('covoar.css'<wbr>,<br>
> + covoar_css_path)<br>
> + with open(html_file, 'w') as f:<br>
> + f.write(file_data)<br>
> +<br>
> +class build_path_generator(object):<br>
> + '''<br>
> + Generates the build path from the path to executables<br>
> + '''<br>
> + def __init__(self, executables, target):<br>
> + self.executables = executables<br>
> + self.target = target<br>
> + def run(self):<br>
> + build_path = '/'<br>
> + Path = self.executables[0].split('/')<br>
> + for P in Path:<br>
> + if P == self.target:<br>
> + break;<br>
> + else:<br>
> + build_path = path.join(build_path, P)<br>
> + return build_path<br>
> +<br>
> +class symbol_parser(object):<br>
> + '''<br>
> + Parse the symbol sets ini and create custom ini file for covoar<br>
> + '''<br>
> + def __init__(self, symbol_config_path,<br>
> + symbol_select_path, coverage_arg, build_dir):<br>
> + self.symbol_select_file = symbol_select_path<br>
> + self.symbol_file = symbol_config_path<br>
> + self.build_dir = build_dir<br>
> + self.symbol_sets = {}<br>
> + self.cov_arg = coverage_arg<br>
> + self.ssets = []<br>
> +<br>
> + def parse(self):<br>
> + config = configparser.ConfigParser()<br>
> + try:<br>
> + config.read(self.symbol_file)<br>
> + if self.cov_arg:<br>
> + self.ssets = self.cov_arg.split(',')<br>
> + else:<br>
> + self.ssets = config.get('symbol-sets', 'sets').split(',')<br>
> + self.ssets = [ sset.encode('utf-8') for sset in self.ssets]<br>
> + for sset in self.ssets:<br>
> + lib = path.join(self.build_dir,<br>
> + config.get('libraries', sset))<br>
> + self.symbol_sets[sset] = lib.encode('utf-8')<br>
> + except:<br>
> + raise error.general('Symbol set parsing failed')<br>
> +<br>
> + def _write_ini(self):<br>
> + config = configparser.ConfigParser()<br>
> + try:<br>
> + sets = ', '.join(self.symbol_sets.keys()<wbr>)<br>
> + config.add_section('symbol-set<wbr>s')<br>
> + config.set('symbol-sets', 'sets', sets)<br>
> + for key in self.symbol_sets.keys():<br>
> + config.add_section(key)<br>
> + config.set(key, 'libraries', self.symbol_sets[key])<br>
> + with open(self.symbol_select_file, 'w') as conf:<br>
> + config.write(conf)<br>
> + except:<br>
> + raise error.general('write failed')<br>
> +<br>
> + def run(self):<br>
> + self.parse()<br>
> + self._write_ini()<br>
> +<br>
> +<br>
> +class covoar(object):<br>
> + '''<br>
> + Covoar runner<br>
> + '''<br>
> + def __init__(self, base_result_dir, config_dir, executables, explanations_txt):<br>
> + self.base_result_dir = base_result_dir<br>
> + self.config_dir = config_dir<br>
> + self.executables = ' '.join(executables)<br>
> + self.explanations_txt = explanations_txt<br>
> + self.project_name = 'RTEMS-5'<br>
> +<br>
> + def run(self, set_name, symbol_file):<br>
> + covoar_result_dir = path.join(self.base_result_dir<wbr>, set_name)<br>
> + if (not path.exists(covoar_result_dir)<wbr>):<br>
> + path.mkdir(covoar_result_dir)<br>
> + if (not path.exists(symbol_file)):<br>
> + raise error.general('symbol set file: coverage %s was not created for covoar, skipping %s'% (symbol_file, set_name))<br>
> + command = ('covoar -S ' + symbol_file<br>
> + + ' -O ' + covoar_result_dir<br>
> + + ' -E ' + self.explanations_txt<br>
> + + ' -p ' + self.project_name + ' ' + self.executables)<br>
> + log.notice('Running covoar for %s' % (set_name))<br>
> + print( 'covoar results directory:\n' + covoar_result_dir )<br>
> + executor = execute.execute(verbose = True, output = self.output_handler)<br>
> + exit_code = executor.shell(command, cwd=os.getcwd())<br>
> + if (exit_code[0] != 0):<br>
> + raise error.general('covoar failure exit code: %d' % (exit_code[0]))<br>
> + log.notice('Coverage run for %s finished successfully.' % (set_name))<br>
> + log.notice('------------------<wbr>-----------------------------'<wbr>)<br>
> +<br>
> + def output_handler(self, text):<br>
> + log.notice('%s' % (text))<br>
> +<br>
> +class coverage_run(object):<br>
> + '''<br>
> + Coverage analysis support for rtems-test<br>
> + '''<br>
> + def __init__(self, p_macros, coverage_arg, target):<br>
> + '''<br>
> + Constructor<br>
> + '''<br>
> + self.macros = p_macros<br>
> + self.build_dir = self.macros['_cwd']<br>
> + self.explanations_txt = self.macros.expand(self.macros<wbr>['cov_explanations'])<br>
> + self.test_dir = path.join(self.build_dir, 'coverage')<br>
> + if (not path.exists(self.test_dir)):<br>
> + path.mkdir(self.test_dir)<br>
> + self.rtdir = path.abspath(self.macros['_rtd<wbr>ir'])<br>
> + self.rtscripts = self.macros.expand(self.macros<wbr>['_rtscripts'])<br>
> + self.coverage_config_path = path.join(self.rtscripts, 'coverage')<br>
> + self.symbol_config_path = path.join(self.coverage_config<wbr>_path,<br>
> + 'symbol-sets.ini')<br>
> + self.symbol_select_path = path.join(self.coverage_config<wbr>_path,<br>
> + 'symbol-select.ini')<br>
> + self.executables = None<br>
> + self.symbol_sets = []<br>
> + self.no_clean = int(self.macros['_no_clean'])<br>
> + self.report_format = self.macros['cov_report_format<wbr>']<br>
> + self.coverage_arg = coverage_arg<br>
> + self.target = target<br>
> +<br>
> + def run(self):<br>
> + try:<br>
> + if self.executables is None:<br>
> + raise error.general('no test executables provided.')<br>
> + build_dir = build_path_generator(self.exec<wbr>utables, self.target).run()<br>
> + parser = symbol_parser(self.symbol_conf<wbr>ig_path,<br>
> + self.symbol_select_path,<br>
> + self.coverage_arg,<br>
> + build_dir)<br>
> + parser.run()<br>
> + covoar_runner = covoar(self.test_dir, self.symbol_select_path,<br>
> + self.executables, self.explanations_txt)<br>
> + covoar_runner.run('score', self.symbol_select_path)<br>
> + self._generate_reports();<br>
> + self._summarize();<br>
> + finally:<br>
> + self._cleanup();<br>
> +<br>
> + def _generate_reports(self):<br>
> + log.notice('Generating reports')<br>
> + if self.report_format == 'html':<br>
> + report = report_gen_html(self.symbol_se<wbr>ts,<br>
> + self.build_dir,<br>
> + self.rtdir)<br>
> + report.generate()<br>
> + report.add_covoar_src_path()<br>
> +<br>
> + def _cleanup(self):<br>
> + if not self.no_clean:<br>
> + log.notice('***Cleaning tempfiles***')<br>
> + for exe in self.executables:<br>
> + trace_file = exe + '.cov'<br>
> + if path.exists(trace_file):<br>
> + os.remove(trace_file)<br>
> +<br>
> + def _summarize(self):<br>
> + log.notice('Coverage analysis finished. You can find results in %s' % (self.build_dir))<br>
> diff --git a/tester/rt/test.py b/tester/rt/test.py<br>
> index f4d9b5c..cabec7b 100644<br>
> --- a/tester/rt/test.py<br>
> +++ b/tester/rt/test.py<br>
> @@ -48,12 +48,14 @@ from rtemstoolkit import mailer<br>
> from rtemstoolkit import reraise<br>
> from rtemstoolkit import stacktraces<br>
> from rtemstoolkit import version<br>
> +from rtemstoolkit import check<br>
><br>
> from . import bsps<br>
> from . import config<br>
> from . import console<br>
> from . import options<br>
> from . import report<br>
> +from . import coverage<br>
><br>
> class log_capture(object):<br>
> def __init__(self):<br>
> @@ -147,7 +149,7 @@ class test_run(object):<br>
><br>
> def run(self):<br>
> self.thread = threading.Thread(target = self.runner,<br>
> - name = 'test[%s]' % path.basename(self.executable)<wbr>)<br>
> + name = 'test[%s]' % path.basename(self.executable)<wbr>)<br>
> self.thread.start()<br>
><br>
> def is_alive(self):<br>
> @@ -214,6 +216,11 @@ def killall(tests):<br>
> for test in tests:<br>
> test.kill()<br>
><br>
> +<br>
</div></div>why extra new line here?<br></blockquote></div></div><div>Will correct it. Thanks. </div><div><div class="m_7284862638170494635m_727216205359767578h5"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div><div class="m_7284862638170494635m_727216205359767578m_7921129128105952456m_6803257989446512162h5"><br>
> +def coverage_run(opts, coverage, executables):<br>
> + coverage.executables = executables<br>
> + coverage.run()<br>
> +<br>
> def run(command_path = None):<br>
> import sys<br>
> tests = []<br>
> @@ -221,15 +228,16 @@ def run(command_path = None):<br>
> opts = None<br>
> default_exefilter = '*.exe'<br>
> try:<br>
> - optargs = { '--rtems-tools': 'The path to the RTEMS tools',<br>
> - '--rtems-bsp': 'The RTEMS BSP to run the test on',<br>
> - '--user-config': 'Path to your local user configuration INI file',<br>
> - '--report-mode': 'Reporting modes, failures (default),all,none',<br>
> - '--list-bsps': 'List the supported BSPs',<br>
> - '--debug-trace': 'Debug trace based on specific flags',<br>
> - '--filter': 'Glob that executables must match to run (default: ' +<br>
> + optargs = { '--rtems-tools': 'The path to the RTEMS tools',<br>
> + '--rtems-bsp': 'The RTEMS BSP to run the test on',<br>
> + '--user-config': 'Path to your local user configuration INI file',<br>
> + '--report-mode': 'Reporting modes, failures (default),all,none',<br>
> + '--list-bsps': 'List the supported BSPs',<br>
> + '--debug-trace': 'Debug trace based on specific flags',<br>
> + '--filter': 'Glob that executables must match to run (default: ' +<br>
> default_exefilter + ')',<br>
> - '--stacktrace': 'Dump a stack trace on a user termination (^C)' }<br>
> + '--stacktrace': 'Dump a stack trace on a user termination (^C)',<br>
> + '--coverage': 'Perform coverage analysis of test exectuables.'}<br>
</div></div>Why are there changes outside of the last two lines (stacktrace and<br>
coverage)? Only those two need to be modified to add the new option.<br>
<br>
Typo: s/exectuables/executables<br></blockquote></div></div><div>Thanks </div><span><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span><br>
> mailer.append_options(optargs)<br>
> opts = options.load(sys.argv,<br>
> optargs = optargs,<br>
> @@ -279,6 +287,14 @@ def run(command_path = None):<br>
> raise error.general('RTEMS BSP not provided or an invalid option')<br>
> bsp = config.load(bsp[1], opts)<br>
> bsp_config = opts.defaults.expand(opts.defa<wbr>ults['tester'])<br>
> + coverage_enabled = opts.find_arg('--coverage')<br>
> + if coverage_enabled:<br>
> + if len(coverage_enabled) == 2:<br>
<br>
</span>What is this len() check doing? This seems horribly hackish to me. It<br>
is obviously assuming something, but I can't tell what that is. It<br>
might be better to do an error check (like what is done with the bsp<br>
variable before this) to see if len(coverage_enabled) != 2. From what<br>
I see here, I have no clue what should be the arguments to this<br>
--coverage, it seems like a binary value to me (on/off).<br>
<span><br></span></blockquote></span><div>it's supposed to work this way ...</div><div>--coverage option is supposed to run coverage for all the sets </div><div>mentioned in the ini file.</div><div>--coverage=set1,set2</div><div>will run it specifically for set1 and set 2. </div><span><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span>> + coverage_runner = coverage.coverage_run(opts.def<wbr>aults,<br>
> + coverage_enabled[1],<br>
> + opts.defaults['target'])<br>
> + else:<br>
> + coverage_runner = coverage.coverage_run(opts.def<wbr>aults, 0)<br>
> report_mode = opts.find_arg('--report-mode')<br>
> if report_mode:<br>
> if report_mode[1] != 'failures' and \<br>
> @@ -365,6 +381,8 @@ def run(command_path = None):<br>
> reports.failures(),<br>
> 'Log', '===', ''] + output.get()<br>
> mail.send(to_addr, subject, os.linesep.join(body))<br>
> + if coverage_enabled:<br>
> + coverage_run(opts, coverage_runner, executables)<br>
<br>
</span>Design question: Should the coverage run replace the test_run? Or<br>
should coverage be an option to test_run instead of how this is being<br>
done?<br>
<div><div class="m_7284862638170494635m_727216205359767578m_7921129128105952456m_6803257989446512162h5"><br></div></div></blockquote></span><div>It should be an option to test_run</div><div>just adding --coverage will run coverage analysis. </div></div></div></div></blockquote><div><br></div></div></div><div>Can this change be a follow up patch? I know Chris and I just want to get</div><div>work focused on the master.</div></div></div></div></blockquote></div></div><div>Sorry I couldn't understand.</div><div>Which change ? </div></div></div></div></blockquote><div><br></div><div>I may not understand correctly but there is test_run and coverage_run. Someone</div><div>suggested making coverage_running an option to test_run. If that's what's being</div><div>asked for, then I think doing it in a follow up patch is OK.</div><div><br></div><div>If that's the intended request, perhaps a ticket should be filed.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><div class="h5"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><div class="m_7284862638170494635h5"><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><div class="m_7284862638170494635m_727216205359767578h5"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div class="m_7284862638170494635m_727216205359767578m_7921129128105952456m_6803257989446512162h5">
><br>
> except error.general as gerr:<br>
> print(gerr)<br>
> diff --git a/tester/rtems/testing/bsps/le<wbr>on3-qemu-cov.ini b/tester/rtems/testing/bsps/le<wbr>on3-qemu-cov.ini<br>
> index 6b5e7e6..2f89117 100644<br>
> --- a/tester/rtems/testing/bsps/le<wbr>on3-qemu-cov.ini<br>
> +++ b/tester/rtems/testing/bsps/le<wbr>on3-qemu-cov.ini<br>
> @@ -31,9 +31,10 @@<br>
> #<br>
> # The Leon 3 QEMU BSP<br>
> #<br>
> -[leon3-qemu]<br>
> +[leon3-qemu-cov]<br>
> bsp = leon3-qemu<br>
> arch = sparc<br>
> +target = sparc-rtems5<br>
> tester = %{_rtscripts}/qemu.cfg<br>
> bsp_qemu_opts = %{qemu_opts_base} -M leon3_generic<br>
> bsp_qemu_cov_opts = -exec-trace %{test_executable}.cov<br>
> diff --git a/tester/rtems/testing/coverag<wbr>e/symbol-sets.ini b/tester/rtems/testing/coverag<wbr>e/symbol-sets.ini<br>
> new file mode 100644<br>
> index 0000000..a2ec7bc<br>
> --- /dev/null<br>
> +++ b/tester/rtems/testing/coverag<wbr>e/symbol-sets.ini<br>
> @@ -0,0 +1,36 @@<br>
> +#<br>
> +# RTEMS Tools Project (<a href="http://www.rtems.org/" rel="noreferrer" target="_blank">http://www.rtems.org/</a>)<br>
> +# Copyright 2018 Chris Johns (<a href="mailto:chrisj@rtems.org" target="_blank">chrisj@rtems.org</a>)<br>
> +# All rights reserved.<br>
> +#<br>
> +# This file is part of the RTEMS Tools package in 'rtems-tools'.<br>
> +#<br>
> +# Redistribution and use in source and binary forms, with or without<br>
> +# modification, are permitted provided that the following conditions are met:<br>
> +#<br>
> +# 1. Redistributions of source code must retain the above copyright notice,<br>
> +# this list of conditions and the following disclaimer.<br>
> +#<br>
> +# 2. Redistributions in binary form must reproduce the above copyright notice,<br>
> +# this list of conditions and the following disclaimer in the documentation<br>
> +# and/or other materials provided with the distribution.<br>
> +#<br>
> +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"<br>
> +# AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE<br>
> +# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE<br>
> +# ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE<br>
> +# LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR<br>
> +# CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF<br>
> +# SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS<br>
> +# INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN<br>
> +# CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)<br>
> +# ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE<br>
> +# POSSIBILITY OF SUCH DAMAGE.<br>
> +#<br>
> +<br>
> +[symbol-sets]<br>
> +sets = score,rtems<br>
> +<br>
> +[libraries]<br>
> +score = @BUILD-TARGET@/c/@BSP@/cpukit/<wbr>score/libscore.a<br>
> +rtems = @BUILD-TARGET@/c/@BSP@/cpukit/<wbr>rtems/librtems.a<br>
> diff --git a/tester/rtems/testing/<a href="http://qemu.cf" target="_blank">qemu.cf</a><wbr>g b/tester/rtems/testing/<a href="http://qemu.cf" target="_blank">qemu.cf</a><wbr>g<br>
> index bfcd2f5..52a3752 100644<br>
> --- a/tester/rtems/testing/<a href="http://qemu.cf" target="_blank">qemu.cf</a><wbr>g<br>
> +++ b/tester/rtems/testing/<a href="http://qemu.cf" target="_blank">qemu.cf</a><wbr>g<br>
> @@ -51,8 +51,8 @@<br>
> #<br>
> # Qemu common option patterns.<br>
> #<br>
> -#%define qemu_opts_base -no-reboot -monitor none -serial stdio -nographic<br>
> -%define qemu_opts_base -no-reboot -serial null -serial mon:stdio -nographic<br>
> +%define qemu_opts_base -no-reboot -monitor none -serial stdio -nographic<br>
> +#%define qemu_opts_base -no-reboot -serial null -serial mon:stdio -nographic<br>
<br>
</div></div>Why changing the common options for qemu?<br>
<span><br></span></blockquote></div></div><div>actually it's a bit experimental on advice of Cillian. </div><div>It stayed in the patch. </div></div></div></div></blockquote><div><br></div></div></div><div>I know the impact of some of those options but maybe it would make sense</div><div>to add a comment block with the impact of each option? It would help future</div><div>readers.</div><div><br></div><div>And, from personal experience, qemu changes arguments from time to time. Knowing</div><div>what the old intent was helps mapping to different versions and target architectures.</div><span><div><br></div></span></div></div></div></blockquote></div></div><div>That's a good Idea. </div><span class=""><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span><div></div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span>
> %define qemu_opts_no_net -net none<br>
><br>
> #<br>
> --<br>
> 2.14.3<br>
><br>
</span>> ______________________________<wbr>_________________<br>
> devel mailing list<br>
> <a href="mailto:devel@rtems.org" target="_blank">devel@rtems.org</a><br>
> <a href="http://lists.rtems.org/mailman/listinfo/devel" rel="noreferrer" target="_blank">http://lists.rtems.org/mailman<wbr>/listinfo/devel</a><br>
</blockquote></span></div><br></div></div>
<br>______________________________<wbr>_________________<br>
devel mailing list<br>
<a href="mailto:devel@rtems.org" target="_blank">devel@rtems.org</a><br>
<a href="http://lists.rtems.org/mailman/listinfo/devel" rel="noreferrer" target="_blank">http://lists.rtems.org/mailman<wbr>/listinfo/devel</a><br></blockquote></span></div><br></div></div>
</blockquote></span></div><br></div></div>
</blockquote></div><br></div></div>