From ca4d0f5143b432492ab2bb2b35295484cb722f6a Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Fri, 4 Apr 2025 14:21:41 +0000 Subject: [PATCH] Address PR feedback: Improve code structure and test quality Co-Authored-By: Joe Moura --- src/crewai/cli/install_crew.py | 20 +++++++- tests/cli/install_crew_test.py | 83 +++++++++++++++++++++++++++------- 2 files changed, 85 insertions(+), 18 deletions(-) diff --git a/src/crewai/cli/install_crew.py b/src/crewai/cli/install_crew.py index fcfd51971..055b42138 100644 --- a/src/crewai/cli/install_crew.py +++ b/src/crewai/cli/install_crew.py @@ -1,16 +1,32 @@ import subprocess from pathlib import Path +from typing import List import click -def install_crew(proxy_options: list[str]) -> None: +def _check_pyproject_exists() -> bool: """ - Install the crew by running the UV command to lock and install. + Check if pyproject.toml exists in the current directory. + + Returns: + bool: True if pyproject.toml exists, False otherwise. """ if not Path("pyproject.toml").exists(): click.echo("Error: No pyproject.toml found in current directory.", err=True) click.echo("This command must be run from the root of a crew project.", err=True) + return False + return True + + +def install_crew(proxy_options: List[str]) -> None: + """ + Install the crew by running the UV command to lock and install. + + Args: + proxy_options: List of proxy options to pass to UV. + """ + if not _check_pyproject_exists(): return try: diff --git a/tests/cli/install_crew_test.py b/tests/cli/install_crew_test.py index f3a7a1f0f..c0459a083 100644 --- a/tests/cli/install_crew_test.py +++ b/tests/cli/install_crew_test.py @@ -1,67 +1,118 @@ import subprocess from pathlib import Path +from typing import List from unittest import mock import pytest from click.testing import CliRunner -from crewai.cli.install_crew import install_crew +from crewai.cli.install_crew import install_crew, _check_pyproject_exists + +TEST_CONSTANTS = { + "ERROR_NO_PYPROJECT": "Error: No pyproject.toml found in current directory.", + "ERROR_MUST_RUN_FROM_ROOT": "This command must be run from the root of a crew project.", + "ERROR_RUNNING_CREW": "An error occurred while running the crew", + "ERROR_UNEXPECTED": "An unexpected error occurred: Test exception", +} @pytest.fixture -def mock_subprocess_run(): +def mock_subprocess_run() -> mock.MagicMock: + """Mock subprocess.run for testing.""" with mock.patch("subprocess.run") as mock_run: yield mock_run @pytest.fixture -def mock_path_exists(): +def mock_path_exists() -> mock.MagicMock: + """Mock Path.exists for testing.""" with mock.patch("pathlib.Path.exists") as mock_exists: yield mock_exists -def test_install_crew_pyproject_exists(mock_subprocess_run, mock_path_exists): +@pytest.mark.parametrize( + "proxy_options,expected_command", + [ + ([], ["uv", "sync"]), + (["--proxy", "http://proxy.com"], ["uv", "sync", "--proxy", "http://proxy.com"]), + ], +) +def test_install_crew_pyproject_exists( + mock_subprocess_run: mock.MagicMock, + mock_path_exists: mock.MagicMock, + proxy_options: List[str], + expected_command: List[str], +) -> None: + """Test install_crew when pyproject.toml exists.""" mock_path_exists.return_value = True - proxy_options = [] install_crew(proxy_options) mock_subprocess_run.assert_called_once_with( - ["uv", "sync"] + proxy_options, check=True, capture_output=False, text=True + expected_command, check=True, capture_output=False, text=True ) -def test_install_crew_pyproject_not_exists(mock_subprocess_run, mock_path_exists, capsys): +def test_install_crew_pyproject_not_exists( + mock_subprocess_run: mock.MagicMock, + mock_path_exists: mock.MagicMock, + capsys: pytest.CaptureFixture, +) -> None: + """Test install_crew when pyproject.toml does not exist.""" mock_path_exists.return_value = False - proxy_options = [] + proxy_options: List[str] = [] install_crew(proxy_options) mock_subprocess_run.assert_not_called() captured = capsys.readouterr() - assert "Error: No pyproject.toml found in current directory." in captured.err - assert "This command must be run from the root of a crew project." in captured.err + assert TEST_CONSTANTS["ERROR_NO_PYPROJECT"] in captured.err + assert TEST_CONSTANTS["ERROR_MUST_RUN_FROM_ROOT"] in captured.err -def test_install_crew_subprocess_error(mock_subprocess_run, mock_path_exists, capsys): +def test_install_crew_subprocess_error( + mock_subprocess_run: mock.MagicMock, + mock_path_exists: mock.MagicMock, + capsys: pytest.CaptureFixture, +) -> None: + """Test install_crew when subprocess raises CalledProcessError.""" mock_path_exists.return_value = True mock_subprocess_run.side_effect = subprocess.CalledProcessError( cmd=["uv", "sync"], returncode=1 ) - proxy_options = [] + proxy_options: List[str] = [] install_crew(proxy_options) captured = capsys.readouterr() - assert "An error occurred while running the crew" in captured.err + assert TEST_CONSTANTS["ERROR_RUNNING_CREW"] in captured.err -def test_install_crew_general_exception(mock_subprocess_run, mock_path_exists, capsys): +def test_install_crew_general_exception( + mock_subprocess_run: mock.MagicMock, + mock_path_exists: mock.MagicMock, + capsys: pytest.CaptureFixture, +) -> None: + """Test install_crew when a general exception occurs.""" mock_path_exists.return_value = True mock_subprocess_run.side_effect = Exception("Test exception") - proxy_options = [] + proxy_options: List[str] = [] install_crew(proxy_options) captured = capsys.readouterr() - assert "An unexpected error occurred: Test exception" in captured.err + assert TEST_CONSTANTS["ERROR_UNEXPECTED"] in captured.err + + +def test_check_pyproject_exists( + mock_path_exists: mock.MagicMock, capsys: pytest.CaptureFixture +) -> None: + """Test _check_pyproject_exists function.""" + mock_path_exists.return_value = True + assert _check_pyproject_exists() is True + + mock_path_exists.return_value = False + assert _check_pyproject_exists() is False + captured = capsys.readouterr() + assert TEST_CONSTANTS["ERROR_NO_PYPROJECT"] in captured.err + assert TEST_CONSTANTS["ERROR_MUST_RUN_FROM_ROOT"] in captured.err