bpo-36954: Skip test_recursive_repr test when running test_xml_etree under coverage trace by GPHemsley · Pull Request #13267 · python/cpython (original) (raw)

GPHemsley

@GPHemsley

…age trace

The test_recursive_repr test modifies the tracing environment, causing the remainder of the tests to lose coverage information.

@GPHemsley

@serhiy-storchaka

How does it modify the tracing environment?

Please open an issue for discussion. This is not trivial PR like fixing a typo.

@blueyed

The RuntimeError causes the settrace to get lost.

https://bugs.python.org/issue10933
https://bugs.python.org/issue23012
https://bugs.python.org/issue36474

Instead of skipping the test it should restore any sys.gettrace() in the end, e.g. for pytest it would be:

@pytest.fixture(autouse=True) def restore_settrace(): """(Re)store sys.gettrace after test run.

This is required to re-enable coverage tracking.
"""
assert sys.gettrace() is _orig_trace

yield

newtrace = sys.gettrace()
if newtrace is not _orig_trace:
    sys.settrace(_orig_trace)
    assert newtrace is None

(I am using this with pdbpp's tests, where sys.settrace is called all the time in the tests)

@blueyed

..but since this is cpython itself, it would make more sense to fix the issue in the first place.. :)

@GPHemsley

To be clear, my goal with this PR was to stop this test from destroying the code coverage for the rest of the file. I didn't dive into why this test was causing the problem.

@serhiy-storchaka: Given the above tickets and discussion, do you still want a separate ticket for investigating this test?

@blueyed

To be clear, my goal with this PR was to stop this test from destroying the code coverage for the rest of the file.

Do you agree that it's better to restore tracing than not running the test at all?
(if all CI jobs use coverage, it would be skipped then completely)

@GPHemsley

To be clear, my goal with this PR was to stop this test from destroying the code coverage for the rest of the file.

Do you agree that it's better to restore tracing than not running the test at all?
(if all CI jobs use coverage, it would be skipped then completely)

The primary jobs on Travis CI (I can't speak to the others) do not use coverage; coverage is run in separate jobs. So the primary jobs will still run the test -- and will fail if the test fails.

@serhiy-storchaka

Please open a new issue for coverage (or tracing?) and RecursionError. test_recursive_repr does not have particular relation to tracing, it is just one place where this issue is reproduced. I think you can create simple example that demonstrates the problem.

@blueyed

@serhiy-storchaka

Please open a new issue for coverage (or tracing?) and RecursionError

We have those already (see above).

test_recursive_repr does not have particular relation to tracing, it is just one place where this issue is reproduced. I think you can create simple example that demonstrates the problem.

It is not meant to show the issue, but this PR is about not losing coverage after this test (running into / affected by the issue) is being run.

@GPHemsley
I'd still suggest to restore tracing after the test (just wrap it in try / finally).

blueyed

@@ -2320,6 +2320,7 @@ def __eq__(self, o):
e.extend([ET.Element('bar')])
self.assertRaises(ValueError, e.remove, X('baz'))
@unittest.skipIf(sys.gettrace(), "Skips under coverage.")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just for completeness: this would also skip it when using pdb then.. ;)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the record, I merely copied a line already used by another test. :)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@GPHemsley GPHemsley changed the titleSkip test_recursive_repr test when running test_xml_etree under coverage trace bpo-36954: Skip test_recursive_repr test when running test_xml_etree under coverage trace

May 18, 2019

@GPHemsley