-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Add an option to not delete TemporaryFile on dealloc #1625
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
Conversation
file.fileHandle.seek(toFileOffset: 0) | ||
// Read the contents. | ||
let contents = try? file.fileHandle.readFileContents() | ||
XCTAssertEqual(contents, "foobarbaz") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test could be simplified I think. We can write data in the do block and then just read it outside the block using the FileSystem
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm I kept consistent with the other tests in this file, do you think they should all be changed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there was a reason to do that for other tests but anyway it was just a nitpick. LGTM!
filePath = file.path | ||
} | ||
// File should not be deleted. | ||
XCTAssertTrue(isFile(filePath)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should also remove the temporary file before exiting this test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch, fixed.
@swift-ci please smoke test |
CI green \o/ |
It's sometimes useful to create a temp file but keep it around for a little bit longer, instead of automatically deleting it on dealloc.
TemporaryDirectory
already has a similar option.