From 1e11a99991064cb8326d42c380374e122029179b Mon Sep 17 00:00:00 2001 From: Fabricio Dematte Date: Mon, 17 May 2021 21:56:22 -0300 Subject: [PATCH 1/7] Add unknown short flag test --- oof/src/lib.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/oof/src/lib.rs b/oof/src/lib.rs index 6621867..2158ae2 100644 --- a/oof/src/lib.rs +++ b/oof/src/lib.rs @@ -239,6 +239,19 @@ mod tests { assert!(!flags.is_present("output_file")); } + #[test] + fn test_unknown_short_flag() { + let flags_info = [ + ArgFlag::long("output_file").short('o'), + Flag::long("verbose").short('v'), + Flag::long("help").short('h'), + ]; + + let args = gen_args("ouch a.zip -s b.tar.gz"); + let result = filter_flags(args, &flags_info).unwrap_err(); + assert!(matches!(result, OofError::UnknownShortFlag('s'))); + } + #[test] fn test_pop_subcommand() { let subcommands = &["commit", "add", "push", "remote"]; From dbb329344a43f13f3efc2530bbc27dc2f641569e Mon Sep 17 00:00:00 2001 From: Fabricio Dematte Date: Tue, 18 May 2021 20:53:35 -0300 Subject: [PATCH 2/7] add pre test setup function --- oof/src/lib.rs | 32 +++++++++++++++++++------------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/oof/src/lib.rs b/oof/src/lib.rs index 2158ae2..53aed70 100644 --- a/oof/src/lib.rs +++ b/oof/src/lib.rs @@ -220,6 +220,25 @@ mod tests { args.map(OsString::from).collect() } + fn setup_args_scenario(arg_str: &str) -> Result<(Vec, Flags), OofError> { + let flags_info = [ + ArgFlag::long("output_file").short('o'), + Flag::long("verbose").short('v'), + Flag::long("help").short('h'), + ]; + + let args = gen_args(arg_str); + filter_flags(args, &flags_info) + } + + #[test] + fn test_unknown_short_flag() { + let result = setup_args_scenario("ouch a.zip -s b.tar.gz c.tar").unwrap_err(); + assert!(matches!(result, OofError::UnknownShortFlag('s'))); + let result = setup_args_scenario("ouch a.zip --foobar b.tar.gz c.tar").unwrap_err(); + // TODO: assert `UnknownLongFlag` error + } + // asdasdsa #[test] fn test_filter_flags() { @@ -239,19 +258,6 @@ mod tests { assert!(!flags.is_present("output_file")); } - #[test] - fn test_unknown_short_flag() { - let flags_info = [ - ArgFlag::long("output_file").short('o'), - Flag::long("verbose").short('v'), - Flag::long("help").short('h'), - ]; - - let args = gen_args("ouch a.zip -s b.tar.gz"); - let result = filter_flags(args, &flags_info).unwrap_err(); - assert!(matches!(result, OofError::UnknownShortFlag('s'))); - } - #[test] fn test_pop_subcommand() { let subcommands = &["commit", "add", "push", "remote"]; From 5e7ee4f959691e20149ab7422cfcb5404ea4e632 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20M=2E=20Bezerra?= Date: Wed, 19 May 2021 12:18:05 -0300 Subject: [PATCH 3/7] Removing lifetime from OofError --- oof/src/error.rs | 10 +++++----- oof/src/lib.rs | 20 +++++++++++--------- 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/oof/src/error.rs b/oof/src/error.rs index 306f176..3da0415 100644 --- a/oof/src/error.rs +++ b/oof/src/error.rs @@ -3,7 +3,7 @@ use std::{error, ffi::OsString, fmt}; use crate::Flag; #[derive(Debug)] -pub enum OofError<'t> { +pub enum OofError { FlagValueConflict { flag: Flag, previous_value: OsString, @@ -15,17 +15,17 @@ pub enum OofError<'t> { UnknownShortFlag(char), UnknownLongFlag(String), MisplacedShortArgFlagError(char), - MissingValueToFlag(&'t Flag), - DuplicatedFlag(&'t Flag), + MissingValueToFlag(Flag), + DuplicatedFlag(Flag), } -impl<'t> error::Error for OofError<'t> { +impl error::Error for OofError { fn source(&self) -> Option<&(dyn error::Error + 'static)> { None } } -impl<'t> fmt::Display for OofError<'t> { +impl fmt::Display for OofError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { // TODO: implement proper debug messages match self { diff --git a/oof/src/lib.rs b/oof/src/lib.rs index 53aed70..c03fe5b 100644 --- a/oof/src/lib.rs +++ b/oof/src/lib.rs @@ -131,7 +131,7 @@ pub fn filter_flags( let is_last_letter = i == letters.len() - 1; let flag_info = - short_flags_info.get(&letter).ok_or(OofError::UnknownShortFlag(letter))?; + *short_flags_info.get(&letter).ok_or(OofError::UnknownShortFlag(letter))?; if !is_last_letter && flag_info.takes_value { return Err(OofError::MisplacedShortArgFlagError(letter)); @@ -145,19 +145,20 @@ pub fn filter_flags( if flag_info.takes_value { // If it was already inserted if result_flags.argument_flags.contains_key(flag_name) { - return Err(OofError::DuplicatedFlag(flag_info)); + return Err(OofError::DuplicatedFlag(flag_info.clone())); } // pop the next one - let flag_argument = - iter.next().ok_or(OofError::MissingValueToFlag(flag_info))?; + let flag_argument = iter + .next() + .ok_or_else(|| OofError::MissingValueToFlag(flag_info.clone()))?; // Otherwise, insert it. result_flags.argument_flags.insert(flag_name, flag_argument); } else { // If it was already inserted if result_flags.boolean_flags.contains(flag_name) { - return Err(OofError::DuplicatedFlag(flag_info)); + return Err(OofError::DuplicatedFlag(flag_info.clone())); } // Otherwise, insert it result_flags.boolean_flags.insert(flag_name); @@ -168,7 +169,7 @@ pub fn filter_flags( if let FlagType::Long = flag_type { let flag = trim_double_hyphen(flag); - let flag_info = long_flags_info + let flag_info = *long_flags_info .get(flag) .ok_or_else(|| OofError::UnknownLongFlag(String::from(flag)))?; @@ -177,15 +178,16 @@ pub fn filter_flags( if flag_info.takes_value { // If it was already inserted if result_flags.argument_flags.contains_key(&flag_name) { - return Err(OofError::DuplicatedFlag(flag_info)); + return Err(OofError::DuplicatedFlag(flag_info.clone())); } - let flag_argument = iter.next().ok_or(OofError::MissingValueToFlag(flag_info))?; + let flag_argument = + iter.next().ok_or_else(|| OofError::MissingValueToFlag(flag_info.clone()))?; result_flags.argument_flags.insert(flag_name, flag_argument); } else { // If it was already inserted if result_flags.boolean_flags.contains(&flag_name) { - return Err(OofError::DuplicatedFlag(flag_info)); + return Err(OofError::DuplicatedFlag(flag_info.clone())); } // Otherwise, insert it result_flags.boolean_flags.insert(&flag_name); From f203b80eb882761f734eac04ccbd25ff3b02c147 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20M=2E=20Bezerra?= Date: Wed, 19 May 2021 12:28:49 -0300 Subject: [PATCH 4/7] Fixing ouch OofError convertion --- src/error.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/error.rs b/src/error.rs index 83657e1..a64ff91 100644 --- a/src/error.rs +++ b/src/error.rs @@ -120,7 +120,7 @@ impl From for Error { } } -impl<'t> From> for Error { +impl From for Error { fn from(err: oof::OofError) -> Self { // To avoid entering a lifetime hell, we'll just print the Oof error here // and skip saving it into a variant of Self From c057d9c682433b73bfa1a4874b3c1856036a4274 Mon Sep 17 00:00:00 2001 From: Fabricio Dematte Date: Wed, 19 May 2021 21:35:50 -0300 Subject: [PATCH 5/7] add further flag testing to oof crate --- oof/src/lib.rs | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/oof/src/lib.rs b/oof/src/lib.rs index c03fe5b..0c1e665 100644 --- a/oof/src/lib.rs +++ b/oof/src/lib.rs @@ -234,11 +234,29 @@ mod tests { } #[test] - fn test_unknown_short_flag() { + fn test_unknown_flags() { let result = setup_args_scenario("ouch a.zip -s b.tar.gz c.tar").unwrap_err(); assert!(matches!(result, OofError::UnknownShortFlag('s'))); let result = setup_args_scenario("ouch a.zip --foobar b.tar.gz c.tar").unwrap_err(); - // TODO: assert `UnknownLongFlag` error + // TODO: Try removing the `_` from `_foobar` + let _foobar = "foobar".to_string(); + assert!(matches!(result, OofError::UnknownLongFlag(_foobar))); + } + + #[test] + fn test_incomplete_flags() { + // TODO: Try removing the `_` from `_incomplete_flag` + let _incomplete_flag = Flag::long("output_file").short('o'); + let result = setup_args_scenario("ouch a.zip b.tar.gz c.tar -o").unwrap_err(); + assert!(matches!(result, OofError::MissingValueToFlag(_incomplete_flag))); + } + + #[test] + fn test_duplicated_flags() { + // TODO: Try removing the `_` from `_duplicated_flag` + let _duplicated_flag = Flag::long("output_file").short('o'); + let result = setup_args_scenario("ouch a.zip b.tar.gz c.tar -o -o -o").unwrap_err(); + assert!(matches!(result, OofError::DuplicatedFlag(_duplicated_flag))); } // asdasdsa From 67bd239acb3cbf2c9de99b0449e90674fe98e3bf Mon Sep 17 00:00:00 2001 From: Fabricio Dematte Date: Fri, 21 May 2021 08:06:17 -0300 Subject: [PATCH 6/7] fix test asserts to properly match error variants. add misplaced flag test --- oof/src/lib.rs | 46 +++++++++++++++++++++++++++++++++++++--------- 1 file changed, 37 insertions(+), 9 deletions(-) diff --git a/oof/src/lib.rs b/oof/src/lib.rs index 0c1e665..deafff4 100644 --- a/oof/src/lib.rs +++ b/oof/src/lib.rs @@ -237,26 +237,54 @@ mod tests { fn test_unknown_flags() { let result = setup_args_scenario("ouch a.zip -s b.tar.gz c.tar").unwrap_err(); assert!(matches!(result, OofError::UnknownShortFlag('s'))); + + let unknown_long_flag = "foobar".to_string(); let result = setup_args_scenario("ouch a.zip --foobar b.tar.gz c.tar").unwrap_err(); - // TODO: Try removing the `_` from `_foobar` - let _foobar = "foobar".to_string(); - assert!(matches!(result, OofError::UnknownLongFlag(_foobar))); + + assert!(match result { + OofError::UnknownLongFlag(flag) if flag == unknown_long_flag => true, + _ => false, + }) } #[test] fn test_incomplete_flags() { - // TODO: Try removing the `_` from `_incomplete_flag` - let _incomplete_flag = Flag::long("output_file").short('o'); + let incomplete_flag = ArgFlag::long("output_file").short('o'); let result = setup_args_scenario("ouch a.zip b.tar.gz c.tar -o").unwrap_err(); - assert!(matches!(result, OofError::MissingValueToFlag(_incomplete_flag))); + + assert!(match result { + OofError::MissingValueToFlag(flag) if flag == incomplete_flag => true, + _ => false, + }) } #[test] fn test_duplicated_flags() { - // TODO: Try removing the `_` from `_duplicated_flag` - let _duplicated_flag = Flag::long("output_file").short('o'); + let duplicated_flag = ArgFlag::long("output_file").short('o'); let result = setup_args_scenario("ouch a.zip b.tar.gz c.tar -o -o -o").unwrap_err(); - assert!(matches!(result, OofError::DuplicatedFlag(_duplicated_flag))); + + assert!(match result { + OofError::DuplicatedFlag(flag) if flag == duplicated_flag => true, + _ => false, + }) + } + + #[test] + fn test_misplaced_flag() { + let misplaced_flag = ArgFlag::long("output_file").short('o'); + let result = setup_args_scenario("ouch -ov a.zip b.tar.gz c.tar").unwrap_err(); + + assert!(match result { + OofError::MisplacedShortArgFlagError(flag) if flag == misplaced_flag.short.unwrap() => + true, + _ => false, + }) + } + + #[test] + fn test_invalid_unicode_flag() { + // let invalid_unicode_flag = char::from(); + // TODO: how to store invalid unicode? } // asdasdsa From 1059ba03ec3674995dba735b8140abe2fe04d53d Mon Sep 17 00:00:00 2001 From: Fabricio Dematte Date: Sun, 23 May 2021 22:04:17 -0300 Subject: [PATCH 7/7] test every OofError variant done --- oof/src/lib.rs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/oof/src/lib.rs b/oof/src/lib.rs index deafff4..78632f9 100644 --- a/oof/src/lib.rs +++ b/oof/src/lib.rs @@ -215,6 +215,8 @@ where #[cfg(test)] mod tests { + use std::os::unix::prelude::OsStringExt; + use crate::*; fn gen_args(text: &str) -> Vec { @@ -283,8 +285,14 @@ mod tests { #[test] fn test_invalid_unicode_flag() { - // let invalid_unicode_flag = char::from(); - // TODO: how to store invalid unicode? + // `invalid_unicode_flag` has to contain a leading hyphen to be considered a flag. + let invalid_unicode_flag = OsString::from_vec(vec![45, 0, 0, 0, 255, 255, 255, 255]); + let result = filter_flags(vec![invalid_unicode_flag.clone()], &[]).unwrap_err(); + + assert!(match result { + OofError::InvalidUnicode(flag) if flag == invalid_unicode_flag => true, + _ => false, + }) } // asdasdsa