A-1376 part 3: e2e test for filesystem integrity#4011
Conversation
|
On it! Follow along with my review here: https://buildkite.com/buildkite/buildsworth-review-pull-request/builds/811 |
![buildsworth-bk-app[bot]](https://avatars.githubusercontent.com/in/3010600?s=60&v=4){kind=small}
There was a problem hiding this comment.
I found one coverage gap against the stated filesystem cases: the home target is using tilde expansion rather than a true absolute path, so I don't think the absolute-path e2e coverage is there yet.
Want to dig deeper? The full session log is attached to this Buildkite build. Download the session file and open a new pi session with it:
Download the buildsworth logs from build 811, then answer my questions about the findings.
| - { env: BUILDKITE_BUILD_ID } | ||
| target_paths: | ||
| - fs-test | ||
| - ~/cache-abs-fs |
There was a problem hiding this comment.
Blocking: This target doesn't exercise the true absolute-path case the PR/issue says this e2e covers. In the cache archive code, ~/... is handled by the tilde-expansion branch before the filepath.IsAbs(path) branch, so this would still pass without covering a literal $HOME/... target path. Could the fixture write the resolved $HOME/cache-abs-fs path into cache.yml for both save and restore?
zhming0
force-pushed
the
ming/a-1376-part-3
branch
from
6877b34 to
1efd0dd
Compare
|
@buildsworth-bk review again? |
|
On it! Follow along with my review here: https://buildkite.com/buildkite/buildsworth-review-pull-request/builds/816 |
buildsworth-bk-app
Bot
dismissed
their stale review
Previous concerns have been addressed.
There was a problem hiding this comment.
Previous concern is resolved: the cache fixture now writes a resolved $HOME path into cache.yml, so the target path is literal absolute rather than tilde-expanded. This is a test-fixture-only change, so I read it as L1.
Want to dig deeper? The full session log is attached to this Buildkite build. Download the session file and open a new pi session with it:
Download the buildsworth logs from build 816, then answer my questions about the findings.
1 participant
Description
Add end to end test coverage for fs integrity after cache restore, it was an issue in v1
Context
part of A-1376
Disclosures / Credits
Clanker and minion.