clean up start_server and test coverage.

Author: Davidyz

src/vectorcode/common.py

@@ -94,20 +94,27 @@ async def start_server(configs: Config):
         exe,
         "run",
         "--host",
-        "localhost",
+        "127.0.0.1",
         "--port",
         str(configs.port),
         "--path",
         db_path,
-        # "--log-path",
-        # os.path.join(str(configs.db_log_path), "chroma.log"),
-        stdout=subprocess.DEVNULL,
-        stderr=sys.stderr,
+        stdout=subprocess.PIPE,
+        stderr=subprocess.PIPE,
         env=env,
     )
 
-    await wait_for_server(configs.host, configs.port)
-    return process
+    try:
+        await wait_for_server("127.0.0.1", configs.port)
+        return process
+    except Exception:  # pragma: nocover
+        process.terminate()
+        logger.error("Failed to start ChromaDB!")
+        if process.stdout is not None:
+            logger.error(f"stdout: {(await process.stdout.read()).decode()}")
+        if process.stderr is not None:
+            logger.error(f"stderr: {(await process.stderr.read()).decode()}")
+        raise
 
 
 __CLIENT_CACHE: dict[tuple[str, int], AsyncClientAPI] = {}

tests/test_common.py

@@ -1,7 +1,6 @@
 import os
 import socket
 import subprocess
-import sys
 import tempfile
 from unittest.mock import MagicMock, patch
 
@@ -430,23 +429,94 @@ def _new_isdir(path):
             expected_args = [
                 "run",
                 "--host",
-                "localhost",
+                "127.0.0.1",
                 "--port",
                 str(12345),  # Check the mocked port
                 "--path",
                 temp_dir,
-                # "--log-path",
-                # os.path.join(str(config.db_log_path), "chroma.log"),
             ]
-            assert os.path.isfile(args[0]) and "chroma" in args[0], (
+            assert "chroma" in args[0], (
                 f"{args[0]} should be the path to the `chroma` executable."
             )
             assert tuple(args[1:]) == tuple(expected_args)
-            assert kwargs["stdout"] == subprocess.DEVNULL
-            assert kwargs["stderr"] == sys.stderr
+            assert kwargs["stdout"] == subprocess.PIPE
+            assert kwargs["stderr"] == subprocess.PIPE
             assert "ANONYMIZED_TELEMETRY" in kwargs["env"]
 
-            MockWaitForServer.assert_called_once_with("localhost", 12345)
+            MockWaitForServer.assert_called_once_with("127.0.0.1", 12345)
+
+            assert process == mock_process
+            mock_makedirs.assert_called_once_with(config.db_log_path)
+
+
+@pytest.mark.asyncio
+async def test_start_server_windows():
+    with tempfile.TemporaryDirectory() as temp_dir:
+
+        def _new_isdir(path):
+            if str(temp_dir) in str(path):
+                return True
+            return False
+
+        def _new_isfile(path):
+            if "/bin/" in path:
+                return False
+            return True
+
+        # Mock subprocess.Popen
+        with (
+            patch("asyncio.create_subprocess_exec") as MockCreateProcess,
+            patch("asyncio.sleep"),
+            patch("socket.socket") as MockSocket,
+            patch("vectorcode.common.wait_for_server") as MockWaitForServer,
+            patch("os.path.isdir") as mock_isdir,
+            patch("os.path.isfile") as mock_isfile,
+            patch("os.makedirs") as mock_makedirs,
+        ):
+            mock_isdir.side_effect = _new_isdir
+            mock_isfile.side_effect = _new_isfile
+            # Mock socket to return a specific port
+            mock_socket = MagicMock()
+            mock_socket.getsockname.return_value = ("localhost", 12345)  # Mock port
+            MockSocket.return_value.__enter__.return_value = mock_socket
+
+            # Mock the process object
+            mock_process = MagicMock()
+            mock_process.returncode = 0  # Simulate successful execution
+            MockCreateProcess.return_value = mock_process
+
+            # Create a config object
+            config = Config(
+                host="localhost",
+                port=8000,
+                db_path=temp_dir,
+                project_root=temp_dir,
+            )
+
+            # Call start_server
+            process = await start_server(config)
+
+            # Assert that asyncio.create_subprocess_exec was called with the correct arguments
+            MockCreateProcess.assert_called_once()
+            args, kwargs = MockCreateProcess.call_args
+            expected_args = [
+                "run",
+                "--host",
+                "127.0.0.1",
+                "--port",
+                str(12345),  # Check the mocked port
+                "--path",
+                temp_dir,
+            ]
+            assert "chroma" in args[0], (
+                f"{args[0]} should be the path to the `chroma` executable."
+            )
+            assert tuple(args[1:]) == tuple(expected_args)
+            assert kwargs["stdout"] == subprocess.PIPE
+            assert kwargs["stderr"] == subprocess.PIPE
+            assert "ANONYMIZED_TELEMETRY" in kwargs["env"]
+
+            MockWaitForServer.assert_called_once_with("127.0.0.1", 12345)
 
             assert process == mock_process
             mock_makedirs.assert_called_once_with(config.db_log_path)