Re: [PATCH] Add pytest for a GCOV test-case
On 1/15/21 1:28 PM, Rainer Orth wrote: Hi Martin, * If we now have an (even optional) dependency on python/pytest, this (with the exact versions and use) needs to be documented in install.texi. Done that. +be installed. Some optional tests also require Python3 and pytest module. It would be better to be more specific here. Or would Python 3.0 and pytest 2.0.0 do ;-) Hello. I would leave it as it is. Python3 is a well established term. About pytest: ... but unfortunately not exactly precise beyond distinguishing between Python 2.x and 3.x. I'd specificially asked about 3.0 because IIRC the early 3.x versions were not only incompatible with 2.7, but even among themselves. Don't remember when this finally stabilized. I've got this, but as Python 3.0 was released more than 10 years ago, I'm leaving Python3 for now. Feel free to propose a better wording. I don't know how to investigate a minimal version right now. It needn't necessarily be a minimal version, it could be one known to work (and worded appropriately to indicate that's what it is, not a minimum). E.g. from what's bundled with Solaris 11.3, I know that pytest 2.6.4 does work (apart from the python3 vs. python3.4 issue). * On to the implementation: your test for the presence of pytest is wrong: set result [remote_exec host "pytest -m pytest --version"] has nothing to do with what you actually use later: on all of Fedora 29, Ubuntu 20.04, and Solaris 11.4 (with a caveat) pytest is Python 2.7 based, but you don't check that. It is well possible that pytest for 2.7 is installed, but pytest for Python 3.x isn't. Besides, while Solaris 11.4 does bundle pytest, they don't deliver pytest, but only py.test due to a conflict with a different pytest from logilab-common, cf. https://github.com/pytest-dev/pytest/issues/1833. This is immaterial, however, since what you actually run is spawn -noecho python3 -m pytest --color=no -rA -s --tb=no $srcdir/$subdir/$pytest_script So you should just run python3 -m pytest --version instead to check for the presence of the version you're going to use. Btw., there's a mess with pytest on Fedora 29: running the above gives I must confirm this is mess. I definitely don't want to support Python2 and I think the best way would be to use 'env python3', hope it's portable enough. @David: What do you think? As I mentioned, it's not: Solaris 11.3 has no python3, only (for the 3.x series) python3.4. However, I don't understand what you expect to gain from running $ env python3 rather than just $ python3 (or a suitable Python 3.x version by any name)? All right, let's replace it just with 'python3'. I just had a quick look and the autoconf-archive has AX_PYTHON which claims to do that: https://www.gnu.org/software/autoconf-archive/ax_python.html Unfortunately, it doesn't know about Python 3.8+ yet. I think we can do better than hardcoding python3 here. If it were only for a quirk of an older Solaris version (11.3) which misses the python3 name, together with one (or soon two) optional testcases, I wouldn't care much. When looking at a VM with FreeBSD 12.2 (the current release), it took quite some searching to find how to get python2 and python3 commands installed, and I suspect those are not they only ones where it's either difficult or missing completely. Given that David also plans to use python3, we could use another approach: Where's a Python3 binary on a FreeBSD system? Rather than relying on an obscure/unmaintained macro from the autoconf-archive, automake includes AM_PATH_PYTHON which allows to specify a minimum version and knows about python2, python3, and python[23].x names. While the version of our current prerequisite, Automake 1.15.1, has a hardcoded list going up to python3.5 only, the git version has removed that limitation. What we should do to get this right is: * import that git automake m4/python.m4 into our toplevel config * call AM_PATH_PYTHON appropriately from gcc/configure.ac * record the resulting python path in site.exp * use it from there e.g. in gcov.exp This way we're future-proof for all possible uses of python3, rather than fiddling with the issue over and over again. Feel free to propose a patch please. I'm sorry to burden you with this, but given that you're the first to follow through with using python3 in the core of gcc rather than in some contrib script, we should get things right now if reasonably possible. Sure, but we speak about an optional tests. The rest of the patch looks good now and passed my testing. I'm going to install the patch. Martin Rainer
Re: [PATCH] Add pytest for a GCOV test-case
Hi Martin, * If we now have an (even optional) dependency on python/pytest, this (with the exact versions and use) needs to be documented in install.texi. >>> >>> Done that. >> +be installed. Some optional tests also require Python3 and pytest >> module. >> It would be better to be more specific here. Or would Python 3.0 and >> pytest 2.0.0 do ;-) > > I would leave it as it is. Python3 is a well established term. About pytest: ... but unfortunately not exactly precise beyond distinguishing between Python 2.x and 3.x. I'd specificially asked about 3.0 because IIRC the early 3.x versions were not only incompatible with 2.7, but even among themselves. Don't remember when this finally stabilized. > I don't know how to investigate a minimal version right now. It needn't necessarily be a minimal version, it could be one known to work (and worded appropriately to indicate that's what it is, not a minimum). E.g. from what's bundled with Solaris 11.3, I know that pytest 2.6.4 does work (apart from the python3 vs. python3.4 issue). * On to the implementation: your test for the presence of pytest is wrong: set result [remote_exec host "pytest -m pytest --version"] has nothing to do with what you actually use later: on all of Fedora 29, Ubuntu 20.04, and Solaris 11.4 (with a caveat) pytest is Python 2.7 based, but you don't check that. It is well possible that pytest for 2.7 is installed, but pytest for Python 3.x isn't. Besides, while Solaris 11.4 does bundle pytest, they don't deliver pytest, but only py.test due to a conflict with a different pytest from logilab-common, cf. https://github.com/pytest-dev/pytest/issues/1833. This is immaterial, however, since what you actually run is spawn -noecho python3 -m pytest --color=no -rA -s --tb=no $srcdir/$subdir/$pytest_script So you should just run python3 -m pytest --version instead to check for the presence of the version you're going to use. Btw., there's a mess with pytest on Fedora 29: running the above gives >>> >>> I must confirm this is mess. I definitely don't want to support Python2 and >>> I think >>> the best way would be to use 'env python3', hope it's portable enough. >>> @David: What do you think? >> As I mentioned, it's not: Solaris 11.3 has no python3, only (for the 3.x >> series) python3.4. >> However, I don't understand what you expect to gain from running >> $ env python3 >> rather than just >> $ python3 >> (or a suitable Python 3.x version by any name)? > > All right, let's replace it just with 'python3'. > >> I just had a quick look and the autoconf-archive has AX_PYTHON which >> claims to do that: >> https://www.gnu.org/software/autoconf-archive/ax_python.html >> Unfortunately, it doesn't know about Python 3.8+ yet. I think we can do better than hardcoding python3 here. If it were only for a quirk of an older Solaris version (11.3) which misses the python3 name, together with one (or soon two) optional testcases, I wouldn't care much. When looking at a VM with FreeBSD 12.2 (the current release), it took quite some searching to find how to get python2 and python3 commands installed, and I suspect those are not they only ones where it's either difficult or missing completely. Given that David also plans to use python3, we could use another approach: Rather than relying on an obscure/unmaintained macro from the autoconf-archive, automake includes AM_PATH_PYTHON which allows to specify a minimum version and knows about python2, python3, and python[23].x names. While the version of our current prerequisite, Automake 1.15.1, has a hardcoded list going up to python3.5 only, the git version has removed that limitation. What we should do to get this right is: * import that git automake m4/python.m4 into our toplevel config * call AM_PATH_PYTHON appropriately from gcc/configure.ac * record the resulting python path in site.exp * use it from there e.g. in gcov.exp This way we're future-proof for all possible uses of python3, rather than fiddling with the issue over and over again. I'm sorry to burden you with this, but given that you're the first to follow through with using python3 in the core of gcc rather than in some contrib script, we should get things right now if reasonably possible. The rest of the patch looks good now and passed my testing. Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University
Re: [PATCH] Add pytest for a GCOV test-case
On 1/14/21 2:22 PM, Rainer Orth wrote: Hi Martin, * Besides, the test outcomes are not generic message facilities but are supposed to follow a common format: : [] with the pathname to the test relative to (in this case) gcc/testsuite. In this case, this might be something like UNSUPPORTED: g++.dg/gcov/pr98273.C run-gcov-pytest Currently, you don't have the pathname in run-gcov-pytest, though. All right, now one will see: UNSUPPORTED: g++.dg/gcov/pr98273.C run-gcov-pytest could not find Python interpreter and (or) pytest module Hello. please shorten this quite a bit: maybe ... run-gcov-pytest python3 pytest missing Sure, done. * If we now have an (even optional) dependency on python/pytest, this (with the exact versions and use) needs to be documented in install.texi. Done that. +be installed. Some optional tests also require Python3 and pytest module. It would be better to be more specific here. Or would Python 3.0 and pytest 2.0.0 do ;-) I would leave it as it is. Python3 is a well established term. About pytest: I don't know how to investigate a minimal version right now. * On to the implementation: your test for the presence of pytest is wrong: set result [remote_exec host "pytest -m pytest --version"] has nothing to do with what you actually use later: on all of Fedora 29, Ubuntu 20.04, and Solaris 11.4 (with a caveat) pytest is Python 2.7 based, but you don't check that. It is well possible that pytest for 2.7 is installed, but pytest for Python 3.x isn't. Besides, while Solaris 11.4 does bundle pytest, they don't deliver pytest, but only py.test due to a conflict with a different pytest from logilab-common, cf. https://github.com/pytest-dev/pytest/issues/1833. This is immaterial, however, since what you actually run is spawn -noecho python3 -m pytest --color=no -rA -s --tb=no $srcdir/$subdir/$pytest_script So you should just run python3 -m pytest --version instead to check for the presence of the version you're going to use. Btw., there's a mess with pytest on Fedora 29: running the above gives I must confirm this is mess. I definitely don't want to support Python2 and I think the best way would be to use 'env python3', hope it's portable enough. @David: What do you think? As I mentioned, it's not: Solaris 11.3 has no python3, only (for the 3.x series) python3.4. However, I don't understand what you expect to gain from running $ env python3 rather than just $ python3 (or a suitable Python 3.x version by any name)? All right, let's replace it just with 'python3'. I just had a quick look and the autoconf-archive has AX_PYTHON which claims to do that: https://www.gnu.org/software/autoconf-archive/ax_python.html Unfortunately, it doesn't know about Python 3.8+ yet. When running the test on Solaris 11.4 (with the bundled pytest 4.4.0), I get = test session starts == platform sunos5 -- Python 3.7.9, pytest-4.4.0, py-1.8.0, pluggy-0.9.0 rootdir: /vol/gcc/src/hg/master/local/gcc/testsuite/g++.dg/gcov collected 2 items ../../../../../../../../../vol/gcc/src/hg/master/local/gcc/testsuite/g++.dg/gcov/test-pr98273.py .. === 2 passed in 0.04 seconds === while 4.6.9 on Linux gives = test session starts == platform linux -- Python 3.8.2, pytest-4.6.9, py-1.8.1, pluggy-0.13.0 rootdir: /vol/gcc/src/hg/master/local/gcc/testsuite/g++.dg/gcov collected 2 items ../../../../../../../../../../vol/gcc/src/hg/master/local/gcc/testsuite/g++.dg/gcov/test-pr98273.py .. === short test summary info PASSED ../../../../../../../../../../vol/gcc/src/hg/master/local/gcc/testsuite/g++.dg/gcov/test-pr98273.py::test_basics PASSED ../../../../../../../../../../vol/gcc/src/hg/master/local/gcc/testsuite/g++.dg/gcov/test-pr98273.py::test_lines === 2 passed in 0.17 seconds === Obviously pytest -rA was introduced only after 4.4.0 and the 'A' is silently ignored. Fortunately, I can just use -rap instead which works with both versions. This will be fixed by this: env python3 -m pytest --color=no -rA -s --tb=no --version No, as I already wrote: pytest 4.4.0 silently ignores -rA and doesn't print the PASSED (or FAILED) lines. With both versions, pytest -rap worked for me instead. Ah, all right, I fixed that. After this has been processed by gcov.exp, g++.sum contains PASS: ../../../../../../../../../../vol/gcc/src/hg/master/local/gcc/testsuite/g++.dg/gcov/test-pr98273.py::test_basic PASS: ../../../../../../../../../../vol/gcc/src/hg/master/local/gcc/testsuite/g++.dg/gcov/test-pr98273.py::test_line which is again completely wrong in light of what I wrote above on the format of test names: it lacks
Re: [PATCH] Add pytest for a GCOV test-case
Hi Martin, >>> * If we now have an (even optional) dependency on python/pytest, this >>>(with the exact versions and use) needs to be documented in >>>install.texi. >> >> Done that. > > +be installed. Some optional tests also require Python3 and pytest module. > > It would be better to be more specific here. Or would Python 3.0 and > pytest 2.0.0 do ;-) and a nit I just noticed: two spaces after full stop. Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University
Re: [PATCH] Add pytest for a GCOV test-case
Hi Martin, >> * Besides, the test outcomes are not generic message facilities but are >>supposed to follow a common format: >>: [] >>with the pathname to the test relative to (in this case) >>gcc/testsuite. In this case, this might be something like >>UNSUPPORTED: g++.dg/gcov/pr98273.C run-gcov-pytest >>Currently, you don't have the pathname in run-gcov-pytest, though. > > All right, now one will see: > > UNSUPPORTED: g++.dg/gcov/pr98273.C run-gcov-pytest could not find Python > interpreter and (or) pytest module please shorten this quite a bit: maybe ... run-gcov-pytest python3 pytest missing >> * If we now have an (even optional) dependency on python/pytest, this >>(with the exact versions and use) needs to be documented in >>install.texi. > > Done that. +be installed. Some optional tests also require Python3 and pytest module. It would be better to be more specific here. Or would Python 3.0 and pytest 2.0.0 do ;-) >> * On to the implementation: your test for the presence of pytest is >>wrong: >> set result [remote_exec host "pytest -m pytest --version"] >>has nothing to do with what you actually use later: on all of Fedora >>29, Ubuntu 20.04, and Solaris 11.4 (with a caveat) pytest is Python >>2.7 based, but you don't check that. It is well possible that pytest >>for 2.7 is installed, but pytest for Python 3.x isn't. >>Besides, while Solaris 11.4 does bundle pytest, they don't deliver >>pytest, but only py.test due to a conflict with a different pytest from >>logilab-common, cf. https://github.com/pytest-dev/pytest/issues/1833. >>This is immaterial, however, since what you actually run is >> spawn -noecho python3 -m pytest --color=no -rA -s --tb=no >> $srcdir/$subdir/$pytest_script >>So you should just run python3 -m pytest --version instead to check >>for the presence of the version you're going to use. >>Btw., there's a mess with pytest on Fedora 29: running the above gives > > I must confirm this is mess. I definitely don't want to support Python2 and > I think > the best way would be to use 'env python3', hope it's portable enough. > @David: What do you think? As I mentioned, it's not: Solaris 11.3 has no python3, only (for the 3.x series) python3.4. However, I don't understand what you expect to gain from running $ env python3 rather than just $ python3 (or a suitable Python 3.x version by any name)? I just had a quick look and the autoconf-archive has AX_PYTHON which claims to do that: https://www.gnu.org/software/autoconf-archive/ax_python.html Unfortunately, it doesn't know about Python 3.8+ yet. >>When running the test on Solaris 11.4 (with the bundled pytest 4.4.0), >>I get >> = test session starts >> == >> platform sunos5 -- Python 3.7.9, pytest-4.4.0, py-1.8.0, pluggy-0.9.0 >> rootdir: /vol/gcc/src/hg/master/local/gcc/testsuite/g++.dg/gcov >> collected 2 items >> ../../../../../../../../../vol/gcc/src/hg/master/local/gcc/testsuite/g++.dg/gcov/test-pr98273.py >> .. >> === 2 passed in 0.04 seconds >> === >> while 4.6.9 on Linux gives >> = test session starts >> == >> platform linux -- Python 3.8.2, pytest-4.6.9, py-1.8.1, pluggy-0.13.0 >> rootdir: /vol/gcc/src/hg/master/local/gcc/testsuite/g++.dg/gcov >> collected 2 items >> ../../../../../../../../../../vol/gcc/src/hg/master/local/gcc/testsuite/g++.dg/gcov/test-pr98273.py >> .. >> === short test summary info >> >> PASSED >> ../../../../../../../../../../vol/gcc/src/hg/master/local/gcc/testsuite/g++.dg/gcov/test-pr98273.py::test_basics >> PASSED >> ../../../../../../../../../../vol/gcc/src/hg/master/local/gcc/testsuite/g++.dg/gcov/test-pr98273.py::test_lines >> === 2 passed in 0.17 seconds >> === >>Obviously pytest -rA was introduced only after 4.4.0 and the 'A' is >>silently ignored. Fortunately, I can just use -rap instead which >>works with both versions. > > This will be fixed by this: > env python3 -m pytest --color=no -rA -s --tb=no --version No, as I already wrote: pytest 4.4.0 silently ignores -rA and doesn't print the PASSED (or FAILED) lines. With both versions, pytest -rap worked for me instead. >>After this has been processed by gcov.exp, g++.sum contains >> PASS: >> ../../../../../../../../../../vol/gcc/src/hg/master/local/gcc/testsuite/g++.dg/gcov/test-pr98273.py::test_basic >> PASS: >> ../../../../../../../../../../vol/gcc/src/hg/master/local/gcc/testsuite/g++.dg/gcov/test-pr98273.py::test_line >>which is again completely wrong in light of what I wrote above on the >>format of test names: it lacks the testname part completely and >>contains absolute pathnames which makes it impossible to compare >>testresults fr
Re: [PATCH] Add pytest for a GCOV test-case
On 1/13/21 2:38 PM, Rainer Orth wrote:
Hi Martin,
On 1/6/21 12:36 AM, Jeff Law wrote:
unresolved "could not find python interpreter $testcase" in
run-gcov-pytest if you find the right magic in the output of your spawn.
Achieved that with the updated patch.
Ready for master?
unfortunately, your patch has a large number of problems:
Hello.
Thank you for the investigation.
* On targets where run-gcov-pytest decides that pytest isn't available
(incorrectly in some cases), mail-report.log is cluttered with
UNRESOLVED: could not find Python interpreter and (or) pytest module for
pr98273.C
I fear you've been misled by David and Jeff here: UNRESOLVED isn't
appropriate for cases like this. Please read the DejaGnu manual for
the semantics of the various test outcomes. If anything (we often
just silently skip testcases that cannot be run on some target), use
UNSUPPORTED instead.
Shame on me, I misread what I was suggested.
* Besides, the test outcomes are not generic message facilities but are
supposed to follow a common format:
: []
with the pathname to the test relative to (in this case)
gcc/testsuite. In this case, this might be something like
UNSUPPORTED: g++.dg/gcov/pr98273.C run-gcov-pytest
Currently, you don't have the pathname in run-gcov-pytest, though.
All right, now one will see:
UNSUPPORTED: g++.dg/gcov/pr98273.C run-gcov-pytest could not find Python
interpreter and (or) pytest module
* If we now have an (even optional) dependency on python/pytest, this
(with the exact versions and use) needs to be documented in
install.texi.
Done that.
* Speaking of documenting, the new run-gcov-pytest needs to be
documented in sourcebuild.texi.
Likewise here.
* On to the implementation: your test for the presence of pytest is
wrong:
set result [remote_exec host "pytest -m pytest --version"]
has nothing to do with what you actually use later: on all of Fedora
29, Ubuntu 20.04, and Solaris 11.4 (with a caveat) pytest is Python
2.7 based, but you don't check that. It is well possible that pytest
for 2.7 is installed, but pytest for Python 3.x isn't.
Besides, while Solaris 11.4 does bundle pytest, they don't deliver
pytest, but only py.test due to a conflict with a different pytest from
logilab-common, cf. https://github.com/pytest-dev/pytest/issues/1833.
This is immaterial, however, since what you actually run is
spawn -noecho python3 -m pytest --color=no -rA -s --tb=no
$srcdir/$subdir/$pytest_script
So you should just run python3 -m pytest --version instead to check
for the presence of the version you're going to use.
Btw., there's a mess with pytest on Fedora 29: running the above gives
I must confirm this is mess. I definitely don't want to support Python2 and I
think
the best way would be to use 'env python3', hope it's portable enough.
@David: What do you think?
[...]
pluggy.PluginValidationError: Plugin 'benchmark' could not be loaded: (pytest
3.6.4 (/usr/lib/python3.7/site-packages), Requirement.parse('pytest>=3.8'))!
Seems the packagers have broken things there.
On top of all this, I wonder why you insist on a particular Python
version here: I tried your single testcase and it PASSes just as well
with Python 2.7!? One reason I'm asking is that Solaris 11.3 bundles
both Python 2.7 and 3.4, but (unlike Linux and Solaris 11.4) don't
have /usr/bin/python3, just python (which is 2.7), python2.7, and
python3.4. Not that it matters too much, but you should be aware of
the issue.
When running the test on Solaris 11.4 (with the bundled pytest 4.4.0),
I get
= test session starts ==
platform sunos5 -- Python 3.7.9, pytest-4.4.0, py-1.8.0, pluggy-0.9.0
rootdir: /vol/gcc/src/hg/master/local/gcc/testsuite/g++.dg/gcov
collected 2 items
../../../../../../../../../vol/gcc/src/hg/master/local/gcc/testsuite/g++.dg/gcov/test-pr98273.py
..
=== 2 passed in 0.04 seconds ===
while 4.6.9 on Linux gives
= test session starts ==
platform linux -- Python 3.8.2, pytest-4.6.9, py-1.8.1, pluggy-0.13.0
rootdir: /vol/gcc/src/hg/master/local/gcc/testsuite/g++.dg/gcov
collected 2 items
../../../../../../../../../../vol/gcc/src/hg/master/local/gcc/testsuite/g++.dg/gcov/test-pr98273.py
..
=== short test summary info
PASSED
../../../../../../../../../../vol/gcc/src/hg/master/local/gcc/testsuite/g++.dg/gcov/test-pr98273.py::test_basics
PASSED
../../../../../../../../../../vol/gcc/src/hg/master/local/gcc/testsuite/g++.dg/gcov/test-pr98273.py::test_lines
=== 2 passed in 0.17 seconds ===
Obviously pytest -rA was introduced only after 4.4.0 and the 'A' is
silently ignored. Fortunately, I can ju
Re: [PATCH] Add pytest for a GCOV test-case
Hi David, >> On top of all this, I wonder why you insist on a particular Python >> version here: I tried your single testcase and it PASSes just as >> well >> with Python 2.7!? One reason I'm asking is that Solaris 11.3 >> bundles >> both Python 2.7 and 3.4, but (unlike Linux and Solaris 11.4) don't >> have /usr/bin/python3, just python (which is 2.7), python2.7, and >> python3.4. Not that it matters too much, but you should be aware >> of >> the issue. > > In particular, given the differences between Python 2 and Python 3 I > think it's a good idea to be explicit about the versions of Python that > we expect i.e. are such tests coded to the common subset of Python 2 > and 3, or just to Python 3? Given that they're intended to be > optional, I suggest just Python 3. (I would like to be able to add > other Python-based scripts to the DejaGnu suite e.g. to verify JSON > output, hence I'd prefer to not have to maintain Python 2 > compatibility) understood. The whole Python 2 vs. 3 situation is a mess, unfortunately, but I expect that all systems we deal with that have Python at all will have some version of Python 3 around, and moving forward Python 2 will phase out over time given the lack of upstream support. Dealing with both at the current time seems a waste of time. The only possible challenge may be to determine a python3 executable, given that this exact name seems not to be ubiquitious. Maybe there's an autoconf macro that does the necessary work instead of having to do this in the testsuite directly? Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University
Re: [PATCH] Add pytest for a GCOV test-case
On Wed, 2021-01-13 at 14:38 +0100, Rainer Orth wrote: > Hi Martin, > > > On 1/6/21 12:36 AM, Jeff Law wrote: > > > unresolved "could not find python interpreter $testcase" in > > > run-gcov-pytest if you find the right magic in the output of your > > > spawn. > > > > Achieved that with the updated patch. > > > > Ready for master? > > unfortunately, your patch has a large number of problems: > > * On targets where run-gcov-pytest decides that pytest isn't > available > (incorrectly in some cases), mail-report.log is cluttered with > > UNRESOLVED: could not find Python interpreter and (or) pytest module > for pr98273.C > > I fear you've been misled by David and Jeff here: UNRESOLVED isn't > appropriate for cases like this. Please read the DejaGnu manual > for > the semantics of the various test outcomes. If anything (we often > just silently skip testcases that cannot be run on some target), > use > UNSUPPORTED instead. This may be lack of sleep on my part, but looking back at the emails, I said that this case should be "UNSUPPORTED", but it looks like Martin misread it as "UNRESOLVED", and I didn't correct him. Sorry. > * Besides, the test outcomes are not generic message facilities but > are > supposed to follow a common format: > > : [] > > with the pathname to the test relative to (in this case) > gcc/testsuite. In this case, this might be something like > > UNSUPPORTED: g++.dg/gcov/pr98273.C run-gcov-pytest > > Currently, you don't have the pathname in run-gcov-pytest, though. > > * If we now have an (even optional) dependency on python/pytest, this > (with the exact versions and use) needs to be documented in > install.texi. Agreed. [...snip various other issues...] > > On top of all this, I wonder why you insist on a particular Python > version here: I tried your single testcase and it PASSes just as > well > with Python 2.7!? One reason I'm asking is that Solaris 11.3 > bundles > both Python 2.7 and 3.4, but (unlike Linux and Solaris 11.4) don't > have /usr/bin/python3, just python (which is 2.7), python2.7, and > python3.4. Not that it matters too much, but you should be aware > of > the issue. In particular, given the differences between Python 2 and Python 3 I think it's a good idea to be explicit about the versions of Python that we expect i.e. are such tests coded to the common subset of Python 2 and 3, or just to Python 3? Given that they're intended to be optional, I suggest just Python 3. (I would like to be able to add other Python-based scripts to the DejaGnu suite e.g. to verify JSON output, hence I'd prefer to not have to maintain Python 2 compatibility) [...snip...] Dave
Re: [PATCH] Add pytest for a GCOV test-case
Hi Martin,
> On 1/6/21 12:36 AM, Jeff Law wrote:
>> unresolved "could not find python interpreter $testcase" in
>> run-gcov-pytest if you find the right magic in the output of your spawn.
>
> Achieved that with the updated patch.
>
> Ready for master?
unfortunately, your patch has a large number of problems:
* On targets where run-gcov-pytest decides that pytest isn't available
(incorrectly in some cases), mail-report.log is cluttered with
UNRESOLVED: could not find Python interpreter and (or) pytest module for
pr98273.C
I fear you've been misled by David and Jeff here: UNRESOLVED isn't
appropriate for cases like this. Please read the DejaGnu manual for
the semantics of the various test outcomes. If anything (we often
just silently skip testcases that cannot be run on some target), use
UNSUPPORTED instead.
* Besides, the test outcomes are not generic message facilities but are
supposed to follow a common format:
: []
with the pathname to the test relative to (in this case)
gcc/testsuite. In this case, this might be something like
UNSUPPORTED: g++.dg/gcov/pr98273.C run-gcov-pytest
Currently, you don't have the pathname in run-gcov-pytest, though.
* If we now have an (even optional) dependency on python/pytest, this
(with the exact versions and use) needs to be documented in
install.texi.
* Speaking of documenting, the new run-gcov-pytest needs to be
documented in sourcebuild.texi.
* On to the implementation: your test for the presence of pytest is
wrong:
set result [remote_exec host "pytest -m pytest --version"]
has nothing to do with what you actually use later: on all of Fedora
29, Ubuntu 20.04, and Solaris 11.4 (with a caveat) pytest is Python
2.7 based, but you don't check that. It is well possible that pytest
for 2.7 is installed, but pytest for Python 3.x isn't.
Besides, while Solaris 11.4 does bundle pytest, they don't deliver
pytest, but only py.test due to a conflict with a different pytest from
logilab-common, cf. https://github.com/pytest-dev/pytest/issues/1833.
This is immaterial, however, since what you actually run is
spawn -noecho python3 -m pytest --color=no -rA -s --tb=no
$srcdir/$subdir/$pytest_script
So you should just run python3 -m pytest --version instead to check
for the presence of the version you're going to use.
Btw., there's a mess with pytest on Fedora 29: running the above gives
[...]
pluggy.PluginValidationError: Plugin 'benchmark' could not be loaded: (pytest
3.6.4 (/usr/lib/python3.7/site-packages), Requirement.parse('pytest>=3.8'))!
Seems the packagers have broken things there.
On top of all this, I wonder why you insist on a particular Python
version here: I tried your single testcase and it PASSes just as well
with Python 2.7!? One reason I'm asking is that Solaris 11.3 bundles
both Python 2.7 and 3.4, but (unlike Linux and Solaris 11.4) don't
have /usr/bin/python3, just python (which is 2.7), python2.7, and
python3.4. Not that it matters too much, but you should be aware of
the issue.
When running the test on Solaris 11.4 (with the bundled pytest 4.4.0),
I get
= test session starts ==
platform sunos5 -- Python 3.7.9, pytest-4.4.0, py-1.8.0, pluggy-0.9.0
rootdir: /vol/gcc/src/hg/master/local/gcc/testsuite/g++.dg/gcov
collected 2 items
../../../../../../../../../vol/gcc/src/hg/master/local/gcc/testsuite/g++.dg/gcov/test-pr98273.py
..
=== 2 passed in 0.04 seconds ===
while 4.6.9 on Linux gives
= test session starts ==
platform linux -- Python 3.8.2, pytest-4.6.9, py-1.8.1, pluggy-0.13.0
rootdir: /vol/gcc/src/hg/master/local/gcc/testsuite/g++.dg/gcov
collected 2 items
../../../../../../../../../../vol/gcc/src/hg/master/local/gcc/testsuite/g++.dg/gcov/test-pr98273.py
..
=== short test summary info
PASSED
../../../../../../../../../../vol/gcc/src/hg/master/local/gcc/testsuite/g++.dg/gcov/test-pr98273.py::test_basics
PASSED
../../../../../../../../../../vol/gcc/src/hg/master/local/gcc/testsuite/g++.dg/gcov/test-pr98273.py::test_lines
=== 2 passed in 0.17 seconds ===
Obviously pytest -rA was introduced only after 4.4.0 and the 'A' is
silently ignored. Fortunately, I can just use -rap instead which
works with both versions.
After this has been processed by gcov.exp, g++.sum contains
PASS:
../../../../../../../../../../vol/gcc/src/hg/master/local/gcc/testsuite/g++.dg/gcov/test-pr98273.py::test_basic
PASS:
../../../../../../../../../../vol/gcc/src/hg/master/local/gcc/testsuite/g++.dg/gcov/test-pr98273.py::test_line
which is again completely wrong in light of wh
Re: [PATCH] Add pytest for a GCOV test-case
On 1/7/21 9:14 AM, Martin Liška wrote: > On 1/6/21 12:36 AM, Jeff Law wrote: >> unresolved "could not find python interpreter $testcase" in >> run-gcov-pytest if you find the right magic in the output of your spawn. > > Achieved that with the updated patch. > > Ready for master? > Thanks, > Martin > > 0001-Add-pytest-for-a-GCOV-test-case.patch > > From 53f5169156044acf8ecec498aa89d6be44c7173a Mon Sep 17 00:00:00 2001 > From: Martin Liska > Date: Mon, 21 Dec 2020 09:14:28 +0100 > Subject: [PATCH] Add pytest for a GCOV test-case > > gcc/testsuite/ChangeLog: > > PR gcov-profile/98273 > * lib/gcov.exp: Add run-gcov-pytest function which runs pytest. > * g++.dg/gcov/pr98273.C: New test. > * g++.dg/gcov/gcov.py: New test. > * g++.dg/gcov/test-pr98273.py: New test. OK jeff
Re: [PATCH] Add pytest for a GCOV test-case
On 1/6/21 12:36 AM, Jeff Law wrote:
unresolved "could not find python interpreter $testcase" in
run-gcov-pytest if you find the right magic in the output of your spawn.
Achieved that with the updated patch.
Ready for master?
Thanks,
Martin
>From 53f5169156044acf8ecec498aa89d6be44c7173a Mon Sep 17 00:00:00 2001
From: Martin Liska
Date: Mon, 21 Dec 2020 09:14:28 +0100
Subject: [PATCH] Add pytest for a GCOV test-case
gcc/testsuite/ChangeLog:
PR gcov-profile/98273
* lib/gcov.exp: Add run-gcov-pytest function which runs pytest.
* g++.dg/gcov/pr98273.C: New test.
* g++.dg/gcov/gcov.py: New test.
* g++.dg/gcov/test-pr98273.py: New test.
---
gcc/testsuite/g++.dg/gcov/gcov.py | 10 +
gcc/testsuite/g++.dg/gcov/pr98273.C | 24
gcc/testsuite/g++.dg/gcov/test-pr98273.py | 27 ++
gcc/testsuite/lib/gcov.exp| 45 +++
4 files changed, 106 insertions(+)
create mode 100644 gcc/testsuite/g++.dg/gcov/gcov.py
create mode 100644 gcc/testsuite/g++.dg/gcov/pr98273.C
create mode 100644 gcc/testsuite/g++.dg/gcov/test-pr98273.py
diff --git a/gcc/testsuite/g++.dg/gcov/gcov.py b/gcc/testsuite/g++.dg/gcov/gcov.py
new file mode 100644
index 000..a8c4ea9ae71
--- /dev/null
+++ b/gcc/testsuite/g++.dg/gcov/gcov.py
@@ -0,0 +1,10 @@
+import gzip
+import json
+import os
+
+
+def gcov_from_env():
+# return parsed JSON content a GCOV_PATH file
+json_filename = os.environ['GCOV_PATH'] + '.gcov.json.gz'
+json_data = gzip.open(json_filename).read()
+return json.loads(json_data)
diff --git a/gcc/testsuite/g++.dg/gcov/pr98273.C b/gcc/testsuite/g++.dg/gcov/pr98273.C
new file mode 100644
index 000..bfa83cbe4d0
--- /dev/null
+++ b/gcc/testsuite/g++.dg/gcov/pr98273.C
@@ -0,0 +1,24 @@
+/* PR gcov-profile/98273 */
+
+/* { dg-options "--coverage -std=c++11" } */
+/* { dg-do run { target native } } */
+
+int
+main ()
+{
+ int i = 42;
+ {
+auto f = [] () {
+ auto g = [] () {};
+ g ();
+ g ();
+};
+f ();
+ }
+ ++i;
+ ++i;
+ ++i;
+ return 45 - i;
+}
+
+/* { dg-final { run-gcov-pytest pr98273.C "test-pr98273.py" } } */
diff --git a/gcc/testsuite/g++.dg/gcov/test-pr98273.py b/gcc/testsuite/g++.dg/gcov/test-pr98273.py
new file mode 100644
index 000..6cb39d10c1e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/gcov/test-pr98273.py
@@ -0,0 +1,27 @@
+from gcov import gcov_from_env
+
+import pytest
+
+
+@pytest.fixture(scope='function', autouse=True)
+def gcov():
+return gcov_from_env()
+
+
+def test_basics(gcov):
+files = gcov['files']
+assert len(files) == 1
+functions = files[0]['functions']
+assert len(functions) == 3
+
+
+def test_lines(gcov):
+lines = gcov['files'][0]['lines']
+linesdict = {}
+for line in lines:
+linesdict[int(line['line_number'])] = line
+
+assert linesdict[21]['function_name'] == 'main'
+assert linesdict[15]['function_name'] == '_ZZ4mainENKUlvE_clEv'
+assert (linesdict[12]['function_name']
+== '_ZZZ4mainENKUlvE_clEvENKUlvE_clEv')
diff --git a/gcc/testsuite/lib/gcov.exp b/gcc/testsuite/lib/gcov.exp
index bb956439cf7..4bcab1d3f1d 100644
--- a/gcc/testsuite/lib/gcov.exp
+++ b/gcc/testsuite/lib/gcov.exp
@@ -247,6 +247,51 @@ proc verify-calls { testname testcase file } {
return $failed
}
+# Call by dg-final to run gcov --json-format which produces a JSON file
+# that is later analysed by a pytest Python script.
+# We pass filename of a test via GCOV_PATH environment variable.
+
+proc run-gcov-pytest { args } {
+global GCOV
+global srcdir subdir
+# Extract the test file name from the arguments.
+set testcase [lindex $args 0]
+
+verbose "Running $GCOV $testcase in $srcdir/$subdir" 2
+set testcase [remote_download host $testcase]
+set result [remote_exec host $GCOV "$testcase -i"]
+
+set result [remote_exec host "pytest -m pytest --version"]
+set status [lindex $result 0]
+if { $status != 0 } then {
+ unresolved "could not find Python interpreter and (or) pytest module for $testcase"
+ return
+}
+
+set pytest_script [lindex $args 1]
+setenv GCOV_PATH $testcase
+verbose "pytest_script: $pytest_script" 2
+spawn -noecho python3 -m pytest --color=no -rA -s --tb=no $srcdir/$subdir/$pytest_script
+
+set prefix "\[^\r\n\]*"
+expect {
+ -re "FAILED($prefix)\[^\r\n\]+\r\n" {
+ fail "$expect_out(1,string)"
+ exp_continue
+ }
+ -re "ERROR($prefix)\[^\r\n\]+\r\n" {
+ fail "$expect_out(1,string)"
+ exp_continue
+ }
+ -re "PASSED($prefix)\[^\r\n\]+\r\n" {
+ pass "$expect_out(1,string)"
+ exp_continue
+ }
+}
+
+clean-gcov $testcase
+}
+
# Called by dg-final to run gcov and analyze the results.
#
# ARGS consists of the optional strings "branches" and/or "calls",
--
2.29.2
Re: [PATCH] Add pytest for a GCOV test-case
On 12/23/20 6:03 AM, Martin Liška wrote: > At a high level, this patch calls out to Python 3, allowing for test >> logic to be written in Python, rather than Tcl. Are we doing this >> anywhere else in our test suite? > > No. I'm surprised. I thought we did this for some of David's work at some point. Clearly I'm mis-remembering. >> >> The test implicitly requires python3, and the 3rd party pytest module >> installed within it. What happens if these aren't installed? (ideally >> an UNSUPPORTED at the DejaGnu level, I think). > > Right now, one will see the following in the .log file: > > /usr/bin/python3: No module named pytest > > > I must confess that I don't know how to properly mark that as UNRESOLVED > in DejaGNU. I think it's just something like unresolved "could not find python interpreter $testcase" in run-gcov-pytest if you find the right magic in the output of your spawn. Jeff
Re: [PATCH] Add pytest for a GCOV test-case
On 12/22/20 6:49 PM, David Malcolm wrote:
On Tue, 2020-12-22 at 12:39 +0100, Martin Liška wrote:
Hello.
The patch adds a new test for an existing GCOV test-case. Newly
added run-gcov-pytest parses JSON format produced by GCOV and
runs pytest on it.
Patch can bootstrap on x86_64-linux-gnu and survives regression
tests.
At a high level, this patch calls out to Python 3, allowing for test
logic to be written in Python, rather than Tcl. Are we doing this
anywhere else in our test suite?
No.
I'm in favor of this (I'm much more
comfortable in Python than in Tcl, I dread anytime I have to touch the
Tcl code).
Yes, that was my original motivation. I always suffer when I'm supposed
to come up with a test-case.
The test implicitly requires python3, and the 3rd party pytest module
installed within it. What happens if these aren't installed? (ideally
an UNSUPPORTED at the DejaGnu level, I think).
Right now, one will see the following in the .log file:
/usr/bin/python3: No module named pytest
I must confess that I don't know how to properly mark that as UNRESOLVED
in DejaGNU.
Some further comments inline below...
Ready to be installed?
Thanks,
Martin
gcc/testsuite/ChangeLog:
PR gcov-profile/98273
* lib/gcov.exp: Add run-gcov-pytest function which runs pytest.
* g++.dg/gcov/pr98273.C: New test.
* g++.dg/gcov/gcov.py: New test.
* g++.dg/gcov/test-pr98273.py: New test.
---
gcc/testsuite/g++.dg/gcov/gcov.py | 10
gcc/testsuite/g++.dg/gcov/pr98273.C | 24 +++
gcc/testsuite/g++.dg/gcov/test-pr98273.py | 27
++
gcc/testsuite/lib/gcov.exp| 28
+++
4 files changed, 89 insertions(+)
create mode 100644 gcc/testsuite/g++.dg/gcov/gcov.py
create mode 100644 gcc/testsuite/g++.dg/gcov/pr98273.C
create mode 100644 gcc/testsuite/g++.dg/gcov/test-pr98273.py
diff --git a/gcc/testsuite/g++.dg/gcov/gcov.py
b/gcc/testsuite/g++.dg/gcov/gcov.py
new file mode 100644
index 000..a8c4ea9ae71
--- /dev/null
+++ b/gcc/testsuite/g++.dg/gcov/gcov.py
@@ -0,0 +1,10 @@
+import gzip
+import json
+import os
+
+
+def gcov_from_env():
+# return parsed JSON content a GCOV_PATH file
+json_filename = os.environ['GCOV_PATH'] + '.gcov.json.gz'
+json_data = gzip.open(json_filename).read()
+return json.loads(json_data)
diff --git a/gcc/testsuite/g++.dg/gcov/pr98273.C
b/gcc/testsuite/g++.dg/gcov/pr98273.C
new file mode 100644
index 000..bfa83cbe4d0
--- /dev/null
+++ b/gcc/testsuite/g++.dg/gcov/pr98273.C
@@ -0,0 +1,24 @@
+/* PR gcov-profile/98273 */
+
+/* { dg-options "--coverage -std=c++11" } */
+/* { dg-do run { target native } } */
+
+int
+main ()
+{
+ int i = 42;
+ {
+auto f = [] () {
+ auto g = [] () {};
+ g ();
+ g ();
+};
+f ();
+ }
+ ++i;
+ ++i;
+ ++i;
+ return 45 - i;
+}
+
+/* { dg-final { run-gcov-pytest pr98273.C "test-pr98273.py" } } */
diff --git a/gcc/testsuite/g++.dg/gcov/test-pr98273.py
b/gcc/testsuite/g++.dg/gcov/test-pr98273.py
new file mode 100644
index 000..6cb39d10c1e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/gcov/test-pr98273.py
I had an idea, not sure if a good one: if all of the test logic is
moved from the .C file to a python script, then perhaps the script
should be the name of the .C file with a .py suffix i.e. here it could
be "pr98273.C.py"
That does not work:
___
ERROR collecting gcc/testsuite/g++.dg/gcov/pr98273.C.py
ImportError while importing test module
'/home/marxin/Programming/gcc/gcc/testsuite/g++.dg/gcov/pr98273.C.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
/usr/lib64/python3.8/importlib/__init__.py:127: in import_module
return _bootstrap._gcd_import(name[level:], package, level)
E ModuleNotFoundError: No module named 'pr98273'
===
short test summary info
ERROR gcc/testsuite/g++.dg/gcov/pr98273.C.py
Interrupted: 1 error during collection
===
1 error in 0.08s
===
Or would there be cases where d
Re: [PATCH] Add pytest for a GCOV test-case
On Tue, 2020-12-22 at 12:39 +0100, Martin Liška wrote:
> Hello.
>
> The patch adds a new test for an existing GCOV test-case. Newly
> added run-gcov-pytest parses JSON format produced by GCOV and
> runs pytest on it.
>
> Patch can bootstrap on x86_64-linux-gnu and survives regression
> tests.
At a high level, this patch calls out to Python 3, allowing for test
logic to be written in Python, rather than Tcl. Are we doing this
anywhere else in our test suite? I'm in favor of this (I'm much more
comfortable in Python than in Tcl, I dread anytime I have to touch the
Tcl code).
The test implicitly requires python3, and the 3rd party pytest module
installed within it. What happens if these aren't installed? (ideally
an UNSUPPORTED at the DejaGnu level, I think).
Some further comments inline below...
> Ready to be installed?
> Thanks,
> Martin
>
> gcc/testsuite/ChangeLog:
>
> PR gcov-profile/98273
> * lib/gcov.exp: Add run-gcov-pytest function which runs pytest.
> * g++.dg/gcov/pr98273.C: New test.
> * g++.dg/gcov/gcov.py: New test.
> * g++.dg/gcov/test-pr98273.py: New test.
> ---
> gcc/testsuite/g++.dg/gcov/gcov.py | 10
> gcc/testsuite/g++.dg/gcov/pr98273.C | 24 +++
> gcc/testsuite/g++.dg/gcov/test-pr98273.py | 27
> ++
> gcc/testsuite/lib/gcov.exp| 28
> +++
> 4 files changed, 89 insertions(+)
> create mode 100644 gcc/testsuite/g++.dg/gcov/gcov.py
> create mode 100644 gcc/testsuite/g++.dg/gcov/pr98273.C
> create mode 100644 gcc/testsuite/g++.dg/gcov/test-pr98273.py
>
> diff --git a/gcc/testsuite/g++.dg/gcov/gcov.py
> b/gcc/testsuite/g++.dg/gcov/gcov.py
> new file mode 100644
> index 000..a8c4ea9ae71
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/gcov/gcov.py
> @@ -0,0 +1,10 @@
> +import gzip
> +import json
> +import os
> +
> +
> +def gcov_from_env():
> +# return parsed JSON content a GCOV_PATH file
> +json_filename = os.environ['GCOV_PATH'] + '.gcov.json.gz'
> +json_data = gzip.open(json_filename).read()
> +return json.loads(json_data)
> diff --git a/gcc/testsuite/g++.dg/gcov/pr98273.C
> b/gcc/testsuite/g++.dg/gcov/pr98273.C
> new file mode 100644
> index 000..bfa83cbe4d0
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/gcov/pr98273.C
> @@ -0,0 +1,24 @@
> +/* PR gcov-profile/98273 */
> +
> +/* { dg-options "--coverage -std=c++11" } */
> +/* { dg-do run { target native } } */
> +
> +int
> +main ()
> +{
> + int i = 42;
> + {
> +auto f = [] () {
> + auto g = [] () {};
> + g ();
> + g ();
> +};
> +f ();
> + }
> + ++i;
> + ++i;
> + ++i;
> + return 45 - i;
> +}
> +
> +/* { dg-final { run-gcov-pytest pr98273.C "test-pr98273.py" } } */
> diff --git a/gcc/testsuite/g++.dg/gcov/test-pr98273.py
> b/gcc/testsuite/g++.dg/gcov/test-pr98273.py
> new file mode 100644
> index 000..6cb39d10c1e
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/gcov/test-pr98273.py
I had an idea, not sure if a good one: if all of the test logic is
moved from the .C file to a python script, then perhaps the script
should be the name of the .C file with a .py suffix i.e. here it could
be "pr98273.C.py"
Or would there be cases where different scripts could be called for the
same .C file? Would scripts get reused between different source files?
(not sure)
> @@ -0,0 +1,27 @@
> +from gcov import gcov_from_env
> +
> +import pytest
> +
> +
> +@pytest.fixture(scope='function', autouse=True)
> +def gcov():
> +return gcov_from_env()
> +
> +
> +def test_basics(gcov):
> +files = gcov['files']
> +assert len(files) == 1
> +functions = files[0]['functions']
> +assert len(functions) == 3
> +
> +
> +def test_lines(gcov):
> +lines = gcov['files'][0]['lines']
> +linesdict = {}
> +for line in lines:
> +linesdict[int(line['line_number'])] = line
> +
> +assert linesdict[21]['function_name'] == 'main'
> +assert linesdict[15]['function_name'] == '_ZZ4mainENKUlvE_clEv'
> +assert (linesdict[12]['function_name']
> +== '_ZZZ4mainENKUlvE_clEvENKUlvE_clEv')
> diff --git a/gcc/testsuite/lib/gcov.exp b/gcc/testsuite/lib/gcov.exp
> index 9276aead06b..dd589d4dd8a 100644
> --- a/gcc/testsuite/lib/gcov.exp
> +++ b/gcc/testsuite/lib/gcov.exp
> @@ -247,6 +247,34 @@ proc verify-calls { testname testcase file } {
> return $failed
> }
>
> +proc run-gcov-pytest { args } {
I think this function needs a leading comment, talking about what is
invoked, and the kinds of lines that are parsed.
What happens if an exception is raised by the script? e.g. if there's
a SyntaxError in the script, ideally it should "bubble up" through the
Tcl layer into a DejaGnu "ERROR" I think, rather than being silently
dropped.
> +global GCOV
> +global srcdir subdir
> +# Extract the test file name from the arguments.
> +set testcase [lindex $args 0]
> +
> +verbose "Running $GCOV $testcase in $srcdir/$s
