From 6b3aecbcbde669859533716225e9c3bbfd2023b2 Mon Sep 17 00:00:00 2001 From: DanConwayDev Date: Fri, 8 Mar 2024 14:37:56 +0000 Subject: feat(send): select commits from a list when since_or_range isn't specified adds resilience as assuming master..HEAD can cause some issues eg when master is not up-to-date with origin/master --- src/cli_interactor.rs | 44 ++++++ src/git.rs | 74 +++++++++ src/sub_commands/push.rs | 2 +- src/sub_commands/send.rs | 216 ++++++++++++++++++++------- test_utils/src/lib.rs | 85 ++++++++++- tests/list.rs | 3 + tests/pull.rs | 3 + tests/push.rs | 22 ++- tests/send.rs | 380 ++++++----------------------------------------- 9 files changed, 438 insertions(+), 391 deletions(-) diff --git a/src/cli_interactor.rs b/src/cli_interactor.rs index dc15c87..4cf6357 100644 --- a/src/cli_interactor.rs +++ b/src/cli_interactor.rs @@ -14,6 +14,7 @@ pub trait InteractorPrompt { fn password(&self, parms: PromptPasswordParms) -> Result; fn confirm(&self, params: PromptConfirmParms) -> Result; fn choice(&self, params: PromptChoiceParms) -> Result; + fn multi_choice(&self, params: PromptMultiChoiceParms) -> Result>; } impl InteractorPrompt for Interactor { fn input(&self, parms: PromptInputParms) -> Result { @@ -53,6 +54,18 @@ impl InteractorPrompt for Interactor { } choice.interact().context("failed to get choice") } + fn multi_choice(&self, parms: PromptMultiChoiceParms) -> Result> { + // the colorful theme is not very clear so falling back to default + let mut choice = dialoguer::MultiSelect::default(); + choice + .with_prompt(parms.prompt) + .report(parms.report) + .items(&parms.choices); + if let Some(defaults) = parms.defaults { + choice.defaults(&defaults); + } + choice.interact().context("failed to get choice") + } } #[derive(Default)] @@ -140,3 +153,34 @@ impl PromptChoiceParms { self } } + +#[derive(Default)] +pub struct PromptMultiChoiceParms { + pub prompt: String, + pub choices: Vec, + pub defaults: Option>, + pub report: bool, +} + +impl PromptMultiChoiceParms { + pub fn with_prompt>(mut self, prompt: S) -> Self { + self.prompt = prompt.into(); + self.report = true; + self + } + + pub fn dont_report(mut self) -> Self { + self.report = false; + self + } + + pub fn with_choices(mut self, choices: Vec) -> Self { + self.choices = choices; + self + } + + pub fn with_defaults(mut self, defaults: Vec) -> Self { + self.defaults = Some(defaults); + self + } +} diff --git a/src/git.rs b/src/git.rs index ef14ab1..42297be 100644 --- a/src/git.rs +++ b/src/git.rs @@ -41,6 +41,7 @@ pub trait RepoActions { fn get_head_commit(&self) -> Result; fn get_commit_parent(&self, commit: &Sha1Hash) -> Result; fn get_commit_message(&self, commit: &Sha1Hash) -> Result; + fn get_commit_message_summary(&self, commit: &Sha1Hash) -> Result; /// returns vector ["name", "email", "unixtime", "offset"] /// eg ["joe bloggs", "joe@pm.me", "12176","-300"] fn get_commit_author(&self, commit: &Sha1Hash) -> Result>; @@ -52,6 +53,7 @@ pub trait RepoActions { base_commit: &Sha1Hash, latest_commit: &Sha1Hash, ) -> Result<(Vec, Vec)>; + fn get_refs(&self, commit: &Sha1Hash) -> Result>; // including (un)staged changes and (un)tracked files fn has_outstanding_changes(&self) -> Result; fn make_patch_from_commit( @@ -199,6 +201,22 @@ impl RepoActions for Repo { .to_string()) } + fn get_commit_message_summary(&self, commit: &Sha1Hash) -> Result { + Ok(self + .git_repo + .find_commit(sha1_to_oid(commit)?) + .context(format!("could not find commit {commit}"))? + .message_raw() + .context("commit message has unusual characters in (not valid utf-8)")? + .split('\r') + .collect::>()[0] + .split('\n') + .collect::>()[0] + .to_string() + .trim() + .to_string()) + } + fn get_commit_author(&self, commit: &Sha1Hash) -> Result> { let commit = self .git_repo @@ -217,6 +235,25 @@ impl RepoActions for Repo { Ok(git_sig_to_tag_vec(&sig)) } + fn get_refs(&self, commit: &Sha1Hash) -> Result> { + Ok(self + .git_repo + .references()? + .filter(|r| { + if let Ok(r) = r { + if let Ok(ref_tip) = r.peel_to_commit() { + ref_tip.id().to_string().eq(&commit.to_string()) + } else { + false + } + } else { + false + } + }) + .map(|r| r.unwrap().shorthand().unwrap().to_string()) + .collect::>()) + } + fn make_patch_from_commit( &self, commit: &Sha1Hash, @@ -744,6 +781,43 @@ mod tests { } } + mod get_commit_message_summary { + use super::*; + fn run(message: &str, summary: &str) -> Result<()> { + let test_repo = GitTestRepo::default(); + test_repo.populate()?; + std::fs::write(test_repo.dir.join("t100.md"), "some content")?; + let oid = test_repo.stage_and_commit(message)?; + + let git_repo = Repo::from_path(&test_repo.dir)?; + + assert_eq!( + summary, + git_repo.get_commit_message_summary(&oid_to_sha1(&oid))?, + ); + Ok(()) + } + #[test] + fn one_liner() -> Result<()> { + run("add t100.md", "add t100.md") + } + + #[test] + fn multiline() -> Result<()> { + run("add t100.md\r\nanother line\r\nthird line", "add t100.md") + } + + #[test] + fn trailing_newlines() -> Result<()> { + run("add t100.md\r\n\r\n\r\n\r\n\r\n\r\n", "add t100.md") + } + + #[test] + fn unicode_characters() -> Result<()> { + run("add t100.md ❤️", "add t100.md ❤️") + } + } + mod get_commit_author { use super::*; diff --git a/src/sub_commands/push.rs b/src/sub_commands/push.rs index bcac178..fdaab8e 100644 --- a/src/sub_commands/push.rs +++ b/src/sub_commands/push.rs @@ -107,7 +107,7 @@ pub async fn launch(cli_args: &Cli, args: &SubCommandArgs) -> Result<()> { sub_commands::send::launch( cli_args, &sub_commands::send::SubCommandArgs { - since_or_revision_range: String::new(), + since_or_range: String::new(), in_reply_to: Some(proposal_root_event.id.to_string()), title: None, description: None, diff --git a/src/sub_commands/send.rs b/src/sub_commands/send.rs index 16f10c4..9b44cc3 100644 --- a/src/sub_commands/send.rs +++ b/src/sub_commands/send.rs @@ -1,6 +1,7 @@ use std::{str::FromStr, time::Duration}; use anyhow::{bail, Context, Result}; +use console::Style; use futures::future::join_all; use indicatif::{MultiProgress, ProgressBar, ProgressStyle}; use nostr::{ @@ -14,7 +15,9 @@ use crate::client::Client; #[cfg(test)] use crate::client::MockConnect; use crate::{ - cli_interactor::{Interactor, InteractorPrompt, PromptConfirmParms, PromptInputParms}, + cli_interactor::{ + Interactor, InteractorPrompt, PromptConfirmParms, PromptInputParms, PromptMultiChoiceParms, + }, client::Connect, git::{Repo, RepoActions}, login, @@ -24,9 +27,9 @@ use crate::{ #[derive(Debug, clap::Args)] pub struct SubCommandArgs { - #[arg(default_value = "master..HEAD")] - /// commits to send as proposal; like in `git format-patch` - pub(crate) since_or_revision_range: String, + #[arg(default_value = "")] + /// commits to send as proposal; like in `git format-patch` eg. HEAD~2 + pub(crate) since_or_range: String, #[clap(long)] /// nevent or event id of an existing proposal for which this is a new /// version @@ -46,67 +49,83 @@ pub struct SubCommandArgs { pub async fn launch(cli_args: &Cli, args: &SubCommandArgs) -> Result<()> { let git_repo = Repo::discover().context("cannot find a git repository")?; + if let Some(id) = &args.in_reply_to { + println!("creating proposal revision for: {id}"); + } + let mut commits: Vec = { - if args.since_or_revision_range.is_empty() - || args.since_or_revision_range.eq("master..HEAD") - { + if args.since_or_range.is_empty() { let branch_name = git_repo.get_checked_out_branch_name()?; let (main_branch_name, main_tip) = git_repo .get_main_or_master_branch() .context("the default branches (main or master) do not exist")?; - if branch_name.eq(main_branch_name) { - println!("creating 1 patch from latest commit"); + + let proposed_commits = if branch_name.eq(main_branch_name) { vec![main_tip] } else { - let (from_branch, to_branch, ahead, behind) = - identify_ahead_behind(&git_repo, &None, &None)?; - - if ahead.is_empty() { - bail!(format!( - "'{from_branch}' is 0 commits ahead of '{to_branch}' so no patches were created" - )); - } - - if behind.is_empty() { - println!( - "creating patch for {} commits from '{from_branch}' that can be merged into '{to_branch}'", - ahead.len(), - ); - } else { - if !Interactor::default().confirm( - PromptConfirmParms::default() - .with_prompt( - format!( - "'{from_branch}' is {} commits behind '{to_branch}' and {} ahead. Consider rebasing before sending patches. Proceed anyway?", - behind.len(), - ahead.len(), - ) - ) - .with_default(false) - ).context("failed to get confirmation response from interactor confirm")? { - bail!("aborting so branch can be rebased"); - } - println!( - "creating patch for {} commit{} from '{from_branch}' that {} {} behind '{to_branch}'", - ahead.len(), - if ahead.len() > 1 { "s" } else { "" }, - if ahead.len() > 1 { "are" } else { "is" }, - behind.len(), - ); - } + let (_, _, ahead, _) = identify_ahead_behind(&git_repo, &None, &None)?; ahead - } + }; + choose_commits(&git_repo, proposed_commits)? } else { - let ahead = git_repo - .parse_starting_commits(&args.since_or_revision_range) - .context("cannot parse specified starting commit or range")?; - println!("creating patch for {} commits", ahead.len(),); - ahead + git_repo + .parse_starting_commits(&args.since_or_range) + .context("cannot parse specified starting commit or range")? } }; - if let Some(id) = &args.in_reply_to { - println!("as a revision to proposal: {id}"); + if commits.is_empty() { + bail!("no commits selected"); + } + println!("creating proposal from {} commits:", commits.len()); + + let dim = Style::new().color256(247); + for commit in &commits { + println!( + "{} {}", + dim.apply_to(commit.to_string().chars().take(7).collect::()), + git_repo.get_commit_message_summary(commit)? + ); + } + + let (main_branch_name, main_tip) = git_repo + .get_main_or_master_branch() + .context("the default branches (main or master) do not exist")?; + let (first_commit_ahead, behind) = + git_repo.get_commits_ahead_behind(&main_tip, commits.last().context("no commits")?)?; + + // check proposal ahead of origin/main + if first_commit_ahead.len().gt(&1) && !Interactor::default().confirm( + PromptConfirmParms::default() + .with_prompt( + format!("proposal builds on a commit {} ahead of '{main_branch_name}' - do you want to continue?", first_commit_ahead.len() - 1) + ) + .with_default(false) + ).context("failed to get confirmation response from interactor confirm")? { + bail!("aborting because selected commits were ahead of origin/master"); + } + + // check if a selected commit is already in origin + if commits.iter().any(|c| c.eq(&main_tip)) { + if !Interactor::default().confirm( + PromptConfirmParms::default() + .with_prompt( + format!("proposal contains commit(s) already in '{main_branch_name}'. proceed anyway?") + ) + .with_default(false) + ).context("failed to get confirmation response from interactor confirm")? { + bail!("aborting as proposal contains commit(s) already in '{main_branch_name}'"); + } + } + // check proposal isn't behind origin/main + else if !behind.is_empty() && !Interactor::default().confirm( + PromptConfirmParms::default() + .with_prompt( + format!("proposal is {} behind '{main_branch_name}'. consider rebasing before submission. proceed anyway?", behind.len()) + ) + .with_default(false) + ).context("failed to get confirmation response from interactor confirm")? { + bail!("aborting so commits can be rebased"); } let title = if args.no_cover_letter { @@ -359,6 +378,97 @@ where (vec1_u, vec2_u, dup, all) } +fn choose_commits(git_repo: &Repo, proposed_commits: Vec) -> Result> { + let mut proposed_commits = if proposed_commits.len().gt(&10) { + vec![] + } else { + proposed_commits + }; + + let tip_of_head = git_repo.get_tip_of_local_branch(&git_repo.get_checked_out_branch_name()?)?; + let most_recent_commit = proposed_commits.first().unwrap_or(&tip_of_head); + + let mut last_15_commits = vec![*most_recent_commit]; + + while last_15_commits.len().lt(&15) { + if let Ok(parent_commit) = git_repo.get_commit_parent(last_15_commits.last().unwrap()) { + last_15_commits.push(parent_commit); + } else { + break; + } + } + + let term = console::Term::stderr(); + let mut printed_error_line = false; + + let selected_commits = 'outer: loop { + let selected = Interactor::default().multi_choice( + PromptMultiChoiceParms::default() + .with_prompt("select commits for proposal") + .dont_report() + .with_choices( + last_15_commits + .iter() + .map(|h| summarise_commit_for_selection(git_repo, h).unwrap()) + .collect(), + ) + .with_defaults( + last_15_commits + .iter() + .map(|h| proposed_commits.iter().any(|c| c.eq(h))) + .collect(), + ), + )?; + proposed_commits = selected.iter().map(|i| last_15_commits[*i]).collect(); + + if printed_error_line { + term.clear_last_lines(1)?; + } + + if proposed_commits.is_empty() { + term.write_line("no commits selected")?; + printed_error_line = true; + continue; + } + for (i, selected_i) in selected.iter().enumerate() { + if i.gt(&0) && selected_i.ne(&(selected[i - 1] + 1)) { + term.write_line("commits must be consecutive. try again.")?; + printed_error_line = true; + continue 'outer; + } + } + + break proposed_commits; + }; + Ok(selected_commits) +} + +fn summarise_commit_for_selection(git_repo: &Repo, commit: &Sha1Hash) -> Result { + let references = git_repo.get_refs(commit)?; + let dim = Style::new().color256(247); + let prefix = format!("({})", git_repo.get_commit_author(commit)?[0],); + let references_string = if references.is_empty() { + String::new() + } else { + format!( + " {}", + references + .iter() + .map(|r| format!("[{r}]")) + .collect::>() + .join(" ") + ) + }; + + Ok(format!( + "{} {}{} {}", + dim.apply_to(prefix), + git_repo.get_commit_message_summary(commit)?, + Style::new().magenta().apply_to(references_string), + dim.apply_to(commit.to_string().chars().take(7).collect::(),), + )) +} + mod tests_unique_and_duplicate { #[test] diff --git a/test_utils/src/lib.rs b/test_utils/src/lib.rs index b4f0360..2edbc60 100644 --- a/test_utils/src/lib.rs +++ b/test_utils/src/lib.rs @@ -1,6 +1,6 @@ use std::{ffi::OsStr, path::PathBuf}; -use anyhow::{ensure, Context, Result}; +use anyhow::{bail, ensure, Context, Result}; use dialoguer::theme::{ColorfulTheme, Theme}; use directories::ProjectDirs; use nostr::{self, prelude::FromSkStr, Kind, Tag}; @@ -259,6 +259,20 @@ impl CliTester { i.prompt(false).context("initial confirm prompt")?; Ok(i) } + + pub fn expect_multi_select( + &mut self, + prompt: &str, + choices: Vec, + ) -> Result { + let mut i = CliTesterMultiSelectPrompt { + tester: self, + prompt: prompt.to_string(), + choices, + }; + i.prompt(false).context("initial confirm prompt")?; + Ok(i) + } } pub struct CliTesterInputPrompt<'a> { @@ -448,6 +462,75 @@ impl CliTesterConfirmPrompt<'_> { } } +pub struct CliTesterMultiSelectPrompt<'a> { + tester: &'a mut CliTester, + prompt: String, + choices: Vec, +} + +impl CliTesterMultiSelectPrompt<'_> { + fn prompt(&mut self, eventually: bool) -> Result<&mut Self> { + if eventually { + self.tester + .expect_eventually(format!("{}:\r\n", self.prompt)) + .context("expect multi-select prompt eventually")?; + } else { + self.tester + .expect(format!("{}:\r\n", self.prompt)) + .context("expect multi-select prompt")?; + } + Ok(self) + } + + pub fn succeeds_with( + &mut self, + chosen_indexes: Vec, + report: bool, + default_indexes: Vec, + ) -> Result<&mut Self> { + if report { + bail!("TODO: add support for report") + } + + fn show_options( + tester: &mut CliTester, + choices: &[String], + active_index: usize, + selected_indexes: &[usize], + ) -> Result<()> { + for (index, item) in choices.iter().enumerate() { + tester.expect(format!( + "{}{}{}\r\n", + if active_index.eq(&index) { "> " } else { " " }, + if selected_indexes.iter().any(|i| i.eq(&index)) { + "[x] " + } else { + "[ ] " + }, + item, + ))?; + } + Ok(()) + } + + show_options(self.tester, &self.choices, 0, &default_indexes)?; + + if default_indexes.eq(&chosen_indexes) { + self.tester.send("\r\n")?; + } else { + bail!("TODO: add support changing options"); + } + + for _ in self.choices.iter() { + self.tester.expect("\r")?; + } + // one for removing prompt maybe? + self.tester.expect("\r")?; + + Ok(self) + } +} + pub struct CliTesterChoicePrompt<'a> { tester: &'a mut CliTester, prompt: String, diff --git a/tests/list.rs b/tests/list.rs index 7e12dc1..4f55645 100644 --- a/tests/list.rs +++ b/tests/list.rs @@ -83,6 +83,7 @@ fn cli_tester_create_proposal( TEST_PASSWORD, "--disable-cli-spinners", "send", + "HEAD~2", "--no-cover-letter", "--in-reply-to", in_reply_to.as_str(), @@ -99,6 +100,7 @@ fn cli_tester_create_proposal( TEST_PASSWORD, "--disable-cli-spinners", "send", + "HEAD~2", "--title", format!("\"{title}\"").as_str(), "--description", @@ -116,6 +118,7 @@ fn cli_tester_create_proposal( TEST_PASSWORD, "--disable-cli-spinners", "send", + "HEAD~2", "--no-cover-letter", ], ); diff --git a/tests/pull.rs b/tests/pull.rs index 77ff87b..50dddbf 100644 --- a/tests/pull.rs +++ b/tests/pull.rs @@ -81,6 +81,7 @@ fn cli_tester_create_proposal( TEST_PASSWORD, "--disable-cli-spinners", "send", + "HEAD~2", "--no-cover-letter", "--in-reply-to", in_reply_to.as_str(), @@ -97,6 +98,7 @@ fn cli_tester_create_proposal( TEST_PASSWORD, "--disable-cli-spinners", "send", + "HEAD~2", "--title", format!("\"{title}\"").as_str(), "--description", @@ -114,6 +116,7 @@ fn cli_tester_create_proposal( TEST_PASSWORD, "--disable-cli-spinners", "send", + "HEAD~2", "--no-cover-letter", ], ); diff --git a/tests/push.rs b/tests/push.rs index db7a8b8..5fe1f15 100644 --- a/tests/push.rs +++ b/tests/push.rs @@ -80,6 +80,7 @@ fn cli_tester_create_proposal( TEST_PASSWORD, "--disable-cli-spinners", "send", + "HEAD~2", "--title", format!("\"{title}\"").as_str(), "--description", @@ -571,13 +572,26 @@ mod when_branch_is_checked_out { p.expect("finding proposal root event...\r\n")?; p.expect("found proposal root event. finding commits...\r\n")?; p.expect("preparing to force push proposal revision...\r\n")?; - // standard output from `ngit send` - p.expect(format!("creating patch for 2 commits from '{FEATURE_BRANCH_NAME_1}' that can be merged into 'main'\r\n"))?; - p.expect("as a revision to proposal: ")?; + p.expect("creating proposal revision for: ")?; // proposal id will be printed in this gap p.expect_eventually("\r\n")?; - + let mut selector = p.expect_multi_select( + "select commits for proposal", + vec![ + "(Joe Bloggs) add a4.md [feature-example-t] 355bdf1".to_string(), + "(Joe Bloggs) add a3.md dbd1115".to_string(), + "(Joe Bloggs) commit for rebasing on top of [main] 1aa2cfe" + .to_string(), + "(Joe Bloggs) add t2.md 431b84e".to_string(), + "(Joe Bloggs) add t1.md af474d8".to_string(), + "(Joe Bloggs) Initial commit 9ee507f".to_string(), + ], + )?; + selector.succeeds_with(vec![0, 1], false, vec![0, 1])?; + p.expect("creating proposal from 2 commits:\r\n")?; + p.expect("355bdf1 add a4.md\r\n")?; + p.expect("dbd1115 add a3.md\r\n")?; p.expect("searching for profile and relay updates...\r\n")?; p.expect("\r")?; p.expect("logged in as fred\r\n")?; diff --git a/tests/send.rs b/tests/send.rs index 3c619a4..538f38a 100644 --- a/tests/send.rs +++ b/tests/send.rs @@ -12,19 +12,8 @@ fn when_no_main_or_master_branch_return_error() -> Result<()> { Ok(()) } -#[test] -fn when_no_commits_ahead_of_main_return_error() -> Result<()> { - let test_repo = GitTestRepo::default(); - test_repo.populate()?; - // create feature branch with 1 commit ahead - test_repo.create_branch("feature")?; - test_repo.checkout("feature")?; - - let mut p = CliTester::new_from_dir(&test_repo.dir, ["send"]); - p.expect("Error: 'feature' is 0 commits ahead of 'main' so no patches were created")?; - Ok(()) -} - +// TODO when commits ahead of origin/master - test ask to proceed +// TODO when commits in origin/master - test ask to proceed mod when_commits_behind_ask_to_proceed { use super::*; @@ -46,16 +35,13 @@ mod when_commits_behind_ask_to_proceed { test_repo.checkout("feature")?; Ok(test_repo) } - static BEHIND_LEN: u8 = 1; - static AHEAD_LEN: u8 = 2; - - fn expect_confirm_prompt( - p: &mut CliTester, - behind: u8, - ahead: u8, - ) -> Result { + + fn expect_confirm_prompt(p: &mut CliTester) -> Result { + p.expect("creating proposal from 2 commits:\r\n")?; + p.expect("fe973a8 add t4.md\r\n")?; + p.expect("232efb3 add t3.md\r\n")?; p.expect_confirm( - format!("'feature' is {behind} commits behind 'main' and {ahead} ahead. Consider rebasing before sending patches. Proceed anyway?").as_str(), + "proposal is 1 behind 'main'. consider rebasing before submission. proceed anyway?", Some(false), ) } @@ -64,8 +50,8 @@ mod when_commits_behind_ask_to_proceed { fn asked_with_default_no() -> Result<()> { let test_repo = prep_test_repo()?; - let mut p = CliTester::new_from_dir(&test_repo.dir, ["send"]); - expect_confirm_prompt(&mut p, BEHIND_LEN, AHEAD_LEN)?; + let mut p = CliTester::new_from_dir(&test_repo.dir, ["send", "HEAD~2"]); + expect_confirm_prompt(&mut p)?; p.exit()?; Ok(()) } @@ -74,11 +60,11 @@ mod when_commits_behind_ask_to_proceed { fn when_response_is_false_aborts() -> Result<()> { let test_repo = prep_test_repo()?; - let mut p = CliTester::new_from_dir(&test_repo.dir, ["send"]); + let mut p = CliTester::new_from_dir(&test_repo.dir, ["send", "HEAD~2"]); - expect_confirm_prompt(&mut p, BEHIND_LEN, AHEAD_LEN)?.succeeds_with(Some(false))?; + expect_confirm_prompt(&mut p)?.succeeds_with(Some(false))?; - p.expect_end_with("Error: aborting so branch can be rebased\r\n")?; + p.expect_end_with("Error: aborting so commits can be rebased\r\n")?; Ok(()) } @@ -87,37 +73,14 @@ mod when_commits_behind_ask_to_proceed { fn when_response_is_true_proceeds() -> Result<()> { let test_repo = prep_test_repo()?; - let mut p = CliTester::new_from_dir(&test_repo.dir, ["send"]); - expect_confirm_prompt(&mut p, BEHIND_LEN, AHEAD_LEN)?.succeeds_with(Some(true))?; - p.expect( - format!("creating patch for {AHEAD_LEN} commits from 'feature' that are {BEHIND_LEN} behind 'main'",) - .as_str(), - )?; + let mut p = CliTester::new_from_dir(&test_repo.dir, ["send", "HEAD~2"]); + expect_confirm_prompt(&mut p)?.succeeds_with(Some(true))?; + p.expect("? include cover letter")?; p.exit()?; Ok(()) } } -#[test] -#[serial] -fn cli_message_creating_patches() -> Result<()> { - let test_repo = GitTestRepo::default(); - test_repo.populate()?; - // create feature branch with 2 commit ahead - test_repo.create_branch("feature")?; - test_repo.checkout("feature")?; - std::fs::write(test_repo.dir.join("t3.md"), "some content")?; - test_repo.stage_and_commit("add t3.md")?; - std::fs::write(test_repo.dir.join("t4.md"), "some content")?; - test_repo.stage_and_commit("add t4.md")?; - - let mut p = CliTester::new_from_dir(&test_repo.dir, ["send"]); - - p.expect("creating patch for 2 commits from 'feature' that can be merged into 'main'")?; - p.exit()?; - Ok(()) -} - fn is_cover_letter(event: &nostr::Event) -> bool { event.kind.as_u64().eq(&PATCH_KIND) && event.iter_tags().any(|t| t.as_vec()[1].eq("cover-letter")) @@ -149,6 +112,7 @@ fn cli_tester_create_proposal(git_repo: &GitTestRepo, include_cover_letter: bool TEST_PASSWORD, "--disable-cli-spinners", "send", + "HEAD~2", ]; if include_cover_letter { for arg in [ @@ -166,7 +130,9 @@ fn cli_tester_create_proposal(git_repo: &GitTestRepo, include_cover_letter: bool } fn expect_msgs_first(p: &mut CliTester, include_cover_letter: bool) -> Result<()> { - p.expect("creating patch for 2 commits from 'feature' that can be merged into 'main'\r\n")?; + p.expect("creating proposal from 2 commits:\r\n")?; + p.expect("fe973a8 add t4.md\r\n")?; + p.expect("232efb3 add t3.md\r\n")?; p.expect("searching for profile and relay updates...\r\n")?; p.expect("\r")?; p.expect("logged in as fred\r\n")?; @@ -247,7 +213,7 @@ async fn prep_run_create_proposal( Ok((r51, r52, r53, r55, r56)) } -mod sends_cover_letter_and_2_patches_to_3_relays { +mod when_cover_letter_details_specified_with_range_of_head_2_sends_cover_letter_and_2_patches_to_3_relays { use super::*; #[tokio::test] @@ -937,7 +903,7 @@ mod sends_cover_letter_and_2_patches_to_3_relays { } } -mod sends_2_patches_without_cover_letter { +mod when_no_cover_letter_flag_set_with_range_of_head_2_sends_2_patches_without_cover_letter { use super::*; mod cli_ouput { @@ -1118,206 +1084,9 @@ mod sends_2_patches_without_cover_letter { } } -mod when_on_main_branch_defaults_to_last_commit { - use super::*; - - fn prep_git_repo() -> Result { - let test_repo = GitTestRepo::default(); - test_repo.populate()?; - // dont checkout feature branch - std::fs::write(test_repo.dir.join("t3.md"), "some content")?; - test_repo.stage_and_commit("add t3.md")?; - std::fs::write(test_repo.dir.join("t4.md"), "some content")?; - test_repo.stage_and_commit("add t4.md")?; - Ok(test_repo) - } - - fn cli_tester_create_proposal(git_repo: &GitTestRepo) -> CliTester { - let args = vec![ - "--nsec", - TEST_KEY_1_NSEC, - "--password", - TEST_PASSWORD, - "--disable-cli-spinners", - "send", - "--no-cover-letter", - ]; - CliTester::new_from_dir(&git_repo.dir, args) - } - fn expect_msgs_first(p: &mut CliTester) -> Result<()> { - p.expect("creating 1 patch from latest commit\r\n")?; - p.expect("searching for profile and relay updates...\r\n")?; - p.expect("\r")?; - p.expect("logged in as fred\r\n")?; - p.expect("posting 1 patch without a covering letter...\r\n")?; - Ok(()) - } - async fn prep_run_create_proposal() -> Result<( - Relay<'static>, - Relay<'static>, - Relay<'static>, - Relay<'static>, - Relay<'static>, - )> { - let git_repo = prep_git_repo()?; - - // fallback (51,52) user write (53, 55) repo (55, 56) - let (mut r51, mut r52, mut r53, mut r55, mut r56) = ( - Relay::new( - 8051, - None, - Some(&|relay, client_id, subscription_id, _| -> Result<()> { - relay.respond_events( - client_id, - &subscription_id, - &vec![ - generate_test_key_1_metadata_event("fred"), - generate_test_key_1_relay_list_event(), - ], - )?; - Ok(()) - }), - ), - Relay::new(8052, None, None), - Relay::new(8053, None, None), - Relay::new( - 8055, - None, - Some(&|relay, client_id, subscription_id, _| -> Result<()> { - relay.respond_events( - client_id, - &subscription_id, - &vec![generate_repo_ref_event()], - )?; - Ok(()) - }), - ), - Relay::new(8056, None, None), - ); - - // // check relay had the right number of events - let cli_tester_handle = std::thread::spawn(move || -> Result<()> { - let mut p = cli_tester_create_proposal(&git_repo); - p.expect_end_eventually()?; - for p in [51, 52, 53, 55, 56] { - relay::shutdown_relay(8000 + p)?; - } - Ok(()) - }); - - // launch relay - let _ = join!( - r51.listen_until_close(), - r52.listen_until_close(), - r53.listen_until_close(), - r55.listen_until_close(), - r56.listen_until_close(), - ); - cli_tester_handle.join().unwrap()?; - Ok((r51, r52, r53, r55, r56)) - } - mod cli_ouput { - use super::*; - - #[tokio::test] - #[serial] - async fn check_cli_output() -> Result<()> { - let git_repo = prep_git_repo()?; - - let (mut r51, mut r52, mut r53, mut r55, mut r56) = ( - Relay::new( - 8051, - None, - Some(&|relay, client_id, subscription_id, _| -> Result<()> { - relay.respond_events( - client_id, - &subscription_id, - &vec![ - generate_test_key_1_metadata_event("fred"), - generate_test_key_1_relay_list_event(), - ], - )?; - Ok(()) - }), - ), - Relay::new(8052, None, None), - Relay::new(8053, None, None), - Relay::new( - 8055, - None, - Some(&|relay, client_id, subscription_id, _| -> Result<()> { - relay.respond_events( - client_id, - &subscription_id, - &vec![generate_repo_ref_event()], - )?; - Ok(()) - }), - ), - Relay::new(8056, None, None), - ); - - // // check relay had the right number of events - let cli_tester_handle = std::thread::spawn(move || -> Result<()> { - let mut p = cli_tester_create_proposal(&git_repo); - - expect_msgs_first(&mut p)?; - relay::expect_send_with_progress( - &mut p, - vec![ - (" [my-relay] [repo-relay] ws://localhost:8055", true, ""), - (" [my-relay] ws://localhost:8053", true, ""), - (" [repo-relay] ws://localhost:8056", true, ""), - (" [default] ws://localhost:8051", true, ""), - (" [default] ws://localhost:8052", true, ""), - ], - 1, - )?; - p.expect_end_with_whitespace()?; - for p in [51, 52, 53, 55, 56] { - relay::shutdown_relay(8000 + p)?; - } - Ok(()) - }); - - // launch relay - let _ = join!( - r51.listen_until_close(), - r52.listen_until_close(), - r53.listen_until_close(), - r55.listen_until_close(), - r56.listen_until_close(), - ); - cli_tester_handle.join().unwrap()?; - Ok(()) - } - } - - #[tokio::test] - #[serial] - async fn one_patch_event_sent() -> Result<()> { - let (_, _, r53, r55, r56) = prep_run_create_proposal().await?; - for relay in [&r53, &r55, &r56] { - assert_eq!(relay.events.iter().filter(|e| is_patch(e)).count(), 1); - } - Ok(()) - } -} - -mod specify_starting_commits_whist_on_main_branch { +mod when_range_ommited_prompts_for_selection_defaulting_ahead_of_main { use super::*; - fn prep_git_repo() -> Result { - let test_repo = GitTestRepo::default(); - test_repo.populate()?; - // dont checkout feature branch - std::fs::write(test_repo.dir.join("t3.md"), "some content")?; - test_repo.stage_and_commit("add t3.md")?; - std::fs::write(test_repo.dir.join("t4.md"), "some content")?; - test_repo.stage_and_commit("add t4.md")?; - Ok(test_repo) - } - fn cli_tester_create_proposal(git_repo: &GitTestRepo) -> CliTester { let args = vec![ "--nsec", @@ -1326,17 +1095,29 @@ mod specify_starting_commits_whist_on_main_branch { TEST_PASSWORD, "--disable-cli-spinners", "send", - "HEAD~3", "--no-cover-letter", ]; CliTester::new_from_dir(&git_repo.dir, args) } fn expect_msgs_first(p: &mut CliTester) -> Result<()> { - p.expect("creating patch for 3 commits\r\n")?; + let mut selector = p.expect_multi_select( + "select commits for proposal", + vec![ + "(Joe Bloggs) add t4.md [feature] fe973a8".to_string(), + "(Joe Bloggs) add t3.md 232efb3".to_string(), + "(Joe Bloggs) add t2.md [main] 431b84e".to_string(), + "(Joe Bloggs) add t1.md af474d8".to_string(), + "(Joe Bloggs) Initial commit 9ee507f".to_string(), + ], + )?; + selector.succeeds_with(vec![0, 1], false, vec![0, 1])?; + p.expect("creating proposal from 2 commits:\r\n")?; + p.expect("fe973a8 add t4.md\r\n")?; + p.expect("232efb3 add t3.md\r\n")?; p.expect("searching for profile and relay updates...\r\n")?; p.expect("\r")?; p.expect("logged in as fred\r\n")?; - p.expect("posting 3 patches without a covering letter...\r\n")?; + p.expect("posting 2 patches without a covering letter...\r\n")?; Ok(()) } async fn prep_run_create_proposal() -> Result<( @@ -1385,6 +1166,7 @@ mod specify_starting_commits_whist_on_main_branch { // // check relay had the right number of events let cli_tester_handle = std::thread::spawn(move || -> Result<()> { let mut p = cli_tester_create_proposal(&git_repo); + expect_msgs_first(&mut p)?; p.expect_end_eventually()?; for p in [51, 52, 53, 55, 56] { relay::shutdown_relay(8000 + p)?; @@ -1444,7 +1226,6 @@ mod specify_starting_commits_whist_on_main_branch { Relay::new(8056, None, None), ); - // // check relay had the right number of events let cli_tester_handle = std::thread::spawn(move || -> Result<()> { let mut p = cli_tester_create_proposal(&git_repo); @@ -1458,7 +1239,7 @@ mod specify_starting_commits_whist_on_main_branch { (" [default] ws://localhost:8051", true, ""), (" [default] ws://localhost:8052", true, ""), ], - 3, + 2, )?; p.expect_end_with_whitespace()?; for p in [51, 52, 53, 55, 56] { @@ -1482,84 +1263,16 @@ mod specify_starting_commits_whist_on_main_branch { #[tokio::test] #[serial] - async fn three_patch_events() -> Result<()> { + async fn two_patch_events_sent() -> Result<()> { let (_, _, r53, r55, r56) = prep_run_create_proposal().await?; for relay in [&r53, &r55, &r56] { - assert_eq!(relay.events.iter().filter(|e| is_patch(e)).count(), 3); - } - Ok(()) - } - - #[tokio::test] - #[serial] - async fn root_patch_doesnt_have_a_branch_name_tag() -> Result<()> { - let (_, _, r53, r55, r56) = prep_run_create_proposal().await?; - for relay in [&r53, &r55, &r56] { - let patch_events = relay - .events - .iter() - .filter(|e| is_patch(e)) - .collect::>(); - - assert!( - !patch_events[0] - .iter_tags() - .any(|t| t.as_vec()[0].eq("branch-name")) - ); - } - Ok(()) - } - - #[tokio::test] - #[serial] - async fn first_patch_is_ancestor_and_root_others_in_correct_order() -> Result<()> { - let (_, _, r53, r55, r56) = prep_run_create_proposal().await?; - for relay in [&r53, &r55, &r56] { - let patch_events = relay - .events - .iter() - .filter(|e| is_patch(e)) - .collect::>(); - - // first patch tagged as root - assert!( - patch_events[0] - .iter_tags() - .any(|t| t.as_vec()[0].eq("t") && t.as_vec()[1].eq("root")) - ); - // first patch is ancestor - assert_eq!( - patch_events[0] - .iter_tags() - .find(|t| t.as_vec()[0].eq("commit")) - .unwrap() - .as_vec()[1], - "431b84edc0d2fa118d63faa3c2db9c73d630a5ae" - ); - // second patch not tagged as root - assert_eq!( - patch_events[1] - .iter_tags() - .find(|t| t.as_vec()[0].eq("commit")) - .unwrap() - .as_vec()[1], - "232efb37ebc67692c9e9ff58b83c0d3d63971a0a" - ); - // second patch not tagged as root - assert_eq!( - patch_events[2] - .iter_tags() - .find(|t| t.as_vec()[0].eq("commit")) - .unwrap() - .as_vec()[1], - "fe973a840fba2a8ab37dd505c154854a69a6505c" - ); + assert_eq!(relay.events.iter().filter(|e| is_patch(e)).count(), 2); } Ok(()) } } -mod specify_in_reply_to { +mod in_reply_to_specified_with_range_of_head_2_and_cover_letter_details_specified { use super::*; fn cli_tester_create_proposal(git_repo: &GitTestRepo) -> CliTester { let args = vec![ @@ -1569,6 +1282,7 @@ mod specify_in_reply_to { TEST_PASSWORD, "--disable-cli-spinners", "send", + "HEAD~2", "--in-reply-to", "nevent1qqsypm62fzw7qynvlc4gjl3tr0jw4vmh659nvr2cc5qyhdg92a5yy0qzypumuen7l8wthtz45p3ftn58pvrs9xlumvkuu2xet8egzkcklqtesxygzam", "--title", @@ -1579,8 +1293,10 @@ mod specify_in_reply_to { CliTester::new_from_dir(&git_repo.dir, args) } fn expect_msgs_first(p: &mut CliTester, include_cover_letter: bool) -> Result<()> { - p.expect("creating patch for 2 commits from 'feature' that can be merged into 'main'\r\n")?; - p.expect("as a revision to proposal: nevent1qqsypm62fzw7qynvlc4gjl3tr0jw4vmh659nvr2cc5qyhdg92a5yy0qzypumuen7l8wthtz45p3ftn58pvrs9xlumvkuu2xet8egzkcklqtesxygzam\r\n")?; + p.expect("creating proposal revision for: nevent1qqsypm62fzw7qynvlc4gjl3tr0jw4vmh659nvr2cc5qyhdg92a5yy0qzypumuen7l8wthtz45p3ftn58pvrs9xlumvkuu2xet8egzkcklqtesxygzam\r\n")?; + p.expect("creating proposal from 2 commits:\r\n")?; + p.expect("fe973a8 add t4.md\r\n")?; + p.expect("232efb3 add t3.md\r\n")?; p.expect("searching for profile and relay updates...\r\n")?; p.expect("\r")?; p.expect("logged in as fred\r\n")?; -- cgit v1.2.3