diff options
| -rw-r--r-- | cloudinit/tests/test_conftest.py | 14 | ||||
| -rw-r--r-- | conftest.py | 93 | ||||
| -rw-r--r-- | tests/unittests/test_datasource/test_opennebula.py | 3 | ||||
| -rw-r--r-- | tests/unittests/test_render_cloudcfg.py | 2 | ||||
| -rw-r--r-- | tox.ini | 7 |
5 files changed, 76 insertions, 43 deletions
diff --git a/cloudinit/tests/test_conftest.py b/cloudinit/tests/test_conftest.py index a6537248..6f1263a5 100644 --- a/cloudinit/tests/test_conftest.py +++ b/cloudinit/tests/test_conftest.py @@ -17,12 +17,11 @@ class TestDisableSubpUsage: # pylint: disable=no-value-for-parameter subp.subp() - @pytest.mark.parametrize('disable_subp_usage', [False], indirect=True) + @pytest.mark.allow_all_subp def test_subp_usage_can_be_reenabled(self): subp.subp(['whoami']) - @pytest.mark.parametrize( - 'disable_subp_usage', [['whoami'], 'whoami'], indirect=True) + @pytest.mark.allow_subp_for("whoami") def test_subp_usage_can_be_conditionally_reenabled(self): # The two parameters test each potential invocation with a single # argument @@ -31,8 +30,7 @@ class TestDisableSubpUsage: assert "allowed: whoami" in str(excinfo.value) subp.subp(['whoami']) - @pytest.mark.parametrize( - 'disable_subp_usage', [['whoami', 'bash']], indirect=True) + @pytest.mark.allow_subp_for("whoami", "bash") def test_subp_usage_can_be_conditionally_reenabled_for_multiple_cmds(self): with pytest.raises(AssertionError) as excinfo: subp.subp(["some", "args"]) @@ -40,6 +38,12 @@ class TestDisableSubpUsage: subp.subp(['bash', '-c', 'true']) subp.subp(['whoami']) + @pytest.mark.allow_all_subp + @pytest.mark.allow_subp_for("bash") + def test_both_marks_raise_an_error(self): + with pytest.raises(AssertionError, match="marked both"): + subp.subp(["bash"]) + class TestDisableSubpUsageInTestSubclass(CiTestCase): """Test that disable_subp_usage doesn't impact CiTestCase's subp logic.""" diff --git a/conftest.py b/conftest.py index 251bca59..faf13804 100644 --- a/conftest.py +++ b/conftest.py @@ -5,6 +5,18 @@ import pytest from cloudinit import subp +def _closest_marker_args_or(request, marker_name: str, default): + """Get the args for the closest ``marker_name`` or return ``default``""" + try: + marker = request.node.get_closest_marker(marker_name) + except AttributeError: + # Older versions of pytest don't have the new API + marker = request.node.get_marker(marker_name) + if marker is not None: + return marker.args + return default + + @pytest.yield_fixture(autouse=True) def disable_subp_usage(request): """ @@ -15,25 +27,23 @@ def disable_subp_usage(request): imports happen before the patching here (or the CiTestCase monkey-patching) happens, so are left untouched. - To allow a particular test method or class to use subp.subp you can set the - parameter passed to this fixture to False using pytest.mark.parametrize:: + To allow a particular test method or class to use subp.subp you can mark it + as such:: - @pytest.mark.parametrize("disable_subp_usage", [False], indirect=True) + @pytest.mark.allow_all_subp def test_whoami(self): subp.subp(["whoami"]) - To instead allow subp.subp usage for a specific command, you can set the - parameter passed to this fixture to that command: + To instead allow subp.subp usage for a specific command, you can use the + ``allow_subp_for`` mark:: - @pytest.mark.parametrize("disable_subp_usage", ["bash"], indirect=True) + @pytest.mark.allow_subp_for("bash") def test_bash(self): subp.subp(["bash"]) - To specify multiple commands, set the parameter to a list (note the - double-layered list: we specify a single parameter that is itself a list): + You can pass multiple commands as values; they will all be permitted:: - @pytest.mark.parametrize( - "disable_subp_usage", ["bash", "whoami"], indirect=True) + @pytest.mark.allow_subp_for("bash", "whoami") def test_several_things(self): subp.subp(["bash"]) subp.subp(["whoami"]) @@ -43,30 +53,43 @@ def disable_subp_usage(request): tests, CiTestCase's allowed_subp does take precedence (and we have TestDisableSubpUsageInTestSubclass to confirm that). """ - should_disable = getattr(request, "param", True) - if should_disable: - if not isinstance(should_disable, (list, str)): - def side_effect(args, *other_args, **kwargs): - raise AssertionError("Unexpectedly used subp.subp") - else: - # Look this up before our patch is in place, so we have access to - # the real implementation in side_effect - real_subp = subp.subp - - if isinstance(should_disable, str): - should_disable = [should_disable] - - def side_effect(args, *other_args, **kwargs): - cmd = args[0] - if cmd not in should_disable: - raise AssertionError( - "Unexpectedly used subp.subp to call {} (allowed:" - " {})".format(cmd, ",".join(should_disable)) - ) - return real_subp(args, *other_args, **kwargs) - - with mock.patch('cloudinit.subp.subp', autospec=True) as m_subp: - m_subp.side_effect = side_effect - yield + allow_subp_for = _closest_marker_args_or(request, "allow_subp_for", None) + # Because the mark doesn't take arguments, `allow_all_subp` will be set to + # [] if the marker is present, so explicit None checks are required + allow_all_subp = _closest_marker_args_or(request, "allow_all_subp", None) + + if allow_all_subp is not None and allow_subp_for is None: + # Only allow_all_subp specified, don't mock subp.subp + yield + return + + if allow_all_subp is None and allow_subp_for is None: + # No marks, default behaviour; disallow all subp.subp usage + def side_effect(args, *other_args, **kwargs): + raise AssertionError("Unexpectedly used subp.subp") + + elif allow_all_subp is not None and allow_subp_for is not None: + # Both marks, ambiguous request; raise an exception on all subp usage + def side_effect(args, *other_args, **kwargs): + raise AssertionError( + "Test marked both allow_all_subp and allow_subp_for: resolve" + " this either by modifying your test code, or by modifying" + " disable_subp_usage to handle precedence." + ) else: + # Look this up before our patch is in place, so we have access to + # the real implementation in side_effect + real_subp = subp.subp + + def side_effect(args, *other_args, **kwargs): + cmd = args[0] + if cmd not in allow_subp_for: + raise AssertionError( + "Unexpectedly used subp.subp to call {} (allowed:" + " {})".format(cmd, ",".join(allow_subp_for)) + ) + return real_subp(args, *other_args, **kwargs) + + with mock.patch("cloudinit.subp.subp", autospec=True) as m_subp: + m_subp.side_effect = side_effect yield diff --git a/tests/unittests/test_datasource/test_opennebula.py b/tests/unittests/test_datasource/test_opennebula.py index 80841182..9c6070a5 100644 --- a/tests/unittests/test_datasource/test_opennebula.py +++ b/tests/unittests/test_datasource/test_opennebula.py @@ -928,8 +928,7 @@ class TestOpenNebulaNetwork(unittest.TestCase): class TestParseShellConfig: - - @pytest.mark.parametrize('disable_subp_usage', ['bash'], indirect=True) + @pytest.mark.allow_subp_for("bash") def test_no_seconds(self): cfg = '\n'.join(["foo=bar", "SECONDS=2", "xx=foo"]) # we could test 'sleep 2', but that would make the test run slower. diff --git a/tests/unittests/test_render_cloudcfg.py b/tests/unittests/test_render_cloudcfg.py index 696915a3..495e2669 100644 --- a/tests/unittests/test_render_cloudcfg.py +++ b/tests/unittests/test_render_cloudcfg.py @@ -13,7 +13,7 @@ DISTRO_VARIANTS = ["amazon", "arch", "centos", "debian", "fedora", "freebsd", "netbsd", "openbsd", "rhel", "suse", "ubuntu", "unknown"] -@pytest.mark.parametrize('disable_subp_usage', [sys.executable], indirect=True) +@pytest.mark.allow_subp_for(sys.executable) class TestRenderCloudCfg: cmd = [sys.executable, os.path.realpath('tools/render-cloudcfg')] @@ -133,3 +133,10 @@ commands = {envpython} -m tests.cloud_tests {posargs} passenv = HOME TRAVIS deps = -r{toxinidir}/integration-requirements.txt + +[pytest] +# TODO: s/--strict/--strict-markers/ once xenial support is dropped +addopts = --strict +markers = + allow_subp_for: allow subp usage for the given commands (disable_subp_usage) + allow_all_subp: allow all subp usage (disable_subp_usage) |