Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BUG: Use unique filenames for Nifti tests #4659

Merged

Conversation

thewtex
Copy link
Member

@thewtex thewtex commented May 13, 2024

When the tests are run in parallel they can try to write to or remove
the same file from different processes, causing intermittent failures.

Also add const where it is missing.

Also use full paths as tests inputs, instead of changing directories,
which is more explicit.

@github-actions github-actions bot added type:Bug Inconsistencies or issues which will cause an incorrect result under some or all circumstances type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots type:Testing Ensure that the purpose of a class is met/the results on a wide set of test cases are correct area:IO Issues affecting the IO module labels May 13, 2024
@thewtex
Copy link
Member Author

thewtex commented May 13, 2024

@cookpa please take a look

@thewtex thewtex added this to the ITK 5.4.0 milestone May 13, 2024
Copy link
Member

@jhlegarreta jhlegarreta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if the output NIfTI files can be compressed (*.nii.gz) directly to save space, but approving.

@thewtex thewtex requested a review from N-Dekker May 13, 2024 12:56
@cookpa
Copy link
Contributor

cookpa commented May 13, 2024

Thanks for tracking this down @thewtex . The fixes look good to me.

@thewtex
Copy link
Member Author

thewtex commented May 13, 2024

NIfTI files can be compressed (*.nii.gz) directly to save space, but approving.

Yes, this works locally -- updating.

When the tests are run in parallel they can try to write to or remove
the same file from different processes, causing intermittent failures.

Also add `const` where it is missing.

Also use full paths as tests inputs, instead of changing directories,
which is more explicit.
@hjmjohnson
Copy link
Member

@thewtex THANK YOU!

@thewtex thewtex merged commit 15b98c3 into InsightSoftwareConsortium:master May 13, 2024
14 checks passed
@thewtex thewtex deleted the nifti-unique-filenames branch May 13, 2024 18:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:IO Issues affecting the IO module type:Bug Inconsistencies or issues which will cause an incorrect result under some or all circumstances type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots type:Testing Ensure that the purpose of a class is met/the results on a wide set of test cases are correct
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants