Fixtures: Change default use_subprocess=False for run_cli_command

The `run_cli_command` fixture was updated a few commits ago to allow
running the command as a subprocess instead of through the test runner.
The reason is that the test runner does not execute the exact same
pathway as when run as a subprocess and this was hiding subtle bugs.

Specifically, a bug in the `PsqlDosBackend` where changes made by a CLI
command were not automatically persisted to the database, was not caught
by the tests since when running in the test runner the assertions do not
explicitly check that changes are persisted, just that they are present,
even if just in memory.

Ideally, all tests are run with `use_subprocess=True`, which was set as
the default originally, but unfortunately this is quite a deal slower
than running with the test runner and would make the test suite almost
30 minutes slower. Instead, the tests that failed because of the storage
backend bug are now explicitly marked as using `use_subprocess=True` and
the default is changed. This now guarantees that at least these tests
that are known to depend on the subprocess pathway are properly run in
that way. This should provide greater coverage to prevent these types of
bugs in the future.

An exception is made for the tests of `verdi process list` and a number
of subcommands of `verdi data` (mostly the list command but also some of
the import and export commands. The reason is that all of the commands
failed for the same reason: the group that was created in the setup of
the test, was not persisted and so not found when the command was run
in a subprocess. Since these tests are numerous and take multiple
minutes to run, and the same behavior is already explicitly tested in
the `verdi group` tests, they are run with `use_subprocess=False` to
prevent the test suite from taking too much time.

.github/workflows/ci-code.yml

@@ -44,7 +44,7 @@ jobs:
     needs: [check-requirements]
 
     runs-on: ubuntu-latest
-    timeout-minutes: 35
+    timeout-minutes: 45
 
     strategy:
       fail-fast: false

tests/cmdline/commands/test_archive_create.py

@@ -46,7 +46,7 @@ def test_create_all(run_cli_command, tmp_path, aiida_localhost):
     filename_output = tmp_path / 'archive.aiida'
 
     options = ['--all', filename_output]
-    run_cli_command(cmd_archive.create, options)
+    run_cli_command(cmd_archive.create, options, use_subprocess=True)
     assert filename_output.is_file()
     assert ArchiveFormatSqlZip().read_version(filename_output) == ArchiveFormatSqlZip().latest_version
     with ArchiveFormatSqlZip().open(filename_output, 'r') as archive:

tests/cmdline/commands/test_archive_import.py

@@ -104,7 +104,7 @@ def test_import_make_new_group(run_cli_command, newest_archive):
 
     # Invoke `verdi import`, making sure there are no exceptions
     options = ['-G', group_label] + archives
-    run_cli_command(cmd_archive.import_archive, options)
+    run_cli_command(cmd_archive.import_archive, options, use_subprocess=True)
 
     # Make sure new Group was created
     (group, new_group) = Group.collection.get_or_create(group_label)

tests/cmdline/commands/test_code.py

@@ -123,7 +123,7 @@ def test_relabel_code_full_bad(run_cli_command, code):
 @pytest.mark.usefixtures('aiida_profile_clean')
 def test_code_delete_one_force(run_cli_command, code):
     """Test force code deletion."""
-    run_cli_command(cmd_code.delete, [str(code.pk), '--force'])
+    run_cli_command(cmd_code.delete, [str(code.pk), '--force'], use_subprocess=True)
 
     with pytest.raises(NotExistent):
         load_code('code')

tests/cmdline/commands/test_group.py

@@ -70,15 +70,15 @@ def test_help(self, run_cli_command):
 
     def test_create(self, run_cli_command):
         """Test `verdi group create` command."""
-        result = run_cli_command(cmd_group.group_create, ['dummygroup5'])
+        result = run_cli_command(cmd_group.group_create, ['dummygroup5'], use_subprocess=True)
 
         # check if newly added group in present in list
         result = run_cli_command(cmd_group.group_list)
         assert 'dummygroup5' in result.output
 
     def test_list(self, run_cli_command):
         """Test `verdi group list` command."""
-        result = run_cli_command(cmd_group.group_list)
+        result = run_cli_command(cmd_group.group_list, use_subprocess=True)
         for grp in ['dummygroup1', 'dummygroup2']:
             assert grp in result.output
 
@@ -87,23 +87,23 @@ def test_list_order(self, run_cli_command):
         orm.Group(label='agroup').store()
 
         options = []
-        result = run_cli_command(cmd_group.group_list, options, suppress_warnings=True)
+        result = run_cli_command(cmd_group.group_list, options, suppress_warnings=True, use_subprocess=True)
         group_ordering = [l.split()[1] for l in result.output.split('\n')[3:] if l]
         assert ['agroup', 'dummygroup1', 'dummygroup2', 'dummygroup3', 'dummygroup4'] == group_ordering
 
         options = ['--order-by', 'id']
-        result = run_cli_command(cmd_group.group_list, options, suppress_warnings=True)
+        result = run_cli_command(cmd_group.group_list, options, suppress_warnings=True, use_subprocess=True)
         group_ordering = [l.split()[1] for l in result.output.split('\n')[3:] if l]
         assert ['dummygroup1', 'dummygroup2', 'dummygroup3', 'dummygroup4', 'agroup'] == group_ordering
 
         options = ['--order-by', 'id', '--order-direction', 'desc']
-        result = run_cli_command(cmd_group.group_list, options, suppress_warnings=True)
+        result = run_cli_command(cmd_group.group_list, options, suppress_warnings=True, use_subprocess=True)
         group_ordering = [l.split()[1] for l in result.output.split('\n')[3:] if l]
         assert ['agroup', 'dummygroup4', 'dummygroup3', 'dummygroup2', 'dummygroup1'] == group_ordering
 
     def test_copy(self, run_cli_command):
         """Test `verdi group copy` command."""
-        result = run_cli_command(cmd_group.group_copy, ['dummygroup1', 'dummygroup2'])
+        result = run_cli_command(cmd_group.group_copy, ['dummygroup1', 'dummygroup2'], use_subprocess=True)
         assert 'Success' in result.output
 
     def test_delete(self, run_cli_command):
@@ -113,13 +113,13 @@ def test_delete(self, run_cli_command):
         orm.Group(label='group_test_delete_03').store()
 
         # dry run
-        result = run_cli_command(cmd_group.group_delete, ['--dry-run', 'group_test_delete_01'])
+        result = run_cli_command(cmd_group.group_delete, ['--dry-run', 'group_test_delete_01'], use_subprocess=True)
         orm.load_group(label='group_test_delete_01')
 
-        result = run_cli_command(cmd_group.group_delete, ['--force', 'group_test_delete_01'])
+        result = run_cli_command(cmd_group.group_delete, ['--force', 'group_test_delete_01'], use_subprocess=True)
 
         # Verify that removed group is not present in list
-        result = run_cli_command(cmd_group.group_list)
+        result = run_cli_command(cmd_group.group_list, use_subprocess=True)
         assert 'group_test_delete_01' not in result.output
 
         node_01 = orm.CalculationNode().store()
@@ -131,7 +131,7 @@ def test_delete(self, run_cli_command):
         group.add_nodes([node_01, node_02])
         assert group.count() == 2
 
-        result = run_cli_command(cmd_group.group_delete, ['--force', 'group_test_delete_02'])
+        result = run_cli_command(cmd_group.group_delete, ['--force', 'group_test_delete_02'], use_subprocess=True)
 
         with pytest.raises(exceptions.NotExistent):
             orm.load_group(label='group_test_delete_02')
@@ -143,7 +143,9 @@ def test_delete(self, run_cli_command):
         # delete the group and the nodes it contains
         group = orm.load_group(label='group_test_delete_03')
         group.add_nodes([node_01, node_02])
-        result = run_cli_command(cmd_group.group_delete, ['--force', '--delete-nodes', 'group_test_delete_03'])
+        result = run_cli_command(
+            cmd_group.group_delete, ['--force', '--delete-nodes', 'group_test_delete_03'], use_subprocess=True
+        )
 
         # check group and nodes no longer exist
         with pytest.raises(exceptions.NotExistent):
@@ -154,7 +156,7 @@ def test_delete(self, run_cli_command):
 
     def test_show(self, run_cli_command):
         """Test `verdi group show` command."""
-        result = run_cli_command(cmd_group.group_show, ['dummygroup1'])
+        result = run_cli_command(cmd_group.group_show, ['dummygroup1'], use_subprocess=True)
         for grpline in [
             'Group label', 'dummygroup1', 'Group type_string', 'core', 'Group description', '<no description>'
         ]:
@@ -168,20 +170,22 @@ def test_show_limit(self, run_cli_command):
         group.add_nodes(nodes)
 
         # Default should include all nodes in the output
-        result = run_cli_command(cmd_group.group_show, [label])
+        result = run_cli_command(cmd_group.group_show, [label], use_subprocess=True)
         for node in nodes:
             assert str(node.pk) in result.output
 
         # Repeat test with `limit=1`, use also the `--raw` option to only display nodes
-        result = run_cli_command(cmd_group.group_show, [label, '--limit', '1', '--raw'], suppress_warnings=True)
+        result = run_cli_command(
+            cmd_group.group_show, [label, '--limit', '1', '--raw'], suppress_warnings=True, use_subprocess=True
+        )
 
         # The current `verdi group show` does not support ordering so we cannot rely on that for now to test if only
         # one of the nodes is shown
         assert len(result.output.strip().split('\n')) == 1
         assert str(nodes[0].pk) in result.output or str(nodes[1].pk) in result.output
 
         # Repeat test with `limit=1` but without the `--raw` flag as it has a different code path that is affected
-        result = run_cli_command(cmd_group.group_show, [label, '--limit', '1'])
+        result = run_cli_command(cmd_group.group_show, [label, '--limit', '1'], use_subprocess=True)
 
         # Check that one, and only one pk appears in the output
         assert str(nodes[0].pk) in result.output or str(nodes[1].pk) in result.output
@@ -217,19 +221,23 @@ def test_add_remove_nodes(self, run_cli_command):
         node_02 = orm.CalculationNode().store()
         node_03 = orm.CalculationNode().store()
 
-        result = run_cli_command(cmd_group.group_add_nodes, ['--force', '--group=dummygroup1', node_01.uuid])
+        result = run_cli_command(
+            cmd_group.group_add_nodes, ['--force', '--group=dummygroup1', node_01.uuid], use_subprocess=True
+        )
 
         # Check if node is added in group using group show command
-        result = run_cli_command(cmd_group.group_show, ['dummygroup1'])
+        result = run_cli_command(cmd_group.group_show, ['dummygroup1'], use_subprocess=True)
 
         assert 'CalculationNode' in result.output
         assert str(node_01.pk) in result.output
 
         # Remove same node
-        result = run_cli_command(cmd_group.group_remove_nodes, ['--force', '--group=dummygroup1', node_01.uuid])
+        result = run_cli_command(
+            cmd_group.group_remove_nodes, ['--force', '--group=dummygroup1', node_01.uuid], use_subprocess=True
+        )
 
         # Check that the node is no longer in the group
-        result = run_cli_command(cmd_group.group_show, ['-r', 'dummygroup1'])
+        result = run_cli_command(cmd_group.group_show, ['-r', 'dummygroup1'], use_subprocess=True)
 
         assert 'CalculationNode' not in result.output
         assert str(node_01.pk) not in result.output
@@ -239,16 +247,22 @@ def test_add_remove_nodes(self, run_cli_command):
         group.add_nodes([node_01, node_02, node_03])
         assert group.count() == 3
 
-        result = run_cli_command(cmd_group.group_remove_nodes, ['--force', '--clear', '--group=dummygroup1'])
+        result = run_cli_command(
+            cmd_group.group_remove_nodes, ['--force', '--clear', '--group=dummygroup1'], use_subprocess=True
+        )
 
         assert group.count() == 0
 
         # Try to remove node that isn't in the group
-        result = run_cli_command(cmd_group.group_remove_nodes, ['--group=dummygroup1', node_01.uuid], raises=True)
+        result = run_cli_command(
+            cmd_group.group_remove_nodes, ['--group=dummygroup1', node_01.uuid], raises=True, use_subprocess=True
+        )
         assert result.exit_code == ExitCode.CRITICAL
 
         # Try to remove no nodes nor clear the group
-        result = run_cli_command(cmd_group.group_remove_nodes, ['--group=dummygroup1'], raises=True)
+        result = run_cli_command(
+            cmd_group.group_remove_nodes, ['--group=dummygroup1'], raises=True, use_subprocess=True
+        )
         assert result.exit_code == ExitCode.CRITICAL
 
         # Try to remove both nodes and clear the group
@@ -258,21 +272,27 @@ def test_add_remove_nodes(self, run_cli_command):
         assert result.exit_code == ExitCode.CRITICAL
 
         # Add a node with confirmation
-        result = run_cli_command(cmd_group.group_add_nodes, ['--group=dummygroup1', node_01.uuid], user_input='y')
+        result = run_cli_command(
+            cmd_group.group_add_nodes, ['--group=dummygroup1', node_01.uuid], user_input='y', use_subprocess=True
+        )
         assert group.count() == 1
 
         # Try to remove two nodes, one that isn't in the group, but abort
         result = run_cli_command(
             cmd_group.group_remove_nodes, ['--group=dummygroup1', node_01.uuid, node_02.uuid],
             user_input='N',
-            raises=True
+            raises=True,
+            use_subprocess=True
         )
         assert 'Warning' in result.output
         assert group.count() == 1
 
         # Try to clear all nodes from the group, but abort
         result = run_cli_command(
-            cmd_group.group_remove_nodes, ['--group=dummygroup1', '--clear'], user_input='N', raises=True
+            cmd_group.group_remove_nodes, ['--group=dummygroup1', '--clear'],
+            user_input='N',
+            raises=True,
+            use_subprocess=True
         )
         assert 'Are you sure you want to remove ALL' in result.output
         assert 'Aborted' in result.output
@@ -292,57 +312,67 @@ def test_move_nodes(self, run_cli_command):
         # Moving the nodes to the same group
         result = run_cli_command(
             cmd_group.group_move_nodes, ['-s', 'dummygroup1', '-t', 'dummygroup1', node_01.uuid, node_02.uuid],
-            raises=True
+            raises=True,
+            use_subprocess=True
         )
         assert 'Source and target group are the same:' in result.output
 
         # Not specifying NODES or `--all`
-        result = run_cli_command(cmd_group.group_move_nodes, ['-s', 'dummygroup1', '-t', 'dummygroup2'], raises=True)
+        result = run_cli_command(
+            cmd_group.group_move_nodes, ['-s', 'dummygroup1', '-t', 'dummygroup2'], raises=True, use_subprocess=True
+        )
         assert 'Neither NODES or the `-a, --all` option was specified.' in result.output
 
         # Moving the nodes from the empty group
         result = run_cli_command(
             cmd_group.group_move_nodes, ['-s', 'dummygroup2', '-t', 'dummygroup1', node_01.uuid, node_02.uuid],
-            raises=True
+            raises=True,
+            use_subprocess=True
         )
         assert 'None of the specified nodes are in' in result.output
 
         # Move two nodes to the second dummy group, but specify a missing uuid
         result = run_cli_command(
             cmd_group.group_move_nodes, ['-s', 'dummygroup1', '-t', 'dummygroup2', node_01.uuid, node_03.uuid],
-            raises=True
+            raises=True,
+            use_subprocess=True
         )
         assert f'1 nodes with PK {{{node_03.pk}}} are not in' in result.output
         # Check that the node that is present is actually moved
         result = run_cli_command(
-            cmd_group.group_move_nodes,
-            ['-f', '-s', 'dummygroup1', '-t', 'dummygroup2', node_01.uuid, node_03.uuid],
+            cmd_group.group_move_nodes, ['-f', '-s', 'dummygroup1', '-t', 'dummygroup2', node_01.uuid, node_03.uuid],
+            use_subprocess=True
         )
         assert node_01 not in group1.nodes
         assert node_01 in group2.nodes
 
         # Add the first node back to the first group, and try to move it from the second one
         group1.add_nodes(node_01)
         result = run_cli_command(
-            cmd_group.group_move_nodes, ['-s', 'dummygroup2', '-t', 'dummygroup1', node_01.uuid], raises=True
+            cmd_group.group_move_nodes, ['-s', 'dummygroup2', '-t', 'dummygroup1', node_01.uuid],
+            raises=True,
+            use_subprocess=True
         )
         assert f'1 nodes with PK {{{node_01.pk}}} are already' in result.output
         # Check that it is still removed from the second group
         result = run_cli_command(
-            cmd_group.group_move_nodes,
-            ['-f', '-s', 'dummygroup2', '-t', 'dummygroup1', node_01.uuid],
+            cmd_group.group_move_nodes, ['-f', '-s', 'dummygroup2', '-t', 'dummygroup1', node_01.uuid],
+            use_subprocess=True
         )
         assert node_01 not in group2.nodes
 
         # Force move the two nodes to the second dummy group
         result = run_cli_command(
-            cmd_group.group_move_nodes, ['-f', '-s', 'dummygroup1', '-t', 'dummygroup2', node_01.uuid, node_02.uuid]
+            cmd_group.group_move_nodes, ['-f', '-s', 'dummygroup1', '-t', 'dummygroup2', node_01.uuid, node_02.uuid],
+            use_subprocess=True
         )
         assert node_01 in group2.nodes
         assert node_02 in group2.nodes
 
         # Force move all nodes back to the first dummy group
-        result = run_cli_command(cmd_group.group_move_nodes, ['-f', '-s', 'dummygroup2', '-t', 'dummygroup1', '--all'])
+        result = run_cli_command(
+            cmd_group.group_move_nodes, ['-f', '-s', 'dummygroup2', '-t', 'dummygroup1', '--all'], use_subprocess=True
+        )
         assert node_01 not in group2.nodes
         assert node_02 not in group2.nodes
         assert node_01 in group1.nodes
@@ -362,7 +392,7 @@ def test_copy_existing_group(self, run_cli_command):
 
         # Copy using `verdi group copy` - making sure all is successful
         options = [source_label, dest_label]
-        result = run_cli_command(cmd_group.group_copy, options)
+        result = run_cli_command(cmd_group.group_copy, options, use_subprocess=True)
         assert f'Success: Nodes copied from {source_group} to {source_group.__class__.__name__}<{dest_label}>.' in \
             result.output, result.exception
 
@@ -373,7 +403,7 @@ def test_copy_existing_group(self, run_cli_command):
         assert nodes_source_group == nodes_dest_group
 
         # Copy again, making sure an abort error is raised, since no user input can be made and default is abort
-        result = run_cli_command(cmd_group.group_copy, options, raises=True)
+        result = run_cli_command(cmd_group.group_copy, options, raises=True, use_subprocess=True)
         assert f'Warning: Destination {dest_group} already exists and is not empty.' in result.output, result.exception
 
         # Check destination group is unchanged

tests/cmdline/commands/test_node.py

@@ -71,23 +71,23 @@ def test_node_show(self, run_cli_command):
         node = orm.Data().store()
         node.label = 'SOMELABEL'
         options = [str(node.pk)]
-        result = run_cli_command(cmd_node.node_show, options)
+        result = run_cli_command(cmd_node.node_show, options, use_subprocess=True)
 
         # Let's check some content in the output. At least the UUID and the label should be in there
         assert node.label in result.output
         assert node.uuid in result.output
 
         # Let's now test the '--print-groups' option
         options.append('--print-groups')
-        result = run_cli_command(cmd_node.node_show, options)
+        result = run_cli_command(cmd_node.node_show, options, use_subprocess=True)
         # I don't check the list of groups - it might be in an autogroup
 
         # Let's create a group and put the node in there
         group_name = 'SOMEGROUPNAME'
         group = orm.Group(group_name).store()
         group.add_nodes(node)
 
-        result = run_cli_command(cmd_node.node_show, options)
+        result = run_cli_command(cmd_node.node_show, options, use_subprocess=True)
 
         # Now the group should be in there
         assert group_name in result.output
@@ -591,10 +591,10 @@ def test_node_delete_basics(run_cli_command, options):
     node = orm.Data().store()
     pk = node.pk
 
-    run_cli_command(cmd_node.node_delete, options + [str(pk), '--dry-run'])
+    run_cli_command(cmd_node.node_delete, options + [str(pk), '--dry-run'], use_subprocess=True)
 
     # To delete the created node
-    run_cli_command(cmd_node.node_delete, [str(pk), '--force'])
+    run_cli_command(cmd_node.node_delete, [str(pk), '--force'], use_subprocess=True)
 
     with pytest.raises(NotExistent):
         orm.load_node(pk)