Fixed #23745 -- Reused states as much as possible in migrations

Thanks Tim Graham and Markus Holtermann for the reviews.

4 files changed, 32 insertions, 31 deletions

diff --git a/tests/migrations/test_autodetector.py b/tests/migrations/test_autodetector.py
index cfb59d4851..9c878a2bea 100644
--- a/tests/migrations/test_autodetector.py
+++ b/tests/migrations/test_autodetector.py
@@ -410,7 +410,7 @@ class AutodetectorTests(TestCase):
         "Shortcut to make ProjectStates from lists of predefined models"
         project_state = ProjectState()
         for model_state in model_states:
-            project_state.add_model_state(model_state.clone())
+            project_state.add_model(model_state.clone())
         return project_state
 
     def test_arrange_for_graph(self):
diff --git a/tests/migrations/test_executor.py b/tests/migrations/test_executor.py
index 3a7a779f8c..78ee131d18 100644
--- a/tests/migrations/test_executor.py
+++ b/tests/migrations/test_executor.py
@@ -223,7 +223,7 @@ class ExecutorTests(MigrationTestBase):
         global_apps.get_app_config("migrations").models["author"] = migrations_apps.get_model("migrations", "author")
         try:
             migration = executor.loader.get_migration("auth", "0001_initial")
-            self.assertEqual(executor.detect_soft_applied(None, migration), True)
+            self.assertEqual(executor.detect_soft_applied(None, migration)[0], True)
         finally:
             connection.introspection.table_names = old_table_names
             del global_apps.get_app_config("migrations").models["author"]
diff --git a/tests/migrations/test_operations.py b/tests/migrations/test_operations.py
index b52a4df585..90ec456f84 100644
--- a/tests/migrations/test_operations.py
+++ b/tests/migrations/test_operations.py
@@ -828,12 +828,13 @@ class OperationTests(OperationTestBase):
         ])
         self.assertTableExists("test_rmflmm_pony_stables")
 
+        with_field_state = project_state.clone()
         operations = [migrations.RemoveField("Pony", "stables")]
-        self.apply_operations("test_rmflmm", project_state, operations=operations)
+        project_state = self.apply_operations("test_rmflmm", project_state, operations=operations)
         self.assertTableNotExists("test_rmflmm_pony_stables")
 
         # And test reversal
-        self.unapply_operations("test_rmflmm", project_state, operations=operations)
+        self.unapply_operations("test_rmflmm", with_field_state, operations=operations)
         self.assertTableExists("test_rmflmm_pony_stables")
 
     def test_remove_field_m2m_with_through(self):
diff --git a/tests/migrations/test_state.py b/tests/migrations/test_state.py
index 143ce49f8d..6f510813af 100644
--- a/tests/migrations/test_state.py
+++ b/tests/migrations/test_state.py
@@ -159,7 +159,7 @@ class StateTests(TestCase):
         Tests rendering a ProjectState into an Apps.
         """
         project_state = ProjectState()
-        project_state.add_model_state(ModelState(
+        project_state.add_model(ModelState(
             app_label="migrations",
             name="Tag",
             fields=[
@@ -168,7 +168,7 @@ class StateTests(TestCase):
                 ("hidden", models.BooleanField()),
             ],
         ))
-        project_state.add_model_state(ModelState(
+        project_state.add_model(ModelState(
             app_label="migrations",
             name="SubTag",
             fields=[
@@ -187,7 +187,7 @@ class StateTests(TestCase):
         base_mgr = models.Manager()
         mgr1 = FoodManager('a', 'b')
         mgr2 = FoodManager('x', 'y', c=3, d=4)
-        project_state.add_model_state(ModelState(
+        project_state.add_model(ModelState(
             app_label="migrations",
             name="Food",
             fields=[
@@ -324,21 +324,21 @@ class StateTests(TestCase):
 
         # Make a ProjectState and render it
         project_state = ProjectState()
-        project_state.add_model_state(ModelState.from_model(A))
-        project_state.add_model_state(ModelState.from_model(B))
-        project_state.add_model_state(ModelState.from_model(C))
-        project_state.add_model_state(ModelState.from_model(D))
-        project_state.add_model_state(ModelState.from_model(E))
-        project_state.add_model_state(ModelState.from_model(F))
+        project_state.add_model(ModelState.from_model(A))
+        project_state.add_model(ModelState.from_model(B))
+        project_state.add_model(ModelState.from_model(C))
+        project_state.add_model(ModelState.from_model(D))
+        project_state.add_model(ModelState.from_model(E))
+        project_state.add_model(ModelState.from_model(F))
         final_apps = project_state.apps
         self.assertEqual(len(final_apps.get_models()), 6)
 
         # Now make an invalid ProjectState and make sure it fails
         project_state = ProjectState()
-        project_state.add_model_state(ModelState.from_model(A))
-        project_state.add_model_state(ModelState.from_model(B))
-        project_state.add_model_state(ModelState.from_model(C))
-        project_state.add_model_state(ModelState.from_model(F))
+        project_state.add_model(ModelState.from_model(A))
+        project_state.add_model(ModelState.from_model(B))
+        project_state.add_model(ModelState.from_model(C))
+        project_state.add_model(ModelState.from_model(F))
         with self.assertRaises(InvalidBasesError):
             project_state.apps
 
@@ -358,8 +358,8 @@ class StateTests(TestCase):
 
         # Make a ProjectState and render it
         project_state = ProjectState()
-        project_state.add_model_state(ModelState.from_model(A))
-        project_state.add_model_state(ModelState.from_model(B))
+        project_state.add_model(ModelState.from_model(A))
+        project_state.add_model(ModelState.from_model(B))
         self.assertEqual(len(project_state.apps.get_models()), 2)
 
     def test_equality(self):
@@ -369,7 +369,7 @@ class StateTests(TestCase):
 
         # Test two things that should be equal
         project_state = ProjectState()
-        project_state.add_model_state(ModelState(
+        project_state.add_model(ModelState(
             "migrations",
             "Tag",
             [
@@ -388,7 +388,7 @@ class StateTests(TestCase):
 
         # Make a very small change (max_len 99) and see if that affects it
         project_state = ProjectState()
-        project_state.add_model_state(ModelState(
+        project_state.add_model(ModelState(
             "migrations",
             "Tag",
             [
@@ -428,20 +428,20 @@ class StateTests(TestCase):
 
         # Make a valid ProjectState and render it
         project_state = ProjectState()
-        project_state.add_model_state(ModelState.from_model(Author))
-        project_state.add_model_state(ModelState.from_model(Book))
-        project_state.add_model_state(ModelState.from_model(Magazine))
+        project_state.add_model(ModelState.from_model(Author))
+        project_state.add_model(ModelState.from_model(Book))
+        project_state.add_model(ModelState.from_model(Magazine))
         self.assertEqual(len(project_state.apps.get_models()), 3)
 
         # now make an invalid one with a ForeignKey
         project_state = ProjectState()
-        project_state.add_model_state(ModelState.from_model(Book))
+        project_state.add_model(ModelState.from_model(Book))
         with self.assertRaises(ValueError):
             project_state.apps
 
         # and another with ManyToManyField
         project_state = ProjectState()
-        project_state.add_model_state(ModelState.from_model(Magazine))
+        project_state.add_model(ModelState.from_model(Magazine))
         with self.assertRaises(ValueError):
             project_state.apps
 
@@ -461,13 +461,13 @@ class StateTests(TestCase):
 
         # If we just stick it into an empty state it should fail
         project_state = ProjectState()
-        project_state.add_model_state(ModelState.from_model(TestModel))
+        project_state.add_model(ModelState.from_model(TestModel))
         with self.assertRaises(ValueError):
             project_state.apps
 
         # If we include the real app it should succeed
         project_state = ProjectState(real_apps=["contenttypes"])
-        project_state.add_model_state(ModelState.from_model(TestModel))
+        project_state.add_model(ModelState.from_model(TestModel))
         rendered_state = project_state.apps
         self.assertEqual(
             len([x for x in rendered_state.get_models() if x._meta.app_label == "migrations"]),
@@ -498,8 +498,8 @@ class StateTests(TestCase):
 
         # Make a valid ProjectState and render it
         project_state = ProjectState()
-        project_state.add_model_state(ModelState.from_model(Author))
-        project_state.add_model_state(ModelState.from_model(Book))
+        project_state.add_model(ModelState.from_model(Author))
+        project_state.add_model(ModelState.from_model(Book))
         self.assertEqual(
             [name for name, field in project_state.models["migrations", "book"].fields],
             ["id", "author"],
@@ -534,6 +534,6 @@ class ModelStateTests(TestCase):
         self.assertEqual(repr(state), "<ModelState: 'app.Model'>")
 
         project_state = ProjectState()
-        project_state.add_model_state(state)
+        project_state.add_model(state)
         with self.assertRaisesMessage(InvalidBasesError, "Cannot resolve bases for [<ModelState: 'app.Model'>]"):
             project_state.apps