-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
feat: add more fs cheatcodes #4803
Conversation
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.
File.t.sol -> Fs.t.sol
testdata/cheats/Fs.t.sol
Outdated
// string memory path = "../testdata/fixtures/Dir"; | ||
|
||
// { | ||
// Cheats.DirEntry[] memory entries = cheats.readDir(path); |
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.
Just a single call fails with "allocation overflow", any ideas why?
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.
remembered this issue:
same error but turned out to be abi decoding related
but perhaps configured memory limit?
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.
Thanks, it was an ABI encoding error: 66723e4
Really weird that it throws a "memory overflow" tho
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.
yeh, perhaps our mapping of the internal solidity error is not entirely accurate
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 is awesome!
codewise looks great, test look good.
suggestion re file layout, and failing test
createDir(string, bool) | ||
removeDir(string, bool) | ||
readDir(string)(DirEntry[]) | ||
readDir(string, uint64)(DirEntry[]) | ||
readDir(string, uint64, bool)(DirEntry[]) | ||
readLink(string)(string) |
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.
very useful imo
/// Creates a new, empty directory at the provided path. | ||
/// | ||
/// If `recursive` is true, it will also create all the parent directories if they don't exist. | ||
/// | ||
/// # Errors | ||
/// | ||
/// This function will return an error in the following situations, but is not limited to just these | ||
/// cases: | ||
/// | ||
/// - User lacks permissions to modify `path`. | ||
/// - A parent of the given path doesn't exist and `recursive` is false. | ||
/// - `path` already exists and `recursive` is false. | ||
fn create_dir(state: &Cheatcodes, path: impl AsRef<Path>, recursive: bool) -> Result<Bytes, Bytes> { | ||
let path = | ||
state.config.ensure_path_allowed(path, FsAccessKind::Write).map_err(error::encode_error)?; | ||
if recursive { fs::create_dir_all(path) } else { fs::create_dir(path) } | ||
.map(|()| Bytes::new()) | ||
.map_err(error::encode_error) | ||
} | ||
|
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 expect fs
to grow eventually so would like to move all of them to new file fs.rs
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.
done in f9eb363
testdata/cheats/Fs.t.sol
Outdated
// string memory path = "../testdata/fixtures/Dir"; | ||
|
||
// { | ||
// Cheats.DirEntry[] memory entries = cheats.readDir(path); |
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.
remembered this issue:
same error but turned out to be abi decoding related
but perhaps configured memory limit?
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 is great!
needs forge-std and book followups
Motivation
Fixes #4797
Added to
Cheats.sol
:TODO: read dir test fails due to memory allocation overflow, I'm not sure why
Solution