diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 890150a..5e50557 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1,11 +1,20 @@ --- repos: - - repo: https://github.com/psf/black.git - rev: 24.2.0 + - repo: https://github.com/astral-sh/ruff-pre-commit + rev: v0.8.4 hooks: - - id: black - language_version: python3 - exclude: ^(tests\/hooks-abort-render\/hooks|docs\/HelloCookieCutter1) + # Run the linter + - id: ruff + args: [--fix] + # Run the formatter + - id: ruff-format + + - repo: https://github.com/pycqa/isort + rev: 5.13.2 + hooks: + - id: isort + args: [--profile, plone, --force-alphabetical-sort, --force-single-line, --lines-after-imports, "2"] + additional_dependencies: [setuptools] - repo: https://github.com/pre-commit/mirrors-mypy rev: 'v1.9.0' # Use the sha / tag you want to point at diff --git a/CHANGES.md b/CHANGES.md index 1145e79..16cc531 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -10,6 +10,8 @@ - Modernize type hints to use Python 3.10+ syntax (PEP 604: `X | Y` instead of `Union[X, Y]`) - Use built-in generic types (`list`, `dict`, `tuple`) instead of `typing.List`, `typing.Dict`, `typing.Tuple` [jensens] +- Replace black with ruff for faster linting and formatting. Configure ruff with line-length=120 and appropriate rule selections. Keep isort for import sorting with plone profile and force-alphabetical-sort. This modernizes the tooling stack for better Python 3.10+ support and faster CI runs. + [jensens] ## 4.1.2 (unreleased) diff --git a/CLAUDE.md b/CLAUDE.md index ab7577a..b753ae4 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -117,14 +117,20 @@ Key settings: # Run all pre-commit hooks (using uvx with tox-uv) uvx --with tox-uv tox -e lint -# Run type checking -mypy src/mxdev +# Run ruff linter (with auto-fix) +uvx ruff check --fix src/mxdev tests + +# Run ruff formatter +uvx ruff format src/mxdev tests -# Run flake8 -flake8 src/mxdev +# Sort imports with isort +uvx isort src/mxdev tests + +# Run type checking +uvx mypy src/mxdev -# Sort imports -isort src/mxdev +# Run all pre-commit hooks manually +uvx pre-commit run --all-files ``` ### Testing Multiple Python Versions (using uvx tox with uv) @@ -401,9 +407,16 @@ myext-package_setting = value ## Code Style -- **Formatting**: Black-compatible (max line length: 120) -- **Import sorting**: isort with `force_alphabetical_sort = true`, `force_single_line = true` -- **Type hints**: Use throughout (Python 3.10+ compatible) +- **Formatting**: Ruff formatter (max line length: 120, target Python 3.10+) + - Configured in [pyproject.toml](pyproject.toml) under `[tool.ruff]` + - Rules: E, W, F, UP, D (with selective ignores for docstrings) + - Automatically enforced via pre-commit hooks +- **Import sorting**: isort with plone profile, `force_alphabetical_sort = true`, `force_single_line = true` + - Configured in [pyproject.toml](pyproject.toml) under `[tool.isort]` + - Runs after ruff in pre-commit pipeline +- **Type hints**: Use throughout (Python 3.10+ syntax) + - Use `X | Y` instead of `Union[X, Y]` + - Use `list[T]`, `dict[K, V]` instead of `List[T]`, `Dict[K, V]` - **Path handling**: Prefer `pathlib.Path` over `os.path` for path operations - Use `pathlib.Path().as_posix()` for cross-platform path comparison - Use `/` operator for path joining: `Path("dir") / "file.txt"` diff --git a/Makefile b/Makefile index 80523b1..c26a1fd 100644 --- a/Makefile +++ b/Makefile @@ -6,9 +6,9 @@ #: core.mxenv #: core.mxfiles #: core.packages -#: qa.black #: qa.isort #: qa.mypy +#: qa.ruff #: qa.test # # SETTINGS (ALL CHANGES MADE BELOW SETTINGS WILL BE LOST) @@ -54,18 +54,19 @@ PRIMARY_PYTHON?=3.14 PYTHON_MIN_VERSION?=3.10 # Install packages using the given package installer method. -# Supported are `pip` and `uv`. If uv is used, its global availability is -# checked. Otherwise, it is installed, either in the virtual environment or -# using the `PRIMARY_PYTHON`, dependent on the `VENV_ENABLED` setting. If -# `VENV_ENABLED` and uv is selected, uv is used to create the virtual -# environment. +# Supported are `pip` and `uv`. When `uv` is selected, a global installation +# is auto-detected and used if available. Otherwise, uv is installed in the +# virtual environment or using `PRIMARY_PYTHON`, depending on the +# `VENV_ENABLED` setting. # Default: pip PYTHON_PACKAGE_INSTALLER?=uv -# Flag whether to use a global installed 'uv' or install -# it in the virtual environment. -# Default: false -MXENV_UV_GLOBAL?=true +# Python version for UV to install/use when creating virtual +# environments with global UV. Passed to `uv venv -p VALUE`. Supports version +# specs like `3.11`, `3.14`, `cpython@3.14`. Defaults to PRIMARY_PYTHON value +# for backward compatibility. +# Default: $(PRIMARY_PYTHON) +UV_PYTHON?=$(PRIMARY_PYTHON) # Flag whether to use virtual environment. If `false`, the # interpreter according to `PRIMARY_PYTHON` found in `PATH` is used. @@ -94,17 +95,17 @@ MXDEV?=mxdev # Default: mxmake MXMAKE?=mxmake -## qa.isort +## qa.ruff -# Source folder to scan for Python files to run isort on. +# Source folder to scan for Python files to run ruff on. # Default: src -ISORT_SRC?=src +RUFF_SRC?=src -## qa.black +## qa.isort -# Source folder to scan for Python files to run black on. +# Source folder to scan for Python files to run isort on. # Default: src -BLACK_SRC?=src +ISORT_SRC?=src ## core.mxfiles @@ -199,30 +200,57 @@ else MXENV_PYTHON=$(PRIMARY_PYTHON) endif -# Determine the package installer +# Determine the package installer with non-interactive flags ifeq ("$(PYTHON_PACKAGE_INSTALLER)","uv") -PYTHON_PACKAGE_COMMAND=uv pip +PYTHON_PACKAGE_COMMAND=uv pip --quiet --no-progress else PYTHON_PACKAGE_COMMAND=$(MXENV_PYTHON) -m pip endif +# Auto-detect global uv availability (simple existence check) +ifeq ("$(PYTHON_PACKAGE_INSTALLER)","uv") +UV_AVAILABLE:=$(shell command -v uv >/dev/null 2>&1 && echo "true" || echo "false") +else +UV_AVAILABLE:=false +endif + +# Determine installation strategy +USE_GLOBAL_UV:=$(shell [[ "$(PYTHON_PACKAGE_INSTALLER)" == "uv" && "$(UV_AVAILABLE)" == "true" ]] && echo "true" || echo "false") +USE_LOCAL_UV:=$(shell [[ "$(PYTHON_PACKAGE_INSTALLER)" == "uv" && "$(UV_AVAILABLE)" == "false" ]] && echo "true" || echo "false") + +# Check if global UV is outdated (non-blocking warning) +ifeq ("$(USE_GLOBAL_UV)","true") +UV_OUTDATED:=$(shell uv self update --dry-run 2>&1 | grep -q "Would update" && echo "true" || echo "false") +else +UV_OUTDATED:=false +endif + MXENV_TARGET:=$(SENTINEL_FOLDER)/mxenv.sentinel $(MXENV_TARGET): $(SENTINEL) -ifneq ("$(PYTHON_PACKAGE_INSTALLER)$(MXENV_UV_GLOBAL)","uvfalse") + # Validation: Check Python version if not using global uv +ifneq ("$(USE_GLOBAL_UV)","true") @$(PRIMARY_PYTHON) -c "import sys; vi = sys.version_info; sys.exit(1 if (int(vi[0]), int(vi[1])) >= tuple(map(int, '$(PYTHON_MIN_VERSION)'.split('.'))) else 0)" \ && echo "Need Python >= $(PYTHON_MIN_VERSION)" && exit 1 || : else - @echo "Use Python $(PYTHON_MIN_VERSION) over uv" + @echo "Using global uv for Python $(UV_PYTHON)" endif + # Validation: Check VENV_FOLDER is set if venv enabled @[[ "$(VENV_ENABLED)" == "true" && "$(VENV_FOLDER)" == "" ]] \ && echo "VENV_FOLDER must be configured if VENV_ENABLED is true" && exit 1 || : - @[[ "$(VENV_ENABLED)$(PYTHON_PACKAGE_INSTALLER)" == "falseuv" ]] \ + # Validation: Check uv not used with system Python + @[[ "$(VENV_ENABLED)" == "false" && "$(PYTHON_PACKAGE_INSTALLER)" == "uv" ]] \ && echo "Package installer uv does not work with a global Python interpreter." && exit 1 || : + # Warning: Notify if global UV is outdated +ifeq ("$(UV_OUTDATED)","true") + @echo "WARNING: A newer version of uv is available. Run 'uv self update' to upgrade." +endif + + # Create virtual environment ifeq ("$(VENV_ENABLED)", "true") ifeq ("$(VENV_CREATE)", "true") -ifeq ("$(PYTHON_PACKAGE_INSTALLER)$(MXENV_UV_GLOBAL)","uvtrue") - @echo "Setup Python Virtual Environment using package 'uv' at '$(VENV_FOLDER)'" - @uv venv -p $(PRIMARY_PYTHON) --seed $(VENV_FOLDER) +ifeq ("$(USE_GLOBAL_UV)","true") + @echo "Setup Python Virtual Environment using global uv at '$(VENV_FOLDER)'" + @uv venv --quiet --no-progress -p $(UV_PYTHON) --seed $(VENV_FOLDER) else @echo "Setup Python Virtual Environment using module 'venv' at '$(VENV_FOLDER)'" @$(PRIMARY_PYTHON) -m venv $(VENV_FOLDER) @@ -232,10 +260,14 @@ endif else @echo "Using system Python interpreter" endif -ifeq ("$(PYTHON_PACKAGE_INSTALLER)$(MXENV_UV_GLOBAL)","uvfalse") - @echo "Install uv" + + # Install uv locally if needed +ifeq ("$(USE_LOCAL_UV)","true") + @echo "Install uv in virtual environment" @$(MXENV_PYTHON) -m pip install uv endif + + # Install/upgrade core packages @$(PYTHON_PACKAGE_COMMAND) install -U pip setuptools wheel @echo "Install/Update MXStack Python packages" @$(PYTHON_PACKAGE_COMMAND) install -U $(MXDEV) $(MXMAKE) @@ -263,6 +295,41 @@ INSTALL_TARGETS+=mxenv DIRTY_TARGETS+=mxenv-dirty CLEAN_TARGETS+=mxenv-clean +############################################################################## +# ruff +############################################################################## + +RUFF_TARGET:=$(SENTINEL_FOLDER)/ruff.sentinel +$(RUFF_TARGET): $(MXENV_TARGET) + @echo "Install Ruff" + @$(PYTHON_PACKAGE_COMMAND) install ruff + @touch $(RUFF_TARGET) + +.PHONY: ruff-check +ruff-check: $(RUFF_TARGET) + @echo "Run ruff check" + @ruff check $(RUFF_SRC) + +.PHONY: ruff-format +ruff-format: $(RUFF_TARGET) + @echo "Run ruff format" + @ruff format $(RUFF_SRC) + +.PHONY: ruff-dirty +ruff-dirty: + @rm -f $(RUFF_TARGET) + +.PHONY: ruff-clean +ruff-clean: ruff-dirty + @test -e $(MXENV_PYTHON) && $(MXENV_PYTHON) -m pip uninstall -y ruff || : + @rm -rf .ruff_cache + +INSTALL_TARGETS+=$(RUFF_TARGET) +CHECK_TARGETS+=ruff-check +FORMAT_TARGETS+=ruff-format +DIRTY_TARGETS+=ruff-dirty +CLEAN_TARGETS+=ruff-clean + ############################################################################## # isort ############################################################################## @@ -297,40 +364,6 @@ FORMAT_TARGETS+=isort-format DIRTY_TARGETS+=isort-dirty CLEAN_TARGETS+=isort-clean -############################################################################## -# black -############################################################################## - -BLACK_TARGET:=$(SENTINEL_FOLDER)/black.sentinel -$(BLACK_TARGET): $(MXENV_TARGET) - @echo "Install Black" - @$(PYTHON_PACKAGE_COMMAND) install black - @touch $(BLACK_TARGET) - -.PHONY: black-check -black-check: $(BLACK_TARGET) - @echo "Run black checks" - @black --check $(BLACK_SRC) - -.PHONY: black-format -black-format: $(BLACK_TARGET) - @echo "Run black format" - @black $(BLACK_SRC) - -.PHONY: black-dirty -black-dirty: - @rm -f $(BLACK_TARGET) - -.PHONY: black-clean -black-clean: black-dirty - @test -e $(MXENV_PYTHON) && $(MXENV_PYTHON) -m pip uninstall -y black || : - -INSTALL_TARGETS+=$(BLACK_TARGET) -CHECK_TARGETS+=black-check -FORMAT_TARGETS+=black-format -DIRTY_TARGETS+=black-dirty -CLEAN_TARGETS+=black-clean - ############################################################################## # mxfiles ############################################################################## diff --git a/PLAN_ISSUE_54.md b/PLAN_ISSUE_54.md new file mode 100644 index 0000000..25ee4ed --- /dev/null +++ b/PLAN_ISSUE_54.md @@ -0,0 +1,494 @@ +# Plan to Solve Issue #54: Add Non-Editable Install Mode + +## Problem Summary +Currently, mxdev always installs local packages as **editable** (with `-e` prefix) in the generated requirements file. This is ideal for development but problematic for deployment/Docker containers where packages should be installed to site-packages as standard packages. + +**Current behavior:** +``` +-e ./sources/iaem.mediaarchive +``` + +**Desired behavior for deployment:** +``` +./sources/iaem.mediaarchive +``` + +## Solution +Update `install-mode` configuration with clearer naming and add non-editable mode: + +**New install modes:** +- `editable` (default): Install as editable with `-e` prefix (development) - **NEW NAME** +- `fixed`: Install as standard package without `-e` (deployment) - **NEW MODE** +- `direct`: Deprecated alias for `editable` (backward compatibility) - **DEPRECATED** +- `skip`: Don't install at all (existing) + +## TDD Implementation Steps + +### Step 1: Write Failing Tests First 🔴 + +Following Test-Driven Development, we write tests that define the desired behavior before implementing any code. + +#### 1.1 Add Test Data Files ([tests/data/config_samples/](tests/data/config_samples/)) + +Create test configuration files first: + +**config_editable_mode.ini:** +```ini +[settings] +default-install-mode = editable + +[example.package] +url = git+https://github.com/example/package.git +``` + +**config_fixed_mode.ini:** +```ini +[settings] +default-install-mode = fixed + +[example.package] +url = git+https://github.com/example/package.git +``` + +**config_deprecated_direct.ini:** +```ini +[settings] +default-install-mode = direct # Should log deprecation warning + +[example.package] +url = git+https://github.com/example/package.git +``` + +**config_package_direct.ini:** +```ini +[settings] +default-install-mode = editable + +[example.package] +url = git+https://github.com/example/package.git +install-mode = direct # Should log deprecation warning +``` + +**Update config_invalid_mode.ini:** +```ini +[settings] +default-install-mode = invalid-mode # Should raise error mentioning valid modes + +[example.package] +url = git+https://github.com/example/package.git +``` + +#### 1.2 Add Configuration Tests ([tests/test_config.py](tests/test_config.py)) + +Add tests that will initially FAIL: + +```python +def test_configuration_editable_install_mode(): + """Test Configuration with editable install-mode (new default).""" + from mxdev.config import Configuration + + base = pathlib.Path(__file__).parent / "data" / "config_samples" + config = Configuration(str(base / "config_editable_mode.ini")) + + # Test that editable mode works + pkg = config.packages["example.package"] + assert pkg["install-mode"] == "editable" + + # Test that it's the default (when not specified) + config2 = Configuration(str(base / "config_minimal.ini")) + pkg2 = config2.packages["example.package"] + assert pkg2["install-mode"] == "editable" + + +def test_configuration_fixed_install_mode(): + """Test Configuration with fixed install-mode (new non-editable mode).""" + from mxdev.config import Configuration + + base = pathlib.Path(__file__).parent / "data" / "config_samples" + config = Configuration(str(base / "config_fixed_mode.ini")) + + # Test that fixed mode works + pkg = config.packages["example.package"] + assert pkg["install-mode"] == "fixed" + + +def test_configuration_direct_mode_deprecated(caplog): + """Test that 'direct' mode shows deprecation warning but still works.""" + from mxdev.config import Configuration + import logging + + base = pathlib.Path(__file__).parent / "data" / "config_samples" + + # Test default-install-mode deprecation + with caplog.at_level(logging.WARNING): + config = Configuration(str(base / "config_deprecated_direct.ini")) + + # Verify deprecation warning is logged + assert "install-mode 'direct' is deprecated" in caplog.text + assert "use 'editable' instead" in caplog.text + + # Verify it's treated as 'editable' internally + pkg = config.packages["example.package"] + assert pkg["install-mode"] == "editable" + + # Test per-package level deprecation + caplog.clear() + with caplog.at_level(logging.WARNING): + config2 = Configuration(str(base / "config_package_direct.ini")) + + assert "install-mode 'direct' in package" in caplog.text + + +def test_configuration_invalid_install_mode_new_message(): + """Test that error messages mention new mode names.""" + from mxdev.config import Configuration + + base = pathlib.Path(__file__).parent / "data" / "config_samples" + + # Test invalid default-install-mode + with pytest.raises(ValueError, match="must be one of 'editable', 'fixed', or 'skip'"): + Configuration(str(base / "config_invalid_mode.ini")) +``` + +**Update existing tests:** +```python +# Update test_configuration_invalid_default_install_mode() +def test_configuration_invalid_default_install_mode(): + """Test Configuration with invalid default-install-mode.""" + from mxdev.config import Configuration + + base = pathlib.Path(__file__).parent / "data" / "config_samples" + with pytest.raises(ValueError, match="default-install-mode must be one of 'editable', 'fixed', or 'skip'"): + Configuration(str(base / "config_invalid_mode.ini")) + + +# Update test_configuration_invalid_package_install_mode() +def test_configuration_invalid_package_install_mode(): + """Test Configuration with invalid package install-mode.""" + from mxdev.config import Configuration + + base = pathlib.Path(__file__).parent / "data" / "config_samples" + with pytest.raises(ValueError, match="install-mode in .* must be one of 'editable', 'fixed', or 'skip'"): + Configuration(str(base / "config_package_invalid_mode.ini")) + + +# Update test_configuration_minimal() to verify new default +def test_configuration_minimal(): + """Test Configuration with minimal settings.""" + from mxdev.config import Configuration + + base = pathlib.Path(__file__).parent / "data" / "config_samples" + config = Configuration(str(base / "config_minimal.ini")) + + pkg = config.packages["example.package"] + assert pkg["install-mode"] == "editable" # Changed from "direct" to "editable" +``` + +#### 1.3 Add Processing Tests ([tests/test_processing.py](tests/test_processing.py)) + +Update and add tests that will initially FAIL: + +```python +def test_write_dev_sources(tmp_path): + """Test write_dev_sources() creates correct output for different install modes.""" + from mxdev.processing import write_dev_sources + + packages = { + "editable.package": { + "target": "sources", + "extras": "", + "subdirectory": "", + "install-mode": "editable", # Should output: -e ./sources/editable.package + }, + "fixed.package": { + "target": "sources", + "extras": "", + "subdirectory": "", + "install-mode": "fixed", # Should output: ./sources/fixed.package (no -e) + }, + "skip.package": { + "target": "sources", + "extras": "", + "subdirectory": "", + "install-mode": "skip", # Should not appear in output + }, + "extras.package": { + "target": "sources", + "extras": "test,docs", + "subdirectory": "packages/core", + "install-mode": "fixed", # Test fixed mode with extras and subdirectory + }, + } + + outfile = tmp_path / "requirements.txt" + with open(outfile, "w") as fio: + write_dev_sources(fio, packages) + + content = outfile.read_text() + + # Verify editable mode includes -e prefix + assert "-e ./sources/editable.package\n" in content + + # Verify fixed mode does NOT include -e prefix + assert "./sources/fixed.package\n" in content + assert "-e ./sources/fixed.package" not in content + + # Verify skip mode is not in output + assert "skip.package" not in content + + # Verify fixed mode with extras and subdirectory + assert "./sources/extras.package/packages/core[test,docs]\n" in content + assert "-e ./sources/extras.package" not in content +``` + +### Step 2: Run Tests to Verify They Fail 🔴 + +Run pytest to confirm tests fail (Red phase): + +```bash +source .venv/bin/activate +pytest tests/test_config.py::test_configuration_editable_install_mode -v +pytest tests/test_config.py::test_configuration_fixed_install_mode -v +pytest tests/test_config.py::test_configuration_direct_mode_deprecated -v +pytest tests/test_processing.py::test_write_dev_sources -v +``` + +Expected: All new tests should FAIL because the implementation doesn't exist yet. + +### Step 3: Implement Configuration Changes 🟢 + +Now implement the code to make tests pass. + +#### 3.1 Update Configuration Validation ([config.py](src/mxdev/config.py)) +**Files:** `src/mxdev/config.py` lines 54-55, 111-113 + +Add deprecation handling and new validation: + +**Changes:** +```python +# Line 53-55: Update default-install-mode validation with deprecation +mode = settings.get("default-install-mode", "editable") # Changed default from "direct" + +# Handle deprecated "direct" mode +if mode == "direct": + logger.warning( + "install-mode 'direct' is deprecated and will be removed in a future version. " + "Please use 'editable' instead." + ) + mode = "editable" # Treat as editable internally + +if mode not in ["editable", "fixed", "skip"]: + raise ValueError( + "default-install-mode must be one of 'editable', 'fixed', or 'skip' " + "('direct' is deprecated, use 'editable')" + ) + +# Line 104: Set package install-mode +package.setdefault("install-mode", mode) + +# Line 111-113: Update per-package install-mode validation with deprecation +pkg_mode = package.get("install-mode") + +# Handle deprecated "direct" mode at package level +if pkg_mode == "direct": + logger.warning( + f"install-mode 'direct' in package [{name}] is deprecated and will be removed " + "in a future version. Please use 'editable' instead." + ) + package["install-mode"] = "editable" # Normalize internally + +if package.get("install-mode") not in ["editable", "fixed", "skip"]: + raise ValueError( + f"install-mode in [{name}] must be one of 'editable', 'fixed', or 'skip' " + "('direct' is deprecated, use 'editable')" + ) +``` + +#### 3.2 Update Processing Logic ([processing.py](src/mxdev/processing.py)) +**Files:** `src/mxdev/processing.py` lines 213-227 + +Modify `write_dev_sources()` function to handle the new modes: + +**Changes:** +```python +def write_dev_sources(fio, packages: typing.Dict[str, typing.Dict[str, typing.Any]]): + """Create requirements configuration for fetched source packages.""" + if not packages: + return + fio.write("#" * 79 + "\n") + fio.write("# mxdev development sources\n") + for name, package in packages.items(): + if package["install-mode"] == "skip": + continue + extras = f"[{package['extras']}]" if package["extras"] else "" + subdir = f"/{package['subdirectory']}" if package["subdirectory"] else "" + + # Add -e prefix only for 'editable' mode (not for 'fixed') + prefix = "-e " if package["install-mode"] == "editable" else "" + install_line = f"""{prefix}./{package['target']}/{name}{subdir}{extras}\n""" + + logger.debug(f"-> {install_line.strip()}") + fio.write(install_line) + fio.write("\n\n") +``` + +### Step 4: Run Tests to Verify They Pass 🟢 + +Run pytest again to confirm all tests now pass (Green phase): + +```bash +source .venv/bin/activate +pytest tests/test_config.py::test_configuration_editable_install_mode -v +pytest tests/test_config.py::test_configuration_fixed_install_mode -v +pytest tests/test_config.py::test_configuration_direct_mode_deprecated -v +pytest tests/test_processing.py::test_write_dev_sources -v + +# Run all tests to ensure nothing broke +pytest tests/test_config.py -v +pytest tests/test_processing.py -v +``` + +Expected: All tests should PASS now. + +### Step 5: Update Documentation + +Once tests pass, update user-facing documentation: + +#### README.md +**Location:** Lines 89 and 223 + +Update the tables describing install-mode: + +```markdown +| `default-install-mode` | Default `install-mode` for packages: `editable`, `fixed`, or `skip` | `editable` | +``` + +```markdown +| `install-mode` | `editable`: Install as editable with `pip install -e PACKAGEPATH` (development)
`fixed`: Install as regular package with `pip install PACKAGEPATH` (deployment)
`skip`: Only clone, don't install

**Note:** `direct` is deprecated, use `editable` | `default-install-mode` | +``` + +**Add a Migration/Deprecation Notice section:** +```markdown +### Deprecation Notice + +**`install-mode = direct` is deprecated** and will be removed in a future version. Please update your configuration to use `install-mode = editable` instead. The behavior is identical - only the name has changed for clarity. + +```ini +# Old (deprecated) +[settings] +default-install-mode = direct + +# New (recommended) +[settings] +default-install-mode = editable +``` + +mxdev will log a warning when deprecated mode names are used. +``` + +#### CLAUDE.md +**Location:** Line 215 + +Update description: +```markdown +- Validates install-mode (`editable`, `fixed`, or `skip`; `direct` deprecated), version overrides, and package settings +``` + +**Location:** Line 289-294 + +Update example showing new modes: +```ini +[package1] +url = git+https://github.com/org/package1.git +branch = feature-branch +extras = test +install-mode = editable # For development (with -e) + +[package2] +url = git+https://github.com/org/package2.git +branch = main +install-mode = fixed # For deployment/production (without -e) + +[package3] +url = git+https://github.com/org/package3.git +install-mode = skip # Clone only, don't install +``` + +### Step 6: Update CHANGES.md + +Add entry: +```markdown +- Fix #54: Add `fixed` install-mode option for non-editable installations. Packages with `install-mode = fixed` are installed as regular packages without the `-e` (editable) flag, making them suitable for deployment/production builds where packages should be installed to site-packages. + + **Breaking change (naming only):** Renamed `direct` to `editable` for clarity. The `direct` mode name is still supported but deprecated and will be removed in a future version. Update your configs to use `install-mode = editable` instead of `install-mode = direct`. A deprecation warning is logged when the old name is used. + + [jensens] +``` + +### Step 7: Update Example Configs (Optional) + +**example/mx.ini** - Update any references to show new naming: +```ini +[settings] +default-install-mode = editable # Development (was: direct) + +[some.package] +install-mode = fixed # For production deployment +``` + +## Testing Strategy + +### Manual Testing +1. Create test config with `install-mode = fixed` +2. Run mxdev +3. Verify generated requirements file has packages without `-e` prefix +4. Test that `pip install -r requirements-mxdev.txt` installs to site-packages +5. Test deprecated `install-mode = direct` shows warning but works + +### Automated Testing +1. Unit tests for config validation (all modes including deprecated) +2. Unit tests for deprecation warnings +3. Unit tests for processing output (editable vs fixed) +4. Integration test verifying end-to-end behavior + +## Backward Compatibility + +✅ **Fully backward compatible** +- `direct` mode continues to work (with deprecation warning) +- Existing configs work unchanged +- Default behavior unchanged (still installs as editable) +- Migration path is clear and documented + +## Deprecation Timeline + +**Current Release:** +- Add `editable` and `fixed` modes +- Make `direct` deprecated alias for `editable` +- Log warning when `direct` is used +- Update all documentation + +**Future Release (e.g., 5.0.0):** +- Remove support for `direct` mode +- Raise error if `direct` is used + +## Files to Modify + +1. `src/mxdev/config.py` (validation + deprecation logic) +2. `src/mxdev/processing.py` (output generation) +3. `README.md` (user documentation + migration guide) +4. `CLAUDE.md` (developer documentation) +5. `tests/test_config.py` (configuration tests + deprecation tests) +6. `tests/test_processing.py` (processing tests) +7. `CHANGES.md` (changelog with breaking change notice) +8. `tests/data/config_samples/` (test fixtures) +9. `example/mx.ini` (if exists - update examples) + +## Summary of Changes + +| Old Mode | New Mode | Behavior | Status | +|----------|----------|----------|--------| +| `direct` | `editable` | Install with `-e` flag | Deprecated alias | +| N/A | `fixed` | Install without `-e` flag | **NEW** | +| `skip` | `skip` | Don't install | Unchanged | + +**Default:** `editable` (same behavior as old `direct`, just clearer name) diff --git a/pyproject.toml b/pyproject.toml index 0c27119..6733e2d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -111,27 +111,51 @@ testpaths = [ ] [tool.isort] -profile = "black" +profile = "plone" force_alphabetical_sort = true force_single_line = true lines_after_imports = 2 +[tool.ruff] +line-length = 120 +target-version = "py310" + +[tool.ruff.lint] +# Enable pycodestyle (E/W), pyflakes (F), and pyupgrade (UP) rules +# Note: isort (I) rules are disabled because we use isort directly +select = ["E", "W", "F", "UP", "D"] +# Ignore specific rules that conflict with our style or are too strict +ignore = [ + "D100", "D101", "D102", "D103", "D104", "D105", "D106", "D107", # Missing docstrings + "D202", # No blank lines allowed after function docstring + "D205", # 1 blank line required between summary line and description + "D301", # Use r""" if any backslashes in a docstring + "D400", # First line should end with a period + "D401", # First line should be in imperative mood + "D415", # First line should end with a period, question mark, or exclamation point +] + +[tool.ruff.format] +# Use tabs instead of spaces (if needed), but default is spaces +quote-style = "double" +indent-style = "space" + +[tool.ruff.lint.per-file-ignores] +# Ignore unused variables in tests (often used for mocking) +"tests/**/*.py" = ["F841"] + [tool.mypy] ignore_missing_imports = true [tool.flake8] -# Excludes due to known issues or incompatibilities with black: -# BLK100: Black would make changes. https://pypi.org/project/flake8-black/ -# W503: https://github.com/psf/black/search?q=W503&unscoped_q=W503 -# E231: https://github.com/psf/black/issues/1202 -ignore = "BLK100,E231,W503,D100,D101,D102,D102,D103,D104,D105,D106,D107,D202,D205" +# Note: flake8 is now largely replaced by ruff, but keeping config for compatibility +# Excludes for docstring rules (now handled by ruff) +ignore = "D100,D101,D102,D103,D104,D105,D106,D107,D202,D205" statistics = 1 -# black official is 88, but can get longer max-line-length = 120 [tool.doc8] -# TODO: Remove current max-line-lengh ignore in follow-up and adopt black limit. -# max-line-length = 88 +# Using 120 character line length to match ruff configuration ignore = "D001" [tool.coverage.run] diff --git a/src/mxdev/hooks.py b/src/mxdev/hooks.py index ff55330..b2d42f7 100644 --- a/src/mxdev/hooks.py +++ b/src/mxdev/hooks.py @@ -1,8 +1,6 @@ from .entry_points import load_eps_by_group from .state import State -import typing - try: # do we have Python 3.12+ diff --git a/src/mxdev/including.py b/src/mxdev/including.py index 3fb16a3..f88e83e 100644 --- a/src/mxdev/including.py +++ b/src/mxdev/including.py @@ -6,7 +6,6 @@ import os import tempfile -import typing def resolve_dependencies( diff --git a/src/mxdev/logging.py b/src/mxdev/logging.py index 1cec1ad..de4f915 100644 --- a/src/mxdev/logging.py +++ b/src/mxdev/logging.py @@ -11,8 +11,6 @@ def setup_logger(level: int) -> None: handler = logging.StreamHandler(sys.stdout) handler.setLevel(level) if level == logging.DEBUG: - formatter = logging.Formatter( - "%(asctime)s - %(name)s - %(levelname)s - %(message)s" - ) + formatter = logging.Formatter("%(asctime)s - %(name)s - %(levelname)s - %(message)s") handler.setFormatter(formatter) root.addHandler(handler) diff --git a/src/mxdev/main.py b/src/mxdev/main.py index bf82040..9d3dd3c 100644 --- a/src/mxdev/main.py +++ b/src/mxdev/main.py @@ -27,15 +27,9 @@ type=str, default="mx.ini", ) -parser.add_argument( - "-n", "--no-fetch", help="Do not fetch sources", action="store_true" -) -parser.add_argument( - "-f", "--fetch-only", help="Only fetch sources", action="store_true" -) -parser.add_argument( - "-o", "--offline", help="Do not fetch sources, work offline", action="store_true" -) +parser.add_argument("-n", "--no-fetch", help="Do not fetch sources", action="store_true") +parser.add_argument("-f", "--fetch-only", help="Only fetch sources", action="store_true") +parser.add_argument("-o", "--offline", help="Do not fetch sources, work offline", action="store_true") parser.add_argument( "-t", "--threads", diff --git a/src/mxdev/processing.py b/src/mxdev/processing.py index f212a4d..8855c7d 100644 --- a/src/mxdev/processing.py +++ b/src/mxdev/processing.py @@ -82,9 +82,7 @@ def process_io( and constraint lists. """ for line in fio: - new_requirements, new_constraints = process_line( - line, package_keys, override_keys, ignore_keys, variety - ) + new_requirements, new_constraints = process_line(line, package_keys, override_keys, ignore_keys, variety) requirements += new_requirements constraints += new_constraints @@ -127,8 +125,7 @@ def resolve_dependencies( ) else: logger.info( - f"Can not read {variety_verbose} file '{file_or_url}', " - "it does not exist. Empty file assumed." + f"Can not read {variety_verbose} file '{file_or_url}', " "it does not exist. Empty file assumed." ) else: try: @@ -237,9 +234,7 @@ def write_dev_overrides(fio, overrides: dict[str, str], package_keys: list[str]) fio.write("# mxdev constraint overrides\n") for pkg, line in overrides.items(): if pkg.lower() in [k.lower() for k in package_keys]: - fio.write( - f"# {line} IGNORE mxdev constraint override. Source override wins!\n" - ) + fio.write(f"# {line} IGNORE mxdev constraint override. Source override wins!\n") else: fio.write(f"{line}\n") fio.write("\n\n") diff --git a/src/mxdev/state.py b/src/mxdev/state.py index 9526e52..a272c3f 100644 --- a/src/mxdev/state.py +++ b/src/mxdev/state.py @@ -2,8 +2,6 @@ from dataclasses import dataclass from dataclasses import field -import typing - @dataclass class State: diff --git a/src/mxdev/vcs/bazaar.py b/src/mxdev/vcs/bazaar.py index 7d1796f..ec07155 100644 --- a/src/mxdev/vcs/bazaar.py +++ b/src/mxdev/vcs/bazaar.py @@ -66,13 +66,10 @@ def checkout(self, **kwargs): if update: self.update(**kwargs) elif self.matches(): - self.output( - (logger.info, f"Skipped checkout of existing package {name!r}.") - ) + self.output((logger.info, f"Skipped checkout of existing package {name!r}.")) else: raise BazaarError( - "Source URL for existing package {!r} differs. " - "Expected {!r}.".format(name, self.source["url"]) + "Source URL for existing package {!r} differs. " "Expected {!r}.".format(name, self.source["url"]) ) else: return self.bzr_branch(**kwargs) @@ -114,9 +111,7 @@ def status(self, **kwargs): def update(self, **kwargs): name = self.source["name"] if not self.matches(): - raise BazaarError( - f"Can't update package {name!r} because its URL doesn't match." - ) + raise BazaarError(f"Can't update package {name!r} because its URL doesn't match.") if self.status() != "clean" and not kwargs.get("force", False): raise BazaarError(f"Can't update package {name!r} because it's dirty.") return self.bzr_pull(**kwargs) diff --git a/src/mxdev/vcs/common.py b/src/mxdev/vcs/common.py index a04849b..a2b1289 100644 --- a/src/mxdev/vcs/common.py +++ b/src/mxdev/vcs/common.py @@ -45,8 +45,7 @@ def which(name_root: str, default: str | None = None) -> str: def version_sorted(inp: list, *args, **kwargs) -> list: - """ - Sorts components versions, it means that numeric parts of version + """Sorts components versions, it means that numeric parts of version treats as numeric and string as string. Eg.: version-1-0-1 < version-1-0-2 < version-1-0-10 @@ -173,9 +172,7 @@ def __init__( self.errors = False self.workingcopytypes = get_workingcopytypes() - def _separate_https_packages( - self, packages: list[str] - ) -> tuple[list[str], list[str]]: + def _separate_https_packages(self, packages: list[str]) -> tuple[list[str], list[str]]: """Separate HTTPS packages from others for smart threading. Returns (https_packages, other_packages) @@ -260,20 +257,12 @@ def _checkout_impl(self, packages: list[str], **kwargs) -> None: elif kwargs["update"].lower() in ("false", "no", "off"): kwargs["update"] = False else: - logger.error( - "Unknown value '{}' for always-checkout option.".format( - kwargs["update"] - ) - ) + logger.error("Unknown value '{}' for always-checkout option.".format(kwargs["update"])) sys.exit(1) kwargs.setdefault("submodules", "always") # XXX: submodules is git related, move to GitWorkingCopy if kwargs["submodules"] not in ["always", "never", "checkout", "recursive"]: - logger.error( - "Unknown value '{}' for update-git-submodules option.".format( - kwargs["submodules"] - ) - ) + logger.error("Unknown value '{}' for update-git-submodules option.".format(kwargs["submodules"])) sys.exit(1) for name in packages: kw = kwargs.copy() @@ -295,9 +284,7 @@ def _checkout_impl(self, packages: list[str], **kwargs) -> None: continue elif update and not kw.get("force", False) and wc.status() != "clean": print_stderr(f"The package '{name}' is dirty.") - answer = yesno( - "Do you want to update it anyway?", default=False, all=True - ) + answer = yesno("Do you want to update it anyway?", default=False, all=True) if answer: kw["force"] = True if answer == "all": @@ -409,9 +396,7 @@ def _update_impl(self, packages: list[str], **kwargs) -> None: wc = wc_class(source) if wc.status() != "clean" and not kw.get("force", False): print_stderr(f"The package '{name}' is dirty.") - answer = yesno( - "Do you want to update it anyway?", default=False, all=True - ) + answer = yesno("Do you want to update it anyway?", default=False, all=True) if answer: kw["force"] = True if answer == "all": @@ -444,11 +429,7 @@ def worker(working_copies: WorkingCopies, the_queue: queue.Queue) -> None: with output_lock: for lvl, msg in wc._output: lvl(msg) - if ( - kwargs.get("verbose", False) - and output is not None - and output.strip() - ): + if kwargs.get("verbose", False) and output is not None and output.strip(): if isinstance(output, bytes): output = output.decode("utf8") print(output) diff --git a/src/mxdev/vcs/darcs.py b/src/mxdev/vcs/darcs.py index 9f56a21..5868b4e 100755 --- a/src/mxdev/vcs/darcs.py +++ b/src/mxdev/vcs/darcs.py @@ -49,9 +49,7 @@ def darcs_update(self, **kwargs) -> str | None: ) stdout, stderr = cmd.communicate() if cmd.returncode != 0: - raise DarcsError( - f"darcs pull for '{name}' failed.\n{stderr.decode('utf8')}" - ) + raise DarcsError(f"darcs pull for '{name}' failed.\n{stderr.decode('utf8')}") if kwargs.get("verbose", False): return stdout.decode("utf8") return None @@ -66,13 +64,9 @@ def checkout(self, **kwargs) -> str | None: self.update(**kwargs) return None if self.matches(): - self.output( - (logger.info, f"Skipped checkout of existing package '{name}'.") - ) + self.output((logger.info, f"Skipped checkout of existing package '{name}'.")) return None - raise DarcsError( - f"Checkout URL for existing package '{name}' differs. Expected '{self.source['url']}'." - ) + raise DarcsError(f"Checkout URL for existing package '{name}' differs. Expected '{self.source['url']}'.") def _darcs_related_repositories(self) -> typing.Generator: name = self.source["name"] @@ -134,9 +128,7 @@ def status(self, **kwargs) -> str | tuple[str, str]: def update(self, **kwargs) -> str | None: name = self.source["name"] if not self.matches(): - raise DarcsError( - f"Can't update package '{name}' because it's URL doesn't match." - ) + raise DarcsError(f"Can't update package '{name}' because it's URL doesn't match.") if self.status() != "clean" and not kwargs.get("force", False): raise DarcsError(f"Can't update package '{name}' because it's dirty.") return self.darcs_update(**kwargs) diff --git a/src/mxdev/vcs/filesystem.py b/src/mxdev/vcs/filesystem.py index 60c5c66..a3076ed 100644 --- a/src/mxdev/vcs/filesystem.py +++ b/src/mxdev/vcs/filesystem.py @@ -1,7 +1,6 @@ from . import common import os -import typing logger = common.logger @@ -25,8 +24,9 @@ def checkout(self, **kwargs) -> str | None: ) else: raise FilesystemError( - "Directory name for existing package {!r} differs. " - "Expected {!r}.".format(name, self.source["url"]) + "Directory name for existing package {!r} differs. " "Expected {!r}.".format( + name, self.source["url"] + ) ) else: raise FilesystemError( @@ -48,8 +48,7 @@ def update(self, **kwargs): name = self.source["name"] if not self.matches(): raise FilesystemError( - "Directory name for existing package {!r} differs. " - "Expected {!r}.".format(name, self.source["url"]) + "Directory name for existing package {!r} differs. " "Expected {!r}.".format(name, self.source["url"]) ) self.output((logger.info, f"Filesystem package {name!r} doesn't need update.")) return "" diff --git a/src/mxdev/vcs/git.py b/src/mxdev/vcs/git.py index 6f8abe7..4d5f420 100644 --- a/src/mxdev/vcs/git.py +++ b/src/mxdev/vcs/git.py @@ -5,7 +5,6 @@ import re import subprocess import sys -import typing logger = common.logger @@ -30,8 +29,7 @@ def __init__(self, source: dict[str, str]): self.git_executable = common.which("git") if "rev" in source and "revision" in source: raise ValueError( - "The source definition of '{}' contains " - "duplicate revision options.".format(source["name"]) + "The source definition of '{}' contains " "duplicate revision options.".format(source["name"]) ) # 'rev' is canonical if "revision" in source: @@ -43,8 +41,7 @@ def __init__(self, source: dict[str, str]): del source["branch"] elif "branch" in source: logger.error( - "Cannot specify both branch (%s) and rev/revision " - "(%s) in source for %s", + "Cannot specify both branch (%s) and rev/revision " "(%s) in source for %s", source["branch"], source["rev"], source["name"], @@ -103,9 +100,7 @@ def run_git(self, commands: list[str], **kwargs) -> subprocess.Popen: kwargs["universal_newlines"] = True return subprocess.Popen(commands, **kwargs) - def git_merge_rbranch( - self, stdout_in: str, stderr_in: str, accept_missing: bool = False - ) -> tuple[str, str]: + def git_merge_rbranch(self, stdout_in: str, stderr_in: str, accept_missing: bool = False) -> tuple[str, str]: path = self.source["path"] branch = self.source.get("branch", "master") @@ -129,9 +124,7 @@ def git_merge_rbranch( cmd = self.run_git(["merge", f"{rbp}/{branch}"], cwd=path) stdout, stderr = cmd.communicate() if cmd.returncode != 0: - raise GitError( - f"git merge of remote branch 'origin/{branch}' failed.\n{stderr}" - ) + raise GitError(f"git merge of remote branch 'origin/{branch}' failed.\n{stderr}") return stdout_in + stdout, stderr_in + stderr def git_checkout(self, **kwargs) -> str | None: @@ -169,9 +162,7 @@ def git_checkout(self, **kwargs) -> str | None: # Update only new submodules that we just registered. this is for safety reasons # as git submodule update on modified submodules may cause code loss for submodule in initialized: - stdout, stderr = self.git_update_submodules( - stdout, stderr, submodule=submodule - ) + stdout, stderr = self.git_update_submodules(stdout, stderr, submodule=submodule) self.output( ( logger.info, @@ -183,9 +174,7 @@ def git_checkout(self, **kwargs) -> str | None: return stdout return None - def git_switch_branch( - self, stdout_in: str, stderr_in: str, accept_missing: bool = False - ) -> tuple[str, str]: + def git_switch_branch(self, stdout_in: str, stderr_in: str, accept_missing: bool = False) -> tuple[str, str]: """Switch branches. If accept_missing is True, we do not switch the branch if it @@ -203,16 +192,12 @@ def git_switch_branch( if "rev" in self.source: # A tag or revision was specified instead of a branch argv = ["checkout", self.source["rev"]] - self.output( - (logger.info, "Switching to rev '{}'.".format(self.source["rev"])) - ) + self.output((logger.info, "Switching to rev '{}'.".format(self.source["rev"]))) elif re.search(rf"^(\*| ) {re.escape(branch)}$", stdout, re.M): # the branch is local, normal checkout will work argv = ["checkout", branch] self.output((logger.info, f"Switching to branch '{branch}'.")) - elif re.search( - "^ " + re.escape(rbp) + r"\/" + re.escape(branch) + "$", stdout, re.M - ): + elif re.search("^ " + re.escape(rbp) + r"\/" + re.escape(branch) + "$", stdout, re.M): # the branch is not local, normal checkout won't work here rbranch = f"{rbp}/{branch}" argv = ["checkout", "-b", branch, rbranch] @@ -266,9 +251,7 @@ def git_update(self, **kwargs) -> str | None: cmd = self.run_git(["checkout", branch_value], cwd=path) tag_stdout, tag_stderr = cmd.communicate() if cmd.returncode != 0: - raise GitError( - f"git checkout of tag '{branch_value}' failed.\n{tag_stderr}" - ) + raise GitError(f"git checkout of tag '{branch_value}' failed.\n{tag_stderr}") stdout += tag_stdout stderr += tag_stderr self.output((logger.info, f"Switched to tag '{branch_value}'.")) @@ -315,16 +298,12 @@ def checkout(self, **kwargs) -> str | None: if update: return self.update(**kwargs) elif self.matches(): - self.output( - (logger.info, f"Skipped checkout of existing package '{name}'.") - ) + self.output((logger.info, f"Skipped checkout of existing package '{name}'.")) else: self.output( ( logger.warning, - "Checkout URL for existing package '{}' differs. Expected '{}'.".format( - name, self.source["url"] - ), + "Checkout URL for existing package '{}' differs. Expected '{}'.".format(name, self.source["url"]), ) ) return None @@ -382,9 +361,7 @@ def git_set_pushurl(self, stdout_in, stderr_in) -> tuple[str, str]: if cmd.returncode != 0: raise GitError( - "git config remote.{}.pushurl {} \nfailed.\n".format( - self._upstream_name, self.source["pushurl"] - ) + "git config remote.{}.pushurl {} \nfailed.\n".format(self._upstream_name, self.source["pushurl"]) ) return (stdout_in + stdout, stderr_in + stderr) @@ -399,9 +376,7 @@ def git_init_submodules(self, stdout_in, stderr_in) -> tuple[str, str, list]: initialized_submodules = re.findall(r'\s+[\'"](.*?)[\'"]\s+\(.+\)', output) return (stdout_in + stdout, stderr_in + stderr, initialized_submodules) - def git_update_submodules( - self, stdout_in, stderr_in, submodule="all", recursive: bool = False - ) -> tuple[str, str]: + def git_update_submodules(self, stdout_in, stderr_in, submodule="all", recursive: bool = False) -> tuple[str, str]: params = ["submodule", "update"] if recursive: params.append("--init") diff --git a/src/mxdev/vcs/mercurial.py b/src/mxdev/vcs/mercurial.py index c0891db..e0f9c31 100644 --- a/src/mxdev/vcs/mercurial.py +++ b/src/mxdev/vcs/mercurial.py @@ -58,9 +58,7 @@ def get_rev(self): if branch != "default": if rev: - raise ValueError( - "'branch' and 'rev' parameters cannot be used simultanously" - ) + raise ValueError("'branch' and 'rev' parameters cannot be used simultanously") else: rev = branch else: @@ -118,9 +116,7 @@ def get_tag_name(line): return [tag for tag in tags if tag and tag != "tip"] def _get_newest_tag(self): - mask = self.source.get( - "newest_tag_prefix", self.source.get("newest_tag_mask", "") - ) + mask = self.source.get("newest_tag_prefix", self.source.get("newest_tag_mask", "")) name = self.source["name"] tags = self._get_tags() if mask: @@ -174,13 +170,10 @@ def checkout(self, **kwargs): if update: self.update(**kwargs) elif self.matches(): - self.output( - (logger.info, f"Skipped checkout of existing package {name!r}.") - ) + self.output((logger.info, f"Skipped checkout of existing package {name!r}.")) else: raise MercurialError( - "Source URL for existing package {!r} differs. " - "Expected {!r}.".format(name, self.source["url"]) + "Source URL for existing package {!r} differs. " "Expected {!r}.".format(name, self.source["url"]) ) else: return self.hg_clone(**kwargs) @@ -235,9 +228,7 @@ def status(self, **kwargs): def update(self, **kwargs): name = self.source["name"] if not self.matches(): - raise MercurialError( - f"Can't update package {name!r} because its URL doesn't match." - ) + raise MercurialError(f"Can't update package {name!r} because its URL doesn't match.") if self.status() != "clean" and not kwargs.get("force", False): raise MercurialError(f"Can't update package {name!r} because it's dirty.") return self.hg_pull(**kwargs) diff --git a/src/mxdev/vcs/svn.py b/src/mxdev/vcs/svn.py index 4430500..7a6c512 100644 --- a/src/mxdev/vcs/svn.py +++ b/src/mxdev/vcs/svn.py @@ -7,7 +7,6 @@ import re import subprocess import sys -import typing import xml.etree.ElementTree as etree @@ -53,9 +52,7 @@ def _normalized_url_rev(self): url[2] = path if "rev" in self.source and "revision" in self.source: raise ValueError( - "The source definition of '{}' contains duplicate revision options.".format( - self.source["name"] - ) + "The source definition of '{}' contains duplicate revision options.".format(self.source["name"]) ) if rev is not None and ("rev" in self.source or "revision" in self.source): raise ValueError( @@ -139,11 +136,7 @@ def _svn_error_wrapper(self, f, **kwargs): common.input_lock.release() common.output_lock.release() continue - print( - "Authorization needed for '{}' at '{}'".format( - self.source["name"], self.source["url"] - ) - ) + print("Authorization needed for '{}' at '{}'".format(self.source["name"], self.source["url"])) user = input("Username: ") passwd = getpass.getpass("Password: ") self._svn_auth_cache[root] = dict( @@ -170,9 +163,7 @@ def _svn_error_wrapper(self, f, **kwargs): if answer.lower() in ["r", "t"]: break else: - print( - "Invalid answer, type 'r' for reject or 't' for temporarily." - ) + print("Invalid answer, type 'r' for reject or 't' for temporarily.") if answer == "r": self._svn_cert_cache[root] = False else: @@ -188,11 +179,7 @@ def _svn_checkout(self, **kwargs): args = [self.svn_executable, "checkout", url, path] stdout, stderr, returncode = self._svn_communicate(args, url, **kwargs) if returncode != 0: - raise SVNError( - "Subversion checkout for '{}' failed.\n{}".format( - name, stderr.decode("utf8") - ) - ) + raise SVNError("Subversion checkout for '{}' failed.\n{}".format(name, stderr.decode("utf8"))) if kwargs.get("verbose", False): return stdout.decode("utf8") @@ -217,15 +204,9 @@ def _svn_communicate(self, args, url, **kwargs): stdout, stderr = cmd.communicate() if cmd.returncode != 0: lines = stderr.strip().split(b"\n") - if ( - "authorization failed" in lines[-1] - or "Could not authenticate to server" in lines[-1] - ): + if "authorization failed" in lines[-1] or "Could not authenticate to server" in lines[-1]: raise SVNAuthorizationError(stderr.strip()) - if ( - "Server certificate verification failed: issuer is not trusted" - in lines[-1] - ): + if "Server certificate verification failed: issuer is not trusted" in lines[-1]: cmd = subprocess.Popen( interactive_args, stdin=subprocess.PIPE, @@ -248,11 +229,7 @@ def _svn_info(self): ) stdout, stderr = cmd.communicate() if cmd.returncode != 0: - raise SVNError( - "Subversion info for '{}' failed.\n{}".format( - name, stderr.decode("utf8") - ) - ) + raise SVNError("Subversion info for '{}' failed.\n{}".format(name, stderr.decode("utf8"))) info = etree.fromstring(stdout) result = {} entry = info.find("entry") @@ -280,11 +257,7 @@ def _svn_switch(self, **kwargs): args.insert(2, f"-r{rev}") stdout, stderr, returncode = self._svn_communicate(args, url, **kwargs) if returncode != 0: - raise SVNError( - "Subversion switch of '{}' failed.\n{}".format( - name, stderr.decode("utf8") - ) - ) + raise SVNError("Subversion switch of '{}' failed.\n{}".format(name, stderr.decode("utf8"))) if kwargs.get("verbose", False): return stdout.decode("utf8") @@ -297,11 +270,7 @@ def _svn_update(self, **kwargs): args.insert(2, f"-r{rev}") stdout, stderr, returncode = self._svn_communicate(args, url, **kwargs) if returncode != 0: - raise SVNError( - "Subversion update of '{}' failed.\n{}".format( - name, stderr.decode("utf8") - ) - ) + raise SVNError("Subversion update of '{}' failed.\n{}".format(name, stderr.decode("utf8"))) if kwargs.get("verbose", False): return stdout.decode("utf8") @@ -309,9 +278,7 @@ def svn_checkout(self, **kwargs): name = self.source["name"] path = self.source["path"] if os.path.exists(path): - self.output( - (logger.info, f"Skipped checkout of existing package '{name}'.") - ) + self.output((logger.info, f"Skipped checkout of existing package '{name}'.")) return self.output((logger.info, f"Checked out '{name}' with subversion.")) return self._svn_error_wrapper(self._svn_checkout, **kwargs) @@ -349,15 +316,9 @@ def checkout(self, **kwargs): url = self._svn_info().get("url", "") if url: msg = f"The current checkout of '{name}' is from '{url}'." - msg += ( - "\nCan't switch package to '{}' because it's dirty.".format( - self.source["url"] - ) - ) + msg += "\nCan't switch package to '{}' because it's dirty.".format(self.source["url"]) else: - msg = "Can't switch package '{}' to '{}' because it's dirty.".format( - name, self.source["url"] - ) + msg = "Can't switch package '{}' to '{}' because it's dirty.".format(name, self.source["url"]) raise SVNError(msg) else: return self.svn_checkout(**kwargs) @@ -370,13 +331,9 @@ def matches(self): if rev is None: rev = info.get("revision") if rev.startswith(">="): - return (info.get("url") == url) and ( - int(info.get("revision")) >= int(rev[2:]) - ) + return (info.get("url") == url) and (int(info.get("revision")) >= int(rev[2:])) elif rev.startswith(">"): - return (info.get("url") == url) and ( - int(info.get("revision")) > int(rev[1:]) - ) + return (info.get("url") == url) and (int(info.get("revision")) > int(rev[1:])) else: return (info.get("url") == url) and (info.get("revision") == rev) @@ -390,7 +347,7 @@ def status(self, **kwargs): ) stdout, stderr = cmd.communicate() if cmd.returncode != 0: - raise SVNError(f"Subversion status for '{name}' failed.\n{s(stderr)}") + raise SVNError(f"Subversion status for '{name}' failed.\n{stderr.decode()}") info = etree.fromstring(stdout) clean = True for target in info.findall("target"): @@ -408,9 +365,7 @@ def status(self, **kwargs): ) stdout, stderr = cmd.communicate() if cmd.returncode != 0: - raise SVNError( - f"Subversion status for '{name}' failed.\n{stderr.decode('utf8')}" - ) + raise SVNError(f"Subversion status for '{name}' failed.\n{stderr.decode('utf8')}") return status, stdout.decode("utf8") return status diff --git a/tests/conftest.py b/tests/conftest.py index 523dc25..d29cad0 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,8 +1,8 @@ +from utils import Process + import os import pytest -from utils import Process - @pytest.fixture def tempdir(tmp_path): @@ -37,19 +37,13 @@ def _mkgitrepo(name): @pytest.fixture def git_allow_file_protocol(): - """ - Allow file protocol + """Allow file protocol This is needed for the submodule to be added from a local path """ - from utils import GitRepo shell = Process() - file_allow = ( - shell.check_call("git config --global --get protocol.file.allow")[0] - .decode("utf8") - .strip() - ) - shell.check_call(f"git config --global protocol.file.allow always") + file_allow = shell.check_call("git config --global --get protocol.file.allow")[0].decode("utf8").strip() + shell.check_call("git config --global protocol.file.allow always") yield file_allow shell.check_call(f"git config --global protocol.file.allow {file_allow}") diff --git a/tests/test_common.py b/tests/test_common.py index 54f7665..a6b957f 100644 --- a/tests/test_common.py +++ b/tests/test_common.py @@ -5,7 +5,6 @@ import os import pytest import queue -import typing def test_print_stderr(mocker): @@ -182,9 +181,7 @@ def __init__(self): with pytest.raises(SysExit): wc.checkout(packages=[], submodules="invalid") - assert caplog.messages == [ - "Unknown value 'invalid' for update-git-submodules option." - ] + assert caplog.messages == ["Unknown value 'invalid' for update-git-submodules option."] caplog.clear() with pytest.raises(SysExit): @@ -209,9 +206,7 @@ def __init__(self): caplog.clear() package_dir = tmpdir.mkdir("package_dir") - os.symlink( - package_dir.strpath, tmpdir.join("package").strpath, target_is_directory=True - ) + os.symlink(package_dir.strpath, tmpdir.join("package").strpath, target_is_directory=True) wc.checkout(packages=["package"], update=True) assert caplog.messages == ["Skipped update of linked 'package'."] caplog.clear() @@ -322,9 +317,7 @@ def update(self, **kwargs): exit_mock = mocker.patch("sys.exit") mocker.patch("mxdev.vcs.common._workingcopytypes", {"test": TestWorkingCopy}) - wc = common.WorkingCopies( - sources={"package": {"vcs": "test", "name": "package", "url": "test://url"}} - ) + wc = common.WorkingCopies(sources={"package": {"vcs": "test", "name": "package", "url": "test://url"}}) # Test successful match result = wc.matches({"name": "package"}) @@ -385,9 +378,7 @@ def update(self, **kwargs): exit_mock = mocker.patch("sys.exit") mocker.patch("mxdev.vcs.common._workingcopytypes", {"test": TestWorkingCopy}) - wc = common.WorkingCopies( - sources={"package": {"vcs": "test", "name": "package", "url": "test://url"}} - ) + wc = common.WorkingCopies(sources={"package": {"vcs": "test", "name": "package", "url": "test://url"}}) # Test successful status result = wc.status({"name": "package"}) @@ -455,9 +446,7 @@ def update(self, **kwargs): package_dir = tmp_path / "package" package_dir.mkdir() wc = common.WorkingCopies( - sources={ - "package": {"vcs": "test", "name": "package", "path": str(package_dir)} - }, + sources={"package": {"vcs": "test", "name": "package", "path": str(package_dir)}}, threads=1, ) @@ -473,9 +462,7 @@ def update(self, **kwargs): caplog.clear() # Test with unregistered VCS type - wc.sources = { - "package": {"vcs": "unknown", "name": "package", "path": str(package_dir)} - } + wc.sources = {"package": {"vcs": "unknown", "name": "package", "path": str(package_dir)}} try: wc.update(packages=["package"]) except TypeError: @@ -492,9 +479,7 @@ def update(self, **kwargs): print_stderr = mocker.patch("mxdev.vcs.common.print_stderr") TestWorkingCopy.package_status = "dirty" - wc.sources = { - "package": {"vcs": "test", "name": "package", "path": str(package_dir)} - } + wc.sources = {"package": {"vcs": "test", "name": "package", "path": str(package_dir)}} wc.update(packages=["package"]) print_stderr.assert_called_with("The package 'package' is dirty.") assert "Skipped update of 'package'." in caplog.text diff --git a/tests/test_config.py b/tests/test_config.py index e643b12..de0b322 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -42,10 +42,7 @@ def test_configuration_basic(): assert config.settings["requirements-out"] == "requirements-mxdev.txt" assert config.settings["constraints-out"] == "constraints-mxdev.txt" assert "example.package" in config.packages - assert ( - config.packages["example.package"]["url"] - == "https://github.com/example/package.git" - ) + assert config.packages["example.package"]["url"] == "https://github.com/example/package.git" assert config.packages["example.package"]["branch"] == "main" @@ -123,10 +120,7 @@ def test_configuration_direct_mode_deprecated(caplog): assert config.packages["example.package"]["install-mode"] == "editable" # Should have logged deprecation warning - assert any( - "install-mode 'direct' is deprecated" in record.message - for record in caplog.records - ) + assert any("install-mode 'direct' is deprecated" in record.message for record in caplog.records) def test_configuration_package_direct_mode_deprecated(caplog): @@ -140,10 +134,7 @@ def test_configuration_package_direct_mode_deprecated(caplog): assert config.packages["example.package"]["install-mode"] == "editable" # Should have logged deprecation warning - assert any( - "install-mode 'direct' is deprecated" in record.message - for record in caplog.records - ) + assert any("install-mode 'direct' is deprecated" in record.message for record in caplog.records) def test_configuration_invalid_default_install_mode(): @@ -201,9 +192,7 @@ def test_configuration_override_args_offline(): from mxdev.config import Configuration base = pathlib.Path(__file__).parent / "data" / "config_samples" - config = Configuration( - str(base / "basic_config.ini"), override_args={"offline": True} - ) + config = Configuration(str(base / "basic_config.ini"), override_args={"offline": True}) assert config.settings["offline"] == "true" # Package should inherit offline setting @@ -215,9 +204,7 @@ def test_configuration_override_args_threads(): from mxdev.config import Configuration base = pathlib.Path(__file__).parent / "data" / "config_samples" - config = Configuration( - str(base / "basic_config.ini"), override_args={"threads": 16} - ) + config = Configuration(str(base / "basic_config.ini"), override_args={"threads": 16}) assert config.settings["threads"] == "16" @@ -267,9 +254,7 @@ def test_per_package_target_override(): # Normalize paths for comparison (handles both Unix / and Windows \) assert ( pathlib.Path(pkg_default["path"]).as_posix() - == pathlib.Path(pkg_default["target"]) - .joinpath("package.with.default.target") - .as_posix() + == pathlib.Path(pkg_default["target"]).joinpath("package.with.default.target").as_posix() ) # Package with custom target should use its own target @@ -277,9 +262,7 @@ def test_per_package_target_override(): assert pkg_custom["target"] == "custom-dir" assert ( pathlib.Path(pkg_custom["path"]).as_posix() - == pathlib.Path(pkg_custom["target"]) - .joinpath("package.with.custom.target") - .as_posix() + == pathlib.Path(pkg_custom["target"]).joinpath("package.with.custom.target").as_posix() ) # Package with interpolated target should use the interpolated value @@ -287,7 +270,5 @@ def test_per_package_target_override(): assert pkg_interpolated["target"] == "documentation" assert ( pathlib.Path(pkg_interpolated["path"]).as_posix() - == pathlib.Path(pkg_interpolated["target"]) - .joinpath("package.with.interpolated.target") - .as_posix() + == pathlib.Path(pkg_interpolated["target"]).joinpath("package.with.interpolated.target").as_posix() ) diff --git a/tests/test_entry_points.py b/tests/test_entry_points.py index 4c46370..45de7fa 100644 --- a/tests/test_entry_points.py +++ b/tests/test_entry_points.py @@ -1,5 +1,5 @@ -import pytest -from unittest.mock import patch, MagicMock +from unittest.mock import MagicMock +from unittest.mock import patch def test_has_importlib_entrypoints_constant(): @@ -22,9 +22,7 @@ def test_load_eps_by_group_with_python312(): mock_eps = [mock_ep1, mock_ep2] with patch("mxdev.entry_points.HAS_IMPORTLIB_ENTRYPOINTS", True): - with patch( - "mxdev.entry_points.entry_points", return_value=mock_eps - ) as mock_entry_points: + with patch("mxdev.entry_points.entry_points", return_value=mock_eps) as mock_entry_points: result = load_eps_by_group("test-group") # Should call entry_points with group parameter @@ -48,9 +46,7 @@ def test_load_eps_by_group_with_old_python(): mock_eps_dict = {"test-group": [mock_ep1, mock_ep2], "other-group": []} with patch("mxdev.entry_points.HAS_IMPORTLIB_ENTRYPOINTS", False): - with patch( - "mxdev.entry_points.entry_points", return_value=mock_eps_dict - ) as mock_entry_points: + with patch("mxdev.entry_points.entry_points", return_value=mock_eps_dict) as mock_entry_points: result = load_eps_by_group("test-group") # Should call entry_points without parameters diff --git a/tests/test_git.py b/tests/test_git.py index 935dceb..f33233c 100644 --- a/tests/test_git.py +++ b/tests/test_git.py @@ -1,16 +1,14 @@ # pylint: disable=redefined-outer-name from logging import getLogger from logging import Logger -from struct import pack from unittest.mock import patch - -import os -import pytest - from utils import vcs_checkout from utils import vcs_status from utils import vcs_update +import os +import pytest + logger: Logger = getLogger("vcs_test_git") @@ -110,9 +108,7 @@ def test_update_with_revision_pin_branch(mkgitrepo, src): vcs_update(sources, packages, verbose) assert {x for x in path.iterdir()} == {path / ".git", path / "foo", path / "foo2"} - sources = { - "egg": dict(vcs="git", name="egg", url=str(repository.base), path=str(path)) - } + sources = {"egg": dict(vcs="git", name="egg", url=str(repository.base), path=str(path))} vcs_update(sources, packages, verbose) assert {x for x in path.iterdir()} == {path / ".git", path / "bar", path / "foo"} diff --git a/tests/test_git_additional.py b/tests/test_git_additional.py index 33807a6..1b7bc0b 100644 --- a/tests/test_git_additional.py +++ b/tests/test_git_additional.py @@ -1,15 +1,15 @@ """Additional tests for git.py to increase coverage to >90%.""" -import os +from unittest.mock import Mock +from unittest.mock import patch + import pytest -from unittest.mock import Mock, patch, MagicMock -import sys def test_git_error_class(): """Test GitError exception class.""" - from mxdev.vcs.git import GitError from mxdev.vcs.common import WCError + from mxdev.vcs.git import GitError assert issubclass(GitError, WCError) @@ -228,7 +228,8 @@ def test_remote_branch_prefix_new_git(): def test_git_merge_rbranch_failure(): """Test git_merge_rbranch handles git branch failure.""" - from mxdev.vcs.git import GitWorkingCopy, GitError + from mxdev.vcs.git import GitError + from mxdev.vcs.git import GitWorkingCopy with patch("mxdev.vcs.common.which", return_value="/usr/bin/git"): source = { @@ -298,7 +299,8 @@ def test_git_merge_rbranch_missing_branch_no_accept(): def test_git_merge_rbranch_merge_failure(): """Test git_merge_rbranch handles merge failure.""" - from mxdev.vcs.git import GitWorkingCopy, GitError + from mxdev.vcs.git import GitError + from mxdev.vcs.git import GitWorkingCopy with patch("mxdev.vcs.common.which", return_value="/usr/bin/git"): source = { @@ -349,7 +351,8 @@ def test_git_checkout_existing_path(tmp_path): def test_git_checkout_clone_failure(): """Test git_checkout handles clone failure.""" - from mxdev.vcs.git import GitWorkingCopy, GitError + from mxdev.vcs.git import GitError + from mxdev.vcs.git import GitWorkingCopy with patch("mxdev.vcs.common.which", return_value="/usr/bin/git"): source = { @@ -442,9 +445,7 @@ def test_git_checkout_with_pushurl(): mock_process.communicate.return_value = ("", "") with patch.object(wc, "run_git", return_value=mock_process): - with patch.object( - wc, "git_set_pushurl", return_value=("", "") - ) as mock_pushurl: + with patch.object(wc, "git_set_pushurl", return_value=("", "")) as mock_pushurl: wc.git_checkout(submodules="never") # Verify git_set_pushurl was called mock_pushurl.assert_called_once() @@ -452,7 +453,8 @@ def test_git_checkout_with_pushurl(): def test_git_set_pushurl_failure(): """Test git_set_pushurl handles failure.""" - from mxdev.vcs.git import GitWorkingCopy, GitError + from mxdev.vcs.git import GitError + from mxdev.vcs.git import GitWorkingCopy with patch("mxdev.vcs.common.which", return_value="/usr/bin/git"): source = { @@ -475,7 +477,8 @@ def test_git_set_pushurl_failure(): def test_git_init_submodules_failure(): """Test git_init_submodules handles failure.""" - from mxdev.vcs.git import GitWorkingCopy, GitError + from mxdev.vcs.git import GitError + from mxdev.vcs.git import GitWorkingCopy with patch("mxdev.vcs.common.which", return_value="/usr/bin/git"): source = { @@ -522,7 +525,8 @@ def test_git_init_submodules_stderr_output(): def test_git_update_submodules_failure(): """Test git_update_submodules handles failure.""" - from mxdev.vcs.git import GitWorkingCopy, GitError + from mxdev.vcs.git import GitError + from mxdev.vcs.git import GitWorkingCopy with patch("mxdev.vcs.common.which", return_value="/usr/bin/git"): source = { @@ -606,9 +610,7 @@ def test_checkout_path_not_exist(): wc = GitWorkingCopy(source) - with patch.object( - wc, "git_checkout", return_value="checkout output" - ) as mock_checkout: + with patch.object(wc, "git_checkout", return_value="checkout output") as mock_checkout: result = wc.checkout(submodules="never") # Should call git_checkout @@ -631,9 +633,7 @@ def test_checkout_update_needed(): with patch("os.path.exists", return_value=True): with patch.object(wc, "should_update", return_value=True): - with patch.object( - wc, "update", return_value="update output" - ) as mock_update: + with patch.object(wc, "update", return_value="update output") as mock_update: result = wc.checkout() mock_update.assert_called_once() @@ -664,7 +664,8 @@ def test_checkout_no_update_doesnt_match(): def test_matches_failure(): """Test matches() handles git remote failure.""" - from mxdev.vcs.git import GitWorkingCopy, GitError + from mxdev.vcs.git import GitError + from mxdev.vcs.git import GitWorkingCopy with patch("mxdev.vcs.common.which", return_value="/usr/bin/git"): source = { @@ -699,9 +700,7 @@ def test_update_not_matching(): with patch.object(wc, "matches", return_value=False): with patch.object(wc, "status", return_value="clean"): - with patch.object( - wc, "git_update", return_value="updated" - ) as mock_git_update: + with patch.object(wc, "git_update", return_value="updated") as mock_git_update: result = wc.update() # Should still call git_update even if not matching @@ -710,7 +709,8 @@ def test_update_not_matching(): def test_update_dirty_no_force(): """Test update with dirty status and no force.""" - from mxdev.vcs.git import GitWorkingCopy, GitError + from mxdev.vcs.git import GitError + from mxdev.vcs.git import GitWorkingCopy with patch("mxdev.vcs.common.which", return_value="/usr/bin/git"): source = { @@ -742,9 +742,7 @@ def test_update_dirty_with_force(): with patch.object(wc, "matches", return_value=True): with patch.object(wc, "status", return_value="dirty"): - with patch.object( - wc, "git_update", return_value="updated" - ) as mock_git_update: + with patch.object(wc, "git_update", return_value="updated") as mock_git_update: result = wc.update(force=True) # Should call git_update when forced @@ -753,7 +751,8 @@ def test_update_dirty_with_force(): def test_git_update_fetch_failure(): """Test git_update handles fetch failure.""" - from mxdev.vcs.git import GitWorkingCopy, GitError + from mxdev.vcs.git import GitError + from mxdev.vcs.git import GitWorkingCopy with patch("mxdev.vcs.common.which", return_value="/usr/bin/git"): source = { @@ -865,7 +864,8 @@ def test_status_verbose(): def test_git_switch_branch_failure(): """Test git_switch_branch handles branch -a failure.""" - from mxdev.vcs.git import GitWorkingCopy, GitError + from mxdev.vcs.git import GitError + from mxdev.vcs.git import GitWorkingCopy with patch("mxdev.vcs.common.which", return_value="/usr/bin/git"): source = { @@ -936,7 +936,8 @@ def test_git_switch_branch_missing_accept(): def test_git_switch_branch_checkout_failure(): """Test git_switch_branch handles checkout failure.""" - from mxdev.vcs.git import GitWorkingCopy, GitError + from mxdev.vcs.git import GitError + from mxdev.vcs.git import GitWorkingCopy with patch("mxdev.vcs.common.which", return_value="/usr/bin/git"): source = { diff --git a/tests/test_git_submodules.py b/tests/test_git_submodules.py index 36386b1..a55a099 100644 --- a/tests/test_git_submodules.py +++ b/tests/test_git_submodules.py @@ -1,20 +1,15 @@ from unittest.mock import patch - -import os -import pytest - from utils import GitRepo from utils import vcs_checkout from utils import vcs_update +import os +import pytest -@pytest.mark.skipif( - condition=os.name == "nt", reason="submodules seem not to work on windows" -) + +@pytest.mark.skipif(condition=os.name == "nt", reason="submodules seem not to work on windows") def test_checkout_with_submodule(mkgitrepo, src, caplog, git_allow_file_protocol): - """ - Tests the checkout of a module 'egg' with a submodule 'submodule_a' in itith - """ + """Tests the checkout of a module 'egg' with a submodule 'submodule_a' in itith""" submodule_name = "submodule_a" submodule_a = mkgitrepo(submodule_name) @@ -48,12 +43,9 @@ def test_checkout_with_submodule(mkgitrepo, src, caplog, git_allow_file_protocol ) -@pytest.mark.skipif( - condition=os.name == "nt", reason="submodules seem not to work on windows" -) +@pytest.mark.skipif(condition=os.name == "nt", reason="submodules seem not to work on windows") def test_checkout_with_two_submodules(mkgitrepo, src, git_allow_file_protocol): - """ - Tests the checkout of a module 'egg' with a submodule 'submodule_a' + """Tests the checkout of a module 'egg' with a submodule 'submodule_a' and a submodule 'submodule_b' in it. """ @@ -100,14 +92,9 @@ def test_checkout_with_two_submodules(mkgitrepo, src, git_allow_file_protocol): ] -@pytest.mark.skipif( - condition=os.name == "nt", reason="submodules seem not to work on windows" -) -def test_checkout_with_two_submodules_recursive( - mkgitrepo, src, git_allow_file_protocol -): - """ - Tests the checkout of a module 'egg' with a submodule 'submodule_a' +@pytest.mark.skipif(condition=os.name == "nt", reason="submodules seem not to work on windows") +def test_checkout_with_two_submodules_recursive(mkgitrepo, src, git_allow_file_protocol): + """Tests the checkout of a module 'egg' with a submodule 'submodule_a' and a submodule 'submodule_b' in it. but this time we test it with the "recursive" option """ @@ -145,12 +132,9 @@ def test_checkout_with_two_submodules_recursive( ] -@pytest.mark.skipif( - condition=os.name == "nt", reason="submodules seem not to work on windows" -) +@pytest.mark.skipif(condition=os.name == "nt", reason="submodules seem not to work on windows") def test_update_with_submodule(mkgitrepo, src, git_allow_file_protocol): - """ - Tests the checkout of a module 'egg' with a submodule 'submodule_a' in it. + """Tests the checkout of a module 'egg' with a submodule 'submodule_a' in it. Add a new 'submodule_b' to 'egg' and check it succesfully initializes. """ submodule_name = "submodule_a" @@ -208,12 +192,9 @@ def test_update_with_submodule(mkgitrepo, src, git_allow_file_protocol): ] -@pytest.mark.skipif( - condition=os.name == "nt", reason="submodules seem not to work on windows" -) +@pytest.mark.skipif(condition=os.name == "nt", reason="submodules seem not to work on windows") def test_update_with_submodule_recursive(mkgitrepo, src, git_allow_file_protocol): - """ - Tests the checkout of a module 'egg' with a submodule 'submodule_a' in it. + """Tests the checkout of a module 'egg' with a submodule 'submodule_a' in it. Add a new 'submodule_b' to 'egg' and check it succesfully initializes. """ submodule_name = "submodule_a" @@ -256,8 +237,8 @@ def test_update_with_submodule_recursive(mkgitrepo, src, git_allow_file_protocol "foo_b", } assert log.method_calls == [ - ("info", (f"Updated 'egg' with git.",)), - ("info", (f"Switching to branch 'master'.",)), + ("info", ("Updated 'egg' with git.",)), + ("info", ("Switching to branch 'master'.",)), ( "info", (f"Initialized 'egg' submodule at '{submodule_b_name}' with git.",), @@ -265,12 +246,9 @@ def test_update_with_submodule_recursive(mkgitrepo, src, git_allow_file_protocol ] -@pytest.mark.skipif( - condition=os.name == "nt", reason="submodules seem not to work on windows" -) +@pytest.mark.skipif(condition=os.name == "nt", reason="submodules seem not to work on windows") def test_checkout_with_submodules_option_never(mkgitrepo, src, git_allow_file_protocol): - """ - Tests the checkout of a module 'egg' with a submodule 'submodule_a' in it + """Tests the checkout of a module 'egg' with a submodule 'submodule_a' in it without initializing the submodule, restricted by global 'never' """ @@ -292,19 +270,12 @@ def test_checkout_with_submodules_option_never(mkgitrepo, src, git_allow_file_pr ".gitmodules", } assert set(os.listdir(src / "egg" / submodule_name)) == set() - assert log.method_calls == [ - ("info", (f"Cloned 'egg' with git from '{egg.url}'.",), {}) - ] + assert log.method_calls == [("info", (f"Cloned 'egg' with git from '{egg.url}'.",), {})] -@pytest.mark.skipif( - condition=os.name == "nt", reason="submodules seem not to work on windows" -) -def test_checkout_with_submodules_option_never_source_always( - mkgitrepo, src, git_allow_file_protocol -): - """ - Tests the checkout of a module 'egg' with a submodule 'submodule_a' in it +@pytest.mark.skipif(condition=os.name == "nt", reason="submodules seem not to work on windows") +def test_checkout_with_submodules_option_never_source_always(mkgitrepo, src, git_allow_file_protocol): + """Tests the checkout of a module 'egg' with a submodule 'submodule_a' in it and a module 'egg2' with the same submodule, initializing only the submodule on egg that has the 'always' option """ @@ -331,9 +302,7 @@ def test_checkout_with_submodules_option_never_source_always( "egg2": dict(vcs="git", name="egg2", url=egg2.url, path=src / "egg2"), } with patch("mxdev.vcs.git.logger") as log: - vcs_checkout( - sources, ["egg", "egg2"], verbose=False, update_git_submodules="never" - ) + vcs_checkout(sources, ["egg", "egg2"], verbose=False, update_git_submodules="never") assert set(os.listdir(src / "egg")) == { "submodule_a", ".git", @@ -360,14 +329,9 @@ def test_checkout_with_submodules_option_never_source_always( ] -@pytest.mark.skipif( - condition=os.name == "nt", reason="submodules seem not to work on windows" -) -def test_checkout_with_submodules_option_always_source_never( - mkgitrepo, src, git_allow_file_protocol -): - """ - Tests the checkout of a module 'egg' with a submodule 'submodule_a' in it +@pytest.mark.skipif(condition=os.name == "nt", reason="submodules seem not to work on windows") +def test_checkout_with_submodules_option_always_source_never(mkgitrepo, src, git_allow_file_protocol): + """Tests the checkout of a module 'egg' with a submodule 'submodule_a' in it and a module 'egg2' with the same submodule, not initializing the submodule on egg2 that has the 'never' option @@ -422,12 +386,9 @@ def test_checkout_with_submodules_option_always_source_never( ] -@pytest.mark.skipif( - condition=os.name == "nt", reason="submodules seem not to work on windows" -) +@pytest.mark.skipif(condition=os.name == "nt", reason="submodules seem not to work on windows") def test_update_with_submodule_checkout(mkgitrepo, src, git_allow_file_protocol): - """ - Tests the checkout of a module 'egg' with a submodule 'submodule_a' in it. + """Tests the checkout of a module 'egg' with a submodule 'submodule_a' in it. Add a new 'submodule_b' to 'egg' and check it doesn't get initialized. """ @@ -486,14 +447,9 @@ def test_update_with_submodule_checkout(mkgitrepo, src, git_allow_file_protocol) ] -@pytest.mark.skipif( - condition=os.name == "nt", reason="submodules seem not to work on windows" -) -def test_update_with_submodule_dont_update_previous_submodules( - mkgitrepo, src, git_allow_file_protocol -): - """ - Tests the checkout of a module 'egg' with a submodule 'submodule_a' in it. +@pytest.mark.skipif(condition=os.name == "nt", reason="submodules seem not to work on windows") +def test_update_with_submodule_dont_update_previous_submodules(mkgitrepo, src, git_allow_file_protocol): + """Tests the checkout of a module 'egg' with a submodule 'submodule_a' in it. Commits changes in the detached submodule, and checks update didn't break the changes. """ diff --git a/tests/test_hooks.py b/tests/test_hooks.py index 422ceb8..6ce7856 100644 --- a/tests/test_hooks.py +++ b/tests/test_hooks.py @@ -1,5 +1,5 @@ -import pytest -from unittest.mock import MagicMock, patch +from unittest.mock import MagicMock +from unittest.mock import patch def test_hook_class_exists(): @@ -25,9 +25,10 @@ def test_hook_class_can_be_instantiated(): def test_hook_read_method(): """Test Hook class has read method.""" + from mxdev.config import Configuration from mxdev.hooks import Hook from mxdev.state import State - from mxdev.config import Configuration + import pathlib base = pathlib.Path(__file__).parent / "data" / "config_samples" @@ -42,9 +43,10 @@ def test_hook_read_method(): def test_hook_write_method(): """Test Hook class has write method.""" + from mxdev.config import Configuration from mxdev.hooks import Hook from mxdev.state import State - from mxdev.config import Configuration + import pathlib base = pathlib.Path(__file__).parent / "data" / "config_samples" @@ -100,9 +102,7 @@ def test_load_hooks_filters_by_name(): mock_ep_other = MagicMock() mock_ep_other.name = "other" - with patch( - "mxdev.hooks.load_eps_by_group", return_value=[mock_ep_hook, mock_ep_other] - ): + with patch("mxdev.hooks.load_eps_by_group", return_value=[mock_ep_hook, mock_ep_other]): hooks = load_hooks() assert len(hooks) == 1 assert hooks[0] == mock_hook_instance @@ -112,9 +112,10 @@ def test_load_hooks_filters_by_name(): def test_read_hooks(): """Test read_hooks calls read on all hooks.""" + from mxdev.config import Configuration from mxdev.hooks import read_hooks from mxdev.state import State - from mxdev.config import Configuration + import pathlib base = pathlib.Path(__file__).parent / "data" / "config_samples" @@ -133,9 +134,10 @@ def test_read_hooks(): def test_read_hooks_empty_list(): """Test read_hooks with empty hooks list.""" + from mxdev.config import Configuration from mxdev.hooks import read_hooks from mxdev.state import State - from mxdev.config import Configuration + import pathlib base = pathlib.Path(__file__).parent / "data" / "config_samples" @@ -148,9 +150,10 @@ def test_read_hooks_empty_list(): def test_write_hooks(): """Test write_hooks calls write on all hooks.""" + from mxdev.config import Configuration from mxdev.hooks import write_hooks from mxdev.state import State - from mxdev.config import Configuration + import pathlib base = pathlib.Path(__file__).parent / "data" / "config_samples" @@ -169,9 +172,10 @@ def test_write_hooks(): def test_write_hooks_empty_list(): """Test write_hooks with empty hooks list.""" + from mxdev.config import Configuration from mxdev.hooks import write_hooks from mxdev.state import State - from mxdev.config import Configuration + import pathlib base = pathlib.Path(__file__).parent / "data" / "config_samples" diff --git a/tests/test_logging.py b/tests/test_logging.py index fde23e7..4290c23 100644 --- a/tests/test_logging.py +++ b/tests/test_logging.py @@ -99,10 +99,7 @@ def test_setup_logger_no_formatter_for_info(): # The code only sets formatter for DEBUG if handler.formatter: # If a formatter exists, it shouldn't be the DEBUG formatter - assert ( - "%(asctime)s" not in handler.formatter._fmt - or handler.formatter._fmt is None - ) + assert "%(asctime)s" not in handler.formatter._fmt or handler.formatter._fmt is None def test_emoji_logging_with_cp1252_encoding(capsys, caplog): @@ -123,9 +120,7 @@ def test_emoji_logging_with_cp1252_encoding(capsys, caplog): # Create a stream with cp1252 encoding (simulating Windows console) # Use errors='strict' to ensure it raises on unencodable characters - stream = io.TextIOWrapper( - io.BytesIO(), encoding="cp1252", errors="strict", line_buffering=True - ) + stream = io.TextIOWrapper(io.BytesIO(), encoding="cp1252", errors="strict", line_buffering=True) # Set up handler with the cp1252 stream handler = logging.StreamHandler(stream) diff --git a/tests/test_main.py b/tests/test_main.py index 023d5f7..b72f114 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -1,9 +1,5 @@ -import io -import pathlib -import pytest -import sys -from unittest.mock import patch, MagicMock -import logging +from unittest.mock import MagicMock +from unittest.mock import patch def test_parser_defaults(): diff --git a/tests/test_mercurial.py b/tests/test_mercurial.py index 438edf1..cf59ce6 100644 --- a/tests/test_mercurial.py +++ b/tests/test_mercurial.py @@ -1,10 +1,9 @@ from unittest.mock import patch +from utils import Process import os import pytest -from utils import Process - class TestMercurial: @pytest.mark.skip("Needs rewrite") @@ -71,7 +70,7 @@ def testUpdateWithRevisionPin(self, develop, src, tempdir): try: # XXX older version - rev = lines[0].split()[1].split(b(":"))[1] + rev = lines[0].split()[1].split(b":")[1] except Exception: rev = lines[0].split()[1] diff --git a/tests/test_processing.py b/tests/test_processing.py index 6c77989..b470302 100644 --- a/tests/test_processing.py +++ b/tests/test_processing.py @@ -1,7 +1,7 @@ +from io import StringIO + import os import pathlib -import pytest -from io import StringIO def test_process_line_plain(): @@ -159,6 +159,7 @@ def test_resolve_dependencies_file_not_found(): def test_resolve_dependencies_with_constraints(): """Test resolve_dependencies with -c constraint reference.""" from mxdev.processing import resolve_dependencies + import os base = pathlib.Path(__file__).parent / "data" / "requirements" @@ -184,6 +185,7 @@ def test_resolve_dependencies_with_constraints(): def test_resolve_dependencies_nested(): """Test resolve_dependencies with -r nested requirements.""" from mxdev.processing import resolve_dependencies + import os base = pathlib.Path(__file__).parent / "data" / "requirements" @@ -314,8 +316,8 @@ def test_write_dev_sources_mixed_modes(tmp_path): def test_write_dev_sources_empty(): """Test write_dev_sources with no packages.""" - from mxdev.processing import write_dev_sources from io import StringIO + from mxdev.processing import write_dev_sources fio = StringIO() write_dev_sources(fio, {}) @@ -376,8 +378,8 @@ def test_write_main_package(tmp_path): def test_write_main_package_not_set(): """Test write_main_package when main-package not set.""" - from mxdev.processing import write_main_package from io import StringIO + from mxdev.processing import write_main_package settings = {} fio = StringIO() @@ -389,9 +391,9 @@ def test_write_main_package_not_set(): def test_write(tmp_path): """Test write function creates output files correctly.""" + from mxdev.config import Configuration from mxdev.processing import write from mxdev.state import State - from mxdev.config import Configuration # Create a simple config config_file = tmp_path / "mx.ini" @@ -438,9 +440,9 @@ def test_write(tmp_path): def test_write_no_constraints(tmp_path): """Test write function when there are no constraints.""" + from mxdev.config import Configuration from mxdev.processing import write from mxdev.state import State - from mxdev.config import Configuration # Create a simple config without constraints config_file = tmp_path / "mx.ini" @@ -486,9 +488,10 @@ def test_relative_constraints_path_in_subdirectory(tmp_path): This reproduces issue #22: when requirements-out and constraints-out are in subdirectories, the constraints reference should be relative to the requirements file's directory. """ - from mxdev.processing import read, write - from mxdev.state import State from mxdev.config import Configuration + from mxdev.processing import read + from mxdev.processing import write + from mxdev.state import State old_cwd = os.getcwd() try: @@ -530,8 +533,7 @@ def test_relative_constraints_path_in_subdirectory(tmp_path): # Bug: Currently writes "-c requirements/constraints.txt" # Expected: Should write "-c constraints.txt" (relative to requirements file's directory) assert "-c constraints.txt\n" in req_content, ( - f"Expected '-c constraints.txt' (relative path), " - f"but got:\n{req_content}" + f"Expected '-c constraints.txt' (relative path), " f"but got:\n{req_content}" ) # Should NOT contain the full path from config file's perspective @@ -542,9 +544,10 @@ def test_relative_constraints_path_in_subdirectory(tmp_path): def test_relative_constraints_path_different_directories(tmp_path): """Test constraints path when requirements and constraints are in different directories.""" - from mxdev.processing import read, write - from mxdev.state import State from mxdev.config import Configuration + from mxdev.processing import read + from mxdev.processing import write + from mxdev.state import State old_cwd = os.getcwd() try: @@ -584,8 +587,7 @@ def test_relative_constraints_path_different_directories(tmp_path): # Should write path relative to reqs/ directory # From reqs/ to constraints/constraints.txt = ../constraints/constraints.txt assert "-c ../constraints/constraints.txt\n" in req_content, ( - f"Expected '-c ../constraints/constraints.txt' (relative path), " - f"but got:\n{req_content}" + f"Expected '-c ../constraints/constraints.txt' (relative path), " f"but got:\n{req_content}" ) finally: os.chdir(old_cwd) diff --git a/tests/test_svn.py b/tests/test_svn.py index 236fee3..3896772 100644 --- a/tests/test_svn.py +++ b/tests/test_svn.py @@ -1,10 +1,9 @@ from unittest.mock import patch +from utils import Process import os import pytest -from utils import Process - class TestSVN: @pytest.fixture(autouse=True) @@ -31,11 +30,7 @@ def testUpdateWithoutRevisionPin(self, develop, src, tempdir): bar.create_file("bar") process.check_call(f"svn add {bar}", echo=False) process.check_call(f"svn commit {bar} -m bar", echo=False) - develop.sources = { - "egg": dict( - kind="svn", name="egg", url=f"file://{repository}", path=src["egg"] - ) - } + develop.sources = {"egg": dict(kind="svn", name="egg", url=f"file://{repository}", path=src["egg"])} _log = patch("mxdev.vcs.svn.logger") log = _log.__enter__() try: @@ -68,11 +63,7 @@ def testUpdateWithRevisionPin(self, develop, src, tempdir): bar.create_file("bar") process.check_call(f"svn add {bar}", echo=False) process.check_call(f"svn commit {bar} -m bar", echo=False) - develop.sources = { - "egg": dict( - kind="svn", name="egg", url=f"file://{repository}@1", path=src["egg"] - ) - } + develop.sources = {"egg": dict(kind="svn", name="egg", url=f"file://{repository}@1", path=src["egg"])} CmdCheckout(develop)(develop.parser.parse_args(["co", "egg"])) assert set(os.listdir(src["egg"])) == {".svn", "foo"} CmdUpdate(develop)(develop.parser.parse_args(["up", "egg"])) diff --git a/tests/test_vcs_filesystem.py b/tests/test_vcs_filesystem.py index def5abd..58517b7 100644 --- a/tests/test_vcs_filesystem.py +++ b/tests/test_vcs_filesystem.py @@ -1,11 +1,10 @@ -import os import pytest def test_filesystem_error_exists(): """Test FilesystemError exception class exists.""" - from mxdev.vcs.filesystem import FilesystemError from mxdev.vcs.common import WCError + from mxdev.vcs.filesystem import FilesystemError # Should be a subclass of WCError assert issubclass(FilesystemError, WCError) @@ -13,8 +12,8 @@ def test_filesystem_error_exists(): def test_filesystem_working_copy_class_exists(): """Test FilesystemWorkingCopy class exists.""" - from mxdev.vcs.filesystem import FilesystemWorkingCopy from mxdev.vcs.common import BaseWorkingCopy + from mxdev.vcs.filesystem import FilesystemWorkingCopy # Should be a subclass of BaseWorkingCopy assert issubclass(FilesystemWorkingCopy, BaseWorkingCopy) @@ -43,7 +42,8 @@ def test_checkout_path_exists_and_matches(tmp_path): def test_checkout_path_exists_but_doesnt_match(tmp_path): """Test checkout when path exists but doesn't match expected name.""" - from mxdev.vcs.filesystem import FilesystemWorkingCopy, FilesystemError + from mxdev.vcs.filesystem import FilesystemError + from mxdev.vcs.filesystem import FilesystemWorkingCopy # Create a directory with different name than expected test_dir = tmp_path / "actual-name" @@ -57,15 +57,14 @@ def test_checkout_path_exists_but_doesnt_match(tmp_path): wc = FilesystemWorkingCopy(source) - with pytest.raises( - FilesystemError, match="Directory name for existing package .* differs" - ): + with pytest.raises(FilesystemError, match="Directory name for existing package .* differs"): wc.checkout() def test_checkout_path_doesnt_exist(tmp_path): """Test checkout when path doesn't exist.""" - from mxdev.vcs.filesystem import FilesystemWorkingCopy, FilesystemError + from mxdev.vcs.filesystem import FilesystemError + from mxdev.vcs.filesystem import FilesystemWorkingCopy # Don't create the directory test_dir = tmp_path / "nonexistent" @@ -78,9 +77,7 @@ def test_checkout_path_doesnt_exist(tmp_path): wc = FilesystemWorkingCopy(source) - with pytest.raises( - FilesystemError, match="Directory .* for package .* doesn't exist" - ): + with pytest.raises(FilesystemError, match="Directory .* for package .* doesn't exist"): wc.checkout() @@ -188,7 +185,8 @@ def test_update_when_matches(tmp_path): def test_update_when_doesnt_match(tmp_path): """Test update() when path doesn't match raises error.""" - from mxdev.vcs.filesystem import FilesystemWorkingCopy, FilesystemError + from mxdev.vcs.filesystem import FilesystemError + from mxdev.vcs.filesystem import FilesystemWorkingCopy test_dir = tmp_path / "actual-name" test_dir.mkdir() @@ -201,16 +199,14 @@ def test_update_when_doesnt_match(tmp_path): wc = FilesystemWorkingCopy(source) - with pytest.raises( - FilesystemError, match="Directory name for existing package .* differs" - ): + with pytest.raises(FilesystemError, match="Directory name for existing package .* differs"): wc.update() def test_logger_exists(): """Test that logger is imported from common.""" - from mxdev.vcs.filesystem import logger from mxdev.vcs.common import logger as common_logger + from mxdev.vcs.filesystem import logger # Should be the same logger instance assert logger is common_logger diff --git a/tests/utils.py b/tests/utils.py index 582c516..4fdf56b 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -1,8 +1,8 @@ +from collections.abc import Iterable from mxdev.vcs.common import WorkingCopies from subprocess import PIPE from subprocess import Popen from typing import Any -from collections.abc import Iterable import os import sys