Skip to content

Commit

Permalink
- fixed collection errors incorrectly being handled as process crashes;
Browse files Browse the repository at this point in the history
- added handling of exceptions in subprocesses, more or less passing
  them on to parent processes.  This is not perfect: the traceback is
  likely lost as the exception is serialized.  Still, this now correctly
  leads to INTERRUPTED exit status when appropriate;
  • Loading branch information
jaltmayerpizzorno committed Sep 3, 2024
1 parent eca5a8e commit b810517
Show file tree
Hide file tree
Showing 3 changed files with 80 additions and 11 deletions.
2 changes: 1 addition & 1 deletion src/pytest_cleanslate/__version__.py
Original file line number Diff line number Diff line change
@@ -1 +1 @@
__version__ = "1.0.5"
__version__ = "1.0.6"
53 changes: 46 additions & 7 deletions src/pytest_cleanslate/plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ def runtest(self):

def collect_and_run(self):
# adapted from pytest-forked
import marshal
import pickle
import _pytest
import pytest_forked as ptf # FIXME pytest-forked is unmaintained
import py # FIXME py is maintenance only
Expand All @@ -52,7 +52,16 @@ def collect_items(collector):
else:
yield it

self.session.items = list(collect_items(module))
try:
self.session.items = list(collect_items(module))
except BaseException:
excinfo = pytest.ExceptionInfo.from_current()
return pickle.dumps([pytest.CollectReport(
nodeid=self.nodeid,
outcome='failed',
result=None,
longrepr=self._repr_failure_py(excinfo, "short"))
])

pm = self.config.pluginmanager
caller = pm.subset_hook_caller('pytest_collection_modifyitems', remove_plugins=[self.parent.plugin])
Expand All @@ -70,8 +79,10 @@ def pytest_runtest_logreport(self, report):
self.ihook.pytest_runtestloop(session=self.session)
except (pytest.Session.Interrupted, pytest.Session.Failed):
pass
except BaseException as e:
return pickle.dumps(e)

return marshal.dumps([self.config.hook.pytest_report_to_serializable(config=self.config, report=r) for r in reports])
return pickle.dumps(reports)

with IgnoreOsCloseErrors():
ff = py.process.ForkedFunc(runforked)
Expand All @@ -80,7 +91,11 @@ def pytest_runtest_logreport(self, report):
if result.retval is None:
return [ptf.report_process_crash(self, result)]

return [self.config.hook.pytest_report_from_serializable(config=self.config, data=r) for r in marshal.loads(result.retval)]
retval = pickle.loads(result.retval)
if isinstance(retval, BaseException):
raise retval

return retval


class CleanSlateCollector(pytest.File, pytest.Collector):
Expand All @@ -92,6 +107,32 @@ def collect(self):
yield CleanSlateItem.from_parent(parent=self, name=self.name)


def run_item_forked(item):
import _pytest.runner
import pytest_forked as ptf # FIXME pytest-forked is unmaintained
import py # FIXME py is maintenance only
import pickle

def runforked():
try:
return pickle.dumps(_pytest.runner.runtestprotocol(item, log=False))
except BaseException as e:
return pickle.dumps(e)

ff = py.process.ForkedFunc(runforked)
result = ff.waitfinish()

if result.retval is None:
return [ptf.report_process_crash(item, result)]

retval = pickle.loads(result.retval)

if isinstance(retval, BaseException):
raise retval

return retval


class CleanSlatePlugin:
"""Pytest plugin to isolate test collection, so that if a test's collection pollutes the in-memory
state, it doesn't affect the execution of other tests."""
Expand All @@ -104,15 +145,13 @@ def pytest_pycollect_makemodule(self, module_path: Path, parent):

@pytest.hookimpl(tryfirst=True)
def pytest_runtest_protocol(self, item: pytest.Item, nextitem: pytest.Item):
import pytest_forked as ptf # FIXME pytest-forked is unmaintained

ihook = item.ihook
ihook.pytest_runtest_logstart(nodeid=item.nodeid, location=item.location)
if isinstance(item, CleanSlateItem):
reports = item.collect_and_run()
else:
# note any side effects, such as setting session.shouldstop, are lost...
reports = ptf.forked_run_report(item)
reports = run_item_forked(item)

for rep in reports:
ihook.pytest_runtest_logreport(report=rep)
Expand Down
36 changes: 33 additions & 3 deletions tests/test_cleanslate.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ def seq2p(tests_dir, seq):
'assert': 'assert False',
'exception': 'raise RuntimeError("test")',
'kill': 'os.kill(os.getpid(), 9)',
'exit': 'pytest.exit("goodbye")',
# 'exit': 'pytest.exit("goodbye")', # FIXME add support
'exit': 'sys.exit(0)',
'interrupt': 'raise KeyboardInterrupt()'
}

Expand Down Expand Up @@ -107,7 +108,7 @@ def test_check_suite_fails(tests_dir, pollute_in_collect, fail_collect, fail_kin
fail_collect=fail_collect, fail_kind=fail_kind)

p = subprocess.run([sys.executable, '-m', 'pytest', tests_dir], check=False)
if fail_collect or fail_kind in ('exit', 'interrupt'):
if fail_collect or fail_kind == 'interrupt':
assert p.returncode == pytest.ExitCode.INTERRUPTED
else:
assert p.returncode == pytest.ExitCode.TESTS_FAILED
Expand Down Expand Up @@ -154,9 +155,38 @@ def test_foo():
{'test_foo()' if fail_collect else ''}
""")

p = subprocess.run([sys.executable, '-m', 'pytest', '--cleanslate', tests_dir], check=False)
p = subprocess.run([sys.executable, '-m', 'pytest', '--cleanslate', tests_dir], check=False,
capture_output=True)
print(failing.read_text())
assert p.returncode == (pytest.ExitCode.INTERRUPTED if (fail_kind == 'interrupt' and not fail_collect)
else pytest.ExitCode.TESTS_FAILED)

# pytest-forked error message that shouldn't be used unless the process was truly killed
assert 'CRASHED with signal 0' not in str(p.stdout, 'utf-8')


def test_collect_failure(tests_dir):
# _unconditionally_ failing test
failing = seq2p(tests_dir, 1)
failing.write_text(f"""\
import sys
import os
import pytest
def test_foo():
failure()
assert False
""")

p = subprocess.run([sys.executable, '-m', 'pytest', '--cleanslate', tests_dir], check=False,
capture_output=True)
# FIXME collection errors should show as interrupted
assert p.returncode == pytest.ExitCode.TESTS_FAILED

# pytest-forked error message that shouldn't be used unless the process was truly killed
assert 'CRASHED with signal 0' not in str(p.stdout, 'utf-8')


def test_isolate_module_yields_collector(tests_dir):
# A pytest.Collector.collect()'s return value may include not only pytest.Item,
Expand Down

0 comments on commit b810517

Please sign in to comment.