tests: check exit code on failure, and fix a bug in argument parsing

We didn't have any testing of exit codes on failure, other than
checking that they were not 0. This patch changes that so we always
check. Since we have the special exit code 2 (set by `clap`) for
incorrect command line, I've replaced some testing of error messages
by testing of just the exit code.

As part of this, I also fixed `jj branch --allow-backwards` to
actually require `-r` (it didn't before because having a default value
means the argument is considered always provided).

src/commands.rs

@@ -1593,8 +1593,8 @@ struct BackoutArgs {
 #[derive(clap::Args, Clone, Debug)]
 struct BranchArgs {
     /// The branch's target revision
-    #[clap(long, short, default_value = "@", group = "action")]
-    revision: String,
+    #[clap(long, short, group = "action")]
+    revision: Option<String>,
 
     /// Allow moving the branch backwards or sideways
     #[clap(long, requires = "revision")]
@@ -4019,7 +4019,8 @@ fn cmd_branch(ui: &mut Ui, command: &CommandHelper, args: &BranchArgs) -> Result
             ))?;
         }
 
-        let target_commit = workspace_command.resolve_single_rev(ui, &args.revision)?;
+        let target_commit =
+            workspace_command.resolve_single_rev(ui, args.revision.as_deref().unwrap_or("@"))?;
         if !args.allow_backwards
             && !branch_names.iter().all(|branch_name| {
                 is_fast_forward(

tests/common/mod.rs

@@ -77,12 +77,19 @@ impl TestEnvironment {
         get_stdout_string(&assert)
     }
 
-    /// Run a `jj` command, check that it was successful, and return its stdout
+    /// Run a `jj` command, check that it failed with code 1, and return its
+    /// stderr
     pub fn jj_cmd_failure(&self, current_dir: &Path, args: &[&str]) -> String {
-        let assert = self.jj_cmd(current_dir, args).assert().failure().stdout("");
+        let assert = self.jj_cmd(current_dir, args).assert().code(1).stdout("");
         get_stderr_string(&assert)
     }
 
+    /// Run a `jj` command and check that it failed with code 2 (for invalid
+    /// usage)
+    pub fn jj_cmd_cli_error(&self, current_dir: &Path, args: &[&str]) {
+        self.jj_cmd(current_dir, args).assert().code(2).stdout("");
+    }
+
     pub fn env_root(&self) -> &Path {
         &self.env_root
     }

tests/test_branch_command.rs

@@ -24,11 +24,8 @@ fn test_branch_mutually_exclusive_actions() {
     test_env.jj_cmd_success(test_env.env_root(), &["init", "repo", "--git"]);
     let repo_path = test_env.env_root().join("repo");
 
-    test_env.jj_cmd_failure(&repo_path, &["branch", "--forget", "--delete", "foo"]);
-    test_env.jj_cmd_failure(
-        &repo_path,
-        &["branch", "--delete", "--allow-backwards", "foo"],
-    );
+    test_env.jj_cmd_cli_error(&repo_path, &["branch", "--forget", "--delete", "foo"]);
+    test_env.jj_cmd_cli_error(&repo_path, &["branch", "--allow-backwards", "foo"]);
 }
 
 #[test]

tests/test_move_command.rs

@@ -14,8 +14,6 @@
 
 use std::path::Path;
 
-use itertools::Itertools;
-
 use crate::common::TestEnvironment;
 
 pub mod common;
@@ -70,11 +68,7 @@ fn test_move() {
     "###);
 
     // Errors out without arguments
-    let stderr = test_env.jj_cmd_failure(&repo_path, &["move"]);
-    insta::assert_snapshot!(stderr.lines().take(2).join("\n"), @r###"
-    error: The following required arguments were not provided:
-        <--from <FROM>|--to <TO>>
-    "###);
+    test_env.jj_cmd_cli_error(&repo_path, &["move"]);
     // Errors out if source and destination are the same
     let stderr = test_env.jj_cmd_failure(&repo_path, &["move", "--to", "@"]);
     insta::assert_snapshot!(stderr, @r###"

tests/test_rebase_command.rs

@@ -14,8 +14,6 @@
 
 use std::path::Path;
 
-use itertools::Itertools;
-
 use crate::common::TestEnvironment;
 
 pub mod common;
@@ -48,19 +46,13 @@ fn test_rebase_invalid() {
     create_commit(&test_env, &repo_path, "b", &["a"]);
 
     // Missing destination
-    let stderr = test_env.jj_cmd_failure(&repo_path, &["rebase"]);
-    insta::assert_snapshot!(stderr.lines().take(3).join("\n"), @r###"
-    error: The following required arguments were not provided:
-        --destination <DESTINATION>
-    "###);
+    test_env.jj_cmd_cli_error(&repo_path, &["rebase"]);
 
     // Both -r and -s
-    let stderr = test_env.jj_cmd_failure(&repo_path, &["rebase", "-r", "a", "-s", "a", "-d", "b"]);
-    insta::assert_snapshot!(stderr.lines().next().unwrap(), @"error: The argument '--revision <REVISION>' cannot be used with '--source <SOURCE>'");
+    test_env.jj_cmd_cli_error(&repo_path, &["rebase", "-r", "a", "-s", "a", "-d", "b"]);
 
     // Both -b and -s
-    let stderr = test_env.jj_cmd_failure(&repo_path, &["rebase", "-b", "a", "-s", "a", "-d", "b"]);
-    insta::assert_snapshot!(stderr.lines().next().unwrap(), @"error: The argument '--branch <BRANCH>' cannot be used with '--source <SOURCE>'");
+    test_env.jj_cmd_cli_error(&repo_path, &["rebase", "-b", "a", "-s", "a", "-d", "b"]);
 
     // Rebase onto descendant with -r
     let stderr = test_env.jj_cmd_failure(&repo_path, &["rebase", "-r", "a", "-d", "b"]);

tests/test_restore_command.rs

@@ -191,13 +191,5 @@ fn test_restore_interactive() {
 
     // Combining paths with -i is not yet supported
     std::fs::write(&edit_script, "").unwrap();
-    let stderr = test_env.jj_cmd_failure(&repo_path, &["restore", "-i", "file2"]);
-    insta::assert_snapshot!(stderr.replace("jj.exe", "jj"), @r###"
-    error: The argument '--interactive' cannot be used with '<PATHS>...'
-
-    USAGE:
-        jj restore --interactive
-
-    For more information try --help
-    "###);
+    test_env.jj_cmd_cli_error(&repo_path, &["restore", "-i", "file2"]);
 }

tests/test_untrack_command.rs

@@ -44,16 +44,7 @@ fn test_untrack() {
     Error: Refusing to commit working copy (maybe because you're using --at-op)
     "###);
     // Errors out when no path is specified
-    let stderr = test_env.jj_cmd_failure(&repo_path, &["untrack"]);
-    insta::assert_snapshot!(stderr.replace("jj.exe", "jj"), @r###"
-    error: The following required arguments were not provided:
-        <PATHS>...
-
-    USAGE:
-        jj untrack [OPTIONS] <PATHS>...
-
-    For more information try --help
-    "###);
+    test_env.jj_cmd_cli_error(&repo_path, &["untrack"]);
     // Errors out when a specified file is not ignored
     let stderr = test_env.jj_cmd_failure(&repo_path, &["untrack", "file1", "file1.bak"]);
     insta::assert_snapshot!(stderr, @"Error: 'file1' would be added back because it's not ignored. Make sure it's ignored, \