Closed Drulex closed 6 months ago
joshuacwnewton
commented
4 years ago sct_testing should run in a temporary directory, so I don't think this affects the old tests.
At the very least, moving forward I agree with @Drulex's recommendations that new tests should clean up after themselves.
joshuacwnewton
commented
3 years ago I wanted to find out which tests were to blame, so I modified the check_testing_data_integrity function that @Drulex wrote so that it would run once per test:
Here are the tests that don't clean up after themselves:
@Drulex The "172 files" amount you noticed roughly matches up with the first 8 pytest tests (1+6+7+6+4+3+144+2=173 in total).
The remaining files are generated inside sct_testing_data/, which was previously covered by #2925. (It was decided to leave it due to the amount of work required to prevent this from happening.)
joshuacwnewton
commented
2 years ago I'm reopening this issue because our tests are once again saving files to the working directory without cleaning up after themselves.
For context: I recently ported our custom-framework sct_testing tests to pytest in https://github.com/spinalcordtoolbox/spinalcordtoolbox/pull/3373. While doing this, I noticed that many new files were being created in the working directory. To avoid a sudden influx of clutter, I added a quick hack:
This doesn't fix the problem -- it just changes the working directory to be sct_testing_data to make the clutter invisible to users/devs.
However, now sct_testing_data is cluttered with output files. So, I would like to try to ensure that our tests really do start and end on a fresh copy of sct_testing_data.
joshuacwnewton
commented
1 year ago I had a bit of a crackpot idea about this. Within the current "data integrity" finalizer function, we already have a list of all "added" files. So, what if we did the cleanup for all of the files at once, within the finalizer, rather than on a per-test basis?
We could even preserve the "files added" by moving them to /tmp, that way they can still be accessed for post-test analysis.
for file in [Path(f) for f in files_added]:
move_and_cleanup(file, tmp_path)
def move_and_cleanup(file, dest):
# Identifiy "<subdirs>" in path: "$SCT_DIR/data/sct_testing_data/<subdirs>/added_file.txt"
# NB: "+1" excludes `sct_testing_data` and "-1" excludes the filename
subdirs = Path(*file.parts[file.parts.index("sct_testing_data")+1:-1])
# Recreate the subdirs inside the destination folder, then move the file inside of them
(dest / subdirs).mkdir(parents=True, exist_ok=True)
shutil.move(file, dest / subdirs)
# Clean up any empty directories left behind in `sct_testing_data`
for i in range(len(file.parent)):
if not os.listdir(file.parent[i]):
shutil.remove(file.parent[i]The upside here is that it saves us a lot of manual work in specifying file deletions for each test.
However, the downside is that we're less aware of the output files for each test (which is probably something we should be aware of when writing tests!). Also that this behavior is a bit opaque to people who are new to pytest -- the file-moving code would be done silently within conftest.py, away from the tests themselves, unless we can somehow write logging messages to the end of pytest's output to clearly indicate that this is happening.
Still, this might be a nice short-term solution for keeping sct_testing_data clean, before we're able to properly rewrite each of our tests.
mguaypaq
commented
1 year ago I added a comment about this issue on the dev wiki in the section on local testing, since it means that the test suite fails on the second run after a fresh install, which is mysterious if you don't know about it (and annoying if you do know about it).
This is because, although the integrity check ignores new files (with a commented out assertion), those new files become changed files after the first run.
I think the proper fix is to run each test in a temporary directory, rather than in data/sct_testing_data.
Before finding this existing issue, I tried making data/sct_testing_data and its contents read-only, and got 62 failures and 1 error in the test suite:
========================================================================== short test summary info ==========================================================================
FAILED testing/api/test_register.py::test_register_step_ants_slice_regularized_registration - PermissionError: [Errno 13] Permission denied: '$SCT_DIR/data/sct_testing_data/mt/mt0_seg_crop.nii.gz'
FAILED testing/cli/test_cli_sct_analyze_lesion.py::test_sct_analyze_lesion_matches_expected_dummy_lesion_measurements[dummy_lesion0-0.001] - PermissionError: [Errno 13] Permission denied: '/tmp/sct_2023-04-04_15-18-59_analyze-lesion_5w9eprmb/t2_seg-manual.nii.gz'
FAILED testing/cli/test_cli_sct_analyze_lesion.py::test_sct_analyze_lesion_matches_expected_dummy_lesion_measurements[dummy_lesion1-0.001] - PermissionError: [Errno 13] Permission denied: '/tmp/sct_2023-04-04_15-18-59_analyze-lesion_dqykol66/t2_seg-manual.nii.gz'
FAILED testing/cli/test_cli_sct_analyze_lesion.py::test_sct_analyze_lesion_matches_expected_dummy_lesion_measurements[dummy_lesion2-0.01] - PermissionError: [Errno 13] Permission denied: '/tmp/sct_2023-04-04_15-18-59_analyze-lesion_rj_71f_b/t2_seg-manual.nii.gz'
FAILED testing/cli/test_cli_sct_analyze_texture.py::test_sct_analyze_texture_image_data_within_threshold - PermissionError: [Errno 13] Permission denied: './t2_contrast_1_0.nii.gz'
FAILED testing/cli/test_cli_sct_apply_transfo.py::test_sct_apply_transfo_output_image_attributes[template/template/PAM50_small_t2.nii.gz-t2/t2.nii.gz-t2/warp_template2anat.nii.gz-PAM50_small_t2_reg.nii-remaining_args0] - FileNotFoundError: No such file or no access: '$SCT_DIR/data/sct_testing_data/PAM50_small_t2_reg.nii'
FAILED testing/cli/test_cli_sct_apply_transfo.py::test_sct_apply_transfo_output_image_attributes[template/template/PAM50_small_t2.nii.gz-t2/t2.nii.gz-t2/warp_template2anat.nii.gz-PAM50_small_t2_reg-crop1.nii-remaining_args1] - FileNotFoundError: No such file or no access: '$SCT_DIR/data/sct_testing_data/PAM50_small_t2_reg-crop1.nii'
FAILED testing/cli/test_cli_sct_apply_transfo.py::test_sct_apply_transfo_output_image_attributes[template/template/PAM50_small_t2.nii.gz-t2/t2.nii.gz-t2/warp_template2anat.nii.gz-PAM50_small_t2_reg-crop2.nii-remaining_args2] - FileNotFoundError: No such file or no access: '$SCT_DIR/data/sct_testing_data/PAM50_small_t2_reg-crop2.nii'
FAILED testing/cli/test_cli_sct_apply_transfo.py::test_sct_apply_transfo_output_image_attributes[template/template/PAM50_small_t2.nii.gz-t2/t2.nii.gz-t2/warp_template2anat.nii.gz-PAM50_small_t2_reg-concatWarp.nii-remaining_args3] - FileNotFoundError: No such file or no access: '$SCT_DIR/data/sct_testing_data/PAM50_small_t2_reg-concatWarp.nii'
FAILED testing/cli/test_cli_sct_apply_transfo.py::test_sct_apply_transfo_output_image_attributes[template/template/PAM50_small_t2.nii.gz-dmri/dmri.nii.gz-t2/warp_template2anat.nii.gz-PAM50_small_t2_reg-4Dref.nii-remaining_args4] - FileNotFoundError: No such file or no access: '$SCT_DIR/data/sct_testing_data/PAM50_small_t2_reg-4Dref.nii'
FAILED testing/cli/test_cli_sct_apply_transfo.py::test_sct_apply_transfo_output_image_attributes[dmri/dmri.nii.gz-t2/t2.nii.gz-mt/warp_t22mt1.nii.gz-PAM50_small_t2_reg-4Din.nii-remaining_args5] - PermissionError: [Errno 13] Permission denied: 'PAM50_small_t2_reg-4Din.nii'
FAILED testing/cli/test_cli_sct_compute_ernst_angle.py::test_sct_compute_ernst_angle_value_against_groundtruth - PermissionError: [Errno 13] Permission denied: 'ernst_angle.txt'
FAILED testing/cli/test_cli_sct_compute_hausdorff_distance.py::test_sct_compute_hausdorff_distance_null_values - PermissionError: [Errno 13] Permission denied: '$SCT_DIR/data/sct_testing_data/t2s_gmseg_manual_thinned.nii.gz'
FAILED testing/cli/test_cli_sct_concat_transfo.py::test_sct_concat_transfo_no_checks - ValueError: Warping field was not generated! output_image_filename: warp_final.nii.gz
FAILED testing/cli/test_cli_sct_convert.py::test_sct_convert_output_file_exists - PermissionError: [Errno 13] Permission denied: '$SCT_DIR/data/sct_testing_data/t2.nii'
FAILED testing/cli/test_cli_sct_create_mask.py::test_sct_create_mask_no_checks[mt/mt1.nii.gz-coord,15x17-10] - PermissionError: [Errno 13] Permission denied: '$SCT_DIR/data/sct_testing_data/mask_mt1.nii.gz'
FAILED testing/cli/test_cli_sct_create_mask.py::test_sct_create_mask_no_checks[mt/mt1.nii.gz-point,mt/mt1_point.nii.gz-10] - PermissionError: [Errno 13] Permission denied: '$SCT_DIR/data/sct_testing_data/mask_mt1.nii.gz'
FAILED testing/cli/test_cli_sct_create_mask.py::test_sct_create_mask_no_checks[mt/mt1.nii.gz-center-10] - PermissionError: [Errno 13] Permission denied: '$SCT_DIR/data/sct_testing_data/mask_mt1.nii.gz'
FAILED testing/cli/test_cli_sct_create_mask.py::test_sct_create_mask_no_checks[mt/mt1.nii.gz-centerline,mt/mt1_seg.nii.gz-5] - PermissionError: [Errno 13] Permission denied: '$SCT_DIR/data/sct_testing_data/mask_mt1.nii.gz'
FAILED testing/cli/test_cli_sct_create_mask.py::test_sct_create_mask_no_checks[dmri/dmri.nii.gz-center-10] - PermissionError: [Errno 13] Permission denied: '$SCT_DIR/data/sct_testing_data/mask_dmri.nii.gz'
FAILED testing/cli/test_cli_sct_crop_image.py::test_sct_crop_image_output_has_expected_dimensions[t2/t2.nii.gz-t2_crop_xyz.nii-remaining_args0-expected_dim0] - PermissionError: [Errno 13] Permission denied: '$SCT_DIR/data/sct_testing_data/t2_crop_xyz.nii'
FAILED testing/cli/test_cli_sct_crop_image.py::test_sct_crop_image_output_has_expected_dimensions[t2/t2.nii.gz-t2_crop_mask.nii-remaining_args1-expected_dim1] - PermissionError: [Errno 13] Permission denied: '$SCT_DIR/data/sct_testing_data/t2_crop_mask.nii'
FAILED testing/cli/test_cli_sct_crop_image.py::test_sct_crop_image_output_has_expected_dimensions[t2/t2.nii.gz-t2_crop_ref.nii-remaining_args2-expected_dim2] - PermissionError: [Errno 13] Permission denied: '$SCT_DIR/data/sct_testing_data/t2_crop_ref.nii'
FAILED testing/cli/test_cli_sct_crop_image.py::test_sct_crop_image_output_has_expected_dimensions[t2/t2.nii.gz-t2_crop_dilate_xyz.nii-remaining_args3-expected_dim3] - PermissionError: [Errno 13] Permission denied: '$SCT_DIR/data/sct_testing_data/t2_crop_dilate_xyz.nii'
FAILED testing/cli/test_cli_sct_crop_image.py::test_sct_crop_image_output_has_expected_dimensions[t2/t2.nii.gz-t2_crop_dilate_mask.nii-remaining_args4-expected_dim4] - PermissionError: [Errno 13] Permission denied: '$SCT_DIR/data/sct_testing_data/t2_crop_dilate_mask.nii'
FAILED testing/cli/test_cli_sct_crop_image.py::test_sct_crop_image_output_has_expected_dimensions[t2/t2.nii.gz-t2_crop_dilate_ref.nii-remaining_args5-expected_dim5] - PermissionError: [Errno 13] Permission denied: '$SCT_DIR/data/sct_testing_data/t2_crop_dilate_ref.nii'
FAILED testing/cli/test_cli_sct_deepseg_gm.py::test_sct_deepseg_gm_check_dice_coefficient_against_groundtruth - PermissionError: [Errno 13] Permission denied: 'output.nii.gz'
FAILED testing/cli/test_cli_sct_deepseg_lesion.py::test_sct_deepseg_lesion_no_checks - PermissionError: [Errno 13] Permission denied: '$SCT_DIR/data/sct_testing_data/t2/t2_res_RPI_seg.nii.gz'
FAILED testing/cli/test_cli_sct_deepseg_sc.py::test_sct_deepseg_sc_qc_report_exists - PermissionError: [Errno 13] Permission denied: '$SCT_DIR/data/sct_testing_data/t2_seg.nii.gz'
FAILED testing/cli/test_cli_sct_denoising_onlm.py::test_sct_denoising_onlm_no_checks - PermissionError: [Errno 13] Permission denied: 't2_denoised.nii.gz'
FAILED testing/cli/test_cli_sct_detect_pmj.py::test_sct_detect_pmj_check_euclidean_distance_against_groundtruth - PermissionError: [Errno 13] Permission denied: '/tmp/sct_2023-04-04_15-19-18_detect-pmj_ivtfjt59/PAM50_small_t2.nii.gz'
FAILED testing/cli/test_cli_sct_dice_coefficient.py::test_sct_dice_coefficient_check_output_against_groundtruth - PermissionError: [Errno 13] Permission denied: '/tmp/sct_2023-04-04_15-19-18_dice-coefficient__063is41/tmp2_t2_seg-manual.nii.gz'
FAILED testing/cli/test_cli_sct_dmri_compute_dti.py::test_sct_dmri_compute_dti_no_checks - PermissionError: [Errno 13] Permission denied: '$SCT_DIR/data/sct_testing_data/dti_FA.nii.gz'
FAILED testing/cli/test_cli_sct_dmri_concat_b0_and_dwi.py::test_sct_dmri_concat_b0_and_dwi_no_checks - PermissionError: [Errno 13] Permission denied: '$SCT_DIR/data/sct_testing_data/b0_dwi_concat.nii'
FAILED testing/cli/test_cli_sct_dmri_concat_bvals.py::test_sct_dmri_concat_bvals_no_checks - PermissionError: [Errno 13] Permission denied: 'bvals_concat.txt'
FAILED testing/cli/test_cli_sct_dmri_concat_bvecs.py::test_sct_dmri_concat_bvecs_no_checks - PermissionError: [Errno 13] Permission denied: 'bvecs_concat.txt'
FAILED testing/cli/test_cli_sct_dmri_separate_b0_and_dwi.py::test_sct_dmri_separate_b0_and_dwi_image_data_within_threshold - PermissionError: [Errno 13] Permission denied: '$SCT_DIR/data/sct_testing_data/dmri_b0.nii.gz'
FAILED testing/cli/test_cli_sct_dmri_transpose_bvecs.py::test_sct_dmri_transpose_bvecs_no_checks - PermissionError: [Errno 13] Permission denied: 'bvecs.txt'
FAILED testing/cli/test_cli_sct_extract_metric.py::test_sct_extract_metric_against_groundtruth - PermissionError: [Errno 13] Permission denied: 'quantif_mtr.csv'
FAILED testing/cli/test_cli_sct_flatten_sagittal.py::test_sct_flatten_sagittal_no_checks - PermissionError: [Errno 13] Permission denied: '$SCT_DIR/data/sct_testing_data/t2/t2_flatten.nii.gz'
FAILED testing/cli/test_cli_sct_fmri_compute_tsnr.py::test_sct_sct_fmri_compute_tsnr_no_checks - PermissionError: [Errno 13] Permission denied: '$SCT_DIR/data/sct_testing_data/out_fmri_tsnr.nii.gz'
FAILED testing/cli/test_cli_sct_fmri_moco.py::test_sct_fmri_moco_no_checks - PermissionError: [Errno 13] Permission denied: '$SCT_DIR/data/sct_testing_data/fmri_r_moco.nii.gz'
FAILED testing/cli/test_cli_sct_get_centerline.py::test_sct_get_centerline_output_file_exists - PermissionError: [Errno 13] Permission denied: '$SCT_DIR/data/sct_testing_data/t2s/t2s_centerline.nii.gz'
FAILED testing/cli/test_cli_sct_get_centerline.py::test_sct_get_centerline_output_file_exists_with_o_arg[] - AssertionError: assert False
FAILED testing/cli/test_cli_sct_get_centerline.py::test_sct_get_centerline_output_file_exists_with_o_arg[.nii.gz] - AssertionError: assert False
FAILED testing/cli/test_cli_sct_image.py::test_sct_image_pad - PermissionError: [Errno 13] Permission denied: '$SCT_DIR/data/sct_testing_data/sct_image_out.nii.gz'
FAILED testing/cli/test_cli_sct_image.py::test_sct_image_display_warp_check_output_exists - FileNotFoundError: No such file or no access: '$SCT_DIR/data/sct_testing_data/t2/grid_3_resample_warp_template2anat.nii.gz'
FAILED testing/cli/test_cli_sct_label_utils.py::test_sct_label_utils_cubic_to_point - PermissionError: [Errno 13] Permission denied: '$SCT_DIR/data/sct_testing_data/test_centerofmass.nii.gz'
FAILED testing/cli/test_cli_sct_label_utils.py::test_sct_label_utils_create - PermissionError: [Errno 13] Permission denied: '$SCT_DIR/data/sct_testing_data/labels.nii.gz'
FAILED testing/cli/test_cli_sct_label_vertebrae.py::test_sct_label_vertebrae_initfile_qc_no_checks - PermissionError: [Errno 13] Permission denied: 'testing-qc'
FAILED testing/cli/test_cli_sct_maths.py::test_sct_maths_percent_no_checks - PermissionError: [Errno 13] Permission denied: '$SCT_DIR/data/sct_testing_data/test.nii.gz'
FAILED testing/cli/test_cli_sct_maths.py::test_sct_maths_add_integer_no_checks - PermissionError: [Errno 13] Permission denied: '$SCT_DIR/data/sct_testing_data/test.nii.gz'
FAILED testing/cli/test_cli_sct_merge_images.py::test_sct_merge_images_no_checks - PermissionError: [Errno 13] Permission denied: '$SCT_DIR/data/sct_testing_data/merged_images.nii.gz'
FAILED testing/cli/test_cli_sct_process_segmentation.py::test_sct_process_segmentation_no_checks - PermissionError: [Errno 13] Permission denied: '$SCT_DIR/data/sct_testing_data/csa.csv'
FAILED testing/cli/test_cli_sct_propseg.py::test_sct_propseg_check_dice_coefficient_against_groundtruth - PermissionError: [Errno 13] Permission denied: 'testing-qc'
FAILED testing/cli/test_cli_sct_qc.py::test_sct_qc_no_checks - PermissionError: [Errno 13] Permission denied: './qc'
FAILED testing/cli/test_cli_sct_register_to_template.py::test_sct_register_to_template_dice_coefficient_against_groundtruth[template/template/PAM50_small_cord.nii.gz-remaining_args0] - PermissionError: [Errno 13] Permission denied: 'qc-testing'
FAILED testing/cli/test_cli_sct_register_to_template.py::test_sct_register_to_template_dice_coefficient_against_groundtruth[$SCT_DIR/data/PAM50/template/PAM50_cord.nii.gz-remaining_args1] - PermissionError: [Errno 13] Permission denied: 'warp_template2anat.nii.gz'
FAILED testing/cli/test_cli_sct_smooth_spinalcord.py::test_sct_smooth_spinalcord_check_output_files - PermissionError: [Errno 13] Permission denied: '$SCT_DIR/data/sct_testing_data/straightening.cache'
FAILED testing/cli/test_cli_sct_straighten_spinalcord.py::test_sct_straighten_spinalcord_no_checks - PermissionError: [Errno 13] Permission denied: './warp_curve2straight.nii.gz'
FAILED testing/cli/test_cli_sct_warp_template.py::test_sct_warp_template_no_checks - PermissionError: [Errno 13] Permission denied: 'testing-qc'
FAILED testing/cli/test_sct_deepseg.py::test_install_model - PermissionError: [Errno 13] Permission denied: 't2star_sc.pt'
ERROR testing/cli/test_cli_sct_dmri_moco.py::test_sct_dmri_moco_sagittal_no_checks - PermissionError: [Errno 13] Permission denied: '$SCT_DIR/data/sct_testing_data/dmri/dmri_AIL.nii'
============================================== 62 failed, 355 passed, 11 skipped, 66876 warnings, 1 error in 176.62s (0:02:56) ==============================================
I think the proper fix is to run each test in a temporary directory, rather than in
data/sct_testing_data.
By using an autouse fixture that calls Pytest's tmp_path, each test gets its own output directory, which results in lovely organization for free:
Plus, all of our existing usages of tmp_path can remain unchanged, because both the cd fixture and the test itself refer to the same tmp_path location per-test.
(The only tedious thing will be updating all of the relative input data paths (t2/t2.nii.gz) with explicit paths to the testing directory (sct_test_path('t2', 't2.nii.gz'). But, that was kind of an inevitability with a task like this...)
After a successful run of
pytestI noticed that 172 files were created in my working directory. Not a big deal, but it would be nice to cleanup after running the tests.