Skip to content

Commit

Permalink
Collect insta version in standard cargo metadata call (#655)
Browse files Browse the repository at this point in the history
...otherwise it doesn't respond to `--manifest-path`. If it does raise
an error, we just continue after printing a warning, rather than
panicking.

Also added some docs on running a reproducible example in another path.

(I've also found a bug with `--workspace-root`, will handle separately,
and will add a test for both)
  • Loading branch information
max-sixty authored Oct 10, 2024
1 parent c3d6f16 commit 19a2faf
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 27 deletions.
6 changes: 6 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,12 @@ cargo run -p cargo-insta -- test # (or `review` or whatever command you want to
...in contrast to running `cargo insta`, which invokes the installed version of
`cargo-insta`, and so make iterating more difficult.

To run the version of `cargo-insta` in the working directory on another crate, run:

```sh
cargo run -p cargo-insta -- test --manifest-path=../insta-bug-repro/Cargo.toml
```

## Writing tests

If making non-trivial changes to `cargo-insta`, please add an integration test to
Expand Down
25 changes: 20 additions & 5 deletions cargo-insta/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ use uuid::Uuid;
use crate::cargo::{find_snapshot_roots, Package};
use crate::container::{Operation, SnapshotContainer};
use crate::utils::cargo_insta_version;
use crate::utils::INSTA_VERSION;
use crate::utils::{err_msg, QuietExit};
use crate::walk::{find_pending_snapshots, make_snapshot_walker, FindFlags};

Expand Down Expand Up @@ -395,6 +394,9 @@ struct LocationInfo<'a> {
packages: Vec<Package>,
exts: Vec<&'a str>,
find_flags: FindFlags,
/// The insta version in the current workspace (i.e. not the `cargo-insta`
/// binary that's running).
insta_version: Version,
}

fn get_find_flags(tool_config: &ToolConfig, target_args: &TargetArgs) -> FindFlags {
Expand Down Expand Up @@ -439,7 +441,7 @@ fn handle_target_args<'a>(
(None, None) => {}
};

let metadata = cmd.no_deps().exec()?;
let metadata = cmd.exec()?;
let workspace_root = metadata.workspace_root.as_std_path().to_path_buf();
let tool_config = ToolConfig::from_workspace(&workspace_root)?;

Expand All @@ -462,6 +464,13 @@ fn handle_target_args<'a>(
} else {
vec![metadata.root_package().unwrap().clone()]
};
let insta_version = metadata
.packages
.iter()
.find(|package| package.name == "insta")
.map(|package| package.version.clone())
.ok_or_else(|| eprintln!("insta not found in cargo metadata; defaulting to 1.0.0"))
.unwrap_or(Version::new(1, 0, 0));

Ok(LocationInfo {
workspace_root,
Expand All @@ -478,6 +487,7 @@ fn handle_target_args<'a>(
.collect(),
find_flags: get_find_flags(&tool_config, target_args),
tool_config,
insta_version
})
}

Expand Down Expand Up @@ -693,6 +703,7 @@ fn test_run(mut cmd: TestCommand, color: ColorWhen) -> Result<(), Box<dyn Error>
color,
&[],
None,
&loc,
)?;

if !cmd.keep_pending {
Expand All @@ -717,6 +728,7 @@ fn test_run(mut cmd: TestCommand, color: ColorWhen) -> Result<(), Box<dyn Error>
color,
&["--doc"],
snapshot_ref_file.as_deref(),
&loc,
)?;
success = success && proc.status()?.success();
}
Expand Down Expand Up @@ -892,7 +904,9 @@ fn handle_unreferenced_snapshots(
}

/// Create and setup a `Command`, translating our configs into env vars & cli options
// TODO: possibly we can clean this function up a bit, reduce the number of args
#[allow(clippy::type_complexity)]
#[allow(clippy::too_many_arguments)]
fn prepare_test_runner<'snapshot_ref>(
test_runner: TestRunner,
test_runner_fallback: bool,
Expand All @@ -901,6 +915,7 @@ fn prepare_test_runner<'snapshot_ref>(
color: ColorWhen,
extra_args: &[&str],
snapshot_ref_file: Option<&'snapshot_ref Path>,
loc: &LocationInfo,
) -> Result<(process::Command, Option<Cow<'snapshot_ref, Path>>, bool), Box<dyn Error>> {
let cargo = env::var_os("CARGO");
let cargo = cargo
Expand All @@ -910,7 +925,7 @@ fn prepare_test_runner<'snapshot_ref>(
// `test_runner_fallback` is true
let test_runner = if test_runner == TestRunner::Nextest
&& test_runner_fallback
&& std::process::Command::new("cargo")
&& std::process::Command::new(cargo)
.arg("nextest")
.arg("--version")
.output()
Expand Down Expand Up @@ -1006,7 +1021,7 @@ fn prepare_test_runner<'snapshot_ref>(
"INSTA_UPDATE",
// Don't set `INSTA_UPDATE=force` for `--force-update-snapshots` on
// older versions
if *INSTA_VERSION >= Version::new(1,41,0) {
if loc.insta_version >= Version::new(1,41,0) {
match (cmd.check, cmd.accept_unseen, cmd.force_update_snapshots) {
(true, false, false) => "no",
(false, true, false) => "unseen",
Expand All @@ -1022,7 +1037,7 @@ fn prepare_test_runner<'snapshot_ref>(
}
}
);
if cmd.force_update_snapshots && *INSTA_VERSION < Version::new(1, 40, 0) {
if cmd.force_update_snapshots && loc.insta_version < Version::new(1, 40, 0) {
// Currently compatible with older versions of insta.
proc.env("INSTA_FORCE_UPDATE_SNAPSHOTS", "1");
proc.env("INSTA_FORCE_UPDATE", "1");
Expand Down
22 changes: 1 addition & 21 deletions cargo-insta/src/utils.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,6 @@
use std::error::Error;
use std::fmt;

use cargo_metadata::MetadataCommand;
use lazy_static::lazy_static;
use semver::Version;

/// Close without message but exit code.
#[derive(Debug)]
pub(crate) struct QuietExit(pub(crate) i32);
Expand Down Expand Up @@ -32,23 +28,7 @@ pub(crate) fn err_msg<S: Into<String>>(s: S) -> Box<dyn Error> {
Box::new(ErrMsg(s.into()))
}

/// The insta version in the current workspace (i.e. not the `cargo-insta`
/// binary that's running).
fn read_insta_version() -> Result<Version, Box<dyn std::error::Error>> {
MetadataCommand::new()
.exec()?
.packages
.iter()
.find(|package| package.name == "insta")
.map(|package| package.version.clone())
.ok_or("insta not found in cargo metadata".into())
}

lazy_static! {
pub static ref INSTA_VERSION: Version = read_insta_version().unwrap();
}

/// `cargo-insta` version
/// `cargo-insta` version (i.e. the binary that's currently running).
// We could put this in a lazy_static
pub(crate) fn cargo_insta_version() -> String {
env!("CARGO_PKG_VERSION").to_string()
Expand Down
11 changes: 10 additions & 1 deletion insta/src/env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -406,7 +406,9 @@ pub fn snapshot_update_behavior(tool_config: &ToolConfig, unseen: bool) -> Snaps
}
}

/// Returns the cargo workspace for a manifest
/// Returns the cargo workspace path for a crate manifest, like
/// `/Users/janedoe/projects/insta` when passed
/// `/Users/janedoe/projects/insta/insta/Cargo.toml`.
pub fn get_cargo_workspace(manifest_dir: &str) -> Arc<PathBuf> {
// If INSTA_WORKSPACE_ROOT environment variable is set, use the value as-is.
// This is useful where CARGO_MANIFEST_DIR at compilation points to some
Expand Down Expand Up @@ -465,6 +467,13 @@ pub fn get_cargo_workspace(manifest_dir: &str) -> Arc<PathBuf> {
.clone()
}

#[test]
fn test_get_cargo_workspace() {
let workspace = get_cargo_workspace(env!("CARGO_MANIFEST_DIR"));
// The absolute path of the workspace, like `/Users/janedoe/projects/insta`
assert!(workspace.ends_with("insta"));
}

#[cfg(feature = "_cargo_insta_internal")]
impl std::str::FromStr for TestRunner {
type Err = ();
Expand Down

0 comments on commit 19a2faf

Please sign in to comment.