On Tue, 5 Feb 2019 at 03:37, Chris Johns <chr...@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
>
_______________________________________________
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
