tests: add helper for failing tests as well

tests/common/mod.rs

@@ -76,6 +76,14 @@ impl TestEnvironment {
         get_stdout_string(&assert)
     }
 
+    /// Run a `jj` command, check that it was successful, and return its stdout
+    // TODO: We should return the stderr instead (or maybe in addition), once we've
+    // fixed errors to go to stderr
+    pub fn jj_cmd_failure(&self, current_dir: &Path, args: &[&str]) -> String {
+        let assert = self.jj_cmd(current_dir, args).assert().failure().stderr("");
+        get_stdout_string(&assert)
+    }
+
     pub fn env_root(&self) -> &Path {
         &self.env_root
     }

tests/test_git_push.rs

@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-use crate::common::{get_stdout_string, TestEnvironment};
+use crate::common::TestEnvironment;
 
 pub mod common;
 
@@ -43,11 +43,9 @@ fn test_git_push() {
     "###);
 
     // When pushing a specific branch, won't push it if it points to an open commit
-    let assert = test_env
+    let stdout =
-        .jj_cmd(&workspace_root, &["git", "push", "--branch", "my-branch"])
+        test_env.jj_cmd_failure(&workspace_root, &["git", "push", "--branch", "my-branch"]);
-        .assert()
+    insta::assert_snapshot!(stdout, @"Error: Won't push open commit
-        .failure();
-    insta::assert_snapshot!(get_stdout_string(&assert), @"Error: Won't push open commit
 ");
 
     // Try pushing a conflict
@@ -59,10 +57,7 @@ fn test_git_push() {
     test_env.jj_cmd_success(&workspace_root, &["rebase", "-d", "@--"]);
     test_env.jj_cmd_success(&workspace_root, &["branch", "my-branch"]);
     test_env.jj_cmd_success(&workspace_root, &["close", "-m", "third"]);
-    let assert = test_env
+    let stdout = test_env.jj_cmd_failure(&workspace_root, &["git", "push"]);
-        .jj_cmd(&workspace_root, &["git", "push"])
+    insta::assert_snapshot!(stdout, @"Error: Won't push commit 28b5642cb786 since it has conflicts
-        .assert()
-        .failure();
-    insta::assert_snapshot!(get_stdout_string(&assert), @"Error: Won't push commit 28b5642cb786 since it has conflicts
 ");
 }

tests/test_global_opts.rs

@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-use crate::common::{get_stdout_string, TestEnvironment};
+use crate::common::TestEnvironment;
 
 pub mod common;
 
@@ -50,22 +50,16 @@ fn test_no_commit_working_copy() {
 #[test]
 fn test_repo_arg_with_init() {
     let test_env = TestEnvironment::default();
-    let assert = test_env
+    let stdout = test_env.jj_cmd_failure(test_env.env_root(), &["init", "-R=.", "repo"]);
-        .jj_cmd(test_env.env_root(), &["init", "-R=.", "repo"])
+    insta::assert_snapshot!(stdout, @"Error: '--repository' cannot be used with 'init'
-        .assert()
-        .failure();
-    insta::assert_snapshot!(get_stdout_string(&assert), @"Error: '--repository' cannot be used with 'init'
 ");
 }
 
 #[test]
 fn test_repo_arg_with_git_clone() {
     let test_env = TestEnvironment::default();
-    let assert = test_env
+    let stdout = test_env.jj_cmd_failure(test_env.env_root(), &["git", "clone", "-R=.", "remote"]);
-        .jj_cmd(test_env.env_root(), &["git", "clone", "-R=.", "remote"])
+    insta::assert_snapshot!(stdout, @"Error: '--repository' cannot be used with 'git clone'
-        .assert()
-        .failure();
-    insta::assert_snapshot!(get_stdout_string(&assert), @"Error: '--repository' cannot be used with 'git clone'
 ");
 }
 
@@ -106,10 +100,7 @@ fn test_invalid_config() {
         "[section]key = value-missing-quotes",
     )
     .unwrap();
-    let assert = test_env
+    let stdout = test_env.jj_cmd_failure(test_env.env_root(), &["init", "repo"]);
-        .jj_cmd(test_env.env_root(), &["init", "repo"])
+    insta::assert_snapshot!(stdout, @"Invalid config: expected newline, found an identifier at line 1 column 10 in config.toml
-        .assert()
-        .failure();
-    insta::assert_snapshot!(get_stdout_string(&assert), @"Invalid config: expected newline, found an identifier at line 1 column 10 in config.toml
 ");
 }

tests/test_untrack_command.rs

@@ -14,7 +14,7 @@
 
 use std::path::PathBuf;
 
-use crate::common::{get_stdout_string, TestEnvironment};
+use crate::common::TestEnvironment;
 
 pub mod common;
 
@@ -39,14 +39,9 @@ fn test_untrack() {
     let files_before = test_env.jj_cmd_success(&repo_path, &["files"]);
 
     // Errors out when a specified file is not ignored
-    let assert = test_env
+    let stdout = test_env.jj_cmd_failure(&repo_path, &["untrack", "file1", "file1.bak"]);
-        .jj_cmd(&repo_path, &["untrack", "file1", "file1.bak"])
+    insta::assert_snapshot!(stdout, @"Error: 'file1' would be added back because it's not ignored. Make sure it's ignored, \
-        .assert()
+         then try again.\n");
-        .failure();
-    assert.stdout(
-        "Error: 'file1' would be added back because it's not ignored. Make sure it's ignored, \
-         then try again.\n",
-    );
     let files_after = test_env.jj_cmd_success(&repo_path, &["files"]);
     // There should be no changes to the state when there was an error
     assert_eq!(files_after, files_before);
@@ -65,12 +60,9 @@ fn test_untrack() {
     assert!(repo_path.join("file2.bak").exists());
 
     // Errors out when multiple specified files are not ignored
-    let assert = test_env
+    let stdout = test_env.jj_cmd_failure(&repo_path, &["untrack", "target"]);
-        .jj_cmd(&repo_path, &["untrack", "target"])
-        .assert()
-        .failure();
     assert_eq!(
-        get_stdout_string(&assert),
+        stdout,
         format!(
             "Error: '{}' and 1 other files would be added back because they're not ignored. Make \
              sure they're ignored, then try again.\n",