diff --git a/agentstack/packaging.py b/agentstack/packaging.py index 73d5fb98..7d944b49 100644 --- a/agentstack/packaging.py +++ b/agentstack/packaging.py @@ -1,9 +1,9 @@ -import os, sys -from typing import Optional, Callable +import os +import sys +from typing import Callable from pathlib import Path import re import subprocess -import select import site from packaging.requirements import Requirement from agentstack import conf, log @@ -20,18 +20,19 @@ # the packages are installed into the correct virtual environment. # In testing, when this was not set, packages could end up in the pyenv's # site-packages directory; it's possible an environment variable can control this. +def _get_executeable_paths(): + """Get environment paths based on platform.""" + python_path = '.venv/Scripts/python.exe' if sys.platform == 'win32' else '.venv/bin/python' + venv_path = conf.PATH / VENV_DIR_NAME.absolute() + return venv_path, python_path -_python_executable = ".venv/bin/python" -def set_python_executable(path: str): - global _python_executable - - _python_executable = path +_VENV_PATH, _PYTHON_EXECUTABLE = _get_executeable_paths() def install(package: str): """Install a package with `uv` and add it to pyproject.toml.""" - global _python_executable + global _PYTHON_EXECUTABLE from agentstack.cli.spinner import Spinner def on_progress(line: str): @@ -40,10 +41,10 @@ def on_progress(line: str): def on_error(line: str): log.error(f"uv: [error]\n {line.strip()}") - + with Spinner(f"Installing {package}") as spinner: _wrap_command_with_callbacks( - [get_uv_bin(), 'add', '--python', _python_executable, package], + [get_uv_bin(), 'add', '--python', _PYTHON_EXECUTABLE, package], on_progress=on_progress, on_error=on_error, ) @@ -51,7 +52,7 @@ def on_error(line: str): def install_project(): """Install all dependencies for the user's project.""" - global _python_executable + global _PYTHON_EXECUTABLE from agentstack.cli.spinner import Spinner def on_progress(line: str): @@ -59,24 +60,30 @@ def on_progress(line: str): spinner.clear_and_log(line.strip(), 'info') def on_error(line: str): - log.error(f"uv: [error]\n {line.strip()}") + log.error(f"UV installation error:\n{line.strip()}") try: - with Spinner(f"Installing project dependencies.") as spinner: + with Spinner("Installing project dependencies...") as spinner: result = _wrap_command_with_callbacks( - [get_uv_bin(), 'pip', 'install', '--python', _python_executable, '.'], + [get_uv_bin(), 'pip', 'install', '--python', _PYTHON_EXECUTABLE, '.'], on_progress=on_progress, on_error=on_error, ) if result is False: - spinner.clear_and_log("Retrying uv installation with --no-cache flag...", 'info') - _wrap_command_with_callbacks( - [get_uv_bin(), 'pip', 'install', '--no-cache', '--python', _python_executable, '.'], + spinner.clear_and_log( + "⚠️ Initial installation failed, retrying with --no-cache flag...", 'warning' + ) + result = _wrap_command_with_callbacks( + [get_uv_bin(), 'pip', 'install', '--no-cache', '--python', _PYTHON_EXECUTABLE, '.'], on_progress=on_progress, on_error=on_error, ) + if result is False: + raise Exception("Installation failed with --no-cache") + else: + spinner.clear_and_log("✨ All dependencies installed successfully!", 'success') except Exception as e: - log.error(f"Installation failed: {str(e)}") + log.error(f"❌ Installation failed: {str(e)}") raise @@ -95,7 +102,7 @@ def on_error(line: str): log.info(f"Uninstalling {requirement.name}") _wrap_command_with_callbacks( - [get_uv_bin(), 'remove', '--python', _python_executable, requirement.name], + [get_uv_bin(), 'remove', '--python', _PYTHON_EXECUTABLE, requirement.name], on_progress=on_progress, on_error=on_error, ) @@ -119,7 +126,7 @@ def on_error(line: str): log.info(f"Upgrading {package}") _wrap_command_with_callbacks( - [get_uv_bin(), 'pip', 'install', '-U', '--python', _python_executable, *extra_args, package], + [get_uv_bin(), 'pip', 'install', '-U', '--python', _PYTHON_EXECUTABLE, *extra_args, package], on_progress=on_progress, on_error=on_error, use_venv=use_venv, @@ -128,7 +135,7 @@ def on_error(line: str): def create_venv(python_version: str = DEFAULT_PYTHON_VERSION): """Initialize a virtual environment in the project directory of one does not exist.""" - if os.path.exists(conf.PATH / VENV_DIR_NAME): + if os.path.exists(_VENV_PATH): return # venv already exists RE_VENV_PROGRESS = re.compile(r'^(Using|Creating)') @@ -160,7 +167,7 @@ def get_uv_bin() -> str: def _setup_env() -> dict[str, str]: """Copy the current environment and add the virtual environment path for use by a subprocess.""" env = os.environ.copy() - env["VIRTUAL_ENV"] = str(conf.PATH / VENV_DIR_NAME.absolute()) + env["VIRTUAL_ENV"] = str(_VENV_PATH) env["UV_INTERNAL__PARENT_INTERPRETER"] = sys.executable return env @@ -184,33 +191,60 @@ def _wrap_command_with_callbacks( } if use_venv: sub_args['env'] = _setup_env() + + log.debug(f"Running command: {' '.join(command)}") process = subprocess.Popen(command, **sub_args) # type: ignore assert process.stdout and process.stderr # appease type checker - readable = [process.stdout, process.stderr] - while readable: - ready, _, _ = select.select(readable, [], []) - for fd in ready: - line = fd.readline() - if not line: - readable.remove(fd) + try: + while process.poll() is None: + try: + stdout, stderr = process.communicate(timeout=1.0) + if stdout: + on_progress(stdout) + all_lines += stdout + if stderr: + on_progress(stderr) + all_lines += stderr + except subprocess.TimeoutExpired: continue - - on_progress(line) - all_lines += line - - if process.wait() == 0: # return code: success + except Exception as e: + log.error(f"Error reading output: {e}") + break + + # Get any remaining output + stdout, stderr = process.communicate() + if stdout: + on_progress(stdout) + all_lines += stdout + if stderr: + on_progress(stderr) + all_lines += stderr + + except Exception as e: + log.error(f"Error during output reading: {e}") + process.kill() + raise + + return_code = process.wait() + log.debug(f"Process completed with return code: {return_code}") + + if return_code == 0: on_complete(all_lines) return True else: + error_msg = f"Process failed with return code {return_code}" + log.error(error_msg) on_error(all_lines) return False except Exception as e: - on_error(str(e)) + error_msg = f"Exception running command: {str(e)}" + log.error(error_msg) + on_error(error_msg) return False finally: if process: try: process.terminate() - except: - pass + except Exception as e: + log.error(f"Error terminating process: {e}") diff --git a/tests/test_cli_init.py b/tests/test_cli_init.py index 92ff999d..30c07540 100644 --- a/tests/test_cli_init.py +++ b/tests/test_cli_init.py @@ -1,4 +1,4 @@ -import os, sys +import os import unittest from parameterized import parameterized from pathlib import Path @@ -6,7 +6,6 @@ from cli_test_utils import run_cli from agentstack import conf from agentstack import frameworks -from agentstack.cli import init_project from agentstack.templates import get_all_templates BASE_PATH = Path(__file__).parent @@ -15,10 +14,16 @@ class CLIInitTest(unittest.TestCase): def setUp(self): self.framework = os.getenv('TEST_FRAMEWORK') - self.project_dir = Path(BASE_PATH / 'tmp' / self.framework / 'test_cli_init') - os.chdir(BASE_PATH) # Change to parent directory first + self.project_dir = BASE_PATH / 'tmp' / self.framework / 'test_repo' + os.chdir(str(BASE_PATH)) # Change directory before cleanup to avoid Windows file locks + + # Clean up any existing test directory + if self.project_dir.exists(): + shutil.rmtree(self.project_dir, ignore_errors=True) + os.makedirs(self.project_dir, exist_ok=True) - os.chdir(self.project_dir) + os.chdir(self.project_dir) # gitpython needs a cwd + # Force UTF-8 encoding for the test environment os.environ['PYTHONIOENCODING'] = 'utf-8' @@ -38,7 +43,10 @@ def test_init_command_aliased_framework_empty_project(self, alias: str, framewor if framework != self.framework: self.skipTest(f"{alias} is not related to this framework") - conf.set_path(self.project_dir) # set working dir, init adds `slug_name` - init_project(slug_name='test_project', template='empty', framework=alias) + result = run_cli('init', 'test_project', '--template', 'empty', '--framework', alias) + self.assertEqual(result.returncode, 0) + + # Verify the framework was set correctly + conf.set_path(self.project_dir / 'test_project') config = conf.ConfigFile() - assert config.framework == framework + self.assertEqual(config.framework, framework) diff --git a/tests/test_cli_tools.py b/tests/test_cli_tools.py index 34b262b0..ad11c894 100644 --- a/tests/test_cli_tools.py +++ b/tests/test_cli_tools.py @@ -1,5 +1,4 @@ -import subprocess -import os, sys +import os import unittest from parameterized import parameterized from pathlib import Path @@ -15,12 +14,22 @@ BASE_PATH = Path(__file__).parent TEMPLATE_NAME = "empty" + class CLIToolsTest(unittest.TestCase): def setUp(self): self.framework = os.getenv('TEST_FRAMEWORK') - self.project_dir = BASE_PATH / 'tmp' / self.framework / 'cli_tools' + self.project_dir = BASE_PATH / 'tmp' / self.framework / 'test_repo' + os.chdir(str(BASE_PATH)) # Change directory before cleanup to avoid Windows file locks + + # Clean up any existing test directory + if self.project_dir.exists(): + shutil.rmtree(self.project_dir, ignore_errors=True) + os.makedirs(self.project_dir, exist_ok=True) - os.chdir(self.project_dir) + os.chdir(self.project_dir) # gitpython needs a cwd + + # Force UTF-8 encoding for the test environment + os.environ['PYTHONIOENCODING'] = 'utf-8' def tearDown(self): shutil.rmtree(self.project_dir, ignore_errors=True) @@ -133,8 +142,8 @@ def test_create_tool_invalid_name(self): # Create agent run_cli('generate', 'agent', 'test_agent', '--llm', 'openai/gpt-4') - # Test various invalid names - invalid_names = ['TestTool', 'test-tool', 'test tool'] + # Quote the space-containing name (for win) + invalid_names = ['TestTool', 'test-tool', '"test tool"'] for name in invalid_names: result = run_cli('tools', 'new', name) self.assertNotEqual(result.returncode, 0) @@ -144,4 +153,4 @@ def test_create_tool_no_project(self): """Test creating a tool outside a project directory""" # Try to create tool without initializing project result = run_cli('tools', 'new', 'test_tool') - self.assertNotEqual(result.returncode, 0) \ No newline at end of file + self.assertNotEqual(result.returncode, 0) diff --git a/tests/test_compile_llms.py b/tests/test_compile_llms.py index 1c15bebb..35aa93eb 100644 --- a/tests/test_compile_llms.py +++ b/tests/test_compile_llms.py @@ -5,88 +5,90 @@ from pathlib import Path import sys + sys.path.append(os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) from docs.compile_llms_txt import compile_llms_txt + class TestCompileLLMsTxt(unittest.TestCase): def setUp(self): self.original_cwd = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) - + # Create a temporary directory for test files self.test_dir = tempfile.mkdtemp() self.docs_dir = Path(self.test_dir) - + # Change to the temporary directory os.chdir(self.docs_dir) - + def tearDown(self): os.chdir(self.original_cwd) shutil.rmtree(self.test_dir) - + def create_test_mdx_file(self, path: str, content: str): """Helper to create test MDX files""" file_path = self.docs_dir / path file_path.parent.mkdir(parents=True, exist_ok=True) file_path.write_text(content) - + def test_basic_compilation(self): """Test basic MDX file compilation""" # Create test MDX files self.create_test_mdx_file("test1.mdx", "Test content 1") self.create_test_mdx_file("test2.mdx", "Test content 2") - + # Run compilation compile_llms_txt() - + # Check output file exists and contains expected content output_path = self.docs_dir / "llms.txt" self.assertTrue(output_path.exists()) - + content = output_path.read_text() self.assertIn("## test1.mdx", content) self.assertIn("Test content 1", content) self.assertIn("## test2.mdx", content) self.assertIn("Test content 2", content) - + def test_excluded_directories(self): """Test that files in excluded directories are skipped""" # Create files in both regular and excluded directories self.create_test_mdx_file("regular/file.mdx", "Regular content") self.create_test_mdx_file("tool/file.mdx", "Tool content") - + compile_llms_txt() - + content = (self.docs_dir / "llms.txt").read_text() self.assertIn("Regular content", content) self.assertNotIn("Tool content", content) - + def test_excluded_files(self): """Test that excluded files are skipped""" self.create_test_mdx_file("regular.mdx", "Regular content") self.create_test_mdx_file("tool.mdx", "Tool content") - + compile_llms_txt() - + content = (self.docs_dir / "llms.txt").read_text() self.assertIn("Regular content", content) self.assertNotIn("Tool content", content) - + def test_nested_directories(self): """Test compilation from nested directory structure""" self.create_test_mdx_file("dir1/test1.mdx", "Content 1") self.create_test_mdx_file("dir1/dir2/test2.mdx", "Content 2") - + compile_llms_txt() - + content = (self.docs_dir / "llms.txt").read_text() - self.assertIn("## dir1/test1.mdx", content) - self.assertIn("## dir1/dir2/test2.mdx", content) + self.assertIn(f"## dir1{os.path.sep}test1.mdx", content) + self.assertIn(f"## dir1{os.path.sep}dir2{os.path.sep}test2.mdx", content) self.assertIn("Content 1", content) self.assertIn("Content 2", content) - + def test_empty_directory(self): """Test compilation with no MDX files""" compile_llms_txt() - + content = (self.docs_dir / "llms.txt").read_text() - self.assertEqual(content, "") \ No newline at end of file + self.assertEqual(content, "") diff --git a/tests/test_frameworks.py b/tests/test_frameworks.py index 96d4edf8..9872fff1 100644 --- a/tests/test_frameworks.py +++ b/tests/test_frameworks.py @@ -1,5 +1,4 @@ -from typing import Callable -import os, sys +import os from pathlib import Path import shutil import unittest @@ -21,10 +20,11 @@ def setUp(self): self.framework = os.getenv('TEST_FRAMEWORK') self.project_dir = BASE_PATH / 'tmp' / self.framework / 'test_frameworks' - os.makedirs(self.project_dir) + # Clean up and recreate test directory + os.makedirs(self.project_dir, exist_ok=True) os.chdir(self.project_dir) # importing the crewai module requires us to be in a working directory - os.makedirs(self.project_dir / 'src') - os.makedirs(self.project_dir / 'src' / 'config') + os.makedirs(self.project_dir / 'src', exist_ok=True) + os.makedirs(self.project_dir / 'src' / 'config', exist_ok=True) (self.project_dir / 'src' / '__init__.py').touch() @@ -34,7 +34,9 @@ def setUp(self): config.framework = self.framework def tearDown(self): - shutil.rmtree(self.project_dir) + # Change directory before cleanup to avoid Windows file locks + os.chdir(str(BASE_PATH)) + shutil.rmtree(self.project_dir, ignore_errors=True) def _populate_min_entrypoint(self): """This entrypoint does not have any tools or agents.""" @@ -80,7 +82,7 @@ def test_framework_module_implements_protocol(self): assert hasattr(module, method_name), f"Method {method_name} not implemented in {self.framework}" def test_get_framework_module_invalid(self): - with self.assertRaises(Exception) as context: + with self.assertRaises(Exception): frameworks.get_framework_module('invalid') def test_validate_project(self): @@ -89,21 +91,21 @@ def test_validate_project(self): def test_validate_project_invalid(self): self._populate_min_entrypoint() - with self.assertRaises(ValidationError) as context: + with self.assertRaises(ValidationError): frameworks.validate_project() def test_validate_project_has_agent_no_task_invalid(self): self._populate_min_entrypoint() shutil.copy(BASE_PATH / 'fixtures/agents_max.yaml', self.project_dir / AGENTS_FILENAME) - + frameworks.add_agent(self._get_test_agent()) - with self.assertRaises(ValidationError) as context: + with self.assertRaises(ValidationError): frameworks.validate_project() def test_validate_project_has_task_no_agent_invalid(self): self._populate_min_entrypoint() frameworks.add_task(self._get_test_task()) - with self.assertRaises(ValidationError) as context: + with self.assertRaises(ValidationError): frameworks.validate_project() def test_validate_project_missing_agent_method_invalid(self): @@ -119,7 +121,7 @@ def test_validate_project_missing_agent_method_invalid(self): backstory: >- this is a backstory llm: openai/gpt-4o""") - with self.assertRaises(ValidationError) as context: + with self.assertRaises(ValidationError): frameworks.validate_project() def test_validate_project_missing_task_method_invalid(self): @@ -134,8 +136,8 @@ def test_validate_project_missing_task_method_invalid(self): Add your expected output here agent: >- default_agent""") - - with self.assertRaises(ValidationError) as context: + + with self.assertRaises(ValidationError): frameworks.validate_project() def test_get_agent_tool_names(self): @@ -161,7 +163,7 @@ def test_add_tool_duplicate(self): def test_add_tool_invalid(self): self._populate_min_entrypoint() - with self.assertRaises(ValidationError) as context: + with self.assertRaises(ValidationError): frameworks.add_tool(self._get_test_tool(), 'agent_name') def test_remove_tool(self): diff --git a/tests/test_log.py b/tests/test_log.py index 25316f5f..48c9c437 100644 --- a/tests/test_log.py +++ b/tests/test_log.py @@ -1,11 +1,10 @@ import unittest -import os, sys +import os import io import logging import shutil from pathlib import Path from agentstack import log, conf -from agentstack.log import SUCCESS, NOTIFY BASE_PATH = Path(__file__).parent @@ -17,7 +16,7 @@ def setUp(self): self.test_dir.mkdir(parents=True, exist_ok=True) conf.set_path(self.test_dir) - self.test_log_file = (self.test_dir / log.LOG_FILENAME) + self.test_log_file = self.test_dir / log.LOG_FILENAME self.test_log_file.touch() # Create string IO objects to capture stdout/stderr @@ -30,7 +29,17 @@ def setUp(self): log.set_stderr(self.stderr) def tearDown(self): - shutil.rmtree(self.test_dir) + # Close all handlers before cleanup + logger = logging.getLogger('agentstack') + for handler in logger.handlers[:]: # [:] creates a copy of the list + handler.close() + logger.removeHandler(handler) + + # reset log -> change dir -> remove dir + log.instance = None + os.chdir(str(BASE_PATH)) + if self.test_dir.exists(): + shutil.rmtree(self.test_dir) def test_debug_message(self): log.debug("Debug message") diff --git a/tests/test_repo.py b/tests/test_repo.py index 2499654d..e16fb648 100644 --- a/tests/test_repo.py +++ b/tests/test_repo.py @@ -1,4 +1,4 @@ -import os, sys +import os import shutil from pathlib import Path import unittest @@ -11,7 +11,6 @@ import git - BASE_PATH = Path(__file__).parent @@ -19,13 +18,26 @@ class TestRepo(unittest.TestCase): def setUp(self): self.framework = os.getenv('TEST_FRAMEWORK') self.test_dir = BASE_PATH / 'tmp' / self.framework / 'test_repo' - os.makedirs(self.test_dir) + os.chdir(str(BASE_PATH)) # Change directory before cleanup to avoid Windows file locks + os.makedirs(self.test_dir, exist_ok=True) os.chdir(self.test_dir) # gitpython needs a cwd - + conf.set_path(self.test_dir) def tearDown(self): - shutil.rmtree(self.test_dir) + # Close any open git repo before cleanup + try: + git_repo = git.Repo(self.test_dir) + git_repo.close() + except (git.exc.InvalidGitRepositoryError, git.exc.NoSuchPathError): + pass + + # Change directory out before removing + os.chdir(str(BASE_PATH)) + + # Remove the test directory + if self.test_dir.exists(): + shutil.rmtree(self.test_dir, ignore_errors=True) def test_init(self): repo.init(force_creation=True) @@ -41,8 +53,8 @@ def test_init(self): self.assertEqual(commits[0].message, f"{repo.INITIAL_COMMIT_MESSAGE}{repo.AUTOMATION_NOTE}") def test_init_parent_repo_exists(self): - os.makedirs(self.test_dir.parent / '.git') - + os.makedirs(self.test_dir.parent / '.git', exist_ok=True) + repo.init(force_creation=False) self.assertFalse((self.test_dir / '.git').is_dir()) @@ -126,30 +138,34 @@ def test_require_git_when_disabled(self, mock_should_track): def test_require_git_when_disabled_manually(self): # Disable git tracking repo.dont_track_changes() - + with self.assertRaises(repo.TrackingDisabledError): repo._require_git() - + # Reset _USE_GIT for other tests repo._USE_GIT = None - @parameterized.expand([ - ("apt", "/usr/bin/apt", "Hint: run `sudo apt install git`"), - ("brew", "/usr/local/bin/brew", "Hint: run `brew install git`"), - ("port", "/opt/local/bin/port", "Hint: run `sudo port install git`"), - ("none", None, ""), - ]) + @parameterized.expand( + [ + ("apt", "/usr/bin/apt", "Hint: run `sudo apt install git`"), + ("brew", "/usr/local/bin/brew", "Hint: run `brew install git`"), + ("port", "/opt/local/bin/port", "Hint: run `sudo port install git`"), + ("none", None, ""), + ] + ) @patch('agentstack.repo.should_track_changes', return_value=True) @patch('agentstack.repo.shutil.which') - def test_require_git_not_installed(self, name, package_manager_path, expected_hint, mock_which, mock_should_track): + def test_require_git_not_installed( + self, name, package_manager_path, expected_hint, mock_which, mock_should_track + ): mock_which.side_effect = lambda x: None if x != name else package_manager_path - + with self.assertRaises(EnvironmentError) as context: repo._require_git() - + error_message = str(context.exception) self.assertIn("git is not installed.", error_message) - + if expected_hint: self.assertIn(expected_hint, error_message) @@ -168,7 +184,7 @@ def test_transaction_context_manager(self): (self.test_dir / "test_file.txt").touch() transaction.add_message("Test message") - mock_commit.assert_called_once_with(f"Test message", ["test_file.txt"], automated=True) + mock_commit.assert_called_once_with("Test message", ["test_file.txt"], automated=True) def test_transaction_multiple_messages(self): repo.init(force_creation=True) @@ -182,7 +198,7 @@ def test_transaction_multiple_messages(self): transaction.add_message("Second message") mock_commit.assert_called_once_with( - f"First message, Second message", ["test_file.txt", "test_file_2.txt"], automated=True + "First message, Second message", ["test_file.txt", "test_file_2.txt"], automated=True ) def test_transaction_no_changes(self): @@ -242,20 +258,20 @@ def test_get_uncommitted_files_when_git_disabled(self): def test_commit_user_changes(self): repo.init(force_creation=True) - + # Create a new file test_file = self.test_dir / "user_file.txt" test_file.write_text("User content") - + # Commit user changes repo.commit_user_changes() - + # Check if the file was committed git_repo = git.Repo(self.test_dir) commits = list(git_repo.iter_commits()) - + self.assertEqual(len(commits), 2) # Initial commit + user changes commit self.assertEqual(commits[0].message, f"{repo.USER_CHANGES_COMMIT_MESSAGE}{repo.AUTOMATION_NOTE}") - + # Check if the file is no longer in uncommitted files self.assertNotIn("user_file.txt", repo.get_uncommitted_files())