Skip to content

Use native Rust tests for Rust frontend#21805

Open
MikeMcQuaid wants to merge 1 commit intomainfrom
rust-frontend-ci-tests
Open

Use native Rust tests for Rust frontend#21805
MikeMcQuaid wants to merge 1 commit intomainfrom
rust-frontend-ci-tests

Conversation

@MikeMcQuaid
Copy link
Member

  • add unit and CLI parity tests under rust/brew-rs
  • remove the Ruby cmd/brew_rs integration spec
  • call cargo directly in brew-rs.yml and gate benchmarks

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same change?
  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests (excluding integration tests) for your changes? Here's an example.
  • Have you successfully run brew lgtm (style, typechecking and tests) with your changes locally?

  • AI was used to generate or assist with generating this PR. Please specify below how you used AI to help you, and what steps you have taken to manually verify the changes.

OpenAI Codex with several rounds of manual review.


- add unit and CLI parity tests under rust/brew-rs
- remove the Ruby cmd/brew_rs integration spec
- call cargo directly in brew-rs.yml and gate benchmarks
Copilot AI review requested due to automatic review settings March 22, 2026 19:26
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Migrates Rust-frontend verification from a Ruby integration spec to native Rust unit + CLI parity tests, and updates CI to run those tests directly via Cargo while adding a benchmark regression gate.

Changes:

  • Remove the cmd/brew_rs Ruby integration spec and add Rust-native CLI parity tests under rust/brew-rs/tests.
  • Add/expand Rust unit tests for matcher, Homebrew helpers, and command logic (search/list).
  • Update brew-rs.yml to run cargo fmt/clippy/test directly and gate performance via rake benchmark:check (with artifact upload).

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
Library/Homebrew/test/cmd/brew_rs_spec.rb Removes Ruby integration coverage for brew-rs dispatch/parity.
Library/Homebrew/rust/brew-rs/tests/cli.rs Adds Rust CLI parity tests comparing Rust vs Ruby outputs.
Library/Homebrew/rust/brew-rs/src/matcher.rs Adds unit tests for matcher parsing/matching behavior.
Library/Homebrew/rust/brew-rs/src/homebrew.rs Adds unit tests for file/line utilities.
Library/Homebrew/rust/brew-rs/src/commands/search.rs Adds unit tests for fuzzy vs plain match behavior.
Library/Homebrew/rust/brew-rs/src/commands/list.rs Adjusts list output behavior + adds unit tests for keg/cask path resolution.
Library/Homebrew/rust/brew-rs/Rakefile Enhances benchmarking to export JSON, summarize results, and fail CI on regression.
Library/Homebrew/rust/brew-rs/README.md Updates build/check instructions away from rake build/check and away from brew tests.
Library/Homebrew/rust/brew-rs/AGENTS.md Updates contributor instructions to use vendor-install + cargo commands and benchmark gate.
.github/workflows/brew-rs.yml Switches workflow to cargo fmt/clippy/test, removes Ruby spec run, adds benchmark gating + artifacts.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 21 to +26
if args.len() == 1 {
let formulae = homebrew::installed_names(&cellar)?;
let casks = homebrew::installed_names(&caskroom)?;
homebrew::print_sections(&formulae, &casks);
let mut installed = homebrew::installed_names(&cellar)?;
installed.extend(homebrew::installed_names(&caskroom)?);
if !installed.is_empty() {
println!("{}", installed.join("\n"));
}
Copy link

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

brew list in the Rust frontend now prints all installed formulae + casks as a single newline-joined list with no TTY headings/section separation. The Ruby implementation prints ohai "Formulae"/ohai "Casks" when $stdout.tty? (see Library/Homebrew/cmd/list.rb), so this is a user-visible parity regression when running interactively. Consider restoring TTY-aware section headers (and any spacing) while keeping the non-TTY output identical to Ruby for the CLI parity tests.

Copilot uses AI. Check for mistakes.
Comment on lines 36 to 40
## Build

```bash
cd Library/Homebrew/rust/brew-rs
rake build
./bin/brew vendor-install brew-rs
```
Copy link

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The build instructions use ./bin/brew vendor-install brew-rs, but this README is located under Library/Homebrew/rust/brew-rs/ and the surrounding examples previously had you cd into that directory. As written, running this from Library/Homebrew/rust/brew-rs will fail because ./bin/brew won’t exist there. Please clarify the expected working directory (e.g., “from the repo root”) or adjust the path accordingly.

Copilot uses AI. Check for mistakes.
Comment on lines +10 to +14
- Run `./bin/brew vendor-install brew-rs` to vendor the binary locally.
- Run `cargo fmt --check`, `cargo clippy --all-targets --locked -- -D warnings`, and `cargo test --locked` from `Library/Homebrew/rust/brew-rs`.
- Use `BREW_RS_STAGE_REPOSITORY=/path/to/Homebrew rake stage` to stage into another checkout.
- Use `rake benchmark` for Ruby vs Rust benchmarks.
- Use `rake benchmark:check` for the CI benchmark gate.
Copy link

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

./bin/brew vendor-install brew-rs assumes the repository root as the working directory, but this guidance appears inside Library/Homebrew/rust/brew-rs/AGENTS.md where earlier instructions were explicitly run from within this directory. Please clarify “from the repo root” or update the command to a correct relative path from Library/Homebrew/rust/brew-rs.

Copilot uses AI. Check for mistakes.
Comment on lines +114 to +121
{
slug: "list",
title: "brew list",
ruby_command: shell_command(BREW, "list", env: common_env),
rust_command: shell_command(BREW, "list", env: rust_env),
runs: 5,
warmup: 2,
},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that an unqualified brew list will not call the Rust version, because we invoke the shell fast path here:

# falls back to cmd/list.rb on a non-zero return
list* | ls*)
source "${HOMEBREW_LIBRARY}/Homebrew/list.sh"
homebrew-list "$@" && exit 0
;;

To reach the Rust frontend, you need to do something like brew list <formula-name>.

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.

3 participants