Skip to content

Commit 71fc14f

Browse files
Align the yes/no prompt in verdi computer delete with other prompts
The yes/no prompt of `verdi computer delete` is the only prompt in the CLI that does not accept 'y' as valid answer but only 'yes'. This has been aligned with the other prompts to also support 'y'. Furthermore, no prompt is invoked when no nodes are associated with the computer. The tests have been restructured to more granular test cases and the names have been cleaned up We had to increase check count in `MockProcess` to avoid the timeout.
1 parent 474e0fa commit 71fc14f

File tree

3 files changed

+87
-45
lines changed

3 files changed

+87
-45
lines changed

src/aiida/cmdline/commands/cmd_computer.py

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -628,30 +628,35 @@ def computer_delete(computer, dry_run):
628628
builder = QueryBuilder()
629629
builder.append(Computer, filters={'label': label}, tag='computer')
630630
builder.append(Node, with_computer='computer', project=Node.fields.pk)
631-
node_pks = builder.all(flat=True)
631+
associated_nodes_pk = builder.all(flat=True)
632632

633-
echo.echo_report(f'This computer has {len(node_pks)} associated nodes')
633+
echo.echo_report(f'This computer has {len(associated_nodes_pk)} associated nodes')
634634

635635
def _dry_run_callback(pks):
636636
if pks:
637637
echo.echo_report('The nodes with the following pks would be deleted: ' + ' '.join(map(str, pks)))
638-
echo.echo_warning(f'YOU ARE ABOUT TO DELETE {len(pks)} NODES! THIS CANNOT BE UNDONE!')
639-
confirm = click.prompt('Shall I continue? [yes/N]', type=str) == 'yes'
638+
echo.echo_warning(
639+
f'YOU ARE ABOUT TO DELETE {len(pks)} NODES AND COMPUTER {label!r}! THIS CANNOT BE UNDONE!'
640+
)
641+
642+
confirm = click.confirm('Shall I continue?', default=False)
640643
if not confirm:
641644
raise click.Abort
642645
return not confirm
643646

644-
delete_nodes(node_pks, dry_run=dry_run or _dry_run_callback)
647+
if associated_nodes_pk:
648+
delete_nodes(associated_nodes_pk, dry_run=dry_run or _dry_run_callback)
645649

646650
if dry_run:
647651
return
648652

653+
# We delete the computer separately from the associated nodes, since the computer pk is in a different table
649654
try:
650655
orm.Computer.collection.delete(computer.pk)
651656
except InvalidOperation as error:
652657
echo.echo_critical(str(error))
653658

654-
echo.echo_success(f'Computer `{label}` {"and all its associated nodes" if node_pks else ""} deleted.')
659+
echo.echo_success(f'Computer `{label}` {"and all its associated nodes " if associated_nodes_pk else ""}deleted.')
655660

656661

657662
class LazyConfigureGroup(VerdiCommandGroup):

tests/cmdline/commands/test_computer.py

Lines changed: 73 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -773,77 +773,113 @@ def test_computer_relabel(self):
773773
# The new label should be available
774774
orm.Computer.collection.get(label='comp_cli_test_computer')
775775

776-
def test_computer_delete_with_nodes(self):
777-
"""check if 'verdi computer delete' works when there are associated nodes"""
776+
# Ensures that 'yes' works for backwards compatibility, see issue #6422
777+
@pytest.mark.parametrize('user_input', ['y', 'yes'])
778+
def test_computer_delete_existing_computer_successful(self, user_input):
779+
"""Test if `verdi computer delete` successfully deletes when applied on an existing computer."""
778780
from aiida.common.exceptions import NotExistent
779781

780-
label = 'computer_69'
781-
compute_temp = orm.Computer(
782+
label = 'computer_temp'
783+
computer_temp = orm.Computer(
782784
label=label,
783785
hostname='localhost',
784786
transport_type='core.local',
785787
scheduler_type='core.direct',
786788
workdir='/tmp/aiida',
787789
)
788-
compute_temp.store()
789-
compute_temp.configure(safe_interval=0)
790+
computer_temp.store()
791+
computer_temp.configure(safe_interval=0)
790792

791-
c_label = 'code_69'
793+
code_label = 'code_temp'
792794
orm.InstalledCode(
793-
label=c_label,
795+
label=code_label,
794796
default_calc_job_plugin='core.arithmetic.add',
795-
computer=compute_temp,
797+
computer=computer_temp,
796798
filepath_executable='/remote/abs/path',
797799
).store()
798800

799-
false_user_input = 'y' # most common mistake
800-
user_input = 'yes'
801-
802-
# Abort in case of wrong input
803-
self.cli_runner(computer_delete, [label], user_input=false_user_input, raises=True)
804-
orm.load_code(c_label)
805-
806-
# Safety check in case of --dry-run
807-
options = [label, '--dry-run']
808-
self.cli_runner(computer_delete, options)
809-
orm.load_code(c_label)
810-
811801
# A successul delete, including all associated nodes
812802
self.cli_runner(computer_delete, [label], user_input=user_input)
813803

814804
with pytest.raises(NotExistent):
815805
orm.Computer.collection.get(label=label)
816806

817807
with pytest.raises(NotExistent):
818-
orm.load_code(c_label)
808+
orm.load_computer(label)
809+
810+
with pytest.raises(NotExistent):
811+
orm.load_code(code_label)
812+
813+
# Ensures that 'yes' works for backwards compatibility, see issue #6422
814+
@pytest.mark.parametrize('user_input', ['y', 'yes'])
815+
def test_computer_delete_existing_computer_successful_no_assciated_nodes(self, user_input):
816+
"""Test if `verdi computer delete` successfully deletes when applied on a computer without associated nodes.
819817
820-
def test_computer_delete(self):
821-
"""Test if 'verdi computer delete' command works"""
818+
In this case no prompt should appear."""
822819
from aiida.common.exceptions import NotExistent
823820

824-
# Setup a computer to delete during the test
825-
label = 'computer_for_test_label'
826-
orm.Computer(
821+
label = 'computer_temp'
822+
computer_temp = orm.Computer(
827823
label=label,
828824
hostname='localhost',
829825
transport_type='core.local',
830826
scheduler_type='core.direct',
831827
workdir='/tmp/aiida',
832-
).store()
833-
# and configure it
834-
options = ['core.local', label, '--non-interactive', '--safe-interval', '0']
835-
self.cli_runner(computer_configure, options)
828+
)
829+
computer_temp.store()
836830

837-
# See if the command complains about not getting an invalid computer
838-
user_input = 'yes'
839-
self.cli_runner(computer_delete, ['computer_that_does_not_exist'], raises=True, user_input=user_input)
831+
# A successul delete, including all associated nodes
832+
self.cli_runner(computer_delete, [label])
840833

841-
# Delete a computer name successully.
842-
self.cli_runner(computer_delete, [label], user_input=user_input)
843-
# Check that the computer really was deleted
844834
with pytest.raises(NotExistent):
845835
orm.Computer.collection.get(label=label)
846836

837+
with pytest.raises(NotExistent):
838+
orm.load_computer(label)
839+
840+
def test_computer_delete_existing_computer_not_deleted(self):
841+
"""Test if `verdi computer delete` behaves correctly when applied on an existing computer without deletion.
842+
843+
This case includes when the user enters a wrong input on the prompt and the usage of the dry-run option."""
844+
label = 'computer_temp'
845+
computer_temp = orm.Computer(
846+
label=label,
847+
hostname='localhost',
848+
transport_type='core.local',
849+
scheduler_type='core.direct',
850+
workdir='/tmp/aiida',
851+
)
852+
computer_temp.store()
853+
computer_temp.configure(safe_interval=0)
854+
855+
code_label = 'code_temp'
856+
orm.InstalledCode(
857+
label=code_label,
858+
default_calc_job_plugin='core.arithmetic.add',
859+
computer=computer_temp,
860+
filepath_executable='/remote/abs/path',
861+
).store()
862+
863+
# Tests that Abort in case of wrong input
864+
false_user_input = '' # most common input that could happen by accident
865+
self.cli_runner(computer_delete, [label], user_input=false_user_input, raises=True)
866+
# raises an error if not existing
867+
assert orm.load_code(code_label).label == code_label
868+
assert orm.Computer.collection.get(label=label).label == label
869+
870+
# Safety check in case of --dry-run
871+
options = [label, '--dry-run']
872+
self.cli_runner(computer_delete, options, user_input='y')
873+
# raises an error if not existing
874+
assert orm.load_code(code_label).label == code_label
875+
assert orm.Computer.collection.get(label=label).label == label
876+
877+
def test_computer_delete_nonexisting_computer(self):
878+
"""Test if `verdi computer delete` command behaves correctly when deleting a nonexisting computer"""
879+
# See if the command complains about not getting an existing computer
880+
result = self.cli_runner(computer_delete, ['computer_that_does_not_exist'], user_input='y', raises=True)
881+
assert 'no Computer found with LABEL<computer_that_does_not_exist>' in result.stderr
882+
847883

848884
@pytest.mark.parametrize('non_interactive_editor', ('vim -cwq',), indirect=True)
849885
def test_computer_duplicate_interactive(run_cli_command, aiida_localhost, non_interactive_editor):

tests/manage/test_profile_access.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -201,12 +201,13 @@ def _wait_for_checkfile(self, temppath_checkfile):
201201
import time
202202

203203
check_count = 0
204+
max_check_count = 400
204205

205-
while check_count < 100 and not temppath_checkfile.exists():
206+
while check_count < max_check_count and not temppath_checkfile.exists():
206207
time.sleep(0.1)
207208
check_count += 1
208209

209-
if check_count == 100:
210+
if check_count == max_check_count:
210211
raise RuntimeError('Process loading was too slow!')
211212

212213
def start(self, should_raise=False, runtime_secs=60):

0 commit comments

Comments
 (0)