[PATCH 1/2] patman: do not hardcode coverage tool

[Michal Suchánek]

Hello,

On Wed, Aug 31, 2022 at 12:08:32PM +0200, Quentin Schulz wrote:
> Hi Michal,
> 
> On 8/30/22 12:11, Michal Suchánek wrote:
> > On Tue, Aug 30, 2022 at 12:01:55PM +0200, Quentin Schulz wrote:
> > > Hi Michal,
> > > 
> > > On 8/25/22 08:49, Michal Suchanek wrote:
> > > > The coverage tool name varies across distributions.
> > > > 
> > > > Add COVERAGE variable to specify the tool name.
> > > > 
> > > > Also there is one place where prefix is prepended to the tool path,
> > > > remove the prefix.
> > > > 
> > > > Signed-off-by: Michal Suchanek <msuchanek at suse.de>
> > > > ---
> > > >    doc/develop/testing.rst   |  3 +++
> > > >    tools/patman/test_util.py | 18 ++++++++++--------
> > > >    2 files changed, 13 insertions(+), 8 deletions(-)
> > > > 
> > > > diff --git a/doc/develop/testing.rst b/doc/develop/testing.rst
> > > > index 1abe4d7f0f..054fbfc814 100644
> > > > --- a/doc/develop/testing.rst
> > > > +++ b/doc/develop/testing.rst
> > > > @@ -17,6 +17,9 @@ To run most tests on sandbox, type this::
> > > >    in the U-Boot directory. Note that only the pytest suite is run using this
> > > >    command.
> > > > +Note: external tool `python3-coverage` is used by tests. The environment
> > > > +variable `COVERAGE` can be set to alternative name or location of this tool.
> > > > +
> > > >    Some tests take ages to run and are marked with @pytest.mark.slow. To run just
> > > >    the quick ones, type this::
> > > > diff --git a/tools/patman/test_util.py b/tools/patman/test_util.py
> > > > index 0f6d1aa902..e11806b626 100644
> > > > --- a/tools/patman/test_util.py
> > > > +++ b/tools/patman/test_util.py
> > > > @@ -15,6 +15,8 @@ from patman import command
> > > >    from io import StringIO
> > > > +coverage = os.environ.get('COVERAGE', 'python3-coverage')
> > > > +
> > > >    buffer_outputs = True
> > > >    use_concurrent = True
> > > >    try:
> > > > @@ -58,11 +60,11 @@ def run_test_coverage(prog, filter_fname, exclude_list, build_dir, required=None
> > > >        prefix = ''
> > > >        if build_dir:
> > > >            prefix = 'PYTHONPATH=$PYTHONPATH:%s/sandbox_spl/tools ' % build_dir
> > > > -    cmd = ('%spython3-coverage run '
> > > > -           '--omit "%s" %s %s %s -P1' % (prefix, ','.join(glob_list),
> > > > +    cmd = ('%s run '
> > > > +           '--omit "%s" %s %s %s -P1' % (coverage, ','.join(glob_list),
> > > >                                             prog, extra_args or '', test_cmd))
> > > 
> > > What about using
> > > python3 -m coverage run
> > > instead?
> > > This way we wouldn't rely on the binary name the host distribution chooses
> > > (python3-coverage for Ubuntu, coverage for Fedora).
> > > 
> > > I'm not sure there is a need to give the user the ability to override this
> > > value since I expect only coverage.py is supported at the moment?
> > 
> > Then you run into the problems that you do not have coverage on
> > python3.4 but only python3.6 or whatever is the relevant version for
> > your distribution ATM.
> > 
> 
> I don't understand this point? If coverage cannot run with the python from
> the host, then it won't just be possible to run it... so... we don't care?
> Is there something we can do about this I'm missing?

Host can have multiple python versions with varying tools installed.

> 
> I would like to advocate for COVERAGE to default to "coverage" or "python3
> -m coverage" since this is more likely to be a sane default for everybody,
> using pip or distro installs.

"coverage" will likely break current test environment but "python3 -m
coverage" may just work, and also test that expanding the arguments
works correctly.

Thanks

Michal