conftest: implement partial disable_subp_usage (#371)

This allows tests to be configured to permit some commands to be run via
util.subp, while still rejecting any unexpected calls.  See the
documentation for further details.

2 files changed, 58 insertions, 4 deletions

diff --git a/cloudinit/conftest.py b/cloudinit/conftest.py
index af458c31..37cbbcda 100644
--- a/cloudinit/conftest.py
+++ b/cloudinit/conftest.py
@@ -2,6 +2,8 @@ from unittest import mock
 
 import pytest
 
+from cloudinit import util
+
 
 @pytest.yield_fixture(autouse=True)
 def disable_subp_usage(request):
@@ -20,18 +22,51 @@ def disable_subp_usage(request):
         def test_whoami(self):
             util.subp(["whoami"])
 
+    To instead allow util.subp usage for a specific command, you can set the
+    parameter passed to this fixture to that command:
+
+        @pytest.mark.parametrize("disable_subp_usage", ["bash"], indirect=True)
+        def test_bash(self):
+            util.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):
+
+        @pytest.mark.parametrize(
+            "disable_subp_usage", ["bash", "whoami"], indirect=True)
+        def test_several_things(self):
+            util.subp(["bash"])
+            util.subp(["whoami"])
+
     This fixture (roughly) mirrors the functionality of
     CiTestCase.allowed_subp.  N.B. While autouse fixtures do affect non-pytest
     tests, CiTestCase's allowed_subp does take precedence (and we have
     TestDisableSubpUsageInTestSubclass to confirm that).
-
-    TODO:
-        * Enable select subp usage (i.e. allowed_subp=[...])
     """
     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 util.subp")
+        else:
+            # Look this up before our patch is in place, so we have access to
+            # the real implementation in side_effect
+            subp = util.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 util.subp to call {} (allowed:"
+                        " {})".format(cmd, ",".join(should_disable))
+                    )
+                return subp(args, *other_args, **kwargs)
+
         with mock.patch('cloudinit.util.subp', autospec=True) as m_subp:
-            m_subp.side_effect = AssertionError("Unexpectedly used util.subp")
+            m_subp.side_effect = side_effect
             yield
     else:
         yield
diff --git a/cloudinit/tests/test_conftest.py b/cloudinit/tests/test_conftest.py
index b39637d8..773ef8fe 100644
--- a/cloudinit/tests/test_conftest.py
+++ b/cloudinit/tests/test_conftest.py
@@ -21,6 +21,25 @@ class TestDisableSubpUsage:
     def test_subp_usage_can_be_reenabled(self):
         util.subp(['whoami'])
 
+    @pytest.mark.parametrize(
+        'disable_subp_usage', [['whoami'], 'whoami'], indirect=True)
+    def test_subp_usage_can_be_conditionally_reenabled(self):
+        # The two parameters test each potential invocation with a single
+        # argument
+        with pytest.raises(AssertionError) as excinfo:
+            util.subp(["some", "args"])
+        assert "allowed: whoami" in str(excinfo.value)
+        util.subp(['whoami'])
+
+    @pytest.mark.parametrize(
+        'disable_subp_usage', [['whoami', 'bash']], indirect=True)
+    def test_subp_usage_can_be_conditionally_reenabled_for_multiple_cmds(self):
+        with pytest.raises(AssertionError) as excinfo:
+            util.subp(["some", "args"])
+        assert "allowed: whoami,bash" in str(excinfo.value)
+        util.subp(['bash', '-c', 'true'])
+        util.subp(['whoami'])
+
 
 class TestDisableSubpUsageInTestSubclass(CiTestCase):
     """Test that disable_subp_usage doesn't impact CiTestCase's subp logic."""