From 5bdc38cc6b2974a34bfbb9e20670c9b64ec60b27 Mon Sep 17 00:00:00 2001 From: Jesse Hallett Date: Thu, 28 Mar 2019 17:30:02 -0400 Subject: [PATCH 1/7] step line numbers are recorded when protocol is loaded (#65) (#121) This change depends on a fork of yaml-rust, https://github.com/hallettj/yaml-rust/tree/load_from_str_with_markers --- Cargo.lock | 20 ++++- Cargo.toml | 2 +- src/protocol/marker.rs | 32 +++++++ src/protocol/mod.rs | 162 +++++++++++++++++++++++------------- src/protocol/yaml.rs | 30 ++++--- src/protocol_checker/mod.rs | 20 ++++- src/recorder/mod.rs | 1 + tests/holes.rs | 18 +++- tests/path.rs | 1 + tests/run.rs | 13 ++- 10 files changed, 216 insertions(+), 83 deletions(-) create mode 100644 src/protocol/marker.rs diff --git a/Cargo.lock b/Cargo.lock index 63dd289..5e2642a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -82,6 +82,16 @@ dependencies = [ "syn 0.15.26 (registry+https://github.com/rust-lang/crates.io-index)", ] +[[package]] +name = "derivative" +version = "1.0.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +dependencies = [ + "proc-macro2 0.4.27 (registry+https://github.com/rust-lang/crates.io-index)", + "quote 0.6.11 (registry+https://github.com/rust-lang/crates.io-index)", + "syn 0.15.26 (registry+https://github.com/rust-lang/crates.io-index)", +] + [[package]] name = "difference" version = "2.0.0" @@ -264,7 +274,7 @@ dependencies = [ "tempdir 0.3.7 (registry+https://github.com/rust-lang/crates.io-index)", "test-utils 0.1.0", "trim-margin 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)", - "yaml-rust 0.4.2 (registry+https://github.com/rust-lang/crates.io-index)", + "yaml-rust 0.4.3 (git+https://github.com/hallettj/yaml-rust?rev=5216abe1f7d24ccd17a3b1b1e56c0114758d5ed7)", ] [[package]] @@ -396,9 +406,10 @@ source = "registry+https://github.com/rust-lang/crates.io-index" [[package]] name = "yaml-rust" -version = "0.4.2" -source = "registry+https://github.com/rust-lang/crates.io-index" +version = "0.4.3" +source = "git+https://github.com/hallettj/yaml-rust?rev=5216abe1f7d24ccd17a3b1b1e56c0114758d5ed7#5216abe1f7d24ccd17a3b1b1e56c0114758d5ed7" dependencies = [ + "derivative 1.0.2 (registry+https://github.com/rust-lang/crates.io-index)", "linked-hash-map 0.5.1 (registry+https://github.com/rust-lang/crates.io-index)", ] @@ -414,6 +425,7 @@ dependencies = [ "checksum cfg-if 0.1.6 (registry+https://github.com/rust-lang/crates.io-index)" = "082bb9b28e00d3c9d39cc03e64ce4cea0f1bb9b3fde493f0cbc008472d22bdf4" "checksum clap 2.32.0 (registry+https://github.com/rust-lang/crates.io-index)" = "b957d88f4b6a63b9d70d5f454ac8011819c6efa7727858f458ab71c756ce2d3e" "checksum ctor 0.1.7 (registry+https://github.com/rust-lang/crates.io-index)" = "9a43db2bba5cafdc6aa068c892a518e477ee0df3705e53ec70247a9ff93546d5" +"checksum derivative 1.0.2 (registry+https://github.com/rust-lang/crates.io-index)" = "6073e9676dbebdddeabaeb63e3b7cefd23c86f5c41d381ee1237cc77b1079898" "checksum difference 2.0.0 (registry+https://github.com/rust-lang/crates.io-index)" = "524cbf6897b527295dff137cec09ecf3a05f4fddffd7dfcd1585403449e74198" "checksum fuchsia-cprng 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)" = "a06f77d526c1a601b7c4cdd98f54b5eaabffc14d5f2f0296febdc7f357c6d3ba" "checksum lazy_static 1.3.0 (registry+https://github.com/rust-lang/crates.io-index)" = "bc5729f27f159ddd61f4df6228e827e86643d4d3e7c32183cb30a1c08f604a14" @@ -454,4 +466,4 @@ dependencies = [ "checksum winapi 0.3.6 (registry+https://github.com/rust-lang/crates.io-index)" = "92c1eb33641e276cfa214a0522acad57be5c56b10cb348b3c5117db75f3ac4b0" "checksum winapi-i686-pc-windows-gnu 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)" = "ac3b87c63620426dd9b991e5ce0329eff545bccbbb34f3be09ff6fb6ab51b7b6" "checksum winapi-x86_64-pc-windows-gnu 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)" = "712e227841d057c1ee1cd2fb22fa7e5a5461ae8e48fa2ca79ec42cfc1931183f" -"checksum yaml-rust 0.4.2 (registry+https://github.com/rust-lang/crates.io-index)" = "95acf0db5515d07da9965ec0e0ba6cc2d825e2caeb7303b66ca441729801254e" +"checksum yaml-rust 0.4.3 (git+https://github.com/hallettj/yaml-rust?rev=5216abe1f7d24ccd17a3b1b1e56c0114758d5ed7)" = "" diff --git a/Cargo.toml b/Cargo.toml index 06a893f..7721454 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -15,7 +15,7 @@ path = "src/main.rs" nix = "*" libc = "*" tempdir = "*" -yaml-rust = "*" +yaml-rust = { git = "https://github.com/hallettj/yaml-rust", rev = "5216abe1f7d24ccd17a3b1b1e56c0114758d5ed7" } linked-hash-map = "*" bincode = "*" quale = "*" diff --git a/src/protocol/marker.rs b/src/protocol/marker.rs new file mode 100644 index 0000000..5bbc2ab --- /dev/null +++ b/src/protocol/marker.rs @@ -0,0 +1,32 @@ +use yaml_rust; + +#[derive(Clone, Copy, PartialEq, PartialOrd, Debug, Eq, Ord, Hash)] +pub struct Marker { + pub line: usize, + pub col: usize, +} + +impl Marker { + pub fn line(&self) -> usize { + let Marker { line, .. } = self; + *line + } +} + +impl<'a> From<&'a yaml_rust::Marker> for Marker { + fn from(marker: &'a yaml_rust::Marker) -> Self { + Marker { + line: marker.line(), + col: marker.col(), + } + } +} + +impl From for Marker { + fn from(marker: yaml_rust::Marker) -> Self { + Marker { + line: marker.line(), + col: marker.col(), + } + } +} diff --git a/src/protocol/mod.rs b/src/protocol/mod.rs index d447d3e..db5ca08 100644 --- a/src/protocol/mod.rs +++ b/src/protocol/mod.rs @@ -4,10 +4,12 @@ mod argument_parser; pub mod command; pub mod command_matcher; mod executable_path; +pub mod marker; pub mod yaml; use self::argument_parser::Parser; pub use self::executable_path::compare_executables; +pub use self::marker::Marker; use crate::protocol::yaml::*; use crate::utils::{path_to_string, with_has_more}; use crate::R; @@ -18,11 +20,12 @@ use std::collections::{HashMap, VecDeque}; use std::ffi::OsString; use std::fs; use std::path::{Path, PathBuf}; -use yaml_rust::{yaml::Hash, Yaml, YamlLoader}; +use yaml_rust::{yaml::Hash, yaml::HashNode, Node, Yaml, YamlLoader, YamlMarked}; #[derive(PartialEq, Eq, Debug, Clone)] pub struct Step { pub command_matcher: CommandMatcher, + pub marker: Option, pub stdout: Vec, pub exitcode: i32, } @@ -31,33 +34,39 @@ impl Step { pub fn new(command_matcher: CommandMatcher) -> Step { Step { command_matcher, + marker: None, stdout: vec![], exitcode: 0, } } + pub fn marker(&self) -> Option { + let Step { marker, .. } = self; + *marker + } + fn from_string(string: &str) -> R { Ok(Step::new(CommandMatcher::ExactMatch(Command::new(string)?))) } - fn add_exitcode(&mut self, object: &Hash) -> R<()> { + fn add_exitcode(&mut self, object: &HashNode) -> R<()> { if let Ok(exitcode) = object.expect_field("exitcode") { self.exitcode = exitcode.expect_integer()?; } Ok(()) } - fn add_stdout(&mut self, object: &Hash) -> R<()> { + fn add_stdout(&mut self, object: &HashNode) -> R<()> { if let Ok(stdout) = object.expect_field("stdout") { self.stdout = stdout.expect_str()?.as_bytes().to_vec(); } Ok(()) } - fn parse(yaml: &Yaml) -> R { - match yaml { - Yaml::String(string) => Step::from_string(string), - Yaml::Hash(object) => { + fn parse(Node(yaml, marker): &Node) -> R { + let mut step = match yaml { + YamlMarked::String(string) => Step::from_string(string), + YamlMarked::Hash(object) => { check_keys(&["command", "stdout", "exitcode", "regex"], object)?; let mut step = match (object.expect_field("command"), object.expect_field("regex")) { @@ -72,7 +81,9 @@ impl Step { Ok(step) } _ => Err(format!("expected: string or array, got: {:?}", yaml))?, - } + }?; + step.marker = marker.as_ref().map(From::from); + Ok(step) } fn serialize(&self) -> Yaml { @@ -93,12 +104,14 @@ impl Step { #[cfg(test)] mod parse_step { + use self::test_helpers::step_with_marker; use super::*; + use pretty_assertions::assert_eq; use test_utils::assert_error; - use yaml_rust::Yaml; + use yaml_rust::YamlMarked; fn test_parse_step(yaml: &str) -> R { - let yaml = YamlLoader::load_from_str(yaml)?; + let yaml = YamlLoader::load_from_str_with_markers(yaml)?; assert_eq!(yaml.len(), 1); let yaml = &yaml[0]; Step::parse(yaml) @@ -108,10 +121,13 @@ mod parse_step { fn parses_strings_to_steps() -> R<()> { assert_eq!( test_parse_step(r#""foo""#)?, - Step::new(CommandMatcher::ExactMatch(Command { - executable: PathBuf::from("foo"), - arguments: vec![], - })), + step_with_marker( + CommandMatcher::ExactMatch(Command { + executable: PathBuf::from("foo"), + arguments: vec![], + }), + Marker { line: 1, col: 0 } + ), ); Ok(()) } @@ -132,10 +148,13 @@ mod parse_step { fn parses_objects_to_steps() -> R<()> { assert_eq!( test_parse_step(r#"{command: "foo"}"#)?, - Step::new(CommandMatcher::ExactMatch(Command { - executable: PathBuf::from("foo"), - arguments: vec![], - })), + step_with_marker( + CommandMatcher::ExactMatch(Command { + executable: PathBuf::from("foo"), + arguments: vec![], + }), + Marker { line: 1, col: 0 } + ), ); Ok(()) } @@ -155,7 +174,7 @@ mod parse_step { #[test] fn gives_nice_parse_errors() { assert_error!( - Step::parse(&Yaml::Null), + Step::parse(&Node(YamlMarked::Null, None)), "expected: string or array, got: Null" ) } @@ -171,6 +190,7 @@ mod parse_step { mod exitcode { use super::*; + use pretty_assertions::assert_eq; #[test] fn allows_to_specify_the_mocked_exit_code() -> R<()> { @@ -220,15 +240,15 @@ impl Protocol { } } - fn from_array(array: &[Yaml]) -> R { + fn from_array(array: &[Node]) -> R { enum StepOrHole { Step(Step), Hole, } - fn parse_step_or_hole(yaml: &Yaml) -> R { - Ok(match yaml { - Yaml::String(step) if step == "_" => StepOrHole::Hole, - yaml => StepOrHole::Step(Step::parse(yaml)?), + fn parse_step_or_hole(node: &Node) -> R { + Ok(match node.value() { + YamlMarked::String(step) if step == "_" => StepOrHole::Hole, + _ => StepOrHole::Step(Step::parse(node)?), }) } let mut protocol = Protocol::empty(); @@ -248,14 +268,14 @@ impl Protocol { Ok(protocol) } - fn add_arguments(&mut self, object: &Hash) -> R<()> { + fn add_arguments(&mut self, object: &HashNode) -> R<()> { if let Ok(arguments) = object.expect_field("arguments") { self.arguments = Parser::parse_arguments(arguments.expect_str()?)?; } Ok(()) } - fn add_env(&mut self, object: &Hash) -> R<()> { + fn add_env(&mut self, object: &HashNode) -> R<()> { if let Ok(env) = object.expect_field("env") { for (key, value) in env.expect_object()?.into_iter() { self.env.insert( @@ -267,7 +287,7 @@ impl Protocol { Ok(()) } - fn add_cwd(&mut self, object: &Hash) -> R<()> { + fn add_cwd(&mut self, object: &HashNode) -> R<()> { if let Ok(cwd) = object.expect_field("cwd") { let cwd = cwd.expect_str()?; if !cwd.starts_with('/') { @@ -281,21 +301,21 @@ impl Protocol { Ok(()) } - fn add_stderr(&mut self, object: &Hash) -> R<()> { + fn add_stderr(&mut self, object: &HashNode) -> R<()> { if let Ok(stderr) = object.expect_field("stderr") { self.stderr = Some(stderr.expect_str()?.as_bytes().to_vec()); } Ok(()) } - fn add_exitcode(&mut self, object: &Hash) -> R<()> { + fn add_exitcode(&mut self, object: &HashNode) -> R<()> { if let Ok(exitcode) = object.expect_field("exitcode") { self.exitcode = Some(exitcode.expect_integer()?); } Ok(()) } - fn add_mocked_files(&mut self, object: &Hash) -> R<()> { + fn add_mocked_files(&mut self, object: &HashNode) -> R<()> { if let Ok(paths) = object.expect_field("mockedFiles") { for path in paths.expect_array()?.iter() { self.mocked_files.push(PathBuf::from(path.expect_str()?)); @@ -304,7 +324,7 @@ impl Protocol { Ok(()) } - fn from_object(object: &Hash) -> R { + fn from_object(object: &HashNode) -> R { check_keys( &[ "protocol", @@ -380,7 +400,7 @@ impl Protocols { } } - fn from_array(array: &[Yaml]) -> R { + fn from_array(array: &[Node]) -> R { let mut result = vec![]; for element in array.iter() { result.push(Protocol::from_object(element.expect_object()?)?); @@ -388,14 +408,14 @@ impl Protocols { Ok(Protocols::new(result)) } - fn add_interpreter(&mut self, object: &Hash) -> R<()> { + fn add_interpreter(&mut self, object: &HashNode) -> R<()> { if let Ok(interpreter) = object.expect_field("interpreter") { self.interpreter = Some(PathBuf::from(interpreter.expect_str()?)); } Ok(()) } - fn add_unmocked_commands(&mut self, object: &Hash) -> R<()> { + fn add_unmocked_commands(&mut self, object: &HashNode) -> R<()> { if let Ok(unmocked_commands) = object.expect_field("unmockedCommands") { for unmocked_command in unmocked_commands.expect_array()? { self.unmocked_commands @@ -405,10 +425,10 @@ impl Protocols { Ok(()) } - fn parse(yaml: Yaml) -> R { - Ok(match &yaml { - Yaml::Array(array) => Protocols::from_array(&array)?, - Yaml::Hash(object) => match ( + fn parse(yaml: Node) -> R { + Ok(match &yaml.value() { + YamlMarked::Array(array) => Protocols::from_array(&array)?, + YamlMarked::Hash(object) => match ( object.expect_field("protocols"), object.expect_field("protocol"), ) { @@ -432,14 +452,15 @@ impl Protocols { pub fn load(executable_path: &Path) -> R<(PathBuf, Protocols)> { let protocols_file = find_protocol_file(executable_path); let file_contents = read_protocols_file(&protocols_file)?; - let yaml: Vec = YamlLoader::load_from_str(&file_contents).map_err(|error| { - format!( - "invalid YAML in {}: {}", - protocols_file.to_string_lossy(), - error - ) - })?; - let yaml: Yaml = { + let yaml: Vec = + YamlLoader::load_from_str_with_markers(&file_contents).map_err(|error| { + format!( + "invalid YAML in {}: {}", + protocols_file.to_string_lossy(), + error + ) + })?; + let yaml: Node = { if yaml.len() > 1 { Err(format!( "multiple YAML documents not allowed (in {})", @@ -494,6 +515,7 @@ impl Protocols { #[cfg(test)] mod load { + use self::test_helpers::step_with_marker; use super::*; use crate::R; use pretty_assertions::assert_eq; @@ -523,10 +545,13 @@ mod load { | - /bin/true ", )?, - Protocol::new(vec![Step::new(CommandMatcher::ExactMatch(Command { - executable: PathBuf::from("/bin/true"), - arguments: vec![], - }))]), + Protocol::new(vec![step_with_marker( + CommandMatcher::ExactMatch(Command { + executable: PathBuf::from("/bin/true"), + arguments: vec![], + }), + Marker { line: 2, col: 4 } + )]), ); Ok(()) } @@ -665,12 +690,15 @@ mod load { r" |protocol: | - /bin/true - " + ", )?, - Protocol::new(vec![Step::new(CommandMatcher::ExactMatch(Command { - executable: PathBuf::from("/bin/true"), - arguments: vec![], - }))]), + Protocol::new(vec![step_with_marker( + CommandMatcher::ExactMatch(Command { + executable: PathBuf::from("/bin/true"), + arguments: vec![], + }), + Marker { line: 2, col: 4 } + )]), ); Ok(()) } @@ -763,7 +791,7 @@ mod load { format!( "error in {}.protocols.yaml: \ expected top-level field \"protocol\" or \"protocols\", \ - got: Hash({{}})", + got: Node(Hash({{}}), Some(Marker {{ index: 0, line: 1, col: 0 }}))", path_to_string(&tempfile.path())? ) ); @@ -820,7 +848,8 @@ mod load { ), format!( "error in {}.protocols.yaml: \ - expected: string, got: Integer(42)", + expected: string, \ + got: Node(Integer(42), Some(Marker {{ index: 35, line: 3, col: 11 }}))", path_to_string(&tempfile.path())? ) ); @@ -865,7 +894,7 @@ mod load { #[test] fn disallows_relative_paths() -> R<()> { - let yaml = YamlLoader::load_from_str(&trim_margin( + let yaml = YamlLoader::load_from_str_with_markers(&trim_margin( r" |protocol: | - /bin/true @@ -979,7 +1008,8 @@ mod load { ), format!( "error in {}.protocols.yaml: \ - expected: string, got: Integer(42)", + expected: string, \ + got: Node(Integer(42), Some(Marker {{ index: 41, line: 3, col: 13 }}))", path_to_string(&tempfile.path())? ) ); @@ -1172,7 +1202,7 @@ mod serialize { fn roundtrip(protocols: Protocols) -> R<()> { let yaml = protocols.serialize()?; - let result = Protocols::parse(yaml)?; + let result = Protocols::parse(yaml.into())?; assert_eq!(result, protocols); Ok(()) } @@ -1223,6 +1253,7 @@ mod serialize { fn includes_the_step_exitcodes() -> R<()> { let protocol = Protocol::new(vec![Step { command_matcher: CommandMatcher::ExactMatch(Command::new("cp")?), + marker: None, stdout: vec![], exitcode: 42, }]); @@ -1288,3 +1319,14 @@ mod find_protocol_file { ); } } + +#[cfg(test)] +mod test_helpers { + use super::*; + + pub fn step_with_marker(command_matcher: CommandMatcher, marker: Marker) -> Step { + let mut step = Step::new(command_matcher); + step.marker = Some(marker); + step + } +} diff --git a/src/protocol/yaml.rs b/src/protocol/yaml.rs index 0e77bc6..80a8127 100644 --- a/src/protocol/yaml.rs +++ b/src/protocol/yaml.rs @@ -3,32 +3,40 @@ use linked_hash_map::LinkedHashMap; use std::fmt; use std::io; use std::io::Cursor; -use yaml_rust::{yaml::Hash, Yaml, YamlEmitter}; +use yaml_rust::{yaml::HashNode, Node, Yaml, YamlEmitter, YamlMarked, YamlNode}; pub trait YamlExt { + type Child; + fn expect_str(&self) -> R<&str>; - fn expect_array(&self) -> R<&Vec>; + fn expect_array(&self) -> R<&Vec>; - fn expect_object(&self) -> R<&LinkedHashMap>; + fn expect_object(&self) -> R<&LinkedHashMap>; fn expect_integer(&self) -> R; } -impl YamlExt for Yaml { +impl YamlExt for T +where + T: fmt::Debug + YamlNode, + ::Child: fmt::Debug, +{ + type Child = ::Child; + fn expect_str(&self) -> R<&str> { Ok(self .as_str() .ok_or_else(|| format!("expected: string, got: {:?}", self))?) } - fn expect_array(&self) -> R<&Vec> { + fn expect_array(&self) -> R<&Vec> { Ok(self .as_vec() .ok_or_else(|| format!("expected: array, got: {:?}", self))?) } - fn expect_object(&self) -> R<&LinkedHashMap> { + fn expect_object(&self) -> R<&LinkedHashMap> { Ok(self .as_hash() .ok_or_else(|| format!("expected: object, got: {:?}", self))?) @@ -75,18 +83,18 @@ mod yaml_ext { } pub trait MapExt { - fn expect_field(&self, field: &str) -> R<&Yaml>; + fn expect_field(&self, field: &str) -> R<&Node>; } -impl MapExt for LinkedHashMap { - fn expect_field(&self, field: &str) -> R<&Yaml> { +impl MapExt for LinkedHashMap { + fn expect_field(&self, field: &str) -> R<&Node> { Ok(self - .get(&Yaml::String(field.to_string())) + .get(&Node(YamlMarked::String(field.to_string()), None)) .ok_or_else(|| format!("expected field '{}', got: {:?}", field, self))?) } } -pub fn check_keys(known_keys: &[&str], object: &Hash) -> R<()> { +pub fn check_keys(known_keys: &[&str], object: &HashNode) -> R<()> { for key in object.keys() { let key = key.expect_str()?; if !known_keys.contains(&key) { diff --git a/src/protocol_checker/mod.rs b/src/protocol_checker/mod.rs index ec4ba33..d8f532e 100644 --- a/src/protocol_checker/mod.rs +++ b/src/protocol_checker/mod.rs @@ -52,6 +52,7 @@ impl ProtocolChecker { Some(next_protocol_step) => { if !next_protocol_step.command_matcher.matches(&received) { self.register_step_error( + next_protocol_step.marker(), &next_protocol_step.command_matcher.format(), &received.format(), ); @@ -62,7 +63,7 @@ impl ProtocolChecker { } } None => { - self.register_step_error("", &received.format()); + self.register_step_error(None, "", &received.format()); ProtocolChecker::allow_failing_scripts_to_continue() } }; @@ -74,10 +75,19 @@ impl ProtocolChecker { Ok(path) } - fn register_step_error(&mut self, expected: &str, received: &str) { + fn register_step_error( + &mut self, + marker: Option, + expected: &str, + received: &str, + ) { + let location = match marker { + Some(marker) => format!(" in step on line {},\n", marker.line()), + _ => "".to_string(), + }; self.register_error(format!( - " expected: {}\n received: {}\n", - expected, received + "{} expected: {}\n received: {}\n", + location, expected, received )); } @@ -156,6 +166,7 @@ impl SyscallMock for ProtocolChecker { fn handle_end(mut self, exitcode: i32, redirector: &Redirector) -> R { if let Some(expected_step) = self.protocol.steps.pop_front() { self.register_step_error( + expected_step.marker(), &expected_step.command_matcher.format(), "