fix: use backwards_plan to build exact rollback plan in downgrade() (#50)

* fix: use backwards_plan to build exact rollback plan in downgrade()

The previous implementation targeted same_app_parents[0] as the migration
target, which caused Django's executor to find all same-app children of
that parent and roll them all back. In a merge-migration graph:

  0001 -> 0002a -> 0003_merge
       -> 0002b /

Calling downgrade('myapp', '0002b') would target 0001, causing Django to
roll back 0003_merge + 0002a + 0002b — leaving only 0001 applied. 0002a
should have been left untouched.

Fix: use graph.backwards_plan(key) to compute the exact set of migrations
to unapply (the target + anything that depends on it), then pass the result
directly to executor.migrate([], plan=...). This rolls back only what is
necessary and leaves sibling branches intact.

Fixes #45

* ci: skip unchanged jobs using dorny/paths-filter

Add a changes job that detects which paths were modified. Each job now
only runs when relevant files change:

- lint, test (unit): pytest_mrt/** or pyproject.toml
- test-django: above + pytest_mrt/adapters/django*.py or tests/test_django*.py
- test-postgres/mysql/oracle/mssql: above + their respective test files
- ci.yml changes trigger all jobs

Docs-only and README changes no longer trigger any CI jobs.

* style: ruff format django_runner.py

Author: croc100

.github/workflows/ci.yml

@@ -7,7 +7,42 @@ on:
     branches: [main]
 
 jobs:
+  changes:
+    runs-on: ubuntu-latest
+    outputs:
+      src:      ${{ steps.filter.outputs.src }}
+      django:   ${{ steps.filter.outputs.django }}
+      postgres: ${{ steps.filter.outputs.postgres }}
+      mysql:    ${{ steps.filter.outputs.mysql }}
+      oracle:   ${{ steps.filter.outputs.oracle }}
+      mssql:    ${{ steps.filter.outputs.mssql }}
+      ci:       ${{ steps.filter.outputs.ci }}
+    steps:
+      - uses: actions/checkout@v6
+      - uses: dorny/paths-filter@v3
+        id: filter
+        with:
+          filters: |
+            src:
+              - 'pytest_mrt/**'
+              - 'pyproject.toml'
+            django:
+              - 'pytest_mrt/adapters/django*.py'
+              - 'tests/test_django*.py'
+            postgres:
+              - 'tests/test_postgres.py'
+            mysql:
+              - 'tests/test_mysql.py'
+            oracle:
+              - 'tests/test_oracle.py'
+            mssql:
+              - 'tests/test_mssql.py'
+            ci:
+              - '.github/workflows/ci.yml'
+
   lint:
+    needs: changes
+    if: ${{ needs.changes.outputs.src == 'true' || needs.changes.outputs.ci == 'true' }}
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v6
@@ -29,6 +64,8 @@ jobs:
         run: mypy pytest_mrt/ --ignore-missing-imports --no-strict-optional
 
   test:
+    needs: changes
+    if: ${{ needs.changes.outputs.src == 'true' || needs.changes.outputs.ci == 'true' }}
     runs-on: ubuntu-latest
     strategy:
       fail-fast: false
@@ -61,6 +98,8 @@ jobs:
           fail_ci_if_error: false
 
   test-postgres:
+    needs: changes
+    if: ${{ needs.changes.outputs.src == 'true' || needs.changes.outputs.postgres == 'true' || needs.changes.outputs.ci == 'true' }}
     runs-on: ubuntu-latest
 
     services:
@@ -94,6 +133,8 @@ jobs:
         run: coverage run -m pytest tests/test_postgres.py -v
 
   test-mysql:
+    needs: changes
+    if: ${{ needs.changes.outputs.src == 'true' || needs.changes.outputs.mysql == 'true' || needs.changes.outputs.ci == 'true' }}
     runs-on: ubuntu-latest
 
     services:
@@ -128,6 +169,8 @@ jobs:
         run: coverage run -m pytest tests/test_mysql.py -v
 
   test-django:
+    needs: changes
+    if: ${{ needs.changes.outputs.src == 'true' || needs.changes.outputs.django == 'true' || needs.changes.outputs.ci == 'true' }}
     runs-on: ubuntu-latest
     strategy:
       fail-fast: false
@@ -148,6 +191,8 @@ jobs:
         run: pytest tests/test_django_dynamic.py -v
 
   test-oracle:
+    needs: changes
+    if: ${{ needs.changes.outputs.src == 'true' || needs.changes.outputs.oracle == 'true' || needs.changes.outputs.ci == 'true' }}
     runs-on: ubuntu-latest
 
     services:
@@ -181,6 +226,8 @@ jobs:
         run: coverage run -m pytest tests/test_oracle.py -v
 
   test-mssql:
+    needs: changes
+    if: ${{ needs.changes.outputs.src == 'true' || needs.changes.outputs.mssql == 'true' || needs.changes.outputs.ci == 'true' }}
     runs-on: ubuntu-latest
 
     services:

pytest_mrt/adapters/django_runner.py

@@ -157,25 +157,29 @@ def upgrade(self, app_label: str, migration_name: str) -> None:
         executor.migrate([(app_label, migration_name)])
 
     def downgrade(self, app_label: str, migration_name: str) -> None:
-        """Roll back to just before migration_name within app_label."""
+        """Roll back to just before migration_name within app_label.
+
+        Uses backwards_plan() to build an exact rollback plan so that only
+        the target migration and its dependents are unapplied. This correctly
+        handles branch migrations in a merge-migration graph: sibling branches
+        that do not depend on the target are left untouched.
+        """
         executor = self._executor()
         loader = executor.loader
         key = (app_label, migration_name)
         graph = loader.graph
 
-        # node_map holds MigrationNode objects which have .parents/.children
-        node = graph.node_map.get(key)
-        if node is None:
+        if graph.node_map.get(key) is None:
             raise KeyError(f"Migration not found: {app_label}/{migration_name}")
 
-        same_app_parents = [p.key for p in node.parents if p.key[0] == app_label]
-
-        if same_app_parents:
-            target = [same_app_parents[0]]
-        else:
-            target = [(app_label, None)]
+        # backwards_plan(key) returns [key] + all migrations that depend on it,
+        # in the order they must be unapplied. Filter to only those currently
+        # applied so we never try to unapply something that was already rolled back.
+        applied = set(loader.applied_migrations.keys())
+        plan = [(loader.graph.nodes[m], True) for m in graph.backwards_plan(key) if m in applied]
 
-        executor.migrate(target)
+        if plan:
+            executor.migrate([], plan=plan)
 
     def downgrade_app_zero(self, app_label: str) -> None:
         executor = self._executor()

tests/test_django_runner_unit.py

@@ -225,37 +225,100 @@ def test_runner_upgrade(runner):
     mock_exec.migrate.assert_called_once_with([("myapp", "0001_initial")])
 
 
-def test_runner_downgrade_to_parent(runner):
-    """downgrade() targets the first same-app parent."""
-    mock_parent = mock.MagicMock()
-    mock_parent.key = ("myapp", "0000_squashed")
+def _setup_downgrade_mock(runner, *, backwards_plan_keys, applied_keys):
+    """
+    Wire up a mock executor for downgrade() tests.
+
+    backwards_plan_keys: list of (app, name) tuples returned by graph.backwards_plan()
+    applied_keys:        set of (app, name) tuples in loader.applied_migrations
+    Returns the mock executor.
+    """
+    mock_exec = mock.MagicMock()
+    mock_exec.loader.applied_migrations = {k: mock.MagicMock() for k in applied_keys}
+
+    # graph.node_map.get returns a non-None sentinel so KeyError is not raised
+    mock_exec.loader.graph.node_map.get.return_value = mock.MagicMock()
 
-    mock_exec = _attach_executor(runner, node_parent=mock_parent)
-    mock_exec.loader.graph.node_map.get.return_value.parents = [mock_parent]
+    # graph.nodes[key] returns a distinct mock per key
+    node_mocks = {k: mock.MagicMock() for k in backwards_plan_keys}
+    mock_exec.loader.graph.nodes.__getitem__ = lambda _, k: node_mocks[k]
+    mock_exec.loader.graph.backwards_plan.return_value = backwards_plan_keys
 
-    runner.downgrade("myapp", "0001_initial")
-    mock_exec.migrate.assert_called_once_with([("myapp", "0000_squashed")])
+    runner._executor = mock.MagicMock(return_value=mock_exec)
+    return mock_exec, node_mocks
 
 
-def test_runner_downgrade_to_zero_when_no_same_app_parent(runner):
-    """downgrade() targets (app, None) when parent is from a different app."""
-    mock_parent = mock.MagicMock()
-    mock_parent.key = ("otherapp", "0001_dep")
+def test_runner_downgrade_single_migration(runner):
+    """downgrade() rolls back only the target when it has no dependents."""
+    key = ("myapp", "0002_add_field")
+    mock_exec, node_mocks = _setup_downgrade_mock(
+        runner,
+        backwards_plan_keys=[key],
+        applied_keys={("myapp", "0001_initial"), key},
+    )
 
-    mock_exec = _attach_executor(runner, node_parent=mock_parent)
-    mock_exec.loader.graph.node_map.get.return_value.parents = [mock_parent]
+    runner.downgrade("myapp", "0002_add_field")
 
-    runner.downgrade("myapp", "0001_initial")
-    mock_exec.migrate.assert_called_once_with([("myapp", None)])
+    mock_exec.migrate.assert_called_once_with([], plan=[(node_mocks[key], True)])
 
 
-def test_runner_downgrade_no_parents_at_all(runner):
-    """downgrade() targets (app, None) when the migration has no parents."""
-    mock_exec = _attach_executor(runner)  # node_parent=None → parents=[]
-    mock_exec.loader.graph.node_map.get.return_value.parents = []
+def test_runner_downgrade_branch_with_merge_child(runner):
+    """downgrade() rolls back the merge child before the branch migration.
 
-    runner.downgrade("myapp", "0001_initial")
-    mock_exec.migrate.assert_called_once_with([("myapp", None)])
+    Graph: 0001 -> 0002a -> 0003_merge
+                -> 0002b /
+
+    downgrade("myapp", "0002b") must roll back 0003_merge first, then 0002b,
+    leaving 0002a untouched.
+    """
+    key_0002b = ("myapp", "0002b")
+    key_0003 = ("myapp", "0003_merge")
+    applied = {("myapp", "0001_initial"), ("myapp", "0002a"), key_0002b, key_0003}
+
+    mock_exec, node_mocks = _setup_downgrade_mock(
+        runner,
+        backwards_plan_keys=[key_0003, key_0002b],  # Django order: dependents first
+        applied_keys=applied,
+    )
+
+    runner.downgrade("myapp", "0002b")
+
+    mock_exec.migrate.assert_called_once_with(
+        [], plan=[(node_mocks[key_0003], True), (node_mocks[key_0002b], True)]
+    )
+
+
+def test_runner_downgrade_skips_already_unapplied(runner):
+    """downgrade() omits migrations not in applied_migrations from the plan."""
+    key = ("myapp", "0002_add_field")
+    # 0002 is NOT in applied_keys — already rolled back
+    mock_exec, _ = _setup_downgrade_mock(
+        runner,
+        backwards_plan_keys=[key],
+        applied_keys={("myapp", "0001_initial")},
+    )
+
+    runner.downgrade("myapp", "0002_add_field")
+
+    mock_exec.migrate.assert_not_called()
+
+
+def test_runner_downgrade_merge_migration(runner):
+    """downgrade() on a merge migration rolls back only the merge node."""
+    key_0002a = ("myapp", "0002a")
+    key_0002b = ("myapp", "0002b")
+    key_0003 = ("myapp", "0003_merge")
+    applied = {("myapp", "0001_initial"), key_0002a, key_0002b, key_0003}
+
+    mock_exec, node_mocks = _setup_downgrade_mock(
+        runner,
+        backwards_plan_keys=[key_0003],
+        applied_keys=applied,
+    )
+
+    runner.downgrade("myapp", "0003_merge")
+
+    mock_exec.migrate.assert_called_once_with([], plan=[(node_mocks[key_0003], True)])
 
 
 def test_runner_downgrade_migration_not_found(runner):