From 66193af4e53799299c04ecf8715e44693f1bff8a Mon Sep 17 00:00:00 2001 From: Teddy Wing Date: Sun, 24 Apr 2016 06:36:32 -0400 Subject: Extract `alias_write_to_file` test function contents to helper Create a new helper function that does the work of `alias_write_to_file_must_write_given_alias_to_file`. We want to be able to add another test of the `write_to_file` method that does the same thing but for non-preexisting aliases. Making a helper method will allow us to avoid duplicating work. --- src/tests.rs | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) (limited to 'src') diff --git a/src/tests.rs b/src/tests.rs index 708121e..e4ec254 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -122,10 +122,7 @@ fn update_alias_id_increments_alias() { } -#[test] -fn alias_write_to_file_must_write_given_alias_to_file() { - let mut alias = update_alias_id_sample_alias(); - +fn alias_write_to_file_helper(alias: &mut Alias) -> String { // Create a new test file let test_file = "./testdata/write_to_file"; fs::copy("./testdata/aliases", test_file).expect("Alias file copy failed"); @@ -148,5 +145,13 @@ fn alias_write_to_file_must_write_given_alias_to_file() { let file_contents: Vec<&str> = file_contents.split('\n').collect(); fs::remove_file(test_file).expect("Failed to delete test file"); - assert_eq!(alias.to_string(), file_contents[file_contents.len() - 2]); + file_contents[file_contents.len() - 2].to_string() +} + +#[test] +fn alias_write_to_file_must_write_given_alias_to_file() { + let mut alias = update_alias_id_sample_alias(); + let alias_line = alias_write_to_file_helper(&mut alias); + + assert_eq!(alias.to_string(), alias_line); } -- cgit v1.2.3 From 7c9439bc11ae95b6f2da59eabf2950d15df4d558 Mon Sep 17 00:00:00 2001 From: Teddy Wing Date: Sun, 24 Apr 2016 08:44:04 -0400 Subject: `write_to_file` test: Extract append lines to closure We want to be able to write a test that ensures aliases can be written to the file even if they don't already exist. To do this, I want to be able to reuse the code in the helper function. We can't use the code that appends to the file because this is relevant only to the `alias_write_to_file_must_write_given_alias_to_file` test (which I just realised should actually be renamed to something more specific). In order to run this code without requiring it to be in the helper function, extract it to a closure that gets passed to the helper. We need to pass `alias` into the function explicitly in order to use it otherwise we get an error on an immutable borrow. --- src/tests.rs | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) (limited to 'src') diff --git a/src/tests.rs b/src/tests.rs index e4ec254..c897a32 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -122,18 +122,13 @@ fn update_alias_id_increments_alias() { } -fn alias_write_to_file_helper(alias: &mut Alias) -> String { +fn alias_write_to_file_helper(alias: &mut Alias, f: F) -> String + where F: Fn(&Alias, &str) { // Create a new test file let test_file = "./testdata/write_to_file"; fs::copy("./testdata/aliases", test_file).expect("Alias file copy failed"); - // Write a duplicate alias so that `write_to_file` is able to append a - // new one - let mut f = OpenOptions::new().append(true).open(test_file) - .expect("Failed to open test file for appending"); - writeln!(f, "{}", Alias { email: "derpy@home.pv".to_owned(), .. alias.clone() } - .to_string()) - .expect("Failed to append matching alias"); + f(alias, test_file); // Write our new alias to the file alias.write_to_file(test_file).expect("`write_to_file` failed"); @@ -151,7 +146,15 @@ fn alias_write_to_file_helper(alias: &mut Alias) -> String { #[test] fn alias_write_to_file_must_write_given_alias_to_file() { let mut alias = update_alias_id_sample_alias(); - let alias_line = alias_write_to_file_helper(&mut alias); + let alias_line = alias_write_to_file_helper(&mut alias, |alias: &Alias, filename: &str| { + // Write a duplicate alias so that `write_to_file` is able to append a + // new one + let mut f = OpenOptions::new().append(true).open(filename) + .expect("Failed to open test file for appending"); + writeln!(f, "{}", Alias { email: "derpy@home.pv".to_owned(), .. alias.clone() } + .to_string()) + .expect("Failed to append matching alias"); + }); assert_eq!(alias.to_string(), alias_line); } -- cgit v1.2.3 From 2e844b5c528a39e75702d0b412b0396080fa103e Mon Sep 17 00:00:00 2001 From: Teddy Wing Date: Sun, 24 Apr 2016 09:31:25 -0400 Subject: Write alias even if we haven't found a matching one If we get an `AliasSearchError::NotFound` error we still want to write the alias to the file. After all, it's an email we haven't encountered yet so we should store an alias for it. If the email exists, we can do what we were doing before and change the current alias to be unique. Rename our "append" `write_to_file` test method to be a little more clear about its purpose. Create a new `write_to_file` test function that tests the case we're adding here. Note that having these two tests together seems to cause a race condition because they're both using the same temporary test file. If we run tests with `RUST_TEST_THREADS=1 cargo test` then they pass. Will see about fixing that next. --- src/alias.rs | 7 +++++-- src/tests.rs | 13 ++++++++++++- 2 files changed, 17 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/src/alias.rs b/src/alias.rs index 8a66db8..0fd05d2 100644 --- a/src/alias.rs +++ b/src/alias.rs @@ -75,8 +75,11 @@ impl Alias { } pub fn write_to_file>(&mut self, file: P) -> Result<(), AliasSearchError> { - let similar_aliases = try!(self.find_in_file(&file)); - self.update_alias_id(similar_aliases); + match self.find_in_file(&file) { + Ok(similar_aliases) => self.update_alias_id(similar_aliases), + Err(AliasSearchError::NotFound) => {}, + Err(e) => return Err(e), + }; let mut f = try!(OpenOptions::new().append(true).open(file)); try!(f.write_all(format!("{}\n", self.to_string()).as_bytes())); diff --git a/src/tests.rs b/src/tests.rs index c897a32..f00d103 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -144,7 +144,7 @@ fn alias_write_to_file_helper(alias: &mut Alias, f: F) -> String } #[test] -fn alias_write_to_file_must_write_given_alias_to_file() { +fn alias_write_to_file_must_write_unique_alias_to_file_if_one_already_exists() { let mut alias = update_alias_id_sample_alias(); let alias_line = alias_write_to_file_helper(&mut alias, |alias: &Alias, filename: &str| { // Write a duplicate alias so that `write_to_file` is able to append a @@ -158,3 +158,14 @@ fn alias_write_to_file_must_write_given_alias_to_file() { assert_eq!(alias.to_string(), alias_line); } + +#[test] +#[allow(unused_variables)] +fn alias_write_to_file_must_write_given_alias_to_file() { + let mut alias = update_alias_id_sample_alias(); + let alias_line = alias_write_to_file_helper( + &mut alias, + |alias: &Alias, filename: &str| {}); + + assert_eq!(alias.to_string(), alias_line); +} -- cgit v1.2.3 From 79e78aaa7c0c77064264ffdad0c3172aa0f5c376 Mon Sep 17 00:00:00 2001 From: Teddy Wing Date: Sun, 24 Apr 2016 09:39:14 -0400 Subject: test: Fix `write_to_file` race condition Have each `write_to_file` test pass in a custom filename to the helper function. This way each test has its own file to deal with, eliminating the race condition when running tests in parallel (the default). --- src/tests.rs | 37 +++++++++++++++++++++---------------- 1 file changed, 21 insertions(+), 16 deletions(-) (limited to 'src') diff --git a/src/tests.rs b/src/tests.rs index f00d103..15b5ae8 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -122,23 +122,23 @@ fn update_alias_id_increments_alias() { } -fn alias_write_to_file_helper(alias: &mut Alias, f: F) -> String +fn alias_write_to_file_helper(alias: &mut Alias, filename: &str, f: F) -> String where F: Fn(&Alias, &str) { // Create a new test file - let test_file = "./testdata/write_to_file"; - fs::copy("./testdata/aliases", test_file).expect("Alias file copy failed"); + let test_file = format!("./testdata/{}", filename); + fs::copy("./testdata/aliases", &test_file).expect("Alias file copy failed"); - f(alias, test_file); + f(alias, &test_file); // Write our new alias to the file - alias.write_to_file(test_file).expect("`write_to_file` failed"); + alias.write_to_file(&test_file).expect("`write_to_file` failed"); // Get the file's contents for testing - let mut f = File::open(test_file).expect("Failed to open test file"); + let mut f = File::open(&test_file).expect("Failed to open test file"); let mut file_contents = String::new(); f.read_to_string(&mut file_contents).expect("Failed to read test file contents"); let file_contents: Vec<&str> = file_contents.split('\n').collect(); - fs::remove_file(test_file).expect("Failed to delete test file"); + fs::remove_file(&test_file).expect("Failed to delete test file"); file_contents[file_contents.len() - 2].to_string() } @@ -146,15 +146,19 @@ fn alias_write_to_file_helper(alias: &mut Alias, f: F) -> String #[test] fn alias_write_to_file_must_write_unique_alias_to_file_if_one_already_exists() { let mut alias = update_alias_id_sample_alias(); - let alias_line = alias_write_to_file_helper(&mut alias, |alias: &Alias, filename: &str| { - // Write a duplicate alias so that `write_to_file` is able to append a - // new one - let mut f = OpenOptions::new().append(true).open(filename) - .expect("Failed to open test file for appending"); - writeln!(f, "{}", Alias { email: "derpy@home.pv".to_owned(), .. alias.clone() } - .to_string()) - .expect("Failed to append matching alias"); - }); + let alias_line = alias_write_to_file_helper( + &mut alias, + "alias_write_to_file_must_write_unique_alias_to_file_if_one_already_exists", + |alias: &Alias, filename: &str| { + // Write a duplicate alias so that `write_to_file` is able to append a + // new one + let mut f = OpenOptions::new().append(true).open(filename) + .expect("Failed to open test file for appending"); + writeln!(f, "{}", Alias { email: "derpy@home.pv".to_owned(), .. alias.clone() } + .to_string()) + .expect("Failed to append matching alias"); + } + ); assert_eq!(alias.to_string(), alias_line); } @@ -165,6 +169,7 @@ fn alias_write_to_file_must_write_given_alias_to_file() { let mut alias = update_alias_id_sample_alias(); let alias_line = alias_write_to_file_helper( &mut alias, + "alias_write_to_file_must_write_given_alias_to_file", |alias: &Alias, filename: &str| {}); assert_eq!(alias.to_string(), alias_line); -- cgit v1.2.3