diff --git a/README.md b/README.md index 796381e..6a6b407 100644 --- a/README.md +++ b/README.md @@ -123,6 +123,30 @@ Comfy provides commands that allow you to easily run the installed ComfyUI. - If you want to run ComfyUI with a specific pull request, you can use the `--pr` option. This will automatically install the specified pull request and run ComfyUI with it. - Important: When using --pr, any --version and --commit parameters are ignored. The PR branch will be checked out regardless of version settings. +- To test a frontend pull request: + + ``` + comfy launch --frontend-pr "#456" + comfy launch --frontend-pr "username:branch-name" + comfy launch --frontend-pr "https://github.com/Comfy-Org/ComfyUI_frontend/pull/456" + ``` + + - The `--frontend-pr` option allows you to test frontend PRs by automatically cloning, building, and using the frontend for that session. + - Requirements: Node.js and npm must be installed to build the frontend. + - Builds are cached for quick switching between PRs - subsequent uses of the same PR are instant. + - Each PR is used only for that launch session. Normal launches use the default frontend. + + **Managing PR cache**: + ``` + comfy pr-cache list # List cached PR builds + comfy pr-cache clean # Clean all cached builds + comfy pr-cache clean 456 # Clean specific PR cache + ``` + + - Cache automatically expires after 7 days + - Maximum of 10 PR builds are kept (oldest are removed automatically) + - Cache limits help manage disk space while keeping recent builds available + ### Managing Custom Nodes comfy provides a convenient way to manage custom nodes for extending ComfyUI's functionality. Here are some examples: diff --git a/comfy_cli/cmdline.py b/comfy_cli/cmdline.py index 7efe9cf..d5a5a08 100644 --- a/comfy_cli/cmdline.py +++ b/comfy_cli/cmdline.py @@ -10,7 +10,7 @@ from rich.console import Console from comfy_cli import constants, env_checker, logging, tracking, ui, utils -from comfy_cli.command import custom_nodes +from comfy_cli.command import custom_nodes, pr_command from comfy_cli.command import install as install_inner from comfy_cli.command import run as run_inner from comfy_cli.command.install import validate_version @@ -485,10 +485,18 @@ def stop(): @app.command(help="Launch ComfyUI: ?[--background] ?[-- ]") @tracking.track_command() def launch( - background: Annotated[bool, typer.Option(help="Launch ComfyUI in background")] = False, extra: list[str] = typer.Argument(None), + background: Annotated[bool, typer.Option(help="Launch ComfyUI in background")] = False, + frontend_pr: Annotated[ + Optional[str], + typer.Option( + "--frontend-pr", + show_default=False, + help="Use a specific frontend PR. Supports formats: username:branch, #123, or PR URL", + ), + ] = None, ): - launch_command(background, extra) + launch_command(background, extra, frontend_pr) @app.command("set-default", help="Set default ComfyUI path") @@ -658,4 +666,7 @@ def standalone( app.add_typer(models_command.app, name="model", help="Manage models.") app.add_typer(custom_nodes.app, name="node", help="Manage custom nodes.") app.add_typer(custom_nodes.manager_app, name="manager", help="Manage ComfyUI-Manager.") + +app.add_typer(pr_command.app, name="pr-cache", help="Manage PR cache.") + app.add_typer(tracking.app, name="tracking", help="Manage analytics tracking settings.") diff --git a/comfy_cli/command/install.py b/comfy_cli/command/install.py index d549d33..7dc8d86 100755 --- a/comfy_cli/command/install.py +++ b/comfy_cli/command/install.py @@ -631,3 +631,172 @@ def find_pr_by_branch(repo_owner: str, repo_name: str, username: str, branch: st except requests.RequestException: return None + + +def verify_node_tools() -> bool: + """Verify that Node.js, npm, and vite are available""" + try: + # Check Node.js + node_result = subprocess.run(["node", "--version"], capture_output=True, text=True, check=False) + if node_result.returncode != 0: + rprint("[bold red]Node.js is not installed.[/bold red]") + rprint("[yellow]To use --frontend-pr, please install Node.js first:[/yellow]") + rprint(" • Download from: https://nodejs.org/") + rprint(" • Or use a package manager:") + rprint(" - macOS: brew install node") + rprint(" - Ubuntu/Debian: sudo apt install nodejs npm") + rprint(" - Windows: winget install OpenJS.NodeJS") + return False + + node_version = node_result.stdout.strip() + rprint(f"[green]Found Node.js {node_version}[/green]") + + # Check npm + npm_result = subprocess.run(["npm", "--version"], capture_output=True, text=True, check=False) + if npm_result.returncode != 0: + rprint("[bold red]npm is not installed.[/bold red]") + rprint("[yellow]npm usually comes with Node.js. Try reinstalling Node.js.[/yellow]") + return False + + npm_version = npm_result.stdout.strip() + rprint(f"[green]Found npm {npm_version}[/green]") + + return True + except FileNotFoundError as e: + rprint(f"[bold red]Error checking Node.js tools: {e}[/bold red]") + return False + + +def handle_temporary_frontend_pr(frontend_pr: str) -> Optional[str]: + """Handle temporary frontend PR for launch - returns path to built frontend""" + from comfy_cli.pr_cache import PRCache + + rprint("\n[bold blue]Preparing frontend PR for launch...[/bold blue]") + + # Verify Node.js tools first + if not verify_node_tools(): + rprint("[bold red]Cannot build frontend without Node.js and npm[/bold red]") + return None + + # Parse frontend PR reference + try: + repo_owner, repo_name, pr_number = parse_frontend_pr_reference(frontend_pr) + except ValueError as e: + rprint(f"[bold red]Error parsing frontend PR reference: {e}[/bold red]") + return None + + # Fetch PR info + try: + if pr_number: + pr_info = fetch_pr_info(repo_owner, repo_name, pr_number) + else: + username, branch = frontend_pr.split(":", 1) + pr_info = find_pr_by_branch(repo_owner, repo_name, username, branch) + + if not pr_info: + rprint(f"[bold red]Frontend PR not found: {frontend_pr}[/bold red]") + return None + except Exception as e: + rprint(f"[bold red]Error fetching frontend PR information: {e}[/bold red]") + return None + + # Check cache first + cache = PRCache() + cached_path = cache.get_cached_frontend_path(pr_info) + if cached_path: + rprint(f"[bold green]Using cached frontend build for PR #{pr_info.number}[/bold green]") + rprint(f"[bold green]PR #{pr_info.number}: {pr_info.title} by {pr_info.user}[/bold green]") + return str(cached_path) + + # Need to build - show PR info + console.print( + Panel( + f"[bold]Frontend PR #{pr_info.number}[/bold]: {pr_info.title}\n" + f"[yellow]Author[/yellow]: {pr_info.user}\n" + f"[yellow]Branch[/yellow]: {pr_info.head_branch}\n" + f"[yellow]Source[/yellow]: {pr_info.head_repo_url}", + title="[bold blue]Building Frontend PR[/bold blue]", + border_style="blue", + ) + ) + + # Build in cache directory + cache_path = cache.get_frontend_cache_path(pr_info) + cache_path.mkdir(parents=True, exist_ok=True) + + # Clone or update repository + repo_path = cache_path / "repo" + if not (repo_path / ".git").exists(): + rprint("Cloning frontend repository...") + clone_comfyui(url=pr_info.base_repo_url, repo_dir=str(repo_path)) + + # Checkout PR + rprint(f"Checking out PR #{pr_info.number}...") + success = checkout_pr(str(repo_path), pr_info) + if not success: + rprint("[bold red]Failed to checkout frontend PR[/bold red]") + return None + + # Build frontend + rprint("\n[bold yellow]Building frontend (this may take a moment)...[/bold yellow]") + original_dir = os.getcwd() + try: + os.chdir(repo_path) + + # Run npm install + rprint("Running npm install...") + npm_install = subprocess.run(["npm", "install"], capture_output=True, text=True, check=False) + if npm_install.returncode != 0: + rprint(f"[bold red]npm install failed:[/bold red]\n{npm_install.stderr}") + return None + + # Build with vite + rprint("Building with vite...") + vite_build = subprocess.run(["npx", "vite", "build"], capture_output=True, text=True, check=False) + if vite_build.returncode != 0: + rprint(f"[bold red]vite build failed:[/bold red]\n{vite_build.stderr}") + return None + + # Check if dist exists + dist_path = repo_path / "dist" + if dist_path.exists(): + # Save cache info + cache.save_cache_info(pr_info, cache_path) + rprint("[bold green]✓ Frontend built and cached successfully[/bold green]") + rprint(f"[bold green]Using frontend from PR #{pr_info.number}: {pr_info.title}[/bold green]") + rprint(f"[dim]Cache will expire in {cache.DEFAULT_MAX_CACHE_AGE_DAYS} days[/dim]") + return str(dist_path) + else: + rprint("[bold red]Frontend build completed but dist folder not found[/bold red]") + return None + + finally: + os.chdir(original_dir) + + +def parse_frontend_pr_reference(pr_ref: str) -> tuple[str, str, Optional[int]]: + """ + Parse frontend PR reference. Similar to parse_pr_reference but defaults to Comfy-Org/ComfyUI_frontend + """ + pr_ref = pr_ref.strip() + + if pr_ref.startswith("https://github.com/"): + parsed = urlparse(pr_ref) + if "/pull/" in parsed.path: + path_parts = parsed.path.strip("/").split("/") + if len(path_parts) >= 4: + repo_owner = path_parts[0] + repo_name = path_parts[1] + pr_number = int(path_parts[3]) + return repo_owner, repo_name, pr_number + + elif pr_ref.startswith("#"): + pr_number = int(pr_ref[1:]) + return "Comfy-Org", "ComfyUI_frontend", pr_number + + elif ":" in pr_ref: + username, branch = pr_ref.split(":", 1) + return "Comfy-Org", "ComfyUI_frontend", None + + else: + raise ValueError(f"Invalid frontend PR reference format: {pr_ref}") diff --git a/comfy_cli/command/launch.py b/comfy_cli/command/launch.py index 52885e6..30f92a8 100644 --- a/comfy_cli/command/launch.py +++ b/comfy_cli/command/launch.py @@ -22,7 +22,7 @@ console = Console() -def launch_comfyui(extra): +def launch_comfyui(extra, frontend_pr=None): reboot_path = None new_env = os.environ.copy() @@ -36,6 +36,20 @@ def launch_comfyui(extra): extra = extra if extra is not None else [] + # Handle temporary frontend PR + if frontend_pr: + from comfy_cli.command.install import handle_temporary_frontend_pr + + try: + frontend_path = handle_temporary_frontend_pr(frontend_pr) + if frontend_path: + # Check if --front-end-root is not already specified + if not any(arg.startswith("--front-end-root") for arg in extra): + extra = ["--front-end-root", frontend_path] + extra + except Exception as e: + print(f"[bold red]Failed to prepare frontend PR: {e}[/bold red]") + # Continue with default frontend + process = None if "COMFY_CLI_BACKGROUND" not in os.environ: @@ -107,6 +121,7 @@ def redirector_stdout(): def launch( background: bool = False, extra: list[str] | None = None, + frontend_pr: str | None = None, ): check_for_updates() resolved_workspace = workspace_manager.workspace_path @@ -133,12 +148,12 @@ def launch( os.chdir(resolved_workspace) if background: - background_launch(extra) + background_launch(extra, frontend_pr) else: - launch_comfyui(extra) + launch_comfyui(extra, frontend_pr) -def background_launch(extra): +def background_launch(extra, frontend_pr=None): config_background = ConfigManager().background if config_background is not None and utils.is_running(config_background[2]): console.print( @@ -171,7 +186,13 @@ def background_launch(extra): "comfy", f"--workspace={os.path.abspath(os.getcwd())}", "launch", - ] + extra + ] + + # Add frontend PR option if specified + if frontend_pr: + cmd.extend(["--frontend-pr", frontend_pr]) + + cmd.extend(extra) loop = asyncio.get_event_loop() log = loop.run_until_complete(launch_and_monitor(cmd, listen, port)) diff --git a/comfy_cli/command/pr_command.py b/comfy_cli/command/pr_command.py new file mode 100644 index 0000000..3105ce9 --- /dev/null +++ b/comfy_cli/command/pr_command.py @@ -0,0 +1,85 @@ +"""PR cache management commands. + +This module provides CLI commands for managing the PR cache, including: +- Listing cached PR builds +- Cleaning specific or all cached builds +- Displaying cache information in a user-friendly format +""" + +import typer +from rich import print as rprint +from rich.console import Console +from rich.table import Table + +from comfy_cli import tracking +from comfy_cli.pr_cache import PRCache + +app = typer.Typer(help="Manage PR cache") +console = Console() + + +@app.command("list", help="List cached PR builds") +@tracking.track_command() +def list_cached() -> None: + """List all cached PR builds.""" + cache = PRCache() + cached_frontends = cache.list_cached_frontends() + + if not cached_frontends: + rprint("[yellow]No cached PR builds found[/yellow]") + return + + table = Table(title="Cached Frontend PR Builds") + table.add_column("PR #", style="cyan") + table.add_column("Title", style="white") + table.add_column("Author", style="green") + table.add_column("Age", style="yellow") + table.add_column("Size (MB)", style="magenta") + + for info in cached_frontends: + age = cache.get_cache_age(info.get("cached_at", "")) + table.add_row( + str(info.get("pr_number", "?")), + info.get("pr_title", "Unknown")[:50], # Truncate long titles + info.get("user", "Unknown"), + age, + f"{info.get('size_mb', 0):.1f}", + ) + + console.print(table) + + # Show cache settings + rprint( + f"\n[dim]Cache settings: Max age: {cache.DEFAULT_MAX_CACHE_AGE_DAYS} days, " + f"Max items: {cache.DEFAULT_MAX_CACHE_ITEMS}[/dim]" + ) + + +@app.command("clean", help="Clean PR cache") +@tracking.track_command() +def clean_cache( + pr_number: int = typer.Argument(None, help="Specific PR number to clean (omit to clean all)"), + yes: bool = typer.Option(False, "--yes", "-y", help="Skip confirmation"), +) -> None: + """Clean cached PR builds.""" + cache = PRCache() + + if pr_number: + if not yes: + confirm = typer.confirm(f"Remove cache for PR #{pr_number}?") + if not confirm: + rprint("[yellow]Cancelled[/yellow]") + return + cache.clean_frontend_cache(pr_number) + rprint(f"[green]✓ Cleaned cache for PR #{pr_number}[/green]") + else: + if not yes: + cached = cache.list_cached_frontends() + if cached: + rprint(f"[yellow]This will remove {len(cached)} cached PR build(s)[/yellow]") + confirm = typer.confirm("Remove all cached PR builds?") + if not confirm: + rprint("[yellow]Cancelled[/yellow]") + return + cache.clean_frontend_cache() + rprint("[green]✓ Cleaned all PR cache[/green]") diff --git a/comfy_cli/pr_cache.py b/comfy_cli/pr_cache.py new file mode 100644 index 0000000..e0b63a6 --- /dev/null +++ b/comfy_cli/pr_cache.py @@ -0,0 +1,235 @@ +"""PR Cache Management for temporary PR testing. + +This module provides functionality for caching built frontend PRs to enable +quick switching between different PR versions without rebuilding. +""" + +from __future__ import annotations + +import json +import shutil +from datetime import datetime, timedelta +from pathlib import Path + +from rich import print as rprint + +from comfy_cli.config_manager import ConfigManager + + +class PRCache: + """Manages cached PR builds for quick switching. + + This class handles the caching of built frontend PRs, including: + - Cache directory management + - Cache validity checking with age limits + - Automatic cleanup of old/excess cache entries + - Human-readable cache information display + """ + + # Default cache settings + DEFAULT_MAX_CACHE_AGE_DAYS = 7 # Cache entries older than this are considered stale + DEFAULT_MAX_CACHE_ITEMS = 10 # Maximum number of cached PRs to keep + + def __init__(self) -> None: + """Initialize PR cache with default settings.""" + self.cache_dir = Path(ConfigManager().get_config_path()) / "pr-cache" + self.cache_dir.mkdir(parents=True, exist_ok=True) + self.max_cache_age = timedelta(days=self.DEFAULT_MAX_CACHE_AGE_DAYS) + self.max_cache_items = self.DEFAULT_MAX_CACHE_ITEMS + + def get_frontend_cache_path(self, pr_info) -> Path: + """Get cache path for a frontend PR""" + # Use PR number and repo as cache key + cache_key = f"{pr_info.user}-{pr_info.number}-{pr_info.head_branch}" + # Sanitize for filesystem + cache_key = "".join(c if c.isalnum() or c in "-_" else "_" for c in cache_key) + return self.cache_dir / "frontend" / cache_key + + def get_cache_info_path(self, cache_path: Path) -> Path: + """Get path to cache info file""" + return cache_path / ".cache-info.json" + + def is_cache_valid(self, pr_info, cache_path: Path) -> bool: + """Check if cached build is still valid""" + info_path = self.get_cache_info_path(cache_path) + if not info_path.exists(): + return False + + try: + with open(info_path, encoding="utf-8") as file: + cache_info = json.load(file) + + # Check if cache metadata matches + if not ( + cache_info.get("pr_number") == pr_info.number + and cache_info.get("head_branch") == pr_info.head_branch + and cache_info.get("user") == pr_info.user + ): + return False + + # Check if cache is too old + cached_at = cache_info.get("cached_at") + if cached_at: + cache_time = datetime.fromisoformat(cached_at) + if datetime.now() - cache_time > self.max_cache_age: + return False + + return True + except (json.JSONDecodeError, OSError): + return False + + def save_cache_info(self, pr_info, cache_path: Path) -> None: + """Save cache metadata.""" + info_path = self.get_cache_info_path(cache_path) + cache_info = { + "pr_number": pr_info.number, + "pr_title": pr_info.title, + "user": pr_info.user, + "head_branch": pr_info.head_branch, + "head_repo_url": pr_info.head_repo_url, + "cached_at": datetime.now().isoformat(), + } + + with open(info_path, "w", encoding="utf-8") as file: + json.dump(cache_info, file, indent=2) + + # Enforce cache limits after saving new cache + self.enforce_cache_limits() + + def get_cached_frontend_path(self, pr_info) -> Path | None: + """Get path to cached frontend build if valid""" + cache_path = self.get_frontend_cache_path(pr_info) + dist_path = cache_path / "repo" / "dist" + + if dist_path.exists() and self.is_cache_valid(pr_info, cache_path): + return dist_path + return None + + def _load_cache_info(self, cache_dir: Path) -> dict | None: + """Load cache info from a directory.""" + info_path = self.get_cache_info_path(cache_dir) + if not info_path.exists(): + return None + + try: + with open(info_path, encoding="utf-8") as file: + return json.load(file) + except (json.JSONDecodeError, OSError): + return None + + def _clean_specific_pr_cache(self, frontend_cache: Path, pr_number: int) -> None: + """Clean cache for a specific PR number.""" + for cache_dir in frontend_cache.iterdir(): + if not cache_dir.is_dir(): + continue + info = self._load_cache_info(cache_dir) + if info and info.get("pr_number") == pr_number: + rprint(f"[yellow]Removing cache for PR #{pr_number}[/yellow]") + shutil.rmtree(cache_dir) + break + + def clean_frontend_cache(self, pr_number: int | None = None) -> None: + """Clean frontend cache (specific PR or all).""" + frontend_cache = self.cache_dir / "frontend" + if not frontend_cache.exists(): + return + + if pr_number: + self._clean_specific_pr_cache(frontend_cache, pr_number) + else: + # Clean all + rprint("[yellow]Removing all frontend PR cache[/yellow]") + shutil.rmtree(frontend_cache) + + def _calculate_cache_size_mb(self, cache_dir: Path) -> float: + """Calculate the size of a cache directory in MB.""" + total_size = sum(f.stat().st_size for f in cache_dir.rglob("*") if f.is_file()) + return total_size / (1024 * 1024) + + def _get_cache_info_with_metadata(self, cache_dir: Path) -> dict | None: + """Get cache info with additional metadata like path and size.""" + info = self._load_cache_info(cache_dir) + if info: + info["cache_path"] = str(cache_dir) + info["size_mb"] = self._calculate_cache_size_mb(cache_dir) + return info + + def list_cached_frontends(self) -> list[dict]: + """List all cached frontend PRs.""" + frontend_cache = self.cache_dir / "frontend" + if not frontend_cache.exists(): + return [] + + cached_prs = [] + for cache_dir in frontend_cache.iterdir(): + if not cache_dir.is_dir(): + continue + info = self._get_cache_info_with_metadata(cache_dir) + if info: + cached_prs.append(info) + + return sorted(cached_prs, key=lambda x: x.get("cached_at", ""), reverse=True) + + def _is_cache_expired(self, cached_at: str) -> bool: + """Check if a cache entry is expired based on its timestamp.""" + try: + cache_time = datetime.fromisoformat(cached_at) + return datetime.now() - cache_time > self.max_cache_age + except (ValueError, TypeError): + return True # Consider invalid timestamps as expired + + def _get_expired_items(self, cached_items: list[dict]) -> list[dict]: + """Get list of expired cache items.""" + expired = [] + for item in cached_items: + cached_at = item.get("cached_at") + if cached_at and self._is_cache_expired(cached_at): + expired.append(item) + return expired + + def _get_excess_items(self, cached_items: list[dict], expired_items: list[dict]) -> list[dict]: + """Get list of items that exceed the maximum cache limit.""" + remaining_items = [item for item in cached_items if item not in expired_items] + if len(remaining_items) > self.max_cache_items: + # Return oldest items that exceed the limit + return remaining_items[self.max_cache_items :] + return [] + + def _remove_cache_item(self, item: dict) -> None: + """Remove a single cache item.""" + cache_path = Path(item["cache_path"]) + if cache_path.exists(): + pr_info = f"PR #{item.get('pr_number', '?')} ({item.get('pr_title', 'Unknown')[:30]}...)" + rprint(f"[yellow]Removing old cache: {pr_info}[/yellow]") + shutil.rmtree(cache_path) + + def enforce_cache_limits(self) -> None: + """Remove old and excess cache entries to maintain limits.""" + cached_items = self.list_cached_frontends() + + # Get items to remove + expired_items = self._get_expired_items(cached_items) + excess_items = self._get_excess_items(cached_items, expired_items) + + # Remove all identified items + items_to_remove = expired_items + excess_items + for item in items_to_remove: + self._remove_cache_item(item) + + def get_cache_age(self, cached_at: str) -> str: + """Get human-readable age of cache entry""" + try: + cache_time = datetime.fromisoformat(cached_at) + age = datetime.now() - cache_time + + if age.days > 0: + return f"{age.days} day{'s' if age.days != 1 else ''} ago" + if age.seconds > 3600: + hours = age.seconds // 3600 + return f"{hours} hour{'s' if hours != 1 else ''} ago" + if age.seconds > 60: + minutes = age.seconds // 60 + return f"{minutes} minute{'s' if minutes != 1 else ''} ago" + return "just now" + except (json.JSONDecodeError, OSError): + return "unknown" diff --git a/tests/comfy_cli/command/github/test_pr.py b/tests/comfy_cli/command/github/test_pr.py index 2802922..8f72d16 100644 --- a/tests/comfy_cli/command/github/test_pr.py +++ b/tests/comfy_cli/command/github/test_pr.py @@ -118,16 +118,16 @@ def test_fetch_pr_info_rate_limit(self, mock_get): @patch("requests.get") def test_find_pr_by_branch_success(self, mock_get): - """Test finding PR by branch name""" + """Test successful PR search by branch""" mock_response = Mock() mock_response.status_code = 200 mock_response.json.return_value = [ { - "number": 123, - "title": "Add 3D node loading support", + "number": 456, + "title": "Test PR", "head": { - "repo": {"clone_url": "https://github.com/jtydhr88/ComfyUI.git", "owner": {"login": "jtydhr88"}}, - "ref": "load-3d-nodes", + "repo": {"clone_url": "https://github.com/testuser/ComfyUI.git", "owner": {"login": "testuser"}}, + "ref": "test-branch", }, "base": {"repo": {"clone_url": "https://github.com/comfyanonymous/ComfyUI.git"}, "ref": "master"}, "mergeable": True, @@ -135,21 +135,31 @@ def test_find_pr_by_branch_success(self, mock_get): ] mock_get.return_value = mock_response - result = find_pr_by_branch("comfyanonymous", "ComfyUI", "jtydhr88", "load-3d-nodes") + result = find_pr_by_branch("comfyanonymous", "ComfyUI", "testuser", "test-branch") assert result is not None - assert result.number == 123 - assert result.user == "jtydhr88" + assert result.number == 456 + assert result.title == "Test PR" + assert result.user == "testuser" + assert result.head_branch == "test-branch" @patch("requests.get") def test_find_pr_by_branch_not_found(self, mock_get): - """Test branch not found returns None""" + """Test PR not found by branch""" mock_response = Mock() mock_response.status_code = 200 - mock_response.json.return_value = [] # Empty list + mock_response.json.return_value = [] mock_get.return_value = mock_response - result = find_pr_by_branch("comfyanonymous", "ComfyUI", "user", "nonexistent-branch") + result = find_pr_by_branch("comfyanonymous", "ComfyUI", "testuser", "nonexistent-branch") + assert result is None + + @patch("requests.get") + def test_find_pr_by_branch_error(self, mock_get): + """Test error when searching PR by branch""" + mock_get.side_effect = requests.RequestException("Network error") + + result = find_pr_by_branch("comfyanonymous", "ComfyUI", "testuser", "test-branch") assert result is None diff --git a/tests/comfy_cli/command/test_frontend_pr.py b/tests/comfy_cli/command/test_frontend_pr.py new file mode 100644 index 0000000..a557324 --- /dev/null +++ b/tests/comfy_cli/command/test_frontend_pr.py @@ -0,0 +1,127 @@ +from unittest.mock import Mock, patch + +import pytest +from typer.testing import CliRunner + +from comfy_cli.command.install import ( + PRInfo, + parse_frontend_pr_reference, + verify_node_tools, +) + + +@pytest.fixture +def runner(): + return CliRunner() + + +@pytest.fixture +def sample_frontend_pr_info(): + return PRInfo( + number=456, + head_repo_url="https://github.com/testuser/ComfyUI_frontend.git", + head_branch="feature-branch", + base_repo_url="https://github.com/Comfy-Org/ComfyUI_frontend.git", + base_branch="main", + title="Add new feature to frontend", + user="testuser", + mergeable=True, + ) + + +class TestFrontendPRReferenceParsing: + """Test frontend PR reference parsing functionality""" + + def test_parse_frontend_pr_number_format(self): + """Test parsing #123 format for frontend""" + repo_owner, repo_name, pr_number = parse_frontend_pr_reference("#456") + assert repo_owner == "Comfy-Org" + assert repo_name == "ComfyUI_frontend" + assert pr_number == 456 + + def test_parse_frontend_user_branch_format(self): + """Test parsing username:branch format for frontend""" + repo_owner, repo_name, pr_number = parse_frontend_pr_reference("testuser:feature-branch") + assert repo_owner == "Comfy-Org" + assert repo_name == "ComfyUI_frontend" + assert pr_number is None + + def test_parse_frontend_github_url_format(self): + """Test parsing full GitHub PR URL for frontend""" + url = "https://github.com/Comfy-Org/ComfyUI_frontend/pull/789" + repo_owner, repo_name, pr_number = parse_frontend_pr_reference(url) + assert repo_owner == "Comfy-Org" + assert repo_name == "ComfyUI_frontend" + assert pr_number == 789 + + def test_parse_frontend_custom_repo_url(self): + """Test parsing URL from custom repository""" + url = "https://github.com/customuser/customrepo/pull/123" + repo_owner, repo_name, pr_number = parse_frontend_pr_reference(url) + assert repo_owner == "customuser" + assert repo_name == "customrepo" + assert pr_number == 123 + + def test_parse_frontend_invalid_format(self): + """Test parsing invalid format raises ValueError""" + with pytest.raises(ValueError, match="Invalid frontend PR reference format"): + parse_frontend_pr_reference("invalid-format") + + def test_parse_frontend_empty_string(self): + """Test parsing empty string raises ValueError""" + with pytest.raises(ValueError): + parse_frontend_pr_reference("") + + +class TestNodeToolsVerification: + """Test Node.js tools verification""" + + @patch("subprocess.run") + def test_verify_node_tools_success(self, mock_run): + """Test successful Node.js and npm verification""" + # Mock successful node and npm commands + node_result = Mock() + node_result.returncode = 0 + node_result.stdout = "v18.0.0" + + npm_result = Mock() + npm_result.returncode = 0 + npm_result.stdout = "9.0.0" + + mock_run.side_effect = [node_result, npm_result] + + assert verify_node_tools() is True + assert mock_run.call_count == 2 + + @patch("subprocess.run") + def test_verify_node_tools_missing_node(self, mock_run): + """Test when Node.js is not installed""" + node_result = Mock() + node_result.returncode = 1 + + mock_run.return_value = node_result + + assert verify_node_tools() is False + mock_run.assert_called_once_with(["node", "--version"], capture_output=True, text=True, check=False) + + @patch("subprocess.run") + def test_verify_node_tools_missing_npm(self, mock_run): + """Test when npm is not installed""" + node_result = Mock() + node_result.returncode = 0 + node_result.stdout = "v18.0.0" + + npm_result = Mock() + npm_result.returncode = 1 + + mock_run.side_effect = [node_result, npm_result] + + assert verify_node_tools() is False + assert mock_run.call_count == 2 + + @patch("subprocess.run") + def test_verify_node_tools_file_not_found(self, mock_run): + """Test when commands are not found""" + mock_run.side_effect = FileNotFoundError("node not found") + + assert verify_node_tools() is False diff --git a/tests/comfy_cli/command/test_launch_frontend_pr.py b/tests/comfy_cli/command/test_launch_frontend_pr.py new file mode 100644 index 0000000..74871f6 --- /dev/null +++ b/tests/comfy_cli/command/test_launch_frontend_pr.py @@ -0,0 +1,269 @@ +"""Tests for launch-time frontend PR functionality""" + +import json +from datetime import datetime, timedelta +from pathlib import Path +from unittest.mock import Mock, patch + +import pytest +from typer.testing import CliRunner + +from comfy_cli.cmdline import app +from comfy_cli.command.install import PRInfo, handle_temporary_frontend_pr +from comfy_cli.pr_cache import PRCache + + +@pytest.fixture +def runner(): + return CliRunner() + + +@pytest.fixture +def sample_frontend_pr_info(): + return PRInfo( + number=789, + head_repo_url="https://github.com/testuser/ComfyUI_frontend.git", + head_branch="test-feature", + base_repo_url="https://github.com/Comfy-Org/ComfyUI_frontend.git", + base_branch="main", + title="Test feature for frontend", + user="testuser", + mergeable=True, + ) + + +@pytest.fixture +def mock_pr_cache(): + with patch("comfy_cli.pr_cache.PRCache") as mock_cache_cls: + mock_cache = Mock() + mock_cache_cls.return_value = mock_cache + yield mock_cache + + +class TestLaunchWithFrontendPR: + """Test launching with temporary frontend PR""" + + @patch("comfy_cli.command.install.verify_node_tools") + def test_launch_frontend_pr_without_node(self, mock_verify): + """Test launch with frontend PR when Node.js is missing""" + mock_verify.return_value = False + + result = handle_temporary_frontend_pr("#123") + assert result is None + mock_verify.assert_called_once() + + @patch("comfy_cli.command.install.verify_node_tools") + @patch("comfy_cli.command.install.parse_frontend_pr_reference") + @patch("comfy_cli.command.install.fetch_pr_info") + def test_launch_frontend_pr_with_cache_hit( + self, mock_fetch, mock_parse, mock_verify, mock_pr_cache, sample_frontend_pr_info + ): + """Test launch with cached frontend PR""" + mock_verify.return_value = True + mock_parse.return_value = ("Comfy-Org", "ComfyUI_frontend", 789) + mock_fetch.return_value = sample_frontend_pr_info + + # Mock cache hit + cached_path = Path("/cache/frontend/pr-789/dist") + mock_pr_cache.get_cached_frontend_path.return_value = cached_path + + result = handle_temporary_frontend_pr("#789") + + assert result == str(cached_path) + mock_pr_cache.get_cached_frontend_path.assert_called_once() + # Should not build if cache hit + mock_pr_cache.save_cache_info.assert_not_called() + + @patch("pathlib.Path.mkdir") + @patch("os.chdir") + @patch("subprocess.run") + @patch("comfy_cli.command.install.checkout_pr") + @patch("comfy_cli.command.install.clone_comfyui") + @patch("comfy_cli.command.install.verify_node_tools") + @patch("comfy_cli.command.install.parse_frontend_pr_reference") + @patch("comfy_cli.command.install.fetch_pr_info") + def test_launch_frontend_pr_cache_miss_builds( + self, + mock_fetch, + mock_parse, + mock_verify, + mock_clone, + mock_checkout, + mock_run, + mock_chdir, + mock_mkdir, + mock_pr_cache, + sample_frontend_pr_info, + ): + """Test launch builds frontend when not cached""" + mock_verify.return_value = True + mock_parse.return_value = ("Comfy-Org", "ComfyUI_frontend", 789) + mock_fetch.return_value = sample_frontend_pr_info + mock_checkout.return_value = True + + # Mock cache miss + mock_pr_cache.get_cached_frontend_path.return_value = None + cache_path = Path("/cache/frontend/pr-789") + mock_pr_cache.get_frontend_cache_path.return_value = cache_path + + # Mock successful build + mock_run.side_effect = [ + Mock(returncode=0), # npm install + Mock(returncode=0), # vite build + ] + + # Mock dist exists + with patch("pathlib.Path.exists") as mock_exists: + mock_exists.return_value = True + + result = handle_temporary_frontend_pr("#789") + + # Should return built path + assert result == str(cache_path / "repo" / "dist") + # Should save cache info + mock_pr_cache.save_cache_info.assert_called_once_with(sample_frontend_pr_info, cache_path) + + +class TestPRCacheManagement: + """Test PR cache functionality""" + + def test_pr_cache_get_frontend_path(self, sample_frontend_pr_info): + """Test getting frontend cache path""" + cache = PRCache() + path = cache.get_frontend_cache_path(sample_frontend_pr_info) + + assert "frontend" in str(path) + assert "testuser" in str(path) + assert "789" in str(path) + + def test_pr_cache_list_empty(self): + """Test listing empty cache""" + cache = PRCache() + with patch("pathlib.Path.exists", return_value=False): + result = cache.list_cached_frontends() + assert result == [] + + def test_pr_cache_clean_specific(self, tmp_path): + """Test cleaning specific PR cache""" + cache = PRCache() + cache.cache_dir = tmp_path / "test-cache" + + # Create mock cache structure + frontend_cache = cache.cache_dir / "frontend" / "pr-123" + frontend_cache.mkdir(parents=True) + cache_info = frontend_cache / ".cache-info.json" + cache_info.write_text('{"pr_number": 123}') + + # Clean specific PR + cache.clean_frontend_cache(123) + + assert not frontend_cache.exists() + + def test_pr_cache_age_check(self, sample_frontend_pr_info, tmp_path): + """Test cache age validation""" + cache = PRCache() + cache.cache_dir = tmp_path / "test-cache" + cache_path = cache.get_frontend_cache_path(sample_frontend_pr_info) + cache_path.mkdir(parents=True) + + # Create cache info with old timestamp + old_time = datetime.now() - timedelta(days=10) + cache_info = { + "pr_number": sample_frontend_pr_info.number, + "pr_title": sample_frontend_pr_info.title, + "user": sample_frontend_pr_info.user, + "head_branch": sample_frontend_pr_info.head_branch, + "cached_at": old_time.isoformat(), + } + + info_path = cache.get_cache_info_path(cache_path) + with open(info_path, "w") as f: + json.dump(cache_info, f) + + # Should be invalid due to age + assert not cache.is_cache_valid(sample_frontend_pr_info, cache_path) + + def test_pr_cache_enforce_limits(self, tmp_path): + """Test cache limit enforcement""" + cache = PRCache() + cache.cache_dir = tmp_path / "test-cache" + cache.max_cache_items = 3 # Set low limit for testing + + # Create multiple cache entries + for i in range(5): + cache_dir = cache.cache_dir / "frontend" / f"pr-{i}" + cache_dir.mkdir(parents=True) + cache_info = { + "pr_number": i, + "pr_title": f"Test PR {i}", + "cached_at": (datetime.now() - timedelta(hours=i)).isoformat(), + } + with open(cache_dir / ".cache-info.json", "w") as f: + json.dump(cache_info, f) + + # Enforce limits + cache.enforce_cache_limits() + + # Should only have 3 newest items + remaining = list((cache.cache_dir / "frontend").iterdir()) + assert len(remaining) == 3 + # Check that newest items remain + remaining_numbers = sorted([int(d.name.split("-")[1]) for d in remaining]) + assert remaining_numbers == [0, 1, 2] # Newest 3 + + def test_get_cache_age(self): + """Test human-readable cache age""" + cache = PRCache() + + # Test various ages + now = datetime.now() + assert cache.get_cache_age(now.isoformat()) == "just now" + + age_5_min = (now - timedelta(minutes=5)).isoformat() + assert "5 minutes ago" in cache.get_cache_age(age_5_min) + + age_2_hours = (now - timedelta(hours=2)).isoformat() + assert "2 hours ago" in cache.get_cache_age(age_2_hours) + + age_3_days = (now - timedelta(days=3)).isoformat() + assert "3 days ago" in cache.get_cache_age(age_3_days) + + +class TestPRCacheCommands: + """Test PR cache CLI commands""" + + def test_pr_cache_list_command(self, runner): + """Test pr-cache list command""" + with patch("comfy_cli.command.pr_command.PRCache") as mock_cache_cls: + mock_cache = Mock() + mock_cache.list_cached_frontends.return_value = [] + mock_cache_cls.return_value = mock_cache + + result = runner.invoke(app, ["pr-cache", "list"]) + assert result.exit_code == 0 + assert "No cached PR builds found" in result.output + + def test_pr_cache_clean_command_with_confirmation(self, runner): + """Test pr-cache clean command with confirmation""" + with patch("comfy_cli.command.pr_command.PRCache") as mock_cache_cls: + mock_cache = Mock() + mock_cache.list_cached_frontends.return_value = [ + {"pr_number": 123, "pr_title": "Test PR"} # Mock some cached items + ] + mock_cache_cls.return_value = mock_cache + + # Simulate user saying "no" + result = runner.invoke(app, ["pr-cache", "clean"], input="n\n") + assert result.exit_code == 0 + assert "Cancelled" in result.output + mock_cache.clean_frontend_cache.assert_not_called() + + def test_pr_cache_clean_command_with_yes_flag(self, runner): + """Test pr-cache clean command with --yes flag""" + with patch("comfy_cli.command.pr_command.PRCache") as mock_cache_cls: + mock_cache = Mock() + mock_cache_cls.return_value = mock_cache + + result = runner.invoke(app, ["pr-cache", "clean", "--yes"]) + assert result.exit_code == 0 + mock_cache.clean_frontend_cache.assert_called_once_with()