-
Notifications
You must be signed in to change notification settings - Fork 192
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixed the yes/no prompt in verdi computer delete #6624
base: main
Are you sure you want to change the base?
Changes from all commits
5c2d52c
64c97a1
f0681ff
a33cfb2
f30bd1f
15b82d1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -772,76 +772,75 @@ 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 = 'y' # 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) | ||||||
|
||||||
with pytest.raises(NotExistent): | ||||||
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): | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I found this test name kinda misleading for it's purpose
Suggested change
|
||||||
"""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) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
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) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please check explicitly if the raise is due to |
||||||
|
||||||
|
||||||
@pytest.mark.parametrize('non_interactive_editor', ('vim -cwq',), indirect=True) | ||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
May I take the chance to improve the message at line 642:
(if there are no nodes, this is a bit annoying saying "0 nodes marked for deletion")