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

Finish [SuppressTempFileChecks] attribute functionality, #898 #1084

Merged
merged 3 commits into from
Jan 8, 2025

Conversation

paulirwin
Copy link
Contributor

  • You've read the Contributor Guide and Code of Conduct.
  • You've included unit or integration tests for your change, where applicable.
  • You've included inline docs for your change, where applicable.
  • There's an open issue for the PR that you are making. If you'd like to propose a change, please open an issue to discuss the change or find an existing issue.

Finishes implementation of the [SuppressTempFileChecks] attribute functionality.

Fixes #898

Description

This attribute will cause the tear down logic to swallow any exceptions when cleaning up temporary files after the tests run. No tests actually need this attribute, but theoretically it is possible that a TestFramework user might want this.

This also fixes an issue where the previous code was treating all tests as if it had this attribute, and was not throwing if removing the temporary files failed. This change will make our tests more reliable by ensuring they fail if temp file handles are held during cleanup.

I'm also open to removing this attribute, since none of our tests need it and latest Lucene doesn't use it internally either. It could be viewed as a bad practice to just ignore these files being held too long.

@paulirwin paulirwin added the notes:bug-fix Contains a fix for a bug label Jan 2, 2025
@paulirwin paulirwin requested a review from NightOwl888 January 2, 2025 04:40
@paulirwin paulirwin requested a review from NightOwl888 January 5, 2025 00:22
@paulirwin paulirwin marked this pull request as draft January 5, 2025 22:03
… false, which will cause all additional tests keep temporary files on disk
@NightOwl888 NightOwl888 marked this pull request as ready for review January 8, 2025 20:55
@NightOwl888 NightOwl888 merged commit c3b6896 into apache:master Jan 8, 2025
265 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
notes:bug-fix Contains a fix for a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Task: Finish [SuppressTempFileChecks] attribute functionality
2 participants