Skip to content

Conversation

@ffimnsr
Copy link
Owner

@ffimnsr ffimnsr commented Dec 19, 2025

This pull request refactors the test setup in tests/cmd_test.rs to use a more idiomatic and explicit way of constructing command invocations for the mk binary. The primary change is replacing Command::cargo_bin("mk") with Command::new(cargo::cargo_bin!("mk")) across all test cases, improving clarity and consistency in how test commands are created.

Test infrastructure improvements:

  • Switched from Command::cargo_bin("mk") to Command::new(cargo::cargo_bin!("mk")) in all test functions to standardize command construction and better leverage the assert_cmd::cargo utilities. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11]

  • Added an explicit import of assert_cmd::cargo at the top of the file to support the new command construction method.

Closes: #2

Signed-off-by: Edward Fitz Abucay <29743013+ffimnsr@users.noreply.github.com>
@ffimnsr ffimnsr self-assigned this Dec 19, 2025
Copilot AI review requested due to automatic review settings December 19, 2025 07:37
@ffimnsr ffimnsr added the enhancement New feature or request label Dec 19, 2025
@ffimnsr ffimnsr merged commit af015df into main Dec 19, 2025
4 checks passed
Copy link

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

This PR refactors the test infrastructure in tests/cmd_test.rs to use a different pattern for constructing command invocations. The change replaces Command::cargo_bin("mk")? with Command::new(cargo::cargo_bin!("mk")) across all 14 test functions.

  • Standardizes command construction using the cargo::cargo_bin! macro with Command::new
  • Adds an explicit import for assert_cmd::cargo module
  • Updates dependency versions in Cargo.lock

Reviewed changes

Copilot reviewed 1 out of 2 changed files in this pull request and generated 1 comment.

File Description
tests/cmd_test.rs Refactored all test functions to use Command::new(cargo::cargo_bin!("mk")) instead of Command::cargo_bin("mk")? and added use assert_cmd::cargo import
Cargo.lock Standard dependency version updates including assert_cmd upgrade from 2.0.17 to 2.1.1 and various other transitive dependencies

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

#[test]
fn test_mk_1() -> anyhow::Result<()> {
let mut cmd = Command::cargo_bin("mk")?;
let mut cmd = Command::new(cargo::cargo_bin!("mk"));
Copy link

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

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

The refactoring changes the command construction from Command::cargo_bin("mk")? to Command::new(cargo::cargo_bin!("mk")), which removes error handling. The cargo_bin! macro returns a String (or &str), not a Result, so errors in locating the binary will now panic at runtime instead of being handled gracefully through the ? operator. Additionally, the function signatures still return anyhow::Result<()>, but the ? operator is no longer needed for command construction, making the return type potentially misleading. Consider whether this loss of error handling is intentional.

Copilot uses AI. Check for mistakes.
@ffimnsr ffimnsr deleted the ifn4/2-tests-rely-on-deprecated-snapboxcmdcargo_bin-which-will-break-on-future-cargo-versions branch December 19, 2025 10:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Tests rely on deprecated snapbox::cmd::cargo_bin which will break on future Cargo versions

2 participants