Refactor directory paths #39
Conversation
|
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
oltrep
force-pushed
the
directory-paths
branch
from
c05ca04
to
126be72
Compare
| thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Error { inner: ErrorInner { kind: Custom, line: None, col: 0, at: None, message: "missing field `tools`", key: [] } }', src/config.rs:97:69 | ||
| note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace |
There was a problem hiding this comment.
A note on this test (and tests/snapshots/install_invalid_tool_configuration.snap): I've let these in even if they're not great just to document the current behavior. I have some changes locally that I'll put up once this merges that refactor the error handling to get better output.
src/tool_cache.rs
Outdated
| .args(args) | ||
| .status() | ||
| .map_err(|e| { | ||
| format!( | ||
| "an error happened trying to run `{}` at `{}`: {}", |
There was a problem hiding this comment.
When does this error occur? If there's any chance it could happen for end users, we should make sure the error is very clear about it being foreman's problem, so they don't chase ghosts in whatever tool they were trying to use
There was a problem hiding this comment.
Yes you're right, I'll precise that in the message!
I refactored how the paths for the different files and directories are obtained, so that it's possible to specify the root from an environment variable. This made it simple to write to integration tests on the CLI that snapshots the content of file generated by foreman.
I don't want to go too deep in the test cases for this PR, but this sets up the pattern.
Closes #17