From 0894ab1bc13acb50465bed56accc0a2be4fee28c Mon Sep 17 00:00:00 2001 From: Krishna Sanjay Bhujade <156920139+Krishnabhujade@users.noreply.github.com> Date: Wed, 20 Nov 2024 19:40:25 +0530 Subject: [PATCH 1/6] Fixed the yes/no prompt in verdi computer delete --- src/aiida/cmdline/commands/cmd_computer.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/aiida/cmdline/commands/cmd_computer.py b/src/aiida/cmdline/commands/cmd_computer.py index d0630bbde7..cc8ded3b5f 100644 --- a/src/aiida/cmdline/commands/cmd_computer.py +++ b/src/aiida/cmdline/commands/cmd_computer.py @@ -636,7 +636,7 @@ def _dry_run_callback(pks): if pks: echo.echo_report('The nodes with the following pks would be deleted: ' + ' '.join(map(str, pks))) echo.echo_warning(f'YOU ARE ABOUT TO DELETE {len(pks)} NODES! THIS CANNOT BE UNDONE!') - confirm = click.prompt('Shall I continue? [yes/N]', type=str) == 'yes' + confirm = click.prompt('Shall I continue? [yes/NO]', type=str) == 'yes' if not confirm: raise click.Abort return not confirm From 654ec1306f8ba77830a9b3e8b8509549bdf67667 Mon Sep 17 00:00:00 2001 From: Alexander Goscinski Date: Mon, 25 Nov 2024 13:37:34 +0100 Subject: [PATCH 2/6] Update src/aiida/cmdline/commands/cmd_computer.py --- src/aiida/cmdline/commands/cmd_computer.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/aiida/cmdline/commands/cmd_computer.py b/src/aiida/cmdline/commands/cmd_computer.py index cc8ded3b5f..e1640cde11 100644 --- a/src/aiida/cmdline/commands/cmd_computer.py +++ b/src/aiida/cmdline/commands/cmd_computer.py @@ -636,7 +636,7 @@ def _dry_run_callback(pks): if pks: echo.echo_report('The nodes with the following pks would be deleted: ' + ' '.join(map(str, pks))) echo.echo_warning(f'YOU ARE ABOUT TO DELETE {len(pks)} NODES! THIS CANNOT BE UNDONE!') - confirm = click.prompt('Shall I continue? [yes/NO]', type=str) == 'yes' + click.confirm('Shall I continue?', default=False) if not confirm: raise click.Abort return not confirm From 8f74b86aaf127eea246e632d1b55575bd9595090 Mon Sep 17 00:00:00 2001 From: Alexander Goscinski Date: Mon, 25 Nov 2024 16:42:04 +0100 Subject: [PATCH 3/6] Update src/aiida/cmdline/commands/cmd_computer.py --- src/aiida/cmdline/commands/cmd_computer.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/aiida/cmdline/commands/cmd_computer.py b/src/aiida/cmdline/commands/cmd_computer.py index e1640cde11..52f3de933b 100644 --- a/src/aiida/cmdline/commands/cmd_computer.py +++ b/src/aiida/cmdline/commands/cmd_computer.py @@ -636,7 +636,7 @@ def _dry_run_callback(pks): if pks: echo.echo_report('The nodes with the following pks would be deleted: ' + ' '.join(map(str, pks))) echo.echo_warning(f'YOU ARE ABOUT TO DELETE {len(pks)} NODES! THIS CANNOT BE UNDONE!') - click.confirm('Shall I continue?', default=False) + confirm = click.confirm('Shall I continue?', default=False) if not confirm: raise click.Abort return not confirm From f74dd06763dc45da8122fbcc6c8948f847d88e82 Mon Sep 17 00:00:00 2001 From: Alexander Goscinski Date: Mon, 25 Nov 2024 17:25:42 +0100 Subject: [PATCH 4/6] fix test --- tests/cmdline/commands/test_computer.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/cmdline/commands/test_computer.py b/tests/cmdline/commands/test_computer.py index 17de0f0761..689195a2a8 100644 --- a/tests/cmdline/commands/test_computer.py +++ b/tests/cmdline/commands/test_computer.py @@ -796,7 +796,7 @@ def test_computer_delete_with_nodes(self): filepath_executable='/remote/abs/path', ).store() - false_user_input = 'y' # most common mistake + false_user_input = '' # most common mistake user_input = 'yes' # Abort in case of wrong input From 3fec7dba7bc9d12946a8325d9074344c46d4fa59 Mon Sep 17 00:00:00 2001 From: Alexander Goscinski Date: Tue, 26 Nov 2024 15:29:48 +0100 Subject: [PATCH 5/6] Refactor tests a bit To simplify the testing of 'y' and 'yes' case without rerunning unecessary database operations the tests for `verdi computer delete` have been refactored. --- tests/cmdline/commands/test_computer.py | 79 ++++++++++++------------- 1 file changed, 39 insertions(+), 40 deletions(-) diff --git a/tests/cmdline/commands/test_computer.py b/tests/cmdline/commands/test_computer.py index 689195a2a8..e5f0c771bb 100644 --- a/tests/cmdline/commands/test_computer.py +++ b/tests/cmdline/commands/test_computer.py @@ -773,41 +773,31 @@ def test_computer_relabel(self): # The new label should be available orm.Computer.collection.get(label='comp_cli_test_computer') - def test_computer_delete_with_nodes(self): - """check if 'verdi computer delete' works when there are associated nodes""" + # ensure that 'yes' works for backwards compatibility, see issue #6422 + @pytest.mark.parametrize('user_input', ['y', 'yes']) + def test_computer_delete_existing_computer_successful(self, user_input): + """Test if `verdi computer delete` works correctly for an existing computer when it is deleted""" from aiida.common.exceptions import NotExistent - label = 'computer_69' - compute_temp = orm.Computer( + label = 'computer_temp' + computer_temp = orm.Computer( label=label, hostname='localhost', transport_type='core.local', scheduler_type='core.direct', workdir='/tmp/aiida', ) - compute_temp.store() - compute_temp.configure(safe_interval=0) + computer_temp.store() + computer_temp.configure(safe_interval=0) - c_label = 'code_69' + code_label = 'code_temp' orm.InstalledCode( - label=c_label, + label=code_label, default_calc_job_plugin='core.arithmetic.add', - computer=compute_temp, + computer=computer_temp, filepath_executable='/remote/abs/path', ).store() - false_user_input = '' # most common mistake - user_input = 'yes' - - # Abort in case of wrong input - self.cli_runner(computer_delete, [label], user_input=false_user_input, raises=True) - orm.load_code(c_label) - - # Safety check in case of --dry-run - options = [label, '--dry-run'] - self.cli_runner(computer_delete, options) - orm.load_code(c_label) - # A successul delete, including all associated nodes self.cli_runner(computer_delete, [label], user_input=user_input) @@ -815,34 +805,43 @@ def test_computer_delete_with_nodes(self): orm.Computer.collection.get(label=label) with pytest.raises(NotExistent): - orm.load_code(c_label) - - def test_computer_delete(self): - """Test if 'verdi computer delete' command works""" - from aiida.common.exceptions import NotExistent + orm.load_code(code_label) - # Setup a computer to delete during the test - label = 'computer_for_test_label' - orm.Computer( + def test_computer_delete_existing_computer(self): + """Test if `verdi computer delete` works correctly for an existing computer when it is not deleted""" + label = 'computer_temp' + computer_temp = orm.Computer( label=label, hostname='localhost', transport_type='core.local', scheduler_type='core.direct', workdir='/tmp/aiida', + ) + computer_temp.store() + computer_temp.configure(safe_interval=0) + + code_label = 'code_temp' + orm.InstalledCode( + label=code_label, + default_calc_job_plugin='core.arithmetic.add', + computer=computer_temp, + filepath_executable='/remote/abs/path', ).store() - # and configure it - options = ['core.local', label, '--non-interactive', '--safe-interval', '0'] - self.cli_runner(computer_configure, options) - # See if the command complains about not getting an invalid computer - user_input = 'yes' - self.cli_runner(computer_delete, ['computer_that_does_not_exist'], raises=True, user_input=user_input) + # Tests that Abort in case of wrong input + false_user_input = '' # most common input that could happen by accident + self.cli_runner(computer_delete, [label], user_input=false_user_input, raises=True) + orm.load_code(code_label) - # Delete a computer name successully. - self.cli_runner(computer_delete, [label], user_input=user_input) - # Check that the computer really was deleted - with pytest.raises(NotExistent): - orm.Computer.collection.get(label=label) + # Safety check in case of --dry-run + options = [label, '--dry-run'] + self.cli_runner(computer_delete, options) + orm.load_code(code_label) + + def test_computer_delete_nonexisting_computer(self): + """Test if `verdi computer delete` command works correctly for a nonexisting computer""" + # See if the command complains about not getting an nonexisting computer + self.cli_runner(computer_delete, ['computer_that_does_not_exist'], raises=True) @pytest.mark.parametrize('non_interactive_editor', ('vim -cwq',), indirect=True) From 6c699932d374161cf500f00fe10c3614041286f4 Mon Sep 17 00:00:00 2001 From: Alexander Goscinski Date: Thu, 6 Mar 2025 08:28:52 +0100 Subject: [PATCH 6/6] Apply khsrali review --- tests/cmdline/commands/test_computer.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/tests/cmdline/commands/test_computer.py b/tests/cmdline/commands/test_computer.py index e5f0c771bb..00061859fa 100644 --- a/tests/cmdline/commands/test_computer.py +++ b/tests/cmdline/commands/test_computer.py @@ -807,7 +807,7 @@ def test_computer_delete_existing_computer_successful(self, user_input): with pytest.raises(NotExistent): orm.load_code(code_label) - def test_computer_delete_existing_computer(self): + def test_computer_delete_existing_computer_not_deleted(self): """Test if `verdi computer delete` works correctly for an existing computer when it is not deleted""" label = 'computer_temp' computer_temp = orm.Computer( @@ -831,17 +831,20 @@ def test_computer_delete_existing_computer(self): # Tests that Abort in case of wrong input false_user_input = '' # most common input that could happen by accident self.cli_runner(computer_delete, [label], user_input=false_user_input, raises=True) + # raises an error if not existing orm.load_code(code_label) # Safety check in case of --dry-run options = [label, '--dry-run'] - self.cli_runner(computer_delete, options) + self.cli_runner(computer_delete, options, user_input='y') + # raises an error if not existing orm.load_code(code_label) def test_computer_delete_nonexisting_computer(self): """Test if `verdi computer delete` command works correctly for a nonexisting computer""" # See if the command complains about not getting an nonexisting computer - self.cli_runner(computer_delete, ['computer_that_does_not_exist'], raises=True) + result = self.cli_runner(computer_delete, ['computer_that_does_not_exist'], user_input='y', raises=True) + assert 'no Computer found with LABEL' in result.stderr @pytest.mark.parametrize('non_interactive_editor', ('vim -cwq',), indirect=True)