Skip to content
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 PathBuf Arbitrary impl with tests #368

Merged
merged 2 commits into from
Sep 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
112 changes: 110 additions & 2 deletions proptest/src/arbitrary/_std/path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,121 @@

use std::path::*;

// TODO: Figure out PathBuf and then Box/Rc/Box<Path>.
use crate::{
arbitrary::{SMapped, StrategyFor},
path::PathParams,
prelude::{any, any_with, Arbitrary, Strategy},
std_facade::{string::ToString, Arc, Box, Rc, String, Vec},
strategy::{statics::static_map, MapInto},
};

arbitrary!(StripPrefixError; Path::new("").strip_prefix("a").unwrap_err());

/// A private type (not actually pub) representing the output of [`PathParams`] that can't be
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

non-blocking: it is actually pub(?) not sure if this just means it isn't intended to be publicly used or if this is residual from previous iterations.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I meant that even though it is pub, it is not exported as an API so that backwards compatibility is maintained. Any suggestions for better phrasing?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think its a huge deal either way. I would probably just say something like:
"An opaque type representing the output of ...

/// referred to by API users.
///
/// The goal of this type is to encapsulate the output of `PathParams`. If this layer weren't
/// present, the type of `<PathBuf as Arbitrary>::Strategy` would be `SMapped<(bool, Vec<String>),
/// Self>`. This is a problem because it exposes the internal representation of `PathParams` as an
/// API. For example, if an additional parameter of randomness (e.g. another bool) were added, the
/// type of `Strategy` would change.
///
/// With `PathParamsOutput`, the type of `Strategy` is `SMapped<PathParamsOutput, Self>`, which is a
/// type that can't be named directly---only via `<PathBuf as Arbitrary>::Strategy`. The internal
/// representation of `PathParams` can be changed without affecting the API.
#[derive(Debug)]
pub struct PathParamsOutput {
is_absolute: bool,
components: Vec<String>,
}

impl Arbitrary for PathParamsOutput {
type Parameters = PathParams;
type Strategy = SMapped<(bool, Vec<String>), Self>;

fn arbitrary_with(args: Self::Parameters) -> Self::Strategy {
static_map(
(
any::<bool>(),
any_with::<Vec<String>>((
args.components(),
args.component_regex(),
)),
),
|(is_absolute, components)| Self {
is_absolute,
components,
},
)
}
}

/// This implementation accepts as its argument a [`PathParams`] struct. It generates either a
/// relative or an absolute path with equal probability.
///
/// Currently, this implementation does not generate:
///
/// * Paths that are not valid UTF-8 (this is unlikely to change)
/// * Paths with a [`PrefixComponent`](std::path::PrefixComponent) on Windows, e.g. `C:\` (this may
/// change in the future)
impl Arbitrary for PathBuf {
type Parameters = PathParams;
type Strategy = SMapped<PathParamsOutput, Self>;

fn arbitrary_with(args: Self::Parameters) -> Self::Strategy {
static_map(
any_with::<PathParamsOutput>(args),
|PathParamsOutput {
is_absolute,
components,
}| {
let mut out = PathBuf::new();
if is_absolute {
out.push(&MAIN_SEPARATOR.to_string());
}

for component in components {
// If a component has an embedded / (or \ on Windows), remove it from the
// string.
let component = component
.chars()
.filter(|&c| !std::path::is_separator(c))
.collect::<String>();
out.push(&component);
}

out
},
)
}
}

macro_rules! dst_wrapped {
($($w: ident),*) => {
$(
/// This implementation is identical to [the `Arbitrary` implementation for
/// `PathBuf`](trait.Arbitrary.html#impl-Arbitrary-for-PathBuf).
impl Arbitrary for $w<Path> {
type Parameters = PathParams;
type Strategy = MapInto<StrategyFor<PathBuf>, Self>;

fn arbitrary_with(args: Self::Parameters) -> Self::Strategy {
any_with::<PathBuf>(args).prop_map_into()
}
}
)*
}
}

dst_wrapped!(Box, Rc, Arc);

#[cfg(test)]
mod test {
no_panic_test!(
strip_prefix_error => StripPrefixError
strip_prefix_error => StripPrefixError,
path_buf => PathBuf,
box_path => Box<Path>,
rc_path => Rc<Path>,
arc_path => Arc<Path>
);
}
3 changes: 3 additions & 0 deletions proptest/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,9 @@ pub mod test_runner;
pub mod tuple;

pub mod option;
#[cfg(feature = "std")]
#[cfg_attr(docsrs, doc(cfg(feature = "std")))]
pub mod path;
pub mod result;
pub mod sample;
#[cfg(feature = "std")]
Expand Down
55 changes: 55 additions & 0 deletions proptest/src/path.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
//! Strategies for generating [`PathBuf`] and related path types.
//!
//! [`PathParams`] in this module is used as the argument to the
//! [`Arbitrary`](crate::arbitrary::Arbitrary) implementation for [`PathBuf`].

use crate::{collection::SizeRange, string::StringParam};

/// Parameters for the [`Arbitrary`] implementation for [`PathBuf`].
///
/// By default, this generates paths with 0 to 8 components uniformly at random, each of which is a
/// default [`StringParam`].
#[derive(Clone, Debug, PartialEq, Eq, Hash)]
pub struct PathParams {
/// The number of components in the path.
components: SizeRange,
/// The regular expression to generate individual components.
component_regex: StringParam,
}

impl PathParams {
/// Gets the number of components in the path.
pub fn components(&self) -> SizeRange {
self.components.clone()
}

/// Sets the number of components in the path.
pub fn with_components(mut self, components: impl Into<SizeRange>) -> Self {
self.components = components.into();
self
}

/// Gets the regular expression to generate individual components.
pub fn component_regex(&self) -> StringParam {
self.component_regex
}

/// Sets the regular expression to generate individual components.
pub fn with_component_regex(
mut self,
component_regex: impl Into<StringParam>,
) -> Self {
self.component_regex = component_regex.into();
self
}
}

impl Default for PathParams {
fn default() -> Self {
Self {
components: (0..8).into(),
// This is the default regex for `any::<String>()`.
component_regex: StringParam::default(),
}
}
}
2 changes: 1 addition & 1 deletion proptest/src/string.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ use crate::test_runner::*;

/// Wraps the regex that forms the `Strategy` for `String` so that a sensible
/// `Default` can be given. The default is a string of non-control characters.
#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
#[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
pub struct StringParam(&'static str);

impl From<StringParam> for &'static str {
Expand Down