Skip to content

Commit 30fc010

Browse files
authored
fix: improve mrt init UX — fix db_url quoting bug, clearer prompts and next steps (#54)
* 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 * fix: improve mrt init UX — fix db_url quoting bug, clearer prompts and next steps 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).
1 parent aa59bff commit 30fc010

2 files changed

Lines changed: 83 additions & 25 deletions

File tree

README.md

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ pip install pytest-mrt[oracle] # python-oracledb
137137
pip install pytest-mrt[mssql] # pymssql
138138
```
139139

140-
## Auto-fix missing reverse operations (v1.3.0)
140+
## Auto-fix missing reverse operations
141141

142142
`mrt fix` generates missing reverse operations for both Alembic and Django migrations.
143143

@@ -158,18 +158,29 @@ mrt clean-backups --db $DATABASE_URL
158158
mrt clean-backups --db $DATABASE_URL --label 0042_remove_user_phone --yes
159159
```
160160

161-
## pre-commit integration (v1.3.0)
161+
## pre-commit integration
162162

163163
Add to `.pre-commit-config.yaml` to run `mrt check` automatically before every push:
164164

165165
```yaml
166+
# Alembic
166167
- repo: https://github.com/croc100/pytest-mrt
167-
rev: v1.3.0
168+
rev: v1.2.0
168169
hooks:
169170
- id: mrt-check
171+
args: [alembic/versions/]
172+
173+
# Django
174+
- repo: https://github.com/croc100/pytest-mrt
175+
rev: v1.2.0
176+
hooks:
177+
- id: mrt-check
178+
args: [myapp/migrations/]
170179
```
171180
172-
## Incremental CI — `--since` (v1.3.0)
181+
Update `rev` to the latest release tag. Run `pre-commit autoupdate` to keep it current.
182+
183+
## Incremental CI — `--since`
173184

174185
Check only migrations added since a given revision. Keeps CI fast on large codebases:
175186

@@ -247,12 +258,13 @@ To suppress all MRT warnings on a line:
247258

248259
Legacy syntax `# mrt: ignore` is still supported for backward compatibility.
249260

250-
## What's new in v1.3.0
261+
## What's new in v1.2.0
251262

252-
- **Django-aware `mrt fix`** — auto-generates reverse operations and data-safe backup/restore code for `RemoveField` and `DeleteModel`
253-
- **`mrt clean-backups`** — removes `_mrt_backups` rows after a deployment is confirmed stable
254-
- **`mrt check --since <revision>`** — incremental scan; only checks migrations added since a given git ref
255-
- **pre-commit hook** — add two lines to `.pre-commit-config.yaml` and `mrt check` runs automatically before every push
263+
- **MRT rule codes** (MRT101-MRT902) on all 44 patterns
264+
- **`# noqa: MRTxxx` suppression** — ruff/flake8-compatible per-line suppression syntax
265+
- **CLI refactored** into `commands/` subpackage
266+
- **`mrt check --since <revision>`** — incremental scan; only checks migrations added since a given revision
267+
- **pre-commit hook** — add to `.pre-commit-config.yaml` and `mrt check` runs automatically before every push
256268

257269
## Changelog
258270

pytest_mrt/commands/init.py

Lines changed: 62 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -17,28 +17,47 @@ def _detect_django_project() -> tuple[bool, str | None]:
1717
return False, None
1818

1919

20+
def _db_url_expr(db_url: str) -> str:
21+
"""Return db_url as a valid Python expression for conftest.py.
22+
23+
If the value looks like a Python expression (e.g. os.environ.get(...))
24+
write it as-is. Otherwise wrap it in quotes so it becomes a string literal.
25+
"""
26+
stripped = db_url.strip()
27+
if stripped.startswith("os.") or stripped.startswith('"') or stripped.startswith("'"):
28+
return stripped
29+
return f'"{stripped}"'
30+
31+
2032
def _write_conftest(path: Path, alembic_ini: str, db_url: str) -> None:
33+
url_expr = _db_url_expr(db_url)
34+
needs_os = "os." in url_expr
35+
imports = (
36+
"import os\nfrom pytest_mrt import MRTConfig\n"
37+
if needs_os
38+
else "from pytest_mrt import MRTConfig\n"
39+
)
2140
path.write_text(
22-
f"import os\n"
23-
f"from pytest_mrt import MRTConfig\n\n\n"
41+
f"{imports}\n\n"
2442
f"def pytest_configure(config):\n"
2543
f" config._mrt_config = MRTConfig(\n"
2644
f' alembic_ini="{alembic_ini}",\n'
27-
f" db_url={db_url},\n"
28-
f' # skip={{"revision_id": "Reason this is a known issue"}},\n'
45+
f" db_url={url_expr},\n"
46+
f' # skip={{"revision_id": "Reason this migration is intentionally irreversible"}},\n'
2947
f" )\n"
3048
)
3149

3250

3351
def _append_conftest(path: Path, alembic_ini: str, db_url: str) -> None:
52+
url_expr = _db_url_expr(db_url)
3453
existing = path.read_text()
3554
addition = (
3655
f"\n\n# Added by mrt init\n"
3756
f"from pytest_mrt import MRTConfig\n\n\n"
3857
f"def pytest_configure(config):\n"
3958
f" config._mrt_config = MRTConfig(\n"
4059
f' alembic_ini="{alembic_ini}",\n'
41-
f" db_url={db_url},\n"
60+
f" db_url={url_expr},\n"
4261
f" )\n"
4362
)
4463
path.write_text(existing + addition)
@@ -53,20 +72,30 @@ def _init_django(detected_settings: str | None) -> None:
5372
console.print("[yellow]Tip:[/yellow] Set DJANGO_SETTINGS_MODULE or provide it below.")
5473
settings = typer.prompt("Django settings module (e.g. myproject.settings_test)")
5574

75+
console.print(
76+
"[dim]Tip: use sqlite:///test.db for local testing, "
77+
"or set TEST_DATABASE_URL env var for CI.[/dim]"
78+
)
5679
db_url = typer.prompt(
5780
"Test database URL",
58-
default='os.environ.get("TEST_DATABASE_URL", "sqlite:///test.db")',
81+
default="sqlite:///test.db",
5982
)
6083

6184
test_dir = "tests" if Path("tests").exists() else "."
6285
conftest_path = Path(test_dir) / "conftest.py"
6386

87+
url_expr = _db_url_expr(db_url)
88+
needs_os = "os." in url_expr
89+
_imports = (
90+
"import os\nfrom pytest_mrt import MRTConfig\n"
91+
if needs_os
92+
else "from pytest_mrt import MRTConfig\n"
93+
)
6494
django_conftest = (
65-
f"import os\n"
66-
f"from pytest_mrt import MRTConfig\n\n\n"
95+
f"{_imports}\n\n"
6796
f"def pytest_configure(config):\n"
6897
f" config._mrt_config = MRTConfig(\n"
69-
f" db_url={db_url},\n"
98+
f" db_url={url_expr},\n"
7099
f' django_settings="{settings}",\n'
71100
f' # django_apps=["myapp", "otherapp"], # restrict to specific apps\n'
72101
f" )\n"
@@ -97,11 +126,16 @@ def _init_django(detected_settings: str | None) -> None:
97126
console.print(f"[green]✓[/green] Created [bold]{test_path}[/bold]")
98127

99128
console.print()
100-
console.print("[bold]Next steps:[/bold]")
101-
console.print(f" [cyan]pytest {test_dir}/test_migrations.py -s[/cyan]")
129+
console.print("[bold]Setup complete. Next steps:[/bold]")
130+
console.print()
131+
console.print(" 1. Run dynamic rollback tests (requires a test DB):")
132+
console.print(f" [cyan]pytest {test_dir}/test_migrations.py -s[/cyan]")
102133
console.print()
103-
console.print("[dim]Static analysis (no DB needed):[/dim]")
104-
console.print(" [cyan]mrt check yourapp/migrations/[/cyan]")
134+
console.print(" 2. Run static analysis (no DB needed):")
135+
console.print(" [cyan]mrt check yourapp/migrations/[/cyan]")
136+
console.print()
137+
console.print(" [dim]If tests fail, run with -v for details or check docs:[/dim]")
138+
console.print(" [dim]https://croc100.github.io/pytest-mrt[/dim]")
105139

106140

107141
def init() -> None:
@@ -124,9 +158,13 @@ def init() -> None:
124158
else:
125159
found_ini = typer.prompt("Path to alembic.ini", default="alembic.ini")
126160

161+
console.print(
162+
"[dim]Tip: use sqlite:///test.db for local testing, "
163+
"or set TEST_DATABASE_URL env var for CI.[/dim]"
164+
)
127165
db_url = typer.prompt(
128166
"Test database URL",
129-
default='os.environ.get("TEST_DATABASE_URL", "sqlite:///test.db")',
167+
default="sqlite:///test.db",
130168
)
131169

132170
test_dir = "tests" if Path("tests").exists() else "."
@@ -153,5 +191,13 @@ def init() -> None:
153191
console.print(f"[green]✓[/green] Created [bold]{test_path}[/bold]")
154192

155193
console.print()
156-
console.print("[bold]Next steps:[/bold]")
157-
console.print(f" [cyan]pytest {test_dir}/test_migrations.py -s[/cyan]")
194+
console.print("[bold]Setup complete. Next steps:[/bold]")
195+
console.print()
196+
console.print(" 1. Run dynamic rollback tests (requires a test DB):")
197+
console.print(f" [cyan]pytest {test_dir}/test_migrations.py -s[/cyan]")
198+
console.print()
199+
console.print(" 2. Run static analysis (no DB needed):")
200+
console.print(f" [cyan]mrt check {found_ini.replace('alembic.ini', 'versions/')}[/cyan]")
201+
console.print()
202+
console.print(" [dim]If tests fail, run with -v for details or check docs:[/dim]")
203+
console.print(" [dim]https://croc100.github.io/pytest-mrt[/dim]")

0 commit comments

Comments
 (0)