Add new host-test xtask subcommand (#4085)

* Add new `host-test` xtask subcommand

+ smaller consistency and fmt fixes

* reviews

* reviews

.github/workflows/ci.yml

@@ -188,16 +188,5 @@ jobs:
       # Check metadata generation for all packages:
       - run: cargo xtask update-metadata --check
 
-      # Run tests in esp-config
-      - run: cd esp-config && cargo test --features build,tui
-
-      # Run tests in esp-bootloader-esp-idf
-      - run: cd esp-bootloader-esp-idf && cargo test --features=std
-
-      # Run tests in esp-storage
-      - run: cd esp-storage && cargo test --features=emulation -- --test-threads=1
-      - run: cd esp-storage && cargo test --features=emulation,bytewise-read -- --test-threads=1
-
-      # Miri tests in esp-storage
-      - run: cd esp-storage && cargo +nightly miri test --features=emulation -- --test-threads=1
-      - run: cd esp-storage && cargo +nightly miri test --features=emulation,bytewise-read -- --test-threads=1
+      # Run host tests for all applicable packages: 
+      - run: cargo xtask host-tests

esp-config/src/generate/mod.rs

@@ -572,7 +572,7 @@ fn screaming_snake_case(name: &str) -> String {
 }
 
 #[cfg(test)]
-mod test {
+mod tests {
     use super::*;
     use crate::generate::{validator::Validator, value::Value};
 

esp-config/src/generate/value.rs

@@ -185,7 +185,7 @@ impl From<String> for Value {
 }
 
 #[cfg(test)]
-mod test {
+mod tests {
     use super::*;
 
     #[test]

esp-config/src/lib.rs

@@ -69,7 +69,7 @@ macro_rules! esp_config_int_parse {
 }
 
 #[cfg(test)]
-mod test {
+mod tests {
     // We can only test success in the const context
     const _: () = {
         core::assert!(esp_config_int_parse!(i64, "-77777") == -77777);

esp-storage/src/nor_flash.rs

@@ -189,7 +189,7 @@ impl MultiwriteNorFlash for FlashStorage {}
 
 // Run the tests with `--test-threads=1` - the emulation is not multithread safe
 #[cfg(test)]
-mod test {
+mod tests {
     use super::*;
 
     const WORD_SIZE: u32 = 4;

xtask/README.md

@@ -18,6 +18,7 @@ Commands:
   semver-check               Semver Checks
   check-changelog            Check the changelog for packages
   update-chip-support-table  Re-generate the chip support table in the esp-hal README
+  host-tests                 Run host tests for all the packages where they are present
   help                       Print this message or the help of the given subcommand(s)
 
 Options:

xtask/src/cargo.rs

@@ -107,7 +107,7 @@ fn get_cargo() -> String {
     cargo
 }
 
-#[derive(Debug, Default)]
+#[derive(Clone, Debug, Default)]
 pub struct CargoArgsBuilder {
     toolchain: Option<String>,
     subcommand: String,

xtask/src/changelog.rs

@@ -342,7 +342,7 @@ fn parse_tag_link(line: &str) -> Result<(&str, &str)> {
 }
 
 #[cfg(test)]
-mod test {
+mod tests {
     use super::*;
 
     #[test]

xtask/src/commands/release/bump_version.rs

@@ -345,7 +345,7 @@ fn finalize_placeholders(
 }
 
 #[cfg(test)]
-mod test {
+mod tests {
     use toml_edit::DocumentMut;
 
     use super::*;

xtask/src/commands/release/execute_plan.rs

@@ -102,9 +102,19 @@ pub fn execute_plan(workspace: &Path, args: ApplyPlanArgs) -> Result<()> {
         );
     }
 
-    let branch = make_git_changes(!args.no_dry_run, "release-branch", "Finalize crate releases")?;
+    let branch = make_git_changes(
+        !args.no_dry_run,
+        "release-branch",
+        "Finalize crate releases",
+    )?;
 
-    open_pull_request(&branch, !args.no_dry_run, args.manual_pull_request, &plan_source, &plan)
+    open_pull_request(
+        &branch,
+        !args.no_dry_run,
+        args.manual_pull_request,
+        &plan_source,
+        &plan,
+    )
     .with_context(|| "Failed to open pull request")?;
 
     if !args.no_dry_run {
@@ -151,7 +161,11 @@ pub(crate) fn make_git_changes(dry_run: bool, branch_name: &str, commit: &str) -
     if dry_run {
         println!("Dry run: would commit changes to branch: {branch_name}");
     } else {
-        Command::new("git").arg("add").arg(".").status().context("Failed to stage changes")?;
+        Command::new("git")
+            .arg("add")
+            .arg(".")
+            .status()
+            .context("Failed to stage changes")?;
         Command::new("git")
             .arg("commit")
             .arg("-m")
@@ -306,7 +320,7 @@ pub(crate) fn comparison_url(base: &str, url: &str, branch_name: &str) -> Result
 }
 
 #[cfg(test)]
-mod test {
+mod tests {
     use super::*;
 
     #[test]

xtask/src/commands/release/plan.rs

@@ -335,7 +335,7 @@ fn topological_sort(dep_graph: &HashMap<Package, Vec<Package>>) -> Vec<Package>
 }
 
 #[cfg(test)]
-mod test {
+mod tests {
     use super::*;
 
     #[test]
@@ -343,7 +343,10 @@ mod test {
     fn test_topological_sort() {
         let mut dep_graph = HashMap::new();
         dep_graph.insert(Package::EspHal, vec![Package::EspAlloc]);
-        dep_graph.insert(Package::EspHalEmbassy, vec![Package::EspHal, Package::EspRadio]);
+        dep_graph.insert(
+            Package::EspHalEmbassy,
+            vec![Package::EspHal, Package::EspRadio],
+        );
         dep_graph.insert(Package::EspRadio, vec![Package::EspHal]);
         dep_graph.insert(Package::EspAlloc, vec![]);
 

xtask/src/commands/release/post_release.rs

@@ -1,11 +1,11 @@
 use std::fs;
 
-use anyhow::{Context, Result};
-use semver::Version;
-use super::execute_plan::make_git_changes;
 use super::PLACEHOLDER;
 use super::Plan;
+use super::execute_plan::make_git_changes;
 use crate::commands::comparison_url;
+use anyhow::{Context, Result};
+use semver::Version;
 
 pub fn post_release(workspace: &std::path::Path) -> Result<()> {
     // Read the release plan

xtask/src/commands/release/publish_plan.rs

@@ -1,6 +1,6 @@
 use std::{path::Path, process::Command};
 
-use anyhow::{bail, ensure, Context, Result};
+use anyhow::{Context, Result, bail, ensure};
 use clap::Args;
 
 use crate::{

xtask/src/lib.rs

@@ -135,6 +135,18 @@ impl Package {
         false
     }
 
+    pub fn has_host_tests(&self, workspace: &Path) -> bool {
+        let package_path = workspace.join(self.to_string()).join("src");
+
+        walkdir::WalkDir::new(package_path)
+            .into_iter()
+            .filter_map(Result::ok)
+            .filter(|e| e.path().extension().is_some_and(|ext| ext == "rs"))
+            .any(|entry| {
+                std::fs::read_to_string(entry.path()).map_or(false, |src| src.contains("#[test]"))
+            })
+    }
+
     /// Does the package need to be built with the standard library?
     pub fn needs_build_std(&self) -> bool {
         use Package::*;
@@ -572,6 +584,87 @@ pub fn format_package(workspace: &Path, package: Package, check: bool) -> Result
     Ok(())
 }
 
+pub fn run_host_tests(workspace: &Path, package: Package) -> Result<()> {
+    log::info!("Running host tests for package: {}", package);
+    let package_path = workspace.join(package.as_ref());
+
+    let cmd = CargoArgsBuilder::default();
+
+    match package {
+        Package::EspConfig => {
+            return cargo::run(
+                &cmd.clone()
+                    .subcommand("test")
+                    .features(&vec!["build".into(), "tui".into()])
+                    .build(),
+                &package_path,
+            );
+        }
+
+        Package::EspBootloaderEspIdf => {
+            return cargo::run(
+                &cmd.clone()
+                    .subcommand("test")
+                    .features(&vec!["std".into()])
+                    .build(),
+                &package_path,
+            );
+        }
+
+        Package::EspStorage => {
+            cargo::run(
+                &cmd.clone()
+                    .subcommand("test")
+                    .features(&vec!["emulation".into()])
+                    .arg("--")
+                    .arg("--test-threads=1")
+                    .build(),
+                &package_path,
+            )?;
+
+            cargo::run(
+                &cmd.clone()
+                    .subcommand("test")
+                    .features(&vec!["emulation".into(), "bytewise-read".into()])
+                    .arg("--")
+                    .arg("--test-threads=1")
+                    .build(),
+                &package_path,
+            )?;
+
+            log::info!("Running miri host tests for package: {}", package);
+
+            cargo::run(
+                &cmd.clone()
+                    .toolchain("nightly")
+                    .subcommand("miri")
+                    .subcommand("test")
+                    .features(&vec!["emulation".into()])
+                    .arg("--")
+                    .arg("--test-threads=1")
+                    .build(),
+                &package_path,
+            )?;
+
+            return cargo::run(
+                &cmd.clone()
+                    .toolchain("nightly")
+                    .subcommand("miri")
+                    .subcommand("test")
+                    .features(&vec!["emulation".into(), "bytewise-read".into()])
+                    .arg("--")
+                    .arg("--test-threads=1")
+                    .build(),
+                &package_path,
+            );
+        }
+        _ => Err(anyhow!(
+            "Instructions for host testing were not provided for: '{}'",
+            package,
+        )),
+    }
+}
+
 fn format_package_path(workspace: &Path, package_path: &Path, check: bool) -> Result<()> {
     // We need to list all source files since modules in `unstable_module!` macros
     // won't get picked up otherwise

xtask/src/main.rs

@@ -45,6 +45,8 @@ enum Cli {
     CheckChangelog(CheckChangelogArgs),
     /// Re-generate metadata and the chip support table in the esp-hal README.
     UpdateMetadata(UpdateMetadataArgs),
+    /// Run host-tests in the workspace with `cargo test`
+    HostTests(HostTestsArgs),
 }
 
 #[derive(Debug, Args)]
@@ -76,6 +78,13 @@ struct CleanArgs {
     packages: Vec<Package>,
 }
 
+#[derive(Debug, Args)]
+struct HostTestsArgs {
+    /// Package(s) to target.
+    #[arg(value_enum, default_values_t = Package::iter())]
+    packages: Vec<Package>,
+}
+
 #[derive(Debug, Args)]
 struct LintPackagesArgs {
     /// Package(s) to target.
@@ -176,6 +185,7 @@ fn main() -> Result<()> {
         Cli::SemverCheck(args) => semver_checks(&workspace, args),
         Cli::CheckChangelog(args) => check_changelog(&workspace, &args.packages, args.normalize),
         Cli::UpdateMetadata(args) => update_metadata(&workspace, args.check),
+        Cli::HostTests(args) => host_tests(&workspace, args),
     }
 }
 
@@ -513,3 +523,16 @@ fn run_ci_checks(workspace: &Path, args: CiArgs) -> Result<()> {
 
     runner.finish()
 }
+
+fn host_tests(workspace: &Path, args: HostTestsArgs) -> Result<()> {
+    let mut packages = args.packages;
+    packages.sort();
+
+    for package in packages {
+        if package.has_host_tests(workspace) {
+            xtask::run_host_tests(workspace, package)?;
+        }
+    }
+
+    Ok(())
+}