Skip to content

Remove dangerous sys.path manipulations in test_pluginmanager #3873

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Aug 26, 2018
Merged

Remove dangerous sys.path manipulations in test_pluginmanager #3873

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f872fcb
Remove dangerous sys.path manipulations in test_pluginmanager
nicoddemus Aug 25, 2018
b0541e9
Correctly restore sys.path in test and remove dead code in test_pytester
nicoddemus Aug 25, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 9 additions & 5 deletions testing/test_monkeypatch.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,11 +228,15 @@ def test_syspath_prepend(mp):


def test_syspath_prepend_double_undo(mp):
mp.syspath_prepend("hello world")
mp.undo()
sys.path.append("more hello world")
mp.undo()
assert sys.path[-1] == "more hello world"
old_syspath = sys.path[:]
try:
mp.syspath_prepend("hello world")
mp.undo()
sys.path.append("more hello world")
mp.undo()
assert sys.path[-1] == "more hello world"
finally:
sys.path[:] = old_syspath


def test_chdir_with_path_local(mp, tmpdir):
Expand Down
4 changes: 1 addition & 3 deletions testing/test_pluginmanager.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ def pytest_myhook(xyz):
)
conf = testdir.makeconftest(
"""
import sys ; sys.path.insert(0, '.')
import newhooks
def pytest_addhooks(pluginmanager):
pluginmanager.addhooks(newhooks)
Expand Down Expand Up @@ -263,8 +262,7 @@ def test_consider_module_import_module(self, testdir):
mod.pytest_plugins = "pytest_a"
aplugin = testdir.makepyfile(pytest_a="#")
reprec = testdir.make_hook_recorder(pytestpm)
# syspath.prepend(aplugin.dirpath())
sys.path.insert(0, str(aplugin.dirpath()))
testdir.syspathinsert(aplugin.dirpath())
pytestpm.consider_module(mod)
call = reprec.getcall(pytestpm.hook.pytest_plugin_registered.name)
assert call.plugin.__name__ == "pytest_a"
Expand Down
51 changes: 0 additions & 51 deletions testing/test_pytester.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,57 +215,6 @@ def test_foo():
assert imported.data == 42


def test_inline_run_clean_sys_paths(testdir):
Copy link
Member Author

Choose a reason for hiding this comment

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

The tests inside this test (!) have been refactored into a test class above this code, but this function (with 3 local functions which are never called) was left by mistake apparently.

Copy link
Member

Choose a reason for hiding this comment

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

I opened #3875 to try and surface these things better :)

def test_sys_path_change_cleanup(self, testdir):
test_path1 = testdir.tmpdir.join("boink1").strpath
test_path2 = testdir.tmpdir.join("boink2").strpath
test_path3 = testdir.tmpdir.join("boink3").strpath
sys.path.append(test_path1)
sys.meta_path.append(test_path1)
original_path = list(sys.path)
original_meta_path = list(sys.meta_path)
test_mod = testdir.makepyfile(
"""
import sys
sys.path.append({:test_path2})
sys.meta_path.append({:test_path2})
def test_foo():
sys.path.append({:test_path3})
sys.meta_path.append({:test_path3})""".format(
locals()
)
)
testdir.inline_run(str(test_mod))
assert sys.path == original_path
assert sys.meta_path == original_meta_path

def spy_factory(self):
class SysPathsSnapshotSpy(object):
instances = []

def __init__(self):
SysPathsSnapshotSpy.instances.append(self)
self._spy_restore_count = 0
self.__snapshot = SysPathsSnapshot()

def restore(self):
self._spy_restore_count += 1
return self.__snapshot.restore()

return SysPathsSnapshotSpy

def test_inline_run_taking_and_restoring_a_sys_paths_snapshot(
self, testdir, monkeypatch
):
spy_factory = self.spy_factory()
monkeypatch.setattr(pytester, "SysPathsSnapshot", spy_factory)
test_mod = testdir.makepyfile("def test_foo(): pass")
testdir.inline_run(str(test_mod))
assert len(spy_factory.instances) == 1
spy = spy_factory.instances[0]
assert spy._spy_restore_count == 1


def test_assert_outcomes_after_pytest_error(testdir):
testdir.makepyfile("def test_foo(): assert True")

Expand Down