From b7dea9b21de7fd7d50cd808ee0687bddd385be0a Mon Sep 17 00:00:00 2001 From: Maximilian Roos <5635139+max-sixty@users.noreply.github.com> Date: Sun, 13 Oct 2024 12:21:48 -0700 Subject: [PATCH] Small simplification of `prepare_test_runner` (#666) Still a bit of a mess, but marginally better --- cargo-insta/src/cli.rs | 56 ++++++++++------------------------------ cargo-insta/src/utils.rs | 2 +- insta/src/env.rs | 23 ++++++++++++++++- insta/src/lib.rs | 1 + insta/src/utils.rs | 8 ++++++ 5 files changed, 46 insertions(+), 44 deletions(-) diff --git a/cargo-insta/src/cli.rs b/cargo-insta/src/cli.rs index 6c71c567..c6c7fa33 100644 --- a/cargo-insta/src/cli.rs +++ b/cargo-insta/src/cli.rs @@ -7,7 +7,8 @@ use std::{io, process}; use console::{set_colors_enabled, style, Key, Term}; use insta::_cargo_insta_support::{ - is_ci, SnapshotPrinter, SnapshotUpdate, TestRunner, ToolConfig, UnreferencedSnapshots, + get_cargo, is_ci, SnapshotPrinter, SnapshotUpdate, TestRunner, ToolConfig, + UnreferencedSnapshots, }; use insta::{internals::SnapshotContents, Snapshot}; use itertools::Itertools; @@ -393,8 +394,8 @@ struct LocationInfo<'a> { packages: Vec, exts: Vec<&'a str>, find_flags: FindFlags, - /// The insta version in the current workspace (i.e. not the `cargo-insta` - /// binary that's running). + /// The tested crate's insta version (i.e. not the `cargo-insta` binary + /// that's running this code). insta_version: Version, } @@ -702,21 +703,13 @@ fn test_run(mut cmd: TestCommand, color: ColorWhen) -> Result<(), Box TestRunner::CargoTest => TestRunner::CargoTest, TestRunner::Nextest => TestRunner::Nextest, }; - // Prioritize the command line over the tool config - let test_runner_fallback = cmd - .test_runner_fallback - .unwrap_or(loc.tool_config.test_runner_fallback()); - - let (mut proc, snapshot_ref_file, prevents_doc_run) = prepare_test_runner( - test_runner, - test_runner_fallback, - cmd.unreferenced, - &cmd, - color, - &[], - None, - &loc, - )?; + let test_runner = test_runner.resolve_fallback( + cmd.test_runner_fallback + .unwrap_or(loc.tool_config.test_runner_fallback()), + ); + + let (mut proc, snapshot_ref_file, prevents_doc_run) = + prepare_test_runner(test_runner, cmd.unreferenced, &cmd, color, &[], None, &loc)?; if !cmd.keep_pending { process_snapshots(true, None, &loc, Some(Operation::Reject))?; @@ -737,8 +730,7 @@ fn test_run(mut cmd: TestCommand, color: ColorWhen) -> Result<(), Box // a way to replicate the `cargo test` behavior. if matches!(cmd.test_runner, TestRunner::Nextest) && !prevents_doc_run { let (mut proc, _, _) = prepare_test_runner( - TestRunner::CargoTest, - false, + &TestRunner::CargoTest, cmd.unreferenced, &cmd, color, @@ -923,8 +915,7 @@ fn handle_unreferenced_snapshots( #[allow(clippy::type_complexity)] #[allow(clippy::too_many_arguments)] fn prepare_test_runner<'snapshot_ref>( - test_runner: TestRunner, - test_runner_fallback: bool, + test_runner: &TestRunner, unreferenced: UnreferencedSnapshots, cmd: &TestCommand, color: ColorWhen, @@ -932,26 +923,7 @@ fn prepare_test_runner<'snapshot_ref>( snapshot_ref_file: Option<&'snapshot_ref Path>, loc: &LocationInfo, ) -> Result<(process::Command, Option>, bool), Box> { - let cargo = env::var_os("CARGO"); - let cargo = cargo - .as_deref() - .unwrap_or_else(|| std::ffi::OsStr::new("cargo")); - // Fall back to `cargo test` if `cargo nextest` isn't installed and - // `test_runner_fallback` is true - let test_runner = if test_runner == TestRunner::Nextest - && test_runner_fallback - && std::process::Command::new(cargo) - .arg("nextest") - .arg("--version") - .output() - .map(|output| !output.status.success()) - .unwrap_or(true) - { - TestRunner::Auto - } else { - test_runner - }; - let mut proc = process::Command::new(cargo); + let mut proc = process::Command::new(get_cargo()); match test_runner { TestRunner::CargoTest | TestRunner::Auto => { proc.arg("test"); diff --git a/cargo-insta/src/utils.rs b/cargo-insta/src/utils.rs index dc252b54..94d25d73 100644 --- a/cargo-insta/src/utils.rs +++ b/cargo-insta/src/utils.rs @@ -1,5 +1,5 @@ -use std::error::Error; use std::fmt; +use std::{env, error::Error}; /// Close without message but exit code. #[derive(Debug)] diff --git a/insta/src/env.rs b/insta/src/env.rs index 327f7b8e..3d59f3fb 100644 --- a/insta/src/env.rs +++ b/insta/src/env.rs @@ -4,7 +4,7 @@ use std::path::{Path, PathBuf}; use std::sync::{Arc, Mutex}; use std::{env, fmt, fs}; -use crate::utils::is_ci; +use crate::utils::{get_cargo, is_ci}; use crate::{ content::{yaml, Content}, elog, @@ -37,6 +37,27 @@ pub enum TestRunner { Nextest, } +#[cfg(feature = "_cargo_insta_internal")] +impl TestRunner { + /// Fall back to `cargo test` if `cargo nextest` isn't installed and + /// `test_runner_fallback` is true + pub fn resolve_fallback(&self, test_runner_fallback: bool) -> &TestRunner { + if self == &TestRunner::Nextest + && test_runner_fallback + && std::process::Command::new(get_cargo()) + .arg("nextest") + .arg("--version") + .output() + .map(|output| !output.status.success()) + .unwrap_or(true) + { + &TestRunner::Auto + } else { + self + } + } +} + /// Controls how information is supposed to be displayed. #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum OutputBehavior { diff --git a/insta/src/lib.rs b/insta/src/lib.rs index 663900a0..6aea3b1c 100644 --- a/insta/src/lib.rs +++ b/insta/src/lib.rs @@ -331,6 +331,7 @@ pub mod _cargo_insta_support { snapshot::PendingInlineSnapshot, snapshot::SnapshotContents, snapshot::TextSnapshotContents, + utils::get_cargo, utils::is_ci, }; } diff --git a/insta/src/utils.rs b/insta/src/utils.rs index 35a39da6..8ee58b14 100644 --- a/insta/src/utils.rs +++ b/insta/src/utils.rs @@ -108,6 +108,14 @@ pub fn format_rust_expression(value: &str) -> Cow<'_, str> { Cow::Borrowed(value) } +pub fn get_cargo() -> std::ffi::OsString { + let cargo = env::var_os("CARGO"); + let cargo = cargo + .as_deref() + .unwrap_or_else(|| std::ffi::OsStr::new("cargo")); + cargo.to_os_string() +} + #[test] fn test_format_rust_expression() { use crate::assert_snapshot;