Skip to content

Commit e140ab6

Browse files
Warn when --fake marks multiple migrations as run (#1400)
* Warn when --fake marks multiple migrations as run * use `input` --------- Co-authored-by: Daniel Townsend <dan@dantownsend.co.uk>
1 parent e8316f8 commit e140ab6

2 files changed

Lines changed: 54 additions & 21 deletions

File tree

piccolo/apps/migrations/commands/forwards.py

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,20 @@ async def run_migrations(self, app_config: AppConfig) -> MigrationResult:
7373

7474
if subset:
7575
n = len(subset)
76+
77+
if self.fake and n > 1:
78+
if (
79+
input(
80+
f"⚠️ --fake will mark all {n} migrations as run "
81+
"without applying them. Continue? [y/N]"
82+
).lower()
83+
!= "y"
84+
):
85+
print("Migration stopped")
86+
return MigrationResult(
87+
success=False, message="Migration stopped"
88+
)
89+
7690
print(f"🚀 Running {n} migration{'s' if n != 1 else ''}:")
7791

7892
for _id in subset:
@@ -81,20 +95,19 @@ async def run_migrations(self, app_config: AppConfig) -> MigrationResult:
8195

8296
if isinstance(response, MigrationManager):
8397
if self.fake or response.fake:
84-
print(f"- {_id}: faked! ⏭️")
98+
print(f" - {_id}: faked! ⏭️")
8599
else:
86100
if self.preview:
87101
response.preview = True
88102
await response.run()
89-
90-
print("ok! ✔️")
103+
print("ok! ✔️")
91104

92105
if not self.preview:
93106
await Migration.insert().add(
94107
Migration(name=_id, app_name=app_config.app_name)
95108
).run()
96109

97-
return MigrationResult(success=True, message="migration succeeded")
110+
return MigrationResult(success=True, message="Migration succeeded")
98111

99112
async def run(self) -> MigrationResult:
100113
await self.create_migration_table()

tests/apps/migrations/commands/test_forwards_backwards.py

Lines changed: 37 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,10 @@
66
from unittest.mock import MagicMock, call, patch
77

88
from piccolo.apps.migrations.commands.backwards import backwards
9+
from piccolo.apps.migrations.commands.base import BaseMigrationManager
910
from piccolo.apps.migrations.commands.forwards import forwards
1011
from piccolo.apps.migrations.tables import Migration
12+
from piccolo.conf.apps import Finder
1113
from piccolo.utils.sync import run_sync
1214
from tests.base import AsyncMock, engines_only
1315
from tests.example_apps.music.tables import (
@@ -46,6 +48,16 @@ class TestForwardsBackwards(TestCase):
4648
Test the forwards and backwards migration commands.
4749
"""
4850

51+
def get_migration_names(self):
52+
app_config = Finder().get_app_config(app_name="music")
53+
finder = BaseMigrationManager()
54+
migration_modules_dict = finder.get_migration_modules(
55+
folder_path=app_config.migrations_folder_path.__str__()
56+
)
57+
return finder.get_migration_ids(
58+
migration_module_dict=migration_modules_dict
59+
)
60+
4961
def test_forwards_backwards_all_migrations(self):
5062
"""
5163
Test running all of the migrations forwards, then backwards.
@@ -221,10 +233,13 @@ def test_forwards_no_migrations(self, print_: MagicMock):
221233
)
222234

223235
@engines_only("postgres")
224-
def test_forwards_fake(self):
236+
@patch("piccolo.apps.migrations.commands.forwards.input")
237+
def test_forwards_fake(self, _input: MagicMock):
225238
"""
226239
Make sure migrations can be faked on the command line.
227240
"""
241+
_input.return_value = "y"
242+
228243
run_sync(forwards(app_name="music", migration_id="all", fake=True))
229244

230245
for table_class in TABLE_CLASSES:
@@ -234,23 +249,28 @@ def test_forwards_fake(self):
234249
Migration.select(Migration.name).output(as_list=True).run_sync()
235250
)
236251

237-
self.assertEqual(
238-
ran_migration_names,
239-
# TODO - rather than hardcoding, might fetch these dynamically.
240-
[
241-
"2020-12-17T18:44:30",
242-
"2020-12-17T18:44:39",
243-
"2020-12-17T18:44:44",
244-
"2021-07-25T22:38:48:009306",
245-
"2021-09-06T13:58:23:024723",
246-
"2021-11-13T14:01:46:114725",
247-
"2024-05-28T23:15:41:018844",
248-
"2024-06-19T18:11:05:793132",
249-
"2026-02-22T00:41:01:493867",
250-
],
251-
)
252+
self.assertListEqual(ran_migration_names, self.get_migration_names())
252253

253254
@engines_only("postgres")
255+
@patch("piccolo.apps.migrations.commands.forwards.input")
256+
def test_forwards_fake_multiple_warns(
257+
self,
258+
input_: MagicMock,
259+
):
260+
"""
261+
``--fake`` marks migrations as run without applying them; when it
262+
covers several migrations at once it should warn the user.
263+
264+
https://github.com/piccolo-orm/piccolo/issues/1255
265+
"""
266+
input_.return_value = "y"
267+
run_sync(forwards(app_name="music", migration_id="all", fake=True))
268+
migration_count = len(self.get_migration_names())
269+
input_.assert_called_once_with(
270+
f"⚠️ --fake will mark all {migration_count} migrations as run "
271+
"without applying them. Continue? [y/N]"
272+
)
273+
254274
@patch("piccolo.apps.migrations.commands.forwards.print")
255275
def test_hardcoded_fake_migrations(self, print_: MagicMock):
256276
"""
@@ -276,7 +296,7 @@ def test_hardcoded_fake_migrations(self, print_: MagicMock):
276296
print_.mock_calls,
277297
)
278298
self.assertIn(
279-
call(f"- {migration_name}: faked! ⏭️"),
299+
call(f" - {migration_name}: faked! ⏭️"),
280300
print_.mock_calls,
281301
)
282302

0 commit comments

Comments
 (0)