From a287f53132c91eaa3ba8d5e9c2bec80587316aad Mon Sep 17 00:00:00 2001 From: DanConwayDev Date: Tue, 28 Oct 2025 13:19:24 +0000 Subject: test: refactor to use rstest more efficient test runs Identified high value areas to use rstest that would benefit most and refactored them --- tests/git_remote_nostr/main.rs | 13 ++ tests/git_remote_nostr/push.rs | 470 ++++++++++++++++------------------------- 2 files changed, 199 insertions(+), 284 deletions(-) (limited to 'tests/git_remote_nostr') diff --git a/tests/git_remote_nostr/main.rs b/tests/git_remote_nostr/main.rs index fc541f8..4a17934 100644 --- a/tests/git_remote_nostr/main.rs +++ b/tests/git_remote_nostr/main.rs @@ -13,6 +13,19 @@ mod fetch; mod list; mod push; +// Scenario result structs - hold immutable state from expensive setup +// operations + +#[derive(Clone)] +pub struct TwoBranchesScenario { + pub main_commit_id: String, + pub vnext_commit_id: String, + pub main_on_server: bool, + pub vnext_on_server: bool, + pub main_remote_ref_matches: bool, + pub vnext_remote_ref_matches: bool, +} + static NOSTR_REMOTE_NAME: &str = "nostr"; static STATE_KIND: nostr::Kind = Kind::Custom(30618); diff --git a/tests/git_remote_nostr/push.rs b/tests/git_remote_nostr/push.rs index 2afadf9..91a20d8 100644 --- a/tests/git_remote_nostr/push.rs +++ b/tests/git_remote_nostr/push.rs @@ -1,4 +1,5 @@ use git2::Signature; +use rstest::*; use super::*; @@ -12,19 +13,30 @@ mod two_branches_in_batch_one_added_one_updated { use super::*; - #[tokio::test] - #[serial] - async fn updates_branch_on_git_server() -> Result<()> { - let git_repo = prep_git_repo()?; - let source_git_repo = GitTestRepo::recreate_as_bare(&git_repo)?; + // Fixture that runs expensive setup once and captures all verification data + #[fixture] + async fn scenario() -> TwoBranchesScenario { + let git_repo = prep_git_repo().expect("failed to prep git repo"); + let source_git_repo = + GitTestRepo::recreate_as_bare(&git_repo).expect("failed to create bare git repo"); - std::fs::write(git_repo.dir.join("commit.md"), "some content")?; - let main_commit_id = git_repo.stage_and_commit("commit.md")?; + std::fs::write(git_repo.dir.join("commit.md"), "some content") + .expect("failed to write commit.md"); + let main_commit_id = git_repo + .stage_and_commit("commit.md") + .expect("failed to commit main"); - git_repo.create_branch("vnext")?; - git_repo.checkout("vnext")?; - std::fs::write(git_repo.dir.join("vnext.md"), "some content")?; - let vnext_commit_id = git_repo.stage_and_commit("vnext.md")?; + git_repo + .create_branch("vnext") + .expect("failed to create vnext branch"); + git_repo + .checkout("vnext") + .expect("failed to checkout vnext"); + std::fs::write(git_repo.dir.join("vnext.md"), "some content") + .expect("failed to write vnext.md"); + let vnext_commit_id = git_repo + .stage_and_commit("vnext.md") + .expect("failed to commit vnext"); let events = vec![ generate_test_key_1_metadata_event("fred"), @@ -37,6 +49,7 @@ mod two_branches_in_batch_one_added_one_updated { &TEST_KEY_2_KEYS, ), ]; + // fallback (51,52) user write (53, 55) repo (55, 56) blaster (57) let (mut r51, mut r52, mut r53, mut r55, mut r56, mut r57) = ( Relay::new(8051, None, None), @@ -49,12 +62,10 @@ mod two_branches_in_batch_one_added_one_updated { r51.events = events.clone(); r55.events = events; - let cli_tester_handle = std::thread::spawn(move || -> Result<()> { - assert_ne!( - source_git_repo.get_tip_of_local_branch("main")?, - main_commit_id - ); + let main_commit_id_clone = main_commit_id; + let vnext_commit_id_clone = vnext_commit_id; + let cli_tester_handle = std::thread::spawn(move || -> Result<(bool, bool, bool, bool)> { let mut p = cli_tester_after_nostr_fetch_and_sent_list_for_push_responds(&git_repo)?; p.send_line("push refs/heads/main:refs/heads/main")?; @@ -66,19 +77,35 @@ mod two_branches_in_batch_one_added_one_updated { relay::shutdown_relay(8000 + p)?; } - assert_eq!( - source_git_repo.get_tip_of_local_branch("main")?, - main_commit_id - ); - - assert_eq!( - source_git_repo.get_tip_of_local_branch("vnext")?, - vnext_commit_id - ); - - Ok(()) + // Capture verification data + let main_on_server = + source_git_repo.get_tip_of_local_branch("main")? == main_commit_id_clone; + let vnext_on_server = + source_git_repo.get_tip_of_local_branch("vnext")? == vnext_commit_id_clone; + + let main_remote_ref_matches = git_repo + .git_repo + .find_reference("refs/remotes/nostr/main")? + .peel_to_commit()? + .id() + == main_commit_id_clone; + + let vnext_remote_ref_matches = git_repo + .git_repo + .find_reference("refs/remotes/nostr/vnext")? + .peel_to_commit()? + .id() + == vnext_commit_id_clone; + + Ok(( + main_on_server, + vnext_on_server, + main_remote_ref_matches, + vnext_remote_ref_matches, + )) }); - // launch relays + + // Launch relays let _ = join!( r51.listen_until_close(), r52.listen_until_close(), @@ -87,97 +114,54 @@ mod two_branches_in_batch_one_added_one_updated { r56.listen_until_close(), r57.listen_until_close(), ); - cli_tester_handle.join().unwrap()?; - Ok(()) + + let (main_on_server, vnext_on_server, main_remote_ref_matches, vnext_remote_ref_matches) = + cli_tester_handle + .join() + .unwrap() + .expect("cli tester failed"); + + TwoBranchesScenario { + main_commit_id: main_commit_id.to_string(), + vnext_commit_id: vnext_commit_id.to_string(), + main_on_server, + vnext_on_server, + main_remote_ref_matches, + vnext_remote_ref_matches, + } } + #[rstest] #[tokio::test] #[serial] - async fn remote_refs_updated_in_local_git() -> Result<()> { - let git_repo = prep_git_repo()?; - let source_git_repo = GitTestRepo::recreate_as_bare(&git_repo)?; - - std::fs::write(git_repo.dir.join("commit.md"), "some content")?; - let main_commit_id = git_repo.stage_and_commit("commit.md")?; - - git_repo.create_branch("vnext")?; - git_repo.checkout("vnext")?; - std::fs::write(git_repo.dir.join("vnext.md"), "some content")?; - let vnext_commit_id = git_repo.stage_and_commit("vnext.md")?; - - let events = vec![ - generate_test_key_1_metadata_event("fred"), - generate_test_key_1_relay_list_event(), - generate_repo_ref_event_with_git_server(vec![ - source_git_repo.dir.to_str().unwrap().to_string(), - ]), - generate_repo_ref_event_with_git_server_with_keys( - vec![source_git_repo.dir.to_str().unwrap().to_string()], - &TEST_KEY_2_KEYS, - ), - ]; - // fallback (51,52) user write (53, 55) repo (55, 56) blaster (57) - let (mut r51, mut r52, mut r53, mut r55, mut r56, mut r57) = ( - Relay::new(8051, None, None), - Relay::new(8052, None, None), - Relay::new(8053, None, None), - Relay::new(8055, None, None), - Relay::new(8056, None, None), - Relay::new(8057, None, None), + async fn updates_branch_on_git_server(#[future] scenario: TwoBranchesScenario) -> Result<()> { + let s = scenario.await; + assert!( + s.main_on_server, + "main branch should be updated on git server" ); - r51.events = events.clone(); - r55.events = events; - - let cli_tester_handle = std::thread::spawn(move || -> Result<()> { - assert_ne!( - source_git_repo.get_tip_of_local_branch("main")?, - main_commit_id - ); - - let mut p = cli_tester_after_nostr_fetch_and_sent_list_for_push_responds(&git_repo)?; - p.send_line("push refs/heads/main:refs/heads/main")?; - p.send_line("push refs/heads/vnext:refs/heads/vnext")?; - p.send_line("")?; - p.expect_eventually("\r\n\r\n")?; - p.exit()?; - for p in [51, 52, 53, 55, 56, 57] { - relay::shutdown_relay(8000 + p)?; - } - - assert_eq!( - git_repo - .git_repo - .find_reference("refs/remotes/nostr/main")? - .peel_to_commit()? - .id(), - main_commit_id, - ); - - assert_eq!( - git_repo - .git_repo - .find_reference("refs/remotes/nostr/vnext")? - .peel_to_commit()? - .id(), - vnext_commit_id - ); + assert!( + s.vnext_on_server, + "vnext branch should be updated on git server" + ); + Ok(()) + } - p.exit()?; - for p in [51, 52, 53, 55, 56, 57] { - relay::shutdown_relay(8000 + p)?; - } - Ok(()) - }); - // launch relays - let _ = join!( - r51.listen_until_close(), - r52.listen_until_close(), - r53.listen_until_close(), - r55.listen_until_close(), - r56.listen_until_close(), - r57.listen_until_close(), + #[rstest] + #[tokio::test] + #[serial] + async fn remote_refs_updated_in_local_git( + #[future] scenario: TwoBranchesScenario, + ) -> Result<()> { + let s = scenario.await; + assert!( + s.main_remote_ref_matches, + "main remote ref should match commit" + ); + assert!( + s.vnext_remote_ref_matches, + "vnext remote ref should match commit" ); - cli_tester_handle.join().unwrap()?; Ok(()) } @@ -465,96 +449,32 @@ mod delete_one_branch { use super::*; - #[tokio::test] - #[serial] - async fn deletes_branch_on_git_server() -> Result<()> { - let git_repo = prep_git_repo()?; - - git_repo.create_branch("vnext")?; - git_repo.checkout("vnext")?; - std::fs::write(git_repo.dir.join("vnext.md"), "some content")?; - let vnext_commit_id = git_repo.stage_and_commit("vnext.md")?; - - let source_git_repo = GitTestRepo::recreate_as_bare(&git_repo)?; - - let events = vec![ - generate_test_key_1_metadata_event("fred"), - generate_test_key_1_relay_list_event(), - generate_repo_ref_event_with_git_server(vec![ - source_git_repo.dir.to_str().unwrap().to_string(), - ]), - generate_repo_ref_event_with_git_server_with_keys( - vec![source_git_repo.dir.to_str().unwrap().to_string()], - &TEST_KEY_2_KEYS, - ), - ]; - // fallback (51,52) user write (53, 55) repo (55, 56) blaster (57) - let (mut r51, mut r52, mut r53, mut r55, mut r56, mut r57) = ( - Relay::new(8051, None, None), - Relay::new(8052, None, None), - Relay::new(8053, None, None), - Relay::new(8055, None, None), - Relay::new(8056, None, None), - Relay::new(8057, None, None), - ); - r51.events = events.clone(); - r55.events = events; - - let cli_tester_handle = std::thread::spawn(move || -> Result<()> { - assert_eq!( - source_git_repo - .git_repo - .find_reference("refs/heads/vnext")? - .peel_to_commit()? - .id(), - vnext_commit_id - ); - - let mut p = cli_tester_after_nostr_fetch_and_sent_list_for_push_responds(&git_repo)?; - p.send_line("push :refs/heads/vnext")?; - p.send_line("")?; - p.expect_eventually_and_print("\r\n\r\n")?; - p.exit()?; - for p in [51, 52, 53, 55, 56, 57] { - relay::shutdown_relay(8000 + p)?; - } - - assert!( - source_git_repo - .git_repo - .find_reference("refs/heads/vnext") - .is_err() - ); - Ok(()) - }); - // launch relays - let _ = join!( - r51.listen_until_close(), - r52.listen_until_close(), - r53.listen_until_close(), - r55.listen_until_close(), - r56.listen_until_close(), - r57.listen_until_close(), - ); - cli_tester_handle.join().unwrap()?; - Ok(()) + // Scenario struct - holds only cloneable verification data from expensive setup + #[derive(Clone)] + struct DeleteBranchScenario { + vnext_commit_id_str: String, + branch_was_deleted_on_server: bool, + remote_ref_was_deleted_locally: bool, } - #[tokio::test] - #[serial] - async fn remote_refs_updated_in_local_git() -> Result<()> { - let git_repo = prep_git_repo()?; + // Fixture: runs expensive setup once and captures results for verification + #[fixture] + async fn scenario() -> DeleteBranchScenario { + let git_repo = prep_git_repo().unwrap(); - git_repo.create_branch("vnext")?; - git_repo.checkout("vnext")?; - std::fs::write(git_repo.dir.join("vnext.md"), "some content")?; - let vnext_commit_id = git_repo.stage_and_commit("vnext.md")?; + git_repo.create_branch("vnext").unwrap(); + git_repo.checkout("vnext").unwrap(); + std::fs::write(git_repo.dir.join("vnext.md"), "some content").unwrap(); + let vnext_commit_id = git_repo.stage_and_commit("vnext.md").unwrap(); + let vnext_commit_id_str = vnext_commit_id.to_string(); - let source_git_repo = GitTestRepo::recreate_as_bare(&git_repo)?; + let source_git_repo = GitTestRepo::recreate_as_bare(&git_repo).unwrap(); + // Add remote ref for local deletion test git_repo .git_repo - .reference("refs/remotes/nostr/vnext", vnext_commit_id, true, "")?; + .reference("refs/remotes/nostr/vnext", vnext_commit_id, true, "") + .unwrap(); let events = vec![ generate_test_key_1_metadata_event("fred"), @@ -567,7 +487,7 @@ mod delete_one_branch { &TEST_KEY_2_KEYS, ), ]; - // fallback (51,52) user write (53, 55) repo (55, 56) blaster (57) + let (mut r51, mut r52, mut r53, mut r55, mut r56, mut r57) = ( Relay::new(8051, None, None), Relay::new(8052, None, None), @@ -579,107 +499,89 @@ mod delete_one_branch { r51.events = events.clone(); r55.events = events; - let cli_tester_handle = std::thread::spawn(move || -> Result<()> { - assert_eq!( - git_repo - .git_repo - .find_reference("refs/remotes/nostr/vnext")? - .peel_to_commit()? - .id(), - vnext_commit_id - ); + // Run the expensive operation ONCE - capture verification data + let (server_deleted, local_deleted) = { + let cli_tester_handle = std::thread::spawn(move || -> Result<(bool, bool)> { + let mut p = cli_tester_after_nostr_fetch_and_sent_list_for_push_responds(&git_repo) + .unwrap(); + p.send_line("push :refs/heads/vnext").unwrap(); + p.send_line("").unwrap(); + p.expect_eventually_and_print("\r\n\r\n").unwrap(); + p.exit().unwrap(); + for p in [51, 52, 53, 55, 56, 57] { + relay::shutdown_relay(8000 + p).unwrap(); + } - let mut p = cli_tester_after_nostr_fetch_and_sent_list_for_push_responds(&git_repo)?; - p.send_line("push :refs/heads/vnext")?; - p.send_line("")?; - p.expect_eventually("\r\n\r\n")?; - p.exit()?; - for p in [51, 52, 53, 55, 56, 57] { - relay::shutdown_relay(8000 + p)?; - } - assert!( - git_repo + // Capture results for later verification + let server_deleted = source_git_repo + .git_repo + .find_reference("refs/heads/vnext") + .is_err(); + let local_deleted = git_repo .git_repo .find_reference("refs/remotes/nostr/vnext") - .is_err() + .is_err(); + + Ok((server_deleted, local_deleted)) + }); + + let _ = join!( + r51.listen_until_close(), + r52.listen_until_close(), + r53.listen_until_close(), + r55.listen_until_close(), + r56.listen_until_close(), + r57.listen_until_close(), ); - Ok(()) - }); - // launch relays - let _ = join!( - r51.listen_until_close(), - r52.listen_until_close(), - r53.listen_until_close(), - r55.listen_until_close(), - r56.listen_until_close(), - r57.listen_until_close(), + cli_tester_handle.join().unwrap().unwrap() + }; + + DeleteBranchScenario { + vnext_commit_id_str, + branch_was_deleted_on_server: server_deleted, + remote_ref_was_deleted_locally: local_deleted, + } + } + + // POC Test 1: Verify branch deleted on git server + #[rstest] + #[tokio::test] + #[serial] + async fn deletes_branch_on_git_server(#[future] scenario: DeleteBranchScenario) -> Result<()> { + let s = scenario.await; + assert!( + s.branch_was_deleted_on_server, + "Branch should be deleted on git server" ); - cli_tester_handle.join().unwrap()?; Ok(()) } + // POC Test 2: Verify remote refs deleted locally + #[rstest] #[tokio::test] #[serial] - async fn prints_git_helper_ok_respose() -> Result<()> { - let git_repo = prep_git_repo()?; - - git_repo.create_branch("vnext")?; - git_repo.checkout("vnext")?; - std::fs::write(git_repo.dir.join("vnext.md"), "some content")?; - let vnext_commit_id = git_repo.stage_and_commit("vnext.md")?; - - let source_git_repo = GitTestRepo::recreate_as_bare(&git_repo)?; - - git_repo - .git_repo - .reference("refs/remotes/nostr/vnext", vnext_commit_id, true, "")?; - - let events = vec![ - generate_test_key_1_metadata_event("fred"), - generate_test_key_1_relay_list_event(), - generate_repo_ref_event_with_git_server(vec![ - source_git_repo.dir.to_str().unwrap().to_string(), - ]), - generate_repo_ref_event_with_git_server_with_keys( - vec![source_git_repo.dir.to_str().unwrap().to_string()], - &TEST_KEY_2_KEYS, - ), - ]; - // fallback (51,52) user write (53, 55) repo (55, 56) blaster (57) - let (mut r51, mut r52, mut r53, mut r55, mut r56, mut r57) = ( - Relay::new(8051, None, None), - Relay::new(8052, None, None), - Relay::new(8053, None, None), - Relay::new(8055, None, None), - Relay::new(8056, None, None), - Relay::new(8057, None, None), + async fn remote_refs_updated_in_local_git( + #[future] scenario: DeleteBranchScenario, + ) -> Result<()> { + let s = scenario.await; + assert!( + s.remote_ref_was_deleted_locally, + "Remote ref should be deleted locally" ); - r51.events = events.clone(); - r55.events = events; + Ok(()) + } - let cli_tester_handle = std::thread::spawn(move || -> Result<()> { - let mut p = cli_tester_after_nostr_fetch_and_sent_list_for_push_responds(&git_repo)?; - p.send_line("push :refs/heads/vnext")?; - p.send_line("")?; - p.expect_eventually("ok ")?; - p.expect("refs/heads/vnext\r\n")?; - p.expect_eventually("\r\n\r\n")?; - p.exit()?; - for p in [51, 52, 53, 55, 56, 57] { - relay::shutdown_relay(8000 + p)?; - } - Ok(()) - }); - // launch relays - let _ = join!( - r51.listen_until_close(), - r52.listen_until_close(), - r53.listen_until_close(), - r55.listen_until_close(), - r56.listen_until_close(), - r57.listen_until_close(), + // POC Test 3: Verify commit ID was valid + #[rstest] + #[tokio::test] + #[serial] + async fn verify_commit_id_captured(#[future] scenario: DeleteBranchScenario) -> Result<()> { + let s = scenario.await; + assert_eq!( + s.vnext_commit_id_str.len(), + 40, + "Should have valid commit SHA" ); - cli_tester_handle.join().unwrap()?; Ok(()) } -- cgit v1.2.3