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
73 changes: 73 additions & 0 deletions docs/plans/2026-03-10-fix-cargo-fmt-test-plan.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
# Fix cargo fmt CI Failure — Test Plan

## Strategy reconciliation

The testing strategy (run `cargo fmt -- --check`, `cargo clippy`, `cargo test`) maps 1:1 to the CI workflow steps in `.github/workflows/ci.yml`. The implementation plan makes formatting changes via `cargo fmt` and fixes pre-existing `clippy::collapsible_if` warnings in `tests/e2e.rs` that were previously masked by the format failure. No new interfaces or external dependencies.

**Source of truth:** `.github/workflows/ci.yml` defines the exact commands and flags the CI pipeline runs. All assertions below mirror those commands.

## Harness requirements

None. All tests use the local Rust toolchain (`cargo fmt`, `cargo clippy`, `cargo test`) which is already available. No test harnesses need to be built.

## Test plan

### 1. Formatting check passes with zero diffs

- **Name:** `cargo fmt --check` produces no diffs after formatting
- **Type:** invariant
- **Harness:** `cargo fmt -- --check` (same command as CI)
- **Preconditions:** `cargo fmt` has been run on the worktree
- **Actions:** Run `cargo fmt -- --check`
- **Expected outcome:** Exit code 0, empty stdout. Source of truth: `.github/workflows/ci.yml` line 27 (`cargo fmt -- --check`). Any non-zero exit or diff output means the fix is incomplete.
- **Interactions:** None. Pure formatting check, no compilation or linking.

### 2. Clippy passes with no warnings

- **Name:** Clippy lints pass after formatting changes
- **Type:** integration
- **Harness:** `cargo clippy --all-targets -- -D warnings` (same command as CI)
- **Preconditions:** Source files have been reformatted by `cargo fmt`
- **Actions:** Run `cargo clippy --all-targets -- -D warnings`
- **Expected outcome:** Exit code 0, no warnings or errors. Source of truth: `.github/workflows/ci.yml` line 29 (`cargo clippy --all-targets -- -D warnings`). This step was previously blocked by the format failure. Pre-existing clippy warnings (e.g., `clippy::collapsible_if` in `tests/e2e.rs`) must also be fixed to achieve a clean pass.
- **Interactions:** Exercises the full Rust compilation pipeline. Any pre-existing clippy issue unrelated to formatting will surface here and must be addressed.

### 3. Test suite passes after formatting changes

- **Name:** All tests pass after formatting changes
- **Type:** integration
- **Harness:** `cargo test` (same command as CI)
- **Preconditions:** Source files have been reformatted by `cargo fmt`
- **Actions:** Run `cargo test`
- **Expected outcome:** Exit code 0, all tests pass. Source of truth: `.github/workflows/ci.yml` line 31 (`cargo test`). Formatting changes cannot alter logic, but this confirms the third CI step passes end-to-end.
- **Interactions:** Compiles and runs all unit and integration tests including `tests/e2e.rs` (which is one of the reformatted files). The e2e tests invoke the compiled binary as a subprocess, so this exercises the full build-and-run path.

### 4. Only formatting and clippy-lint changes in the diff

- **Name:** Diff contains only whitespace/line-wrapping changes and semantically equivalent clippy lint fixes
- **Type:** invariant
- **Harness:** `git diff` inspection
- **Preconditions:** `cargo fmt` has been run and clippy warnings have been fixed, but changes are not yet committed (or compare the commit diff)
- **Actions:** Run `git diff -- src/ tests/` and inspect the output
- **Expected outcome:** In `src/` files, all hunks contain only whitespace, indentation, and line-break changes. In `tests/e2e.rs`, hunks also include collapsing nested `if`/`if let` into `if ... && let ...` chains (a semantically equivalent AST-level change to satisfy `clippy::collapsible_if`). The affected files are exactly: `src/commands/init.rs`, `src/commands/ls.rs`, `src/config.rs`, `src/platform.rs`, `tests/e2e.rs`.
- **Interactions:** None.

### 5. Correct files are modified

- **Name:** Exactly the 5 expected files are changed, no others
- **Type:** boundary
- **Harness:** `git diff --name-only`
- **Preconditions:** `cargo fmt` has been run
- **Actions:** Run `git diff --name-only` (or `git diff --name-only HEAD~1` after commit)
- **Expected outcome:** Output contains exactly these 5 paths (in any order): `src/commands/init.rs`, `src/commands/ls.rs`, `src/config.rs`, `src/platform.rs`, `tests/e2e.rs`. No other files are modified. Source of truth: the `cargo fmt -- --check` output which identified diffs in exactly these files.
- **Interactions:** None.

## Coverage summary

**Covered:**
- All three CI `check` job steps (`fmt --check`, `clippy`, `test`) are verified with the exact same commands and flags used in CI.
- Diff correctness: only formatting and semantically equivalent clippy-lint changes, only expected files.

**Excluded (per strategy):**
- The `install-script` CI job (`tests/test_install.sh`) is not affected by Rust source formatting and is excluded. Risk: none — that job is independent and was not failing.
- No cross-platform or Docker-based Linux test verification. The CI runs on `ubuntu-latest`; local verification runs on macOS. Risk: negligible for formatting-only changes, since `rustfmt` output is platform-independent.
64 changes: 64 additions & 0 deletions docs/plans/2026-03-10-fix-cargo-fmt.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
# Fix cargo fmt CI Failure Implementation Plan

> **For Claude:** REQUIRED SUB-SKILL: Use trycycle-executing to implement this plan task-by-task.

**Goal:** Fix all `cargo fmt` formatting violations so the GitHub Actions CI pipeline passes.

**Architecture:** Run `cargo fmt` to auto-format all Rust source files, then fix any pre-existing `clippy` lint warnings that were previously masked by the format failure, and verify the full CI check sequence (`fmt --check`, `clippy`, `test`) passes.

**Tech Stack:** Rust toolchain (cargo fmt, clippy, cargo test)

**Decision: Use `cargo fmt` auto-format rather than manual edits.**
All 17 diffs across 5 files are standard rustfmt line-length and argument-wrapping reformats. Running `cargo fmt` is idiomatic, deterministic, and guaranteed to produce the exact output `cargo fmt -- --check` expects. Manual editing would be error-prone and pointless.

**Note (discovered during implementation):** After `cargo fmt` fixes the format check, `cargo clippy` reveals 4 pre-existing `clippy::collapsible_if` warnings in `tests/e2e.rs` that were previously masked because CI failed at the earlier `fmt --check` step. These require collapsing nested `if`/`if let` chains into single `if ... && let ...` expressions — a semantically equivalent but AST-level change, not just whitespace.

---

### Task 1: Run cargo fmt to auto-format all files

**Files:**
- Modify: `src/commands/init.rs` (2 formatting diffs — long method chain and long `eprintln!` call)
- Modify: `src/commands/ls.rs` (4 formatting diffs — long `println!` and multi-arg `assert!` calls)
- Modify: `src/config.rs` (5 formatting diffs — long method chains and long `.args()` arrays)
- Modify: `src/platform.rs` (1 formatting diff — `unwrap_or_else` chain wrapping)
- Modify: `tests/e2e.rs` (5 formatting diffs — `.args()` arrays, `temp_dir().join()` chains, compact `assert!`)

**Step 1: Run cargo fmt**

```bash
cargo fmt
```

Expected: exits 0, no output. All 5 files are reformatted in place.

**Step 2: Verify cargo fmt --check passes**

```bash
cargo fmt -- --check
```

Expected: exits 0 with no output (no remaining diffs).

**Step 3: Run cargo clippy**

```bash
cargo clippy --all-targets -- -D warnings
```

Expected: exits 0. If pre-existing clippy warnings surface (previously masked by the format failure), fix them before proceeding.

**Step 4: Run cargo test**

```bash
cargo test
```

Expected: exits 0. Formatting changes cannot break tests, but this mirrors the CI step and confirms the full `check` job would pass.

**Step 5: Commit**

```bash
git add src/commands/init.rs src/commands/ls.rs src/config.rs src/platform.rs tests/e2e.rs
git commit -m "style: apply cargo fmt and fix clippy collapsible-if warnings"
```
11 changes: 9 additions & 2 deletions src/commands/init.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,10 @@ pub fn is_devproxy_process(pid: i32) -> bool {
// On Linux, read /proc/<pid>/exe symlink.
let exe = std::fs::read_link(format!("/proc/{pid}/exe"));
match exe {
Ok(path) => path.file_name().map(|n| n == "devproxy" || n == "devproxy-daemon").unwrap_or(false),
Ok(path) => path
.file_name()
.map(|n| n == "devproxy" || n == "devproxy-daemon")
.unwrap_or(false),
Err(_) => false,
}
}
Expand Down Expand Up @@ -328,7 +331,11 @@ pub fn install_daemon_binary(src: &std::path::Path, dest: &std::path::Path) -> R

super::update::prepare_binary(dest)?;

eprintln!("{} daemon binary installed at {}", "ok:".green(), dest.display());
eprintln!(
"{} daemon binary installed at {}",
"ok:".green(),
dest.display()
);
Ok(())
}

Expand Down
22 changes: 18 additions & 4 deletions src/commands/ls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,12 @@ pub async fn run() -> Result<()> {
.unwrap_or(3)
.max(3); // at least "URL".len()

println!(" {:<width$} {}", "URL".bold(), "PORT".bold(), width = url_width + 2);
println!(
" {:<width$} {}",
"URL".bold(),
"PORT".bold(),
width = url_width + 2
);
for route in &routes {
let is_current = current_slug.as_deref() == Some(&route.slug);
let marker = if is_current { "* " } else { " " };
Expand Down Expand Up @@ -75,7 +80,10 @@ mod tests {
port: 51234,
};
let line = format_route_line(&route, Some("swift-penguin-devproxy.mysite.dev"));
assert!(line.starts_with("* "), "current project should have * marker: {line}");
assert!(
line.starts_with("* "),
"current project should have * marker: {line}"
);
}

#[test]
Expand All @@ -85,7 +93,10 @@ mod tests {
port: 51235,
};
let line = format_route_line(&route, Some("swift-penguin-devproxy.mysite.dev"));
assert!(line.starts_with(" "), "non-current project should not have * marker: {line}");
assert!(
line.starts_with(" "),
"non-current project should not have * marker: {line}"
);
}

#[test]
Expand All @@ -95,6 +106,9 @@ mod tests {
port: 51234,
};
let line = format_route_line(&route, None);
assert!(line.starts_with(" "), "no current project means no marker: {line}");
assert!(
line.starts_with(" "),
"no current project means no marker: {line}"
);
}
}
42 changes: 35 additions & 7 deletions src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,12 @@ pub fn compose_slug(random_slug: &str, app_name: &str) -> String {
if composite.len() <= 63 {
return composite;
}
composite.chars().take(63).collect::<String>().trim_end_matches('-').to_string()
composite
.chars()
.take(63)
.collect::<String>()
.trim_end_matches('-')
.to_string()
}

/// Find a free ephemeral port
Expand Down Expand Up @@ -487,7 +492,12 @@ services:
.output()
.unwrap();
std::process::Command::new("git")
.args(["remote", "add", "origin", "https://github.com/user/my-cool-app.git"])
.args([
"remote",
"add",
"origin",
"https://github.com/user/my-cool-app.git",
])
.current_dir(dir.path())
.output()
.unwrap();
Expand All @@ -504,7 +514,12 @@ services:
.output()
.unwrap();
std::process::Command::new("git")
.args(["remote", "add", "origin", "git@github.com:user/another-app.git"])
.args([
"remote",
"add",
"origin",
"git@github.com:user/another-app.git",
])
.current_dir(dir.path())
.output()
.unwrap();
Expand Down Expand Up @@ -573,16 +588,29 @@ services:

#[test]
fn compose_slug_basic() {
assert_eq!(compose_slug("swift-penguin", "devproxy"), "swift-penguin-devproxy");
assert_eq!(
compose_slug("swift-penguin", "devproxy"),
"swift-penguin-devproxy"
);
}

#[test]
fn compose_slug_truncates_to_63_chars() {
let long_app = "a".repeat(100);
let result = compose_slug("swift-penguin", &long_app);
assert!(result.len() <= 63, "composite slug must fit in a DNS label: len={}", result.len());
assert!(!result.ends_with('-'), "should not end with hyphen after truncation");
assert!(result.starts_with("swift-penguin-"), "should preserve the random slug prefix");
assert!(
result.len() <= 63,
"composite slug must fit in a DNS label: len={}",
result.len()
);
assert!(
!result.ends_with('-'),
"should not end with hyphen after truncation"
);
assert!(
result.starts_with("swift-penguin-"),
"should preserve the random slug prefix"
);
}

#[test]
Expand Down
5 changes: 2 additions & 3 deletions src/platform.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,8 @@ pub fn generate_launchagent_plist(
// launchd provides a minimal PATH (/usr/bin:/bin:/usr/sbin:/sbin) that
// excludes /usr/local/bin (Docker on macOS) and ~/.local/bin. Include
// the common paths so the daemon can find docker, etc.
let path_value = std::env::var("PATH").unwrap_or_else(|_| {
"/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin".to_string()
});
let path_value = std::env::var("PATH")
.unwrap_or_else(|_| "/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin".to_string());
let path_value = xml_escape(&path_value);

let mut env_entries = format!(
Expand Down
Loading
Loading