Migrations tests fail and leave behind files if tests/migrations/migrations exists

[Jacob Walls]

% ./runtests.py -k migrations
% git status

Untracked files:
  (use "git add <file>..." to include in what will be committed)
        migrations/migrations/

[Tim Graham]

Hi Jacob, I can't reproduce it. Can you debug it further on your system?

[Jacob Walls]

Sure thing, turns out I only encountered this because I had an extra directory left around from writing/debugging other tests, and merely deleting the files to get back to a clean git status didn't remove the directory causing the problem, leaving me a bit confused.

To reproduce:

cd tests
mkdir migrations/migrations
./runtests.py migrations.test_commands.MakeMigrationsTests.test_makemigrations_empty_connections
git status

Then, besides a dirty git status, that single test also fails...:

F
======================================================================
FAIL: test_makemigrations_empty_connections (migrations.test_commands.MakeMigrationsTests.test_makemigrations_empty_connections)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/jwalls/django/tests/migrations/test_commands.py", line 1551, in test_makemigrations_empty_connections
    self.assertIn("No changes detected", out.getvalue())
AssertionError: 'No changes detected' not found in "\x1b[36;1mMigrations for 'migrations':\x1b[0m\n  \x1b[1mmigrations/migrations/0001_initial.py\x1b[0m\n    + Create model ModelWithCustomBase\n    + Create model UnmigratedModel\n"

----------------------------------------------------------------------
Ran 1 test in 0.102s

FAILED (failures=1)

...however, when running the whole migrations tests, several more tests fail with distinct outputs, further obscuring the source of the problem.

A sanity check like might make the failures less mysterious? I won't be disappointed if it's a wontfix.

tests/migrations/test_commands.py

@@ -1489 +1489 @@
     def setUp(self):
         super().setUp()
         self._old_models = apps.app_configs["migrations"].models.copy()
+        self.assertFalse(os.path.exists("migrations"))
 
     def tearDown(self):
         apps.app_configs["migrations"].models = self._old_models

[Natalia Bidart]

Hello Jacob, thank you for the report and the additional details! One thing I'm not clear on is whether the unexpected issue first occurred because a failed test left the migrations folder behind or undeleted.

Was that the case? If so, I think we should focus on identifying the tests that don't clean up properly after a failure. However, I would not add assertions to check that certain folders don't exist, as this feels a bit like a code smell to me.