convert relative paths to forward slashes on Windows

A breaking change, but I think bringing Windows in line with Unix is valuable.

Also fix up other path-related documentation, and add tests for everything.

Author: sunshowers

README.md

@@ -21,8 +21,10 @@ Given:
 `datatest-stable` will call the `my_test` function once per matching file in
 the directory. Directory traversals are recursive.
 
-`datatest-stable` works with [cargo nextest](https://nexte.st/), and is part of the [nextest-rs
-organization](https://github.com/nextest-rs/) on GitHub.
+`datatest-stable` works with [cargo nextest](https://nexte.st/), and is part
+of the [nextest-rs organization](https://github.com/nextest-rs/) on GitHub.
+With nextest, each test case is represented as a separate test, and is run
+as a separate process in parallel.
 
 ## Usage
 
@@ -49,8 +51,10 @@ harness = false
   * `fn(&P, Vec<u8>) -> datatest_stable::Result<()>` where `P` is `Path` or `Utf8Path`. If the
     extra `Vec<u8>` parameter is specified, the contents of the file will be loaded and passed
     in as a `Vec<u8>` (erroring out if that failed).
-* `root` - The path to the root directory where the input files (fixtures) live. This path is
-  relative to the root of the crate (the directory where the crate’s `Cargo.toml` is located).
+* `root` - The path to the root directory where the input files (fixtures)
+  live. This path is relative to the root of the crate (the directory where
+  the crate’s `Cargo.toml` is located). Absolute paths are not supported, and
+  will panic at runtime.
 
   `root` is an arbitrary expression that implements `Display`, such as `&str`, or a
   function call that returns a `Utf8PathBuf`.
@@ -62,11 +66,21 @@ harness = false
   `pattern` is an arbitrary expression that implements `Display`, such as
   `&str`, or a function call that returns a `String`.
 
-The passed-in `Path` and `Utf8Path` are **absolute** paths to the files to be tested.
-
 The three parameters can be repeated if you have multiple sets of data-driven tests to be run:
 `datatest_stable::harness!(testfn1, root1, pattern1, testfn2, root2, pattern2)`.
 
+### Relative paths
+
+The `pattern` argument is tested against the **relative** path of each file,
+**excluding** the `root` prefix – not the absolute path.
+
+The `Path` and `Utf8Path` passed into the test functions are **relative** to
+the root of the crate, obtained by joining `root` to the relative path of
+the file.
+
+For universality, all relative paths use `/` as the path separator,
+including on Windows.
+
 ### Examples
 
 This is an example test. Use it with `harness = false`.
@@ -93,6 +107,11 @@ datatest_stable::harness!(
 );
 ````
 
+If `path/to/fixtures` contains a file `foo/bar.txt`, then:
+
+* The pattern `r"^.*/*"` will match `foo/bar.txt`.
+* `my_test` and `my_test_utf8` will be called with `"path/to/fixtures/foo/bar.txt"`.
+
 ### Embedding directories at compile time
 
 With the `include-dir` feature enabled, you can use the
@@ -140,7 +159,9 @@ datatest_stable::harness!(
 ````
 
 In this case, the passed-in `Path` and `Utf8Path` are **relative** to the
-root of the included directory.
+root of the included directory. Like elsewhere in `datatest-stable`, these
+relative paths always use forward slashes as separators, including on
+Windows.
 
 Because the files don’t exist on disk, the test functions must accept their
 contents as either a `String` or a `Vec<u8>`. If the argument is not
@@ -170,9 +191,11 @@ datatest_stable::harness!(
 );
 ````
 
-In this case, note that `path` will be absolute if `FIXTURES` is a string,
-and relative if `FIXTURES` is a `Dir`. Your test should be prepared to
-handle either case.
+In this case, note that `path` will be relative to the **crate directory**
+(e.g. `tests/files/foo/bar.txt`) if `FIXTURES` is a string, and relative to
+the **include directory** (e.g. `foo/bar.txt`) if `FIXTURES` is a
+[`Dir`](https://docs.rs/include_dir/0.7.4/include_dir/dir/struct.Dir.html). Your test should be prepared to handle either
+case.
 
 ## Features
 

src/data_source.rs

@@ -1,11 +1,12 @@
 // Copyright (c) The datatest-stable Contributors
 // SPDX-License-Identifier: MIT OR Apache-2.0
 
-use camino::{Utf8Path, Utf8PathBuf};
+use camino::{Utf8Component, Utf8Path, Utf8PathBuf};
 
 #[derive(Debug)]
 #[doc(hidden)]
 pub enum DataSource {
+    // This is relative to the crate root, and stored with forward slashes.
     Directory(Utf8PathBuf),
     #[cfg(feature = "include-dir")]
     IncludeDir(std::borrow::Cow<'static, include_dir::Dir<'static>>),
@@ -31,18 +32,23 @@ impl DataSource {
     ///
     /// Used for `--exact` matches.
     pub(crate) fn derive_exact(&self, filter: &str, test_name: &str) -> Option<TestEntry> {
-        let rel_path = filter.strip_prefix(test_name)?.strip_prefix("::")?;
+        // include_dir 0.7.4 returns paths with forward slashes, including on
+        // Windows. But that isn't part of the stable API it seems, so we call
+        // `rel_path_to_forward_slashes` anyway.
+        let rel_path = rel_path_to_forward_slashes(
+            filter.strip_prefix(test_name)?.strip_prefix("::")?.as_ref(),
+        );
         match self {
             DataSource::Directory(path) => Some(TestEntry {
-                source: TestSource::Path(path.join(rel_path)),
-                rel_path: rel_path.into(),
+                source: TestSource::Path(rel_path_to_forward_slashes(&path.join(&rel_path))),
+                rel_path,
             }),
             #[cfg(feature = "include-dir")]
             DataSource::IncludeDir(dir) => {
-                let file = dir.get_file(rel_path)?;
+                let file = dir.get_file(&rel_path)?;
                 Some(TestEntry {
                     source: TestSource::IncludeDir(file),
-                    rel_path: rel_path.into(),
+                    rel_path,
                 })
             }
         }
@@ -122,8 +128,15 @@ fn iter_include_dir<'a>(
         stack: dir.entries().iter().collect(),
     }
     .map(|file| {
-        let rel_path = Utf8PathBuf::try_from(file.path().to_path_buf())
-            .map_err(|error| error.into_io_error())?;
+        // include_dir 0.7.4 returns paths with forward slashes, including on
+        // Windows. But that isn't part of the stable API it seems, so we call
+        // `rel_path_to_forward_slashes` anyway.
+        let rel_path = match file.path().try_into() {
+            Ok(path) => rel_path_to_forward_slashes(path),
+            Err(error) => {
+                return Err(error.into_io_error());
+            }
+        };
         Ok(TestEntry {
             source: TestSource::IncludeDir(file),
             rel_path,
@@ -139,10 +152,11 @@ pub(crate) struct TestEntry {
 
 impl TestEntry {
     pub(crate) fn from_full_path(root: &Utf8Path, path: Utf8PathBuf) -> Self {
-        let rel_path = path
-            .strip_prefix(root)
-            .unwrap_or_else(|_| panic!("failed to strip root '{}' from path '{}'", root, path))
-            .to_owned();
+        let path = rel_path_to_forward_slashes(&path);
+        let rel_path =
+            rel_path_to_forward_slashes(path.strip_prefix(root).unwrap_or_else(|_| {
+                panic!("failed to strip root '{}' from path '{}'", root, path)
+            }));
         Self {
             source: TestSource::Path(path),
             rel_path,
@@ -180,10 +194,18 @@ impl TestEntry {
         }
     }
 
-    /// Returns the path to the test data.
+    /// Returns the path to match regexes against.
+    ///
+    /// This is always the relative path to the file from the include directory.
+    pub(crate) fn match_path(&self) -> &Utf8Path {
+        &self.rel_path
+    }
+
+    /// Returns the path to the test data, as passed into the test function.
     ///
-    /// For directories on disk, this is the absolute path. For `include_dir`
-    /// sources, this is the path relative to the root of the include directory.
+    /// For directories on disk, this is the relative path after being joined
+    /// with the include directory. For `include_dir` sources, this is the path
+    /// relative to the root of the include directory.
     pub(crate) fn test_path(&self) -> &Utf8Path {
         match &self.source {
             TestSource::Path(path) => path,
@@ -219,9 +241,37 @@ impl TestEntry {
     }
 }
 
+#[cfg(windows)]
+#[track_caller]
+fn rel_path_to_forward_slashes(path: &Utf8Path) -> Utf8PathBuf {
+    assert!(is_truly_relative(path), "path {path} must be relative");
+    path.as_str().replace('\\', "/").into()
+}
+
+#[cfg(not(windows))]
+#[track_caller]
+fn rel_path_to_forward_slashes(path: &Utf8Path) -> Utf8PathBuf {
+    assert!(is_truly_relative(path), "path {path} must be relative");
+    path.to_owned()
+}
+
+/// Returns true if this is a path with no root-dir or prefix components.
+///
+/// On Windows, unlike `path.is_relative()`, this rejects paths like "C:temp"
+/// and "\temp".
+#[track_caller]
+fn is_truly_relative(path: &Utf8Path) -> bool {
+    path.components().all(|c| match c {
+        Utf8Component::Normal(_) | Utf8Component::CurDir | Utf8Component::ParentDir => true,
+        Utf8Component::RootDir | Utf8Component::Prefix(_) => false,
+    })
+}
+
 #[derive(Debug)]
 #[doc(hidden)]
 pub(crate) enum TestSource {
+    /// A data source on disk, with the path being the relative path to the file
+    /// from the crate root.
     Path(Utf8PathBuf),
     #[cfg(feature = "include-dir")]
     IncludeDir(&'static include_dir::File<'static>),
@@ -256,7 +306,14 @@ pub mod data_source_kinds {
 
     impl<T: ToString> AsDirectory for T {
         fn resolve_data_source(self) -> DataSource {
-            DataSource::Directory(self.to_string().into())
+            let s = self.to_string();
+            let path = Utf8Path::new(&s);
+
+            if !is_truly_relative(path) {
+                panic!("data source path must be relative: '{}'", s);
+            }
+
+            DataSource::Directory(rel_path_to_forward_slashes(path))
         }
     }
 
@@ -290,6 +347,12 @@ pub mod data_source_kinds {
 mod tests {
     use super::*;
 
+    #[test]
+    #[should_panic = "data source path must be relative: '/absolute/path'"]
+    fn data_source_absolute_path_panics() {
+        data_source_kinds::AsDirectory::resolve_data_source("/absolute/path");
+    }
+
     #[test]
     fn missing_test_name() {
         assert_eq!(derive_test_path("root".into(), "file", "test_name"), None);

src/lib.rs

@@ -15,8 +15,10 @@
 //! `datatest-stable` will call the `my_test` function once per matching file in
 //! the directory. Directory traversals are recursive.
 //!
-//! `datatest-stable` works with [cargo nextest](https://nexte.st/), and is part of the [nextest-rs
-//! organization](https://github.com/nextest-rs/) on GitHub.
+//! `datatest-stable` works with [cargo nextest](https://nexte.st/), and is part
+//! of the [nextest-rs organization](https://github.com/nextest-rs/) on GitHub.
+//! With nextest, each test case is represented as a separate test, and is run
+//! as a separate process in parallel.
 //!
 //! # Usage
 //!
@@ -43,8 +45,10 @@
 //!     extra `Vec<u8>` parameter is specified, the contents of the file will be loaded and passed
 //!     in as a `Vec<u8>` (erroring out if that failed).
 //!
-//! * `root` - The path to the root directory where the input files (fixtures) live. This path is
-//!   relative to the root of the crate (the directory where the crate's `Cargo.toml` is located).
+//! * `root` - The path to the root directory where the input files (fixtures)
+//!   live. This path is relative to the root of the crate (the directory where
+//!   the crate's `Cargo.toml` is located). Absolute paths are not supported, and
+//!   will panic at runtime.
 //!
 //!   `root` is an arbitrary expression that implements `Display`, such as `&str`, or a
 //!   function call that returns a `Utf8PathBuf`.
@@ -56,11 +60,21 @@
 //!   `pattern` is an arbitrary expression that implements `Display`, such as
 //!   `&str`, or a function call that returns a `String`.
 //!
-//! The passed-in `Path` and `Utf8Path` are **absolute** paths to the files to be tested.
-//!
 //! The three parameters can be repeated if you have multiple sets of data-driven tests to be run:
 //! `datatest_stable::harness!(testfn1, root1, pattern1, testfn2, root2, pattern2)`.
 //!
+//! ## Relative paths
+//!
+//! The `pattern` argument is tested against the **relative** path of each file,
+//! **excluding** the `root` prefix -- not the absolute path.
+//!
+//! The `Path` and `Utf8Path` passed into the test functions are **relative** to
+//! the root of the crate, obtained by joining `root` to the relative path of
+//! the file.
+//!
+//! For universality, all relative paths use `/` as the path separator,
+//! including on Windows.
+//!
 //! ## Examples
 //!
 //! This is an example test. Use it with `harness = false`.
@@ -87,6 +101,11 @@
 //! );
 //! ```
 //!
+//! If `path/to/fixtures` contains a file `foo/bar.txt`, then:
+//!
+//! * The pattern `r"^.*/*"` will match `foo/bar.txt`.
+//! * `my_test` and `my_test_utf8` will be called with `"path/to/fixtures/foo/bar.txt"`.
+//!
 //! ## Embedding directories at compile time
 //!
 //! With the `include-dir` feature enabled, you can use the
@@ -136,7 +155,9 @@
 //! ```
 //!
 //! In this case, the passed-in `Path` and `Utf8Path` are **relative** to the
-//! root of the included directory.
+//! root of the included directory. Like elsewhere in `datatest-stable`, these
+//! relative paths always use forward slashes as separators, including on
+//! Windows.
 //!
 //! Because the files don't exist on disk, the test functions must accept their
 //! contents as either a `String` or a `Vec<u8>`. If the argument is not
@@ -169,9 +190,11 @@
 //! );
 //! ```
 //!
-//! In this case, note that `path` will be absolute if `FIXTURES` is a string,
-//! and relative if `FIXTURES` is a `Dir`. Your test should be prepared to
-//! handle either case.
+//! In this case, note that `path` will be relative to the **crate directory**
+//! (e.g. `tests/files/foo/bar.txt`) if `FIXTURES` is a string, and relative to
+//! the **include directory** (e.g. `foo/bar.txt`) if `FIXTURES` is a
+//! [`Dir`](include_dir::Dir). Your test should be prepared to handle either
+//! case.
 //!
 //! # Features
 //!

src/runner.rs

@@ -152,7 +152,7 @@ impl Requirements {
             .walk_files()
             .filter_map(|entry_res| {
                 let entry = entry_res.expect("error reading directory");
-                let path_str = entry.test_path().as_str();
+                let path_str = entry.match_path().as_str();
                 if re.is_match(path_str).unwrap_or_else(|error| {
                     panic!(
                         "error matching pattern '{}' against path '{}' : {}",