Merge branch 'develop' into fix-3094-ssh-direct-hanging

.github/workflows/tests.sh

@@ -18,6 +18,7 @@ export PYTEST_ADDOPTS="${PYTEST_ADDOPTS} --cov-config=${GITHUB_WORKSPACE}/.cover
 export PYTEST_ADDOPTS="${PYTEST_ADDOPTS} --cov-report xml"
 export PYTEST_ADDOPTS="${PYTEST_ADDOPTS} --cov-append"
 export PYTEST_ADDOPTS="${PYTEST_ADDOPTS} --cov=aiida"
+export PYTEST_ADDOPTS="${PYTEST_ADDOPTS} --verbose"
 
 # daemon tests
 verdi daemon start 4

aiida/cmdline/commands/cmd_group.py

@@ -47,12 +47,41 @@ def group_add_nodes(group, force, nodes):
 @with_dbenv()
 def group_remove_nodes(group, nodes, clear, force):
     """Remove nodes from a group."""
-    if clear:
-        message = f'Do you really want to remove ALL the nodes from Group<{group.label}>?'
-    else:
-        message = f'Do you really want to remove {len(nodes)} nodes from Group<{group.label}>?'
+    from aiida.orm import QueryBuilder, Group, Node
+
+    label = group.label
+    klass = group.__class__.__name__
+
+    if nodes and clear:
+        echo.echo_critical(
+            'Specify either the `--clear` flag to remove all nodes or the identifiers of the nodes you want to remove.'
+        )
 
     if not force:
+
+        if nodes:
+            node_pks = [node.pk for node in nodes]
+
+            query = QueryBuilder()
+            query.append(Group, filters={'id': group.pk}, tag='group')
+            query.append(Node, with_group='group', filters={'id': {'in': node_pks}}, project='id')
+
+            group_node_pks = query.all(flat=True)
+
+            if not group_node_pks:
+                echo.echo_critical(f'None of the specified nodes are in {klass}<{label}>.')
+
+            if len(node_pks) > len(group_node_pks):
+                node_pks = set(node_pks).difference(set(group_node_pks))
+                echo.echo_warning(f'{len(node_pks)} nodes with PK {node_pks} are not in {klass}<{label}>.')
+
+            message = f'Are you sure you want to remove {len(group_node_pks)} nodes from {klass}<{label}>?'
+
+        elif clear:
+            message = f'Are you sure you want to remove ALL the nodes from {klass}<{label}>?'
+        else:
+            echo.echo_critical(f'No nodes were provided for removal from {klass}<{label}>.')
+
         click.confirm(message, abort=True)
 
     if clear:

setup.json

@@ -92,6 +92,7 @@
             "notebook~=6.1,>=6.1.5"
         ],
         "pre-commit": [
+            "astroid<2.5",
             "mypy==0.790",
             "packaging==20.3",
             "pre-commit~=2.2",

tests/cmdline/commands/test_group.py

@@ -12,6 +12,7 @@
 from aiida.backends.testbase import AiidaTestCase
 from aiida.common import exceptions
 from aiida.cmdline.commands import cmd_group
+from aiida.cmdline.utils.echo import ExitCode
 
 
 class TestVerdiGroup(AiidaTestCase):
@@ -156,7 +157,6 @@ def test_delete(self):
         self.assertEqual(group.count(), 2)
 
         result = self.cli_runner.invoke(cmd_group.group_delete, ['--force', 'group_test_delete_02'])
-        self.assertClickResultNoException(result)
 
         with self.assertRaises(exceptions.NotExistent):
             orm.load_group(label='group_test_delete_02')
@@ -265,7 +265,7 @@ def test_add_remove_nodes(self):
         result = self.cli_runner.invoke(cmd_group.group_remove_nodes, ['--force', '--group=dummygroup1', node_01.uuid])
         self.assertIsNone(result.exception, result.output)
 
-        # Check if node is added in group using group show command
+        # Check that the node is no longer in the group
         result = self.cli_runner.invoke(cmd_group.group_show, ['-r', 'dummygroup1'])
         self.assertClickResultNoException(result)
         self.assertNotIn('CalculationNode', result.output)
@@ -280,6 +280,35 @@ def test_add_remove_nodes(self):
         self.assertClickResultNoException(result)
         self.assertEqual(group.count(), 0)
 
+        # Try to remove node that isn't in the group
+        result = self.cli_runner.invoke(cmd_group.group_remove_nodes, ['--group=dummygroup1', node_01.uuid])
+        self.assertEqual(result.exit_code, ExitCode.CRITICAL)
+
+        # Try to remove no nodes nor clear the group
+        result = self.cli_runner.invoke(cmd_group.group_remove_nodes, ['--group=dummygroup1'])
+        self.assertEqual(result.exit_code, ExitCode.CRITICAL)
+
+        # Try to remove both nodes and clear the group
+        result = self.cli_runner.invoke(cmd_group.group_remove_nodes, ['--group=dummygroup1', '--clear', node_01.uuid])
+        self.assertEqual(result.exit_code, ExitCode.CRITICAL)
+
+        # Add a node with confirmation
+        result = self.cli_runner.invoke(cmd_group.group_add_nodes, ['--group=dummygroup1', node_01.uuid], input='y')
+        self.assertEqual(group.count(), 1)
+
+        # Try to remove two nodes, one that isn't in the group, but abort
+        result = self.cli_runner.invoke(
+            cmd_group.group_remove_nodes, ['--group=dummygroup1', node_01.uuid, node_02.uuid], input='N'
+        )
+        self.assertIn('Warning', result.output)
+        self.assertEqual(group.count(), 1)
+
+        # Try to clear all nodes from the group, but abort
+        result = self.cli_runner.invoke(cmd_group.group_remove_nodes, ['--group=dummygroup1', '--clear'], input='N')
+        self.assertIn('Are you sure you want to remove ALL', result.output)
+        self.assertIn('Aborted', result.output)
+        self.assertEqual(group.count(), 1)
+
     def test_copy_existing_group(self):
         """Test user is prompted to continue if destination group exists and is not empty"""
         source_label = 'source_copy_existing_group'

tests/cmdline/commands/test_process.py

@@ -64,6 +64,7 @@ def tearDown(self):
         os.kill(self.daemon.pid, signal.SIGTERM)
         super().tearDown()
 
+    @pytest.mark.skip(reason='fails to complete randomly (see issue #4731)')
     @pytest.mark.requires_rmq
     def test_pause_play_kill(self):
         """

tests/engine/test_work_chain.py

@@ -680,19 +680,22 @@ def do_run(self):
         run_and_check_success(MainWorkChain)
 
     def test_if_block_persistence(self):
-        """
+        """Test a reloaded `If` conditional can be resumed.
+
         This test was created to capture issue #902
         """
         runner = get_manager().get_runner()
         wc = IfTest()
         runner.schedule(wc)
 
         async def run_async(workchain):
+
+            # run the original workchain until paused
             await run_until_paused(workchain)
             self.assertTrue(workchain.ctx.s1)
             self.assertFalse(workchain.ctx.s2)
 
-            # Now bundle the thing
+            # Now bundle the workchain
             bundle = plumpy.Bundle(workchain)
             # Need to close the process before recreating a new instance
             workchain.close()
@@ -702,13 +705,20 @@ async def run_async(workchain):
             self.assertTrue(workchain2.ctx.s1)
             self.assertFalse(workchain2.ctx.s2)
 
+            # check bundling again creates the same saved state
             bundle2 = plumpy.Bundle(workchain2)
             self.assertDictEqual(bundle, bundle2)
 
-            workchain.play()
-            await workchain.future()
-            self.assertTrue(workchain.ctx.s1)
-            self.assertTrue(workchain.ctx.s2)
+            # run the loaded workchain to completion
+            runner.schedule(workchain2)
+            workchain2.play()
+            await workchain2.future()
+            self.assertTrue(workchain2.ctx.s1)
+            self.assertTrue(workchain2.ctx.s2)
+
+            # ensure the original paused workchain future is finalised
+            # to avoid warnings
+            workchain.future().set_result(None)
 
         runner.loop.run_until_complete(run_async(wc))