fix: Prevent warnings/errors when running `meltano config <plugin> te…

…st` (meltano#6676)

* Wait for subprocess to exit after terminating

* Update tests

* Correct `validate` method return value typing

* Rename test file for conformity with subject code

* Satisfy flake8

* Update other tests

src/meltano/core/plugin_invoker.py

@@ -433,7 +433,7 @@ async def _invoke(
                     self.plugin, self.plugin.executable
                 ) from err
 
-    async def invoke_async(self, *args, **kwargs):
+    async def invoke_async(self, *args, **kwargs) -> asyncio.subprocess.Process:
         """Invoke a command.
 
         Args:

src/meltano/core/plugin_test_service.py

@@ -18,12 +18,24 @@ class PluginTestServiceFactory:
     """Factory class to resolve a plugin test service."""
 
     def __init__(self, plugin_invoker: PluginInvoker):
-        """Construct a PluginTestServiceFactory instance."""
+        """Construct a PluginTestServiceFactory instance.
+
+        Args:
+            plugin_invoker: The invocation instance of the plugin to test.
+        """
         self.plugin_invoker = plugin_invoker
 
     def get_test_service(self):
-        """Resolve a test service instance for a plugin type."""
+        """Resolve a test service instance for a plugin type.
+
+        Returns:
+            The test service instance.
+
+        Raises:
+            PluginNotSupportedError: If the plugin type is not supported for testing.
+        """
         test_services = {PluginType.EXTRACTORS: ExtractorTestService}
+
         try:
             return test_services[self.plugin_invoker.plugin.type](self.plugin_invoker)
         except KeyError as err:
@@ -34,19 +46,27 @@ class PluginTestService(ABC):
     """Abstract base class for plugin test operations."""
 
     def __init__(self, plugin_invoker: PluginInvoker):
-        """Construct a PluginTestService instance."""
+        """Construct a PluginTestService instance.
+
+        Args:
+            plugin_invoker: The invocation instance of the plugin to test
+        """
         self.plugin_invoker = plugin_invoker
 
     @abstractmethod
-    def validate(self) -> bool | str:
+    async def validate(self) -> tuple[bool, str]:
         """Abstract method to validate plugin configuration."""
 
 
 class ExtractorTestService(PluginTestService):
     """Handle extractor test operations."""
 
-    async def validate(self) -> bool | str:
-        """Validate extractor configuration."""
+    async def validate(self) -> tuple[bool, str]:
+        """Validate extractor configuration.
+
+        Returns:
+            The validation result and supporting context message (if applicable).
+        """
         process = None
 
         try:
@@ -71,8 +91,13 @@ async def validate(self) -> bool | str:
 
             if message_type == "RECORD":
                 process.terminate()
-                return True, None
+                break
 
-        await process.wait()
+        returncode = await process.wait()
 
-        return False, last_line if process.returncode else "No RECORD message received"
+        # considered valid if subprocess is terminated (exit status < 0) on RECORD message received
+        # see https://docs.python.org/3/library/subprocess.html#subprocess.CompletedProcess.returncode
+        return (
+            returncode < 0,
+            last_line if returncode else "No RECORD message received",
+        )

tests/meltano/api/controllers/test_orchestration.py

@@ -133,8 +133,8 @@ def test_test_plugin_configuration_success(
         mock_invoke = mock.Mock()
         mock_invoke.sterr.at_eof.side_effect = True
         mock_invoke.stdout.at_eof.side_effect = (False, True)
-        mock_invoke.wait = AsyncMock(return_value=0)
-        mock_invoke.returncode = 0
+        mock_invoke.wait = AsyncMock(return_value=-1)
+        mock_invoke.returncode = -1
         payload = json.dumps({"type": "RECORD"}).encode()
         mock_invoke.stdout.readline = AsyncMock(return_value=b"%b" % payload)
 

tests/meltano/cli/test_config.py

@@ -78,8 +78,8 @@ def test_config_test(
         mock_invoke = mock.Mock()
         mock_invoke.sterr.at_eof.side_effect = True
         mock_invoke.stdout.at_eof.side_effect = (False, True)
-        mock_invoke.wait = AsyncMock(return_value=0)
-        mock_invoke.returncode = 0
+        mock_invoke.wait = AsyncMock(return_value=-1)
+        mock_invoke.returncode = -1
         payload = json.dumps({"type": "RECORD"}).encode()
         mock_invoke.stdout.readline = AsyncMock(return_value=b"%b" % payload)
 

tests/meltano/core/test_plugins_test_service.py → tests/meltano/core/test_plugin_test_service.py

@@ -45,8 +45,8 @@ class TestExtractorTestService:
     def setup(self, mock_invoker):
         self.mock_invoke = Mock()
         self.mock_invoke.name = "utility-mock"
-        self.mock_invoke.wait = AsyncMock(return_value=0)
-        self.mock_invoke.returncode = 0
+        self.mock_invoke.wait = AsyncMock(return_value=-1)
+        self.mock_invoke.returncode = -1
         self.mock_invoker = mock_invoker
         self.mock_invoker.invoke_async = AsyncMock(return_value=self.mock_invoke)
 
@@ -61,7 +61,7 @@ async def test_validate_success(self):
         is_valid, detail = await ExtractorTestService(self.mock_invoker).validate()
 
         assert is_valid
-        assert detail is None
+        assert detail == MOCK_RECORD_MESSAGE
 
     @pytest.mark.asyncio
     async def test_validate_success_ignore_non_json(self):
@@ -74,7 +74,7 @@ async def test_validate_success_ignore_non_json(self):
         is_valid, detail = await ExtractorTestService(self.mock_invoker).validate()
 
         assert is_valid
-        assert detail is None
+        assert detail == MOCK_RECORD_MESSAGE
 
     @pytest.mark.asyncio
     async def test_validate_success_ignore_non_record_msg(self):
@@ -90,7 +90,7 @@ async def test_validate_success_ignore_non_record_msg(self):
         is_valid, detail = await ExtractorTestService(self.mock_invoker).validate()
 
         assert is_valid
-        assert detail is None
+        assert detail == MOCK_RECORD_MESSAGE
 
     @pytest.mark.asyncio
     async def test_validate_success_stop_after_record_msg(self):
@@ -107,7 +107,7 @@ async def test_validate_success_stop_after_record_msg(self):
         is_valid, detail = await ExtractorTestService(self.mock_invoker).validate()
 
         assert is_valid
-        assert detail is None
+        assert detail == MOCK_RECORD_MESSAGE
 
         assert self.mock_invoke.stdout.readline.call_count == 2
 
@@ -119,6 +119,9 @@ async def test_validate_failure_no_record_msg(self):
             return_value=(b"%b" % MOCK_STATE_MESSAGE.encode())
         )
 
+        self.mock_invoke.wait = AsyncMock(return_value=0)
+        self.mock_invoke.returncode = 0
+
         is_valid, detail = await ExtractorTestService(self.mock_invoker).validate()
 
         assert not is_valid