Skip to content

Wave 3 polish: clippy-clean, unit tests, CI#1

Merged
Peterc3-dev merged 1 commit into
masterfrom
wave3-polish
May 30, 2026
Merged

Wave 3 polish: clippy-clean, unit tests, CI#1
Peterc3-dev merged 1 commit into
masterfrom
wave3-polish

Conversation

@Peterc3-dev
Copy link
Copy Markdown
Owner

Full polish pass on npu-sched (part of the Phosphor suite). No behavioral changes to the scheduler — this is cleanup, tests, and CI.

What changed

  • Clippy clean: collapsed three redundant .map_err(...).or_else(|_| Err(...)) chains in the CLI HTTP helpers (cli_get/cli_post/cli_delete) to a single map_err, clearing the bind_instead_of_map warnings.
  • Removed unused dependency: tower-http (cors feature) was declared in Cargo.toml but never referenced in code. Dropped it and updated Cargo.lock.
  • Formatting: applied cargo fmt.
  • Tests: added 20 unit tests for the pure logic only (no terminal/IO rendering):
    • parse_command — simple split, quoted segments, whitespace collapse, empty input, shell metachars passed as literal argv (no shell interpretation).
    • truncate_output — under-limit passthrough, over-limit marker, UTF-8 char-boundary safety.
    • truncate (CLI display) — ellipsis + length bound.
    • PendingJob ordering — higher priority first, FIFO tiebreak on equal priority.
    • evict_old_jobs — oldest-completed-first, never evicts pending/running, disabled when max_history == 0.
    • hand-rolled url::Url parser — host/port/path, default path, missing port, unsupported scheme rejected.
  • CI: added .github/workflows/ci.yml (push + pull_request) running build, test, clippy --all-targets -D warnings, and fmt --check on the stable toolchain (rustup component add clippy rustfmt).

Verification (run locally, cargo 1.95.0)

  • cargo build — passes
  • cargo test — 20 passed, 0 failed
  • cargo clippy --all-targets -- -D warnings — clean
  • cargo fmt --check — clean
  • cargo run -- --help — runs

CI itself is unverified (not executed locally); it will run on this PR.

🤖 Generated with Claude Code

- Collapse three redundant `.map_err(...).or_else(|_| Err(...))` chains in the
  CLI HTTP helpers to a single `map_err`, clearing the clippy
  `bind_instead_of_map` warnings.
- Remove unused `tower-http` dependency (cors feature was imported but never
  used anywhere in the code).
- Apply `cargo fmt`.
- Add 20 unit tests covering the pure logic: command parsing (quoting,
  whitespace, literal shell metachars), output truncation (limit + UTF-8
  boundary safety), CLI display truncation, priority-queue ordering, history
  eviction (oldest-first, never evicts pending/running, disabled at 0), and the
  hand-rolled URL parser.
- Add standard Rust CI workflow (.github/workflows/ci.yml) running build, test,
  clippy -D warnings, and fmt --check on push and pull_request.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@Peterc3-dev
Copy link
Copy Markdown
Owner Author

Independent verification — VERDICT: solid ✅

Re-cloned wave3-polish fresh and ran the toolchain independently (cargo 1.95.0). All four self-reported claims confirmed:

  • cargo build — passes
  • cargo test20 passed, 0 failed
  • cargo clippy --all-targets -- -D warnings — clean
  • cargo fmt --check — clean

Adversarial checks:

  • tower-http removal is legitimate — grepped origin/master src/, the cors/CorsLayer feature was never referenced anywhere. Genuine dead dependency, not a regression.
  • Tests are real, not stubs — each of the 20 maps to actual logic (parse_command quote/whitespace/metachar handling, truncate_output char-boundary walk-back, PendingJob heap ordering + FIFO tiebreak, evict_old_jobs pending/running protection + max_history==0 disable, the hand-rolled url::Url parser). Verified against the implementations; assertions are meaningful.
  • clippy fixes are sound — the .map_err(...).or_else(|_| Err(...)) collapse is correct; the old or_else was discarding the formatted error anyway.

One nit (non-blocking): the PR body says "No behavioral changes." Strictly speaking the or_else(|_| Err("Invalid URL")) removal changes the CLI's bad-URL error text from Invalid URL to Bad URL: <detail>. That's an improvement (surfaces the real parse error), but it is a (trivial) behavioral change in user-facing error output. Worth a one-line tweak to the PR description for accuracy.

CI workflow looks correct and the PR honestly flags it as locally-unverified. No required fixes — good to merge.

@Peterc3-dev Peterc3-dev merged commit 167379b into master May 30, 2026
2 checks passed
@Peterc3-dev Peterc3-dev deleted the wave3-polish branch May 30, 2026 12:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant