-
-
Notifications
You must be signed in to change notification settings - Fork 16
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
Add (optional, feature-gated) proptest::Arbitrary implementation #18
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.
Thank you for the contribution! I love proptest and I'm happy to see camino gain support for it.
Cargo.toml
Outdated
proptest = { version = "1.0.0", optional = true } | ||
serde = { version = "1", features = ["derive"], optional = true } | ||
|
||
[features] | ||
serde1 = ["serde"] | ||
proptest = ["dep:proptest"] |
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 won't work with older versions of Cargo, which camino supports. I'd recommend calling the feature proptest1
to match serde1
.
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
impl Arbitrary for Utf8PathBuf { | ||
type Parameters = <String as Arbitrary>::Parameters; | ||
type Strategy = MapInto<StrategyFor<String>, Self>; | ||
fn arbitrary_with(args: Self::Parameters) -> Self::Strategy { | ||
any_with::<String>(args).prop_map_into() | ||
} | ||
} | ||
|
||
impl Arbitrary for Box<Utf8Path> { | ||
type Parameters = <Utf8PathBuf as Arbitrary>::Parameters; | ||
type Strategy = MapInto<StrategyFor<Utf8PathBuf>, Self>; | ||
fn arbitrary_with(args: Self::Parameters) -> Self::Strategy { | ||
any_with::<Utf8PathBuf>(args).prop_map_into() | ||
} | ||
} |
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 doesn't seem like a very representative generation strategy since most strings won't have slashes in them -- seems like it should instead produce path components and then join them according to the platform's rules.
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.
That's something I would generally leave to individual library/application testers to provide, rather than assuming their data for them. It's also something that could be added on here in the future as a default if there's demand for it. These are essentially copies of the OsString
and Box<OsStr>
impls; anything further is outside my skillset as far as implementing Arbitrary
.
Thanks -- I think I'm going to land this and then iterate on it locally. |
This is out as part of camino 1.0.8 and above (just released 1.0.9 with some documentation fixes). |
Awesome. Thanks! Just to provide a little extra flavor about my use case, I'm using In particular, paths in SFTP are specified to be UTF-8 unless a protocol extension says otherwise. I'm taking an opinionated stance and not supporting any protocol extensions of that nature :) |
This makes it easier to proptest code that makes use of
camino