From 3a781276a6d6b81633c044ddf8abcb23fc6847ee Mon Sep 17 00:00:00 2001 From: cmadjar Date: Thu, 17 Apr 2025 14:12:44 -0400 Subject: [PATCH 01/17] add tests for mass_nifti_pic.py --- python/lib/db/queries/parameter_file.py | 13 ++ python/scripts/mass_nifti_pic.py | 8 +- .../scripts/test_mass_nifti_pic.py | 208 ++++++++++++++++++ 3 files changed, 225 insertions(+), 4 deletions(-) create mode 100644 python/lib/db/queries/parameter_file.py create mode 100644 python/tests/integration/scripts/test_mass_nifti_pic.py diff --git a/python/lib/db/queries/parameter_file.py b/python/lib/db/queries/parameter_file.py new file mode 100644 index 000000000..a88c92343 --- /dev/null +++ b/python/lib/db/queries/parameter_file.py @@ -0,0 +1,13 @@ +from sqlalchemy import delete +from sqlalchemy.orm import Session as Database + +from lib.db.models.file_parameter import DbFileParameter + + +def delete_file_parameter(db: Database, file_parameter_id: int): + """ + Delete from the database a parameter value based on a file ID and parameter ID. + """ + + db.execute(delete(DbFileParameter) + .where(DbFileParameter.id == file_parameter_id)) diff --git a/python/scripts/mass_nifti_pic.py b/python/scripts/mass_nifti_pic.py index 5fdf32311..dba5edea7 100755 --- a/python/scripts/mass_nifti_pic.py +++ b/python/scripts/mass_nifti_pic.py @@ -103,15 +103,15 @@ def input_error_checking(profile, smallest_id, largest_id, usage): sys.exit(lib.exitcode.MISSING_ARG) if not smallest_id: - message = '\n\tERROR: you must specify a smallest PhysiologyFileID on ' \ - 'which to run the chunking script using -s or --smallest_id option' + message = '\n\tERROR: you must specify a smallest FileID on which to run the ' \ + 'the mass_nifti_pic.py script using -s or --smallest_id option' print(message) print(usage) sys.exit(lib.exitcode.MISSING_ARG) if not largest_id: - message = '\n\tERROR: you must specify a largest PhysiologyFileID on ' \ - 'which to run the chunking script using -l or --largest_id option' + message = '\n\tERROR: you must specify a largest FileID on which to run the ' \ + 'mass_nifti_pic.py script using -l or --largest_id option' print(message) print(usage) sys.exit(lib.exitcode.MISSING_ARG) diff --git a/python/tests/integration/scripts/test_mass_nifti_pic.py b/python/tests/integration/scripts/test_mass_nifti_pic.py new file mode 100644 index 000000000..310b8a274 --- /dev/null +++ b/python/tests/integration/scripts/test_mass_nifti_pic.py @@ -0,0 +1,208 @@ +import time + +from lib.db.models.file import DbFile +from lib.db.queries.file import try_get_parameter_value_with_file_id_parameter_name +from lib.db.queries.parameter_file import delete_file_parameter +from lib.exitcode import GETOPT_FAILURE, INVALID_PATH, MISSING_ARG, SUCCESS +from tests.util.database import get_integration_database_session +from tests.util.run_integration_script import run_integration_script + + +def test_missing_profile_arg(): + + process = run_integration_script([ + 'mass_nifti_pic.py', + ]) + + # Check return code, STDOUT and STDERR + message = 'ERROR: you must specify a profile file using -p or --profile option' + assert process.returncode == MISSING_ARG + assert message in process.stdout + assert process.stderr == "" + + +def test_invalid_profile_arg(): + + process = run_integration_script([ + 'mass_nifti_pic.py', + '--profile', 'invalid_profile.py' + ]) + + # Check return code, STDOUT and STDERR + message = 'ERROR: you must specify a valid profile file.\ninvalid_profile.py does not exist!' + assert process.returncode == INVALID_PATH + assert message in process.stdout + assert process.stderr == "" + + +def test_invalid_arg(): + + process = run_integration_script([ + 'mass_nifti_pic.py', + '--profile', 'database_config.py', + '--invalid_arg', + ]) + + # Check return code, STDOUT and STDERR + assert process.returncode == GETOPT_FAILURE + assert "option --invalid_arg not recognized" in process.stdout + assert process.stderr == "" + + +def test_missing_smallest_id_arg(): + + process = run_integration_script([ + 'mass_nifti_pic.py', + '--profile', 'database_config.py', + ]) + + # Check return code, STDOUT and STDERR + message = 'ERROR: you must specify a smallest FileID on which to run the' \ + ' the mass_nifti_pic.py script using -s or --smallest_id option' + assert process.returncode == MISSING_ARG + assert message in process.stdout + assert process.stderr == "" + + +def test_missing_largest_id_arg(): + + process = run_integration_script([ + 'mass_nifti_pic.py', + '--profile', 'database_config.py', + '--smallest_id', '2', + ]) + + # Check return code, STDOUT and STDERR + message = 'ERROR: you must specify a largest FileID on which to run the' \ + ' the mass_nifti_pic.py script using -l or --largest_id option' + assert process.returncode == MISSING_ARG + assert message in process.stdout + assert process.stderr == "" + + +def test_smallest_id_bigger_than_largest_id(): + + process = run_integration_script([ + 'mass_nifti_pic.py', + '--profile', 'database_config.py', + '--smallest_id', '6', + '--largest_id', '2' + ]) + + # Check return code, STDOUT and STDERR + message = 'ERROR: the value provided for --smallest_id option is bigger than value provided for --largest_id option' + assert process.returncode == MISSING_ARG + assert message in process.stdout + assert process.stderr == "" + + +def test_on_invalid_file_id(): + + process = run_integration_script([ + 'mass_nifti_pic.py', + '--profile', 'database_config.py', + '--smallest_id', '999', + '--largest_id', '999' + ]) + + # Check return code, STDOUT and STDERR + message = 'WARNING: no file in the database with FileID = 999' + assert process.returncode == SUCCESS + assert message in process.stdout + assert process.stderr == "" + + +def test_on_file_id_that_already_has_a_pic(): + + process = run_integration_script([ + 'mass_nifti_pic.py', + '--profile', 'database_config.py', + '--smallest_id', '2', + '--largest_id', '2' + ]) + + # Check return code, STDOUT and STDERR + message = 'WARNING: there is already a pic for FileID 2. Use -f or --force to overwrite it' + assert process.returncode == SUCCESS + assert message in process.stdout + assert process.stderr == "" + + +def test_force_option(): + + # database connection + db = get_integration_database_session() + + current_time = time.time() + + process = run_integration_script([ + 'mass_nifti_pic.py', + '--profile', 'database_config.py', + '--smallest_id', '2', + '--largest_id', '2', + '--force' + ]) + + # Check return code, STDOUT and STDERR + assert process.returncode == SUCCESS + assert process.stdout == "" + assert process.stderr == "" + + file_pic_data = try_get_parameter_value_with_file_id_parameter_name(db, 2, 'check_pic_filename') + assert file_pic_data is not None and file_pic_data.insert_time >= current_time + + +def test_running_on_non_nifti_file(): + + # database connection + db = get_integration_database_session() + + # insert fake text file type with file ID 1 + file = DbFile() + file.id = 1 + file.file = 'test.txt' + file.file_type = 'txt' + file.session_id = 564 + file.output_type = 'native' + file.inserted_by_user_id = 'test' + db.add(file) + db.commit() + file_pic_data = try_get_parameter_value_with_file_id_parameter_name(db, 1, 'check_pic_filename') + assert file_pic_data is None + + process = run_integration_script([ + 'mass_nifti_pic.py', + '--profile', 'database_config.py', + '--smallest_id', '1', + '--largest_id', '1' + ]) + + # Check return code, STDOUT and STDERR + message = 'WARNING: wrong file type. File test.txt is not a .nii.gz file' + assert process.returncode == SUCCESS + assert message in process.stdout + assert process.stderr == "" + + +def test_successful_run(): + + # database connection + db = get_integration_database_session() + file_pic_data = try_get_parameter_value_with_file_id_parameter_name(db, 2, 'check_pic_filename') + if file_pic_data: + # delete pic entry based on it parameter file ID + delete_file_parameter(db, file_pic_data.id) + file_pic_data = try_get_parameter_value_with_file_id_parameter_name(db, 2, 'check_pic_filename') + assert file_pic_data is None + + process = run_integration_script([ + 'mass_nifti_pic.py', + '--profile', 'database_config.py', + '--smallest_id', '2', + '--largest_id', '2' + ]) + + # Check return code, STDOUT and STDERR + assert process.returncode == SUCCESS + assert process.stdout == "" + assert process.stderr == "" From 18c30f05ea5d5dfdb6e60e2ca73e2be8401029fc Mon Sep 17 00:00:00 2001 From: cmadjar Date: Thu, 17 Apr 2025 14:41:56 -0400 Subject: [PATCH 02/17] fix some tests --- .../scripts/test_mass_nifti_pic.py | 50 ++++++++++--------- 1 file changed, 26 insertions(+), 24 deletions(-) diff --git a/python/tests/integration/scripts/test_mass_nifti_pic.py b/python/tests/integration/scripts/test_mass_nifti_pic.py index 310b8a274..e4dfdaa37 100644 --- a/python/tests/integration/scripts/test_mass_nifti_pic.py +++ b/python/tests/integration/scripts/test_mass_nifti_pic.py @@ -3,7 +3,7 @@ from lib.db.models.file import DbFile from lib.db.queries.file import try_get_parameter_value_with_file_id_parameter_name from lib.db.queries.parameter_file import delete_file_parameter -from lib.exitcode import GETOPT_FAILURE, INVALID_PATH, MISSING_ARG, SUCCESS +from lib.exitcode import GETOPT_FAILURE, INVALID_ARG, MISSING_ARG, SUCCESS from tests.util.database import get_integration_database_session from tests.util.run_integration_script import run_integration_script @@ -30,53 +30,55 @@ def test_invalid_profile_arg(): # Check return code, STDOUT and STDERR message = 'ERROR: you must specify a valid profile file.\ninvalid_profile.py does not exist!' - assert process.returncode == INVALID_PATH + assert process.returncode == MISSING_ARG assert message in process.stdout assert process.stderr == "" -def test_invalid_arg(): +def test_missing_smallest_id_arg(): process = run_integration_script([ 'mass_nifti_pic.py', '--profile', 'database_config.py', - '--invalid_arg', ]) # Check return code, STDOUT and STDERR - assert process.returncode == GETOPT_FAILURE - assert "option --invalid_arg not recognized" in process.stdout + message = 'ERROR: you must specify a smallest FileID on which to run the' \ + ' mass_nifti_pic.py script using -s or --smallest_id option' + assert process.returncode == MISSING_ARG + assert message in process.stdout assert process.stderr == "" -def test_missing_smallest_id_arg(): +def test_missing_largest_id_arg(): process = run_integration_script([ 'mass_nifti_pic.py', '--profile', 'database_config.py', + '--smallest_id', '2', ]) # Check return code, STDOUT and STDERR - message = 'ERROR: you must specify a smallest FileID on which to run the' \ - ' the mass_nifti_pic.py script using -s or --smallest_id option' + message = 'ERROR: you must specify a largest FileID on which to run the' \ + ' mass_nifti_pic.py script using -l or --largest_id option' assert process.returncode == MISSING_ARG assert message in process.stdout assert process.stderr == "" -def test_missing_largest_id_arg(): +def test_invalid_arg(): process = run_integration_script([ 'mass_nifti_pic.py', '--profile', 'database_config.py', - '--smallest_id', '2', + '--smallest_id', '1', + '--largest_id', '1', + '--invalid_arg', ]) # Check return code, STDOUT and STDERR - message = 'ERROR: you must specify a largest FileID on which to run the' \ - ' the mass_nifti_pic.py script using -l or --largest_id option' - assert process.returncode == MISSING_ARG - assert message in process.stdout + assert process.returncode == GETOPT_FAILURE + assert "option --invalid_arg not recognized" in process.stdout assert process.stderr == "" @@ -91,7 +93,7 @@ def test_smallest_id_bigger_than_largest_id(): # Check return code, STDOUT and STDERR message = 'ERROR: the value provided for --smallest_id option is bigger than value provided for --largest_id option' - assert process.returncode == MISSING_ARG + assert process.returncode == INVALID_ARG assert message in process.stdout assert process.stderr == "" @@ -157,19 +159,18 @@ def test_running_on_non_nifti_file(): # database connection db = get_integration_database_session() - # insert fake text file type with file ID 1 + # insert fake text file with file ID 1 file = DbFile() - file.id = 1 - file.file = 'test.txt' - file.file_type = 'txt' - file.session_id = 564 - file.output_type = 'native' + file.id = 1 + file.rel_path = 'test.txt' + file.file_type = 'txt' + file.session_id = 564 + file.output_type = 'native' file.inserted_by_user_id = 'test' db.add(file) db.commit() - file_pic_data = try_get_parameter_value_with_file_id_parameter_name(db, 1, 'check_pic_filename') - assert file_pic_data is None + # run NIfTI pic script on the inserted file process = run_integration_script([ 'mass_nifti_pic.py', '--profile', 'database_config.py', @@ -192,6 +193,7 @@ def test_successful_run(): if file_pic_data: # delete pic entry based on it parameter file ID delete_file_parameter(db, file_pic_data.id) + db.commit() file_pic_data = try_get_parameter_value_with_file_id_parameter_name(db, 2, 'check_pic_filename') assert file_pic_data is None From adb9951e895e2dd956dea11809f8d7434fbb3c1d Mon Sep 17 00:00:00 2001 From: cmadjar Date: Thu, 17 Apr 2025 15:21:20 -0400 Subject: [PATCH 03/17] fix some tests --- .../scripts/test_mass_nifti_pic.py | 24 ++++--------------- 1 file changed, 5 insertions(+), 19 deletions(-) diff --git a/python/tests/integration/scripts/test_mass_nifti_pic.py b/python/tests/integration/scripts/test_mass_nifti_pic.py index e4dfdaa37..6739b5b2f 100644 --- a/python/tests/integration/scripts/test_mass_nifti_pic.py +++ b/python/tests/integration/scripts/test_mass_nifti_pic.py @@ -1,5 +1,6 @@ import time +from datetime import datetime from lib.db.models.file import DbFile from lib.db.queries.file import try_get_parameter_value_with_file_id_parameter_name from lib.db.queries.parameter_file import delete_file_parameter @@ -43,8 +44,8 @@ def test_missing_smallest_id_arg(): ]) # Check return code, STDOUT and STDERR - message = 'ERROR: you must specify a smallest FileID on which to run the' \ - ' mass_nifti_pic.py script using -s or --smallest_id option' + message = 'ERROR: you must specify a smallest FileID on which to run' \ + ' the mass_nifti_pic.py script using -s or --smallest_id option' assert process.returncode == MISSING_ARG assert message in process.stdout assert process.stderr == "" @@ -66,22 +67,6 @@ def test_missing_largest_id_arg(): assert process.stderr == "" -def test_invalid_arg(): - - process = run_integration_script([ - 'mass_nifti_pic.py', - '--profile', 'database_config.py', - '--smallest_id', '1', - '--largest_id', '1', - '--invalid_arg', - ]) - - # Check return code, STDOUT and STDERR - assert process.returncode == GETOPT_FAILURE - assert "option --invalid_arg not recognized" in process.stdout - assert process.stderr == "" - - def test_smallest_id_bigger_than_largest_id(): process = run_integration_script([ @@ -92,7 +77,7 @@ def test_smallest_id_bigger_than_largest_id(): ]) # Check return code, STDOUT and STDERR - message = 'ERROR: the value provided for --smallest_id option is bigger than value provided for --largest_id option' + message = 'ERROR: the value for --smallest_id option is bigger than value for --largest_id option' assert process.returncode == INVALID_ARG assert message in process.stdout assert process.stderr == "" @@ -166,6 +151,7 @@ def test_running_on_non_nifti_file(): file.file_type = 'txt' file.session_id = 564 file.output_type = 'native' + file.insert_time = datetime.now().timestamp() file.inserted_by_user_id = 'test' db.add(file) db.commit() From 4556a36a8fbc520873d83ea21288b6c96bf4380c Mon Sep 17 00:00:00 2001 From: cmadjar Date: Thu, 17 Apr 2025 15:23:56 -0400 Subject: [PATCH 04/17] fix some tests --- python/tests/integration/scripts/test_mass_nifti_pic.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/python/tests/integration/scripts/test_mass_nifti_pic.py b/python/tests/integration/scripts/test_mass_nifti_pic.py index 6739b5b2f..34e0e52f5 100644 --- a/python/tests/integration/scripts/test_mass_nifti_pic.py +++ b/python/tests/integration/scripts/test_mass_nifti_pic.py @@ -1,10 +1,11 @@ import time -from datetime import datetime +from sqlalchemy.sql.functions import now + from lib.db.models.file import DbFile from lib.db.queries.file import try_get_parameter_value_with_file_id_parameter_name from lib.db.queries.parameter_file import delete_file_parameter -from lib.exitcode import GETOPT_FAILURE, INVALID_ARG, MISSING_ARG, SUCCESS +from lib.exitcode import INVALID_ARG, MISSING_ARG, SUCCESS from tests.util.database import get_integration_database_session from tests.util.run_integration_script import run_integration_script @@ -120,6 +121,8 @@ def test_force_option(): # database connection db = get_integration_database_session() + file_pic_data = try_get_parameter_value_with_file_id_parameter_name(db, 2, 'check_pic_filename') + current_time = time.time() process = run_integration_script([ @@ -151,7 +154,7 @@ def test_running_on_non_nifti_file(): file.file_type = 'txt' file.session_id = 564 file.output_type = 'native' - file.insert_time = datetime.now().timestamp() + file.insert_time = now() file.inserted_by_user_id = 'test' db.add(file) db.commit() From 1068fc3c08d732283cda6c46de0b0f2f50d80ad9 Mon Sep 17 00:00:00 2001 From: cmadjar Date: Thu, 17 Apr 2025 15:28:31 -0400 Subject: [PATCH 05/17] fix some tests --- python/tests/integration/scripts/test_mass_nifti_pic.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/python/tests/integration/scripts/test_mass_nifti_pic.py b/python/tests/integration/scripts/test_mass_nifti_pic.py index 34e0e52f5..b5b7e6fc4 100644 --- a/python/tests/integration/scripts/test_mass_nifti_pic.py +++ b/python/tests/integration/scripts/test_mass_nifti_pic.py @@ -1,7 +1,6 @@ import time -from sqlalchemy.sql.functions import now - +from datetime import datetime from lib.db.models.file import DbFile from lib.db.queries.file import try_get_parameter_value_with_file_id_parameter_name from lib.db.queries.parameter_file import delete_file_parameter @@ -121,7 +120,7 @@ def test_force_option(): # database connection db = get_integration_database_session() - file_pic_data = try_get_parameter_value_with_file_id_parameter_name(db, 2, 'check_pic_filename') + # file_pic_data = try_get_parameter_value_with_file_id_parameter_name(db, 2, 'check_pic_filename') current_time = time.time() @@ -154,7 +153,7 @@ def test_running_on_non_nifti_file(): file.file_type = 'txt' file.session_id = 564 file.output_type = 'native' - file.insert_time = now() + file.insert_time = int(datetime.now().timestamp()) file.inserted_by_user_id = 'test' db.add(file) db.commit() From 0b7f0b061435ad1c64c2798f9d40a0932acb5351 Mon Sep 17 00:00:00 2001 From: cmadjar Date: Thu, 17 Apr 2025 15:36:46 -0400 Subject: [PATCH 06/17] fix some tests --- python/scripts/mass_nifti_pic.py | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/python/scripts/mass_nifti_pic.py b/python/scripts/mass_nifti_pic.py index dba5edea7..55da86190 100755 --- a/python/scripts/mass_nifti_pic.py +++ b/python/scripts/mass_nifti_pic.py @@ -102,9 +102,18 @@ def input_error_checking(profile, smallest_id, largest_id, usage): print(usage) sys.exit(lib.exitcode.MISSING_ARG) + if os.path.isfile(profile): + sys.path.append(os.path.dirname(profile)) + config_file = __import__(os.path.basename(profile[:-3])) + else: + message = f'\n\tERROR: you must specify a valid profile file.\n{profile} does not exist!' + print(message) + print(usage) + sys.exit(lib.exitcode.INVALID_PATH) + if not smallest_id: - message = '\n\tERROR: you must specify a smallest FileID on which to run the ' \ - 'the mass_nifti_pic.py script using -s or --smallest_id option' + message = '\n\tERROR: you must specify a smallest FileID on which to run the' \ + ' mass_nifti_pic.py script using -s or --smallest_id option' print(message) print(usage) sys.exit(lib.exitcode.MISSING_ARG) @@ -123,16 +132,6 @@ def input_error_checking(profile, smallest_id, largest_id, usage): print(usage) sys.exit(lib.exitcode.INVALID_ARG) - if os.path.isfile(profile): - sys.path.append(os.path.dirname(profile)) - config_file = __import__(os.path.basename(profile[:-3])) - else: - message = '\n\tERROR: you must specify a valid profile file.\n' + \ - profile + ' does not exist!' - print(message) - print(usage) - sys.exit(lib.exitcode.INVALID_PATH) - return config_file From 07b2c87249df92afe3112e84d69d43c1c7a3e853 Mon Sep 17 00:00:00 2001 From: cmadjar Date: Thu, 17 Apr 2025 15:38:28 -0400 Subject: [PATCH 07/17] fix some tests --- python/tests/integration/scripts/test_mass_nifti_pic.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/tests/integration/scripts/test_mass_nifti_pic.py b/python/tests/integration/scripts/test_mass_nifti_pic.py index b5b7e6fc4..c1fa1db58 100644 --- a/python/tests/integration/scripts/test_mass_nifti_pic.py +++ b/python/tests/integration/scripts/test_mass_nifti_pic.py @@ -1,6 +1,6 @@ import time - from datetime import datetime + from lib.db.models.file import DbFile from lib.db.queries.file import try_get_parameter_value_with_file_id_parameter_name from lib.db.queries.parameter_file import delete_file_parameter From c9d944ee06f06fd3696438bbe00c58ee91d768ae Mon Sep 17 00:00:00 2001 From: cmadjar Date: Thu, 17 Apr 2025 15:40:11 -0400 Subject: [PATCH 08/17] fix some tests --- python/tests/integration/scripts/test_mass_nifti_pic.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/python/tests/integration/scripts/test_mass_nifti_pic.py b/python/tests/integration/scripts/test_mass_nifti_pic.py index c1fa1db58..fc4707a2b 100644 --- a/python/tests/integration/scripts/test_mass_nifti_pic.py +++ b/python/tests/integration/scripts/test_mass_nifti_pic.py @@ -1,3 +1,4 @@ +import os import time from datetime import datetime @@ -180,8 +181,10 @@ def test_successful_run(): file_pic_data = try_get_parameter_value_with_file_id_parameter_name(db, 2, 'check_pic_filename') if file_pic_data: # delete pic entry based on it parameter file ID + os.remove(file_pic_data.value) delete_file_parameter(db, file_pic_data.id) db.commit() + file_pic_data = try_get_parameter_value_with_file_id_parameter_name(db, 2, 'check_pic_filename') assert file_pic_data is None From 0aed7d48bf0108f5b908ae1e331507f7b5e89479 Mon Sep 17 00:00:00 2001 From: cmadjar Date: Thu, 17 Apr 2025 15:53:36 -0400 Subject: [PATCH 09/17] fix some tests --- .../tests/integration/scripts/test_mass_nifti_pic.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/python/tests/integration/scripts/test_mass_nifti_pic.py b/python/tests/integration/scripts/test_mass_nifti_pic.py index fc4707a2b..6c419f7d9 100644 --- a/python/tests/integration/scripts/test_mass_nifti_pic.py +++ b/python/tests/integration/scripts/test_mass_nifti_pic.py @@ -5,7 +5,7 @@ from lib.db.models.file import DbFile from lib.db.queries.file import try_get_parameter_value_with_file_id_parameter_name from lib.db.queries.parameter_file import delete_file_parameter -from lib.exitcode import INVALID_ARG, MISSING_ARG, SUCCESS +from lib.exitcode import INVALID_ARG, INVALID_PATH, MISSING_ARG, SUCCESS from tests.util.database import get_integration_database_session from tests.util.run_integration_script import run_integration_script @@ -32,7 +32,7 @@ def test_invalid_profile_arg(): # Check return code, STDOUT and STDERR message = 'ERROR: you must specify a valid profile file.\ninvalid_profile.py does not exist!' - assert process.returncode == MISSING_ARG + assert process.returncode == INVALID_PATH assert message in process.stdout assert process.stderr == "" @@ -180,8 +180,11 @@ def test_successful_run(): db = get_integration_database_session() file_pic_data = try_get_parameter_value_with_file_id_parameter_name(db, 2, 'check_pic_filename') if file_pic_data: - # delete pic entry based on it parameter file ID - os.remove(file_pic_data.value) + # remove file from the file system before recreating it + pic_to_remove = os.path.join('/data/loris/pic/', str(file_pic_data.value)) + if os.path.exists(pic_to_remove): + os.remove(pic_to_remove) + # delete pic entry based on its parameter file ID delete_file_parameter(db, file_pic_data.id) db.commit() From 4453200fc256ab92642a4ef4182eb416196846e8 Mon Sep 17 00:00:00 2001 From: cmadjar Date: Thu, 17 Apr 2025 16:16:04 -0400 Subject: [PATCH 10/17] fix some tests --- .../scripts/test_mass_nifti_pic.py | 27 ++++++++++++++++--- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/python/tests/integration/scripts/test_mass_nifti_pic.py b/python/tests/integration/scripts/test_mass_nifti_pic.py index 6c419f7d9..c740c8ea3 100644 --- a/python/tests/integration/scripts/test_mass_nifti_pic.py +++ b/python/tests/integration/scripts/test_mass_nifti_pic.py @@ -31,7 +31,7 @@ def test_invalid_profile_arg(): ]) # Check return code, STDOUT and STDERR - message = 'ERROR: you must specify a valid profile file.\ninvalid_profile.py does not exist!' + message = 'ERROR: you must specify a valid profile file' assert process.returncode == INVALID_PATH assert message in process.stdout assert process.stderr == "" @@ -120,8 +120,13 @@ def test_force_option(): # database connection db = get_integration_database_session() - - # file_pic_data = try_get_parameter_value_with_file_id_parameter_name(db, 2, 'check_pic_filename') + file_pic_data = try_get_parameter_value_with_file_id_parameter_name(db, 2, 'check_pic_filename') + if file_pic_data: + # remove file from the file system before recreating it + # otherwise get Operation Not Permitted (specific to the test environment) + pic_to_remove = os.path.join('/data/loris/pic/', str(file_pic_data.value)) + if os.path.exists(pic_to_remove): + os.remove(pic_to_remove) current_time = time.time() @@ -191,6 +196,8 @@ def test_successful_run(): file_pic_data = try_get_parameter_value_with_file_id_parameter_name(db, 2, 'check_pic_filename') assert file_pic_data is None + current_time = time.time() + process = run_integration_script([ 'mass_nifti_pic.py', '--profile', 'database_config.py', @@ -199,6 +206,18 @@ def test_successful_run(): ]) # Check return code, STDOUT and STDERR + # The NIfTI file is printing a warning when the pic gets created so check that the + # STDERR is exactly that error message. + message = '/usr/local/lib/python3.11/site-packages/nilearn/image/resampling.py:867: UserWarning:' \ + ' Casting data from int32 to float32' \ + '\n return resample_img(' assert process.returncode == SUCCESS assert process.stdout == "" - assert process.stderr == "" + assert process.stderr == message + + # check pic in database and file system + file_pic_data = try_get_parameter_value_with_file_id_parameter_name(db, 2, 'check_pic_filename') + assert file_pic_data is not None + assert file_pic_data.insert_time >= current_time + assert file_pic_data.value is not None + assert os.path.exists(os.path.join('/data/loris/pic/', str(file_pic_data.value))) From 4bca15998f97ea5190c7a6db7dc3153d920d0a21 Mon Sep 17 00:00:00 2001 From: cmadjar Date: Thu, 17 Apr 2025 16:28:05 -0400 Subject: [PATCH 11/17] fix some tests --- python/tests/integration/scripts/test_mass_nifti_pic.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/python/tests/integration/scripts/test_mass_nifti_pic.py b/python/tests/integration/scripts/test_mass_nifti_pic.py index c740c8ea3..a35f60005 100644 --- a/python/tests/integration/scripts/test_mass_nifti_pic.py +++ b/python/tests/integration/scripts/test_mass_nifti_pic.py @@ -139,9 +139,14 @@ def test_force_option(): ]) # Check return code, STDOUT and STDERR + # The NIfTI file is printing a warning when the pic gets created so check that the + # STDERR is exactly that error message. + message = '/usr/local/lib/python3.11/site-packages/nilearn/image/resampling.py:867: UserWarning:' \ + ' Casting data from int32 to float32' \ + '\n return resample_img(\n' assert process.returncode == SUCCESS assert process.stdout == "" - assert process.stderr == "" + assert process.stderr == message file_pic_data = try_get_parameter_value_with_file_id_parameter_name(db, 2, 'check_pic_filename') assert file_pic_data is not None and file_pic_data.insert_time >= current_time @@ -210,7 +215,7 @@ def test_successful_run(): # STDERR is exactly that error message. message = '/usr/local/lib/python3.11/site-packages/nilearn/image/resampling.py:867: UserWarning:' \ ' Casting data from int32 to float32' \ - '\n return resample_img(' + '\n return resample_img(\n' assert process.returncode == SUCCESS assert process.stdout == "" assert process.stderr == message From 3d12c9ee519d21e477a331c641224036ca5ee245 Mon Sep 17 00:00:00 2001 From: cmadjar Date: Thu, 17 Apr 2025 16:32:52 -0400 Subject: [PATCH 12/17] fix some tests --- python/tests/integration/scripts/test_mass_nifti_pic.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/python/tests/integration/scripts/test_mass_nifti_pic.py b/python/tests/integration/scripts/test_mass_nifti_pic.py index a35f60005..3fb2a73c9 100644 --- a/python/tests/integration/scripts/test_mass_nifti_pic.py +++ b/python/tests/integration/scripts/test_mass_nifti_pic.py @@ -3,7 +3,7 @@ from datetime import datetime from lib.db.models.file import DbFile -from lib.db.queries.file import try_get_parameter_value_with_file_id_parameter_name +from lib.db.queries.file import delete_file, try_get_parameter_value_with_file_id_parameter_name from lib.db.queries.parameter_file import delete_file_parameter from lib.exitcode import INVALID_ARG, INVALID_PATH, MISSING_ARG, SUCCESS from tests.util.database import get_integration_database_session @@ -183,6 +183,9 @@ def test_running_on_non_nifti_file(): assert message in process.stdout assert process.stderr == "" + # Clean up the file that was inserted before this test + delete_file(db, 1) + def test_successful_run(): From 8add2baf3a9889ec85447df568d9de5ec11e72a7 Mon Sep 17 00:00:00 2001 From: cmadjar Date: Thu, 17 Apr 2025 16:45:45 -0400 Subject: [PATCH 13/17] fix some tests --- python/lib/db/queries/file.py | 11 ++++++++++- .../tests/integration/scripts/test_mass_nifti_pic.py | 1 + 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/python/lib/db/queries/file.py b/python/lib/db/queries/file.py index 30f3da5af..60578946e 100644 --- a/python/lib/db/queries/file.py +++ b/python/lib/db/queries/file.py @@ -1,4 +1,4 @@ -from sqlalchemy import select +from sqlalchemy import delete, select from sqlalchemy.orm import Session as Database from lib.db.models.file import DbFile @@ -54,3 +54,12 @@ def try_get_file_with_hash(db: Database, file_hash: str) -> DbFile | None: .where(DbParameterType.name.in_(['file_blake2b_hash', 'md5hash'])) .where(DbFileParameter.value == file_hash) ).scalar_one_or_none() + + +def delete_file(db: Database, file_id: int): + """ + Delete from the database a file entry based on a file ID. + """ + + db.execute(delete(DbFile) + .where(DbFile.id == file_id)) diff --git a/python/tests/integration/scripts/test_mass_nifti_pic.py b/python/tests/integration/scripts/test_mass_nifti_pic.py index 3fb2a73c9..2fc8996c3 100644 --- a/python/tests/integration/scripts/test_mass_nifti_pic.py +++ b/python/tests/integration/scripts/test_mass_nifti_pic.py @@ -185,6 +185,7 @@ def test_running_on_non_nifti_file(): # Clean up the file that was inserted before this test delete_file(db, 1) + db.commit() def test_successful_run(): From 82edc1cebabd478e4448791b27d4625102f10122 Mon Sep 17 00:00:00 2001 From: cmadjar Date: Thu, 17 Apr 2025 17:01:31 -0400 Subject: [PATCH 14/17] fix some tests --- python/tests/integration/scripts/test_mass_nifti_pic.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/python/tests/integration/scripts/test_mass_nifti_pic.py b/python/tests/integration/scripts/test_mass_nifti_pic.py index 2fc8996c3..aae1deb42 100644 --- a/python/tests/integration/scripts/test_mass_nifti_pic.py +++ b/python/tests/integration/scripts/test_mass_nifti_pic.py @@ -128,8 +128,6 @@ def test_force_option(): if os.path.exists(pic_to_remove): os.remove(pic_to_remove) - current_time = time.time() - process = run_integration_script([ 'mass_nifti_pic.py', '--profile', 'database_config.py', @@ -149,7 +147,9 @@ def test_force_option(): assert process.stderr == message file_pic_data = try_get_parameter_value_with_file_id_parameter_name(db, 2, 'check_pic_filename') - assert file_pic_data is not None and file_pic_data.insert_time >= current_time + assert file_pic_data is not None + assert file_pic_data.value is not None + assert os.path.exists(os.path.join('/data/loris/pic/', str(file_pic_data.value))) def test_running_on_non_nifti_file(): From 65aa621f1072391edfafa55e63633ce2abcfb653 Mon Sep 17 00:00:00 2001 From: cmadjar Date: Thu, 17 Apr 2025 17:15:39 -0400 Subject: [PATCH 15/17] fix some tests --- .../scripts/test_mass_nifti_pic.py | 34 ++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/python/tests/integration/scripts/test_mass_nifti_pic.py b/python/tests/integration/scripts/test_mass_nifti_pic.py index aae1deb42..1c7f17350 100644 --- a/python/tests/integration/scripts/test_mass_nifti_pic.py +++ b/python/tests/integration/scripts/test_mass_nifti_pic.py @@ -11,6 +11,9 @@ def test_missing_profile_arg(): + """ + Test running the script without the --profile argument. + """ process = run_integration_script([ 'mass_nifti_pic.py', @@ -24,6 +27,9 @@ def test_missing_profile_arg(): def test_invalid_profile_arg(): + """ + Test running the script with an invalid --profile argument. + """ process = run_integration_script([ 'mass_nifti_pic.py', @@ -38,6 +44,9 @@ def test_invalid_profile_arg(): def test_missing_smallest_id_arg(): + """ + Test running the script without the --smallest_id argument. + """ process = run_integration_script([ 'mass_nifti_pic.py', @@ -53,6 +62,9 @@ def test_missing_smallest_id_arg(): def test_missing_largest_id_arg(): + """ + Test running the script without the --largest_id argument. + """ process = run_integration_script([ 'mass_nifti_pic.py', @@ -69,6 +81,9 @@ def test_missing_largest_id_arg(): def test_smallest_id_bigger_than_largest_id(): + """ + Test running the script with a --smallest_id higher than the --largest_id. + """ process = run_integration_script([ 'mass_nifti_pic.py', @@ -85,6 +100,9 @@ def test_smallest_id_bigger_than_largest_id(): def test_on_invalid_file_id(): + """ + Test running the script on an invalid file ID. + """ process = run_integration_script([ 'mass_nifti_pic.py', @@ -101,6 +119,9 @@ def test_on_invalid_file_id(): def test_on_file_id_that_already_has_a_pic(): + """ + Test running the script on a file that already has a pic. + """ process = run_integration_script([ 'mass_nifti_pic.py', @@ -117,6 +138,9 @@ def test_on_file_id_that_already_has_a_pic(): def test_force_option(): + """ + Test running the script on a file that already has the pic with option --force. + """ # database connection db = get_integration_database_session() @@ -152,7 +176,10 @@ def test_force_option(): assert os.path.exists(os.path.join('/data/loris/pic/', str(file_pic_data.value))) -def test_running_on_non_nifti_file(): +def test_running_on_a_text_file(): + """ + Test running the script on a text file (a non-NIfTI file type). + """ # database connection db = get_integration_database_session() @@ -189,9 +216,14 @@ def test_running_on_non_nifti_file(): def test_successful_run(): + """ + Test successful run of the script. + """ # database connection db = get_integration_database_session() + + # Remove file ID 2 pic from the database and filesystem file_pic_data = try_get_parameter_value_with_file_id_parameter_name(db, 2, 'check_pic_filename') if file_pic_data: # remove file from the file system before recreating it From bb00d45a8a8840ea6126055021415812ad76e812 Mon Sep 17 00:00:00 2001 From: cmadjar Date: Thu, 24 Apr 2025 10:29:35 -0400 Subject: [PATCH 16/17] Maxime's feedback --- python/lib/db/queries/file.py | 16 ------ python/lib/db/queries/file_parameter.py | 30 +++++++++++ python/lib/db/queries/parameter_file.py | 13 ----- .../scripts/test_mass_nifti_pic.py | 53 ++++++++++--------- .../scripts/test_run_nifti_insertion.py | 6 +-- 5 files changed, 60 insertions(+), 58 deletions(-) create mode 100644 python/lib/db/queries/file_parameter.py delete mode 100644 python/lib/db/queries/parameter_file.py diff --git a/python/lib/db/queries/file.py b/python/lib/db/queries/file.py index 60578946e..e1d4817a4 100644 --- a/python/lib/db/queries/file.py +++ b/python/lib/db/queries/file.py @@ -26,22 +26,6 @@ def try_get_file_with_unique_combination( ).scalar_one_or_none() -def try_get_parameter_value_with_file_id_parameter_name( - db: Database, - file_id: int, - parameter_name: str -) -> DbFileParameter | None: - """ - Get parameter value from file ID and parameter name, or return `None` if no entry was found - """ - - return db.execute(select(DbFileParameter) - .join(DbFileParameter.type) - .where(DbParameterType.name == parameter_name) - .where(DbFileParameter.file_id == file_id) - ).scalar_one_or_none() - - def try_get_file_with_hash(db: Database, file_hash: str) -> DbFile | None: """ Get an imaging file from the database using its BLAKE2b or MD5 hash, or return `None` if no diff --git a/python/lib/db/queries/file_parameter.py b/python/lib/db/queries/file_parameter.py new file mode 100644 index 000000000..db4ba107f --- /dev/null +++ b/python/lib/db/queries/file_parameter.py @@ -0,0 +1,30 @@ +from sqlalchemy import delete, select +from sqlalchemy.orm import Session as Database + +from lib.db.models.file_parameter import DbFileParameter +from lib.db.models.parameter_type import DbParameterType + + +def delete_file_parameter(db: Database, file_parameter_id: int): + """ + Delete from the database a parameter value based on a file ID and parameter ID. + """ + + db.execute(delete(DbFileParameter) + .where(DbFileParameter.id == file_parameter_id)) + + +def try_get_parameter_value_with_file_id_parameter_name( + db: Database, + file_id: int, + parameter_name: str +) -> DbFileParameter | None: + """ + Get parameter value from file ID and parameter name, or return `None` if no entry was found + """ + + return db.execute(select(DbFileParameter) + .join(DbFileParameter.type) + .where(DbParameterType.name == parameter_name) + .where(DbFileParameter.file_id == file_id) + ).scalar_one_or_none() diff --git a/python/lib/db/queries/parameter_file.py b/python/lib/db/queries/parameter_file.py deleted file mode 100644 index a88c92343..000000000 --- a/python/lib/db/queries/parameter_file.py +++ /dev/null @@ -1,13 +0,0 @@ -from sqlalchemy import delete -from sqlalchemy.orm import Session as Database - -from lib.db.models.file_parameter import DbFileParameter - - -def delete_file_parameter(db: Database, file_parameter_id: int): - """ - Delete from the database a parameter value based on a file ID and parameter ID. - """ - - db.execute(delete(DbFileParameter) - .where(DbFileParameter.id == file_parameter_id)) diff --git a/python/tests/integration/scripts/test_mass_nifti_pic.py b/python/tests/integration/scripts/test_mass_nifti_pic.py index 1c7f17350..7e2580ab1 100644 --- a/python/tests/integration/scripts/test_mass_nifti_pic.py +++ b/python/tests/integration/scripts/test_mass_nifti_pic.py @@ -3,8 +3,8 @@ from datetime import datetime from lib.db.models.file import DbFile -from lib.db.queries.file import delete_file, try_get_parameter_value_with_file_id_parameter_name -from lib.db.queries.parameter_file import delete_file_parameter +from lib.db.queries.file import delete_file +from lib.db.queries.file_parameter import delete_file_parameter, try_get_parameter_value_with_file_id_parameter_name from lib.exitcode import INVALID_ARG, INVALID_PATH, MISSING_ARG, SUCCESS from tests.util.database import get_integration_database_session from tests.util.run_integration_script import run_integration_script @@ -145,12 +145,13 @@ def test_force_option(): # database connection db = get_integration_database_session() file_pic_data = try_get_parameter_value_with_file_id_parameter_name(db, 2, 'check_pic_filename') - if file_pic_data: - # remove file from the file system before recreating it - # otherwise get Operation Not Permitted (specific to the test environment) - pic_to_remove = os.path.join('/data/loris/pic/', str(file_pic_data.value)) - if os.path.exists(pic_to_remove): - os.remove(pic_to_remove) + assert file_pic_data is not None + # remove file from the file system before recreating it + # otherwise get Operation Not Permitted (specific to the test environment) + # TODO: Investigate + pic_to_remove = os.path.join('/data/loris/pic/', str(file_pic_data.value)) + if os.path.exists(pic_to_remove): + os.remove(pic_to_remove) process = run_integration_script([ 'mass_nifti_pic.py', @@ -185,14 +186,14 @@ def test_running_on_a_text_file(): db = get_integration_database_session() # insert fake text file with file ID 1 - file = DbFile() - file.id = 1 - file.rel_path = 'test.txt' - file.file_type = 'txt' - file.session_id = 564 - file.output_type = 'native' - file.insert_time = int(datetime.now().timestamp()) - file.inserted_by_user_id = 'test' + file = DbFile( + rel_path = 'test.txt', + file_type = 'txt', + session_id = 564, + output_type = 'native', + insert_time = int(datetime.now().timestamp()), + inserted_by_user_id = 'test' + ) db.add(file) db.commit() @@ -211,7 +212,7 @@ def test_running_on_a_text_file(): assert process.stderr == "" # Clean up the file that was inserted before this test - delete_file(db, 1) + delete_file(db, file.id) db.commit() @@ -225,14 +226,16 @@ def test_successful_run(): # Remove file ID 2 pic from the database and filesystem file_pic_data = try_get_parameter_value_with_file_id_parameter_name(db, 2, 'check_pic_filename') - if file_pic_data: - # remove file from the file system before recreating it - pic_to_remove = os.path.join('/data/loris/pic/', str(file_pic_data.value)) - if os.path.exists(pic_to_remove): - os.remove(pic_to_remove) - # delete pic entry based on its parameter file ID - delete_file_parameter(db, file_pic_data.id) - db.commit() + assert file_pic_data is not None + # remove file from the file system before recreating it + # otherwise get Operation Not Permitted (specific to the test environment) + # TODO: Investigate + pic_to_remove = os.path.join('/data/loris/pic/', str(file_pic_data.value)) + if os.path.exists(pic_to_remove): + os.remove(pic_to_remove) + # delete pic entry based on its parameter file ID + delete_file_parameter(db, file_pic_data.id) + db.commit() file_pic_data = try_get_parameter_value_with_file_id_parameter_name(db, 2, 'check_pic_filename') assert file_pic_data is None diff --git a/python/tests/integration/scripts/test_run_nifti_insertion.py b/python/tests/integration/scripts/test_run_nifti_insertion.py index 6266fe095..0387c8b2d 100644 --- a/python/tests/integration/scripts/test_run_nifti_insertion.py +++ b/python/tests/integration/scripts/test_run_nifti_insertion.py @@ -2,10 +2,8 @@ import shutil from os.path import basename -from lib.db.queries.file import ( - try_get_file_with_unique_combination, - try_get_parameter_value_with_file_id_parameter_name, -) +from lib.db.queries.file import try_get_file_with_unique_combination +from lib.db.queries.file_parameter import try_get_parameter_value_with_file_id_parameter_name from lib.db.queries.mri_protocol_violated_scan import get_protocol_violated_scans_with_unique_series_combination from lib.db.queries.mri_upload import get_mri_upload_with_patient_name from lib.db.queries.mri_violation_log import get_violations_log_with_unique_series_combination From 9ec9c4a5af31a3d60551aec009e9d04af60a6391 Mon Sep 17 00:00:00 2001 From: cmadjar Date: Thu, 24 Apr 2025 10:50:10 -0400 Subject: [PATCH 17/17] wake up Cecile, wake up :) --- python/tests/integration/scripts/test_mass_nifti_pic.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/python/tests/integration/scripts/test_mass_nifti_pic.py b/python/tests/integration/scripts/test_mass_nifti_pic.py index 7e2580ab1..6977a1d38 100644 --- a/python/tests/integration/scripts/test_mass_nifti_pic.py +++ b/python/tests/integration/scripts/test_mass_nifti_pic.py @@ -185,7 +185,7 @@ def test_running_on_a_text_file(): # database connection db = get_integration_database_session() - # insert fake text file with file ID 1 + # insert fake text file file = DbFile( rel_path = 'test.txt', file_type = 'txt', @@ -201,8 +201,8 @@ def test_running_on_a_text_file(): process = run_integration_script([ 'mass_nifti_pic.py', '--profile', 'database_config.py', - '--smallest_id', '1', - '--largest_id', '1' + '--smallest_id', str(file.id), + '--largest_id', str(file.id) ]) # Check return code, STDOUT and STDERR