Skip to content

Commit 37f296d

Browse files
authored
fix: keep --format json/html output free of status preamble (#97)
mrt check printed human-readable status lines (Detected: Django migrations, --since/--min-revision/--check-compat notices) to stdout regardless of --format. With --format json this corrupted the output: consumers piping to a JSON parser hit 'Expecting value: line 1 column 1'. The bug was latent for Alembic and became easy to hit now that Django + --check-compat + --format json is a supported combination. Gate all informational console.print calls on fmt == 'table'. Error warnings that precede typer.Exit are unaffected (they abort before any JSON is emitted). Add regression tests for Alembic and Django --check-compat json.
1 parent 0217b9a commit 37f296d

3 files changed

Lines changed: 54 additions & 12 deletions

File tree

CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,9 @@ Versioning: [Semantic Versioning](https://semver.org/spec/v2.0.0.html)
1212
### Added
1313
- **`mrt check --check-compat` now supports Django migrations.** Rolling-deploy compatibility checks (MRT7xx) were previously Alembic-only. Django operations are now mapped to the same patterns: `RemoveField` → MRT701 (DROP COLUMN), `RenameField` → MRT702 (RENAME COLUMN), `DeleteModel`/`RenameModel`/`AlterModelTable` → MRT703 (DROP/RENAME TABLE), and `AddField` NOT NULL without a default → MRT704. Per-line `# noqa: MRTxxx` suppression works for these too. MRT705 (column type change) remains Alembic-only — Django's `AlterField` carries the full field with no reference to the previous type, so a type change cannot be detected statically.
1414

15+
### Fixed
16+
- **`mrt check --format json` leaked human-readable status lines into stdout**, making the output unparseable whenever `--since`, `--min-revision`, `--check-compat`, or Django auto-detection printed an informational line (e.g. `Detected: Django migrations`). These lines are now suppressed for non-`table` formats, so `--format json` emits pure JSON.
17+
1518
### Removed
1619
- The "`--check-compat` is not yet supported for Django migrations" warning, now that it is supported.
1720

pytest_mrt/commands/check.py

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -163,15 +163,16 @@ def check(
163163
)
164164
raise typer.Exit(1)
165165

166-
console.print(
167-
f"[dim]--since {since}: checking {len(since_set)} migration(s) after this point[/dim]"
168-
)
169-
console.print(
170-
"[dim]Note: graph checks (orphan, data-hole detection) skipped — "
171-
"run without --since for full analysis.[/dim]"
172-
)
166+
if fmt == "table":
167+
console.print(
168+
f"[dim]--since {since}: checking {len(since_set)} migration(s) after this point[/dim]"
169+
)
170+
console.print(
171+
"[dim]Note: graph checks (orphan, data-hole detection) skipped — "
172+
"run without --since for full analysis.[/dim]"
173+
)
173174

174-
if is_django:
175+
if is_django and fmt == "table":
175176
console.print("[dim]Detected: Django migrations[/dim]")
176177

177178
if min_revision:
@@ -190,11 +191,12 @@ def check(
190191
"Check the revision ID and try again.[/yellow]"
191192
)
192193
raise typer.Exit(1)
193-
console.print(
194-
f"[dim]--min-revision {min_revision}: checking {len(min_set)} newer migration(s), older ones skipped[/dim]"
195-
)
194+
if fmt == "table":
195+
console.print(
196+
f"[dim]--min-revision {min_revision}: checking {len(min_set)} newer migration(s), older ones skipped[/dim]"
197+
)
196198

197-
if check_compat:
199+
if check_compat and fmt == "table":
198200
console.print(
199201
"[dim]--check-compat: rolling-deploy compatibility checks enabled (MRT7xx)[/dim]"
200202
)

tests/test_cli.py

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -293,6 +293,43 @@ class Migration(migrations.Migration):
293293
assert "Django" in result.output or result.exit_code == 0
294294

295295

296+
def test_check_compat_json_output_is_pure_json_alembic(tmp_path, versions_dir):
297+
"""--check-compat --format json must not leak the status preamble into stdout."""
298+
import json
299+
300+
_risky_migration(versions_dir)
301+
result = runner.invoke(
302+
app, ["check", str(versions_dir), "--check-compat", "--format", "json"]
303+
)
304+
data = json.loads(result.output) # would raise if preamble leaked
305+
assert "findings" in data
306+
307+
308+
def test_check_compat_json_output_is_pure_json_django(tmp_path):
309+
"""Django + --check-compat + --format json emits parseable JSON with MRT7xx codes."""
310+
import json
311+
312+
d = tmp_path / "myapp" / "migrations"
313+
d.mkdir(parents=True)
314+
(d / "0001_initial.py").write_text(
315+
"from django.db import migrations\n"
316+
"class Migration(migrations.Migration):\n"
317+
" dependencies = []\n"
318+
" operations = []\n"
319+
)
320+
(d / "0002_rename.py").write_text(
321+
"from django.db import migrations\n"
322+
"class Migration(migrations.Migration):\n"
323+
" dependencies = [('myapp', '0001_initial')]\n"
324+
" operations = [migrations.RenameField("
325+
"model_name='u', old_name='a', new_name='b')]\n"
326+
)
327+
result = runner.invoke(app, ["check", str(d), "--check-compat", "--format", "json"])
328+
data = json.loads(result.output)
329+
assert any(f["rule"] == "MRT702" for f in data["findings"])
330+
assert result.exit_code == 2
331+
332+
296333
def test_check_empty_directory_exits_0(tmp_path):
297334
empty = tmp_path / "empty"
298335
empty.mkdir()

0 commit comments

Comments
 (0)