| Message ID | 0c73060925b6fa31a423fad5f5fddb7452c3dd8e.1661410032.git.msuchanek@suse.de |
|---|---|
| State | Changes Requested |
| Delegated to: | Simon Glass |
| Headers | show |
| Series | [1/2] patman: do not hardcode coverage tool | expand |
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@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? Cheers, Quentin
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@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. In other words the only general solution is to specify the tool AFAICT. Thanks Michal
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@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? 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. Cheers, Quentin
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@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
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)) os.system(cmd) - stdout = command.output('python3-coverage', 'report') + stdout = command.output(coverage, 'report') lines = stdout.splitlines() if required: # Convert '/path/to/name.py' just the module name 'name' @@ -76,13 +78,13 @@ def run_test_coverage(prog, filter_fname, exclude_list, build_dir, required=None print(stdout) ok = False - coverage = lines[-1].split(' ')[-1] + cov_result = lines[-1].split(' ')[-1] ok = True - print(coverage) - if coverage != '100%': + print(cov_result) + if cov_result != '100%': print(stdout) - print("To get a report in 'htmlcov/index.html', type: python3-coverage html") - print('Coverage error: %s, but should be 100%%' % coverage) + print("To get a report in 'htmlcov/index.html', type: %s html" % coverage) + print('Coverage error: %s, but should be 100%%' % cov_result) ok = False if not ok: raise ValueError('Test coverage failure')
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@suse.de> --- doc/develop/testing.rst | 3 +++ tools/patman/test_util.py | 18 ++++++++++-------- 2 files changed, 13 insertions(+), 8 deletions(-)