From 5928db8bc09077946c30cd3199932cdc6b670cb4 Mon Sep 17 00:00:00 2001 From: finswimmer Date: Sat, 16 Nov 2024 19:04:48 +0100 Subject: [PATCH] feat: add confirmation step --- src/poetry/config/config_source.py | 24 +++++++++--- src/poetry/console/commands/config.py | 21 ++++++++-- tests/console/commands/test_config.py | 56 +++++++++++++++++++-------- 3 files changed, 75 insertions(+), 26 deletions(-) diff --git a/src/poetry/config/config_source.py b/src/poetry/config/config_source.py index 61f7b37b648..e06c934e70a 100644 --- a/src/poetry/config/config_source.py +++ b/src/poetry/config/config_source.py @@ -39,25 +39,22 @@ class ConfigSourceMigration: new_key: str | None value_migration: dict[Any, Any] = dataclasses.field(default_factory=dict) - def apply(self, config_source: ConfigSource, io: IO | None = None) -> None: + def dry_run(self, config_source: ConfigSource, io: IO | None = None) -> bool: io = io or NullIO() try: old_value = config_source.get_property(self.old_key) except PropertyNotFoundError: - return + return False new_value = ( self.value_migration[old_value] if self.value_migration else old_value ) - config_source.remove_property(self.old_key) - msg = f"{self.old_key} = {json.dumps(old_value)}" if self.new_key is not None and new_value is not UNSET: msg += f" -> {self.new_key} = {json.dumps(new_value)}" - config_source.add_property(self.new_key, new_value) elif self.new_key is None: msg += " -> Removed from config" elif self.new_key and new_value is UNSET: @@ -65,6 +62,23 @@ def apply(self, config_source: ConfigSource, io: IO | None = None) -> None: io.write_line(msg) + return True + + def apply(self, config_source: ConfigSource) -> None: + try: + old_value = config_source.get_property(self.old_key) + except PropertyNotFoundError: + return + + new_value = ( + self.value_migration[old_value] if self.value_migration else old_value + ) + + config_source.remove_property(self.old_key) + + if self.new_key is not None and new_value is not UNSET: + config_source.add_property(self.new_key, new_value) + def drop_empty_config_category( keys: list[str], config: dict[Any, Any] diff --git a/src/poetry/console/commands/config.py b/src/poetry/console/commands/config.py index cb0083ef63c..8002e537811 100644 --- a/src/poetry/console/commands/config.py +++ b/src/poetry/console/commands/config.py @@ -357,9 +357,22 @@ def _migrate(self) -> None: config_source = FileConfigSource(config_file) - self.io.write_line("Starting config migration ...") + self.io.write_line("Checking for required migrations ...") - for migration in CONFIG_MIGRATIONS: - migration.apply(config_source, io=self.io) + required_migrations = [ + migration + for migration in CONFIG_MIGRATIONS + if migration.dry_run(config_source, io=self.io) + ] - self.io.write_line("Config migration successfully done.") + if not required_migrations: + self.io.write_line("Already up to date.") + return + + if not self.io.is_interactive() or self.confirm( + "Proceed with migration?: ", False + ): + for migration in required_migrations: + migration.apply(config_source) + + self.io.write_line("Config migration successfully done.") diff --git a/tests/console/commands/test_config.py b/tests/console/commands/test_config.py index 9c811532526..d4925b85129 100644 --- a/tests/console/commands/test_config.py +++ b/tests/console/commands/test_config.py @@ -569,32 +569,54 @@ def test_config_solver_lazy_wheel( assert not repo._lazy_wheel +current_config = """\ +[experimental] +system-git-client = true + +[virtualenvs] +prefer-active-python = false +""" + +config_migrated = """\ +system-git-client = true + +[virtualenvs] +use-poetry-python = true +""" + + +@pytest.mark.parametrize( + ["proceed", "expected_config"], + [ + ("yes", config_migrated), + ("no", current_config), + ], +) def test_config_migrate( - tester: CommandTester, mocker: MockerFixture, tmp_path: Path + proceed: str, + expected_config: str, + tester: CommandTester, + mocker: MockerFixture, + tmp_path: Path, ) -> None: config_dir = tmp_path / "config" mocker.patch("poetry.locations.CONFIG_DIR", config_dir) config_file = Path(config_dir / "config.toml") - config_data = textwrap.dedent("""\ - [experimental] - system-git-client = true - - [virtualenvs] - prefer-active-python = false - """) with config_file.open("w") as fh: - fh.write(config_data) - - tester.execute("--migrate") + fh.write(current_config) - expected_config = textwrap.dedent("""\ - system-git-client = true + tester.execute("--migrate", inputs=proceed) - [virtualenvs] - use-poetry-python = true + expected_output = textwrap.dedent("""\ + Checking for required migrations ... + experimental.system-git-client = true -> system-git-client = true + virtualenvs.prefer-active-python = false -> virtualenvs.use-poetry-python = true """) + output = tester.io.fetch_output() + assert output.startswith(expected_output) + with config_file.open("r") as fh: assert fh.read() == expected_config @@ -612,7 +634,7 @@ def test_config_migrate_local_config(tester: CommandTester, poetry: Poetry) -> N with local_config.open("w") as fh: fh.write(config_data) - tester.execute("--migrate --local") + tester.execute("--migrate --local", inputs="yes") expected_config = textwrap.dedent("""\ system-git-client = true @@ -629,4 +651,4 @@ def test_config_migrate_local_config_should_raise_if_not_found( tester: CommandTester, ) -> None: with pytest.raises(RuntimeError, match="No local config file found"): - tester.execute("--migrate --local") + tester.execute("--migrate --local", inputs="yes")