diff --git a/.config/nextest.toml b/.config/nextest.toml index af32017a9..84f6d845d 100644 --- a/.config/nextest.toml +++ b/.config/nextest.toml @@ -1,19 +1,19 @@ [profile.default] -# Unit tests: flag SLOW after 1.5s, hard-kill after 3s for most crates -slow-timeout = { period = "1.5s", terminate-after = 2 } +# Default-profile tests: flag SLOW after 1s, hard-kill after 3s +slow-timeout = { period = "1s", terminate-after = 3 } leak-timeout = "500ms" [[profile.default.overrides]] filter = "package(fabro-cli) & kind(test)" - slow-timeout = { period = "5s", terminate-after = 4 } + slow-timeout = { period = "3s", terminate-after = 4 } [[profile.default.overrides]] filter = "package(fabro-server) & kind(test)" - slow-timeout = { period = "5s", terminate-after = 2 } + slow-timeout = { period = "5s", terminate-after = 4 } [[profile.default.overrides]] filter = "package(fabro-workflow) & kind(test)" - slow-timeout = { period = "3s", terminate-after = 2 } + slow-timeout = { period = "2s", terminate-after = 3 } [profile.e2e] # E2E (ignored) tests: flag SLOW after 10s, hard-kill after 30s diff --git a/lib/crates/fabro-workflow/src/git.rs b/lib/crates/fabro-workflow/src/git.rs index b971c4da0..7931abd4a 100644 --- a/lib/crates/fabro-workflow/src/git.rs +++ b/lib/crates/fabro-workflow/src/git.rs @@ -584,141 +584,6 @@ mod tests { remove_worktree(dir.path(), &wt_path).unwrap(); } - #[test] - fn replace_worktree_replaces_stale() { - let dir = tempfile::tempdir().unwrap(); - init_repo(dir.path()); - create_branch(dir.path(), "stale-branch").unwrap(); - - let wt_path = dir.path().join("stale-wt"); - add_worktree(dir.path(), &wt_path, "stale-branch").unwrap(); - assert!(wt_path.join(".git").exists()); - - // Calling replace_worktree again succeeds (removes stale, re-creates) - replace_worktree(dir.path(), &wt_path, "stale-branch").unwrap(); - assert!(wt_path.join(".git").exists()); - - remove_worktree(dir.path(), &wt_path).unwrap(); - } - - #[test] - fn push_ref_to_bare_remote() { - let dir = tempfile::tempdir().unwrap(); - let repo_dir = dir.path().join("repo"); - let remote_dir = dir.path().join("remote.git"); - - // Create a bare remote - Command::new("git") - .args(["init", "--bare"]) - .arg(&remote_dir) - .output() - .unwrap(); - - // Create a local repo with origin pointing at the bare remote - Command::new("git") - .args(["init"]) - .arg(&repo_dir) - .output() - .unwrap(); - Command::new("git") - .args(["remote", "add", "origin"]) - .arg(&remote_dir) - .current_dir(&repo_dir) - .output() - .unwrap(); - Command::new("git") - .args([ - "-c", - "user.name=test", - "-c", - "user.email=test@test", - "commit", - "--allow-empty", - "-m", - "init", - ]) - .current_dir(&repo_dir) - .output() - .unwrap(); - - // Create a branch and push it via push_ref - create_branch(&repo_dir, "test-push").unwrap(); - let url = format!("file://{}", remote_dir.display()); - push_ref(&repo_dir, &url, "refs/heads/test-push").unwrap(); - - // Verify the remote now has the branch - let output = Command::new("git") - .args(["branch", "--list", "test-push"]) - .current_dir(&remote_dir) - .output() - .unwrap(); - let stdout = String::from_utf8_lossy(&output.stdout); - assert!( - stdout.contains("test-push"), - "remote should have test-push branch" - ); - } - - #[test] - fn push_branch_to_remote() { - let dir = tempfile::tempdir().unwrap(); - let repo_dir = dir.path().join("repo"); - let remote_dir = dir.path().join("remote.git"); - - // Create a bare remote - Command::new("git") - .args(["init", "--bare"]) - .arg(&remote_dir) - .output() - .unwrap(); - - // Create a local repo with origin pointing at the bare remote - Command::new("git") - .args(["init"]) - .arg(&repo_dir) - .output() - .unwrap(); - Command::new("git") - .args(["remote", "add", "origin"]) - .arg(&remote_dir) - .current_dir(&repo_dir) - .output() - .unwrap(); - Command::new("git") - .args([ - "-c", - "user.name=test", - "-c", - "user.email=test@test", - "commit", - "--allow-empty", - "-m", - "init", - ]) - .current_dir(&repo_dir) - .output() - .unwrap(); - - // Rename default branch to "main" for predictability - Command::new("git") - .args(["branch", "-M", "main"]) - .current_dir(&repo_dir) - .output() - .unwrap(); - - // Push using push_branch - push_branch(&repo_dir, "origin", "main").unwrap(); - - // Verify the remote now has the commit - let output = Command::new("git") - .args(["branch", "--list", "main"]) - .current_dir(&remote_dir) - .output() - .unwrap(); - let stdout = String::from_utf8_lossy(&output.stdout); - assert!(stdout.contains("main"), "remote should have main branch"); - } - #[test] fn push_branch_fails_for_nonexistent_remote() { let dir = tempfile::tempdir().unwrap(); @@ -727,117 +592,6 @@ mod tests { assert!(result.is_err()); } - #[test] - fn branch_needs_push_when_ahead() { - let dir = tempfile::tempdir().unwrap(); - let repo_dir = dir.path().join("repo"); - let remote_dir = dir.path().join("remote.git"); - - Command::new("git") - .args(["init", "--bare"]) - .arg(&remote_dir) - .output() - .unwrap(); - Command::new("git") - .args(["init"]) - .arg(&repo_dir) - .output() - .unwrap(); - Command::new("git") - .args(["remote", "add", "origin"]) - .arg(&remote_dir) - .current_dir(&repo_dir) - .output() - .unwrap(); - Command::new("git") - .args([ - "-c", - "user.name=test", - "-c", - "user.email=test@test", - "commit", - "--allow-empty", - "-m", - "init", - ]) - .current_dir(&repo_dir) - .output() - .unwrap(); - Command::new("git") - .args(["branch", "-M", "main"]) - .current_dir(&repo_dir) - .output() - .unwrap(); - - // Push once to establish remote tracking - push_branch(&repo_dir, "origin", "main").unwrap(); - - // Make another commit locally (now ahead of remote) - Command::new("git") - .args([ - "-c", - "user.name=test", - "-c", - "user.email=test@test", - "commit", - "--allow-empty", - "-m", - "second", - ]) - .current_dir(&repo_dir) - .output() - .unwrap(); - - assert!(branch_needs_push(&repo_dir, "origin", "main")); - } - - #[test] - fn branch_needs_push_when_in_sync() { - let dir = tempfile::tempdir().unwrap(); - let repo_dir = dir.path().join("repo"); - let remote_dir = dir.path().join("remote.git"); - - Command::new("git") - .args(["init", "--bare"]) - .arg(&remote_dir) - .output() - .unwrap(); - Command::new("git") - .args(["init"]) - .arg(&repo_dir) - .output() - .unwrap(); - Command::new("git") - .args(["remote", "add", "origin"]) - .arg(&remote_dir) - .current_dir(&repo_dir) - .output() - .unwrap(); - Command::new("git") - .args([ - "-c", - "user.name=test", - "-c", - "user.email=test@test", - "commit", - "--allow-empty", - "-m", - "init", - ]) - .current_dir(&repo_dir) - .output() - .unwrap(); - Command::new("git") - .args(["branch", "-M", "main"]) - .current_dir(&repo_dir) - .output() - .unwrap(); - - push_branch(&repo_dir, "origin", "main").unwrap(); - - assert!(!branch_needs_push(&repo_dir, "origin", "main")); - } - #[test] fn branch_needs_push_when_no_remote_ref() { let dir = tempfile::tempdir().unwrap(); diff --git a/lib/crates/fabro-workflow/src/pipeline/execute/tests.rs b/lib/crates/fabro-workflow/src/pipeline/execute/tests.rs index bf7229285..0c2c79bbf 100644 --- a/lib/crates/fabro-workflow/src/pipeline/execute/tests.rs +++ b/lib/crates/fabro-workflow/src/pipeline/execute/tests.rs @@ -1028,91 +1028,3 @@ async fn run_with_lifecycle_emits_initialize_and_setup_events() { assert!(setup_idx < run_started_idx); assert!(run_started_idx < run_running_idx); } - -#[tokio::test] -async fn git_checkpoint_skips_start_node() { - let repo_dir = tempfile::tempdir().unwrap(); - let repo = repo_dir.path(); - std::process::Command::new("git") - .args(["init"]) - .current_dir(repo) - .output() - .unwrap(); - std::process::Command::new("git") - .args([ - "-c", - "user.name=Test", - "-c", - "user.email=test@test.com", - "commit", - "--allow-empty", - "-m", - "initial", - ]) - .current_dir(repo) - .output() - .unwrap(); - let base_sha = String::from_utf8( - std::process::Command::new("git") - .args(["rev-parse", "HEAD"]) - .current_dir(repo) - .output() - .unwrap() - .stdout, - ) - .unwrap() - .trim() - .to_string(); - - let run_tmp = tempfile::tempdir().unwrap(); - let mut g = simple_graph(); - g.nodes.insert("work".to_string(), Node::new("work")); - g.edges.clear(); - g.edges.push(Edge::new("start", "work")); - g.edges.push(Edge::new("work", "exit")); - - let events = Arc::new(std::sync::Mutex::new(Vec::::new())); - let events_clone = Arc::clone(&events); - let emitter = test_emitter("git-cp-test"); - emitter.on_event(move |event| { - events_clone.lock().unwrap().push(event.clone()); - }); - - let sandbox: Arc = Arc::new(fabro_agent::LocalSandbox::new(repo.to_path_buf())); - let mut run_options = test_run_options(run_tmp.path(), "git-cp-test"); - run_options.git = Some(GitCheckpointOptions { - base_sha: Some(base_sha), - run_branch: None, - meta_branch: Some(crate::git::MetadataStore::branch_name( - &test_run_id("git-cp-test").to_string(), - )), - }); - run_options.host_repo_path = Some(repo.to_path_buf()); - - run_graph( - make_registry(), - Arc::new(emitter), - sandbox, - &g, - &run_options, - ) - .await - .unwrap(); - - let collected = events.lock().unwrap(); - let checkpoint_node_ids: Vec<&str> = collected - .iter() - .filter(|event| { - event.event_name() == "checkpoint.completed" - && event.properties().is_ok_and(|properties| { - properties - .get("git_commit_sha") - .and_then(|value| value.as_str()) - .is_some() - }) - }) - .filter_map(|event| event.node_id.as_deref()) - .collect(); - assert!(!checkpoint_node_ids.contains(&"start")); - assert!(checkpoint_node_ids.contains(&"work")); -} diff --git a/lib/crates/fabro-workflow/tests/it/git_integration.rs b/lib/crates/fabro-workflow/tests/it/git_integration.rs new file mode 100644 index 000000000..1a4241f33 --- /dev/null +++ b/lib/crates/fabro-workflow/tests/it/git_integration.rs @@ -0,0 +1,322 @@ +use std::collections::HashMap; +use std::path::{Path, PathBuf}; +use std::process::{Command, Output}; +use std::sync::Arc; + +use fabro_agent::Sandbox; +use fabro_graphviz::graph::{AttrValue, Edge, Graph, Node}; +use fabro_types::settings::SettingsLayer; +use fabro_types::{RunEvent, fixtures}; +use fabro_workflow::event::Emitter; +use fabro_workflow::git::{ + MetadataStore, add_worktree, branch_needs_push, create_branch, push_branch, push_ref, + remove_worktree, replace_worktree, +}; +use fabro_workflow::handler::HandlerRegistry; +use fabro_workflow::handler::exit::ExitHandler; +use fabro_workflow::handler::start::StartHandler; +use fabro_workflow::run_options::{GitCheckpointOptions, RunOptions}; +use fabro_workflow::test_support::run_graph; + +fn assert_success(output: Output, context: &str) { + assert!( + output.status.success(), + "{context} failed: {}", + String::from_utf8_lossy(&output.stderr) + ); +} + +fn init_repo(dir: &Path) { + std::fs::create_dir_all(dir).unwrap(); + assert_success( + Command::new("git") + .args(["init"]) + .current_dir(dir) + .output() + .unwrap(), + "git init", + ); + assert_success( + Command::new("git") + .args([ + "-c", + "user.name=test", + "-c", + "user.email=test@test", + "commit", + "--allow-empty", + "-m", + "init", + ]) + .current_dir(dir) + .output() + .unwrap(), + "git commit --allow-empty", + ); +} + +fn init_bare_remote(dir: &Path) { + std::fs::create_dir_all(dir.parent().unwrap()).unwrap(); + assert_success( + Command::new("git") + .args(["init", "--bare"]) + .arg(dir) + .output() + .unwrap(), + "git init --bare", + ); +} + +fn add_origin(repo_dir: &Path, remote_dir: &Path) { + assert_success( + Command::new("git") + .args(["remote", "add", "origin"]) + .arg(remote_dir) + .current_dir(repo_dir) + .output() + .unwrap(), + "git remote add origin", + ); +} + +fn rename_branch(repo_dir: &Path, branch: &str) { + assert_success( + Command::new("git") + .args(["branch", "-M", branch]) + .current_dir(repo_dir) + .output() + .unwrap(), + "git branch -M", + ); +} + +fn empty_commit(repo_dir: &Path, message: &str) { + assert_success( + Command::new("git") + .args([ + "-c", + "user.name=test", + "-c", + "user.email=test@test", + "commit", + "--allow-empty", + "-m", + message, + ]) + .current_dir(repo_dir) + .output() + .unwrap(), + "git commit --allow-empty", + ); +} + +fn list_branch(repo_dir: &Path, branch: &str) -> String { + let output = Command::new("git") + .args(["branch", "--list", branch]) + .current_dir(repo_dir) + .output() + .unwrap(); + assert_success(output.clone(), "git branch --list"); + String::from_utf8(output.stdout).unwrap() +} + +fn local_env(repo: &Path) -> Arc { + Arc::new(fabro_agent::LocalSandbox::new(repo.to_path_buf())) +} + +fn simple_graph() -> Graph { + let mut g = Graph::new("git_checkpoint"); + g.attrs.insert( + "goal".to_string(), + AttrValue::String("Create git checkpoints".to_string()), + ); + + let mut start = Node::new("start"); + start.attrs.insert( + "shape".to_string(), + AttrValue::String("Mdiamond".to_string()), + ); + g.nodes.insert("start".to_string(), start); + + let mut exit = Node::new("exit"); + exit.attrs.insert( + "shape".to_string(), + AttrValue::String("Msquare".to_string()), + ); + g.nodes.insert("exit".to_string(), exit); + + g +} + +fn make_registry() -> HandlerRegistry { + let mut registry = HandlerRegistry::new(Box::new(StartHandler)); + registry.register("start", Box::new(StartHandler)); + registry.register("exit", Box::new(ExitHandler)); + registry +} + +fn test_run_options(run_dir: &Path) -> RunOptions { + RunOptions { + run_dir: run_dir.to_path_buf(), + cancel_token: None, + run_id: fixtures::RUN_2, + settings: SettingsLayer::default(), + git: None, + host_repo_path: None, + labels: HashMap::new(), + github_app: None, + base_branch: None, + display_base_sha: None, + workflow_slug: None, + } +} + +#[test] +fn replace_worktree_replaces_stale() { + let dir = tempfile::tempdir().unwrap(); + init_repo(dir.path()); + create_branch(dir.path(), "stale-branch").unwrap(); + + let wt_path = dir.path().join("stale-wt"); + add_worktree(dir.path(), &wt_path, "stale-branch").unwrap(); + assert!(wt_path.join(".git").exists()); + + replace_worktree(dir.path(), &wt_path, "stale-branch").unwrap(); + assert!(wt_path.join(".git").exists()); + + remove_worktree(dir.path(), &wt_path).unwrap(); +} + +#[test] +fn push_ref_to_bare_remote() { + let dir = tempfile::tempdir().unwrap(); + let repo_dir = dir.path().join("repo"); + let remote_dir = dir.path().join("remote.git"); + + init_bare_remote(&remote_dir); + init_repo(&repo_dir); + add_origin(&repo_dir, &remote_dir); + + create_branch(&repo_dir, "test-push").unwrap(); + let url = format!("file://{}", remote_dir.display()); + push_ref(&repo_dir, &url, "refs/heads/test-push").unwrap(); + + assert!(list_branch(&remote_dir, "test-push").contains("test-push")); +} + +#[test] +fn push_branch_to_remote() { + let dir = tempfile::tempdir().unwrap(); + let repo_dir = dir.path().join("repo"); + let remote_dir = dir.path().join("remote.git"); + + init_bare_remote(&remote_dir); + init_repo(&repo_dir); + add_origin(&repo_dir, &remote_dir); + rename_branch(&repo_dir, "main"); + + push_branch(&repo_dir, "origin", "main").unwrap(); + + assert!(list_branch(&remote_dir, "main").contains("main")); +} + +#[test] +fn branch_needs_push_when_ahead() { + let dir = tempfile::tempdir().unwrap(); + let repo_dir = dir.path().join("repo"); + let remote_dir = dir.path().join("remote.git"); + + init_bare_remote(&remote_dir); + init_repo(&repo_dir); + add_origin(&repo_dir, &remote_dir); + rename_branch(&repo_dir, "main"); + + push_branch(&repo_dir, "origin", "main").unwrap(); + empty_commit(&repo_dir, "second"); + + assert!(branch_needs_push(&repo_dir, "origin", "main")); +} + +#[test] +fn branch_needs_push_when_in_sync() { + let dir = tempfile::tempdir().unwrap(); + let repo_dir = dir.path().join("repo"); + let remote_dir = dir.path().join("remote.git"); + + init_bare_remote(&remote_dir); + init_repo(&repo_dir); + add_origin(&repo_dir, &remote_dir); + rename_branch(&repo_dir, "main"); + + push_branch(&repo_dir, "origin", "main").unwrap(); + + assert!(!branch_needs_push(&repo_dir, "origin", "main")); +} + +#[tokio::test] +async fn git_checkpoint_skips_start_node() { + let repo_dir = tempfile::tempdir().unwrap(); + let repo = repo_dir.path(); + init_repo(repo); + + let base_sha = String::from_utf8( + Command::new("git") + .args(["rev-parse", "HEAD"]) + .current_dir(repo) + .output() + .unwrap() + .stdout, + ) + .unwrap() + .trim() + .to_string(); + + let run_tmp = tempfile::tempdir().unwrap(); + let mut g = simple_graph(); + g.nodes.insert("work".to_string(), Node::new("work")); + g.edges.clear(); + g.edges.push(Edge::new("start", "work")); + g.edges.push(Edge::new("work", "exit")); + + let events = Arc::new(std::sync::Mutex::new(Vec::::new())); + let events_clone = Arc::clone(&events); + let emitter = Emitter::new(fixtures::RUN_2); + emitter.on_event(move |event| { + events_clone.lock().unwrap().push(event.clone()); + }); + + let mut run_options = test_run_options(run_tmp.path()); + run_options.git = Some(GitCheckpointOptions { + base_sha: Some(base_sha), + run_branch: None, + meta_branch: Some(MetadataStore::branch_name(&fixtures::RUN_2.to_string())), + }); + run_options.host_repo_path = Some(PathBuf::from(repo)); + + run_graph( + make_registry(), + Arc::new(emitter), + local_env(repo), + &g, + &run_options, + ) + .await + .unwrap(); + + let collected = events.lock().unwrap(); + let checkpoint_node_ids: Vec<&str> = collected + .iter() + .filter(|event| { + event.event_name() == "checkpoint.completed" + && event.properties().is_ok_and(|properties| { + properties + .get("git_commit_sha") + .and_then(|value| value.as_str()) + .is_some() + }) + }) + .filter_map(|event| event.node_id.as_deref()) + .collect(); + assert!(!checkpoint_node_ids.contains(&"start")); + assert!(checkpoint_node_ids.contains(&"work")); +} diff --git a/lib/crates/fabro-workflow/tests/it/main.rs b/lib/crates/fabro-workflow/tests/it/main.rs index f75d33431..9a34818fb 100644 --- a/lib/crates/fabro-workflow/tests/it/main.rs +++ b/lib/crates/fabro-workflow/tests/it/main.rs @@ -1,4 +1,5 @@ mod attractor_compat; mod cp_integration; mod daytona_integration; +mod git_integration; mod integration;