From 7ab5b6852bde8ab00af660d9a5e8d56a5e861e86 Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Thu, 19 Dec 2024 15:10:31 +0900 Subject: [PATCH] tests: accept convertible type by set_config_path(), add_config(), add_env_var() It's convenient. We occasionally pass format!()-ed value to these functions. --- cli/tests/common/mod.rs | 14 +++++++------- cli/tests/test_config_command.rs | 22 +++++++++++----------- cli/tests/test_fix_command.rs | 22 +++++++++++----------- 3 files changed, 29 insertions(+), 29 deletions(-) diff --git a/cli/tests/common/mod.rs b/cli/tests/common/mod.rs index 7b69f45ad..2b5f93dff 100644 --- a/cli/tests/common/mod.rs +++ b/cli/tests/common/mod.rs @@ -254,11 +254,11 @@ impl TestEnvironment { .join(format!("config{config_file_number:04}.toml")) } - pub fn set_config_path(&mut self, config_path: PathBuf) { - self.config_path = config_path; + pub fn set_config_path(&mut self, config_path: impl Into) { + self.config_path = config_path.into(); } - pub fn add_config(&self, content: &str) { + pub fn add_config(&self, content: impl AsRef<[u8]>) { if self.config_path.is_file() { panic!("add_config not supported when config_path is a file"); } @@ -275,8 +275,8 @@ impl TestEnvironment { .unwrap(); } - pub fn add_env_var(&mut self, key: &str, val: &str) { - self.env_vars.insert(key.to_string(), val.to_string()); + pub fn add_env_var(&mut self, key: impl Into, val: impl Into) { + self.env_vars.insert(key.into(), val.into()); } pub fn current_operation_id(&self, repo_path: &Path) -> String { @@ -294,7 +294,7 @@ impl TestEnvironment { // in it let escaped_editor_path = editor_path.to_str().unwrap().replace('\\', r"\\"); self.add_env_var("EDITOR", &escaped_editor_path); - self.add_config(&format!( + self.add_config(format!( r###" [ui] merge-editor = "fake-editor" @@ -314,7 +314,7 @@ impl TestEnvironment { /// path pub fn set_up_fake_diff_editor(&mut self) -> PathBuf { let escaped_diff_editor_path = escaped_fake_diff_editor_path(); - self.add_config(&format!( + self.add_config(format!( r###" ui.diff-editor = "fake-diff-editor" merge-tools.fake-diff-editor.program = "{escaped_diff_editor_path}" diff --git a/cli/tests/test_config_command.rs b/cli/tests/test_config_command.rs index 0c213fd3e..e902f316c 100644 --- a/cli/tests/test_config_command.rs +++ b/cli/tests/test_config_command.rs @@ -203,7 +203,7 @@ fn test_config_list_layer() { test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]); // Test with fresh new config file let user_config_path = test_env.config_path().join("config.toml"); - test_env.set_config_path(user_config_path.clone()); + test_env.set_config_path(&user_config_path); let repo_path = test_env.env_root().join("repo"); // User @@ -269,7 +269,7 @@ fn test_config_layer_override_default() { "###); // User - test_env.add_config(&format!("{config_key} = {value:?}\n", value = "user")); + test_env.add_config(format!("{config_key} = {value:?}\n", value = "user")); let stdout = test_env.jj_cmd_success(&repo_path, &["config", "list", config_key]); insta::assert_snapshot!(stdout, @r###" merge-tools.vimdiff.program = "user" @@ -350,7 +350,7 @@ fn test_config_layer_override_env() { "###); // User - test_env.add_config(&format!("{config_key} = {value:?}\n", value = "user")); + test_env.add_config(format!("{config_key} = {value:?}\n", value = "user")); let stdout = test_env.jj_cmd_success(&repo_path, &["config", "list", config_key]); insta::assert_snapshot!(stdout, @r###" ui.editor = "user" @@ -492,7 +492,7 @@ fn test_config_set_for_user() { test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]); // Test with fresh new config file let user_config_path = test_env.config_path().join("config.toml"); - test_env.set_config_path(user_config_path.clone()); + test_env.set_config_path(&user_config_path); let repo_path = test_env.env_root().join("repo"); test_env.jj_cmd_ok( @@ -586,7 +586,7 @@ fn test_config_set_toml_types() { test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]); // Test with fresh new config file let user_config_path = test_env.config_path().join("config.toml"); - test_env.set_config_path(user_config_path.clone()); + test_env.set_config_path(&user_config_path); let repo_path = test_env.env_root().join("repo"); let set_value = |key, value| { @@ -700,7 +700,7 @@ fn test_config_unset_table_like() { let mut test_env = TestEnvironment::default(); // Test with fresh new config file let user_config_path = test_env.config_path().join("config.toml"); - test_env.set_config_path(user_config_path.clone()); + test_env.set_config_path(&user_config_path); std::fs::write( &user_config_path, @@ -740,7 +740,7 @@ fn test_config_unset_for_user() { test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]); // Test with fresh new config file let user_config_path = test_env.config_path().join("config.toml"); - test_env.set_config_path(user_config_path.clone()); + test_env.set_config_path(&user_config_path); let repo_path = test_env.env_root().join("repo"); test_env.jj_cmd_ok(&repo_path, &["config", "set", "--user", "foo", "true"]); @@ -820,7 +820,7 @@ fn test_config_edit_user_new_file() { let mut test_env = TestEnvironment::default(); let user_config_path = test_env.config_path().join("config").join("file.toml"); test_env.set_up_fake_editor(); // set $EDITOR, but added configuration is ignored - test_env.set_config_path(user_config_path.clone()); + test_env.set_config_path(&user_config_path); assert!(!user_config_path.exists()); test_env.jj_cmd_ok(test_env.env_root(), &["config", "edit", "--user"]); @@ -856,7 +856,7 @@ fn test_config_path() { let user_config_path = test_env.env_root().join("config.toml"); let repo_config_path = repo_path.join(PathBuf::from_iter([".jj", "repo", "config.toml"])); - test_env.set_config_path(user_config_path.clone()); + test_env.set_config_path(&user_config_path); insta::assert_snapshot!( test_env.jj_cmd_success(&repo_path, &["config", "path", "--user"]), @@ -1057,7 +1057,7 @@ fn test_config_conditional() { let repo2_path = test_env.home_dir().join("repo2"); // Test with fresh new config file let user_config_path = test_env.env_root().join("config.toml"); - test_env.set_config_path(user_config_path.clone()); + test_env.set_config_path(&user_config_path); std::fs::write( &user_config_path, indoc! {" @@ -1127,7 +1127,7 @@ fn test_config_conditional_without_home_dir() { let repo_path = test_env.env_root().join("repo"); // Test with fresh new config file let user_config_path = test_env.env_root().join("config.toml"); - test_env.set_config_path(user_config_path.clone()); + test_env.set_config_path(&user_config_path); std::fs::write( &user_config_path, format!( diff --git a/cli/tests/test_fix_command.rs b/cli/tests/test_fix_command.rs index 8df6cb2a5..eaa5a0bc3 100644 --- a/cli/tests/test_fix_command.rs +++ b/cli/tests/test_fix_command.rs @@ -34,7 +34,7 @@ fn init_with_fake_formatter(args: &[&str]) -> (TestEnvironment, PathBuf, impl Fn // make a meaningful difference in coverage. Otherwise, we would have to add // dedicated test coverage for the deprecated syntax until it is removed. We use // single quotes here to avoid escaping issues when running the test on Windows. - test_env.add_config(&format!( + test_env.add_config(format!( r#"fix.tool-command = ['{}']"#, [formatter_path.to_str().unwrap()] .iter() @@ -77,7 +77,7 @@ fn test_config_both_legacy_and_table_tools() { let formatter_path = assert_cmd::cargo::cargo_bin("fake-formatter"); assert!(formatter_path.is_file()); let escaped_formatter_path = formatter_path.to_str().unwrap().replace('\\', r"\\"); - test_env.add_config(&format!( + test_env.add_config(format!( r###" [fix] tool-command = ["{formatter}", "--append", "legacy change"] @@ -115,7 +115,7 @@ fn test_config_multiple_tools() { let formatter_path = assert_cmd::cargo::cargo_bin("fake-formatter"); assert!(formatter_path.is_file()); let escaped_formatter_path = formatter_path.to_str().unwrap().replace('\\', r"\\"); - test_env.add_config(&format!( + test_env.add_config(format!( r###" [fix.tools.tool-1] command = ["{formatter}", "--uppercase"] @@ -153,7 +153,7 @@ fn test_config_multiple_tools_with_same_name() { // Multiple definitions with the same `name` are not allowed, because it is // likely to be a mistake, and mistakes are risky when they rewrite files. - test_env.add_config(&format!( + test_env.add_config(format!( r###" [fix.tools.my-tool] command = ["{formatter}", "--uppercase"] @@ -183,7 +183,7 @@ fn test_config_multiple_tools_with_same_name() { For help, see https://jj-vcs.github.io/jj/latest/config/. "); - test_env.set_config_path("/dev/null".into()); + test_env.set_config_path("/dev/null"); let content = test_env.jj_cmd_success(&repo_path, &["file", "show", "foo", "-r", "@"]); insta::assert_snapshot!(content, @"Foo\n"); let content = test_env.jj_cmd_success(&repo_path, &["file", "show", "bar", "-r", "@"]); @@ -199,7 +199,7 @@ fn test_config_tables_overlapping_patterns() { assert!(formatter_path.is_file()); let escaped_formatter_path = formatter_path.to_str().unwrap().replace('\\', r"\\"); - test_env.add_config(&format!( + test_env.add_config(format!( r###" [fix.tools.tool-1] command = ["{formatter}", "--append", "tool-1"] @@ -274,7 +274,7 @@ fn test_config_tables_some_commands_missing() { let formatter_path = assert_cmd::cargo::cargo_bin("fake-formatter"); assert!(formatter_path.is_file()); let escaped_formatter_path = formatter_path.to_str().unwrap().replace('\\', r"\\"); - test_env.add_config(&format!( + test_env.add_config(format!( r###" [fix.tools.tool-1] command = ["{formatter}", "--uppercase"] @@ -309,7 +309,7 @@ fn test_config_tables_empty_patterns_list() { let formatter_path = assert_cmd::cargo::cargo_bin("fake-formatter"); assert!(formatter_path.is_file()); let escaped_formatter_path = formatter_path.to_str().unwrap().replace('\\', r"\\"); - test_env.add_config(&format!( + test_env.add_config(format!( r###" [fix.tools.my-tool-empty-patterns] command = ["{formatter}", "--uppercase"] @@ -339,7 +339,7 @@ fn test_config_filesets() { let formatter_path = assert_cmd::cargo::cargo_bin("fake-formatter"); assert!(formatter_path.is_file()); let escaped_formatter_path = formatter_path.to_str().unwrap().replace('\\', r"\\"); - test_env.add_config(&format!( + test_env.add_config(format!( r###" [fix.tools.my-tool-match-one] command = ["{formatter}", "--uppercase"] @@ -378,7 +378,7 @@ fn test_relative_paths() { let formatter_path = assert_cmd::cargo::cargo_bin("fake-formatter"); assert!(formatter_path.is_file()); let escaped_formatter_path = formatter_path.to_str().unwrap().replace('\\', r"\\"); - test_env.add_config(&format!( + test_env.add_config(format!( r###" [fix.tools.tool] command = ["{formatter}", "--stdout", "Fixed!"] @@ -1145,7 +1145,7 @@ fn test_all_files() { // File D: NOT in patterns, changed in child // Some files will be in subdirectories to make sure we're covering that aspect // of matching. - test_env.add_config(&format!( + test_env.add_config(format!( r###" [fix.tools.tool] command = ["{formatter}", "--append", "fixed"]