tests: migrate unusual patterns of jj_cmd_ok() to run_jj_in()

These callers are manually ported to run_jj_in().

cli/tests/common/test_environment.rs

@@ -29,8 +29,6 @@ use tempfile::TempDir;
 
 use super::fake_diff_editor_path;
 use super::fake_editor_path;
-use super::get_stderr_string;
-use super::get_stdout_string;
 use super::strip_last_line;
 use super::to_toml_value;
 
@@ -153,24 +151,6 @@ impl TestEnvironment {
         cmd
     }
 
-    fn get_ok(&self, mut cmd: assert_cmd::Command) -> (CommandOutputString, CommandOutputString) {
-        let assert = cmd.assert().success();
-        let stdout = self.normalize_output(get_stdout_string(&assert));
-        let stderr = self.normalize_output(get_stderr_string(&assert));
-        (stdout, stderr)
-    }
-
-    /// Run a `jj` command, check that it was successful, and return its
-    /// `(stdout, stderr)`.
-    // TODO: remove jj_cmd_*() in favor of run_jj_*()
-    pub fn jj_cmd_ok(
-        &self,
-        current_dir: &Path,
-        args: &[&str],
-    ) -> (CommandOutputString, CommandOutputString) {
-        self.get_ok(self.jj_cmd(current_dir, args))
-    }
-
     pub fn env_root(&self) -> &Path {
         &self.env_root
     }

cli/tests/test_bookmark_command.rs

@@ -519,8 +519,7 @@ fn test_bookmark_move_conflicting() {
     let repo_path = test_env.env_root().join("repo");
     let get_log = || {
         let template = r#"separate(" ", description.first_line(), bookmarks)"#;
-        let (stdout, _stderr) = test_env.jj_cmd_ok(&repo_path, &["log", "-T", template]);
+        test_env.run_jj_in(&repo_path, ["log", "-T", template])
-        stdout
     };
 
     test_env
@@ -564,6 +563,9 @@ fn test_bookmark_move_conflicting() {
     ├─╯
     ◆
     [EOF]
+    ------- stderr -------
+    Concurrent modification detected, resolving automatically.
+    [EOF]
     ");
 
     // Can't move the bookmark to C0 since it's sibling.

cli/tests/test_config_command.rs

@@ -1498,8 +1498,13 @@ fn test_config_author_change_warning() {
     log_cmd.env_remove("JJ_EMAIL");
     log_cmd.assert().success();
 
-    let (stdout, _) = test_env.jj_cmd_ok(&repo_path, &["log"]);
+    let output = test_env.run_jj_in(&repo_path, ["log"]);
-    assert!(stdout.raw().contains("Foo"));
+    insta::assert_snapshot!(output, @r"
+    @  qpvuntsm Foo 2001-02-03 08:05:09 ed1febd8
+    │  (empty) (no description set)
+    ◆  zzzzzzzz root() 00000000
+    [EOF]
+    ");
 }
 
 #[test]

cli/tests/test_diff_command.rs

@@ -2231,15 +2231,15 @@ fn test_diff_external_tool() {
 
     // nonzero exit codes should print a warning
     std::fs::write(&edit_script, "fail").unwrap();
-    let (stdout, stderr) = test_env.jj_cmd_ok(
+    let output = test_env.run_jj_in(
         &repo_path,
-        &["diff", "--config=ui.diff.tool=fake-diff-editor"],
+        ["diff", "--config=ui.diff.tool=fake-diff-editor"],
     );
     let mut insta_settings = insta::Settings::clone_current();
     insta_settings.add_filter("exit (status|code)", "<exit status>");
     insta_settings.bind(|| {
-        insta::assert_snapshot!(stdout, @r"");
+        insta::assert_snapshot!(output, @r"
-        insta::assert_snapshot!(stderr, @r"
+        ------- stderr -------
         Warning: Tool exited with <exit status>: 1 (run with --debug to see the exact invocation)
         [EOF]
         ");

cli/tests/test_git_colocated.rs

@@ -509,11 +509,10 @@ fn test_git_colocated_conflicting_git_refs() {
     test_env
         .run_jj_in(&workspace_root, ["bookmark", "create", "-r@", "main"])
         .success();
-    let (stdout, stderr) =
+    let output = test_env.run_jj_in(&workspace_root, ["bookmark", "create", "-r@", "main/sub"]);
-        test_env.jj_cmd_ok(&workspace_root, &["bookmark", "create", "-r@", "main/sub"]);
-    insta::assert_snapshot!(stdout, @"");
     insta::with_settings!({filters => vec![("Failed to set: .*", "Failed to set: ...")]}, {
-        insta::assert_snapshot!(stderr, @r#"
+        insta::assert_snapshot!(output, @r#"
+        ------- stderr -------
         Created 1 bookmarks pointing to qpvuntsm 230dd059 main main/sub | (empty) (no description set)
         Warning: Failed to export some bookmarks:
           main/sub: Failed to set: ...

cli/tests/test_git_import_export.rs

@@ -81,10 +81,10 @@ fn test_git_export_conflicting_git_refs() {
     test_env
         .run_jj_in(&repo_path, ["bookmark", "create", "-r@", "main/sub"])
         .success();
-    let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["git", "export"]);
+    let output = test_env.run_jj_in(&repo_path, ["git", "export"]);
-    insta::assert_snapshot!(stdout, @"");
     insta::with_settings!({filters => vec![("Failed to set: .*", "Failed to set: ...")]}, {
-        insta::assert_snapshot!(stderr, @r#"
+        insta::assert_snapshot!(output, @r#"
+        ------- stderr -------
         Warning: Failed to export some bookmarks:
           main/sub: Failed to set: ...
         Hint: Git doesn't allow a branch name that looks like a parent directory of

cli/tests/test_operations.rs

@@ -1236,8 +1236,15 @@ fn test_op_diff() {
             ],
         )
         .success();
-    let (_, stderr) = test_env.jj_cmd_ok(&repo_path, &["log"]);
+    let output = test_env.run_jj_in(&repo_path, ["log"]);
-    insta::assert_snapshot!(&stderr, @r"
+    insta::assert_snapshot!(output, @r"
+    @  sqpuoqvx test.user@example.com 2001-02-03 08:05:07 c7b48fea
+    │  (empty) (no description set)
+    ◆  pukowqtp someone@example.org 1970-01-01 11:00:00 bookmark-1?? bookmark-1@origin 0cb7e07e
+    │  Commit 1
+    ~
+    [EOF]
+    ------- stderr -------
     Concurrent modification detected, resolving automatically.
     [EOF]
     ");
@@ -1985,8 +1992,15 @@ fn test_op_show() {
             ],
         )
         .success();
-    let (_, stderr) = test_env.jj_cmd_ok(&repo_path, &["log"]);
+    let output = test_env.run_jj_in(&repo_path, ["log"]);
-    insta::assert_snapshot!(&stderr, @r"
+    insta::assert_snapshot!(output, @r"
+    @  sqpuoqvx test.user@example.com 2001-02-03 08:05:07 c7b48fea
+    │  (empty) (no description set)
+    ◆  pukowqtp someone@example.org 1970-01-01 11:00:00 bookmark-1?? bookmark-1@origin 0cb7e07e
+    │  Commit 1
+    ~
+    [EOF]
+    ------- stderr -------
     Concurrent modification detected, resolving automatically.
     [EOF]
     ");

cli/tests/test_shell_completion.rs

@@ -12,22 +12,18 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-use insta::assert_snapshot;
-
 use crate::common::TestEnvironment;
 
 #[test]
 fn test_deprecated_flags() {
     let test_env = TestEnvironment::default();
-    let (stdout, stderr) =
+    let output = test_env
-        test_env.jj_cmd_ok(test_env.env_root(), &["util", "completion", "--bash"]);
+        .run_jj_in(test_env.env_root(), ["util", "completion", "--bash"])
-    assert_snapshot!(
+        .success();
-        stderr,
+    insta::assert_snapshot!(output.stderr, @r"
-        @r"
     Warning: `jj util completion --bash` will be removed in a future version, and this will be a hard error
     Hint: Use `jj util completion bash` instead
     [EOF]
-    "
+    ");
-    );
+    assert!(output.stdout.raw().contains("COMPREPLY"), "{output}");
-    assert!(stdout.raw().contains("COMPREPLY"));
 }

cli/tests/test_split_command.rs

@@ -955,11 +955,12 @@ fn test_split_with_bookmarks(bookmark_behavior: BookmarkBehavior) {
         ["", "next invocation\n", "write\nsecond-commit"].join("\0"),
     )
     .unwrap();
-    let (_, stderr) = test_env.jj_cmd_ok(&main_path, &["split", "file2"]);
+    let output = test_env.run_jj_in(&main_path, ["split", "file2"]);
     match bookmark_behavior {
         BookmarkBehavior::Default | BookmarkBehavior::Modern => {
             insta::allow_duplicates! {
-            insta::assert_snapshot!(stderr, @r"
+            insta::assert_snapshot!(output, @r"
+            ------- stderr -------
             First part: qpvuntsm 63d0c5ed *le-signet* | first-commit
             Second part: mzvwutvl a9f5665f second-commit
             Working copy now at: mzvwutvl a9f5665f second-commit
@@ -978,7 +979,8 @@ fn test_split_with_bookmarks(bookmark_behavior: BookmarkBehavior) {
         }
         BookmarkBehavior::Legacy => {
             insta::allow_duplicates! {
-            insta::assert_snapshot!(stderr, @r"
+            insta::assert_snapshot!(output, @r"
+            ------- stderr -------
             Warning: `jj split` will leave bookmarks on the first commit in the next release.
             Warning: Run `jj config set --user split.legacy-bookmark-behavior false` to silence this message and use the new behavior.
             Warning: See https://github.com/jj-vcs/jj/issues/3419

cli/tests/test_util_command.rs

@@ -124,10 +124,14 @@ fn test_shell_completions() {
     fn test(shell: &str) {
         let test_env = TestEnvironment::default();
         // Use the local backend because GitBackend::gc() depends on the git CLI.
-        let (out, err) = test_env.jj_cmd_ok(test_env.env_root(), &["util", "completion", shell]);
+        let output = test_env
+            .run_jj_in(test_env.env_root(), ["util", "completion", shell])
+            .success();
         // Ensures only stdout contains text
-        assert!(!out.is_empty());
+        assert!(
-        assert!(err.is_empty());
+            !output.stdout.is_empty() && output.stderr.is_empty(),
+            "{output}"
+        );
     }
 
     test("bash");
@@ -140,9 +144,9 @@ fn test_shell_completions() {
 fn test_util_exec() {
     let test_env = TestEnvironment::default();
     let formatter_path = assert_cmd::cargo::cargo_bin("fake-formatter");
-    let (out, err) = test_env.jj_cmd_ok(
+    let output = test_env.run_jj_in(
         test_env.env_root(),
-        &[
+        [
             "util",
             "exec",
             "--",
@@ -151,9 +155,8 @@ fn test_util_exec() {
             "hello",
         ],
     );
-    insta::assert_snapshot!(out, @"hello[EOF]");
     // Ensures only stdout contains text
-    assert!(err.is_empty());
+    insta::assert_snapshot!(output, @"hello[EOF]");
 }
 
 #[test]