Test file copying preserves mtime #335
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Adds a test to ensure that a file copied via the AppCache feature preserves the file's original mtime. In #302 I determined that as long as the files copied into the cache dir preserve the original mtime then the FIFO cache cleaning logic that we rely on is valid. This test closes out this comment that asserted we need to verify whether or not the buildpack preserved mtime when copying files #302 (comment). [#close 302]
schneems
force-pushed
the
schneems/assert-mtime-preserved-when-copying
branch
from
a6f802d
to
77fca26
Compare
I previously introduced a commit that had a test intended to prove that we were persisting the mtime of files copied into the cache to close out this comment #302 (comment). The test passed locally, but failed on CI. It appears the behavior of fs-extra with regards to mtime for copying/moving directories is system dependent. To compensate, I switched to `cp_r` which explicitly is designed to preserve mtime https://crates.io/crates/cp_r. It has ~27,000 lifetime downloads and has activity as recently as a 9 days ago. https://docs.rs/cp_r/latest/cp_r/#release-history Copying and preserving mtime is the primary reason this library exists. From the docs.rs: > Copy a directory tree, including mtimes and permissions. It does not have the ability to skip-overwriting existing files which I need, however it has a generic `filter` closure which allows me to assemble this behavior myself.
schneems
commented
Oct 16, 2024
|
|
||
| store.load().unwrap(); | ||
|
|
||
| let metadata = fs_err::metadata(store.cache.join("lol.txt")).unwrap(); |
There was a problem hiding this comment.
Should be store.path and not store.cache
|
Cleaned up and re-opened #336 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adds a test to ensure that a file copied via the AppCache feature preserves the file's original mtime.
In #302 I determined that as long as the files copied into the cache dir preserve the original mtime then the FIFO cache cleaning logic that we rely on is valid. This test closes out this comment that asserted we need to verify whether or not the buildpack preserved mtime when copying files #302 (comment).
#close 302
Commits are in historical order. I'm interested in feedback regarding tests, possibly some edge cases I've overlooked regarding test cases. app_cache.rs is approx 213 lines without comments or tests.