Skip to content

Conversation

emaxx-google
Copy link
Collaborator

@emaxx-google emaxx-google commented Aug 22, 2025

Instead of directly writing to the test case, write first to a temporary file and then rename it. This is atomic on popular file systems.

Also refactor the code to have a dedicated fileutil helper, and cover it with unit tests. It'll make the upcoming support of directory inputs in #301 slightly cleaner.

Instead of directly writing to the test case, write first to a temporary
file and then rename it. This is atomic on popular file systems.

Also refactor the code to have a dedicated fileutil helper, and cover it
with unit tests.
@emaxx-google emaxx-google requested a review from VitaNuo August 22, 2025 11:39
Copy link
Collaborator

@VitaNuo VitaNuo left a comment

Choose a reason for hiding this comment

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

Thanks!

@emaxx-google
Copy link
Collaborator Author

After playing more with this, I've discovered the previous solution with CloseableTemporaryFile wasn't fully robust. It took me some time to implement a test to relatively reliably catch concurrency issues, and it was fun to see how it breaks many of my attempts... The new version seems to be actually more robust.

@emaxx-google emaxx-google merged commit a3b6497 into marxin:master Aug 25, 2025
10 checks passed
@emaxx-google emaxx-google deleted the atomic-test-update branch August 25, 2025 11:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants