Modularize verdi profile delete (#2151)

Modularized the code to delete a profile by splitting it into functions that
have been moved to `aiida.manage.configuration.setup` module. Then, three new
options are added to `verdi profile delete`

 * `--include-db/--skip-db`
 * `--include-repository/--skip-repository`
 * `--include-config/--skip-config`

The unittest for `verdi profile delete` now makes use of `--skip-db` such that
the test no longer prompts for a sudo password locally. The full functionality
is tested on Travis in `.ci/test_profile.py`.

.ci/test_setup.py → .ci/test_profile.py

@@ -8,15 +8,21 @@
 # For further information please visit http://www.aiida.net               #
 ###########################################################################
 """
-Integration tests for setup and quicksetup
+Integration tests for setup, quicksetup, and delete
 
-These can not be added to the locally run test suite as long as that does
-not use a separate (temporary) configuration directory, it might overwrite
-user profiles and leave behind partial profiles. It also does not clean up
-the file system behind itself.
+These can not be added to test_profile.py in the locally run test suite as long as that does
+not use a separate (temporary) configuration directory:
+ * it might overwrite user profiles
+ * it might leave behind partial profiles
+ * it does not clean up the file system behind itself
 
-These problems could also be addressed in tearDown methods of the test cases instead.
-It has not been done due to time constraints yet.
+Possible ways of solving this problem:
+
+ * migrate all tests to the fixtures in aiida.utils.fixtures, which already provide this functionality
+ * implement the functionality in the Aiidatestcase
+ * implement the functionality specifically for the verdi profile tests (using setUp and tearDown methods)
+
+It has not been done yet due to time constraints.
 """
 from __future__ import division
 from __future__ import print_function
@@ -29,6 +35,7 @@
 from pgtest.pgtest import PGTest
 
 from aiida.backends import settings as backend_settings
+from aiida.backends.tests.utils.configuration import create_mock_profile, with_temporary_config_instance
 from aiida.cmdline.commands.cmd_setup import setup
 from aiida.cmdline.commands.cmd_quicksetup import quicksetup
 from aiida.manage.external.postgres import Postgres
@@ -41,23 +48,19 @@ def setUp(self):
         self.runner = CliRunner()
         self.backend = os.environ.get('TEST_AIIDA_BACKEND', 'django')
 
+    @with_temporary_config_instance
     def test_user_setup(self):
-        """
-        Test `verdi quicksetup` non-interactively
-        """
-        backend_settings.AIIDADB_PROFILE = None
+        """Test `verdi quicksetup` non-interactively."""
         result = self.runner.invoke(quicksetup, [
             '--backend={}'.format(self.backend), '--email=giuseppe.verdi@ope.ra', '--first-name=Giuseppe',
             '--last-name=Verdi', '--institution=Scala', '--db-name=aiida_giuseppe_{}'.format(
                 self.backend), '--repository=aiida_giuseppe_{}'.format(self.backend), 'giuseppe-{}'.format(self.backend)
         ])
         self.assertFalse(result.exception, msg=get_debug_msg(result))
 
+    @with_temporary_config_instance
     def test_postgres_failure(self):
-        """
-        Test `verdi quicksetup` non-interactively
-        """
-        backend_settings.AIIDADB_PROFILE = None
+        """Test `verdi quicksetup` non-interactively."""
         result = self.runner.invoke(
             quicksetup, [
                 '--backend={}'.format(self.backend), '--email=giuseppe2.verdi@ope.ra', '--first-name=Giuseppe',
@@ -135,6 +138,59 @@ def test_user_configure(self):
         self.assertFalse(result.exception, msg=get_debug_msg(result))
 
 
+class DeleteTestCase(unittest.TestCase):
+    """Test `verdi profile delete`."""
+
+    def setUp(self):
+        self.runner = CliRunner()
+        self.backend = os.environ.get('TEST_AIIDA_BACKEND', 'django')
+        self.profile_list = ['mock_profile1', 'mock_profile2', 'mock_profile3', 'mock_profile4']
+
+    def mock_profiles(self):
+        """Create mock profiles and a runner object to invoke the CLI commands.
+
+        Note: this cannot be done in the `setUp` or `setUpClass` methods, because the temporary configuration instance
+        is not generated until the test function is entered, which calls the `with_temporary_config_instance`
+        decorator.
+        """
+        from aiida.manage import get_config
+
+        config = get_config()
+
+        for profile_name in self.profile_list:
+            profile = create_mock_profile(profile_name)
+            config.add_profile(profile_name, profile)
+
+        config.set_default_profile(self.profile_list[0], overwrite=True).store()
+
+    @with_temporary_config_instance
+    def test_delete(self):
+        """Test for verdi profile delete command."""
+        from aiida.cmdline.commands.cmd_profile import profile_delete, profile_list
+
+        self.mock_profiles()
+
+        # Delete single profile
+        result = self.runner.invoke(profile_delete, ['--force', self.profile_list[1]])
+        self.assertIsNone(result.exception, result.output)
+
+        result = self.runner.invoke(profile_list)
+        self.assertIsNone(result.exception, result.output)
+
+        self.assertNotIn(self.profile_list[1], result.output)
+        self.assertIsNone(result.exception, result.output)
+
+        # Delete multiple profiles
+        result = self.runner.invoke(profile_delete, ['--force', self.profile_list[2], self.profile_list[3]])
+        self.assertIsNone(result.exception, result.output)
+
+        result = self.runner.invoke(profile_list)
+        self.assertIsNone(result.exception, result.output)
+        self.assertNotIn(self.profile_list[2], result.output)
+        self.assertNotIn(self.profile_list[3], result.output)
+        self.assertIsNone(result.exception, result.output)
+
+
 def get_debug_msg(result):
     msg = '{}\n---\nOutput:\n{}'
     return msg.format(result.exception, result.output)

.ci/test_script.sh

@@ -24,7 +24,7 @@ case "$TEST_TYPE" in
         coverage erase
 
         # Run preliminary tests
-        coverage run -a "${CI_DIR}/test_setup.py"
+        coverage run -a "${CI_DIR}/test_profile.py"
         coverage run -a "${CI_DIR}/test_fixtures.py"
         coverage run -a "${CI_DIR}/test_plugin_testcase.py"
 

aiida/backends/tests/cmdline/commands/test_profile.py

@@ -97,7 +97,7 @@ def test_setdefault(self):
 
     @with_temporary_config_instance
     def test_show(self):
-        """Test the `verdi profile show` command"""
+        """Test the `verdi profile show` command."""
         self.mock_profiles()
 
         config = get_config()
@@ -112,21 +112,16 @@ def test_show(self):
 
     @with_temporary_config_instance
     def test_delete(self):
-        """Test the `verdi profile delete` command."""
+        """Test the `verdi profile delete` command.
+
+        .. note:: we skip deleting the database as this might require sudo rights and this is tested in the CI tests
+            defined in the file `.ci/test_profile.py`
+        """
         self.mock_profiles()
 
-        result = self.cli_runner.invoke(cmd_profile.profile_delete, ['--force', self.profile_list[1]])
+        result = self.cli_runner.invoke(cmd_profile.profile_delete, ['--force', '--skip-db', self.profile_list[1]])
         self.assertClickSuccess(result)
 
         result = self.cli_runner.invoke(cmd_profile.profile_list)
         self.assertClickSuccess(result)
         self.assertNotIn(self.profile_list[1], result.output)
-
-        result = self.cli_runner.invoke(cmd_profile.profile_delete,
-                                        ['--force', self.profile_list[2], self.profile_list[3]])
-        self.assertClickSuccess(result)
-
-        result = self.cli_runner.invoke(cmd_profile.profile_list)
-        self.assertClickSuccess(result)
-        self.assertNotIn(self.profile_list[2], result.output)
-        self.assertNotIn(self.profile_list[3], result.output)

aiida/cmdline/commands/cmd_profile.py

@@ -16,11 +16,10 @@
 import tabulate
 
 from aiida.cmdline.commands.cmd_verdi import verdi
-from aiida.cmdline.utils import defaults, echo
 from aiida.cmdline.params import arguments, options
+from aiida.cmdline.utils import defaults, echo
 from aiida.common import exceptions
 from aiida.manage import get_config
-from aiida.manage.external.postgres import Postgres
 
 
 @verdi.group('profile')
@@ -69,74 +68,32 @@ def profile_setdefault(profile):
 
 
 @verdi_profile.command('delete')
-@options.FORCE(help='to skip any questions/warnings about loss of data')
-@arguments.PROFILES()
-def profile_delete(force, profiles):
+@options.FORCE(help='to skip questions and warnings about loss of data')
+@click.option(
+    '--include-config/--skip-config',
+    default=True,
+    show_default=True,
+    help='Include deletion of entry in configuration file.')
+@click.option(
+    '--include-db/--skip-db', default=True, show_default=True, help='Include deletion of associated database.')
+@click.option(
+    '--include-repository/--skip-repository',
+    default=True,
+    show_default=True,
+    help='Include deletion of associated file repository.')
+@arguments.PROFILES(required=True)
+def profile_delete(force, include_config, include_db, include_repository, profiles):
     """
-    Delete PROFILES separated by space from aiida config file
-    along with its associated database and repository.
+    Delete PROFILES (names, separated by spaces) from the aiida config file,
+    including the associated databases and file repositories.
     """
-    import os
-    from six.moves.urllib.parse import urlparse  # pylint: disable=import-error
-    import aiida.common.json as json
-
-    try:
-        config = get_config()
-    except (exceptions.MissingConfigurationError, exceptions.ConfigurationError) as exception:
-        echo.echo_critical(str(exception))
-
-    profile_names = [profile.name for profile in profiles]
-    users = [profile.dictionary.get('AIIDADB_USER', '') for profile in config.profiles]
-
-    for profile_name in profile_names:
-        try:
-            profile = config.get_profile(profile_name)
-        except exceptions.ProfileConfigurationError:
-            echo.echo_error("profile '{}' does not exist".format(profile_name))
-            continue
-
-        profile_dictionary = profile.dictionary
-
-        postgres = Postgres(port=profile_dictionary.get('AIIDADB_PORT'), interactive=True, quiet=False)
-        postgres.dbinfo["user"] = profile_dictionary.get('AIIDADB_USER')
-        postgres.dbinfo["host"] = profile_dictionary.get('AIIDADB_HOST')
-        postgres.determine_setup()
-
-        echo.echo(json.dumps(postgres.dbinfo, indent=4))
-
-        db_name = profile_dictionary.get('AIIDADB_NAME', '')
-        if not postgres.db_exists(db_name):
-            echo.echo_info("Associated database '{}' does not exist.".format(db_name))
-        elif force or click.confirm("Delete associated database '{}'?\n"
-                                    "WARNING: All data will be lost.".format(db_name)):
-            echo.echo_info("Deleting database '{}'.".format(db_name))
-            postgres.drop_db(db_name)
-
-        user = profile_dictionary.get('AIIDADB_USER', '')
-        if not postgres.dbuser_exists(user):
-            echo.echo_info("Associated database user '{}' does not exist.".format(user))
-        elif users.count(user) > 1:
-            echo.echo_info("Associated database user '{}' is used by other profiles "
-                           "and will not be deleted.".format(user))
-        elif force or click.confirm("Delete database user '{}'?".format(user)):
-            echo.echo_info("Deleting user '{}'.".format(user))
-            postgres.drop_dbuser(user)
-
-        repo_uri = profile_dictionary.get('AIIDADB_REPOSITORY_URI', '')
-        repo_path = urlparse(repo_uri).path
-        repo_path = os.path.expanduser(repo_path)
-        if not os.path.isabs(repo_path):
-            echo.echo_info("Associated file repository '{}' does not exist.".format(repo_path))
-        elif not os.path.isdir(repo_path):
-            echo.echo_info("Associated file repository '{}' is not a directory.".format(repo_path))
-        elif force or click.confirm("Delete associated file repository '{}'?\n"
-                                    "WARNING: All data will be lost.".format(repo_path)):
-            echo.echo_info("Deleting directory '{}'.".format(repo_path))
-            import shutil
-            shutil.rmtree(repo_path)
-
-        if force or click.confirm(
-                "Delete configuration for profile '{}'?\n"
-                "WARNING: Permanently removes profile from the list of AiiDA profiles.".format(profile_name)):
-            echo.echo_info("Deleting configuration for profile '{}'.".format(profile_name))
-            config.remove_profile(profile_name).store()
+    from aiida.manage.configuration.setup import delete_profile
+
+    for profile in profiles:
+        echo.echo_info("Deleting profile '{}'".format(profile.name))
+        delete_profile(
+            profile,
+            non_interactive=force,
+            include_db=include_db,
+            include_repository=include_repository,
+            include_config=include_config)