[PATCH] Coverage: Add support for TSIM format

[Vijay Kumar Banerjee]

On Tue, 5 Feb 2019 at 03:37, Chris Johns <chrisj at rtems.org> wrote:

> On 1/2/19 10:47 pm, Vijay Kumar Banerjee wrote:
> > ---
> >  tester/rt/coverage.py                       | 28 ++++++++++++++++-----
> >  tester/rtems/testing/bsps/leon3-sis-cov.ini |  3 ++-
> >  2 files changed, 24 insertions(+), 7 deletions(-)
> >
> > diff --git a/tester/rt/coverage.py b/tester/rt/coverage.py
> > index 9fc9b64..859001a 100644
> > --- a/tester/rt/coverage.py
> > +++ b/tester/rt/coverage.py
> > @@ -288,7 +288,13 @@ class covoar(object):
> >      '''
> >      Covoar runner
> >      '''
> > -    def __init__(self, base_result_dir, config_dir, executables,
> explanations_txt, trace, prefix):
> > +    def __init__( self,
>
> Extra space?
>
> Will fix this in v2.

> > +                  base_result_dir,
> > +                  config_dir, executables,
> > +                  explanations_txt,
> > +                  trace,
> > +                  prefix,
> > +                  bsp ):
> >          self.base_result_dir = base_result_dir
> >          self.config_dir = config_dir
> >          self.executables = ' '.join(executables)
> > @@ -296,6 +302,7 @@ class covoar(object):
> >          self.project_name = 'RTEMS-5'
>
> I had not noticed this before. We should not be hard coding version
> numbers into
> this part of the code. It requires updating after each release and all in
> likely
> hood this one would be missed. The tool kit has support for versions.
>
> Understood. Will fix.

> >          self.trace = trace
> >          self.prefix = prefix
> > +        self.bsp = bsp
> >
> >      def _find_covoar(self):
> >          covoar_exe = 'covoar'
> > @@ -316,10 +323,18 @@ class covoar(object):
> >          if not path.exists(symbol_file):
> >              raise error.general('coverage: no symbol set file: %s'%
> (symbol_file))
> >          exe = self._find_covoar()
> > -        command = exe + ' -S ' + symbol_file + \
> > -                  ' -O ' + covoar_result_dir + \
> > -                  ' -E ' + self.explanations_txt + \
> > -                  ' -p ' + self.project_name + ' ' + self.executables
> > +        if 'qemu' in self.bsp.split('-'):
>
> Are you checking for 'qemu' in the BSP name? The naming used here is a
> convention and making the code depend on a naming convention is fragile.
> Lets
> not do this.
>
> There is a `%{qemu-cmd}` macro defined which must exist for a qemu run so
> it is
> better to check for this. However ...
>
> I'm not able to get it with macros.find(). What's the right way to find it?
Also, there's a 'cov_format' in testing.mc that is hardcoded to 'QEMU'. Can
this value be updated from the cfg files ?

> > +            command = exe + ' -S ' + symbol_file + \
> > +                      ' -O ' + covoar_result_dir + \
> > +                      ' -E ' + self.explanations_txt + \
> > +                      ' -p ' + self.project_name + ' ' +
> self.executables
> > +        else:
> > +            command = exe + ' -S ' + symbol_file + \
> > +                      ' -O ' + covoar_result_dir + \
> > +                      ' -E ' + self.explanations_txt + \
> > +                      ' -f TSIM' + \
> > +                      ' -p ' + self.project_name + ' ' +
> self.executables
>
> ... I would prefer the command be managed in the config files and this code
> removed. Do you think this can be done?
>
> the exe and the symbol_files are generated by the script itself. I think
we have to call
covoar from the script only.

Thanks for the review.

> Thanks
> Chris
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rtems.org/pipermail/devel/attachments/20190205/9294f428/attachment-0002.html>