[cargo-nextest] fail if --target wasn't understood

Currently, if nextest cannot parse a target triple it prints a warning, and
instead just builds on the host platform. I think that isn't the right
behavior, and instead nextest should loudly fail.

Implement this change: fail with a clear error message. I consider the current
behavior a bug, so I don't think this needs to go through the behavior change
process.

Also, hook up miette support to produce better error messages if deserializing
custom JSON fails, and add tests to verify this behavior.

For target runners, we continue to warn rather than fail.

Author: sunshowers

Cargo.lock

@@ -47,6 +47,7 @@ clap = { version = "4.5.23", features = ["derive"] }
 console-subscriber = "0.4.1"
 cp_r = "0.5.2"
 crossterm = { version = "0.28.1", features = ["event-stream"] }
+datatest-stable = { version = "0.3.0", features = ["include-dir"] }
 dialoguer = "0.11.0"
 debug-ignore = "1.0.5"
 derive-where = "1.2.7"

Cargo.toml

@@ -187,7 +187,7 @@ pub(crate) struct CargoOptions {
 
     // NOTE: this does not conflict with reuse build opts (not part of the cargo-opts group) since
     // we let target.runner be specified this way
-    /// Override a configuration value
+    /// Override a Cargo configuration value
     #[arg(long, value_name = "KEY=VALUE", help_heading = "Other Cargo options")]
     pub(crate) config: Vec<String>,
 

cargo-nextest/src/cargo_cli.rs

@@ -21,7 +21,7 @@ use nextest_runner::{
         ToolConfigFile, VersionOnlyConfig,
     },
     double_spawn::DoubleSpawnInfo,
-    errors::WriteTestListError,
+    errors::{TargetTripleError, WriteTestListError},
     input::InputHandlerKind,
     list::{
         BinaryList, OutputFormat, RustTestArtifact, SerializableFormat, TestExecuteContext,
@@ -1160,18 +1160,16 @@ impl BaseApp {
                 let host = HostPlatform::current(PlatformLibdir::from_rustc_stdout(
                     RustcCli::print_host_libdir().read(),
                 ))?;
-                let target = if let Some(triple) = discover_target_triple(
-                    &cargo_configs,
-                    cargo_opts.target.as_deref(),
-                    &output.stderr_styles(),
-                ) {
+
+                let triple_info =
+                    discover_target_triple(&cargo_configs, cargo_opts.target.as_deref())?;
+                let target = triple_info.map(|triple| {
                     let libdir = PlatformLibdir::from_rustc_stdout(
                         RustcCli::print_target_libdir(&triple).read(),
                     );
-                    Some(TargetPlatform::new(triple, libdir))
-                } else {
-                    None
-                };
+                    TargetPlatform::new(triple, libdir)
+                });
+
                 BuildPlatforms { host, target }
             }
         };
@@ -2102,6 +2100,17 @@ enum DebugCommand {
 
     /// Print the current executable path.
     CurrentExe,
+
+    /// Show the target platform that nextest would use.
+    ShowTarget {
+        /// The target triple to use.
+        #[arg(long)]
+        target: Option<String>,
+
+        /// Override a Cargo configuration value.
+        #[arg(long, value_name = "KEY=VALUE")]
+        config: Vec<String>,
+    },
 }
 
 impl DebugCommand {
@@ -2162,6 +2171,15 @@ impl DebugCommand {
                     .map_err(|err| ExpectedError::GetCurrentExeFailed { err })?;
                 println!("{}", exe.display());
             }
+            DebugCommand::ShowTarget { target, config } => {
+                let cargo_configs = CargoConfigs::new(&config).map_err(Box::new)?;
+                let target = discover_target_triple(&cargo_configs, target.as_deref())?;
+                if let Some(target) = target {
+                    println!("{:#?}", target);
+                } else {
+                    println!("no target triple found");
+                }
+            }
         }
 
         Ok(0)
@@ -2286,28 +2304,19 @@ fn acquire_graph_data(
 fn discover_target_triple(
     cargo_configs: &CargoConfigs,
     target_cli_option: Option<&str>,
-    styles: &StderrStyles,
-) -> Option<TargetTriple> {
-    match TargetTriple::find(cargo_configs, target_cli_option) {
-        Ok(Some(triple)) => {
+) -> Result<Option<TargetTriple>, TargetTripleError> {
+    TargetTriple::find(cargo_configs, target_cli_option).inspect(|v| {
+        if let Some(triple) = v {
             debug!(
                 "using target triple `{}` defined by `{}`; {}",
                 triple.platform.triple_str(),
                 triple.source,
                 triple.location,
             );
-            Some(triple)
-        }
-        Ok(None) => {
+        } else {
             debug!("no target triple found, assuming no cross-compilation");
-
-            None
-        }
-        Err(err) => {
-            warn_on_err("target triple", &err, styles);
-            None
         }
-    }
+    })
 }
 
 fn runner_for_target(

cargo-nextest/src/dispatch.rs

@@ -657,8 +657,14 @@ impl ExpectedError {
                 Some(err as &dyn Error)
             }
             Self::TargetTripleError { err } => {
-                error!("{err}");
-                err.source()
+                if let Some(report) = err.source_report() {
+                    // Display the miette report if available.
+                    error!(target: "cargo_nextest::no_heading", "{report:?}");
+                    None
+                } else {
+                    error!("{err}");
+                    err.source()
+                }
             }
             Self::MetadataMaterializeError { arg_name, err } => {
                 error!(

cargo-nextest/src/errors.rs

@@ -17,6 +17,10 @@ path = "test-helpers/build-seed-archive.rs"
 name = "custom-harness"
 harness = false
 
+[[test]]
+name = "datatest"
+harness = false
+
 [dependencies]
 camino.workspace = true
 camino-tempfile.workspace = true
@@ -44,6 +48,7 @@ whoami.workspace = true
 [dev-dependencies]
 cfg-if.workspace = true
 cp_r.workspace = true
+datatest-stable.workspace = true
 fixture-data.workspace = true
 insta.workspace = true
 itertools.workspace = true
@@ -52,6 +57,7 @@ nextest-metadata.workspace = true
 pathdiff.workspace = true
 regex.workspace = true
 target-spec.workspace = true
+target-spec-miette = { workspace = true, features = ["fixtures"] }
 tokio.workspace = true
 
 # These platforms are supported by num_threads.

integration-tests/Cargo.toml

@@ -0,0 +1,46 @@
+// Copyright (c) The nextest Contributors
+// SPDX-License-Identifier: MIT OR Apache-2.0
+
+use crate::helpers::bind_insta_settings;
+use camino::Utf8Path;
+use camino_tempfile::Utf8TempDir;
+use integration_tests::nextest_cli::CargoNextestCli;
+use nextest_metadata::NextestExitCode;
+
+pub(crate) fn custom_invalid(path: &Utf8Path, contents: String) -> datatest_stable::Result<()> {
+    let (_guard, insta_prefix) = bind_insta_settings(path, "snapshots/custom-invalid");
+
+    let dir = Utf8TempDir::with_prefix("nextest-custom-target-")?;
+    let json_path = dir.path().join(path.file_name().unwrap());
+    std::fs::write(&json_path, contents)?;
+
+    let output = CargoNextestCli::for_test()
+        .args([
+            // Use color in snapshots to ensure that it is correctly passed
+            // through.
+            //
+            // It might be nice to use snapbox in the future, because it has
+            // really nice color support.
+            "--color",
+            "always",
+            "debug",
+            "show-target",
+            "--target",
+            json_path.as_str(),
+        ])
+        .unchecked(true)
+        .output();
+
+    // We expect this to fail with a setup error.
+    assert!(!output.exit_status.success());
+    assert_eq!(
+        output.exit_status.code(),
+        Some(NextestExitCode::SETUP_ERROR),
+        "exit code matches"
+    );
+
+    // Print the output.
+    insta::assert_snapshot!(format!("{insta_prefix}-stderr"), output.stderr_as_str());
+
+    Ok(())
+}

integration-tests/tests/datatest/custom_target.rs

@@ -0,0 +1,22 @@
+// Copyright (c) The nextest Contributors
+// SPDX-License-Identifier: MIT OR Apache-2.0
+
+use datatest_stable::Utf8Path;
+use insta::internals::SettingsBindDropGuard;
+
+/// Binds insta settings for a test, and returns the prefix to use for snapshots.
+pub(crate) fn bind_insta_settings<'a>(
+    path: &'a Utf8Path,
+    snapshot_path: &str,
+) -> (SettingsBindDropGuard, &'a str) {
+    let mut settings = insta::Settings::clone_current();
+    // Make insta suitable for datatest-stable use.
+    settings.set_input_file(path);
+    settings.set_snapshot_path(snapshot_path);
+    settings.set_prepend_module_to_snapshot(false);
+
+    let guard = settings.bind_to_scope();
+    let insta_prefix = path.file_name().unwrap();
+
+    (guard, insta_prefix)
+}