From 041ac0abf7a88efeeb35f3520fe985c71fe5b2d0 Mon Sep 17 00:00:00 2001 From: dbeltran Date: Wed, 6 Nov 2024 13:55:23 +0100 Subject: [PATCH 01/12] fixes #1317, added pytest, docstrings, path lib usage and refactored into multiple functions --- autosubmit/autosubmit.py | 239 ++++++++++++++++++++------------- test/unit/test_expid_pytest.py | 148 ++++++++++++++++++++ test/unit/utils/common.py | 13 +- 3 files changed, 304 insertions(+), 96 deletions(-) create mode 100644 test/unit/test_expid_pytest.py diff --git a/autosubmit/autosubmit.py b/autosubmit/autosubmit.py index 797929a5f..e53a157b3 100644 --- a/autosubmit/autosubmit.py +++ b/autosubmit/autosubmit.py @@ -1003,95 +1003,150 @@ class Autosubmit: return owner, eadmin, current_owner @staticmethod - def _delete_expid(expid_delete, force=False): + def _delete_expid(expid_delete: str, force: bool = False) -> bool: """ - Removes an experiment from path and database - If current user is eadmin and -f has been sent, it deletes regardless - of experiment owner + Removes an experiment from the path and database. + If the current user is eadmin and the -f flag has been sent, it deletes regardless of experiment owner. + :param expid_delete: Identifier of the experiment to delete. :type expid_delete: str - :param expid_delete: identifier of the experiment to delete - :type force: boolean - :param force: True if the force flag has been sent - :return: True if successfully deleted, False otherwise - :rtype: boolean + :param force: If True, does not ask for confirmation. + :type force: bool + + :returns: True if successfully deleted, False otherwise. + :rtype: bool + + :raises AutosubmitCritical: If the experiment does not exist or if there are insufficient permissions. """ - message = "The {0} experiment was removed from the local disk and from the database.".format(expid_delete) - message += " Note that this action does not delete any data written by the experiment.\n" - message += "Complete list of files/directories deleted:\n" - for root, dirs, files in os.walk(os.path.join(BasicConfig.LOCAL_ROOT_DIR, expid_delete)): - for dir_ in dirs: - message += os.path.join(root, dir_) + "\n" - message += os.path.join(BasicConfig.LOCAL_ROOT_DIR, BasicConfig.STRUCTURES_DIR, - "structure_{0}.db".format(expid_delete)) + "\n" - message += os.path.join(BasicConfig.LOCAL_ROOT_DIR, BasicConfig.JOBDATA_DIR, - "job_data_{0}.db".format(expid_delete)) + "\n" - owner, eadmin, current_owner = Autosubmit._check_ownership(expid_delete) - if expid_delete == '' or expid_delete is None and not os.path.exists( - os.path.join(str(BasicConfig.LOCAL_ROOT_DIR), str(expid_delete))): + experiment_path = Path(f"{BasicConfig.LOCAL_ROOT_DIR}/{expid_delete}") + structure_db_path = Path(f"{BasicConfig.STRUCTURES_DIR}/structure_{expid_delete}.db") + job_data_db_path = Path(f"{BasicConfig.JOBDATA_DIR}/job_data_{expid_delete}") + + if not experiment_path.exists(): Log.printlog("Experiment directory does not exist.", Log.WARNING) + return False + + owner, eadmin, _ = Autosubmit._check_ownership(expid_delete) + if not (owner or (force and eadmin)): + Autosubmit._raise_permission_error(eadmin, expid_delete) + + message = Autosubmit._generate_deletion_message(expid_delete, experiment_path, structure_db_path, + job_data_db_path) + error_message = Autosubmit._perform_deletion(experiment_path, structure_db_path, job_data_db_path, expid_delete) + + if not error_message: + Log.printlog(message, Log.RESULT) + else: + Log.printlog(error_message, Log.ERROR) + raise AutosubmitError( + "Some experiment files weren't correctly deleted\nPlease if the trace shows DATABASE IS LOCKED, report it to git\nIf there are I/O issues, wait until they're solved and then use this command again.\n", + error_message, 6004 + ) + + return not bool(error_message) # if there is a non-empty error, return False + + @staticmethod + def _raise_permission_error(eadmin: bool, expid_delete: str) -> None: + """ + Raise a permission error if the current user is not allowed to delete the experiment. + + :param eadmin: Indicates if the current user is an eadmin. + :type eadmin: bool + :param expid_delete: Identifier of the experiment to delete. + :type expid_delete: str + + :raises AutosubmitCritical: If the user does not have permission to delete the experiment. + """ + if not eadmin: + raise AutosubmitCritical( + f"Detected Eadmin user however, -f flag is not found. {expid_delete} cannot be deleted!", 7012) else: - # Deletion workflow continues as usual, a disjunction is included for the case when - # force is sent, and user is eadmin - error_message = "" + raise AutosubmitCritical( + f"Current user is not the owner of the experiment. {expid_delete} cannot be deleted!", 7012) + + @staticmethod + def _generate_deletion_message(expid_delete: str, experiment_path: Path, structure_db_path: Path, + job_data_db_path: Path) -> str: + """ + Generate a message detailing what is being deleted from an experiment. + + :param expid_delete: Identifier of the experiment to delete. + :type expid_delete: str + :param experiment_path: Path to the experiment directory. + :type experiment_path: Path + :param structure_db_path: Path to the structure database file. + :type structure_db_path: Path + :param job_data_db_path: Path to the job data database file. + :type job_data_db_path: Path + + :return: A message detailing the deletion of the experiment. + :rtype: str + """ + message_parts = [ + f"The {expid_delete} experiment was removed from the local disk and from the database.\n", + "Note that this action does not delete any data written by the experiment.\n", + "Complete list of files/directories deleted:\n" + ] + message_parts.extend(f"{path}\n" for path in experiment_path.rglob('*')) + message_parts.append(f"{structure_db_path}\n") + message_parts.append(f"{job_data_db_path}.db\n") + message_parts.append(f"{job_data_db_path}.sql\n") + message = '\n'.join(message_parts) + return message + + @staticmethod + def _perform_deletion(experiment_path: Path, structure_db_path: Path, job_data_db_path: Path, + expid_delete: str) -> str: + """ + Perform the deletion of an experiment, including its directory, structure database, and job data database. + + :param experiment_path: Path to the experiment directory. + :type experiment_path: Path + :param structure_db_path: Path to the structure database file. + :type structure_db_path: Path + :param job_data_db_path: Path to the job data database file. + :type job_data_db_path: Path + :param expid_delete: Identifier of the experiment to delete. + :type expid_delete: str + :return: An error message if any errors occurred during deletion, otherwise an empty string. + :rtype: str + """ + error_message = "" + try: + Log.info("Deleting experiment from database...") try: - if owner or (force and eadmin): - if force and eadmin: - if current_owner: - Log.info(f"Preparing deletion of experiment {expid_delete} as eadmin. Current owner: {current_owner}") - else: - Log.info(f"Preparing deletion of experiment {expid_delete} as eadmin. Current owner: Unknown") - try: - Log.info("Deleting experiment from database...") - try: - ret = delete_experiment(expid_delete) - if ret: - Log.result("Experiment {0} deleted".format(expid_delete)) - except BaseException as e: - error_message += 'Can not delete experiment entry: {0}\n'.format(str(e)) - Log.info("Removing experiment directory...") - try: - shutil.rmtree(os.path.join(BasicConfig.LOCAL_ROOT_DIR, expid_delete)) - except BaseException as e: - error_message += 'Can not delete directory: {0}\n'.format(str(e)) - try: - Log.info("Removing Structure db...") - structures_path = os.path.join(BasicConfig.LOCAL_ROOT_DIR, BasicConfig.STRUCTURES_DIR, - "structure_{0}.db".format(expid_delete)) - if os.path.exists(structures_path): - os.remove(structures_path) - except BaseException as e: - error_message += 'Can not delete structure: {0}\n'.format(str(e)) - try: - Log.info("Removing job_data db...") - job_data_path = os.path.join(BasicConfig.LOCAL_ROOT_DIR, BasicConfig.JOBDATA_DIR, - "job_data_{0}.db".format(expid_delete)) - if os.path.exists(job_data_path): - os.remove(job_data_path) - except BaseException as e: - error_message += 'Can not delete job_data: {0}\n'.format(str(e)) - except OSError as e: - error_message += 'Can not delete directory: {0}\n'.format(str(e)) - else: - if not eadmin: - raise AutosubmitCritical( - 'Detected Eadmin user however, -f flag is not found. {0} can not be deleted!'.format( - expid_delete), 7012) - else: - raise AutosubmitCritical( - 'Current user is not the owner of the experiment. {0} can not be deleted!'.format( - expid_delete), 7012) - if error_message == "": - Log.printlog(message, Log.RESULT) - else: - Log.printlog(error_message, Log.ERROR) - except Exception as e: - # Avoid calling Log at this point since it is possible that tmp folder is already deleted. - error_message += "Couldn't delete the experiment".format(str(e)) - if error_message != "": - raise AutosubmitError( - "Some experiment files weren't correctly deleted\nPlease if the trace shows DATABASE IS LOCKED, report it to git\nIf there are I/O issues, wait until they're solved and then use this command again.\n", - error_message, 6004) + ret = delete_experiment(expid_delete) + if ret: + Log.result(f"Experiment {expid_delete} deleted") + except BaseException as e: + error_message += f"Cannot delete experiment entry: {e}\n" + + Log.info("Removing experiment directory...") + try: + shutil.rmtree(experiment_path) + except BaseException as e: + error_message += f"Cannot delete directory: {e}\n" + + Log.info("Removing Structure db...") + try: + if structure_db_path.exists(): + os.remove(structure_db_path) + except BaseException as e: + error_message += f"Cannot delete structure: {e}\n" + + Log.info("Removing job_data db...") + try: + db_path = job_data_db_path.with_suffix(".db") + sql_path = job_data_db_path.with_suffix(".sql") + if db_path.exists(): + os.remove(db_path) + if sql_path.exists(): + os.remove(sql_path) + except BaseException as e: + error_message += f"Cannot delete job_data: {e}\n" + except Exception as e: + error_message += f"Couldn't delete the experiment: {e}" + return error_message @staticmethod def copy_as_config(exp_id,copy_id): @@ -1376,22 +1431,24 @@ class Autosubmit: return exp_id @staticmethod - def delete(expid, force): + def delete(expid: str, force: bool) -> bool: """ - Deletes and experiment from database and experiment's folder + Deletes an experiment from the database, the experiment's folder database entry and all the related metadata files. - :type force: bool + :param expid: Identifier of the experiment to delete. :type expid: str - :param expid: identifier of the experiment to delete - :param force: if True, does not ask for confirmation + :param force: If True, does not ask for confirmation. + :type force: bool - :returns: True if successful, False if not + :returns: True if successful, False otherwise. :rtype: bool - """ - if os.path.exists(os.path.join(BasicConfig.LOCAL_ROOT_DIR, expid)): - if force or Autosubmit._user_yes_no_query("Do you want to delete " + expid + " ?"): + :raises AutosubmitCritical: If the experiment does not exist or if there are insufficient permissions. + """ + experiment_path = Path(f"{BasicConfig.LOCAL_ROOT_DIR}/{expid}") + if experiment_path.exists(): + if force or Autosubmit._user_yes_no_query(f"Do you want to delete {expid} ?"): Log.debug('Enter Autosubmit._delete_expid {0}', expid) try: return Autosubmit._delete_expid(expid, force) diff --git a/test/unit/test_expid_pytest.py b/test/unit/test_expid_pytest.py new file mode 100644 index 000000000..2fcb1f174 --- /dev/null +++ b/test/unit/test_expid_pytest.py @@ -0,0 +1,148 @@ +import pytest +from pathlib import Path +from autosubmit.autosubmit import Autosubmit +import os +import pwd +import sqlite3 + +from test.unit.utils.common import create_database, init_expid + + +def _get_script_files_path() -> Path: + return Path(__file__).resolve().parent / 'files' + + +@pytest.fixture +def create_autosubmit_tmpdir(tmpdir_factory): + folder = tmpdir_factory.mktemp('autosubmit_tests') + Path(folder).joinpath('scratch').mkdir() + file_stat = os.stat(f"{folder.strpath}") + file_owner_id = file_stat.st_uid + file_owner = pwd.getpwuid(file_owner_id).pw_name + folder.owner = file_owner + + # Write an autosubmitrc file in the temporary directory + autosubmitrc = folder.join('autosubmitrc') + autosubmitrc.write(f''' +[database] +path = {folder} +filename = tests.db + +[local] +path = {folder} + +[globallogs] +path = {folder}/globallogs + +[structures] +path = {folder}/metadata/structures + +[historicdb] +path = {folder}/metadata/database + +[historiclog] +path = {folder}/metadata/logs + +[defaultstats] +path = {folder} + +''') + os.environ['AUTOSUBMIT_CONFIGURATION'] = str(folder.join('autosubmitrc')) + create_database(str(folder.join('autosubmitrc'))) + Path(folder).joinpath('metadata').mkdir() + Path(folder).joinpath('metadata/structures').mkdir() + Path(folder).joinpath('metadata/database').mkdir() + Path(folder).joinpath('metadata/logs').mkdir() + assert "tests.db" in [Path(f).name for f in folder.listdir()] + return folder + + +@pytest.fixture +def generate_new_experiment(create_autosubmit_tmpdir, request): + test_type = request.param + # Setup code that depends on the expid parameter + expid = init_expid(os.environ["AUTOSUBMIT_CONFIGURATION"], platform='local', expid=None, create=True, test_type=test_type) + yield expid + + +@pytest.fixture +def setup_experiment_yamlfiles(generate_new_experiment, create_autosubmit_tmpdir): + expid = generate_new_experiment + # touch as_misc + platforms_path = Path(f"{create_autosubmit_tmpdir.strpath}/{expid}/conf/platforms_{expid}.yml") + jobs_path = Path(f"{create_autosubmit_tmpdir.strpath}/{expid}/conf/jobs_{expid}.yml") + # Add each platform to test + with platforms_path.open('w') as f: + f.write(f""" +PLATFORMS: + pytest-ps: + type: ps + host: 127.0.0.1 + user: {create_autosubmit_tmpdir.owner} + project: whatever + scratch_dir: {create_autosubmit_tmpdir}/scratch + DISABLE_RECOVERY_THREADS: True + """) + # add a job of each platform type + with jobs_path.open('w') as f: + f.write(""" +JOBS: + debug: + script: echo "Hello world" + running: once +EXPERIMENT: + DATELIST: '20000101' + MEMBERS: fc0 + CHUNKSIZEUNIT: month + CHUNKSIZE: '1' + NUMCHUNKS: '1' + CHUNKINI: '' + CALENDAR: standard + """) + + expid_dir = Path(f"{create_autosubmit_tmpdir.strpath}/scratch/whatever/{create_autosubmit_tmpdir.owner}/{expid}") + dummy_dir = Path(f"{create_autosubmit_tmpdir.strpath}/scratch/whatever/{create_autosubmit_tmpdir.owner}/{expid}/dummy_dir") + real_data = Path(f"{create_autosubmit_tmpdir.strpath}/scratch/whatever/{create_autosubmit_tmpdir.owner}/{expid}/real_data") + # write some dummy data inside scratch dir + expid_dir.mkdir(parents=True, exist_ok=True) + dummy_dir.mkdir(parents=True, exist_ok=True) + real_data.mkdir(parents=True, exist_ok=True) + + with open(dummy_dir.joinpath('dummy_file'), 'w') as f: + f.write('dummy data') + real_data.joinpath('dummy_symlink').symlink_to(dummy_dir / 'dummy_file') + yield expid + + +@pytest.mark.parametrize("generate_new_experiment", ['test', 'normal', 'operational'], indirect=True) +def test_expid_generated_correctly(create_autosubmit_tmpdir, generate_new_experiment, setup_experiment_yamlfiles): + expid = generate_new_experiment + print(f"Running test for {expid}") + Autosubmit.inspect(expid=f'{expid}', check_wrapper=True, force=True, lst=None, filter_chunks=None, filter_status=None, filter_section=None) + assert expid in ['t000', 'a000', 'o000'] + assert f"{expid}_DEBUG.cmd" in [Path(f).name for f in Path(f"{create_autosubmit_tmpdir.strpath}/{expid}/tmp").iterdir()] + # Consult if the expid is in the database + db_path = Path(f"{create_autosubmit_tmpdir.strpath}/tests.db") + conn = sqlite3.connect(db_path) + cursor = conn.cursor() + cursor.execute(f"SELECT name FROM experiment WHERE name='{expid}'") + assert cursor.fetchone() is not None + cursor.close() + +@pytest.mark.parametrize("generate_new_experiment", ['test', 'normal', 'operational'], indirect=True) +def test_delete_experiment(create_autosubmit_tmpdir, generate_new_experiment, setup_experiment_yamlfiles): + expid = generate_new_experiment + print(f"Running test for {expid}") + Autosubmit.delete(expid=f'{expid}', force=True) + assert all(expid not in Path(f).name for f in Path(f"{create_autosubmit_tmpdir.strpath}").iterdir()) + assert all(expid not in Path(f).name for f in Path(f"{create_autosubmit_tmpdir.strpath}/metadata/database").iterdir()) + assert all(expid not in Path(f).name for f in Path(f"{create_autosubmit_tmpdir.strpath}/metadata/logs").iterdir()) + assert all(expid not in Path(f).name for f in Path(f"{create_autosubmit_tmpdir.strpath}/metadata/structures").iterdir()) + # Consult if the expid is not in the database + db_path = Path(f"{create_autosubmit_tmpdir.strpath}/tests.db") + conn = sqlite3.connect(db_path) + cursor = conn.cursor() + cursor.execute(f"SELECT name FROM experiment WHERE name='{expid}'") + assert cursor.fetchone() is None + cursor.close() + diff --git a/test/unit/utils/common.py b/test/unit/utils/common.py index 50f613c34..88215926d 100644 --- a/test/unit/utils/common.py +++ b/test/unit/utils/common.py @@ -6,13 +6,16 @@ def create_database(envirom): BasicConfig.read() Autosubmit.install() -def init_expid(envirom, platform="local", expid=None, create=True): + +def init_expid(envirom, platform="local", expid=None, create=True, test_type="normal"): os.environ['AUTOSUBMIT_CONFIGURATION'] = envirom if not expid: - expid = Autosubmit.expid("pytest", hpc=platform, copy_id='', dummy=True, minimal_configuration=False, git_repo="", git_branch="", git_as_conf="", operational=False, testcase = True, use_local_minimal=False) + if test_type == "normal": + expid = Autosubmit.expid("pytest", hpc=platform, copy_id='', dummy=True, minimal_configuration=False, git_repo="", git_branch="", git_as_conf="", operational=False, testcase = False, use_local_minimal=False) + elif test_type == "test": + expid = Autosubmit.expid("pytest", hpc=platform, copy_id='', dummy=True, minimal_configuration=False, git_repo="", git_branch="", git_as_conf="", operational=False, testcase = True, use_local_minimal=False) + elif test_type == "operational": + expid = Autosubmit.expid("pytest", hpc=platform, copy_id='', dummy=True, minimal_configuration=False, git_repo="", git_branch="", git_as_conf="", operational=True, testcase = False, use_local_minimal=False) if create: Autosubmit.create(expid, True,False, force=True) return expid - - - -- GitLab From 8b856d1a10f791cf26d9bd987bf2f9be90173fb8 Mon Sep 17 00:00:00 2001 From: dbeltran Date: Wed, 6 Nov 2024 14:00:51 +0100 Subject: [PATCH 02/12] update other test that shares the function --- test/unit/test_scheduler_general.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/test_scheduler_general.py b/test/unit/test_scheduler_general.py index 39215d6d8..0f7819d9e 100644 --- a/test/unit/test_scheduler_general.py +++ b/test/unit/test_scheduler_general.py @@ -50,7 +50,7 @@ path = {folder} os.environ['AUTOSUBMIT_CONFIGURATION'] = str(folder.join('autosubmitrc')) create_database(str(folder.join('autosubmitrc'))) assert "tests.db" in [Path(f).name for f in folder.listdir()] - init_expid(str(folder.join('autosubmitrc')), platform='local', create=False) + init_expid(str(folder.join('autosubmitrc')), platform='local', create=False, test_type='test') assert "t000" in [Path(f).name for f in folder.listdir()] return folder -- GitLab From cd5c0a3641011d73dc2580e36a3589774dc37a2b Mon Sep 17 00:00:00 2001 From: dbeltran Date: Wed, 6 Nov 2024 14:05:47 +0100 Subject: [PATCH 03/12] improve coverage --- test/unit/test_expid_pytest.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test/unit/test_expid_pytest.py b/test/unit/test_expid_pytest.py index 2fcb1f174..b6bcb315c 100644 --- a/test/unit/test_expid_pytest.py +++ b/test/unit/test_expid_pytest.py @@ -5,6 +5,7 @@ import os import pwd import sqlite3 +from log.log import AutosubmitCritical from test.unit.utils.common import create_database, init_expid @@ -129,6 +130,7 @@ def test_expid_generated_correctly(create_autosubmit_tmpdir, generate_new_experi assert cursor.fetchone() is not None cursor.close() + @pytest.mark.parametrize("generate_new_experiment", ['test', 'normal', 'operational'], indirect=True) def test_delete_experiment(create_autosubmit_tmpdir, generate_new_experiment, setup_experiment_yamlfiles): expid = generate_new_experiment @@ -145,4 +147,7 @@ def test_delete_experiment(create_autosubmit_tmpdir, generate_new_experiment, se cursor.execute(f"SELECT name FROM experiment WHERE name='{expid}'") assert cursor.fetchone() is None cursor.close() + # Test doesn't exist + with pytest.raises(AutosubmitCritical): + Autosubmit.delete(expid=f'{expid}', force=True) -- GitLab From b694f1672f740248780067553229bbd74cf543e4 Mon Sep 17 00:00:00 2001 From: dbeltran Date: Wed, 6 Nov 2024 14:23:18 +0100 Subject: [PATCH 04/12] improve coverage --- test/unit/test_expid_pytest.py | 32 +++++++++++++++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/test/unit/test_expid_pytest.py b/test/unit/test_expid_pytest.py index b6bcb315c..8b10c4d51 100644 --- a/test/unit/test_expid_pytest.py +++ b/test/unit/test_expid_pytest.py @@ -1,10 +1,11 @@ import pytest from pathlib import Path + +import log.log from autosubmit.autosubmit import Autosubmit import os import pwd import sqlite3 - from log.log import AutosubmitCritical from test.unit.utils.common import create_database, init_expid @@ -151,3 +152,32 @@ def test_delete_experiment(create_autosubmit_tmpdir, generate_new_experiment, se with pytest.raises(AutosubmitCritical): Autosubmit.delete(expid=f'{expid}', force=True) + +@pytest.mark.parametrize("generate_new_experiment", ['test', 'normal', 'operational'], indirect=True) +def test_delete_experiment_not_owner(create_autosubmit_tmpdir, generate_new_experiment, setup_experiment_yamlfiles, mocker): + expid = generate_new_experiment + print(f"Running test for {expid}") + mocker.patch('autosubmit.autosubmit.Autosubmit._user_yes_no_query', return_value=True) + # raise typeError + + with mocker.patch('pwd.getpwuid', side_effect=TypeError): + _, _, current_owner = Autosubmit._check_ownership(expid) + assert current_owner is None + # test not owner not eadmin + mocker.patch("autosubmit.autosubmit.Autosubmit._check_ownership", return_value=(False, False, create_autosubmit_tmpdir.owner)) + with pytest.raises(AutosubmitCritical): + Autosubmit.delete(expid=f'{expid}', force=True) + # test eadmin + mocker.patch("autosubmit.autosubmit.Autosubmit._check_ownership", return_value=(False, True, create_autosubmit_tmpdir.owner)) + Autosubmit.delete(expid=f'{expid}', force=True) + assert all(expid not in Path(f).name for f in Path(f"{create_autosubmit_tmpdir.strpath}").iterdir()) + assert all(expid not in Path(f).name for f in Path(f"{create_autosubmit_tmpdir.strpath}/metadata/database").iterdir()) + assert all(expid not in Path(f).name for f in Path(f"{create_autosubmit_tmpdir.strpath}/metadata/logs").iterdir()) + assert all(expid not in Path(f).name for f in Path(f"{create_autosubmit_tmpdir.strpath}/metadata/structures").iterdir()) + # Consult if the expid is not in the database + db_path = Path(f"{create_autosubmit_tmpdir.strpath}/tests.db") + conn = sqlite3.connect(db_path) + cursor = conn.cursor() + cursor.execute(f"SELECT name FROM experiment WHERE name='{expid}'") + assert cursor.fetchone() is None + cursor.close() -- GitLab From ae436f25e0fd35ff33bfddd32d7e0dfde2d527f2 Mon Sep 17 00:00:00 2001 From: dbeltran Date: Wed, 6 Nov 2024 14:25:32 +0100 Subject: [PATCH 05/12] improve coverage --- test/unit/test_expid_pytest.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/test/unit/test_expid_pytest.py b/test/unit/test_expid_pytest.py index 8b10c4d51..9fbb96e88 100644 --- a/test/unit/test_expid_pytest.py +++ b/test/unit/test_expid_pytest.py @@ -158,8 +158,6 @@ def test_delete_experiment_not_owner(create_autosubmit_tmpdir, generate_new_expe expid = generate_new_experiment print(f"Running test for {expid}") mocker.patch('autosubmit.autosubmit.Autosubmit._user_yes_no_query', return_value=True) - # raise typeError - with mocker.patch('pwd.getpwuid', side_effect=TypeError): _, _, current_owner = Autosubmit._check_ownership(expid) assert current_owner is None @@ -169,7 +167,10 @@ def test_delete_experiment_not_owner(create_autosubmit_tmpdir, generate_new_expe Autosubmit.delete(expid=f'{expid}', force=True) # test eadmin mocker.patch("autosubmit.autosubmit.Autosubmit._check_ownership", return_value=(False, True, create_autosubmit_tmpdir.owner)) - Autosubmit.delete(expid=f'{expid}', force=True) + with pytest.raises(AutosubmitCritical): + Autosubmit.delete(expid=f'{expid}', force=False) + # test eadmin force + Autosubmit.delete(expid=f'{expid}', force=False) assert all(expid not in Path(f).name for f in Path(f"{create_autosubmit_tmpdir.strpath}").iterdir()) assert all(expid not in Path(f).name for f in Path(f"{create_autosubmit_tmpdir.strpath}/metadata/database").iterdir()) assert all(expid not in Path(f).name for f in Path(f"{create_autosubmit_tmpdir.strpath}/metadata/logs").iterdir()) -- GitLab From 171f2ab7117182833b68090d8045ae54b2e8e70d Mon Sep 17 00:00:00 2001 From: dbeltran Date: Wed, 6 Nov 2024 14:25:44 +0100 Subject: [PATCH 06/12] improve coverage --- test/unit/test_expid_pytest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/test_expid_pytest.py b/test/unit/test_expid_pytest.py index 9fbb96e88..6e378d5b8 100644 --- a/test/unit/test_expid_pytest.py +++ b/test/unit/test_expid_pytest.py @@ -170,7 +170,7 @@ def test_delete_experiment_not_owner(create_autosubmit_tmpdir, generate_new_expe with pytest.raises(AutosubmitCritical): Autosubmit.delete(expid=f'{expid}', force=False) # test eadmin force - Autosubmit.delete(expid=f'{expid}', force=False) + Autosubmit.delete(expid=f'{expid}', force=True) assert all(expid not in Path(f).name for f in Path(f"{create_autosubmit_tmpdir.strpath}").iterdir()) assert all(expid not in Path(f).name for f in Path(f"{create_autosubmit_tmpdir.strpath}/metadata/database").iterdir()) assert all(expid not in Path(f).name for f in Path(f"{create_autosubmit_tmpdir.strpath}/metadata/logs").iterdir()) -- GitLab From 723aaeb2fa5b08be9b4ca300c10f0d2d33f99b4a Mon Sep 17 00:00:00 2001 From: dbeltran Date: Wed, 6 Nov 2024 14:45:16 +0100 Subject: [PATCH 07/12] improve coverage --- test/unit/test_expid_pytest.py | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/test/unit/test_expid_pytest.py b/test/unit/test_expid_pytest.py index 6e378d5b8..d2d972667 100644 --- a/test/unit/test_expid_pytest.py +++ b/test/unit/test_expid_pytest.py @@ -6,7 +6,7 @@ from autosubmit.autosubmit import Autosubmit import os import pwd import sqlite3 -from log.log import AutosubmitCritical +from log.log import AutosubmitCritical, AutosubmitError from test.unit.utils.common import create_database, init_expid @@ -182,3 +182,18 @@ def test_delete_experiment_not_owner(create_autosubmit_tmpdir, generate_new_expe cursor.execute(f"SELECT name FROM experiment WHERE name='{expid}'") assert cursor.fetchone() is None cursor.close() + + +@pytest.mark.parametrize("generate_new_experiment", ['normal'], indirect=True) +def test_delete_expid(create_autosubmit_tmpdir, generate_new_experiment, setup_experiment_yamlfiles, mocker): + expid = generate_new_experiment + mocker.patch("autosubmit.autosubmit.Autosubmit._check_ownership", return_value=(True, True, create_autosubmit_tmpdir.owner)) + mocker.patch('autosubmit.autosubmit.Autosubmit._perform_deletion', return_value="error") + with pytest.raises(AutosubmitError): + Autosubmit._delete_expid(expid, force=True) + mocker.stopall() + mocker.patch("autosubmit.autosubmit.Autosubmit._check_ownership", return_value=(True, True, create_autosubmit_tmpdir.owner)) + Autosubmit._delete_expid(expid, force=True) + assert not Autosubmit._delete_expid(expid, force=True) + + -- GitLab From 01c3401a2b54c9e6bfa2985be5e07fc3dd75a0db Mon Sep 17 00:00:00 2001 From: dbeltran Date: Wed, 6 Nov 2024 16:04:59 +0100 Subject: [PATCH 08/12] improve coverage --- autosubmit/autosubmit.py | 58 ++++++++++++++++------------------ test/unit/test_expid_pytest.py | 19 ++++++++++- 2 files changed, 46 insertions(+), 31 deletions(-) diff --git a/autosubmit/autosubmit.py b/autosubmit/autosubmit.py index e53a157b3..9d343e10c 100644 --- a/autosubmit/autosubmit.py +++ b/autosubmit/autosubmit.py @@ -1112,40 +1112,38 @@ class Autosubmit: :rtype: str """ error_message = "" + Log.info("Deleting experiment from database...") try: - Log.info("Deleting experiment from database...") - try: - ret = delete_experiment(expid_delete) - if ret: - Log.result(f"Experiment {expid_delete} deleted") - except BaseException as e: - error_message += f"Cannot delete experiment entry: {e}\n" + ret = delete_experiment(expid_delete) + if ret: + Log.result(f"Experiment {expid_delete} deleted") + except BaseException as e: + error_message += f"Cannot delete experiment entry: {e}\n" - Log.info("Removing experiment directory...") - try: - shutil.rmtree(experiment_path) - except BaseException as e: - error_message += f"Cannot delete directory: {e}\n" + Log.info("Removing experiment directory...") + try: + shutil.rmtree(experiment_path) + except BaseException as e: + error_message += f"Cannot delete directory: {e}\n" - Log.info("Removing Structure db...") - try: - if structure_db_path.exists(): - os.remove(structure_db_path) - except BaseException as e: - error_message += f"Cannot delete structure: {e}\n" + Log.info("Removing Structure db...") + try: + if structure_db_path.exists(): + os.remove(structure_db_path) + except BaseException as e: + error_message += f"Cannot delete structure: {e}\n" + + Log.info("Removing job_data db...") + try: + db_path = job_data_db_path.with_suffix(".db") + sql_path = job_data_db_path.with_suffix(".sql") + if db_path.exists(): + os.remove(db_path) + if sql_path.exists(): + os.remove(sql_path) + except BaseException as e: + error_message += f"Cannot delete job_data: {e}\n" - Log.info("Removing job_data db...") - try: - db_path = job_data_db_path.with_suffix(".db") - sql_path = job_data_db_path.with_suffix(".sql") - if db_path.exists(): - os.remove(db_path) - if sql_path.exists(): - os.remove(sql_path) - except BaseException as e: - error_message += f"Cannot delete job_data: {e}\n" - except Exception as e: - error_message += f"Couldn't delete the experiment: {e}" return error_message @staticmethod diff --git a/test/unit/test_expid_pytest.py b/test/unit/test_expid_pytest.py index d2d972667..9f0c9c272 100644 --- a/test/unit/test_expid_pytest.py +++ b/test/unit/test_expid_pytest.py @@ -1,15 +1,18 @@ import pytest from pathlib import Path +from more_itertools.more import side_effect + import log.log from autosubmit.autosubmit import Autosubmit import os import pwd import sqlite3 + +from build.lib.autosubmitconfigparser.config.basicconfig import BasicConfig from log.log import AutosubmitCritical, AutosubmitError from test.unit.utils.common import create_database, init_expid - def _get_script_files_path() -> Path: return Path(__file__).resolve().parent / 'files' @@ -197,3 +200,17 @@ def test_delete_expid(create_autosubmit_tmpdir, generate_new_experiment, setup_e assert not Autosubmit._delete_expid(expid, force=True) +@pytest.mark.parametrize("generate_new_experiment", ['normal'], indirect=True) +def test_perform_deletion(create_autosubmit_tmpdir, generate_new_experiment, setup_experiment_yamlfiles, mocker): + expid = generate_new_experiment + mocker.patch("shutil.rmtree", side_effect=FileNotFoundError) + mocker.patch("os.remove", side_effect=FileNotFoundError) + basic_config = BasicConfig() + basic_config.read() + experiment_path = Path(f"{basic_config.LOCAL_ROOT_DIR}/{expid}") + structure_db_path = Path(f"{basic_config.STRUCTURES_DIR}/structure_{expid}.db") + job_data_db_path = Path(f"{basic_config.JOBDATA_DIR}/job_data_{expid}") + mocker.patch("autosubmit.autosubmit.delete_experiment", side_effect=FileNotFoundError) + err_message = Autosubmit._perform_deletion(experiment_path, structure_db_path, job_data_db_path, expid) + # check if these messages "Cannot delete experiment entry", "Cannot delete directory", "Cannot delete structure", "Cannot delete job_data"] are in any part of err_message + assert all(x in err_message for x in ["Cannot delete experiment entry", "Cannot delete directory", "Cannot delete structure", "Cannot delete job_data"]) \ No newline at end of file -- GitLab From 731d2ad4647cbee2ab7f0ffc0dabc3b9bef3f133 Mon Sep 17 00:00:00 2001 From: dbeltran Date: Wed, 6 Nov 2024 16:06:16 +0100 Subject: [PATCH 09/12] improve coverage --- test/unit/test_expid_pytest.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/test/unit/test_expid_pytest.py b/test/unit/test_expid_pytest.py index 9f0c9c272..683b44c34 100644 --- a/test/unit/test_expid_pytest.py +++ b/test/unit/test_expid_pytest.py @@ -1,9 +1,5 @@ import pytest from pathlib import Path - -from more_itertools.more import side_effect - -import log.log from autosubmit.autosubmit import Autosubmit import os import pwd -- GitLab From f67d3f1fe28740245c16148b3ac3b1fc9e63c346 Mon Sep 17 00:00:00 2001 From: dbeltran Date: Wed, 6 Nov 2024 16:07:00 +0100 Subject: [PATCH 10/12] wrong path of import --- test/unit/test_expid_pytest.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/unit/test_expid_pytest.py b/test/unit/test_expid_pytest.py index 683b44c34..bed552fa5 100644 --- a/test/unit/test_expid_pytest.py +++ b/test/unit/test_expid_pytest.py @@ -5,14 +5,14 @@ import os import pwd import sqlite3 -from build.lib.autosubmitconfigparser.config.basicconfig import BasicConfig +from autosubmitconfigparser.config.basicconfig import BasicConfig from log.log import AutosubmitCritical, AutosubmitError from test.unit.utils.common import create_database, init_expid + def _get_script_files_path() -> Path: return Path(__file__).resolve().parent / 'files' - @pytest.fixture def create_autosubmit_tmpdir(tmpdir_factory): folder = tmpdir_factory.mktemp('autosubmit_tests') -- GitLab From d0e593a3db08c27becaf27b9fcada730f507382d Mon Sep 17 00:00:00 2001 From: dbeltran Date: Wed, 6 Nov 2024 16:07:19 +0100 Subject: [PATCH 11/12] lint --- test/unit/test_expid_pytest.py | 1 + 1 file changed, 1 insertion(+) diff --git a/test/unit/test_expid_pytest.py b/test/unit/test_expid_pytest.py index bed552fa5..274f5f6ca 100644 --- a/test/unit/test_expid_pytest.py +++ b/test/unit/test_expid_pytest.py @@ -13,6 +13,7 @@ from test.unit.utils.common import create_database, init_expid def _get_script_files_path() -> Path: return Path(__file__).resolve().parent / 'files' + @pytest.fixture def create_autosubmit_tmpdir(tmpdir_factory): folder = tmpdir_factory.mktemp('autosubmit_tests') -- GitLab From 7bf6d0d58df98993ef2e5bcc82468a18ff191884 Mon Sep 17 00:00:00 2001 From: dbeltran Date: Wed, 6 Nov 2024 16:22:18 +0100 Subject: [PATCH 12/12] removed leftout comment --- test/unit/test_expid_pytest.py | 1 - 1 file changed, 1 deletion(-) diff --git a/test/unit/test_expid_pytest.py b/test/unit/test_expid_pytest.py index 274f5f6ca..f9affb0c2 100644 --- a/test/unit/test_expid_pytest.py +++ b/test/unit/test_expid_pytest.py @@ -209,5 +209,4 @@ def test_perform_deletion(create_autosubmit_tmpdir, generate_new_experiment, set job_data_db_path = Path(f"{basic_config.JOBDATA_DIR}/job_data_{expid}") mocker.patch("autosubmit.autosubmit.delete_experiment", side_effect=FileNotFoundError) err_message = Autosubmit._perform_deletion(experiment_path, structure_db_path, job_data_db_path, expid) - # check if these messages "Cannot delete experiment entry", "Cannot delete directory", "Cannot delete structure", "Cannot delete job_data"] are in any part of err_message assert all(x in err_message for x in ["Cannot delete experiment entry", "Cannot delete directory", "Cannot delete structure", "Cannot delete job_data"]) \ No newline at end of file -- GitLab