1 files changed, 60 insertions, 36 deletions

diff --git a/subplotlib/src/steplibrary/files.rs b/subplotlib/src/steplibrary/files.rs
index 1a55f7a..b1596ae 100644
--- a/subplotlib/src/steplibrary/files.rs
+++ b/subplotlib/src/steplibrary/files.rs
@@ -7,7 +7,7 @@ use std::collections::{HashMap, HashSet};
 use std::ffi::OsString;
 use std::fs::{self, Metadata, OpenOptions};
 use std::io::{self, Write};
-use std::path::PathBuf;
+use std::path::{Path, PathBuf};
 use std::time::{Duration, SystemTime};
 
 use filetime::FileTime;
@@ -31,7 +31,7 @@ pub use super::datadir::Datadir;
 /// Because files can typically only be named in Subplot documents, we assume they
 /// all have names which can be rendered as utf-8 strings.
 pub struct Files {
-    metadata: HashMap<String, Metadata>,
+    metadata: HashMap<PathBuf, Metadata>,
 }
 
 impl ContextElement for Files {
@@ -50,7 +50,7 @@ impl ContextElement for Files {
 #[step]
 #[context(Datadir)]
 pub fn create_from_embedded(context: &ScenarioContext, embedded_file: SubplotDataFile) {
-    let filename_on_disk = format!("{}", embedded_file.name().display());
+    let filename_on_disk = PathBuf::from(format!("{}", embedded_file.name().display()));
     create_from_embedded_with_other_name::call(context, &filename_on_disk, embedded_file)?;
 }
 
@@ -63,7 +63,7 @@ pub fn create_from_embedded(context: &ScenarioContext, embedded_file: SubplotDat
 #[step]
 pub fn create_from_embedded_with_other_name(
     context: &Datadir,
-    filename_on_disk: &str,
+    filename_on_disk: &Path,
     embedded_file: SubplotDataFile,
 ) {
     let filename_on_disk = PathBuf::from(filename_on_disk);
@@ -86,11 +86,11 @@ pub fn create_from_embedded_with_other_name(
 /// Sets the modification time for the given filename to the provided mtime.
 /// If the file does not exist, it will be created.
 #[step]
-pub fn touch_with_timestamp(context: &Datadir, filename: &str, mtime: &str) {
+pub fn touch_with_timestamp(context: &Datadir, filename: &Path, mtime: &str) {
     let fd = format_description!(
         "[year]-[month]-[day] [hour]:[minute]:[second] [offset_hour]:[offset_minute]"
     );
-    let full_time = format!("{} +00:00", mtime);
+    let full_time = format!("{mtime} +00:00");
     let ts = OffsetDateTime::parse(&full_time, &fd)?;
     let (secs, nanos) = (ts.unix_timestamp(), 0);
     let mtime = FileTime::from_unix_time(secs, nanos);
@@ -99,6 +99,7 @@ pub fn touch_with_timestamp(context: &Datadir, filename: &str, mtime: &str) {
     drop(
         OpenOptions::new()
             .create(true)
+            .truncate(false)
             .write(true)
             .open(&full_path)?,
     );
@@ -112,7 +113,7 @@ pub fn touch_with_timestamp(context: &Datadir, filename: &str, mtime: &str) {
 ///
 /// Create/replace the given file with the given content.
 #[step]
-pub fn create_from_text(context: &Datadir, text: &str, filename: &str) {
+pub fn create_from_text(context: &Datadir, text: &str, filename: &Path) {
     context.open_write(filename)?.write_all(text.as_bytes())?;
 }
 
@@ -125,12 +126,12 @@ pub fn create_from_text(context: &Datadir, text: &str, filename: &str) {
 #[step]
 #[context(Datadir)]
 #[context(Files)]
-pub fn remember_metadata(context: &ScenarioContext, filename: &str) {
+pub fn remember_metadata(context: &ScenarioContext, filename: &Path) {
     let full_path = context.with(
         |context: &Datadir| context.canonicalise_filename(filename),
         false,
     )?;
-    let metadata = fs::metadata(&full_path)?;
+    let metadata = fs::metadata(full_path)?;
     context.with_mut(
         |context: &mut Files| {
             context.metadata.insert(filename.to_owned(), metadata);
@@ -147,13 +148,14 @@ pub fn remember_metadata(context: &ScenarioContext, filename: &str) {
 /// This will create the named file if it does not exist, and then it will ensure that the
 /// file's modification time is set to the current time.
 #[step]
-pub fn touch(context: &Datadir, filename: &str) {
+pub fn touch(context: &Datadir, filename: &Path) {
     let full_path = context.canonicalise_filename(filename)?;
     let now = FileTime::now();
     // If the file doesn't exist, create it
     drop(
         OpenOptions::new()
             .create(true)
+            .truncate(false)
             .write(true)
             .open(&full_path)?,
     );
@@ -167,13 +169,13 @@ pub fn touch(context: &Datadir, filename: &str) {
 ///
 /// This simple step will succeed if the given filename exists in some sense.
 #[step]
-pub fn file_exists(context: &Datadir, filename: &str) {
+pub fn file_exists(context: &Datadir, filename: &Path) {
     let full_path = context.canonicalise_filename(filename)?;
     match fs::metadata(full_path) {
         Ok(_) => (),
         Err(e) => {
             if matches!(e.kind(), io::ErrorKind::NotFound) {
-                throw!(format!("file '{}' was not found", filename))
+                throw!(format!("file '{}' was not found", filename.display()))
             } else {
                 throw!(e);
             }
@@ -187,11 +189,14 @@ pub fn file_exists(context: &Datadir, filename: &str) {
 ///
 /// This simple step will succeed if the given filename does not exist in any sense.
 #[step]
-pub fn file_does_not_exist(context: &Datadir, filename: &str) {
+pub fn file_does_not_exist(context: &Datadir, filename: &Path) {
     let full_path = context.canonicalise_filename(filename)?;
     match fs::metadata(full_path) {
         Ok(_) => {
-            throw!(format!("file '{}' was unexpectedly found", filename))
+            throw!(format!(
+                "file '{}' was unexpectedly found",
+                filename.display()
+            ))
         }
         Err(e) => {
             if !matches!(e.kind(), io::ErrorKind::NotFound) {
@@ -229,10 +234,11 @@ pub fn only_these_exist(context: &Datadir, filenames: &str) {
 /// This will load the content of the named file and ensure it contains the given string.
 /// Note: this assumes everything is utf-8 encoded.  If not, things will fail.
 #[step]
-pub fn file_contains(context: &Datadir, filename: &str, data: &str) {
+pub fn file_contains(context: &Datadir, filename: &Path, data: &str) {
     let full_path = context.canonicalise_filename(filename)?;
     let body = fs::read_to_string(full_path)?;
     if !body.contains(data) {
+        println!("file {} contains:\n{}", filename.display(), body);
         throw!("expected file content not found");
     }
 }
@@ -244,10 +250,11 @@ pub fn file_contains(context: &Datadir, filename: &str, data: &str) {
 /// This will load the content of the named file and ensure it lacks the given string.
 /// Note: this assumes everything is utf-8 encoded.  If not, things will fail.
 #[step]
-pub fn file_doesnt_contain(context: &Datadir, filename: &str, data: &str) {
+pub fn file_doesnt_contain(context: &Datadir, filename: &Path, data: &str) {
     let full_path = context.canonicalise_filename(filename)?;
     let body = fs::read_to_string(full_path)?;
     if body.contains(data) {
+        println!("file {} contains:\n{}", filename.display(), body);
         throw!("unexpected file content found");
     }
 }
@@ -260,11 +267,12 @@ pub fn file_doesnt_contain(context: &Datadir, filename: &str, data: &str) {
 /// matches the given regular expression.  This step will fail if the file is not utf-8
 /// encoded, or if the regex fails to compile
 #[step]
-pub fn file_matches_regex(context: &Datadir, filename: &str, regex: &str) {
+pub fn file_matches_regex(context: &Datadir, filename: &Path, regex: &str) {
     let full_path = context.canonicalise_filename(filename)?;
     let regex = Regex::new(regex)?;
     let body = fs::read_to_string(full_path)?;
     if !regex.is_match(&body) {
+        println!("file {} contains:\n{}", filename.display(), body);
         throw!("file content does not match given regex");
     }
 }
@@ -275,12 +283,22 @@ pub fn file_matches_regex(context: &Datadir, filename: &str, regex: &str) {
 ///
 /// This loads the content of the given two files as **bytes** and checks they mach.
 #[step]
-pub fn file_match(context: &Datadir, filename1: &str, filename2: &str) {
+pub fn file_match(context: &Datadir, filename1: &Path, filename2: &Path) {
     let full_path1 = context.canonicalise_filename(filename1)?;
     let full_path2 = context.canonicalise_filename(filename2)?;
     let body1 = fs::read(full_path1)?;
     let body2 = fs::read(full_path2)?;
     if body1 != body2 {
+        println!(
+            "file {} contains:\n{}",
+            filename1.display(),
+            String::from_utf8_lossy(&body1)
+        );
+        println!(
+            "file {} contains:\n{}",
+            filename2.display(),
+            String::from_utf8_lossy(&body2)
+        );
         throw!("file contents do not match each other");
     }
 }
@@ -299,12 +317,12 @@ pub fn file_match(context: &Datadir, filename1: &str, filename2: &str) {
 #[step]
 #[context(Datadir)]
 #[context(Files)]
-pub fn has_remembered_metadata(context: &ScenarioContext, filename: &str) {
+pub fn has_remembered_metadata(context: &ScenarioContext, filename: &Path) {
     let full_path = context.with(
         |context: &Datadir| context.canonicalise_filename(filename),
         false,
     )?;
-    let metadata = fs::metadata(&full_path)?;
+    let metadata = fs::metadata(full_path)?;
     if let Some(remembered) = context.with(
         |context: &Files| Ok(context.metadata.get(filename).cloned()),
         false,
@@ -314,10 +332,13 @@ pub fn has_remembered_metadata(context: &ScenarioContext, filename: &str) {
             || metadata.len() != remembered.len()
             || metadata.is_file() != remembered.is_file()
         {
-            throw!(format!("metadata change detected for {}", filename));
+            throw!(format!(
+                "metadata change detected for {}",
+                filename.display()
+            ));
         }
     } else {
-        throw!(format!("no remembered metadata for {}", filename));
+        throw!(format!("no remembered metadata for {}", filename.display()));
     }
 }
 
@@ -335,12 +356,12 @@ pub fn has_remembered_metadata(context: &ScenarioContext, filename: &str) {
 #[step]
 #[context(Datadir)]
 #[context(Files)]
-pub fn has_different_metadata(context: &ScenarioContext, filename: &str) {
+pub fn has_different_metadata(context: &ScenarioContext, filename: &Path) {
     let full_path = context.with(
         |context: &Datadir| context.canonicalise_filename(filename),
         false,
     )?;
-    let metadata = fs::metadata(&full_path)?;
+    let metadata = fs::metadata(full_path)?;
     if let Some(remembered) = context.with(
         |context: &Files| Ok(context.metadata.get(filename).cloned()),
         false,
@@ -350,10 +371,13 @@ pub fn has_different_metadata(context: &ScenarioContext, filename: &str) {
             && metadata.len() == remembered.len()
             && metadata.is_file() == remembered.is_file()
         {
-            throw!(format!("metadata change not detected for {}", filename));
+            throw!(format!(
+                "metadata change not detected for {}",
+                filename.display()
+            ));
         }
     } else {
-        throw!(format!("no remembered metadata for {}", filename));
+        throw!(format!("no remembered metadata for {}", filename.display()));
     }
 }
 
@@ -363,13 +387,13 @@ pub fn has_different_metadata(context: &ScenarioContext, filename: &str) {
 ///
 /// Specifically this checks that the given file has been modified in the past 5 seconds.
 #[step]
-pub fn mtime_is_recent(context: &Datadir, filename: &str) {
+pub fn mtime_is_recent(context: &Datadir, filename: &Path) {
     let full_path = context.canonicalise_filename(filename)?;
     let metadata = fs::metadata(full_path)?;
     let mtime = metadata.modified()?;
     let diff = SystemTime::now().duration_since(mtime)?;
     if diff > (Duration::from_secs(5)) {
-        throw!(format!("{} is older than 5 seconds", filename));
+        throw!(format!("{} is older than 5 seconds", filename.display()));
     }
 }
 
@@ -379,13 +403,13 @@ pub fn mtime_is_recent(context: &Datadir, filename: &str) {
 ///
 /// Specifically this checks that the file was modified at least 39 years ago.
 #[step]
-pub fn mtime_is_ancient(context: &Datadir, filename: &str) {
+pub fn mtime_is_ancient(context: &Datadir, filename: &Path) {
     let full_path = context.canonicalise_filename(filename)?;
     let metadata = fs::metadata(full_path)?;
     let mtime = metadata.modified()?;
     let diff = SystemTime::now().duration_since(mtime)?;
     if diff < (Duration::from_secs(39 * 365 * 24 * 3600)) {
-        throw!(format!("{} is younger than 39 years", filename));
+        throw!(format!("{} is younger than 39 years", filename.display()));
     }
 }
 
@@ -395,7 +419,7 @@ pub fn mtime_is_ancient(context: &Datadir, filename: &str) {
 ///
 /// This is the equivalent of `mkdir -p` within the data directory for the scenario.
 #[step]
-pub fn make_directory(context: &Datadir, path: &str) {
+pub fn make_directory(context: &Datadir, path: &Path) {
     context.create_dir_all(path)?;
 }
 
@@ -405,7 +429,7 @@ pub fn make_directory(context: &Datadir, path: &str) {
 ///
 /// This is the equivalent of `rm -rf` within the data directory for the scenario.
 #[step]
-pub fn remove_directory(context: &Datadir, path: &str) {
+pub fn remove_directory(context: &Datadir, path: &Path) {
     let full_path = context.canonicalise_filename(path)?;
     remove_dir_all::remove_dir_all(full_path)?;
 }
@@ -417,7 +441,7 @@ pub fn remove_directory(context: &Datadir, path: &str) {
 /// This ensures that the given path exists in the data directory for the scenario and
 /// that it is a directory itself.
 #[step]
-pub fn path_exists(context: &Datadir, path: &str) {
+pub fn path_exists(context: &Datadir, path: &Path) {
     let full_path = context.canonicalise_filename(path)?;
     if !fs::metadata(&full_path)?.is_dir() {
         throw!(format!(
@@ -434,7 +458,7 @@ pub fn path_exists(context: &Datadir, path: &str) {
 /// This ensures that the given path does not exist in the data directory.  If it exists
 /// and is not a directory, then this will also fail.
 #[step]
-pub fn path_does_not_exist(context: &Datadir, path: &str) {
+pub fn path_does_not_exist(context: &Datadir, path: &Path) {
     let full_path = context.canonicalise_filename(path)?;
     match fs::metadata(&full_path) {
         Ok(_) => throw!(format!("{} exists", full_path.display())),
@@ -453,7 +477,7 @@ pub fn path_does_not_exist(context: &Datadir, path: &str) {
 /// This checks that the given path inside the data directory exists and is an
 /// empty directory itself.
 #[step]
-pub fn path_is_empty(context: &Datadir, path: &str) {
+pub fn path_is_empty(context: &Datadir, path: &Path) {
     let full_path = context.canonicalise_filename(path)?;
     let mut iter = fs::read_dir(&full_path)?;
     match iter.next() {
@@ -471,7 +495,7 @@ pub fn path_is_empty(context: &Datadir, path: &str) {
 /// directory itself.  The step also asserts that the given directory contains at least
 /// one entry.
 #[step]
-pub fn path_is_not_empty(context: &Datadir, path: &str) {
+pub fn path_is_not_empty(context: &Datadir, path: &Path) {
     let full_path = context.canonicalise_filename(path)?;
     let mut iter = fs::read_dir(&full_path)?;
     match iter.next() {