test: Fix silent swallowing of unexpected subp error

conftest.py

@@ -79,6 +79,14 @@ def closest_marker_first_arg_or(request, marker_name: str, default):
         return result[0]
 
 
+class UnexpectedSubpError(BaseException):
+    """Error thrown when subp.subp is unexpectedly used.
+
+    We inherit from BaseException so it doesn't get silently swallowed
+    by other error handlers.
+    """
+
+
 @pytest.fixture(autouse=True)
 def disable_subp_usage(request, fixture_utils):
     """
@@ -142,12 +150,12 @@ def test_several_things(self):
     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")
+            raise UnexpectedSubpError("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(
+            raise UnexpectedSubpError(
                 "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."
@@ -161,7 +169,7 @@ def side_effect(args, *other_args, **kwargs):
         def side_effect(args, *other_args, **kwargs):
             cmd = args[0]
             if cmd not in allow_subp_for:
-                raise AssertionError(
+                raise UnexpectedSubpError(
                     "Unexpectedly used subp.subp to call {} (allowed:"
                     " {})".format(cmd, ",".join(allow_subp_for))
                 )

tests/unittests/analyze/test_boot.py

@@ -28,17 +28,12 @@ def test_subp_fails(self, m_subp):
 
 
 class TestSystemCtlReader:
-    @pytest.mark.parametrize(
-        "args",
-        [
-            pytest.param(["dummyProperty"], id="invalid_property"),
-            pytest.param(
-                ["dummyProperty", "dummyParameter"], id="invalid_parameter"
-            ),
-        ],
-    )
-    def test_systemctl_invalid(self, args):
-        reader = SystemctlReader(*args)
+    def test_systemctl_invalid(self, mocker):
+        mocker.patch(
+            "cloudinit.analyze.show.subp.subp",
+            return_value=("", "something_invalid"),
+        )
+        reader = SystemctlReader("dont", "care")
         with pytest.raises(RuntimeError):
             reader.parse_epoch_as_float()
 

tests/unittests/sources/azure/test_errors.py

@@ -121,15 +121,21 @@ def test_reportable_errors(
     assert error.as_encoded_report() == "|".join(data)
 
 
-def test_dhcp_lease():
+def test_dhcp_lease(mocker):
+    mocker.patch(
+        "cloudinit.sources.azure.identity.query_vm_id", return_value="foo"
+    )
     error = errors.ReportableErrorDhcpLease(duration=5.6, interface="foo")
 
     assert error.reason == "failure to obtain DHCP lease"
     assert error.supporting_data["duration"] == 5.6
     assert error.supporting_data["interface"] == "foo"
 
 
-def test_dhcp_interface_not_found():
+def test_dhcp_interface_not_found(mocker):
+    mocker.patch(
+        "cloudinit.sources.azure.identity.query_vm_id", return_value="foo"
+    )
     error = errors.ReportableErrorDhcpInterfaceNotFound(duration=5.6)
 
     assert error.reason == "failure to find DHCP interface"
@@ -180,7 +186,10 @@ def test_dhcp_interface_not_found():
         ),
     ],
 )
-def test_imds_url_error(exception, reason):
+def test_imds_url_error(exception, reason, mocker):
+    mocker.patch(
+        "cloudinit.sources.azure.identity.query_vm_id", return_value="foo"
+    )
     duration = 123.4
     fake_url = "fake://url"
 
@@ -195,7 +204,11 @@ def test_imds_url_error(exception, reason):
     assert error.supporting_data["url"] == fake_url
 
 
-def test_imds_metadata_parsing_exception():
+def test_imds_metadata_parsing_exception(mocker):
+    mocker.patch(
+        "cloudinit.sources.azure.identity.query_vm_id", return_value="foo"
+    )
+
     exception = ValueError("foobar")
 
     error = errors.ReportableErrorImdsMetadataParsingException(
@@ -206,7 +219,11 @@ def test_imds_metadata_parsing_exception():
     assert error.supporting_data["exception"] == repr(exception)
 
 
-def test_unhandled_exception():
+def test_unhandled_exception(mocker):
+    mocker.patch(
+        "cloudinit.sources.azure.identity.query_vm_id", return_value="foo"
+    )
+
     source_error = None
     try:
         raise ValueError("my value error")
@@ -227,7 +244,11 @@ def test_unhandled_exception():
     assert f"|{quoted_value}|" in error.as_encoded_report()
 
 
-def test_imds_invalid_metadata():
+def test_imds_invalid_metadata(mocker):
+    mocker.patch(
+        "cloudinit.sources.azure.identity.query_vm_id", return_value="foo"
+    )
+
     key = "compute"
     value = "Running"
     error = errors.ReportableErrorImdsInvalidMetadata(key=key, value=value)

tests/unittests/sources/azure/test_kvp.py

@@ -39,7 +39,10 @@ def telemetry_reporter(tmp_path):
 
 
 class TestReportFailureToHost:
-    def test_report_failure_to_host(self, caplog, telemetry_reporter):
+    def test_report_failure_to_host(self, caplog, telemetry_reporter, mocker):
+        mocker.patch(
+            "cloudinit.sources.azure.identity.query_vm_id", return_value="foo"
+        )
         error = errors.ReportableError(reason="test")
         assert kvp.report_failure_to_host(error) is True
         assert (
@@ -52,7 +55,10 @@ def test_report_failure_to_host(self, caplog, telemetry_reporter):
         }
         assert report in list(telemetry_reporter._iterate_kvps(0))
 
-    def test_report_skipped_without_telemetry(self, caplog):
+    def test_report_skipped_without_telemetry(self, caplog, mocker):
+        mocker.patch(
+            "cloudinit.sources.azure.identity.query_vm_id", return_value="foo"
+        )
         error = errors.ReportableError(reason="test")
 
         assert kvp.report_failure_to_host(error) is False
@@ -83,6 +89,9 @@ def test_report_success_to_host(
         }
         assert report in list(telemetry_reporter._iterate_kvps(0))
 
-    def test_report_skipped_without_telemetry(self, caplog):
+    def test_report_skipped_without_telemetry(self, caplog, mocker):
+        mocker.patch(
+            "cloudinit.sources.azure.identity.query_vm_id", return_value="foo"
+        )
         assert kvp.report_success_to_host() is False
         assert "KVP handler not enabled, skipping host report." in caplog.text

tests/unittests/test_conftest.py

@@ -1,14 +1,15 @@
 import pytest
 
 from cloudinit import subp
+from conftest import UnexpectedSubpError
 from tests.unittests.helpers import CiTestCase
 
 
 class TestDisableSubpUsage:
     """Test that the disable_subp_usage fixture behaves as expected."""
 
     def test_using_subp_raises_assertion_error(self):
-        with pytest.raises(AssertionError):
+        with pytest.raises(UnexpectedSubpError):
             subp.subp(["some", "args"])
 
     def test_typeerrors_on_incorrect_usage(self):
@@ -17,6 +18,13 @@ def test_typeerrors_on_incorrect_usage(self):
             #  pylint: disable=no-value-for-parameter
             subp.subp()
 
+    def test_subp_exception_escapes_exception_handling(self):
+        with pytest.raises(UnexpectedSubpError):
+            try:
+                subp.subp(["some", "args"])
+            except Exception:
+                pytest.fail("Unexpected exception raised")
+
     @pytest.mark.allow_all_subp
     def test_subp_usage_can_be_reenabled(self):
         subp.subp(["whoami"])
@@ -25,14 +33,14 @@ def test_subp_usage_can_be_reenabled(self):
     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:
+        with pytest.raises(UnexpectedSubpError) as excinfo:
             subp.subp(["some", "args"])
         assert "allowed: whoami" in str(excinfo.value)
         subp.subp(["whoami"])
 
     @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:
+        with pytest.raises(UnexpectedSubpError) as excinfo:
             subp.subp(["some", "args"])
         assert "allowed: whoami,bash" in str(excinfo.value)
         subp.subp(["bash", "-c", "true"])
@@ -41,7 +49,7 @@ def test_subp_usage_can_be_conditionally_reenabled_for_multiple_cmds(self):
     @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"):
+        with pytest.raises(UnexpectedSubpError, match="marked both"):
             subp.subp(["bash"])