tests: add helper method to capture both stdout/stderr outputs

This pattern is common, and should have a shorthand so that we can easily
add warning messages.

tests/common/mod.rs

@@ -135,22 +135,30 @@ impl TestEnvironment {
         cmd
     }
 
+    /// Run a `jj` command, check that it was successful, and return its
+    /// `(stdout, stderr)`.
+    pub fn jj_cmd_ok(&self, current_dir: &Path, args: &[&str]) -> (String, String) {
+        let assert = self.jj_cmd(current_dir, args).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
     pub fn jj_cmd_success(&self, current_dir: &Path, args: &[&str]) -> String {
-        let assert = if self.debug_allow_stderr {
+        if self.debug_allow_stderr {
-            let a = self.jj_cmd(current_dir, args).assert().success();
+            let (stdout, stderr) = self.jj_cmd_ok(current_dir, args);
-            let stderr = self.normalize_output(&get_stderr_string(&a));
             if !stderr.is_empty() {
                 eprintln!(
                     "==== STDERR from running jj with {args:?} args in {current_dir:?} \
                      ====\n{stderr}==== END STDERR ===="
                 );
             }
-            a
+            stdout
         } else {
-            self.jj_cmd(current_dir, args).assert().success().stderr("")
+            let assert = self.jj_cmd(current_dir, args).assert().success().stderr("");
-        };
+            self.normalize_output(&get_stdout_string(&assert))
-        self.normalize_output(&get_stdout_string(&assert))
+        }
     }
 
     /// Run a `jj` command, check that it failed with code 1, and return its

tests/test_branch_command.rs

@@ -14,7 +14,7 @@
 
 use std::path::Path;
 
-use crate::common::{get_stderr_string, get_stdout_string, TestEnvironment};
+use crate::common::TestEnvironment;
 
 pub mod common;
 
@@ -24,13 +24,11 @@ fn test_branch_multiple_names() {
     test_env.jj_cmd_success(test_env.env_root(), &["init", "repo", "--git"]);
     let repo_path = test_env.env_root().join("repo");
 
-    let assert = test_env
+    let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["branch", "set", "foo", "bar"]);
-        .jj_cmd(&repo_path, &["branch", "set", "foo", "bar"])
+    insta::assert_snapshot!(stdout, @"");
-        .assert()
+    insta::assert_snapshot!(stderr, @r###"
-        .success();
+    warning: Updating multiple branches (2).
-    insta::assert_snapshot!(get_stdout_string(&assert), @"");
+    "###);
-    insta::assert_snapshot!(get_stderr_string(&assert), @"warning: Updating multiple branches (2).
-");
 
     insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
     @  bar foo 230dd059e1b0

tests/test_config_command.rs

@@ -41,15 +41,12 @@ fn test_config_list_single() {
 #[test]
 fn test_config_list_nonexistent() {
     let test_env = TestEnvironment::default();
-    let assert = test_env
+    let (stdout, stderr) = test_env.jj_cmd_ok(
-        .jj_cmd(
+        test_env.env_root(),
-            test_env.env_root(),
+        &["config", "list", "nonexistent-test-key"],
-            &["config", "list", "nonexistent-test-key"],
+    );
-        )
+    insta::assert_snapshot!(stdout, @"");
-        .assert()
+    insta::assert_snapshot!(stderr, @r###"
-        .success()
-        .stdout("");
-    insta::assert_snapshot!(common::get_stderr_string(&assert), @r###"
     No matching config key for nonexistent-test-key
     "###);
 }

tests/test_git_colocated.rs

@@ -16,7 +16,7 @@ use std::path::Path;
 
 use git2::Oid;
 
-use crate::common::{get_stderr_string, TestEnvironment};
+use crate::common::TestEnvironment;
 
 pub mod common;
 
@@ -252,12 +252,9 @@ fn test_git_colocated_conflicting_git_refs() {
     git2::Repository::init(&workspace_root).unwrap();
     test_env.jj_cmd_success(&workspace_root, &["init", "--git-repo", "."]);
     test_env.jj_cmd_success(&workspace_root, &["branch", "create", "main"]);
-    let assert = test_env
+    let (stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["branch", "create", "main/sub"]);
-        .jj_cmd(&workspace_root, &["branch", "create", "main/sub"])
+    insta::assert_snapshot!(stdout, @"");
-        .assert()
+    insta::assert_snapshot!(stderr, @r###"
-        .success()
-        .stdout("");
-    insta::assert_snapshot!(get_stderr_string(&assert), @r###"
     Failed to export some branches:
       main/sub
     Hint: Git doesn't allow a branch name that looks like a parent directory of

tests/test_git_import_export.rs

@@ -16,7 +16,7 @@ use std::path::Path;
 use itertools::Itertools as _;
 use jujutsu_lib::backend::{CommitId, ObjectId as _};
 
-use crate::common::{get_stderr_string, TestEnvironment};
+use crate::common::TestEnvironment;
 
 pub mod common;
 
@@ -75,12 +75,9 @@ fn test_git_export_conflicting_git_refs() {
 
     test_env.jj_cmd_success(&repo_path, &["branch", "create", "main"]);
     test_env.jj_cmd_success(&repo_path, &["branch", "create", "main/sub"]);
-    let assert = test_env
+    let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["git", "export"]);
-        .jj_cmd(&repo_path, &["git", "export"])
+    insta::assert_snapshot!(stdout, @"");
-        .assert()
+    insta::assert_snapshot!(stderr, @r###"
-        .success()
-        .stdout("");
-    insta::assert_snapshot!(get_stderr_string(&assert), @r###"
     Failed to export some branches:
       main/sub
     Hint: Git doesn't allow a branch name that looks like a parent directory of

tests/test_global_opts.rs

@@ -428,12 +428,7 @@ fn test_verbose_logging_enabled() {
     // Test that the verbose flag enabled verbose logging
     let test_env = TestEnvironment::default();
 
-    let assert = test_env
+    let (_stdout, stderr) = test_env.jj_cmd_ok(test_env.env_root(), &["version", "-v"]);
-        .jj_cmd(test_env.env_root(), &["version", "-v"])
-        .assert()
-        .success();
-
-    let stderr = get_stderr_string(&assert);
     // Split the first log line into a timestamp and the rest.
     // The timestamp is constant sized so this is a robust operation.
     // Example timestamp: 2022-11-20T06:24:05.477703Z

tests/test_init_command.rs

@@ -14,7 +14,7 @@
 
 use std::path::PathBuf;
 
-use crate::common::{get_stderr_string, get_stdout_string, TestEnvironment};
+use crate::common::TestEnvironment;
 
 pub mod common;
 
@@ -182,14 +182,11 @@ fn test_init_git_internal_but_could_be_colocated() {
     let workspace_root = test_env.env_root().join("repo");
     init_git_repo(&workspace_root);
 
-    let assert = test_env
+    let (stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["init", "--git"]);
-        .jj_cmd(&workspace_root, &["init", "--git"])
+    insta::assert_snapshot!(stdout, @r###"
-        .assert()
-        .success();
-    insta::assert_snapshot!(get_stdout_string(&assert), @r###"
     Initialized repo in "."
     "###);
-    insta::assert_snapshot!(get_stderr_string(&assert), @r###"
+    insta::assert_snapshot!(stderr, @r###"
     Empty repo created.
     Hint: To create a repo backed by the existing Git repo, run `jj init --git-repo=.` instead.
     "###);

tests/test_log_command.rs

@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-use common::{get_stderr_string, get_stdout_string, TestEnvironment};
+use common::{get_stdout_string, TestEnvironment};
 
 pub mod common;
 
@@ -756,63 +756,51 @@ fn test_log_warn_path_might_be_revset() {
     std::fs::write(repo_path.join("file1"), "foo\n").unwrap();
 
     // Don't warn if the file actually exists.
-    let assert = test_env
+    let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["log", "file1", "-T", "description"]);
-        .jj_cmd(&repo_path, &["log", "file1", "-T", "description"])
+    insta::assert_snapshot!(stdout, @r###"
-        .assert()
-        .success();
-    insta::assert_snapshot!(get_stdout_string(&assert), @r###"
     @
     │
     ~
     "###);
-    insta::assert_snapshot!(get_stderr_string(&assert), @"");
+    insta::assert_snapshot!(stderr, @"");
 
     // Warn for `jj log .` specifically, for former Mercurial users.
-    let assert = test_env
+    let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["log", ".", "-T", "description"]);
-        .jj_cmd(&repo_path, &["log", ".", "-T", "description"])
+    insta::assert_snapshot!(stdout, @r###"
-        .assert()
-        .success();
-    insta::assert_snapshot!(get_stdout_string(&assert), @r###"
     @
     │
     ~
     "###);
-    insta::assert_snapshot!(get_stderr_string(&assert), @r###"warning: The argument "." is being interpreted as a path, but this is often not useful because all non-empty commits touch '.'.  If you meant to show the working copy commit, pass -r '@' instead."###);
+    insta::assert_snapshot!(stderr, @r###"
+    warning: The argument "." is being interpreted as a path, but this is often not useful because all non-empty commits touch '.'.  If you meant to show the working copy commit, pass -r '@' instead.
+    "###);
 
     // ...but checking `jj log .` makes sense in a subdirectory.
     let subdir = repo_path.join("dir");
     std::fs::create_dir_all(&subdir).unwrap();
-    let assert = test_env.jj_cmd(&subdir, &["log", "."]).assert().success();
+    let (stdout, stderr) = test_env.jj_cmd_ok(&subdir, &["log", "."]);
-    insta::assert_snapshot!(get_stdout_string(&assert), @"");
+    insta::assert_snapshot!(stdout, @"");
-    insta::assert_snapshot!(get_stderr_string(&assert), @"");
+    insta::assert_snapshot!(stderr, @"");
 
     // Warn for `jj log @` instead of `jj log -r @`.
-    let assert = test_env
+    let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["log", "@", "-T", "description"]);
-        .jj_cmd(&repo_path, &["log", "@", "-T", "description"])
+    insta::assert_snapshot!(stdout, @"");
-        .assert()
+    insta::assert_snapshot!(stderr, @r###"
-        .success();
-    insta::assert_snapshot!(get_stdout_string(&assert), @"");
-    insta::assert_snapshot!(get_stderr_string(&assert), @r###"
     warning: The argument "@" is being interpreted as a path. To specify a revset, pass -r "@" instead.
     "###);
 
     // Warn when there's no path with the provided name.
-    let assert = test_env
+    let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["log", "file2", "-T", "description"]);
-        .jj_cmd(&repo_path, &["log", "file2", "-T", "description"])
+    insta::assert_snapshot!(stdout, @"");
-        .assert()
+    insta::assert_snapshot!(stderr, @r###"
-        .success();
-    insta::assert_snapshot!(get_stdout_string(&assert), @"");
-    insta::assert_snapshot!(get_stderr_string(&assert), @r###"
     warning: The argument "file2" is being interpreted as a path. To specify a revset, pass -r "file2" instead.
     "###);
 
     // If an explicit revision is provided, then suppress the warning.
-    let assert = test_env
+    let (stdout, stderr) =
-        .jj_cmd(&repo_path, &["log", "@", "-r", "@", "-T", "description"])
+        test_env.jj_cmd_ok(&repo_path, &["log", "@", "-r", "@", "-T", "description"]);
-        .assert()
+    insta::assert_snapshot!(stdout, @"");
-        .success();
+    insta::assert_snapshot!(stderr, @r###"
-    insta::assert_snapshot!(get_stdout_string(&assert), @"");
-    insta::assert_snapshot!(get_stderr_string(&assert), @r###"
     "###);
 }
 

tests/test_revset_output.rs

@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-use common::{get_stderr_string, get_stdout_string, TestEnvironment};
+use common::TestEnvironment;
 
 pub mod common;
 
@@ -383,15 +383,12 @@ fn test_bad_alias_decl() {
     );
 
     // Invalid declaration should be warned and ignored.
-    let assert = test_env
+    let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["log", "-r", "my-root"]);
-        .jj_cmd(&repo_path, &["log", "-r", "my-root"])
+    insta::assert_snapshot!(stdout, @r###"
-        .assert()
-        .success();
-    insta::assert_snapshot!(get_stdout_string(&assert), @r###"
     ◉  zzzzzzzzzzzz 1970-01-01 00:00:00.000 +00:00 000000000000
        (empty) (no description set)
     "###);
-    insta::assert_snapshot!(get_stderr_string(&assert), @r###"
+    insta::assert_snapshot!(stderr, @r###"
     Failed to load "revset-aliases."bad"":  --> 1:1
       |
     1 | "bad"

tests/test_split_command.rs

@@ -14,7 +14,7 @@
 
 use std::path::Path;
 
-use crate::common::{get_stderr_string, get_stdout_string, TestEnvironment};
+use crate::common::TestEnvironment;
 
 pub mod common;
 
@@ -110,18 +110,15 @@ fn test_split_by_paths() {
 
     // Insert an empty commit before @- with "split nonexistent"
     test_env.set_up_fake_editor();
-    let assert = test_env
+    let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["split", "-r", "@-", "nonexistent"]);
-        .jj_cmd(&repo_path, &["split", "-r", "@-", "nonexistent"])
+    insta::assert_snapshot!(stdout, @r###"
-        .assert()
-        .success();
-    insta::assert_snapshot!(get_stdout_string(&assert), @r###"
     Rebased 1 descendant commits
     First part: 0647b2cbd0da (no description set)
     Second part: d5d77af65446 (no description set)
     Working copy now at: 86f228dc3a50 (no description set)
     Parent commit      : d5d77af65446 (no description set)
     "###);
-    insta::assert_snapshot!(get_stderr_string(&assert), @r###"
+    insta::assert_snapshot!(stderr, @r###"
     The given paths do not match any file: nonexistent
     "###);
 

tests/test_squash_command.rs

@@ -14,7 +14,7 @@
 
 use std::path::Path;
 
-use crate::common::{get_stderr_string, get_stdout_string, TestEnvironment};
+use crate::common::TestEnvironment;
 
 pub mod common;
 
@@ -249,14 +249,11 @@ fn test_squash_partial() {
 
     // We get a warning if we pass a positional argument that looks like a revset
     test_env.jj_cmd_success(&repo_path, &["undo"]);
-    let assert = test_env
+    let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["squash", "b"]);
-        .jj_cmd(&repo_path, &["squash", "b"])
+    insta::assert_snapshot!(stderr, @r###"
-        .assert()
-        .success();
-    insta::assert_snapshot!(get_stderr_string(&assert), @r###"
     warning: The argument "b" is being interpreted as a path. To specify a revset, pass -r "b" instead.
     "###);
-    insta::assert_snapshot!(get_stdout_string(&assert), @r###"
+    insta::assert_snapshot!(stdout, @r###"
     Working copy now at: 1c4e5596a511 (no description set)
     Parent commit      : 16cc94b4efe9 (no description set)
     "###);

tests/test_templater.rs

@@ -14,7 +14,7 @@
 
 use std::path::Path;
 
-use crate::common::{get_stderr_string, get_stdout_string, TestEnvironment};
+use crate::common::TestEnvironment;
 
 pub mod common;
 
@@ -853,12 +853,10 @@ fn test_templater_bad_alias_decl() {
     );
 
     // Invalid declaration should be warned and ignored.
-    let assert = test_env
+    let (stdout, stderr) =
-        .jj_cmd(&repo_path, &["log", "--no-graph", "-r@-", "-Tmy_commit_id"])
+        test_env.jj_cmd_ok(&repo_path, &["log", "--no-graph", "-r@-", "-Tmy_commit_id"]);
-        .assert()
+    insta::assert_snapshot!(stdout, @"000000000000");
-        .success();
+    insta::assert_snapshot!(stderr, @r###"
-    insta::assert_snapshot!(get_stdout_string(&assert), @"000000000000");
-    insta::assert_snapshot!(get_stderr_string(&assert), @r###"
     Failed to load "template-aliases.badfn(a, a)":  --> 1:7
       |
     1 | badfn(a, a)