From c83abc9dec72b0e96aa7e07f98a83d105fc8c47a Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Sun, 23 Feb 2025 15:38:59 +0900 Subject: [PATCH] tests: migrate non-snapshot users of jj_cmd_success() to run_jj_in().success() --- cli/tests/common/test_environment.rs | 37 ------------------- cli/tests/test_config_command.rs | 54 +++++++++++++++++----------- cli/tests/test_status_command.rs | 40 +++++++++++---------- cli/tests/test_workspaces.rs | 14 +++++--- 4 files changed, 65 insertions(+), 80 deletions(-) diff --git a/cli/tests/common/test_environment.rs b/cli/tests/common/test_environment.rs index 5907692b6..3a5e84948 100644 --- a/cli/tests/common/test_environment.rs +++ b/cli/tests/common/test_environment.rs @@ -41,24 +41,6 @@ pub struct TestEnvironment { env_vars: HashMap, config_file_number: RefCell, command_number: RefCell, - /// If true, `jj_cmd_success` does not abort when `jj` exits with nonempty - /// stderr and outputs it instead. This is meant only for debugging. - /// - /// This allows debugging the execution of an integration test by inserting - /// `eprintln!` or `dgb!` statements in relevant parts of jj source code. - /// - /// You can change the value of this parameter directly, or you can set the - /// `JJ_DEBUG_ALLOW_STDERR` environment variable. - /// - /// To see the output, you can run `cargo test` with the --show-output - /// argument, like so: - /// - /// RUST_BACKTRACE=1 JJ_DEBUG_ALLOW_STDERR=1 cargo test \ - /// --test test_git_colocated -- --show-output fetch_del - /// - /// This would run all the tests that contain `fetch_del` in their name in a - /// file that contains `test_git_colocated` and show the output. - pub debug_allow_stderr: bool, } impl Default for TestEnvironment { @@ -80,7 +62,6 @@ impl Default for TestEnvironment { env_vars, config_file_number: RefCell::new(0), command_number: RefCell::new(0), - debug_allow_stderr: std::env::var("JJ_DEBUG_ALLOW_STDERR").is_ok(), }; // Use absolute timestamps in the operation log to make tests independent of the // current time. @@ -189,24 +170,6 @@ impl TestEnvironment { self.get_ok(self.jj_cmd(current_dir, args)) } - /// Run a `jj` command, check that it was successful, and return its stdout - #[track_caller] - pub fn jj_cmd_success(&self, current_dir: &Path, args: &[&str]) -> CommandOutputString { - if self.debug_allow_stderr { - let (stdout, stderr) = self.jj_cmd_ok(current_dir, args); - if !stderr.is_empty() { - eprintln!( - "==== STDERR from running jj with {args:?} args in {current_dir:?} \ - ====\n{stderr}==== END STDERR ====" - ); - } - stdout - } else { - let assert = self.jj_cmd(current_dir, args).assert().success().stderr(""); - self.normalize_output(get_stdout_string(&assert)) - } - } - pub fn env_root(&self) -> &Path { &self.env_root } diff --git a/cli/tests/test_config_command.rs b/cli/tests/test_config_command.rs index 07f0bf7dd..6707b2504 100644 --- a/cli/tests/test_config_command.rs +++ b/cli/tests/test_config_command.rs @@ -639,7 +639,9 @@ fn test_config_set_toml_types() { let repo_path = test_env.env_root().join("repo"); let set_value = |key, value| { - test_env.jj_cmd_success(&repo_path, &["config", "set", "--user", key, value]); + test_env + .run_jj_in(&repo_path, ["config", "set", "--user", key, value]) + .success(); }; set_value("test-table.integer", "42"); set_value("test-table.float", "3.14"); @@ -681,22 +683,30 @@ fn test_config_set_type_mismatch() { "); // But it's fine to overwrite arrays and inline tables - test_env.jj_cmd_success( - &repo_path, - &["config", "set", "--user", "test-table.array", "[1,2,3]"], - ); - test_env.jj_cmd_success( - &repo_path, - &["config", "set", "--user", "test-table.array", "[4,5,6]"], - ); - test_env.jj_cmd_success( - &repo_path, - &["config", "set", "--user", "test-table.inline", "{ x = 42}"], - ); - test_env.jj_cmd_success( - &repo_path, - &["config", "set", "--user", "test-table.inline", "42"], - ); + test_env + .run_jj_in( + &repo_path, + ["config", "set", "--user", "test-table.array", "[1,2,3]"], + ) + .success(); + test_env + .run_jj_in( + &repo_path, + ["config", "set", "--user", "test-table.array", "[4,5,6]"], + ) + .success(); + test_env + .run_jj_in( + &repo_path, + ["config", "set", "--user", "test-table.inline", "{ x = 42}"], + ) + .success(); + test_env + .run_jj_in( + &repo_path, + ["config", "set", "--user", "test-table.inline", "42"], + ) + .success(); } #[test] @@ -776,10 +786,12 @@ fn test_config_unset_table_like() { .unwrap(); // Inline table is syntactically a "value", so it can be deleted. - test_env.jj_cmd_success( - test_env.env_root(), - &["config", "unset", "--user", "inline-table"], - ); + test_env + .run_jj_in( + test_env.env_root(), + ["config", "unset", "--user", "inline-table"], + ) + .success(); // Non-inline table cannot be deleted. let output = test_env.run_jj_in( test_env.env_root(), diff --git a/cli/tests/test_status_command.rs b/cli/tests/test_status_command.rs index c2e71e5e6..4903c8e9a 100644 --- a/cli/tests/test_status_command.rs +++ b/cli/tests/test_status_command.rs @@ -428,15 +428,17 @@ fn test_status_untracked_files() { [EOF] "); - test_env.jj_cmd_success( - &repo_path, - &[ - "file", - "track", - "initially-untracked-file", - "sub/initially-untracked", - ], - ); + test_env + .run_jj_in( + &repo_path, + [ + "file", + "track", + "initially-untracked-file", + "sub/initially-untracked", + ], + ) + .success(); let output = test_env.run_jj_in(&repo_path, ["status"]); insta::assert_snapshot!(output.normalize_backslash(), @r" @@ -463,15 +465,17 @@ fn test_status_untracked_files() { [EOF] "); - test_env.jj_cmd_success( - &repo_path, - &[ - "file", - "untrack", - "initially-untracked-file", - "sub/initially-untracked", - ], - ); + test_env + .run_jj_in( + &repo_path, + [ + "file", + "untrack", + "initially-untracked-file", + "sub/initially-untracked", + ], + ) + .success(); let output = test_env.run_jj_in(&repo_path, ["status"]); insta::assert_snapshot!(output.normalize_backslash(), @r" Working copy changes: diff --git a/cli/tests/test_workspaces.rs b/cli/tests/test_workspaces.rs index 28dec7517..1184c9ac8 100644 --- a/cli/tests/test_workspaces.rs +++ b/cli/tests/test_workspaces.rs @@ -823,7 +823,7 @@ fn test_workspaces_current_op_discarded_by_other(automatic: bool) { if automatic { // Run a no-op command to set the randomness seed for commit hashes. - test_env.jj_cmd_success(&secondary_path, &["help"]); + test_env.run_jj_in(&secondary_path, ["help"]).success(); let (stdout, stderr) = test_env.jj_cmd_ok(&secondary_path, &["st"]); insta::assert_snapshot!(stdout, @r" @@ -1147,7 +1147,9 @@ fn test_workspaces_forget_abandon_commits() { "); // delete the default workspace (should not abandon commit since not empty) - test_env.jj_cmd_success(&main_path, &["workspace", "forget", "default"]); + test_env + .run_jj_in(&main_path, ["workspace", "forget", "default"]) + .success(); insta::assert_snapshot!(get_log_output(&test_env, &main_path), @r" ○ 57d63245a308 fourth@ second@ third@ │ ○ 4e8f9d2be039 @@ -1158,7 +1160,9 @@ fn test_workspaces_forget_abandon_commits() { // delete the second workspace (should not abandon commit since other workspaces // still have commit checked out) - test_env.jj_cmd_success(&main_path, &["workspace", "forget", "second"]); + test_env + .run_jj_in(&main_path, ["workspace", "forget", "second"]) + .success(); insta::assert_snapshot!(get_log_output(&test_env, &main_path), @r" ○ 57d63245a308 fourth@ third@ │ ○ 4e8f9d2be039 @@ -1169,7 +1173,9 @@ fn test_workspaces_forget_abandon_commits() { // delete the last 2 workspaces (commit should be abandoned now even though // forgotten in same tx) - test_env.jj_cmd_success(&main_path, &["workspace", "forget", "third", "fourth"]); + test_env + .run_jj_in(&main_path, ["workspace", "forget", "third", "fourth"]) + .success(); insta::assert_snapshot!(get_log_output(&test_env, &main_path), @r" ○ 4e8f9d2be039 ◆ 000000000000