From 711f65303cc49f0215d77d33c25d78d624bd5235 Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Sat, 15 Jan 2022 22:06:12 -0800 Subject: [PATCH] tests: use `assert_cmd` for e2e tests I didn't know about `assert_cmd` when I wrote the few e2e tests we have. Let's switch to it and remove our own similar helpers. --- Cargo.lock | 88 +++++++++++++++++++++++++++++++ Cargo.toml | 2 + src/testutils.rs | 73 +++++++++++--------------- tests/smoke_test.rs | 104 ++++++++++++++----------------------- tests/test_init_command.rs | 79 ++++++++++++++-------------- 5 files changed, 198 insertions(+), 148 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2826a1b93..6371535c7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -17,6 +17,20 @@ version = "0.5.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "23b62fc65de8e4e7f52534fb52b0f3ed04746ae267519eef2a83941e8085068b" +[[package]] +name = "assert_cmd" +version = "2.0.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "93ae1ddd39efd67689deb1979d80bad3bf7f2b09c6e6117c8d1f2443b5e2f83e" +dependencies = [ + "bstr", + "doc-comment", + "predicates", + "predicates-core", + "predicates-tree", + "wait-timeout", +] + [[package]] name = "assert_matches" version = "1.5.0" @@ -351,6 +365,12 @@ dependencies = [ "memchr", ] +[[package]] +name = "difflib" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6184e33543162437515c2e2b48714794e37845ec9851711914eec9d308f6ebe8" + [[package]] name = "digest" version = "0.8.1" @@ -392,6 +412,12 @@ dependencies = [ "winapi", ] +[[package]] +name = "doc-comment" +version = "0.3.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fea41bba32d969b513997752735605054bc0dfa92b4c56bf1189f2e174be7a10" + [[package]] name = "either" version = "1.6.1" @@ -413,6 +439,15 @@ dependencies = [ "instant", ] +[[package]] +name = "float-cmp" +version = "0.9.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "98de4bbd547a563b716d8dfa9aad1cb19bfab00f4fa09a6a4ed21dbcf44ce9c4" +dependencies = [ + "num-traits 0.2.14", +] + [[package]] name = "form_urlencoded" version = "1.0.1" @@ -577,6 +612,7 @@ dependencies = [ name = "jujutsu" version = "0.2.0" dependencies = [ + "assert_cmd", "atty", "chrono", "clap 3.1.2", @@ -593,6 +629,7 @@ dependencies = [ "maplit", "pest", "pest_derive", + "predicates", "rand", "regex", "tempfile", @@ -752,6 +789,12 @@ dependencies = [ "version_check", ] +[[package]] +name = "normalize-line-endings" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "61807f77802ff30975e01f4f071c8ba10c022052f98b3294119f3e615d13e5be" + [[package]] name = "num-integer" version = "0.1.44" @@ -919,6 +962,36 @@ version = "0.2.10" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ac74c624d6b2d21f425f752262f42188365d7b8ff1aff74c82e45136510a4857" +[[package]] +name = "predicates" +version = "2.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a5aab5be6e4732b473071984b3164dbbfb7a3674d30ea5ff44410b6bcd960c3c" +dependencies = [ + "difflib", + "float-cmp", + "itertools", + "normalize-line-endings", + "predicates-core", + "regex", +] + +[[package]] +name = "predicates-core" +version = "1.0.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "da1c2388b1513e1b605fcec39a95e0a9e8ef088f71443ef37099fa9ae6673fcb" + +[[package]] +name = "predicates-tree" +version = "1.0.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4d86de6de25020a36c6d3643a86d9a6a9f552107c0559c60ea03551b5e16c032" +dependencies = [ + "predicates-core", + "termtree", +] + [[package]] name = "proc-macro2" version = "1.0.27" @@ -1267,6 +1340,12 @@ dependencies = [ "winapi-util", ] +[[package]] +name = "termtree" +version = "0.2.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "507e9898683b6c43a9aa55b64259b721b52ba226e0f3779137e50ad114a4c90b" + [[package]] name = "test-case" version = "1.2.3" @@ -1455,6 +1534,15 @@ version = "0.9.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "49874b5167b65d7193b8aba1567f5c7d93d001cafc34600cee003eda787e483f" +[[package]] +name = "wait-timeout" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9f200f5b12eb75f8c1ed65abd4b2db8a6e1b138a20de009dacee265a2498f3f6" +dependencies = [ + "libc", +] + [[package]] name = "walkdir" version = "2.3.2" diff --git a/Cargo.toml b/Cargo.toml index ef1e2d35d..578d11782 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -24,6 +24,7 @@ harness = false members = ["lib"] [dependencies] +assert_cmd = "2.0.4" atty = "0.2.14" chrono = "0.4.19" clap = { version = "3.1.0", features = ["cargo"] } @@ -49,3 +50,4 @@ test-case = "1.2.3" regex = "1.5.4" criterion = "0.3.5" criterion_bencher_compat = "0.3.4" +predicates = "2.1.1" diff --git a/src/testutils.rs b/src/testutils.rs index c2770a911..048312c51 100644 --- a/src/testutils.rs +++ b/src/testutils.rs @@ -12,56 +12,43 @@ // See the License for the specific language governing permissions and // limitations under the License. -use std::fmt::{Debug, Error, Formatter}; -use std::io::Cursor; use std::path::{Path, PathBuf}; -use jujutsu_lib::testutils::{new_user_home, user_settings}; +use tempfile::TempDir; -use crate::commands; -use crate::ui::Ui; - -pub struct CommandRunner { - pub cwd: PathBuf, - pub stdout_buf: Vec, +pub struct TestEnvironment { + _temp_dir: TempDir, + env_root: PathBuf, + home_dir: PathBuf, } -impl CommandRunner { - pub fn new(cwd: &Path) -> CommandRunner { - CommandRunner { - cwd: cwd.to_owned(), - stdout_buf: vec![], +impl Default for TestEnvironment { + fn default() -> Self { + let tmp_dir = TempDir::new().unwrap(); + let env_root = tmp_dir.path().canonicalize().unwrap(); + let home_dir = env_root.join("home"); + Self { + _temp_dir: tmp_dir, + env_root, + home_dir, } } +} - pub fn run(self, mut args: Vec<&str>) -> CommandOutput { - let _home_dir = new_user_home(); - let mut stdout_buf = self.stdout_buf; - let stdout = Box::new(Cursor::new(&mut stdout_buf)); - let ui = Ui::new(self.cwd, stdout, false, user_settings()); - args.insert(0, "jj"); - let status = commands::dispatch(ui, args); - CommandOutput { status, stdout_buf } - } -} - -#[derive(PartialEq, Eq)] -pub struct CommandOutput { - pub status: i32, - pub stdout_buf: Vec, -} - -impl CommandOutput { - pub fn stdout_string(&self) -> String { - String::from_utf8(self.stdout_buf.clone()).unwrap() - } -} - -impl Debug for CommandOutput { - fn fmt(&self, f: &mut Formatter<'_>) -> Result<(), Error> { - f.debug_struct("CommandOutput") - .field("status", &self.status) - .field("stdout_buf", &String::from_utf8_lossy(&self.stdout_buf)) - .finish() +impl TestEnvironment { + pub fn jj_cmd(&self, current_dir: &Path, args: &[&str]) -> assert_cmd::Command { + let mut cmd = assert_cmd::Command::cargo_bin("jj").unwrap(); + cmd.current_dir(current_dir); + cmd.args(args); + cmd.env("HOME", self.home_dir.to_str().unwrap()); + cmd + } + + pub fn env_root(&self) -> &Path { + &self.env_root + } + + pub fn home_dir(&self) -> &Path { + &self.home_dir } } diff --git a/tests/smoke_test.rs b/tests/smoke_test.rs index 18a97b391..0d1c23010 100644 --- a/tests/smoke_test.rs +++ b/tests/smoke_test.rs @@ -12,35 +12,29 @@ // See the License for the specific language governing permissions and // limitations under the License. -use jujutsu::testutils; +use jujutsu::testutils::TestEnvironment; use regex::Regex; #[test] fn smoke_test() { - let temp_dir = tempfile::tempdir().unwrap(); - - let output = testutils::CommandRunner::new(temp_dir.path()).run(vec!["init", "repo"]); - assert_eq!(output.status, 0); - let repo_path = temp_dir.path().join("repo"); + let test_env = TestEnvironment::default(); + test_env + .jj_cmd(test_env.env_root(), &["init", "repo"]) + .assert() + .success(); + let repo_path = test_env.env_root().join("repo"); // Check the output of `jj status` right after initializing repo - let output = testutils::CommandRunner::new(&repo_path).run(vec!["status"]); - assert_eq!(output.status, 0); - let stdout_string = output.stdout_string(); - let output_regex = Regex::new( - "^Parent commit: 000000000000[ ] + let assert = test_env.jj_cmd(&repo_path, &["status"]).assert().success(); + let stdout_string_empty = String::from_utf8(assert.get_output().stdout.clone()).unwrap(); + let output_regex = "^Parent commit: 000000000000[ ] Working copy : ([[:xdigit:]]+)[ ] The working copy is clean -$", - ) - .unwrap(); - assert!( - output_regex.is_match(&stdout_string), - "output was: {}", - stdout_string - ); - let wc_hex_id_empty = output_regex - .captures(&stdout_string) +$"; + assert.stdout(predicates::str::is_match(output_regex).unwrap()); + let wc_hex_id_empty = Regex::new(output_regex) + .unwrap() + .captures(&stdout_string_empty) .unwrap() .get(1) .unwrap() @@ -52,26 +46,19 @@ $", std::fs::write(repo_path.join("file2"), "file2").unwrap(); std::fs::write(repo_path.join("file3"), "file3").unwrap(); - let output = testutils::CommandRunner::new(&repo_path).run(vec!["status"]); - assert_eq!(output.status, 0); - let stdout_string = output.stdout_string(); - let output_regex = Regex::new( - "^Parent commit: 000000000000[ ] + let assert = test_env.jj_cmd(&repo_path, &["status"]).assert().success(); + let stdout_string_non_empty = String::from_utf8(assert.get_output().stdout.clone()).unwrap(); + let output_regex = "^Parent commit: 000000000000[ ] Working copy : ([[:xdigit:]]+)[ ] Working copy changes: A file1 A file2 A file3 -$", - ) - .unwrap(); - assert!( - output_regex.is_match(&stdout_string), - "output was: {}", - stdout_string - ); - let wc_hex_id_non_empty = output_regex - .captures(&stdout_string) +$"; + assert.stdout(predicates::str::is_match(output_regex).unwrap()); + let wc_hex_id_non_empty = Regex::new(output_regex) + .unwrap() + .captures(&stdout_string_non_empty) .unwrap() .get(1) .unwrap() @@ -79,40 +66,25 @@ $", .to_owned(); // The working copy's id should have changed - assert_ne!(wc_hex_id_empty, wc_hex_id_non_empty); + assert_ne!(wc_hex_id_non_empty, wc_hex_id_empty); // Running `jj status` again gives the same output - let output2 = testutils::CommandRunner::new(&repo_path).run(vec!["status"]); - assert_eq!(output, output2); + let assert = test_env.jj_cmd(&repo_path, &["status"]).assert().success(); + let stdout_string_again = String::from_utf8(assert.get_output().stdout.clone()).unwrap(); + assert_eq!(stdout_string_again, stdout_string_non_empty); // Add a commit description - let output = - testutils::CommandRunner::new(&repo_path).run(vec!["describe", "-m", "add some files"]); - assert_eq!(output.status, 0); - let stdout_string = output.stdout_string(); - let output_regex = Regex::new( - "^Working copy now at: [[:xdigit:]]+ add some files -$", - ) - .unwrap(); - assert!( - output_regex.is_match(&stdout_string), - "output was: {}", - stdout_string - ); + let assert = test_env + .jj_cmd(&repo_path, &["describe", "-m", "add some files"]) + .assert() + .success(); + let output_regex = "^Working copy now at: [[:xdigit:]]+ add some files +$"; + assert.stdout(predicates::str::is_match(output_regex).unwrap()); // Close the commit - let output = testutils::CommandRunner::new(&repo_path).run(vec!["close"]); - assert_eq!(output.status, 0); - let stdout_string = output.stdout_string(); - let output_regex = Regex::new( - "^Working copy now at: [[:xdigit:]]+[ ] -$", - ) - .unwrap(); - assert!( - output_regex.is_match(&stdout_string), - "output was: {}", - stdout_string - ); + let assert = test_env.jj_cmd(&repo_path, &["close"]).assert().success(); + let output_regex = "^Working copy now at: [[:xdigit:]]+[ ] +$"; + assert.stdout(predicates::str::is_match(output_regex).unwrap()); } diff --git a/tests/test_init_command.rs b/tests/test_init_command.rs index 74c4286e7..2f781bff0 100644 --- a/tests/test_init_command.rs +++ b/tests/test_init_command.rs @@ -12,83 +12,91 @@ // See the License for the specific language governing permissions and // limitations under the License. -use jujutsu::testutils; +use jujutsu::testutils::TestEnvironment; #[test] fn test_init_git_internal() { - let temp_dir = tempfile::tempdir().unwrap(); - let output = testutils::CommandRunner::new(temp_dir.path()).run(vec!["init", "repo", "--git"]); - assert_eq!(output.status, 0); + let test_env = TestEnvironment::default(); + let assert = test_env + .jj_cmd(test_env.env_root(), &["init", "repo", "--git"]) + .assert() + .success(); - let workspace_root = temp_dir.path().join("repo"); + let workspace_root = test_env.env_root().join("repo"); let jj_path = workspace_root.join(".jj"); let repo_path = jj_path.join("repo"); let store_path = repo_path.join("store"); assert!(workspace_root.is_dir()); assert!(jj_path.is_dir()); assert!(jj_path.join("working_copy").is_dir()); + assert.stdout(format!( + "Initialized repo in \"{}\"\n", + workspace_root.to_str().unwrap() + )); assert!(repo_path.is_dir()); assert!(store_path.is_dir()); assert!(store_path.join("git").is_dir()); assert!(store_path.join("git_target").is_file()); let git_target_file_contents = std::fs::read_to_string(store_path.join("git_target")).unwrap(); assert_eq!(git_target_file_contents, "git"); - assert_eq!( - output.stdout_string(), - format!( - "Initialized repo in \"{}\"\n", - workspace_root.to_str().unwrap() - ) - ); } #[test] fn test_init_git_external() { - let temp_dir = tempfile::tempdir().unwrap(); - let git_repo_path = temp_dir.path().join("git-repo"); + let test_env = TestEnvironment::default(); + let git_repo_path = test_env.env_root().join("git-repo"); git2::Repository::init(git_repo_path.clone()).unwrap(); + let assert = test_env + .jj_cmd( + test_env.env_root(), + &[ + "init", + "repo", + "--git-repo", + git_repo_path.to_str().unwrap(), + ], + ) + .assert() + .success(); - let output = testutils::CommandRunner::new(temp_dir.path()).run(vec![ - "init", - "repo", - "--git-repo", - git_repo_path.to_str().unwrap(), - ]); - assert_eq!(output.status, 0); - - let workspace_root = temp_dir.path().join("repo"); + let workspace_root = test_env.env_root().join("repo"); let jj_path = workspace_root.join(".jj"); let repo_path = jj_path.join("repo"); let store_path = repo_path.join("store"); assert!(workspace_root.is_dir()); assert!(jj_path.is_dir()); assert!(jj_path.join("working_copy").is_dir()); + assert.stdout(format!( + "Initialized repo in \"{}\"\n", + workspace_root.display() + )); assert!(repo_path.is_dir()); assert!(store_path.is_dir()); let git_target_file_contents = std::fs::read_to_string(store_path.join("git_target")).unwrap(); assert!(git_target_file_contents .replace('\\', "/") .ends_with("/git-repo")); - assert_eq!( - output.stdout_string(), - format!("Initialized repo in \"{}\"\n", workspace_root.display()) - ); } #[test] fn test_init_local() { - let temp_dir = tempfile::tempdir().unwrap(); + let test_env = TestEnvironment::default(); + let assert = test_env + .jj_cmd(test_env.env_root(), &["init", "repo"]) + .assert() + .success(); - let output = testutils::CommandRunner::new(temp_dir.path()).run(vec!["init", "repo"]); - assert_eq!(output.status, 0); - - let workspace_root = temp_dir.path().join("repo"); + let workspace_root = test_env.env_root().join("repo"); let jj_path = workspace_root.join(".jj"); let repo_path = jj_path.join("repo"); let store_path = repo_path.join("store"); assert!(workspace_root.is_dir()); assert!(jj_path.is_dir()); assert!(jj_path.join("working_copy").is_dir()); + assert.stdout(format!( + "Initialized repo in \"{}\"\n", + workspace_root.display() + )); assert!(repo_path.is_dir()); assert!(store_path.is_dir()); assert!(store_path.join("commits").is_dir()); @@ -96,11 +104,4 @@ fn test_init_local() { assert!(store_path.join("files").is_dir()); assert!(store_path.join("symlinks").is_dir()); assert!(store_path.join("conflicts").is_dir()); - assert_eq!( - output.stdout_string(), - format!( - "Initialized repo in \"{}\"\n", - workspace_root.to_str().unwrap() - ) - ); }