Fixed #27300 -- Made makemigrations --dry-run output a string (no bytes)

Thanks Markus Holtermann for the report and the review.

django/core/management/commands/makemigrations.py

@@ -1,3 +1,4 @@
+import io
 import os
 import sys
 import warnings
@@ -222,7 +223,7 @@ class Command(BaseCommand):
                         # We just do this once per app
                         directory_created[app_label] = True
                     migration_string = writer.as_string()
-                    with open(writer.path, "wb") as fh:
+                    with io.open(writer.path, "w", encoding='utf-8') as fh:
                         fh.write(migration_string)
                 elif self.verbosity == 3:
                     # Alternatively, makemigrations --dry-run --verbosity 3
@@ -301,7 +302,7 @@ class Command(BaseCommand):
 
                 if not self.dry_run:
                     # Write the merge migrations file to the disk
-                    with open(writer.path, "wb") as fh:
+                    with io.open(writer.path, "w", encoding='utf-8') as fh:
                         fh.write(writer.as_string())
                     if self.verbosity > 0:
                         self.stdout.write("\nCreated new merge migration %s" % writer.path)

django/core/management/commands/squashmigrations.py

@@ -1,3 +1,5 @@
+import io
+
 from django.conf import settings
 from django.core.management.base import BaseCommand, CommandError
 from django.db import DEFAULT_DB_ALIAS, connections, migrations
@@ -162,7 +164,7 @@ class Command(BaseCommand):
 
         # Write out the new migration file
         writer = MigrationWriter(new_migration)
-        with open(writer.path, "wb") as fh:
+        with io.open(writer.path, "w", encoding='utf-8') as fh:
             fh.write(writer.as_string())
 
         if self.verbosity > 0:

django/db/migrations/writer.py

@@ -217,7 +217,7 @@ class MigrationWriter(object):
         if self.migration.initial:
             items['initial_str'] = "\n    initial = True\n"
 
-        return (MIGRATION_TEMPLATE % items).encode("utf8")
+        return MIGRATION_TEMPLATE % items
 
     @property
     def basedir(self):

tests/migrations/test_commands.py

@@ -1,9 +1,9 @@
 # -*- coding: utf-8 -*-
 from __future__ import unicode_literals
 
-import codecs
 import datetime
 import importlib
+import io
 import os
 import sys
 
@@ -553,7 +553,7 @@ class MakeMigrationsTests(MigrationTestBase):
             initial_file = os.path.join(migration_dir, "0001_initial.py")
             self.assertTrue(os.path.exists(initial_file))
 
-            with codecs.open(initial_file, 'r', encoding='utf-8') as fp:
+            with io.open(initial_file, 'r', encoding='utf-8') as fp:
                 content = fp.read()
                 self.assertIn('# -*- coding: utf-8 -*-', content)
                 self.assertIn('migrations.CreateModel', content)
@@ -696,7 +696,7 @@ class MakeMigrationsTests(MigrationTestBase):
             initial_file = os.path.join(migration_dir, "0001_initial.py")
             self.assertTrue(os.path.exists(initial_file))
 
-            with codecs.open(initial_file, 'r', encoding='utf-8') as fp:
+            with io.open(initial_file, 'r', encoding='utf-8') as fp:
                 content = fp.read()
                 self.assertIn('# -*- coding: utf-8 -*-', content)
 
@@ -788,7 +788,7 @@ class MakeMigrationsTests(MigrationTestBase):
                 call_command("makemigrations", "migrations", name="merge", merge=True, interactive=True, stdout=out)
                 merge_file = os.path.join(migration_dir, '0003_merge.py')
                 self.assertTrue(os.path.exists(merge_file))
-            self.assertIn("Created new merge migration", force_text(out.getvalue()))
+            self.assertIn("Created new merge migration", out.getvalue())
 
     @mock.patch('django.db.migrations.utils.datetime')
     def test_makemigrations_default_merge_name(self, mock_datetime):
@@ -799,7 +799,7 @@ class MakeMigrationsTests(MigrationTestBase):
                 call_command("makemigrations", "migrations", merge=True, interactive=True, stdout=out)
                 merge_file = os.path.join(migration_dir, '0003_merge_20160102_0304.py')
                 self.assertTrue(os.path.exists(merge_file))
-            self.assertIn("Created new merge migration", force_text(out.getvalue()))
+            self.assertIn("Created new merge migration", out.getvalue())
 
     def test_makemigrations_non_interactive_not_null_addition(self):
         """
@@ -832,7 +832,7 @@ class MakeMigrationsTests(MigrationTestBase):
         out = six.StringIO()
         with self.temporary_migration_module(module="migrations.test_migrations"):
             call_command("makemigrations", "migrations", interactive=False, stdout=out)
-        self.assertIn("Alter field slug on author", force_text(out.getvalue()))
+        self.assertIn("Alter field slug on author", out.getvalue())
 
     def test_makemigrations_non_interactive_no_model_rename(self):
         """
@@ -847,8 +847,8 @@ class MakeMigrationsTests(MigrationTestBase):
         out = six.StringIO()
         with self.temporary_migration_module(module="migrations.test_migrations_no_default"):
             call_command("makemigrations", "migrations", interactive=False, stdout=out)
-        self.assertIn("Delete model SillyModel", force_text(out.getvalue()))
+        self.assertIn("Delete model SillyModel", out.getvalue())
-        self.assertIn("Create model RenamedModel", force_text(out.getvalue()))
+        self.assertIn("Create model RenamedModel", out.getvalue())
 
     def test_makemigrations_non_interactive_no_field_rename(self):
         """
@@ -863,8 +863,8 @@ class MakeMigrationsTests(MigrationTestBase):
         out = six.StringIO()
         with self.temporary_migration_module(module="migrations.test_migrations_no_default"):
             call_command("makemigrations", "migrations", interactive=False, stdout=out)
-        self.assertIn("Remove field silly_field from sillymodel", force_text(out.getvalue()))
+        self.assertIn("Remove field silly_field from sillymodel", out.getvalue())
-        self.assertIn("Add field silly_rename to sillymodel", force_text(out.getvalue()))
+        self.assertIn("Add field silly_rename to sillymodel", out.getvalue())
 
     def test_makemigrations_handle_merge(self):
         """
@@ -875,7 +875,7 @@ class MakeMigrationsTests(MigrationTestBase):
             call_command("makemigrations", "migrations", name="merge", merge=True, interactive=False, stdout=out)
             merge_file = os.path.join(migration_dir, '0003_merge.py')
             self.assertTrue(os.path.exists(merge_file))
-        output = force_text(out.getvalue())
+        output = out.getvalue()
         self.assertIn("Merging migrations", output)
         self.assertIn("Branch 0002_second", output)
         self.assertIn("Branch 0002_conflicting_second", output)
@@ -894,7 +894,7 @@ class MakeMigrationsTests(MigrationTestBase):
             )
             merge_file = os.path.join(migration_dir, '0003_merge.py')
             self.assertFalse(os.path.exists(merge_file))
-        output = force_text(out.getvalue())
+        output = out.getvalue()
         self.assertIn("Merging migrations", output)
         self.assertIn("Branch 0002_second", output)
         self.assertIn("Branch 0002_conflicting_second", output)
@@ -913,7 +913,7 @@ class MakeMigrationsTests(MigrationTestBase):
             )
             merge_file = os.path.join(migration_dir, '0003_merge.py')
             self.assertFalse(os.path.exists(merge_file))
-        output = force_text(out.getvalue())
+        output = out.getvalue()
         self.assertIn("Merging migrations", output)
         self.assertIn("Branch 0002_second", output)
         self.assertIn("Branch 0002_conflicting_second", output)
@@ -921,7 +921,8 @@ class MakeMigrationsTests(MigrationTestBase):
 
         # Additional output caused by verbosity 3
         # The complete merge migration file that would be written
-        self.assertIn("# -*- coding: utf-8 -*-", output)
+        # '\n#' is to verify no bytestring prefix before #
+        self.assertIn("\n# -*- coding: utf-8 -*-", output)
         self.assertIn("class Migration(migrations.Migration):", output)
         self.assertIn("dependencies = [", output)
         self.assertIn("('migrations', '0002_second')", output)
@@ -1092,7 +1093,7 @@ class MakeMigrationsTests(MigrationTestBase):
                 migration_file = os.path.join(migration_dir, "%s_%s.py" % (migration_count, migration_name))
                 # Check for existing migration file in migration folder
                 self.assertTrue(os.path.exists(migration_file))
-                with codecs.open(migration_file, "r", encoding="utf-8") as fp:
+                with io.open(migration_file, "r", encoding="utf-8") as fp:
                     content = fp.read()
                     self.assertIn("# -*- coding: utf-8 -*-", content)
                     content = content.replace(" ", "")
@@ -1182,8 +1183,8 @@ class MakeMigrationsTests(MigrationTestBase):
             out = six.StringIO()
             with self.temporary_migration_module(module='migrations.test_auto_now_add'):
                 call_command('makemigrations', 'migrations', interactive=True, stdout=out)
-            output = force_text(out.getvalue())
+            output = out.getvalue()
-            prompt_output = force_text(prompt_stdout.getvalue())
+            prompt_output = prompt_stdout.getvalue()
             self.assertIn("You can accept the default 'timezone.now' by pressing 'Enter'", prompt_output)
             self.assertIn("Add field creation_date to entry", output)
 
@@ -1208,7 +1209,7 @@ class SquashMigrationsTests(MigrationTestBase):
             call_command("squashmigrations", "migrations", "0002", interactive=False, verbosity=0)
 
             squashed_migration_file = os.path.join(migration_dir, "0001_squashed_0002_second.py")
-            with codecs.open(squashed_migration_file, "r", encoding="utf-8") as fp:
+            with io.open(squashed_migration_file, "r", encoding="utf-8") as fp:
                 content = fp.read()
                 self.assertIn("initial = True", content)
 
@@ -1219,7 +1220,7 @@ class SquashMigrationsTests(MigrationTestBase):
         out = six.StringIO()
         with self.temporary_migration_module(module="migrations.test_migrations"):
             call_command("squashmigrations", "migrations", "0002", interactive=False, verbosity=1, stdout=out)
-        self.assertIn("Optimized from 8 operations to 3 operations.", force_text(out.getvalue()))
+        self.assertIn("Optimized from 8 operations to 3 operations.", out.getvalue())
 
     def test_ticket_23799_squashmigrations_no_optimize(self):
         """
@@ -1229,7 +1230,7 @@ class SquashMigrationsTests(MigrationTestBase):
         with self.temporary_migration_module(module="migrations.test_migrations"):
             call_command("squashmigrations", "migrations", "0002",
                          interactive=False, verbosity=1, no_optimize=True, stdout=out)
-        self.assertIn("Skipping optimization", force_text(out.getvalue()))
+        self.assertIn("Skipping optimization", out.getvalue())
 
     def test_squashmigrations_valid_start(self):
         """
@@ -1241,11 +1242,11 @@ class SquashMigrationsTests(MigrationTestBase):
                          interactive=False, verbosity=1, stdout=out)
 
             squashed_migration_file = os.path.join(migration_dir, "0002_second_squashed_0003_third.py")
-            with codecs.open(squashed_migration_file, "r", encoding="utf-8") as fp:
+            with io.open(squashed_migration_file, "r", encoding="utf-8") as fp:
                 content = fp.read()
                 self.assertIn("        ('migrations', '0001_initial')", content)
                 self.assertNotIn("initial = True", content)
-        out = force_text(out.getvalue())
+        out = out.getvalue()
         self.assertNotIn(" - 0001_initial", out)
         self.assertIn(" - 0002_second", out)
         self.assertIn(" - 0003_third", out)

tests/migrations/test_writer.py

@@ -25,6 +25,7 @@ from django.test import SimpleTestCase, ignore_warnings, mock
 from django.utils import datetime_safe, six
 from django.utils._os import upath
 from django.utils.deconstruct import deconstructible
+from django.utils.encoding import force_str
 from django.utils.functional import SimpleLazyObject
 from django.utils.timezone import FixedOffset, get_default_timezone, utc
 from django.utils.translation import ugettext_lazy as _
@@ -181,7 +182,7 @@ class WriterTests(SimpleTestCase):
     def safe_exec(self, string, value=None):
         l = {}
         try:
-            exec(string, globals(), l)
+            exec(force_str(string), globals(), l)
         except Exception as e:
             if value:
                 self.fail("Could not exec %r (from value %r): %s" % (string.strip(), value, e))
@@ -546,8 +547,6 @@ class WriterTests(SimpleTestCase):
         })
         writer = MigrationWriter(migration)
         output = writer.as_string()
-        # It should NOT be unicode.
-        self.assertIsInstance(output, six.binary_type, "Migration as_string returned unicode")
         # We don't test the output formatting - that's too fragile.
         # Just make sure it runs for now, and that things look alright.
         result = self.safe_exec(output)
@@ -615,7 +614,7 @@ class WriterTests(SimpleTestCase):
             ]
         })
         writer = MigrationWriter(migration)
-        output = writer.as_string().decode('utf-8')
+        output = writer.as_string()
         self.assertIn(
             "import datetime\n"
             "from django.db import migrations, models\n"
@@ -633,7 +632,7 @@ class WriterTests(SimpleTestCase):
         dt = datetime.datetime(2015, 7, 31, 4, 40, 0, 0, tzinfo=utc)
         with mock.patch('django.db.migrations.writer.now', lambda: dt):
             writer = MigrationWriter(migration)
-            output = writer.as_string().decode('utf-8')
+            output = writer.as_string()
 
         self.assertTrue(
             output.startswith(
@@ -657,7 +656,7 @@ class WriterTests(SimpleTestCase):
             ]
         })
         writer = MigrationWriter(migration)
-        output = writer.as_string().decode('utf-8')
+        output = writer.as_string()
         self.assertIn("from django.db import migrations\n", output)
 
     def test_deconstruct_class_arguments(self):