From 896c885ddd095804420e962b96f8f1587f64d9a6 Mon Sep 17 00:00:00 2001 From: croc100 Date: Mon, 8 Jun 2026 19:40:47 +0900 Subject: [PATCH 1/3] docs: update pre-commit section with args example, fix stale v1.3.0 refs - pre-commit section: add args example for Alembic and Django, note on pre-commit autoupdate, update rev from v1.3.0 to v1.2.0 - Remove (v1.3.0) version labels from section headers for unreleased features - Update 'What's new' section to reflect v1.2.0 accurately Closes #35 --- README.md | 30 +++++++++++++++++++++--------- 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index 027412f..2b95523 100644 --- a/README.md +++ b/README.md @@ -137,7 +137,7 @@ pip install pytest-mrt[oracle] # python-oracledb pip install pytest-mrt[mssql] # pymssql ``` -## Auto-fix missing reverse operations (v1.3.0) +## Auto-fix missing reverse operations `mrt fix` generates missing reverse operations for both Alembic and Django migrations. @@ -158,18 +158,29 @@ mrt clean-backups --db $DATABASE_URL mrt clean-backups --db $DATABASE_URL --label 0042_remove_user_phone --yes ``` -## pre-commit integration (v1.3.0) +## pre-commit integration Add to `.pre-commit-config.yaml` to run `mrt check` automatically before every push: ```yaml +# Alembic - repo: https://github.com/croc100/pytest-mrt - rev: v1.3.0 + rev: v1.2.0 hooks: - id: mrt-check + args: [alembic/versions/] + +# Django +- repo: https://github.com/croc100/pytest-mrt + rev: v1.2.0 + hooks: + - id: mrt-check + args: [myapp/migrations/] ``` -## Incremental CI — `--since` (v1.3.0) +Update `rev` to the latest release tag. Run `pre-commit autoupdate` to keep it current. + +## Incremental CI — `--since` Check only migrations added since a given revision. Keeps CI fast on large codebases: @@ -247,12 +258,13 @@ To suppress all MRT warnings on a line: Legacy syntax `# mrt: ignore` is still supported for backward compatibility. -## What's new in v1.3.0 +## What's new in v1.2.0 -- **Django-aware `mrt fix`** — auto-generates reverse operations and data-safe backup/restore code for `RemoveField` and `DeleteModel` -- **`mrt clean-backups`** — removes `_mrt_backups` rows after a deployment is confirmed stable -- **`mrt check --since `** — incremental scan; only checks migrations added since a given git ref -- **pre-commit hook** — add two lines to `.pre-commit-config.yaml` and `mrt check` runs automatically before every push +- **MRT rule codes** (MRT101-MRT902) on all 44 patterns +- **`# noqa: MRTxxx` suppression** — ruff/flake8-compatible per-line suppression syntax +- **CLI refactored** into `commands/` subpackage +- **`mrt check --since `** — incremental scan; only checks migrations added since a given revision +- **pre-commit hook** — add to `.pre-commit-config.yaml` and `mrt check` runs automatically before every push ## Changelog From cb2cb21622778bd6f96fb92f961682b63abf40b8 Mon Sep 17 00:00:00 2001 From: croc100 Date: Mon, 8 Jun 2026 19:46:34 +0900 Subject: [PATCH 2/3] =?UTF-8?q?fix:=20improve=20mrt=20init=20UX=20?= =?UTF-8?q?=E2=80=94=20fix=20db=5Furl=20quoting=20bug,=20clearer=20prompts?= =?UTF-8?q?=20and=20next=20steps?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three issues fixed: 1. db_url quoting bug: when a user typed a literal URL (e.g. sqlite:///test.db), it was written as db_url=sqlite:///test.db without quotes — invalid Python. Now correctly written as db_url="sqlite:///test.db". 2. Confusing default prompt: the old default was the raw Python expression os.environ.get("TEST_DATABASE_URL", "sqlite:///test.db") which is unclear for first-time users. Default is now simply sqlite:///test.db with a tip explaining the env var pattern for CI. 3. Weak next steps: the completion message now shows numbered steps, static analysis command, and a docs link — so users know exactly what to do next. Also removes unnecessary 'import os' from generated conftest when a literal URL is used (os is only imported when the url expr references os.environ). --- pytest_mrt/commands/init.py | 78 +++++++++++++++++++++++++++++-------- 1 file changed, 62 insertions(+), 16 deletions(-) diff --git a/pytest_mrt/commands/init.py b/pytest_mrt/commands/init.py index 0434e12..4e04ce2 100644 --- a/pytest_mrt/commands/init.py +++ b/pytest_mrt/commands/init.py @@ -17,20 +17,39 @@ def _detect_django_project() -> tuple[bool, str | None]: return False, None +def _db_url_expr(db_url: str) -> str: + """Return db_url as a valid Python expression for conftest.py. + + If the value looks like a Python expression (e.g. os.environ.get(...)) + write it as-is. Otherwise wrap it in quotes so it becomes a string literal. + """ + stripped = db_url.strip() + if stripped.startswith("os.") or stripped.startswith('"') or stripped.startswith("'"): + return stripped + return f'"{stripped}"' + + def _write_conftest(path: Path, alembic_ini: str, db_url: str) -> None: + url_expr = _db_url_expr(db_url) + needs_os = "os." in url_expr + imports = ( + "import os\nfrom pytest_mrt import MRTConfig\n" + if needs_os + else "from pytest_mrt import MRTConfig\n" + ) path.write_text( - f"import os\n" - f"from pytest_mrt import MRTConfig\n\n\n" + f"{imports}\n\n" f"def pytest_configure(config):\n" f" config._mrt_config = MRTConfig(\n" f' alembic_ini="{alembic_ini}",\n' - f" db_url={db_url},\n" - f' # skip={{"revision_id": "Reason this is a known issue"}},\n' + f" db_url={url_expr},\n" + f' # skip={{"revision_id": "Reason this migration is intentionally irreversible"}},\n' f" )\n" ) def _append_conftest(path: Path, alembic_ini: str, db_url: str) -> None: + url_expr = _db_url_expr(db_url) existing = path.read_text() addition = ( f"\n\n# Added by mrt init\n" @@ -38,7 +57,7 @@ def _append_conftest(path: Path, alembic_ini: str, db_url: str) -> None: f"def pytest_configure(config):\n" f" config._mrt_config = MRTConfig(\n" f' alembic_ini="{alembic_ini}",\n' - f" db_url={db_url},\n" + f" db_url={url_expr},\n" f" )\n" ) path.write_text(existing + addition) @@ -53,20 +72,30 @@ def _init_django(detected_settings: str | None) -> None: console.print("[yellow]Tip:[/yellow] Set DJANGO_SETTINGS_MODULE or provide it below.") settings = typer.prompt("Django settings module (e.g. myproject.settings_test)") + console.print( + "[dim]Tip: use sqlite:///test.db for local testing, " + "or set TEST_DATABASE_URL env var for CI.[/dim]" + ) db_url = typer.prompt( "Test database URL", - default='os.environ.get("TEST_DATABASE_URL", "sqlite:///test.db")', + default="sqlite:///test.db", ) test_dir = "tests" if Path("tests").exists() else "." conftest_path = Path(test_dir) / "conftest.py" + url_expr = _db_url_expr(db_url) + needs_os = "os." in url_expr + _imports = ( + "import os\nfrom pytest_mrt import MRTConfig\n" + if needs_os + else "from pytest_mrt import MRTConfig\n" + ) django_conftest = ( - f"import os\n" - f"from pytest_mrt import MRTConfig\n\n\n" + f"{_imports}\n\n" f"def pytest_configure(config):\n" f" config._mrt_config = MRTConfig(\n" - f" db_url={db_url},\n" + f" db_url={url_expr},\n" f' django_settings="{settings}",\n' f' # django_apps=["myapp", "otherapp"], # restrict to specific apps\n' f" )\n" @@ -97,11 +126,16 @@ def _init_django(detected_settings: str | None) -> None: console.print(f"[green]✓[/green] Created [bold]{test_path}[/bold]") console.print() - console.print("[bold]Next steps:[/bold]") - console.print(f" [cyan]pytest {test_dir}/test_migrations.py -s[/cyan]") + console.print("[bold]Setup complete. Next steps:[/bold]") + console.print() + console.print(" 1. Run dynamic rollback tests (requires a test DB):") + console.print(f" [cyan]pytest {test_dir}/test_migrations.py -s[/cyan]") console.print() - console.print("[dim]Static analysis (no DB needed):[/dim]") - console.print(" [cyan]mrt check yourapp/migrations/[/cyan]") + console.print(" 2. Run static analysis (no DB needed):") + console.print(" [cyan]mrt check yourapp/migrations/[/cyan]") + console.print() + console.print(" [dim]If tests fail, run with -v for details or check docs:[/dim]") + console.print(" [dim]https://croc100.github.io/pytest-mrt[/dim]") def init() -> None: @@ -124,9 +158,13 @@ def init() -> None: else: found_ini = typer.prompt("Path to alembic.ini", default="alembic.ini") + console.print( + "[dim]Tip: use sqlite:///test.db for local testing, " + "or set TEST_DATABASE_URL env var for CI.[/dim]" + ) db_url = typer.prompt( "Test database URL", - default='os.environ.get("TEST_DATABASE_URL", "sqlite:///test.db")', + default="sqlite:///test.db", ) test_dir = "tests" if Path("tests").exists() else "." @@ -153,5 +191,13 @@ def init() -> None: console.print(f"[green]✓[/green] Created [bold]{test_path}[/bold]") console.print() - console.print("[bold]Next steps:[/bold]") - console.print(f" [cyan]pytest {test_dir}/test_migrations.py -s[/cyan]") + console.print("[bold]Setup complete. Next steps:[/bold]") + console.print() + console.print(" 1. Run dynamic rollback tests (requires a test DB):") + console.print(f" [cyan]pytest {test_dir}/test_migrations.py -s[/cyan]") + console.print() + console.print(" 2. Run static analysis (no DB needed):") + console.print(f" [cyan]mrt check {found_ini.replace('alembic.ini', 'versions/')}[/cyan]") + console.print() + console.print(" [dim]If tests fail, run with -v for details or check docs:[/dim]") + console.print(" [dim]https://croc100.github.io/pytest-mrt[/dim]") From 792f402264be1891449631ec0025eb71752f13c0 Mon Sep 17 00:00:00 2001 From: croc100 Date: Mon, 8 Jun 2026 19:53:18 +0900 Subject: [PATCH 3/3] fix: show config errors once at session start instead of per-test Before this change, a missing alembic.ini produced the same error message for every collected test (typically 7 times), each with a double traceback due to pytest.fail() being called inside an except block. Changes: - Add pytest_sessionstart hook that validates MRTConfig once and calls pytest.exit() on fatal config errors -- one clear message, no tests run - Break exception chain in mrt fixture (capture error string before calling pytest.fail) to eliminate "During handling of the above exception" noise - Simplify MRTConfigError message in MRTFixture.__init__: Django hint is now only shown via pytest_sessionstart, not duplicated - Update test assertion to match revised error message text --- pytest_mrt/plugin.py | 51 +++++++++++++++++++++++++++++++++++++------- tests/test_plugin.py | 4 ++-- 2 files changed, 45 insertions(+), 10 deletions(-) diff --git a/pytest_mrt/plugin.py b/pytest_mrt/plugin.py index ffd0e24..4510b52 100644 --- a/pytest_mrt/plugin.py +++ b/pytest_mrt/plugin.py @@ -83,13 +83,9 @@ def __init__(self, config: MRTConfig): if not _Path(config.alembic_ini).exists(): raise MRTConfigError( - f"\n\n alembic.ini not found: '{config.alembic_ini}'\n\n" - " If you are using Django migrations (not Alembic), use:\n\n" - " config._mrt_config = MRTConfig(\n" - " db_url=os.environ['TEST_DATABASE_URL'],\n" - " django_settings='myproject.settings_test',\n" - " )\n\n" - " See: https://croc100.github.io/pytest-mrt/quickstart/#django" + f"alembic.ini not found: '{config.alembic_ini}'\n\n" + "Check the path and update MRTConfig(alembic_ini=...) in your conftest.py.\n" + "See: https://croc100.github.io/pytest-mrt/quickstart/" ) self._runner = MigrationRunner(config.alembic_ini, config.db_url) @@ -301,6 +297,39 @@ def pytest_configure(config: pytest.Config) -> None: config.addinivalue_line("markers", "mrt: migration rollback test") +def pytest_sessionstart(session: pytest.Session) -> None: + """Validate MRTConfig once at session start. + + Catches obvious setup errors (missing alembic.ini, etc.) before any test + runs, so the user sees one clear error instead of the same message repeated + for every collected test. + """ + cfg: MRTConfig | None = getattr(session.config, "_mrt_config", None) + if cfg is None: + return + + cfg = _auto_detect_django(cfg) + + # Django mode — no alembic.ini needed + if cfg.django_settings is not None: + return + + from pathlib import Path + + if not Path(cfg.alembic_ini).exists(): + msg = ( + f"alembic.ini not found: '{cfg.alembic_ini}'\n\n" + "Check the path and update MRTConfig(alembic_ini=...) in your conftest.py.\n\n" + "If you are using Django migrations (not Alembic), set django_settings instead:\n\n" + " config._mrt_config = MRTConfig(\n" + " db_url=os.environ['TEST_DATABASE_URL'],\n" + " django_settings='myproject.settings_test',\n" + " )\n\n" + "See: https://croc100.github.io/pytest-mrt/quickstart/" + ) + pytest.exit(msg, returncode=4) + + def pytest_collection_modifyitems( session: pytest.Session, config: pytest.Config, @@ -336,9 +365,15 @@ def pytest_collection_modifyitems( @pytest.fixture def mrt(request: pytest.FixtureRequest) -> Iterator[MRTFixture]: cfg: MRTConfig = getattr(request.config, "_mrt_config", None) or MRTConfig() + # Capture error outside the except block to avoid "During handling of the + # above exception, another exception occurred" in the traceback output. + config_error: str | None = None try: fixture = MRTFixture(cfg) except MRTConfigError as e: - pytest.fail(str(e), pytrace=False) + config_error = str(e) + if config_error is not None: + pytest.fail(config_error, pytrace=False) + return # unreachable; keeps type checkers happy yield fixture fixture.reset() diff --git a/tests/test_plugin.py b/tests/test_plugin.py index 22b1c0a..3ac07dd 100644 --- a/tests/test_plugin.py +++ b/tests/test_plugin.py @@ -665,13 +665,13 @@ def test_auto_detect_django_no_switch_when_explicit_django_settings(tmp_path, mo def test_mrt_fixture_raises_on_missing_alembic_ini(tmp_path, monkeypatch): - """MRTFixture raises MRTConfigError with a Django hint when alembic.ini is missing.""" + """MRTFixture raises MRTConfigError when alembic.ini is missing.""" from pytest_mrt.exceptions import MRTConfigError monkeypatch.delenv("DJANGO_SETTINGS_MODULE", raising=False) cfg = MRTConfig(alembic_ini=str(tmp_path / "nonexistent.ini"), db_url="sqlite:///test.db") - with pytest.raises(MRTConfigError, match="django_settings"): + with pytest.raises(MRTConfigError, match="alembic.ini not found"): MRTFixture(cfg)