Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions ARCHITECTURE.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,13 @@ main.rs ──▶ backend/cli.rs ──implements──▶ backend/mod.rs

```
config.yaml (workspaces: list, required; config.yml also accepted)
│ config::load_config → fs::read_to_string + SpreadFile::from_str (serde_yaml_ng)
│ config::read_config → fs::read_to_string
String (raw file contents, ownership kept by the caller)
│ (apply path) validate::validate_config(SourceFile { yaml, path })
│ → SpreadFile::from_str + validate (serde_yaml_ng)
│ — any finding (error or warning) stops processing (Err/exit 1);
│ only Ok proceeds to path resolution and the engine
SpreadFile { workspaces: Vec<Workspace> } (raw, as written by the user)
│ config::resolve_paths(file, env, invocation_cwd)
Expand Down Expand Up @@ -154,7 +160,7 @@ If you're touching path resolution, add a test for the *specific* combination yo

Three layers, each targeting a different seam:

1. **`config.rs` unit tests** — YAML parsing and path resolution, as plain data-in/data-out assertions. No filesystem or process access beyond `load_config`'s own file read.
1. **`config.rs` unit tests** — YAML parsing and path resolution, as plain data-in/data-out assertions. No filesystem or process access beyond `read_config`'s own file read.
2. **`engine.rs` unit tests** — drive `apply_workspace` (and `apply`'s cross-workspace focus-selection folding on top of it) against a hand-written `MockBackend` that just records every call it receives (`Vec<Call>`). This is what makes it possible to assert "for this YAML, exactly these `HerdrBackend` calls happen, in this order, with these ids" without needing herdr installed at all.
3. **`tests/cli_backend_integration.rs`** — the one test that exercises the full `engine::apply` → `CliBackend` → subprocess path, against `tests/fixtures/fake-herdr.sh`, a script that logs the argv it's called with and echoes back canned JSON shaped like real herdr responses. This is what catches integration bugs the mock backend can't see (e.g. an argv-building bug in `backend/cli.rs` that both the mock and the real herdr would parse differently).

Expand Down
8 changes: 6 additions & 2 deletions src/backend/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -619,7 +619,9 @@ mod tests {
BackendError::Herdr { message } => {
assert!(message.contains("unknown variant"));
}
other => panic!("expected BackendError::Herdr, got {other:?}"),
other @ BackendError::CommandFailed { .. } => {
panic!("expected BackendError::Herdr, got {other:?}")
}
}
}

Expand All @@ -635,7 +637,9 @@ mod tests {
assert_eq!(code, Some(7));
assert!(stderr.contains("boom"));
}
other => panic!("expected BackendError::CommandFailed, got {other:?}"),
other @ BackendError::Herdr { .. } => {
panic!("expected BackendError::CommandFailed, got {other:?}")
}
}
}

Expand Down
9 changes: 9 additions & 0 deletions src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,4 +46,13 @@ mod tests {
}
);
}

#[test]
fn should_reject_validate_subcommand() {
let result = Cli::try_parse_from(["herdr-spreader", "validate"]);
assert!(
result.is_err(),
"validate subcommand should no longer be parsed"
);
}
}
52 changes: 21 additions & 31 deletions src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,12 +90,6 @@ pub enum ConfigError {
#[source]
source: std::io::Error,
},
#[error("failed to parse config file {path}: {source}")]
Parse {
path: PathBuf,
#[source]
source: serde_yaml_ng::Error,
},
}

const CONFIG_FILE_NAMES: &[&str] = &["config.yaml", "config.yml"];
Expand Down Expand Up @@ -155,18 +149,13 @@ fn config_candidate_paths(env: &BTreeMap<String, String>) -> Vec<PathBuf> {
paths
}

/// Load a [`SpreadFile`] from a YAML file on disk.
/// Read a config file from disk into a `String`.
///
/// # Errors
///
/// Returns [`ConfigError::Io`] if the file cannot be read, or
/// [`ConfigError::Parse`] if its contents are not valid YAML.
pub fn load_config(path: &Path) -> Result<SpreadFile, ConfigError> {
let contents = fs::read_to_string(path).map_err(|source| ConfigError::Io {
path: path.to_path_buf(),
source,
})?;
SpreadFile::from_str(&contents).map_err(|source| ConfigError::Parse {
/// Returns [`ConfigError::Io`] if the file cannot be read.
pub fn read_config(path: &Path) -> Result<String, ConfigError> {
fs::read_to_string(path).map_err(|source| ConfigError::Io {
path: path.to_path_buf(),
source,
})
Expand Down Expand Up @@ -243,11 +232,11 @@ mod tests {

#[test]
fn should_parse_two_workspaces_given_multi_workspace_yaml() {
let yaml = r#"
let yaml = r"
workspaces:
- name: frontend
- name: backend
"#;
";

let file = SpreadFile::from_str(yaml).unwrap();

Expand All @@ -270,12 +259,12 @@ workspaces:

#[test]
fn should_parse_workspace_level_focus_flag_and_default_it_to_false() {
let yaml = r#"
let yaml = r"
workspaces:
- name: frontend
- name: backend
focus: true
"#;
";

let file = SpreadFile::from_str(yaml).unwrap();

Expand All @@ -285,10 +274,10 @@ workspaces:

#[test]
fn should_parse_workspace_name_given_minimal_yaml_with_only_name() {
let yaml = r#"
let yaml = r"
workspaces:
- name: demo
"#;
";

let file = SpreadFile::from_str(yaml).unwrap();

Expand All @@ -299,7 +288,7 @@ workspaces:

#[test]
fn should_parse_tabs_and_panes_given_tmuxinator_style_yaml() {
let yaml = r#"
let yaml = r"
workspaces:
- name: demo
tabs:
Expand All @@ -309,7 +298,7 @@ workspaces:
- label: server
panes:
- command: cargo run
"#;
";

let file = SpreadFile::from_str(yaml).unwrap();

Expand All @@ -323,13 +312,13 @@ workspaces:

#[test]
fn should_default_split_to_right_and_ratio_to_none_when_omitted() {
let yaml = r#"
let yaml = r"
workspaces:
- name: demo
tabs:
- panes:
- command: nvim
"#;
";

let file = SpreadFile::from_str(yaml).unwrap();

Expand All @@ -342,14 +331,14 @@ workspaces:

#[test]
fn should_reject_config_given_unknown_split_direction() {
let yaml = r#"
let yaml = r"
workspaces:
- name: demo
tabs:
- panes:
- command: nvim
split: left
"#;
";

let result = SpreadFile::from_str(yaml);

Expand Down Expand Up @@ -385,26 +374,27 @@ workspaces:
fn should_include_file_path_in_error_when_config_file_is_missing() {
let path = Path::new("/nonexistent/path/to/config.yaml");

let result = load_config(path);
let result = read_config(path);

let err = result.unwrap_err();
assert!(err.to_string().contains("/nonexistent/path/to/config.yaml"));
}

#[test]
fn should_load_spread_file_from_disk_given_multi_workspace_yaml() {
let yaml = r#"
let yaml = r"
workspaces:
- name: frontend
- name: backend
"#;
";
let path = std::env::temp_dir().join(format!(
"herdr-spreader-test-{}-should_load_spread_file_from_disk_given_multi_workspace_yaml.yml",
std::process::id()
));
fs::write(&path, yaml).unwrap();

let file = load_config(&path).unwrap();
let yaml = read_config(&path).unwrap();
let file = SpreadFile::from_str(&yaml).unwrap();

assert_eq!(file.workspaces.len(), 2);

Expand Down
51 changes: 6 additions & 45 deletions src/engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,6 @@ fn normalize_path(path: &Path) -> PathBuf {
pub enum EngineError {
#[error(transparent)]
Backend(#[from] BackendError),
#[error("wait_for was specified on a pane without a command")]
WaitForWithoutCommand,
}

/// Apply the workspace layout described by `file`, creating workspaces, tabs,
Expand All @@ -83,9 +81,7 @@ pub enum EngineError {
///
/// # Errors
///
/// Returns [`EngineError::Backend`] if any backend operation fails, or
/// [`EngineError::WaitForWithoutCommand`] if a pane specifies `wait_for`
/// without a `command`.
/// Returns [`EngineError::Backend`] if any backend operation fails.
pub fn apply(file: &SpreadFile, backend: &mut dyn HerdrBackend) -> Result<(), EngineError> {
for ws in &file.workspaces {
apply_workspace(ws, backend)?;
Expand All @@ -99,9 +95,7 @@ pub fn apply(file: &SpreadFile, backend: &mut dyn HerdrBackend) -> Result<(), En
///
/// # Errors
///
/// Returns [`EngineError::Backend`] if any backend operation fails, or
/// [`EngineError::WaitForWithoutCommand`] if a pane specifies `wait_for`
/// without a `command`.
/// Returns [`EngineError::Backend`] if any backend operation fails.
pub fn apply_workspace(ws: &Workspace, backend: &mut dyn HerdrBackend) -> Result<(), EngineError> {
let first_pane_focus = ws
.tabs
Expand Down Expand Up @@ -181,15 +175,10 @@ pub fn apply_workspace(ws: &Workspace, backend: &mut dyn HerdrBackend) -> Result
if let Some(wait_for) = &pane.wait_for {
backend.wait_output(&pane_id, wait_for)?;
}
} else {
if pane.wait_for.is_some() {
return Err(EngineError::WaitForWithoutCommand);
}
if pane_index == 0
&& let Some(prefix) = cwd_env_prefix(resolved_cwd.as_deref(), &pane.env)
{
backend.run(&pane_id, &prefix)?;
}
} else if pane_index == 0
&& let Some(prefix) = cwd_env_prefix(resolved_cwd.as_deref(), &pane.env)
{
backend.run(&pane_id, &prefix)?;
}

previous_pane_id = pane_id;
Expand Down Expand Up @@ -759,34 +748,6 @@ mod tests {
);
}

#[test]
fn should_error_when_wait_for_is_set_on_a_pane_without_a_command() {
let config = Workspace {
name: "demo".to_string(),
tabs: vec![Tab {
label: None,
panes: vec![Pane {
command: None,
wait_for: Some(WaitFor {
pattern: "ready".to_string(),
timeout_ms: None,
}),
..Default::default()
}],
..Default::default()
}],
..Default::default()
};
let mut mock = MockBackend::default();

let result = engine::apply_workspace(&config, &mut mock);

assert!(matches!(
result,
Err(engine::EngineError::WaitForWithoutCommand)
));
}

#[test]
fn should_pass_focus_to_create_tab_when_pane_has_focus_true_in_non_first_tab() {
let ws = Workspace {
Expand Down
1 change: 1 addition & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,4 @@ pub mod backend;
pub mod cli;
pub mod config;
pub mod engine;
pub mod validate;
60 changes: 34 additions & 26 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,36 +4,44 @@ use clap::Parser;

use herdr_spreader::backend::cli::CliBackend;
use herdr_spreader::cli::{Cli, Command};
use herdr_spreader::config::{load_config, resolve_config_path, resolve_paths};
use herdr_spreader::config::{read_config, resolve_config_path, resolve_paths};
use herdr_spreader::engine;
use herdr_spreader::validate;

fn main() -> anyhow::Result<()> {
let env: BTreeMap<String, String> = std::env::vars().collect();
let cli = Cli::parse();

let Command::Apply { file } = cli.command;

let config_path = resolve_config_path(file, &env)?;
let spread_file = load_config(&config_path)?;

let bin = CliBackend::resolve_bin(&env);
let mut backend = CliBackend::new(bin);

// Under `herdr plugin action invoke`, the process cwd is the plugin's own
// install directory, not the user's workspace. Query the invoking pane's
// real shell directory over the herdr CLI instead; fall back to the
// process cwd for standalone CLI usage (no herdr session, or the query
// fails) where the process cwd is the correct invocation directory.
let cwd = match env
.get("HERDR_PANE_ID")
.and_then(|id| backend.query_pane_cwd(id))
{
Some(cwd) => cwd,
None => std::env::current_dir()?,
};
let spread_file = resolve_paths(spread_file, &env, &cwd);

engine::apply(&spread_file, &mut backend)?;

Ok(())
match cli.command {
Command::Apply { file } => {
let config_path = resolve_config_path(file, &env)?;
let contents = read_config(&config_path)?;
let spread_file = match validate::validate_config(validate::SourceFile {
yaml: &contents,
path: &config_path,
}) {
Ok(f) => f,
Err(findings) => {
validate::print_findings(&findings);
std::process::exit(1);
}
};

let bin = CliBackend::resolve_bin(&env);
let mut backend = CliBackend::new(bin);

let cwd = match env
.get("HERDR_PANE_ID")
.and_then(|id| backend.query_pane_cwd(id))
{
Some(cwd) => cwd,
None => std::env::current_dir()?,
};
let spread_file = resolve_paths(spread_file, &env, &cwd);

engine::apply(&spread_file, &mut backend)?;

Ok(())
}
}
}
Loading
Loading