[PATCH] Generate coverage analysis Report

Joel Sherrill joel at rtems.org
Wed May 30 17:59:14 UTC 2018


On Wed, May 30, 2018 at 12:54 PM, Vijay Kumar Banerjee <
vijaykumar9597 at gmail.com> wrote:

> On 30 May 2018 at 22:51, Gedare Bloom <gedare at 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 <vijaykumar9597 at 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 <cpodonnell8 at 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.miesowicz at 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(s
>> ummary_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.defaults,
>> > +                                                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.


> >
>> >      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 (chrisj at 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.




> >  %define qemu_opts_no_net -net none
>> >
>> >  #
>> > --
>> > 2.14.3
>> >
>> > _______________________________________________
>> > devel mailing list
>> > devel at rtems.org
>> > http://lists.rtems.org/mailman/listinfo/devel
>>
>
>
> _______________________________________________
> devel mailing list
> devel at rtems.org
> http://lists.rtems.org/mailman/listinfo/devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rtems.org/pipermail/devel/attachments/20180530/8fe1b2e5/attachment-0002.html>


More information about the devel mailing list