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