feat: Reduce method duplication (#1502)

xtask/src/cargo.rs

@@ -7,6 +7,12 @@ use std::{
 
 use anyhow::{bail, Result};
 
+#[derive(Debug, PartialEq)]
+pub enum CargoAction {
+    Build,
+    Run,
+}
+
 /// Execute cargo with the given arguments and from the specified directory.
 pub fn run(args: &[String], cwd: &Path) -> Result<()> {
     if !cwd.is_dir() {

xtask/src/lib.rs

@@ -7,6 +7,7 @@ use std::{
 };
 
 use anyhow::{bail, Result};
+use cargo::CargoAction;
 use clap::ValueEnum;
 use strum::{Display, EnumIter, IntoEnumIterator as _};
 
@@ -228,82 +229,45 @@ pub fn load_examples(path: &Path) -> Result<Vec<Metadata>> {
     Ok(examples)
 }
 
-/// Build the specified example for the specified chip.
+/// Run or build the specified test or example for the specified chip.
-pub fn build_example(
+pub fn execute_app(
     package_path: &Path,
     chip: Chip,
     target: &str,
-    example: &Metadata,
+    app: &Metadata,
+    action: &CargoAction,
 ) -> Result<()> {
     log::info!(
         "Building example '{}' for '{}'",
-        example.example_path().display(),
+        app.example_path().display(),
         chip
     );
-    if !example.features().is_empty() {
+    if !app.features().is_empty() {
-        log::info!("  Features: {}", example.features().join(","));
+        log::info!("  Features: {}", app.features().join(","));
     }
 
-    let package = example.example_path().strip_prefix(package_path)?;
+    let package = app.example_path().strip_prefix(package_path)?;
-    let bin = if package.starts_with("src/bin") {
-        format!("--bin={}", example.name())
-    } else if package.starts_with("tests") {
-        format!("--test={}", example.name())
-    } else {
-        format!("--example={}", example.name())
-    };
-
-    let mut features = example.features().to_vec();
-    features.push(chip.to_string());
-
-    let mut builder = CargoArgsBuilder::default()
-        .subcommand("build")
-        .arg("-Zbuild-std=alloc,core")
-        .arg("--release")
-        .target(target)
-        .features(&features)
-        .arg(bin);
-
-    // If targeting an Xtensa device, we must use the '+esp' toolchain modifier:
-    if target.starts_with("xtensa") {
-        builder = builder.toolchain("esp");
-    }
-
-    let args = builder.build();
-    log::debug!("{args:#?}");
-
-    cargo::run(&args, package_path)?;
-
-    Ok(())
-}
-
-/// Run the specified example for the specified chip.
-pub fn run_example(
-    package_path: &Path,
-    chip: Chip,
-    target: &str,
-    example: &Metadata,
-) -> Result<()> {
-    log::info!(
-        "Building example '{}' for '{}'",
-        example.example_path().display(),
-        chip
-    );
-    if !example.features().is_empty() {
-        log::info!("  Features: {}", example.features().join(","));
-    }
-
-    let package = example.example_path().strip_prefix(package_path)?;
     log::info!("Package: {:?}", package);
-    let (bin, subcommand) = if package.starts_with("src/bin") {
+    let (bin, subcommand) = if action == &CargoAction::Build {
-        (format!("--bin={}", example.name()), "run")
+        let bin = if package.starts_with("src/bin") {
-    } else if package.starts_with("tests") {
+            format!("--bin={}", app.name())
-        (format!("--test={}", example.name()), "test")
+        } else if package.starts_with("tests") {
+            format!("--test={}", app.name())
+        } else {
+            format!("--example={}", app.name())
+        };
+        (bin, "build")
     } else {
-        (format!("--example={}", example.name()), "run")
+        if package.starts_with("src/bin") {
+            (format!("--bin={}", app.name()), "run")
+        } else if package.starts_with("tests") {
+            (format!("--test={}", app.name()), "test")
+        } else {
+            (format!("--example={}", app.name()), "run")
+        }
     };
 
-    let mut features = example.features().to_vec();
+    let mut features = app.features().to_vec();
     features.push(chip.to_string());
 
     let mut builder = CargoArgsBuilder::default()

xtask/src/main.rs

@@ -6,7 +6,7 @@ use std::{
 use anyhow::{bail, Result};
 use clap::{Args, Parser};
 use strum::IntoEnumIterator;
-use xtask::{Chip, Package, Version};
+use xtask::{cargo::CargoAction, Chip, Package, Version};
 
 // ----------------------------------------------------------------------------
 // Command-line Interface
@@ -20,7 +20,7 @@ enum Cli {
     /// Build the specified package with the given options.
     BuildPackage(BuildPackageArgs),
     /// Build all applicable tests or the specified test for a specified chip.
-    BuildTests(RunTestsArgs),
+    BuildTests(TestsArgs),
     /// Bump the version of the specified package(s).
     BumpVersion(BumpVersionArgs),
     /// Generate the eFuse fields source file from a CSV.
@@ -28,7 +28,7 @@ enum Cli {
     /// Run the given example for the specified chip.
     RunExample(RunExampleArgs),
     /// Run all applicable tests or the specified test for a specified chip.
-    RunTests(RunTestsArgs),
+    RunTests(TestsArgs),
 }
 
 #[derive(Debug, Args)]
@@ -105,7 +105,7 @@ struct RunExampleArgs {
 }
 
 #[derive(Debug, Args)]
-struct RunTestsArgs {
+struct TestsArgs {
     /// Which chip to run the tests for.
     #[arg(value_enum)]
     chip: Chip,
@@ -129,11 +129,11 @@ fn main() -> Result<()> {
         Cli::BuildDocumentation(args) => build_documentation(&workspace, args),
         Cli::BuildExamples(args) => build_examples(&workspace, args),
         Cli::BuildPackage(args) => build_package(&workspace, args),
-        Cli::BuildTests(args) => build_tests(&workspace, args),
+        Cli::BuildTests(args) => execute_tests(&workspace, args, CargoAction::Build),
         Cli::BumpVersion(args) => bump_version(&workspace, args),
         Cli::GenerateEfuseFields(args) => generate_efuse_src(&workspace, args),
         Cli::RunExample(args) => run_example(&workspace, args),
-        Cli::RunTests(args) => run_tests(&workspace, args),
+        Cli::RunTests(args) => execute_tests(&workspace, args, CargoAction::Run),
     }
 }
 
@@ -240,7 +240,15 @@ fn build_examples(workspace: &Path, mut args: BuildExamplesArgs) -> Result<()> {
         // Filter down the examples to only those for which the specified chip is supported:
         .filter(|example| example.supports_chip(args.chip))
         // Attempt to build each supported example, with all required features enabled:
-        .try_for_each(|example| xtask::build_example(&package_path, args.chip, target, example))
+        .try_for_each(|example| {
+            xtask::execute_app(
+                &package_path,
+                args.chip,
+                target,
+                example,
+                &CargoAction::Build,
+            )
+        })
 }
 
 fn build_package(workspace: &Path, args: BuildPackageArgs) -> Result<()> {
@@ -329,7 +337,13 @@ fn run_example(workspace: &Path, mut args: RunExampleArgs) -> Result<()> {
         });
 
     if let Some(example) = example {
-        xtask::run_example(&package_path, args.chip, target, &example)?;
+        xtask::execute_app(
+            &package_path,
+            args.chip,
+            target,
+            &example,
+            &CargoAction::Run,
+        )?;
     } else {
         log::error!("Example not found or unsupported for the given chip");
     }
@@ -337,7 +351,11 @@ fn run_example(workspace: &Path, mut args: RunExampleArgs) -> Result<()> {
     Ok(())
 }
 
-fn build_tests(workspace: &Path, args: RunTestsArgs) -> Result<(), anyhow::Error> {
+fn execute_tests(
+    workspace: &Path,
+    args: TestsArgs,
+    action: CargoAction,
+) -> Result<(), anyhow::Error> {
     // Absolute path of the package's root:
     let package_path = xtask::windows_safe_path(&workspace.join("hil-test"));
 
@@ -359,55 +377,14 @@ fn build_tests(workspace: &Path, args: RunTestsArgs) -> Result<(), anyhow::Error
             }
         });
         if let Some(test) = test {
-            xtask::build_example(&package_path, args.chip, target, &test)?;
+            xtask::execute_app(&package_path, args.chip, target, &test, &action)?
         } else {
             log::error!("Test not found or unsupported for the given chip");
         }
     } else {
         let mut failed_tests: Vec<String> = Vec::new();
         for test in supported_tests {
-            if xtask::build_example(&package_path, args.chip, target, test).is_err() {
+            if xtask::execute_app(&package_path, args.chip, target, test, &action).is_err() {
-                failed_tests.push(test.name());
-            }
-        }
-        if !failed_tests.is_empty() {
-            bail!("Failed tests: {:?}", failed_tests);
-        }
-    }
-
-    Ok(())
-}
-
-fn run_tests(workspace: &Path, args: RunTestsArgs) -> Result<(), anyhow::Error> {
-    // Absolute path of the package's root:
-    let package_path = xtask::windows_safe_path(&workspace.join("hil-test"));
-
-    // Determine the appropriate build target for the given package and chip:
-    let target = target_triple(&Package::HilTest, &args.chip)?;
-
-    // Load all examples and parse their metadata:
-    let tests = xtask::load_examples(&package_path.join("tests"))?;
-    let mut supported_tests = tests
-        .iter()
-        // Filter down the examples to only those for which the specified chip is supported:
-        .filter(|example| example.supports_chip(args.chip));
-    if let Some(test_name) = &args.test {
-        let test = supported_tests.find_map(|example| {
-            if &example.name() == test_name {
-                Some(example.clone())
-            } else {
-                None
-            }
-        });
-        if let Some(test) = test {
-            xtask::run_example(&package_path, args.chip, target, &test)?;
-        } else {
-            log::error!("Test not found or unsupported for the given chip");
-        }
-    } else {
-        let mut failed_tests: Vec<String> = Vec::new();
-        for test in supported_tests {
-            if xtask::run_example(&package_path, args.chip, target, test).is_err() {
                 failed_tests.push(test.name());
             }
         }