From 1917cf585327bd21a0c11c63e292b47dc6ee4d7e Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 24 Jan 2026 05:45:09 +0000 Subject: [PATCH 1/2] Add pre-commit configuration with ruff, pyright, and other helpful hooks - Add .pre-commit-config.yaml with comprehensive linting and formatting hooks - Ruff for linting and formatting (replaces flake8, isort, black) - Pyright for type checking (Astral's type checker) - Standard pre-commit hooks for file cleanup and validation - Bandit for security checks - Markdownlint for markdown formatting - Interrogate for docstring coverage - Add root pyproject.toml for monorepo-level tool configuration - Configure ruff with sensible defaults for Python 3.8+ - Configure pyright for basic type checking - Configure bandit to skip test directories - Configure interrogate for docstring coverage reporting - Add GitHub workflow .github/workflows/pre-commit.yml - Runs on workflow_dispatch for manual triggering - Runs on push and pull requests to main branch - Uses prek instead of pre-commit binary - Caches pre-commit hooks for faster runs - Update .gitignore to exclude pre-commit cache and interrogate badge - Apply automatic formatting fixes from ruff and ruff-format to all Python files Note: Using --no-verify for initial commit. Future commits will be validated by hooks. --- .github/workflows/pre-commit.yml | 58 +++ .github/workflows/test.yml | 3 +- .gitignore | 8 +- .pre-commit-config.yaml | 74 +++ Agents.md | 7 +- README.md | 2 + pyproject.toml | 128 +++++ tools/config-utils/README.md | 11 +- tools/config-utils/cli.py | 163 +++---- .../config-utils/tests/test_set_operations.py | 453 +++++++----------- tools/locust-compare/README.md | 7 +- tools/locust-compare/compare_runs.py | 16 +- tools/locust-compare/tests/conftest.py | 8 +- .../tests/test_compare_reports.py | 24 +- .../tests/test_html_extraction.py | 2 +- .../tests/test_html_feature_map.py | 9 +- .../locust-compare/tests/test_load_report.py | 16 +- .../tests/test_markdown_output.py | 38 +- tools/locust-compare/tests/test_metrics.py | 2 +- tools/locust-compare/tests/test_row.py | 4 +- tools/locust-compare/tests/test_utils.py | 6 +- .../locust-compare/tests/test_zip_support.py | 12 +- tools/wt-worktree/PRD.md | 8 + tools/wt-worktree/README.md | 2 + tools/wt-worktree/tests/conftest.py | 5 +- tools/wt-worktree/tests/test_cli.py | 22 +- tools/wt-worktree/tests/test_config.py | 2 +- tools/wt-worktree/tests/test_git.py | 4 +- tools/wt-worktree/tests/test_worktree.py | 2 - tools/wt-worktree/wt/cli.py | 60 ++- tools/wt-worktree/wt/config.py | 21 +- tools/wt-worktree/wt/git.py | 110 +++-- tools/wt-worktree/wt/prompts.py | 2 +- tools/wt-worktree/wt/shell.py | 9 +- tools/wt-worktree/wt/worktree.py | 64 +-- 35 files changed, 793 insertions(+), 569 deletions(-) create mode 100644 .github/workflows/pre-commit.yml create mode 100644 .pre-commit-config.yaml create mode 100644 pyproject.toml diff --git a/.github/workflows/pre-commit.yml b/.github/workflows/pre-commit.yml new file mode 100644 index 0000000..01e2066 --- /dev/null +++ b/.github/workflows/pre-commit.yml @@ -0,0 +1,58 @@ +name: Pre-commit Checks + +on: + # Allow manual trigger + workflow_dispatch: + + # Run on push to main branch + push: + branches: + - main + + # Run on pull requests to main branch + pull_request: + branches: + - main + +jobs: + pre-commit: + runs-on: ubuntu-latest + + steps: + - name: Checkout code + uses: actions/checkout@v4 + + - name: Set up Python + uses: actions/setup-python@v5 + with: + python-version: '3.12' + + - name: Install uv + uses: astral-sh/setup-uv@v4 + with: + version: "latest" + + - name: Install prek + run: | + uv tool install prek + + - name: Cache pre-commit hooks + uses: actions/cache@v4 + with: + path: ~/.cache/pre-commit + key: pre-commit-${{ hashFiles('.pre-commit-config.yaml') }} + restore-keys: | + pre-commit- + + - name: Run pre-commit with prek + run: | + uv tool run prek run --all-files + + - name: Upload pre-commit results + if: always() + uses: actions/upload-artifact@v4 + with: + name: pre-commit-results + path: | + .pre-commit.log + if-no-files-found: ignore diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 9a2a052..4f0e555 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -15,7 +15,7 @@ jobs: - locust-compare - config-utils - wt-worktree - # Just test for 3.12 + # Just test for 3.12 # uncomment if you want to test for all versions # python-version: ["3.10", "3.11", "3.12"] python-version: ["3.12"] @@ -34,4 +34,3 @@ jobs: python -m pip install --upgrade pip pip install -e .[dev] python -m pytest -v - diff --git a/.gitignore b/.gitignore index 67c40a1..35012bc 100644 --- a/.gitignore +++ b/.gitignore @@ -204,6 +204,12 @@ cython_debug/ # Ruff stuff: .ruff_cache/ +# Pre-commit cache +.pre-commit-cache/ + +# Interrogate badge +interrogate_badge.svg + # PyPI configuration file .pypirc @@ -220,4 +226,4 @@ __marimo__/ .DS_Store # Locust -test_runs/ \ No newline at end of file +test_runs/ diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml new file mode 100644 index 0000000..d5383c2 --- /dev/null +++ b/.pre-commit-config.yaml @@ -0,0 +1,74 @@ +# Pre-commit hooks configuration using prek +# See https://github.com/pre-commit/pre-commit-hooks for more hooks + +repos: + # Ruff - Fast Python linter and formatter (replaces flake8, isort, black, etc.) + - repo: https://github.com/astral-sh/ruff-pre-commit + rev: v0.8.4 + hooks: + # Run the linter + - id: ruff + args: [--fix] + # Run the formatter + - id: ruff-format + + # Pyright - Astral's type checker + - repo: https://github.com/RobertCraigie/pyright-python + rev: v1.1.390 + hooks: + - id: pyright + + # Pre-commit hooks for general file cleanup + - repo: https://github.com/pre-commit/pre-commit-hooks + rev: v5.0.0 + hooks: + # Prevent giant files from being committed + - id: check-added-large-files + args: ['--maxkb=1000'] + # Check for files that would conflict in case-insensitive filesystems + - id: check-case-conflict + # Check for merge conflicts + - id: check-merge-conflict + # Check for debugger imports and py37+ breakpoint() + - id: debug-statements + # Ensure files end in a newline + - id: end-of-file-fixer + # Trim trailing whitespace + - id: trailing-whitespace + # Check YAML files for parseable syntax + - id: check-yaml + # Check TOML files for parseable syntax + - id: check-toml + # Check JSON files for parseable syntax + - id: check-json + # Ensure JSON files are properly formatted + - id: pretty-format-json + args: ['--autofix', '--no-sort-keys', '--indent=2'] + # Fix mixed line endings + - id: mixed-line-ending + args: ['--fix=lf'] + # Check for private keys + - id: detect-private-key + + # Check for common security issues + - repo: https://github.com/PyCQA/bandit + rev: 1.7.10 + hooks: + - id: bandit + args: ['-c', 'pyproject.toml'] + additional_dependencies: ['bandit[toml]'] + + # Markdown linting + - repo: https://github.com/igorshubovych/markdownlint-cli + rev: v0.43.0 + hooks: + - id: markdownlint + args: ['--fix'] + + # Python docstring coverage + - repo: https://github.com/econchick/interrogate + rev: 1.7.0 + hooks: + - id: interrogate + args: [--fail-under=0, --verbose] + pass_filenames: false diff --git a/Agents.md b/Agents.md index c65b5b4..926bfa6 100644 --- a/Agents.md +++ b/Agents.md @@ -10,11 +10,12 @@ If you need to create more tasks do it in inside PRD.md and under a story where Add appropirate test for your work. try not to use mocks unless necessary -Mark a task done in PRD after the task is completed and tests are passing +Mark a task done in PRD after the task is completed and tests are passing Build/update README.md at the end if the story is finished Your final commit should include just that folder and selected items from its contents: + - The notes.md, PRD.md and README.md files - Any code you wrote along the way - If you checked out and modified an existing repo, the output of "git diff" against that modified repo saved as a file - but not a copy of the full repo @@ -26,12 +27,14 @@ After everything is done update the root README.md with the tool's information ## Running Tests **First time setup:** + ```bash cd tools/ uv pip install -e ".[dev]" # Install with dev dependencies ``` **Run tests:** + ```bash uv run pytest tests/ -v # Verbose output uv run pytest tests/ -v --cov= # With coverage (if configured in pyproject.toml) @@ -49,5 +52,3 @@ uv run pytest tests/test_foo.py::test_bar -v # Run specific test 5. **Document learnings**: Add to notes.md as you discover gotchas or solutions 6. **Test as you go**: Run tests after each significant change, not just at the end 7. **Git config in tests**: Disable GPG signing in test fixtures: `git config commit.gpgsign false` - - diff --git a/README.md b/README.md index 5a5321e..c8dc8e9 100644 --- a/README.md +++ b/README.md @@ -17,6 +17,7 @@ Option 1: uv tool install Each tool can be installed independently using `uvx` directly from GitHub: For example: + ```bash # Install locust-compare uv tool install 'git+https://github.com/dev-ankit/python-tools.git#subdirectory=tools/locust-compare' @@ -29,6 +30,7 @@ uv tool install 'git+https://github.com/dev-ankit/python-tools.git#subdirectory= ``` After installation, you can run from anywhere: + ```bash locust-compare config-utils capture-env diff --git a/pyproject.toml b/pyproject.toml new file mode 100644 index 0000000..608fa2c --- /dev/null +++ b/pyproject.toml @@ -0,0 +1,128 @@ +# Root pyproject.toml for monorepo-level tooling configuration +[project] +name = "python-tools-monorepo" +version = "0.1.0" +description = "Monorepo containing Python CLI tools" +requires-python = ">=3.8" + +[build-system] +requires = ["hatchling"] +build-backend = "hatchling.build" + +# Ruff configuration +[tool.ruff] +# Target Python 3.8+ (compatible with all tools in the repo) +target-version = "py38" + +# Include all Python files in the tools directory +include = ["tools/**/*.py"] + +# Line length that works well for most projects +line-length = 100 + +[tool.ruff.lint] +# Enable recommended rules plus additional helpful ones +select = [ + "E", # pycodestyle errors + "W", # pycodestyle warnings + "F", # pyflakes + "I", # isort + "N", # pep8-naming + "UP", # pyupgrade + "B", # flake8-bugbear + "C4", # flake8-comprehensions + "DTZ", # flake8-datetimez + "ICN", # flake8-import-conventions + "PIE", # flake8-pie + "PT", # flake8-pytest-style + "RET", # flake8-return + "SIM", # flake8-simplify + "ARG", # flake8-unused-arguments + "PTH", # flake8-use-pathlib + "PL", # pylint + "RUF", # ruff-specific rules +] + +# Disable rules that might be too strict for CLI tools +ignore = [ + "PLR0913", # Too many arguments + "PLR2004", # Magic value comparison + "RET504", # Unnecessary variable before return +] + +# Allow autofix for all enabled rules +fixable = ["ALL"] +unfixable = [] + +[tool.ruff.lint.per-file-ignores] +# Test files can have additional flexibility +"tests/**/*.py" = [ + "ARG001", # Unused function argument (common in fixtures) + "PLR2004", # Magic values in tests are fine + "S101", # Use of assert (required for pytest) +] + +[tool.ruff.format] +# Use double quotes for strings +quote-style = "double" + +# Indent with spaces +indent-style = "space" + +# Respect magic trailing comma +skip-magic-trailing-comma = false + +# Pyright configuration +[tool.pyright] +include = ["tools"] +exclude = [ + "**/node_modules", + "**/__pycache__", + "**/.*", +] + +# Use basic type checking (not too strict for CLI tools) +typeCheckingMode = "basic" + +# Python version compatibility +pythonVersion = "3.8" +pythonPlatform = "All" + +# Report settings +reportMissingImports = true +reportMissingTypeStubs = false +reportUnusedImport = "warning" +reportUnusedVariable = "warning" +reportDuplicateImport = "warning" +reportOptionalMemberAccess = "none" # Can be noisy with Click +reportOptionalCall = "none" # Can be noisy with Click + +# Bandit security linter configuration +[tool.bandit] +exclude_dirs = [ + "tests", + ".venv", + "venv", +] +skips = [ + "B101", # assert_used - we use asserts in tests +] + +# Interrogate docstring coverage configuration +[tool.interrogate] +ignore-init-method = true +ignore-init-module = true +ignore-magic = true +ignore-semiprivate = true +ignore-private = true +ignore-property-decorators = true +ignore-module = true +ignore-nested-functions = true +ignore-nested-classes = true +fail-under = 0 +verbose = 1 +quiet = false +color = true +generate-badge = "." +badge-format = "svg" +exclude = ["tests", "docs"] diff --git a/tools/config-utils/README.md b/tools/config-utils/README.md index eba7eb9..94c0f84 100644 --- a/tools/config-utils/README.md +++ b/tools/config-utils/README.md @@ -48,7 +48,6 @@ Or install in editable mode for development: pip install -e . ``` - ## Usage ### Capture Environment Variables @@ -142,6 +141,7 @@ Perform set operations on two YAML configuration files. All operations output va #### Examples **file1.yml:** + ```yaml database: postgres port: 5432 @@ -149,6 +149,7 @@ debug: true ``` **file2.yml:** + ```yaml database: postgres port: 3306 @@ -156,6 +157,7 @@ logging: verbose ``` **Find common configuration (intersect with key-values):** + ```bash config-utils intersect file1.yml file2.yml # Output: @@ -163,6 +165,7 @@ config-utils intersect file1.yml file2.yml ``` **Find common keys regardless of values:** + ```bash config-utils intersect file1.yml file2.yml --compare keys # Output: @@ -171,6 +174,7 @@ config-utils intersect file1.yml file2.yml --compare keys ``` **Find all unique configuration (union):** + ```bash config-utils union file1.yml file2.yml # Output: @@ -181,6 +185,7 @@ config-utils union file1.yml file2.yml ``` **Find what's in file1 but not file2 (diff):** + ```bash config-utils diff file1.yml file2.yml # Output: @@ -189,6 +194,7 @@ config-utils diff file1.yml file2.yml ``` **Find what's in file2 but not file1 (rdiff):** + ```bash config-utils rdiff file1.yml file2.yml # Output: @@ -199,6 +205,7 @@ config-utils rdiff file1.yml file2.yml **Nested comparison with depth:** **nested1.yml:** + ```yaml database: host: localhost @@ -208,6 +215,7 @@ app: ``` **nested2.yml:** + ```yaml database: host: localhost @@ -295,6 +303,7 @@ python -m pytest tests/test_set_operations.py::TestUnionCommand::test_union_kv_m ### Test Coverage The test suite covers: + - All 5 set operations (union, intersect, diff, rdiff, symdiff) - Both comparison modes (keys and kv) - All depth levels (0, 1, 2+) diff --git a/tools/config-utils/cli.py b/tools/config-utils/cli.py index 5c60a7f..0e987c5 100644 --- a/tools/config-utils/cli.py +++ b/tools/config-utils/cli.py @@ -1,16 +1,19 @@ """CLI commands for config-utils.""" +import json import os -import sys -import yaml import subprocess -import json +import sys from pathlib import Path -from typing import Dict, Any, Set, Tuple +from typing import Any, Dict + import click +import yaml -def flatten_dict(data: Dict[str, Any], depth: int, parent_key: str = '', sep: str = '.') -> Dict[str, Any]: +def flatten_dict( + data: Dict[str, Any], depth: int, parent_key: str = "", sep: str = "." +) -> Dict[str, Any]: """ Flatten a nested dictionary to a specified depth. @@ -48,7 +51,7 @@ def flatten_dict(data: Dict[str, Any], depth: int, parent_key: str = '', sep: st return items -def unflatten_dict(data: Dict[str, Any], sep: str = '.') -> Dict[str, Any]: +def unflatten_dict(data: Dict[str, Any], sep: str = ".") -> Dict[str, Any]: """ Unflatten a dictionary with dot-separated keys back to nested structure. @@ -89,7 +92,7 @@ def load_yaml_file(file_path: str) -> Dict[str, Any]: SystemExit: If file not found or invalid YAML """ try: - with open(file_path, 'r') as f: + with open(file_path) as f: data = yaml.safe_load(f) or {} if not isinstance(data, dict): @@ -117,12 +120,11 @@ def make_hashable(value: Any) -> Any: """ if isinstance(value, dict): return tuple(sorted((k, make_hashable(v)) for k, v in value.items())) - elif isinstance(value, list): + if isinstance(value, list): return tuple(make_hashable(item) for item in value) - elif isinstance(value, set): + if isinstance(value, set): return frozenset(make_hashable(item) for item in value) - else: - return value + return value def perform_set_operation( @@ -130,7 +132,7 @@ def perform_set_operation( file2_data: Dict[str, Any], operation: str, compare_mode: str, - depth: int + depth: int, ) -> Dict[str, Any]: """ Perform set operations on two dictionaries. @@ -149,20 +151,20 @@ def perform_set_operation( flat1 = flatten_dict(file1_data, depth) flat2 = flatten_dict(file2_data, depth) - if compare_mode == 'keys': + if compare_mode == "keys": # Compare only keys keys1 = set(flat1.keys()) keys2 = set(flat2.keys()) - if operation == 'union': + if operation == "union": result_keys = keys1 | keys2 - elif operation == 'intersect': + elif operation == "intersect": result_keys = keys1 & keys2 - elif operation == 'diff': + elif operation == "diff": result_keys = keys1 - keys2 - elif operation == 'rdiff': + elif operation == "rdiff": result_keys = keys2 - keys1 - elif operation == 'symdiff': + elif operation == "symdiff": result_keys = keys1 ^ keys2 else: result_keys = set() @@ -181,7 +183,7 @@ def perform_set_operation( items1 = set((k, make_hashable(v)) for k, v in flat1.items()) items2 = set((k, make_hashable(v)) for k, v in flat2.items()) - if operation == 'union': + if operation == "union": # For union, we need to handle conflicts - file1 takes precedence result_items = items1 | items2 result = {} @@ -191,16 +193,16 @@ def perform_set_operation( result[key] = flat1[key] else: result[key] = flat2[key] - elif operation == 'intersect': + elif operation == "intersect": result_items = items1 & items2 result = {k: flat1[k] for k, _ in result_items} - elif operation == 'diff': + elif operation == "diff": result_items = items1 - items2 result = {k: flat1[k] for k, _ in result_items} - elif operation == 'rdiff': + elif operation == "rdiff": result_items = items2 - items1 result = {k: flat2[k] for k, _ in result_items} - elif operation == 'symdiff': + elif operation == "symdiff": result_items = items1 ^ items2 result = {} for key, _ in result_items: @@ -224,23 +226,22 @@ def perform_set_operation( @click.version_option() def main(): """config-utils: Capture environment variables, Django settings, and perform YAML set operations.""" - pass @main.command() @click.option( - '--output', - '-o', - default='env_config.yaml', - help='Output file path (default: env_config.yaml)', + "--output", + "-o", + default="env_config.yaml", + help="Output file path (default: env_config.yaml)", type=click.Path(), ) @click.option( - '--format', - '-f', - type=click.Choice(['yaml', 'yml'], case_sensitive=False), - default='yaml', - help='Output format (default: yaml)', + "--format", + "-f", + type=click.Choice(["yaml", "yml"], case_sensitive=False), + default="yaml", + help="Output format (default: yaml)", ) def capture_env(output, format): """Capture all environment variables and store them in YAML format.""" @@ -252,43 +253,43 @@ def capture_env(output, format): output_path = Path(output) # Write to YAML file - with open(output_path, 'w') as f: + with open(output_path, "w") as f: yaml.dump(env_vars, f, default_flow_style=False, sort_keys=True) click.echo(f"✓ Captured {len(env_vars)} environment variables to {output_path}") except Exception as e: - click.echo(f"✗ Error: {str(e)}", err=True) + click.echo(f"✗ Error: {e!s}", err=True) sys.exit(1) @main.command() @click.option( - '--output', - '-o', - default='django_settings.yaml', - help='Output file path (default: django_settings.yaml)', + "--output", + "-o", + default="django_settings.yaml", + help="Output file path (default: django_settings.yaml)", type=click.Path(), ) @click.option( - '--format', - '-f', - type=click.Choice(['yaml', 'yml'], case_sensitive=False), - default='yaml', - help='Output format (default: yaml)', + "--format", + "-f", + type=click.Choice(["yaml", "yml"], case_sensitive=False), + default="yaml", + help="Output format (default: yaml)", ) @click.option( - '--manage-py', - '-m', - default='manage.py', - help='Path to manage.py (default: manage.py)', + "--manage-py", + "-m", + default="manage.py", + help="Path to manage.py (default: manage.py)", type=click.Path(exists=True), ) @click.option( - '--settings', - '-s', - help='Django settings module (e.g., myproject.settings)', - envvar='DJANGO_SETTINGS_MODULE', + "--settings", + "-s", + help="Django settings module (e.g., myproject.settings)", + envvar="DJANGO_SETTINGS_MODULE", ) def capture_django_settings(output, format, manage_py, settings): """Capture Django settings and store them in YAML format. @@ -303,7 +304,7 @@ def capture_django_settings(output, format, manage_py, settings): click.echo( f"✗ Error: manage.py not found at {manage_path}. " "Run this command from your Django project root or use --manage-py to specify the path.", - err=True + err=True, ) sys.exit(1) @@ -330,20 +331,21 @@ def capture_django_settings(output, format, manage_py, settings): # Prepare environment variables env = os.environ.copy() if settings: - env['DJANGO_SETTINGS_MODULE'] = settings + env["DJANGO_SETTINGS_MODULE"] = settings # Run manage.py shell with the script result = subprocess.run( - ['python', str(manage_path), 'shell'], + ["python", str(manage_path), "shell"], input=django_script, capture_output=True, text=True, env=env, - timeout=30 + timeout=30, + check=False, ) if result.returncode != 0: - click.echo(f"✗ Error running Django shell:", err=True) + click.echo("✗ Error running Django shell:", err=True) click.echo(result.stderr, err=True) sys.exit(1) @@ -351,7 +353,7 @@ def capture_django_settings(output, format, manage_py, settings): try: settings_dict = json.loads(result.stdout.strip()) except json.JSONDecodeError: - click.echo(f"✗ Error: Could not parse Django settings output", err=True) + click.echo("✗ Error: Could not parse Django settings output", err=True) click.echo(f"Output: {result.stdout}", err=True) sys.exit(1) @@ -359,7 +361,7 @@ def capture_django_settings(output, format, manage_py, settings): output_path = Path(output) # Write to YAML file - with open(output_path, 'w') as f: + with open(output_path, "w") as f: yaml.dump(settings_dict, f, default_flow_style=False, sort_keys=True) click.echo(f"✓ Captured {len(settings_dict)} Django settings to {output_path}") @@ -368,27 +370,28 @@ def capture_django_settings(output, format, manage_py, settings): click.echo("✗ Error: Django shell command timed out", err=True) sys.exit(1) except Exception as e: - click.echo(f"✗ Error: {str(e)}", err=True) + click.echo(f"✗ Error: {e!s}", err=True) sys.exit(1) # Set operation commands def create_set_operation_command(operation: str, description: str): """Factory function to create set operation commands.""" + @main.command(name=operation, help=description) - @click.argument('file1', type=click.Path(exists=True)) - @click.argument('file2', type=click.Path(exists=True)) + @click.argument("file1", type=click.Path(exists=True)) + @click.argument("file2", type=click.Path(exists=True)) @click.option( - '--compare', - type=click.Choice(['keys', 'kv'], case_sensitive=False), - default='kv', - help='Comparison mode: keys or kv (key-values). Default: kv', + "--compare", + type=click.Choice(["keys", "kv"], case_sensitive=False), + default="kv", + help="Comparison mode: keys or kv (key-values). Default: kv", ) @click.option( - '--depth', + "--depth", type=int, default=1, - help='How many levels deep to compare. 1 = root keys only, 0 = unlimited (full depth). Default: 1', + help="How many levels deep to compare. 1 = root keys only, 0 = unlimited (full depth). Default: 1", ) def command(file1, file2, compare, depth): try: @@ -400,10 +403,12 @@ def command(file1, file2, compare, depth): result = perform_set_operation(file1_data, file2_data, operation, compare, depth) # Output result as YAML to stdout - yaml.dump(result, sys.stdout, default_flow_style=False, sort_keys=False, allow_unicode=True) + yaml.dump( + result, sys.stdout, default_flow_style=False, sort_keys=False, allow_unicode=True + ) except Exception as e: - click.echo(f"Error: {str(e)}", err=True) + click.echo(f"Error: {e!s}", err=True) sys.exit(1) return command @@ -411,30 +416,26 @@ def command(file1, file2, compare, depth): # Create all set operation commands union_cmd = create_set_operation_command( - 'union', - 'Returns all keys (or key-value pairs) present in either file.' + "union", "Returns all keys (or key-value pairs) present in either file." ) intersect_cmd = create_set_operation_command( - 'intersect', - 'Returns only keys (or key-value pairs) present in both files.' + "intersect", "Returns only keys (or key-value pairs) present in both files." ) diff_cmd = create_set_operation_command( - 'diff', - 'Returns keys (or key-value pairs) in file1 but not in file2 (A - B).' + "diff", "Returns keys (or key-value pairs) in file1 but not in file2 (A - B)." ) rdiff_cmd = create_set_operation_command( - 'rdiff', - 'Returns keys (or key-value pairs) in file2 but not in file1 (B - A).' + "rdiff", "Returns keys (or key-value pairs) in file2 but not in file1 (B - A)." ) symdiff_cmd = create_set_operation_command( - 'symdiff', - 'Returns keys (or key-value pairs) in either file but not in both (symmetric difference).' + "symdiff", + "Returns keys (or key-value pairs) in either file but not in both (symmetric difference).", ) -if __name__ == '__main__': +if __name__ == "__main__": main() diff --git a/tools/config-utils/tests/test_set_operations.py b/tools/config-utils/tests/test_set_operations.py index d73f864..5c31234 100644 --- a/tools/config-utils/tests/test_set_operations.py +++ b/tools/config-utils/tests/test_set_operations.py @@ -1,12 +1,20 @@ """Tests for YAML set operations.""" -import pytest -import yaml -import tempfile import os +import tempfile from pathlib import Path + +import pytest +import yaml +from cli import ( + flatten_dict, + load_yaml_file, + main, + make_hashable, + perform_set_operation, + unflatten_dict, +) from click.testing import CliRunner -from cli import main, flatten_dict, unflatten_dict, perform_set_operation, load_yaml_file, make_hashable @pytest.fixture @@ -21,40 +29,16 @@ def temp_yaml_files(): files = {} # Simple file 1 - file1_data = { - 'database': 'postgres', - 'port': 5432, - 'debug': True - } + file1_data = {"database": "postgres", "port": 5432, "debug": True} # Simple file 2 - file2_data = { - 'database': 'postgres', - 'port': 3306, - 'logging': 'verbose' - } + file2_data = {"database": "postgres", "port": 3306, "logging": "verbose"} # Nested file 1 - nested1_data = { - 'database': { - 'host': 'localhost', - 'port': 5432 - }, - 'app': { - 'name': 'myapp' - } - } + nested1_data = {"database": {"host": "localhost", "port": 5432}, "app": {"name": "myapp"}} # Nested file 2 - nested2_data = { - 'database': { - 'host': 'localhost', - 'port': 3306 - }, - 'app': { - 'name': 'myapp' - } - } + nested2_data = {"database": {"host": "localhost", "port": 3306}, "app": {"name": "myapp"}} # Empty file empty_data = {} @@ -64,52 +48,52 @@ def temp_yaml_files(): with tempfile.TemporaryDirectory() as tmpdir: # Create file 1 - file1 = Path(tmpdir) / 'file1.yml' - with open(file1, 'w') as f: + file1 = Path(tmpdir) / "file1.yml" + with open(file1, "w") as f: yaml.dump(file1_data, f) - files['file1'] = str(file1) + files["file1"] = str(file1) # Create file 2 - file2 = Path(tmpdir) / 'file2.yml' - with open(file2, 'w') as f: + file2 = Path(tmpdir) / "file2.yml" + with open(file2, "w") as f: yaml.dump(file2_data, f) - files['file2'] = str(file2) + files["file2"] = str(file2) # Create nested file 1 - nested1 = Path(tmpdir) / 'nested1.yml' - with open(nested1, 'w') as f: + nested1 = Path(tmpdir) / "nested1.yml" + with open(nested1, "w") as f: yaml.dump(nested1_data, f) - files['nested1'] = str(nested1) + files["nested1"] = str(nested1) # Create nested file 2 - nested2 = Path(tmpdir) / 'nested2.yml' - with open(nested2, 'w') as f: + nested2 = Path(tmpdir) / "nested2.yml" + with open(nested2, "w") as f: yaml.dump(nested2_data, f) - files['nested2'] = str(nested2) + files["nested2"] = str(nested2) # Create empty file - empty = Path(tmpdir) / 'empty.yml' - with open(empty, 'w') as f: + empty = Path(tmpdir) / "empty.yml" + with open(empty, "w") as f: yaml.dump(empty_data, f) - files['empty'] = str(empty) + files["empty"] = str(empty) # Create identical file - identical = Path(tmpdir) / 'identical.yml' - with open(identical, 'w') as f: + identical = Path(tmpdir) / "identical.yml" + with open(identical, "w") as f: yaml.dump(identical_data, f) - files['identical'] = str(identical) + files["identical"] = str(identical) # Create invalid YAML file - invalid = Path(tmpdir) / 'invalid.yml' - with open(invalid, 'w') as f: - f.write('invalid: yaml: content:\n bad syntax') - files['invalid'] = str(invalid) + invalid = Path(tmpdir) / "invalid.yml" + with open(invalid, "w") as f: + f.write("invalid: yaml: content:\n bad syntax") + files["invalid"] = str(invalid) # Create non-dict file (list) - list_file = Path(tmpdir) / 'list.yml' - with open(list_file, 'w') as f: - yaml.dump(['item1', 'item2'], f) - files['list'] = str(list_file) + list_file = Path(tmpdir) / "list.yml" + with open(list_file, "w") as f: + yaml.dump(["item1", "item2"], f) + files["list"] = str(list_file) yield files @@ -119,50 +103,22 @@ class TestFlattenDict: def test_flatten_depth_1(self): """Test flattening with depth 1 (no flattening).""" - data = { - 'database': { - 'host': 'localhost', - 'port': 5432 - }, - 'app': { - 'name': 'myapp' - } - } + data = {"database": {"host": "localhost", "port": 5432}, "app": {"name": "myapp"}} result = flatten_dict(data, depth=1) assert result == data def test_flatten_depth_2(self): """Test flattening with depth 2.""" - data = { - 'database': { - 'host': 'localhost', - 'port': 5432 - }, - 'app': { - 'name': 'myapp' - } - } + data = {"database": {"host": "localhost", "port": 5432}, "app": {"name": "myapp"}} result = flatten_dict(data, depth=2) - expected = { - 'database.host': 'localhost', - 'database.port': 5432, - 'app.name': 'myapp' - } + expected = {"database.host": "localhost", "database.port": 5432, "app.name": "myapp"} assert result == expected def test_flatten_depth_0(self): """Test flattening with depth 0 (unlimited).""" - data = { - 'level1': { - 'level2': { - 'level3': 'value' - } - } - } + data = {"level1": {"level2": {"level3": "value"}}} result = flatten_dict(data, depth=0) - expected = { - 'level1.level2.level3': 'value' - } + expected = {"level1.level2.level3": "value"} assert result == expected @@ -171,21 +127,9 @@ class TestUnflattenDict: def test_unflatten_simple(self): """Test unflattening a simple dict.""" - data = { - 'database.host': 'localhost', - 'database.port': 5432, - 'app.name': 'myapp' - } + data = {"database.host": "localhost", "database.port": 5432, "app.name": "myapp"} result = unflatten_dict(data) - expected = { - 'database': { - 'host': 'localhost', - 'port': 5432 - }, - 'app': { - 'name': 'myapp' - } - } + expected = {"database": {"host": "localhost", "port": 5432}, "app": {"name": "myapp"}} assert result == expected @@ -194,10 +138,10 @@ class TestMakeHashable: def test_hashable_dict(self): """Test making a dict hashable.""" - data = {'a': 1, 'b': 2} + data = {"a": 1, "b": 2} result = make_hashable(data) assert isinstance(result, tuple) - assert result == (('a', 1), ('b', 2)) + assert result == (("a", 1), ("b", 2)) def test_hashable_list(self): """Test making a list hashable.""" @@ -208,7 +152,7 @@ def test_hashable_list(self): def test_hashable_nested(self): """Test making nested structures hashable.""" - data = {'a': [1, 2], 'b': {'c': 3}} + data = {"a": [1, 2], "b": {"c": 3}} result = make_hashable(data) assert isinstance(result, tuple) @@ -218,32 +162,25 @@ class TestUnionCommand: def test_union_kv_mode(self, runner, temp_yaml_files): """Test union with key-value comparison.""" - result = runner.invoke(main, [ - 'union', - temp_yaml_files['file1'], - temp_yaml_files['file2'] - ]) + result = runner.invoke(main, ["union", temp_yaml_files["file1"], temp_yaml_files["file2"]]) assert result.exit_code == 0 output_data = yaml.safe_load(result.output) - assert output_data['database'] == 'postgres' - assert output_data['port'] == 5432 # file1 takes precedence - assert output_data['debug'] is True - assert output_data['logging'] == 'verbose' + assert output_data["database"] == "postgres" + assert output_data["port"] == 5432 # file1 takes precedence + assert output_data["debug"] is True + assert output_data["logging"] == "verbose" def test_union_keys_mode(self, runner, temp_yaml_files): """Test union with keys-only comparison.""" - result = runner.invoke(main, [ - 'union', - temp_yaml_files['file1'], - temp_yaml_files['file2'], - '--compare', 'keys' - ]) + result = runner.invoke( + main, ["union", temp_yaml_files["file1"], temp_yaml_files["file2"], "--compare", "keys"] + ) assert result.exit_code == 0 output_data = yaml.safe_load(result.output) - assert output_data['database'] == 'postgres' - assert output_data['port'] == 5432 # file1 value - assert output_data['debug'] is True - assert output_data['logging'] == 'verbose' + assert output_data["database"] == "postgres" + assert output_data["port"] == 5432 # file1 value + assert output_data["debug"] is True + assert output_data["logging"] == "verbose" class TestIntersectCommand: @@ -251,70 +188,64 @@ class TestIntersectCommand: def test_intersect_kv_mode(self, runner, temp_yaml_files): """Test intersect with key-value comparison.""" - result = runner.invoke(main, [ - 'intersect', - temp_yaml_files['file1'], - temp_yaml_files['file2'] - ]) + result = runner.invoke( + main, ["intersect", temp_yaml_files["file1"], temp_yaml_files["file2"]] + ) assert result.exit_code == 0 output_data = yaml.safe_load(result.output) - assert output_data == {'database': 'postgres'} + assert output_data == {"database": "postgres"} def test_intersect_keys_mode(self, runner, temp_yaml_files): """Test intersect with keys-only comparison.""" - result = runner.invoke(main, [ - 'intersect', - temp_yaml_files['file1'], - temp_yaml_files['file2'], - '--compare', 'keys' - ]) + result = runner.invoke( + main, + ["intersect", temp_yaml_files["file1"], temp_yaml_files["file2"], "--compare", "keys"], + ) assert result.exit_code == 0 output_data = yaml.safe_load(result.output) - assert output_data['database'] == 'postgres' - assert output_data['port'] == 5432 # file1 value + assert output_data["database"] == "postgres" + assert output_data["port"] == 5432 # file1 value def test_intersect_depth_1(self, runner, temp_yaml_files): """Test intersect with depth 1 (root keys only).""" - result = runner.invoke(main, [ - 'intersect', - temp_yaml_files['nested1'], - temp_yaml_files['nested2'], - '--depth', '1', - '--compare', 'keys' - ]) + result = runner.invoke( + main, + [ + "intersect", + temp_yaml_files["nested1"], + temp_yaml_files["nested2"], + "--depth", + "1", + "--compare", + "keys", + ], + ) assert result.exit_code == 0 output_data = yaml.safe_load(result.output) - assert 'database' in output_data - assert 'app' in output_data - assert output_data['database']['port'] == 5432 # file1 value + assert "database" in output_data + assert "app" in output_data + assert output_data["database"]["port"] == 5432 # file1 value def test_intersect_depth_2(self, runner, temp_yaml_files): """Test intersect with depth 2 (nested keys).""" - result = runner.invoke(main, [ - 'intersect', - temp_yaml_files['nested1'], - temp_yaml_files['nested2'], - '--depth', '2' - ]) + result = runner.invoke( + main, + ["intersect", temp_yaml_files["nested1"], temp_yaml_files["nested2"], "--depth", "2"], + ) assert result.exit_code == 0 output_data = yaml.safe_load(result.output) - assert output_data == { - 'database': {'host': 'localhost'}, - 'app': {'name': 'myapp'} - } + assert output_data == {"database": {"host": "localhost"}, "app": {"name": "myapp"}} def test_intersect_depth_0(self, runner, temp_yaml_files): """Test intersect with depth 0 (unlimited).""" - result = runner.invoke(main, [ - 'intersect', - temp_yaml_files['nested1'], - temp_yaml_files['nested2'], - '--depth', '0' - ]) + result = runner.invoke( + main, + ["intersect", temp_yaml_files["nested1"], temp_yaml_files["nested2"], "--depth", "0"], + ) assert result.exit_code == 0 output_data = yaml.safe_load(result.output) # Should have flattened keys - assert 'app.name' in output_data or 'app' in output_data + assert "app.name" in output_data or "app" in output_data class TestDiffCommand: @@ -322,26 +253,19 @@ class TestDiffCommand: def test_diff_kv_mode(self, runner, temp_yaml_files): """Test diff with key-value comparison.""" - result = runner.invoke(main, [ - 'diff', - temp_yaml_files['file1'], - temp_yaml_files['file2'] - ]) + result = runner.invoke(main, ["diff", temp_yaml_files["file1"], temp_yaml_files["file2"]]) assert result.exit_code == 0 output_data = yaml.safe_load(result.output) - assert output_data == {'port': 5432, 'debug': True} + assert output_data == {"port": 5432, "debug": True} def test_diff_keys_mode(self, runner, temp_yaml_files): """Test diff with keys-only comparison.""" - result = runner.invoke(main, [ - 'diff', - temp_yaml_files['file1'], - temp_yaml_files['file2'], - '--compare', 'keys' - ]) + result = runner.invoke( + main, ["diff", temp_yaml_files["file1"], temp_yaml_files["file2"], "--compare", "keys"] + ) assert result.exit_code == 0 output_data = yaml.safe_load(result.output) - assert output_data == {'debug': True} + assert output_data == {"debug": True} class TestRdiffCommand: @@ -349,26 +273,19 @@ class TestRdiffCommand: def test_rdiff_kv_mode(self, runner, temp_yaml_files): """Test rdiff with key-value comparison.""" - result = runner.invoke(main, [ - 'rdiff', - temp_yaml_files['file1'], - temp_yaml_files['file2'] - ]) + result = runner.invoke(main, ["rdiff", temp_yaml_files["file1"], temp_yaml_files["file2"]]) assert result.exit_code == 0 output_data = yaml.safe_load(result.output) - assert output_data == {'port': 3306, 'logging': 'verbose'} + assert output_data == {"port": 3306, "logging": "verbose"} def test_rdiff_keys_mode(self, runner, temp_yaml_files): """Test rdiff with keys-only comparison.""" - result = runner.invoke(main, [ - 'rdiff', - temp_yaml_files['file1'], - temp_yaml_files['file2'], - '--compare', 'keys' - ]) + result = runner.invoke( + main, ["rdiff", temp_yaml_files["file1"], temp_yaml_files["file2"], "--compare", "keys"] + ) assert result.exit_code == 0 output_data = yaml.safe_load(result.output) - assert output_data == {'logging': 'verbose'} + assert output_data == {"logging": "verbose"} class TestSymdiffCommand: @@ -376,30 +293,26 @@ class TestSymdiffCommand: def test_symdiff_kv_mode(self, runner, temp_yaml_files): """Test symdiff with key-value comparison.""" - result = runner.invoke(main, [ - 'symdiff', - temp_yaml_files['file1'], - temp_yaml_files['file2'] - ]) + result = runner.invoke( + main, ["symdiff", temp_yaml_files["file1"], temp_yaml_files["file2"]] + ) assert result.exit_code == 0 output_data = yaml.safe_load(result.output) # Should have port from both files, debug, and logging - assert 'debug' in output_data - assert 'logging' in output_data - assert 'port' in output_data + assert "debug" in output_data + assert "logging" in output_data + assert "port" in output_data def test_symdiff_keys_mode(self, runner, temp_yaml_files): """Test symdiff with keys-only comparison.""" - result = runner.invoke(main, [ - 'symdiff', - temp_yaml_files['file1'], - temp_yaml_files['file2'], - '--compare', 'keys' - ]) + result = runner.invoke( + main, + ["symdiff", temp_yaml_files["file1"], temp_yaml_files["file2"], "--compare", "keys"], + ) assert result.exit_code == 0 output_data = yaml.safe_load(result.output) # Only keys that are not in both - assert output_data == {'debug': True, 'logging': 'verbose'} + assert output_data == {"debug": True, "logging": "verbose"} class TestErrorHandling: @@ -407,32 +320,26 @@ class TestErrorHandling: def test_file_not_found(self, runner, temp_yaml_files): """Test error when file is not found.""" - result = runner.invoke(main, [ - 'intersect', - '/nonexistent/file.yml', - temp_yaml_files['file1'] - ]) + result = runner.invoke( + main, ["intersect", "/nonexistent/file.yml", temp_yaml_files["file1"]] + ) assert result.exit_code == 2 # Click's error code for invalid argument def test_invalid_yaml(self, runner, temp_yaml_files): """Test error when YAML is invalid.""" - result = runner.invoke(main, [ - 'intersect', - temp_yaml_files['invalid'], - temp_yaml_files['file1'] - ]) + result = runner.invoke( + main, ["intersect", temp_yaml_files["invalid"], temp_yaml_files["file1"]] + ) assert result.exit_code == 1 - assert 'Error' in result.output + assert "Error" in result.output def test_non_dict_root(self, runner, temp_yaml_files): """Test error when root is not a dict.""" - result = runner.invoke(main, [ - 'intersect', - temp_yaml_files['list'], - temp_yaml_files['file1'] - ]) + result = runner.invoke( + main, ["intersect", temp_yaml_files["list"], temp_yaml_files["file1"]] + ) assert result.exit_code == 1 - assert 'Root must be a YAML mapping' in result.output + assert "Root must be a YAML mapping" in result.output class TestEdgeCases: @@ -440,37 +347,29 @@ class TestEdgeCases: def test_empty_file(self, runner, temp_yaml_files): """Test with empty file.""" - result = runner.invoke(main, [ - 'union', - temp_yaml_files['empty'], - temp_yaml_files['file1'] - ]) + result = runner.invoke(main, ["union", temp_yaml_files["empty"], temp_yaml_files["file1"]]) assert result.exit_code == 0 output_data = yaml.safe_load(result.output) # Should return all from file1 - assert output_data['database'] == 'postgres' + assert output_data["database"] == "postgres" def test_identical_files_intersect(self, runner, temp_yaml_files): """Test intersect with identical files.""" - result = runner.invoke(main, [ - 'intersect', - temp_yaml_files['file1'], - temp_yaml_files['identical'] - ]) + result = runner.invoke( + main, ["intersect", temp_yaml_files["file1"], temp_yaml_files["identical"]] + ) assert result.exit_code == 0 output_data = yaml.safe_load(result.output) # Should return full file - assert output_data['database'] == 'postgres' - assert output_data['port'] == 5432 - assert output_data['debug'] is True + assert output_data["database"] == "postgres" + assert output_data["port"] == 5432 + assert output_data["debug"] is True def test_identical_files_diff(self, runner, temp_yaml_files): """Test diff with identical files.""" - result = runner.invoke(main, [ - 'diff', - temp_yaml_files['file1'], - temp_yaml_files['identical'] - ]) + result = runner.invoke( + main, ["diff", temp_yaml_files["file1"], temp_yaml_files["identical"]] + ) assert result.exit_code == 0 output_data = yaml.safe_load(result.output) # Should return empty or None @@ -479,16 +378,12 @@ def test_identical_files_diff(self, runner, temp_yaml_files): def test_no_matches_intersect(self, runner, temp_yaml_files): """Test intersect with no matching keys.""" # Create a file with completely different keys - with tempfile.NamedTemporaryFile(mode='w', suffix='.yml', delete=False) as f: - yaml.dump({'different': 'value', 'other': 'data'}, f) + with tempfile.NamedTemporaryFile(mode="w", suffix=".yml", delete=False) as f: + yaml.dump({"different": "value", "other": "data"}, f) different_file = f.name try: - result = runner.invoke(main, [ - 'intersect', - temp_yaml_files['file1'], - different_file - ]) + result = runner.invoke(main, ["intersect", temp_yaml_files["file1"], different_file]) assert result.exit_code == 0 output_data = yaml.safe_load(result.output) # Should return empty @@ -502,31 +397,31 @@ class TestLoadYamlFile: def test_load_valid_file(self, temp_yaml_files): """Test loading a valid YAML file.""" - data = load_yaml_file(temp_yaml_files['file1']) - assert data['database'] == 'postgres' - assert data['port'] == 5432 + data = load_yaml_file(temp_yaml_files["file1"]) + assert data["database"] == "postgres" + assert data["port"] == 5432 def test_load_empty_file(self, temp_yaml_files): """Test loading an empty YAML file.""" - data = load_yaml_file(temp_yaml_files['empty']) + data = load_yaml_file(temp_yaml_files["empty"]) assert data == {} def test_load_nonexistent_file(self): """Test loading a nonexistent file.""" with pytest.raises(SystemExit) as exc_info: - load_yaml_file('/nonexistent/file.yml') + load_yaml_file("/nonexistent/file.yml") assert exc_info.value.code == 1 def test_load_invalid_yaml(self, temp_yaml_files): """Test loading an invalid YAML file.""" with pytest.raises(SystemExit) as exc_info: - load_yaml_file(temp_yaml_files['invalid']) + load_yaml_file(temp_yaml_files["invalid"]) assert exc_info.value.code == 1 def test_load_non_dict_root(self, temp_yaml_files): """Test loading a YAML file with non-dict root.""" with pytest.raises(SystemExit) as exc_info: - load_yaml_file(temp_yaml_files['list']) + load_yaml_file(temp_yaml_files["list"]) assert exc_info.value.code == 1 @@ -535,35 +430,35 @@ class TestPerformSetOperation: def test_union_operation(self): """Test union operation.""" - file1 = {'a': 1, 'b': 2} - file2 = {'b': 3, 'c': 4} - result = perform_set_operation(file1, file2, 'union', 'kv', 1) - assert result == {'a': 1, 'b': 2, 'c': 4} + file1 = {"a": 1, "b": 2} + file2 = {"b": 3, "c": 4} + result = perform_set_operation(file1, file2, "union", "kv", 1) + assert result == {"a": 1, "b": 2, "c": 4} def test_intersect_operation(self): """Test intersect operation.""" - file1 = {'a': 1, 'b': 2, 'c': 3} - file2 = {'b': 2, 'c': 4, 'd': 5} - result = perform_set_operation(file1, file2, 'intersect', 'kv', 1) - assert result == {'b': 2} + file1 = {"a": 1, "b": 2, "c": 3} + file2 = {"b": 2, "c": 4, "d": 5} + result = perform_set_operation(file1, file2, "intersect", "kv", 1) + assert result == {"b": 2} def test_diff_operation(self): """Test diff operation.""" - file1 = {'a': 1, 'b': 2, 'c': 3} - file2 = {'b': 2, 'c': 4} - result = perform_set_operation(file1, file2, 'diff', 'kv', 1) - assert result == {'a': 1, 'c': 3} + file1 = {"a": 1, "b": 2, "c": 3} + file2 = {"b": 2, "c": 4} + result = perform_set_operation(file1, file2, "diff", "kv", 1) + assert result == {"a": 1, "c": 3} def test_rdiff_operation(self): """Test rdiff operation.""" - file1 = {'a': 1, 'b': 2} - file2 = {'b': 2, 'c': 4} - result = perform_set_operation(file1, file2, 'rdiff', 'kv', 1) - assert result == {'c': 4} + file1 = {"a": 1, "b": 2} + file2 = {"b": 2, "c": 4} + result = perform_set_operation(file1, file2, "rdiff", "kv", 1) + assert result == {"c": 4} def test_symdiff_operation(self): """Test symdiff operation.""" - file1 = {'a': 1, 'b': 2} - file2 = {'b': 2, 'c': 4} - result = perform_set_operation(file1, file2, 'symdiff', 'kv', 1) - assert result == {'a': 1, 'c': 4} + file1 = {"a": 1, "b": 2} + file2 = {"b": 2, "c": 4} + result = perform_set_operation(file1, file2, "symdiff", "kv", 1) + assert result == {"a": 1, "c": 4} diff --git a/tools/locust-compare/README.md b/tools/locust-compare/README.md index 586dd0e..112911b 100644 --- a/tools/locust-compare/README.md +++ b/tools/locust-compare/README.md @@ -88,11 +88,13 @@ Exit code is `0` on success and `1` on error. ## What It Compares From CSV `report.csv` (Aggregated and each request row): + - Requests/s, Request Count, Failure Count - Average, Median, Min, Max Response Time - Percentiles: 50%, 66%, 75%, 80%, 90%, 95%, 98%, 99%, 99.9%, 99.99%, 100% (if present) From HTML feature pages (last entry in `window.templateArgs.history`): + - Requests/s (`current_rps`) - Average Response Time (`total_avg_response_time`) - 50% (`response_time_percentile_0.5`) @@ -104,7 +106,6 @@ If a metric is not available for an item, it is shown as `-`. image - ## Markdown Output Example The `-o markdown` flag produces markdown tables with emoji indicators for verdicts: @@ -122,11 +123,11 @@ The `-o markdown` flag produces markdown tables with emoji indicators for verdic ``` Verdict emojis: + - ✅ Better performance -- ❌ Worse performance +- ❌ Worse performance - ➖ No change - ## JSON Schema The `-o json` output is a single JSON object containing keys for each compared item. diff --git a/tools/locust-compare/compare_runs.py b/tools/locust-compare/compare_runs.py index b08fad9..b55615c 100644 --- a/tools/locust-compare/compare_runs.py +++ b/tools/locust-compare/compare_runs.py @@ -290,7 +290,9 @@ def _parse_html_endpoint_metrics( return data -def _compute_duration_seconds(start_dt: Optional[datetime], end_dt: Optional[datetime]) -> Optional[float]: +def _compute_duration_seconds( + start_dt: Optional[datetime], end_dt: Optional[datetime] +) -> Optional[float]: """Compute test duration in seconds from start and end timestamps.""" if not start_dt or not end_dt: return None @@ -423,7 +425,9 @@ def _metric_direction(metric: str) -> str: return "neutral" -def _verdict_for(metric: str, base_val: Optional[float], curr_val: Optional[float]) -> Optional[str]: +def _verdict_for( + metric: str, base_val: Optional[float], curr_val: Optional[float] +) -> Optional[str]: """Determine the verdict (better/worse/same) for a metric comparison.""" if base_val is None or curr_val is None: return None @@ -472,8 +476,7 @@ def _get_comparison_fields( fields = important_fields[:] extra_percentiles = [ - k for k in (curr_data.keys() | base_data.keys()) - if k.endswith("%") and k not in fields + k for k in (curr_data.keys() | base_data.keys()) if k.endswith("%") and k not in fields ] fields.extend(sorted(extra_percentiles)) return fields @@ -556,10 +559,7 @@ def render_comparison( rows = _build_comparison_rows(base_row, curr_row, fields, show_verdict, use_emoji=False) # Calculate column widths - widths = [ - max(len(headers[i]), *(len(row[i]) for row in rows)) - for i in range(len(headers)) - ] + widths = [max(len(headers[i]), *(len(row[i]) for row in rows)) for i in range(len(headers))] # Print header header_line = " ".join(h.ljust(widths[i]) for i, h in enumerate(headers)) diff --git a/tools/locust-compare/tests/conftest.py b/tools/locust-compare/tests/conftest.py index 5f3d8b4..a92e3c2 100644 --- a/tools/locust-compare/tests/conftest.py +++ b/tools/locust-compare/tests/conftest.py @@ -1,8 +1,10 @@ """Pytest fixtures for locust-compare tests.""" -import pytest -from pathlib import Path -import tempfile + import shutil +import tempfile +from pathlib import Path + +import pytest @pytest.fixture diff --git a/tools/locust-compare/tests/test_compare_reports.py b/tools/locust-compare/tests/test_compare_reports.py index 54cd875..c347334 100644 --- a/tools/locust-compare/tests/test_compare_reports.py +++ b/tools/locust-compare/tests/test_compare_reports.py @@ -1,11 +1,11 @@ """Integration tests for compare_reports function.""" -import pytest -import sys + import json +import sys import tempfile from pathlib import Path -from io import StringIO -from unittest.mock import patch + +import pytest sys.path.insert(0, str(Path(__file__).parent.parent)) from compare_runs import compare_reports @@ -100,8 +100,8 @@ def test_human_output_no_verdict(self, temp_test_dir, temp_test_dir_v2, capsys): output = captured.out # Should NOT have verdict column header - lines = output.split('\n') - header_line = [l for l in lines if 'Metric' in l and 'Base' in l][0] + lines = output.split("\n") + header_line = [l for l in lines if "Metric" in l and "Base" in l][0] assert "Verdict" not in header_line def test_human_output_colorize(self, temp_test_dir, temp_test_dir_v2, capsys): @@ -133,7 +133,10 @@ def test_compare_same_report(self, temp_test_dir, capsys): def test_compare_with_missing_endpoint(self, sample_csv_content, sample_csv_content_v2, capsys): """Test comparison when an endpoint exists only in one report.""" # Create CSV with extra endpoint in base - base_csv = sample_csv_content + "DELETE,/api/delete,10,0,50,55,20,100,1.0,5.0,0.0,50,55,60,65,75,85,95,100,150,200,100\n" + base_csv = ( + sample_csv_content + + "DELETE,/api/delete,10,0,50,55,20,100,1.0,5.0,0.0,50,55,60,65,75,85,95,100,150,200,100\n" + ) with tempfile.TemporaryDirectory() as base_dir, tempfile.TemporaryDirectory() as curr_dir: base_path = Path(base_dir) / "report.csv" @@ -156,7 +159,10 @@ def test_compare_with_missing_endpoint(self, sample_csv_content, sample_csv_cont def test_compare_with_new_endpoint(self, sample_csv_content, sample_csv_content_v2, capsys): """Test comparison when a new endpoint appears in current.""" # Create CSV with extra endpoint in current - curr_csv = sample_csv_content_v2 + "DELETE,/api/new,20,0,40,45,15,90,1.2,10.0,0.0,40,45,50,55,65,75,85,95,140,190,90\n" + curr_csv = ( + sample_csv_content_v2 + + "DELETE,/api/new,20,0,40,45,15,90,1.2,10.0,0.0,40,45,50,55,65,75,85,95,140,190,90\n" + ) with tempfile.TemporaryDirectory() as base_dir, tempfile.TemporaryDirectory() as curr_dir: base_path = Path(base_dir) / "report.csv" @@ -191,6 +197,7 @@ def test_json_includes_html_features(self, temp_dir_with_html, capsys): """) # Copy the HTML file import shutil + for html_file in temp_dir_with_html.glob("*.html"): if html_file.name != "htmlpublisher-wrapper.html": shutil.copy(html_file, other / html_file.name) @@ -214,6 +221,7 @@ def test_human_output_includes_html_section(self, temp_dir_with_html, capsys): """) # Copy the HTML file import shutil + for html_file in temp_dir_with_html.glob("*.html"): if html_file.name != "htmlpublisher-wrapper.html": shutil.copy(html_file, other / html_file.name) diff --git a/tools/locust-compare/tests/test_html_extraction.py b/tools/locust-compare/tests/test_html_extraction.py index f2fe8fb..9b625cd 100644 --- a/tools/locust-compare/tests/test_html_extraction.py +++ b/tools/locust-compare/tests/test_html_extraction.py @@ -1,5 +1,5 @@ """Tests for HTML template extraction functions.""" -import pytest + import sys from pathlib import Path diff --git a/tools/locust-compare/tests/test_html_feature_map.py b/tools/locust-compare/tests/test_html_feature_map.py index 8214aad..29cbaf7 100644 --- a/tools/locust-compare/tests/test_html_feature_map.py +++ b/tools/locust-compare/tests/test_html_feature_map.py @@ -1,10 +1,11 @@ """Integration tests for load_html_feature_map function.""" -import pytest + import sys import tempfile -import shutil from pathlib import Path +import pytest + sys.path.insert(0, str(Path(__file__).parent.parent)) from compare_runs import load_html_feature_map @@ -68,7 +69,9 @@ def test_load_skips_htmlpublisher_wrapper(self): # Create the wrapper file wrapper = base / "htmlpublisher-wrapper.html" - wrapper.write_text("""""") + wrapper.write_text( + """""" + ) features = load_html_feature_map(base) assert "htmlpublisher-wrapper" not in features diff --git a/tools/locust-compare/tests/test_load_report.py b/tools/locust-compare/tests/test_load_report.py index 65a46c8..a94d482 100644 --- a/tools/locust-compare/tests/test_load_report.py +++ b/tools/locust-compare/tests/test_load_report.py @@ -1,11 +1,13 @@ """Integration tests for load_report function.""" -import pytest + import sys import tempfile from pathlib import Path +import pytest + sys.path.insert(0, str(Path(__file__).parent.parent)) -from compare_runs import load_report, Row +from compare_runs import load_report class TestLoadReport: @@ -67,14 +69,14 @@ def test_load_non_csv_file_raises(self): load_report(Path(f.name)) def test_load_empty_csv(self, empty_csv_content): - with tempfile.NamedTemporaryFile(mode='w', suffix='.csv', delete=False) as f: + with tempfile.NamedTemporaryFile(mode="w", suffix=".csv", delete=False) as f: f.write(empty_csv_content) f.flush() rows = load_report(Path(f.name)) assert rows == [] def test_load_handles_malformed_values(self, malformed_csv_content): - with tempfile.NamedTemporaryFile(mode='w', suffix='.csv', delete=False) as f: + with tempfile.NamedTemporaryFile(mode="w", suffix=".csv", delete=False) as f: f.write(malformed_csv_content) f.flush() rows = load_report(Path(f.name)) @@ -106,7 +108,7 @@ def test_csv_with_extra_columns(self): content = """Type,Name,Request Count,CustomField,Average Response Time GET,/api/test,100,ignored,50.0 """ - with tempfile.NamedTemporaryFile(mode='w', suffix='.csv', delete=False) as f: + with tempfile.NamedTemporaryFile(mode="w", suffix=".csv", delete=False) as f: f.write(content) f.flush() rows = load_report(Path(f.name)) @@ -118,7 +120,7 @@ def test_csv_with_whitespace_values(self): content = """Type,Name,Request Count,Average Response Time GET, /api/test , 100 , 50.5 """ - with tempfile.NamedTemporaryFile(mode='w', suffix='.csv', delete=False) as f: + with tempfile.NamedTemporaryFile(mode="w", suffix=".csv", delete=False) as f: f.write(content) f.flush() rows = load_report(Path(f.name)) @@ -130,7 +132,7 @@ def test_csv_file_named_differently(self): content = """Type,Name,Request Count GET,/api/test,100 """ - with tempfile.NamedTemporaryFile(mode='w', suffix='.csv', delete=False) as f: + with tempfile.NamedTemporaryFile(mode="w", suffix=".csv", delete=False) as f: f.write(content) f.flush() rows = load_report(Path(f.name)) diff --git a/tools/locust-compare/tests/test_markdown_output.py b/tools/locust-compare/tests/test_markdown_output.py index 901c185..9fa5bab 100644 --- a/tools/locust-compare/tests/test_markdown_output.py +++ b/tools/locust-compare/tests/test_markdown_output.py @@ -1,13 +1,12 @@ """Tests for markdown output functionality.""" -import pytest -import sys + import json +import sys import tempfile from pathlib import Path -from io import StringIO sys.path.insert(0, str(Path(__file__).parent.parent)) -from compare_runs import compare_reports, _verdict_to_emoji +from compare_runs import _verdict_to_emoji, compare_reports class TestMarkdownOutput: @@ -61,7 +60,9 @@ def test_markdown_output_includes_metrics(self, temp_test_dir, temp_test_dir_v2, def test_markdown_output_includes_emojis(self, temp_test_dir, temp_test_dir_v2, capsys): """Test that markdown output includes emoji indicators.""" - compare_reports(temp_test_dir, temp_test_dir_v2, output_format="markdown", show_verdict=True) + compare_reports( + temp_test_dir, temp_test_dir_v2, output_format="markdown", show_verdict=True + ) captured = capsys.readouterr() output = captured.out @@ -71,14 +72,16 @@ def test_markdown_output_includes_emojis(self, temp_test_dir, temp_test_dir_v2, def test_markdown_output_no_verdict(self, temp_test_dir, temp_test_dir_v2, capsys): """Test markdown output without verdict column.""" - compare_reports(temp_test_dir, temp_test_dir_v2, output_format="markdown", show_verdict=False) + compare_reports( + temp_test_dir, temp_test_dir_v2, output_format="markdown", show_verdict=False + ) captured = capsys.readouterr() output = captured.out # Should NOT have Verdict column in tables - lines = output.split('\n') - header_lines = [l for l in lines if 'Metric' in l and '|' in l] + lines = output.split("\n") + header_lines = [l for l in lines if "Metric" in l and "|" in l] assert len(header_lines) > 0 for header in header_lines: assert "Verdict" not in header @@ -97,24 +100,26 @@ def test_markdown_output_no_color_codes(self, temp_test_dir, temp_test_dir_v2, c def test_markdown_table_format(self, temp_test_dir, temp_test_dir_v2, capsys): """Test that markdown tables are properly formatted.""" - compare_reports(temp_test_dir, temp_test_dir_v2, output_format="markdown", show_verdict=True) + compare_reports( + temp_test_dir, temp_test_dir_v2, output_format="markdown", show_verdict=True + ) captured = capsys.readouterr() output = captured.out - lines = output.split('\n') + lines = output.split("\n") # Find table lines - table_lines = [l for l in lines if l.startswith('|') and 'Metric' in l] + table_lines = [l for l in lines if l.startswith("|") and "Metric" in l] assert len(table_lines) > 0 # Check that separator line follows header for i, line in enumerate(lines): - if 'Metric' in line and line.startswith('|'): + if "Metric" in line and line.startswith("|"): # Next line should be separator if i + 1 < len(lines): next_line = lines[i + 1] - assert '---' in next_line - assert next_line.startswith('|') + assert "---" in next_line + assert next_line.startswith("|") def test_markdown_with_html_features(self, temp_dir_with_html, capsys): """Test markdown output includes HTML features section.""" @@ -126,6 +131,7 @@ def test_markdown_with_html_features(self, temp_dir_with_html, capsys): """) # Copy the HTML file import shutil + for html_file in temp_dir_with_html.glob("*.html"): if html_file.name != "htmlpublisher-wrapper.html": shutil.copy(html_file, other / html_file.name) @@ -177,7 +183,9 @@ def test_normal_output_still_works(self, temp_test_dir, temp_test_dir_v2, capsys def test_color_output_still_works(self, temp_test_dir, temp_test_dir_v2, capsys): """Test that colorized output still works.""" - result = compare_reports(temp_test_dir, temp_test_dir_v2, output_format="text", colorize=True) + result = compare_reports( + temp_test_dir, temp_test_dir_v2, output_format="text", colorize=True + ) assert result == 0 captured = capsys.readouterr() diff --git a/tools/locust-compare/tests/test_metrics.py b/tools/locust-compare/tests/test_metrics.py index fcbb0f0..9028c47 100644 --- a/tools/locust-compare/tests/test_metrics.py +++ b/tools/locust-compare/tests/test_metrics.py @@ -1,5 +1,5 @@ """Unit tests for metric direction and verdict functions.""" -import pytest + import sys from pathlib import Path diff --git a/tools/locust-compare/tests/test_row.py b/tools/locust-compare/tests/test_row.py index 20318ca..784ed77 100644 --- a/tools/locust-compare/tests/test_row.py +++ b/tools/locust-compare/tests/test_row.py @@ -1,8 +1,10 @@ """Unit tests for Row dataclass and index_rows function.""" -import pytest + import sys from pathlib import Path +import pytest + sys.path.insert(0, str(Path(__file__).parent.parent)) from compare_runs import Row, index_rows diff --git a/tools/locust-compare/tests/test_utils.py b/tools/locust-compare/tests/test_utils.py index 5f8ce20..98d4ff7 100644 --- a/tools/locust-compare/tests/test_utils.py +++ b/tools/locust-compare/tests/test_utils.py @@ -1,10 +1,12 @@ """Unit tests for utility functions.""" -import pytest + import sys from pathlib import Path +import pytest + sys.path.insert(0, str(Path(__file__).parent.parent)) -from compare_runs import _as_float, pct_change, diff, format_number +from compare_runs import _as_float, diff, format_number, pct_change class TestAsFloat: diff --git a/tools/locust-compare/tests/test_zip_support.py b/tools/locust-compare/tests/test_zip_support.py index 7ffe4e3..ae49f54 100644 --- a/tools/locust-compare/tests/test_zip_support.py +++ b/tools/locust-compare/tests/test_zip_support.py @@ -1,12 +1,14 @@ """Tests for zip file support.""" -import pytest + import sys -import zipfile import tempfile +import zipfile from pathlib import Path +import pytest + sys.path.insert(0, str(Path(__file__).parent.parent)) -from compare_runs import _resolve_path, compare_reports, _temp_dirs +from compare_runs import _resolve_path, _temp_dirs, compare_reports class TestResolvePath: @@ -140,7 +142,9 @@ def test_compare_directory_to_zip(self, temp_test_dir, sample_csv_content_v2, ca result = compare_reports(temp_test_dir, curr_zip, output_format="json") assert result == 0 - def test_zip_with_nested_directory_structure(self, sample_csv_content, sample_csv_content_v2, capsys): + def test_zip_with_nested_directory_structure( + self, sample_csv_content, sample_csv_content_v2, capsys + ): """Zip with report inside a subdirectory should work.""" with tempfile.TemporaryDirectory() as tmpdir: base_zip = Path(tmpdir) / "base.zip" diff --git a/tools/wt-worktree/PRD.md b/tools/wt-worktree/PRD.md index b5a7c1e..a00a354 100644 --- a/tools/wt-worktree/PRD.md +++ b/tools/wt-worktree/PRD.md @@ -7,6 +7,7 @@ ## Stories and Tasks ### Story 1: Core Infrastructure ✅ + **As a developer, I want the basic project structure and core git operations, so that I can build features on top** - [x] Task 1.1: Set up project structure with pyproject.toml @@ -16,6 +17,7 @@ - [x] Task 1.5: Add tests for core modules ### Story 2: Basic Commands ✅ + **As a user, I want to initialize and manage worktrees, so that I can work on multiple features** - [x] Task 2.1: Implement `wt init` command @@ -25,6 +27,7 @@ - [x] Task 2.5: Add tests for basic commands ### Story 3: Worktree Management Commands ✅ + **As a user, I want to compare, delete, and check status of worktrees** - [x] Task 3.1: Implement `wt diff` command @@ -33,6 +36,7 @@ - [x] Task 3.4: Add tests for management commands ### Story 4: Advanced Features ✅ + **As a user, I want to run commands in worktrees and clean up merged branches** - [x] Task 4.1: Implement `wt run` command @@ -41,6 +45,7 @@ - [x] Task 4.4: Add tests for advanced features ### Story 5: Shell Integration ✅ + **As a user, I want seamless shell integration, so that I can navigate worktrees easily** - [x] Task 5.1: Implement `wt shell-init` command @@ -50,6 +55,7 @@ - [x] Task 5.5: Test shell integration ### Story 6: User Experience ✅ + **As a user, I want helpful prompts and error messages** - [x] Task 6.1: Implement user prompts module (prompts.py) @@ -58,6 +64,7 @@ - [x] Task 6.4: Handle edge cases (spaces in paths, special characters) ### Story 7: Documentation and Testing ✅ + **As a developer, I want comprehensive documentation and tests** - [x] Task 7.1: Write comprehensive test suite @@ -84,6 +91,7 @@ - Shell wrappers for cd integration ### Story 8: Worktree Sync ✅ + **As a user, I want to sync worktrees with their upstream branches** - [x] Task 8.1: Add git operations for stash, pull, and rebase diff --git a/tools/wt-worktree/README.md b/tools/wt-worktree/README.md index 205ecce..64f6988 100644 --- a/tools/wt-worktree/README.md +++ b/tools/wt-worktree/README.md @@ -79,6 +79,7 @@ wt init [--prefix ] [--path ] ``` **Options:** + - `--prefix`: Branch prefix for new worktrees (default: `feature`) - `--path`: Path pattern for worktree directories (default: `../{repo}-{name}`) @@ -302,6 +303,7 @@ wt config --edit ## Configuration Configuration is stored in a single TOML file: + - Default location: `~/.wt.toml` - Custom location: Set `WT_CONFIG` environment variable to a directory diff --git a/tools/wt-worktree/tests/conftest.py b/tools/wt-worktree/tests/conftest.py index 36a29c2..efa68c6 100644 --- a/tools/wt-worktree/tests/conftest.py +++ b/tools/wt-worktree/tests/conftest.py @@ -6,7 +6,6 @@ from pathlib import Path import pytest - from wt import git @@ -43,7 +42,7 @@ def git_repo(temp_dir): if current_branch == "master": git.run_git(["branch", "-m", "main"], cwd=repo_path) - yield repo_path + return repo_path @pytest.fixture @@ -59,7 +58,7 @@ def git_repo_with_remote(git_repo, temp_dir): # Push main branch git.run_git(["push", "-u", "origin", "main"], cwd=git_repo) - yield git_repo, remote_path + return git_repo, remote_path @pytest.fixture diff --git a/tools/wt-worktree/tests/test_cli.py b/tools/wt-worktree/tests/test_cli.py index c50453b..bde0b4e 100644 --- a/tools/wt-worktree/tests/test_cli.py +++ b/tools/wt-worktree/tests/test_cli.py @@ -1,12 +1,11 @@ """Tests for CLI commands.""" import os + import pytest from click.testing import CliRunner -from pathlib import Path - -from wt.cli import cli from wt import git +from wt.cli import cli @pytest.fixture @@ -19,6 +18,7 @@ def runner(): def initialized_repo(git_repo, tmp_path, monkeypatch): """Create a git repo with wt config in temp directory.""" from wt.config import Config + # Use temp directory for config monkeypatch.setenv("WT_CONFIG", str(tmp_path)) @@ -173,6 +173,7 @@ def test_commands_from_secondary_worktree(runner, initialized_repo, no_prompt): # Find the worktree path from wt import git + worktrees = git.list_worktrees(initialized_repo) feat_worktree = None for wt in worktrees: @@ -316,6 +317,7 @@ def test_detached_worktree_create(runner, initialized_repo, no_prompt): # Worktree should be created from wt.config import Config from wt.worktree import WorktreeManager + config = Config(initialized_repo) manager = WorktreeManager(config) wt = manager.find_worktree_by_name("mydetached") @@ -336,7 +338,7 @@ def test_detached_worktree_list(runner, initialized_repo, no_prompt): assert "detached1" in result.output assert "detached2" in result.output # Should not show generic "(detached)" for named worktrees - lines = result.output.split('\n') + lines = result.output.split("\n") detached_lines = [l for l in lines if "detached1" in l or "detached2" in l] assert len(detached_lines) == 2 @@ -379,6 +381,7 @@ def test_multiple_detached_worktrees_unique_names(runner, initialized_repo, no_p # Each should be findable by name from wt.config import Config from wt.worktree import WorktreeManager + config = Config(initialized_repo) manager = WorktreeManager(config) @@ -400,6 +403,7 @@ def test_detached_worktree_delete(runner, initialized_repo, no_prompt): # Verify it's gone from wt.config import Config from wt.worktree import WorktreeManager + config = Config(initialized_repo) manager = WorktreeManager(config) wt = manager.find_worktree_by_name("mydetached") @@ -415,8 +419,14 @@ def test_detached_worktree_backward_compatibility(runner, initialized_repo, no_p config = Config(initialized_repo) wt_path = config.resolve_path_pattern("legacy", "feature/legacy") - git.add_worktree(wt_path, "legacy", create_branch=False, base="HEAD", - detached=True, repo_path=initialized_repo) + git.add_worktree( + wt_path, + "legacy", + create_branch=False, + base="HEAD", + detached=True, + repo_path=initialized_repo, + ) # Note: Not calling set_worktree_name - simulates old behavior diff --git a/tools/wt-worktree/tests/test_config.py b/tools/wt-worktree/tests/test_config.py index 4391e1c..4a44b0d 100644 --- a/tools/wt-worktree/tests/test_config.py +++ b/tools/wt-worktree/tests/test_config.py @@ -1,8 +1,8 @@ """Tests for configuration management.""" -import pytest from pathlib import Path +import pytest from wt.config import Config, ConfigError diff --git a/tools/wt-worktree/tests/test_git.py b/tools/wt-worktree/tests/test_git.py index d4f160d..7882c71 100644 --- a/tools/wt-worktree/tests/test_git.py +++ b/tools/wt-worktree/tests/test_git.py @@ -1,8 +1,5 @@ """Tests for git operations.""" -import pytest -from pathlib import Path - from wt import git @@ -76,6 +73,7 @@ def test_add_worktree(git_repo, temp_dir): worktrees = git.list_worktrees(git_repo) assert len(worktrees) == 2 + def test_add_worktree_with_existing_branch(git_repo, temp_dir): """Test adding a worktree with an existing branch.""" git.create_branch("test-branch", "HEAD", git_repo) diff --git a/tools/wt-worktree/tests/test_worktree.py b/tools/wt-worktree/tests/test_worktree.py index a7cfc05..c62b127 100644 --- a/tools/wt-worktree/tests/test_worktree.py +++ b/tools/wt-worktree/tests/test_worktree.py @@ -1,8 +1,6 @@ """Tests for worktree operations.""" import pytest -from pathlib import Path - from wt import git from wt.config import Config from wt.worktree import WorktreeManager diff --git a/tools/wt-worktree/wt/cli.py b/tools/wt-worktree/wt/cli.py index 9f99312..88ede4b 100644 --- a/tools/wt-worktree/wt/cli.py +++ b/tools/wt-worktree/wt/cli.py @@ -1,8 +1,8 @@ """CLI commands for wt.""" import os -import sys import subprocess +import sys from pathlib import Path from typing import Optional @@ -10,10 +10,9 @@ from . import git from .config import Config, ConfigError -from .worktree import WorktreeManager -from .prompts import confirm, error, info, success, warning +from .prompts import error, info, success, warning from .shell import generate_shell_init, get_supported_shells - +from .worktree import WorktreeManager # Exit codes EXIT_SUCCESS = 0 @@ -58,8 +57,12 @@ def cli(ctx: Context): @cli.command() @click.option("--prefix", default="feature", help="Branch prefix for new worktrees") -@click.option("--path", "path_pattern", default="../{repo}-{name}", - help="Path pattern for worktree directories") +@click.option( + "--path", + "path_pattern", + default="../{repo}-{name}", + help="Path pattern for worktree directories", +) @pass_context def init(ctx: Context, prefix: str, path_pattern: str): """Create or update wt configuration with custom defaults.""" @@ -88,11 +91,18 @@ def init(ctx: Context, prefix: str, path_pattern: str): @click.option("-c", "--create", is_flag=True, help="Create worktree if it doesn't exist") @click.option("-b", "--base", help="Base branch for new worktree") @click.option("-d", "--detached", is_flag=True, help="Create in detached HEAD state") -@click.option("--shell-helper", is_flag=True, hidden=True, - help="Internal flag for shell integration") +@click.option( + "--shell-helper", is_flag=True, hidden=True, help="Internal flag for shell integration" +) @pass_context -def switch(ctx: Context, name: Optional[str], create: bool, base: Optional[str], - detached: bool, shell_helper: bool): +def switch( + ctx: Context, + name: Optional[str], + create: bool, + base: Optional[str], + detached: bool, + shell_helper: bool, +): """Switch to a worktree, optionally creating it.""" if not ctx.repo_root or not ctx.manager: error("Not in a git repository.", EXIT_GIT_ERROR) @@ -180,7 +190,7 @@ def switch(ctx: Context, name: Optional[str], create: bool, base: Optional[str], f"Worktree '{name}' not found\n" f"Available worktrees: {', '.join(available)}\n" f"Use 'wt switch -c {name}' to create it.", - EXIT_NOT_FOUND + EXIT_NOT_FOUND, ) @@ -208,7 +218,7 @@ def list_cmd(ctx: Context, name_only: bool): try: changed = git.get_changed_files_in_commit(commit, wt["path"]) if changed: - for line in changed.strip().split('\n'): + for line in changed.strip().split("\n"): if line: print(f" {line}") else: @@ -324,7 +334,7 @@ def status(ctx: Context): # Show uncommitted files if st["uncommitted_files"]: - for line in st["uncommitted_files"].strip().split('\n')[:5]: + for line in st["uncommitted_files"].strip().split("\n")[:5]: print(f" {line}") if st["uncommitted_count"] > 5: print(f" ... and {st['uncommitted_count'] - 5} more") @@ -344,7 +354,7 @@ def status(ctx: Context): parts.append(f"↓{behind}") print(f" {' '.join(parts)} {st['upstream']}") elif wt.get("branch"): - print(f" (no upstream)") + print(" (no upstream)") print() @@ -401,11 +411,7 @@ def run(ctx: Context, name: str, command: str): # Run command try: - result = subprocess.run( - command, - cwd=wt_path, - shell=True - ) + result = subprocess.run(command, cwd=wt_path, shell=True, check=False) sys.exit(result.returncode) except Exception as e: error(f"Failed to run command: {e}", EXIT_ERROR) @@ -435,8 +441,7 @@ def clean(ctx: Context, dry_run: bool, force: bool): @click.option("--list", "list_all", is_flag=True, help="Show all configuration") @click.option("--edit", is_flag=True, help="Open config file in $EDITOR") @pass_context -def config(ctx: Context, key: Optional[str], value: Optional[str], - list_all: bool, edit: bool): +def config(ctx: Context, key: Optional[str], value: Optional[str], list_all: bool, edit: bool): """View or modify configuration.""" if not ctx.config: ctx.config = Config(None) @@ -452,7 +457,7 @@ def config(ctx: Context, key: Optional[str], value: Optional[str], # Open in editor editor = os.environ.get("EDITOR", "vi") - subprocess.run([editor, str(config_path)]) + subprocess.run([editor, str(config_path)], check=False) return if list_all: @@ -493,8 +498,9 @@ def config(ctx: Context, key: Optional[str], value: Optional[str], @click.option("--exclude", help="Comma-separated list of worktrees to skip") @click.option("--rebase", is_flag=True, help="Rebase onto default base after pull") @pass_context -def sync(ctx: Context, sync_all: bool, include: Optional[str], - exclude: Optional[str], rebase: bool): +def sync( + ctx: Context, sync_all: bool, include: Optional[str], exclude: Optional[str], rebase: bool +): """Sync worktrees with their upstream branches.""" if not ctx.repo_root or not ctx.manager: error("Not in a git repository", EXIT_GIT_ERROR) @@ -545,7 +551,11 @@ def sync(ctx: Context, sync_all: bool, include: Optional[str], # Print conflict details if any("conflict" in f["error"] for f in failed): - info("Conflicts in {} worktree(s):".format(len([f for f in failed if "conflict" in f["error"]]))) + info( + "Conflicts in {} worktree(s):".format( + len([f for f in failed if "conflict" in f["error"]]) + ) + ) for f in failed: if "conflict" in f["error"]: conflict_type = f["error"].replace("_", " ") diff --git a/tools/wt-worktree/wt/config.py b/tools/wt-worktree/wt/config.py index b1954de..31725a4 100644 --- a/tools/wt-worktree/wt/config.py +++ b/tools/wt-worktree/wt/config.py @@ -3,7 +3,7 @@ import os import sys from pathlib import Path -from typing import Optional, Dict, Any +from typing import Any, Dict, Optional # Try tomllib (Python 3.11+), fallback to tomli if sys.version_info >= (3, 11): @@ -17,7 +17,6 @@ class ConfigError(Exception): """Raised when configuration operations fail.""" - pass class Config: @@ -51,8 +50,7 @@ def _read_toml(self, path: Path) -> Dict[str, Any]: """Read a TOML file.""" if tomllib is None: raise ConfigError( - "TOML support not available. " - "Please install tomli: pip install tomli" + "TOML support not available. " "Please install tomli: pip install tomli" ) try: @@ -86,8 +84,9 @@ def save(self, config: Optional[Dict[str, Any]] = None): config_data = config if config is not None else self._config # Filter out None values - filtered = {k: v for k, v in config_data.items() - if v is not None and k in self.DEFAULT_CONFIG} + filtered = { + k: v for k, v in config_data.items() if v is not None and k in self.DEFAULT_CONFIG + } self._write_toml(config_path, filtered) @@ -99,16 +98,16 @@ def _write_toml(self, path: Path, data: Dict[str, Any]): if isinstance(value, str): lines.append(f'{key} = "{value}"') elif isinstance(value, bool): - lines.append(f'{key} = {str(value).lower()}') + lines.append(f"{key} = {str(value).lower()}") elif isinstance(value, (int, float)): - lines.append(f'{key} = {value}') + lines.append(f"{key} = {value}") elif value is None: - lines.append(f'# {key} = null') + lines.append(f"# {key} = null") try: path.parent.mkdir(parents=True, exist_ok=True) with open(path, "w") as f: - f.write('\n'.join(lines) + '\n') + f.write("\n".join(lines) + "\n") except Exception as e: raise ConfigError(f"Failed to write config to {path}: {e}") @@ -179,5 +178,5 @@ def extract_worktree_name(self, branch: str) -> str: """ prefix = self.get("prefix") if prefix and branch.startswith(f"{prefix}/"): - return branch[len(prefix) + 1:] + return branch[len(prefix) + 1 :] return branch diff --git a/tools/wt-worktree/wt/git.py b/tools/wt-worktree/wt/git.py index 40c3736..768abff 100644 --- a/tools/wt-worktree/wt/git.py +++ b/tools/wt-worktree/wt/git.py @@ -1,18 +1,17 @@ """Git operations wrapper module.""" import subprocess -import sys from pathlib import Path -from typing import Optional, List, Tuple +from typing import List, Optional, Tuple class GitError(Exception): """Raised when a git operation fails.""" - pass -def run_git(args: List[str], cwd: Optional[Path] = None, check: bool = True, - capture_output: bool = True) -> subprocess.CompletedProcess: +def run_git( + args: List[str], cwd: Optional[Path] = None, check: bool = True, capture_output: bool = True +) -> subprocess.CompletedProcess: """ Run a git command and return the result. @@ -30,11 +29,7 @@ def run_git(args: List[str], cwd: Optional[Path] = None, check: bool = True, """ try: result = subprocess.run( - ["git"] + args, - cwd=cwd, - capture_output=capture_output, - text=True, - check=False + ["git"] + args, cwd=cwd, capture_output=capture_output, text=True, check=False ) if check and result.returncode != 0: @@ -121,16 +116,15 @@ def get_status_short(path: Optional[Path] = None) -> str: def branch_exists(branch: str, path: Optional[Path] = None) -> bool: """Check if a branch exists locally.""" - result = run_git(["rev-parse", "--verify", f"refs/heads/{branch}"], - cwd=path, check=False) + result = run_git(["rev-parse", "--verify", f"refs/heads/{branch}"], cwd=path, check=False) return result.returncode == 0 -def remote_branch_exists(branch: str, remote: str = "origin", - path: Optional[Path] = None) -> bool: +def remote_branch_exists(branch: str, remote: str = "origin", path: Optional[Path] = None) -> bool: """Check if a branch exists on remote.""" - result = run_git(["rev-parse", "--verify", f"refs/remotes/{remote}/{branch}"], - cwd=path, check=False) + result = run_git( + ["rev-parse", "--verify", f"refs/remotes/{remote}/{branch}"], cwd=path, check=False + ) return result.returncode == 0 @@ -144,8 +138,12 @@ def create_branch(branch: str, base: str = "HEAD", path: Optional[Path] = None): run_git(["branch", branch, base], cwd=path) -def set_upstream(branch: str, remote: str = "origin", - remote_branch: Optional[str] = None, path: Optional[Path] = None): +def set_upstream( + branch: str, + remote: str = "origin", + remote_branch: Optional[str] = None, + path: Optional[Path] = None, +): """ Set upstream tracking for a branch. @@ -160,8 +158,12 @@ def set_upstream(branch: str, remote: str = "origin", run_git(["branch", f"--set-upstream-to={remote}/{remote_branch}", branch], cwd=path) -def configure_push_remote(branch: str, remote: str = "origin", - remote_branch: Optional[str] = None, path: Optional[Path] = None): +def configure_push_remote( + branch: str, + remote: str = "origin", + remote_branch: Optional[str] = None, + path: Optional[Path] = None, +): """ Configure where a branch should push to, even if remote branch doesn't exist yet. @@ -196,7 +198,7 @@ def list_worktrees(path: Optional[Path] = None) -> List[dict]: worktrees = [] current = {} - for line in result.stdout.strip().split('\n'): + for line in result.stdout.strip().split("\n"): if not line: if current: worktrees.append(current) @@ -236,12 +238,14 @@ def worktree_exists(name: str, path: Optional[Path] = None) -> Tuple[bool, Optio return False, None -def add_worktree(path: Path, - branch: str, - create_branch: bool = False, - base: Optional[str] = None, - detached: bool = False, - repo_path: Optional[Path] = None): +def add_worktree( + path: Path, + branch: str, + create_branch: bool = False, + base: Optional[str] = None, + detached: bool = False, + repo_path: Optional[Path] = None, +): """ Create a new worktree. @@ -308,15 +312,13 @@ def get_merge_base(branch1: str, branch2: str, path: Optional[Path] = None) -> s def is_ancestor(ancestor: str, descendant: str, path: Optional[Path] = None) -> bool: """Check if ancestor is an ancestor of descendant (i.e., branch is merged).""" - result = run_git(["merge-base", "--is-ancestor", ancestor, descendant], - cwd=path, check=False) + result = run_git(["merge-base", "--is-ancestor", ancestor, descendant], cwd=path, check=False) return result.returncode == 0 def get_upstream_branch(branch: str, path: Optional[Path] = None) -> Optional[str]: """Get the upstream tracking branch for a local branch.""" - result = run_git(["rev-parse", "--abbrev-ref", f"{branch}@{{upstream}}"], - cwd=path, check=False) + result = run_git(["rev-parse", "--abbrev-ref", f"{branch}@{{upstream}}"], cwd=path, check=False) if result.returncode == 0: return result.stdout.strip() return None @@ -329,14 +331,14 @@ def get_ahead_behind(branch: str, upstream: str, path: Optional[Path] = None) -> Returns: Tuple of (ahead, behind) """ - result = run_git(["rev-list", "--left-right", "--count", f"{upstream}...{branch}"], - cwd=path) + result = run_git(["rev-list", "--left-right", "--count", f"{upstream}...{branch}"], cwd=path) behind, ahead = result.stdout.strip().split() return int(ahead), int(behind) -def diff_trees(tree1: str, tree2: str, path: Optional[Path] = None, - stat: bool = False, name_only: bool = False) -> str: +def diff_trees( + tree1: str, tree2: str, path: Optional[Path] = None, stat: bool = False, name_only: bool = False +) -> str: """ Get diff between two tree-ish objects (commits, branches, etc). @@ -376,7 +378,7 @@ def get_default_branch(path: Optional[Path] = None) -> str: result = run_git(["symbolic-ref", "refs/remotes/origin/HEAD"], cwd=path, check=False) if result.returncode == 0: # Output is like "refs/remotes/origin/main" - return result.stdout.strip().split('/')[-1] + return result.stdout.strip().split("/")[-1] # Fallback: check common branch names for branch in ["main", "master"]: @@ -422,7 +424,9 @@ def stash_pop(path: Optional[Path] = None) -> bool: return result.returncode == 0 -def pull_branch(branch: str, path: Optional[Path] = None, remote: str = "origin") -> Tuple[bool, str]: +def pull_branch( + branch: str, path: Optional[Path] = None, remote: str = "origin" +) -> Tuple[bool, str]: """ Pull changes from remote branch. @@ -440,16 +444,13 @@ def pull_branch(branch: str, path: Optional[Path] = None, remote: str = "origin" # Check if it was a fast-forward or already up to date if "Already up to date" in result.stdout: return True, "already_up_to_date" - elif "Fast-forward" in result.stdout: + if "Fast-forward" in result.stdout: return True, "fast_forward" - else: - return True, "merged" - else: - # Check for conflict - if "CONFLICT" in result.stdout or "CONFLICT" in result.stderr: - return False, "conflict" - else: - return False, result.stderr.strip() + return True, "merged" + # Check for conflict + if "CONFLICT" in result.stdout or "CONFLICT" in result.stderr: + return False, "conflict" + return False, result.stderr.strip() def rebase_branch(branch: str, onto: str, path: Optional[Path] = None) -> Tuple[bool, str]: @@ -470,16 +471,13 @@ def rebase_branch(branch: str, onto: str, path: Optional[Path] = None) -> Tuple[ # Check if it was already up to date or had commits if "is up to date" in result.stdout or "is up to date" in result.stderr: return True, "up_to_date" - else: - return True, "rebased" - else: - # Check for conflict - if "CONFLICT" in result.stdout or "CONFLICT" in result.stderr: - # Abort the rebase to leave repo in clean state - run_git(["rebase", "--abort"], cwd=path, check=False) - return False, "conflict" - else: - return False, result.stderr.strip() + return True, "rebased" + # Check for conflict + if "CONFLICT" in result.stdout or "CONFLICT" in result.stderr: + # Abort the rebase to leave repo in clean state + run_git(["rebase", "--abort"], cwd=path, check=False) + return False, "conflict" + return False, result.stderr.strip() def fetch_remote(remote: str = "origin", path: Optional[Path] = None): diff --git a/tools/wt-worktree/wt/prompts.py b/tools/wt-worktree/wt/prompts.py index 274ded7..2288a44 100644 --- a/tools/wt-worktree/wt/prompts.py +++ b/tools/wt-worktree/wt/prompts.py @@ -57,7 +57,7 @@ def prompt_choice(message: str, choices: list, default: Optional[str] = None) -> print(f" {i}. {choice}") try: - response = input("Enter choice (1-{}): ".format(len(choices))).strip() + response = input(f"Enter choice (1-{len(choices)}): ").strip() if not response: return default diff --git a/tools/wt-worktree/wt/shell.py b/tools/wt-worktree/wt/shell.py index 71c88b5..4ca5ee4 100644 --- a/tools/wt-worktree/wt/shell.py +++ b/tools/wt-worktree/wt/shell.py @@ -1,6 +1,5 @@ """Shell integration code generation.""" - BASH_ZSH_WRAPPER = """ # wt shell integration for {shell} wt() {{ @@ -67,13 +66,9 @@ def generate_shell_init(shell: str) -> str: if shell in ("bash", "zsh"): return BASH_ZSH_WRAPPER.format(shell=shell).strip() - elif shell == "fish": + if shell == "fish": return FISH_WRAPPER.strip() - else: - raise ValueError( - f"Unsupported shell: {shell}\n" - f"Supported shells: bash, zsh, fish" - ) + raise ValueError(f"Unsupported shell: {shell}\n" f"Supported shells: bash, zsh, fish") def get_supported_shells() -> list: diff --git a/tools/wt-worktree/wt/worktree.py b/tools/wt-worktree/wt/worktree.py index a5f8828..f26298f 100644 --- a/tools/wt-worktree/wt/worktree.py +++ b/tools/wt-worktree/wt/worktree.py @@ -1,12 +1,15 @@ """Core worktree operations.""" from pathlib import Path -from typing import Optional, List, Tuple +from typing import List, Optional, Tuple + from . import git from .config import Config from .prompts import confirm, error, info, warning EXIT_ERROR = 1 + + class WorktreeManager: """Manages git worktree operations.""" @@ -41,7 +44,7 @@ def _infer_name_from_path(self, wt_path: Path) -> Optional[str]: if pattern == "../{repo}-{name}": expected_prefix = f"{repo_name}-" if wt_path.name.startswith(expected_prefix): - return wt_path.name[len(expected_prefix):] + return wt_path.name[len(expected_prefix) :] # Try pattern: ../{name} elif pattern == "../{name}": @@ -94,13 +97,13 @@ def get_current_worktree(self) -> Optional[dict]: Worktree dict or None """ import os + cwd = Path(os.getcwd()) worktrees = self.list_worktrees() for wt in worktrees: try: - if cwd.resolve() == wt["path"].resolve() or \ - cwd.is_relative_to(wt["path"]): + if cwd.resolve() == wt["path"].resolve() or cwd.is_relative_to(wt["path"]): return wt except (ValueError, OSError): # is_relative_to can raise on some systems @@ -165,8 +168,9 @@ def get_default_worktree(self) -> Optional[dict]: return None - def create_worktree(self, name: str, base: Optional[str] = None, - detached: bool = False) -> Path: + def create_worktree( + self, name: str, base: Optional[str] = None, detached: bool = False + ) -> Path: """ Create a new worktree. @@ -198,8 +202,7 @@ def create_worktree(self, name: str, base: Optional[str] = None, # Check if path already exists if wt_path.exists(): raise git.GitError( - f"Path {wt_path} already exists. " - f"Please remove it or choose a different name." + f"Path {wt_path} already exists. " f"Please remove it or choose a different name." ) # Determine base branch @@ -244,8 +247,7 @@ def create_worktree(self, name: str, base: Optional[str] = None, return wt_path - def delete_worktree(self, name: str, force: bool = False, - keep_branch: bool = False) -> bool: + def delete_worktree(self, name: str, force: bool = False, keep_branch: bool = False) -> bool: """ Delete a worktree. @@ -282,8 +284,9 @@ def delete_worktree(self, name: str, force: bool = False, error( f"Worktree '{name}' has uncommitted changes:\n{status}\n" "use --force to delete anyway", - EXIT_ERROR) - return + EXIT_ERROR, + ) + return None # Check for unpushed commits if not force and branch: @@ -294,15 +297,14 @@ def delete_worktree(self, name: str, force: bool = False, if ahead > 0: # Get list of unpushed commits result = git.run_git( - ["log", "--oneline", f"{upstream}..{branch}"], - cwd=self.repo_root + ["log", "--oneline", f"{upstream}..{branch}"], cwd=self.repo_root ) commits = result.stdout.strip() if not confirm( f"Worktree '{name}' has {ahead} unpushed commit(s):\n{commits}\n\n" "Delete anyway?", - default=False + default=False, ): return False except git.GitError: @@ -352,7 +354,7 @@ def get_worktree_status(self, wt: dict) -> dict: if git.has_uncommitted_changes(wt_path): files = git.get_status_short(wt_path) status["uncommitted_files"] = files - status["uncommitted_count"] = len(files.strip().split('\n')) + status["uncommitted_count"] = len(files.strip().split("\n")) # Check ahead/behind status if branch: @@ -410,8 +412,8 @@ def clean_merged_worktrees(self, dry_run: bool = False, force: bool = False) -> upstream = git.get_upstream_branch(branch, self.repo_root) if upstream: # Parse remote name from upstream (e.g., "origin/feature/foo" -> "origin", "feature/foo") - remote = upstream.split('/')[0] - remote_branch = '/'.join(upstream.split('/')[1:]) + remote = upstream.split("/")[0] + remote_branch = "/".join(upstream.split("/")[1:]) if not git.remote_branch_exists(remote_branch, remote, self.repo_root): to_remove.append((name, "remote branch deleted")) @@ -480,7 +482,7 @@ def sync_worktree(self, wt: dict, rebase: bool = False) -> dict: return result # Parse remote from upstream (e.g., "origin/feature/foo" -> "origin", "feature/foo") - remote_parts = upstream.split('/', 1) + remote_parts = upstream.split("/", 1) if len(remote_parts) < 2: result["error"] = f"invalid upstream: {upstream}" return result @@ -561,8 +563,9 @@ def sync_worktree(self, wt: dict, rebase: bool = False) -> dict: return result - def sync_worktrees(self, worktree_names: Optional[List[str]] = None, - rebase: bool = False) -> Tuple[List[dict], List[dict]]: + def sync_worktrees( + self, worktree_names: Optional[List[str]] = None, rebase: bool = False + ) -> Tuple[List[dict], List[dict]]: """ Sync multiple worktrees with their upstream branches. @@ -606,20 +609,19 @@ def sync_worktrees(self, worktree_names: Optional[List[str]] = None, result = self.sync_worktree(wt, rebase) if result["success"]: - succeeded.append({ - "name": wt_name, - "message": result["message"] - }) + succeeded.append({"name": wt_name, "message": result["message"]}) # Print success message - for line in result["message"].split('\n'): + for line in result["message"].split("\n"): if line: info(f"[{wt_name}] {line}") else: - failed.append({ - "name": wt_name, - "error": result["error"], - "stashed": result.get("stashed", False) - }) + failed.append( + { + "name": wt_name, + "error": result["error"], + "stashed": result.get("stashed", False), + } + ) # Print error message (without exiting) error_msg = result["error"] if "conflict" in error_msg: From 5a99379ccf717a09af549b8f4dbe57ea36dae545 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 24 Jan 2026 13:49:51 +0000 Subject: [PATCH 2/2] Add comprehensive pre-commit TODO documentation Documents all issues found by pre-commit hooks: - 91 ruff linting issues across Python files - Multiple pyright type checking issues - 71+ markdownlint formatting issues Includes: - Detailed file-by-file breakdown of issues - Specific line numbers and suggested fixes - Configuration options to relax rules - Recommended fix order and estimated effort - Quick win approach for minimal changes --- pre-commit-todo.md | 315 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 315 insertions(+) create mode 100644 pre-commit-todo.md diff --git a/pre-commit-todo.md b/pre-commit-todo.md new file mode 100644 index 0000000..aa4acc5 --- /dev/null +++ b/pre-commit-todo.md @@ -0,0 +1,315 @@ +# Pre-commit Issues TODO + +This document tracks all the changes needed to pass pre-commit hooks. + +## Summary + +- **Ruff**: 91 issues across Python files +- **Pyright**: Multiple type checking issues +- **Markdownlint**: 71+ markdown formatting issues +- **Other hooks**: All passing ✅ + +--- + +## 1. Ruff Issues (Python Linting & Formatting) + +### tools/config-utils/cli.py + +**Issues to fix:** + +1. Line 34: Remove unused variable `current_depth` + + ```python + # REMOVE: current_depth = len(parent_key.split(sep)) if parent_key else 0 + ``` + +2. Lines 95, 256, 364: Replace `open()` with `Path.open()` + - Import `from pathlib import Path` + - Replace `open(file_path)` with `Path(file_path).open()` + - Replace `open(output_path, 'w')` with `Path(output_path).open('w')` + +3. Line 130: Function `perform_set_operation` is too complex + - PLR0912: Too many branches (24 > 12) + - PLR0915: Too many statements (53 > 50) + - **Action**: Consider refactoring into smaller functions + +4. Lines 183, 184: Use set comprehensions instead of generators + + ```python + # CHANGE FROM: + items1 = set((k, make_hashable(v)) for k, v in flat1.items()) + # TO: + items1 = {(k, make_hashable(v)) for k, v in flat1.items()} + ``` + +5. Lines 228, 306, 394: Line too long (>100 chars) + - Line 228: Docstring for `main()` - split across lines + - Line 306: Error message - split string concatenation + - Line 394: Help text - split across lines + +6. Lines 246, 294: Unused function argument `format` + - Either use the parameter or prefix with underscore: `_format` + +### tools/config-utils/tests/test_set_operations.py + +**Issues to fix:** + +1. Lines 52, 65, 101, 107, 147, 159, 184, 191, 229, 236, 272, 279, 315, 322, 348, 354: Replace `open()` with `Path.open()` + - Import `from pathlib import Path` + - Replace all instances of `open(temp_file, 'w')` pattern + +### tools/locust-compare/compare_runs.py + +**Issues to fix:** + +1. Lines 51, 231, 248, 272, 329, 378, 409, 426, 434: Replace `open()` with `Path.open()` + - Import `from pathlib import Path` + - Replace `open()` calls with `Path().open()` + +2. Line 79: Function `extract_from_html` is too complex + - PLR0912: Too many branches (15 > 12) + - **Action**: Consider extracting parsing logic into helper functions + +3. Line 166: Function `load_report` is too complex + - PLR0912: Too many branches (18 > 12) + - **Action**: Split file type handling into separate functions + +4. Lines 208, 214, 221: Replace `os.path` with `pathlib` + - PTH118: `os.path.join()` → use `/` operator with Path + - PTH119: `os.path.basename()` → use `Path.name` + - PTH123: `os.path.dirname()` → use `Path.parent` + +5. Line 319: Ambiguous variable name `l` (E741) + - Rename to something descriptive like `line` or `item` + +6. Line 521: Line too long + - Split the line + +### tools/locust-compare/tests/*.py + +**Multiple test files with PTH123 violations:** + +- `conftest.py`: Lines 14, 27, 28, 29, 30, 52, 56, 60, 64, 75, 79, 91, 93 +- `test_compare_reports.py`: Lines 6, 14 +- `test_html_extraction.py`: Lines 11, 24, 37, 46, 62, 78, 92, 109, 124 +- `test_html_feature_map.py`: Line 6 +- `test_load_report.py`: Lines 11, 27, 43, 59, 75, 96, 115, 144, 171 +- `test_markdown_output.py`: Lines 11, 22 +- `test_metrics.py`: Line 6 +- `test_row.py`: Line 6 +- `test_utils.py`: Lines 7, 17, 29, 39, 45, 56, 68, 80, 92, 104, 116, 123, 132 +- `test_zip_support.py`: Lines 17, 36, 61, 86, 113, 138, 165, 191, 217, 241, 262 + +**Action**: Replace all `open()` with `Path().open()` in test files + +### tools/wt-worktree/wt/*.py + +**Issues to fix:** + +- `cli.py`: Lines 15, 22 - PTH123 violations +- `config.py`: Lines 14, 102, 127, 205 - PTH123 violations; Line 14 - PTH118 violation +- `git.py`: Lines 47, 73, 91, 94 - PTH119 violations +- `prompts.py`: Line 86 - PTH123 violation +- `shell.py`: Lines 129, 130 - PTH118 violations +- `worktree.py`: Lines 72, 84, 89, 103, 107, 121, 151, 158, 176 - PTH119 violations; Line 166 - PTH118 violation + +**Action**: Replace `os.path` functions with `pathlib.Path` equivalents + +### tools/wt-worktree/tests/*.py + +**Issues to fix:** + +- `conftest.py`: Line 31 - PTH123 violation +- `test_cli.py`: Lines 133, 193, 252, 300, 327, 349, 414, 506, 652 - PTH123 violations +- `test_config.py`: Lines 12, 32, 51, 95, 132, 182, 202, 244, 284, 330 - PTH123 violations +- `test_git.py`: Lines 17, 25, 50, 61, 87 - PTH123 violations +- `test_worktree.py`: Lines 149, 165, 186, 236, 289, 363, 418, 569, 673 - PTH123 violations + +**Action**: Replace all `open()` with `Path().open()` + +--- + +## 2. Pyright Issues (Type Checking) + +### General approach + +1. Add missing type hints to function signatures +2. Import `from typing import` necessary types (Dict, List, Optional, Any, etc.) +3. Fix type mismatches +4. Handle Optional/None cases properly + +**Files with type issues:** + +- All Python files in `tools/config-utils/`, `tools/locust-compare/`, and `tools/wt-worktree/` + +**Recommended approach:** + +- Run `prek run pyright` after fixing ruff issues +- Fix type errors incrementally, file by file +- Start with main modules before test files + +--- + +## 3. Markdownlint Issues + +### README.md (root) + +- Line 12: Line too long (83 > 80) +- **Action**: Split long lines or add `.markdownlint.json` to relax line length + +### Agents.md + +- Lines 70, 74, 82: Line too long +- **Action**: Reformat or configure longer line length for docs + +### tools/config-utils/README.md + +**Issues:** + +- MD024: Multiple duplicate headings ("Options", "Examples") +- MD013: Multiple line length violations +- MD040: Missing language specifiers on fenced code blocks + +**Actions:** + +1. Make headings unique (e.g., "Options" → "Command Options", "Subcommand Options") +2. Split long lines or relax line length +3. Add language to code blocks: ` ```bash ` or ` ```yaml ` + +### tools/locust-compare/README.md + +**Issues:** + +- MD013: Line length violations (lines 3, 10, 23, 107, 154, 166, 167, 172, 189) +- MD040: Missing code block languages (lines 76, 140, 151, 177) +- MD033: Inline HTML on line 107 + +**Actions:** + +1. Split long lines +2. Add ` ```bash ` or ` ```python ` to code blocks +3. Replace inline HTML `` with markdown image syntax + +### tools/wt-worktree/README.md + +**Issues:** + +- MD013: Line length violations (lines 3, 7, 73, 75, 140) +- MD040: Missing code block languages (lines 165, 224) + +**Actions:** + +1. Split long lines +2. Add ` ```bash ` to code blocks + +### tools/wt-worktree/PRD.md + +**Issues:** + +- MD013: Line length violations +- MD036: Emphasis used instead of heading (multiple lines) + +**Actions:** + +1. Convert emphasized text to proper headings +2. Split long lines + +### tools/wt-worktree/notes.md + +**Issues:** + +- MD013: Multiple line length violations +- MD040: Missing code block language (line 31) + +**Actions:** + +1. Split long lines +2. Add ` ```bash ` to code block + +--- + +## 4. Configuration Options + +### Option 1: Fix all issues (Recommended for production) + +Work through each section above systematically. + +### Option 2: Relax some rules + +Add to `pyproject.toml`: + +```toml +[tool.ruff.lint] +ignore = [ + # ... existing ignores ... + "PTH123", # Allow open() instead of Path.open() + "PLR0912", # Allow complex branches + "PLR0915", # Allow many statements +] +``` + +Create `.markdownlint.json`: + +```json +{ + "MD013": { "line_length": 120 }, + "MD033": false, + "MD024": false, + "MD036": false +} +``` + +### Option 3: Disable specific hooks temporarily + +Comment out hooks in `.pre-commit-config.yaml` while fixing issues incrementally. + +--- + +## Recommended Fix Order + +1. **Start with auto-fixable issues**: + + ```bash + prek run ruff --all-files + prek run ruff-format --all-files + ``` + +2. **Fix manual ruff issues** (unused variables, pathlib migration) + - Estimated effort: 2-3 hours + - Files: ~30 Python files + +3. **Fix markdown issues** + - Estimated effort: 1-2 hours + - Or relax rules with `.markdownlint.json` + +4. **Fix type checking issues** (most complex) + - Estimated effort: 3-4 hours + - Or temporarily disable pyright hook + +5. **Re-run all hooks**: + + ```bash + prek run --all-files + ``` + +--- + +## Quick Win: Minimal Changes to Pass + +If you want to pass pre-commit quickly without major refactoring: + +1. Add to `pyproject.toml` ignores: + + ```toml + "PTH123", "PTH118", "PTH119", # Allow os.path and open() + "PLR0912", "PLR0915", # Allow complex functions + "E741", # Allow short variable names + "ARG001", # Allow unused arguments + "C401", # Allow generators + ``` + +2. Create `.markdownlint.json` with relaxed rules + +3. Temporarily comment out pyright hook in `.pre-commit-config.yaml` + +This will leave only critical issues (like unused variables and line length) to fix manually.