Fixed #32900 -- Improved migrations questioner prompts.

[mateoradman]

…ive voice and avoid personal pronouns.

[jacobtylerwalls]

Hiya @mateoradman could you target the main branch? As a UX cleanup, this wouldn't qualify for a backport.

[mateoradman]

Hiya @mateoradman could you target the main branch? As a UX cleanup, this wouldn't qualify for a backport.

Sure, no problem! 👍

[mateoradman]

I think this one is ready to be merged. Thanks for everyone’s suggestions and involvement.

[felixxm]

@mateoradman Thanks for this patch 👍 I left comments.

[smithdc1]

Hi @mateoradman -- thank you for your efforts here. 👍

If you've fixed all of the review comments you can untick 'needs tests' and 'needs improvement' on trac so it can go back into the review queue.

https://code.djangoproject.com/ticket/32900

[felixxm]

I don't see any new tests requested in the #14607 (comment) 🤔

[mateoradman]

I don't see any new tests requested in the #14607 (comment) 🤔

Hi @felixxm, I remember that I could not find any other message that can be tested here. If you identify that I missed something, please let me know and I will do it asap.

[jacobtylerwalls]

Thanks for the quick reply. I was wondering, is this the blocker?

All the functions that contain messages either call sys.exit

If so, no biggie. Just also assert that it raises SystemExit. Something like...

diff --git a/tests/migrations/test_commands.py b/tests/migrations/test_commands.py
index daeeaf8edb..621656429b 100644
--- a/tests/migrations/test_commands.py
+++ b/tests/migrations/test_commands.py
@@ -16,6 +16,7 @@ from django.db.backends.utils import truncate_name
 from django.db.migrations.exceptions import InconsistentMigrationHistory
 from django.db.migrations.recorder import MigrationRecorder
 from django.test import TestCase, override_settings, skipUnlessDBFeature
+from django.test.utils import captured_stdout
 
 from .models import UnicodeModel, UnserializableModel
 from .routers import TestRouter
@@ -1365,6 +1366,22 @@ class MakeMigrationsTests(MigrationTestBase):
             call_command("makemigrations", "migrations", interactive=False, stdout=out)
         self.assertIn("Alter field slug on author", out.getvalue())
 
+    @mock.patch('builtins.input', return_value='2')
+    def test_makemigrations_interactive_not_null_addition(self, mock):
+        class Entry(models.Model):
+            title = models.CharField(max_length=255)
+            archived = models.BooleanField(null=False)
+
+            class Meta:
+                app_label = 'migrations'
+
+        with captured_stdout() as out:
+            # this is the template you patch right above this, possibly find a better one
+            with self.temporary_migration_module(module='migrations.test_auto_now_add'):
+                with self.assertRaises(SystemExit):
+                    call_command('makemigrations', 'migrations', interactive=True)
+        self.assertIn('assertion will fail', out.getvalue())
+
     def test_makemigrations_non_interactive_no_model_rename(self):
         """
         makemigrations adds and removes a possible model rename in

Giving you the opportunity to assert against the messages you changed in this patch:

% python3 tests/runtests.py migrations -k test_makemigrations_interactive_not
Testing against Django installed in '...'
Found 1 test(s).
Creating test database for alias 'default'...
Creating test database for alias 'other'...
System check identified no issues (0 silenced).
F
======================================================================
FAIL: test_makemigrations_interactive_not_null_addition (migrations.test_commands.MakeMigrationsTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "../django/tests/migrations/test_commands.py", line 1384, in test_makemigrations_interactive_not_null_addition
    self.assertIn('assertion will fail', out.getvalue())
AssertionError: 'assertion will fail' not found in "You are trying to add the field 'creation_date' with 'auto_now_add=True' to entry without a default; the database needs something to populate existing rows.\n\n 1) Provide a one-off default now (will be set on all existing rows)\n 2) Quit, and let me add a default in models.py\n"

----------------------------------------------------------------------
Ran 1 test in 0.014s

FAILED (failures=1)

The coverage report might be helpful for spotting what's not tested.

At least, that's where I would start. Thanks for giving this module some ❤️ !

[jacobtylerwalls]

Thanks for the tests, Mateo. If the messages you've edited are now covered by your tests, could you uncheck Needs Tests on the ticket? Thanks.

[mateoradman]

Don

Thanks for the tests, Mateo. If the messages you've edited are now covered by your tests, could you uncheck Needs Tests on the ticket? Thanks.

Done, thanks!

[felixxm]

@mateoradman Thanks 👍

I reorganized commits and pushed edits to tests.

[felixxm]

Merged in d00fb4d, 61c5eae, and 02bc716.