Also separate all results and data

xtask/src/main.rs

@@ -3,26 +3,16 @@ mod build;
 mod cargo_command;
 mod run;
 
-use argument_parsing::{ExtraArguments, Globals};
+use argument_parsing::ExtraArguments;
 use clap::Parser;
 use core::fmt;
-use diffy::{create_patch, PatchFormatter};
+use std::{path::Path, str};
-use std::{
-    error::Error,
-    ffi::OsString,
-    fs::File,
-    io::prelude::*,
-    path::{Path, PathBuf},
-    process::ExitStatus,
-    str,
-};
 
 use log::{error, info, log_enabled, trace, Level};
 
 use crate::{
     argument_parsing::{Backends, BuildOrCheck, Cli, Commands},
     build::init_build_dir,
-    cargo_command::CargoCommand,
     run::*,
 };
 
@@ -65,56 +55,6 @@ const ARMV7M: Target = Target::new("thumbv7m-none-eabi", false);
 const ARMV8MBASE: Target = Target::new("thumbv8m.base-none-eabi", false);
 const ARMV8MMAIN: Target = Target::new("thumbv8m.main-none-eabi", false);
 
-#[derive(Debug, Clone)]
-pub struct RunResult {
-    exit_status: ExitStatus,
-    stdout: String,
-    stderr: String,
-}
-
-#[derive(Debug)]
-pub enum TestRunError {
-    FileCmpError { expected: String, got: String },
-    FileError { file: String },
-    PathConversionError(OsString),
-    CommandError(RunResult),
-    IncompatibleCommand,
-}
-impl fmt::Display for TestRunError {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        match self {
-            TestRunError::FileCmpError { expected, got } => {
-                let patch = create_patch(expected, got);
-                writeln!(f, "Differing output in files.\n")?;
-                let pf = PatchFormatter::new().with_color();
-                writeln!(f, "{}", pf.fmt_patch(&patch))?;
-                write!(
-                    f,
-                    "See flag --overwrite-expected to create/update expected output."
-                )
-            }
-            TestRunError::FileError { file } => {
-                write!(f, "File error on: {file}\nSee flag --overwrite-expected to create/update expected output.")
-            }
-            TestRunError::CommandError(e) => {
-                write!(
-                    f,
-                    "Command failed with exit status {}: {} {}",
-                    e.exit_status, e.stdout, e.stderr
-                )
-            }
-            TestRunError::PathConversionError(p) => {
-                write!(f, "Can't convert path from `OsString` to `String`: {p:?}")
-            }
-            TestRunError::IncompatibleCommand => {
-                write!(f, "Can't run that command in this context")
-            }
-        }
-    }
-}
-
-impl Error for TestRunError {}
-
 fn main() -> anyhow::Result<()> {
     // if there's an `xtask` folder, we're *probably* at the root of this repo (we can't just
     // check the name of `env::current_dir()` because people might clone it into a different name)
@@ -299,65 +239,3 @@ fn main() -> anyhow::Result<()> {
 
     handle_results(globals, final_run_results).map_err(|_| anyhow::anyhow!("Commands failed"))
 }
-
-// run example binary `example`
-fn command_parser(
-    glob: &Globals,
-    command: &CargoCommand,
-    overwrite: bool,
-) -> anyhow::Result<RunResult> {
-    let output_mode = if glob.stderr_inherited {
-        OutputMode::Inherited
-    } else {
-        OutputMode::PipedAndCollected
-    };
-
-    match *command {
-        CargoCommand::Qemu { example, .. } | CargoCommand::Run { example, .. } => {
-            let run_file = format!("{example}.run");
-            let expected_output_file = ["rtic", "ci", "expected", &run_file]
-                .iter()
-                .collect::<PathBuf>()
-                .into_os_string()
-                .into_string()
-                .map_err(TestRunError::PathConversionError)?;
-
-            // cargo run <..>
-            let cargo_run_result = run_command(command, output_mode)?;
-
-            // Create a file for the expected output if it does not exist or mismatches
-            if overwrite {
-                let result = run_successful(&cargo_run_result, &expected_output_file);
-                if let Err(e) = result {
-                    // FileError means the file did not exist or was unreadable
-                    error!("Error: {e}");
-                    let mut file_handle = File::create(&expected_output_file).map_err(|_| {
-                        TestRunError::FileError {
-                            file: expected_output_file.clone(),
-                        }
-                    })?;
-                    info!("Flag --overwrite-expected enabled");
-                    info!("Creating/updating file: {expected_output_file}");
-                    file_handle.write_all(cargo_run_result.stdout.as_bytes())?;
-                };
-            } else {
-                run_successful(&cargo_run_result, &expected_output_file)?;
-            };
-
-            Ok(cargo_run_result)
-        }
-        CargoCommand::Format { .. }
-        | CargoCommand::ExampleCheck { .. }
-        | CargoCommand::ExampleBuild { .. }
-        | CargoCommand::Check { .. }
-        | CargoCommand::Build { .. }
-        | CargoCommand::Clippy { .. }
-        | CargoCommand::Doc { .. }
-        | CargoCommand::Test { .. }
-        | CargoCommand::Book { .. }
-        | CargoCommand::ExampleSize { .. } => {
-            let cargo_result = run_command(command, output_mode)?;
-            Ok(cargo_result)
-        }
-    }
-}

xtask/src/run/data.rs

@@ -0,0 +1,87 @@
+use std::{
+    ffi::OsString,
+    process::{ExitStatus, Stdio},
+};
+
+use diffy::{create_patch, PatchFormatter};
+
+use crate::cargo_command::CargoCommand;
+
+#[derive(Debug, Clone, Copy, PartialEq)]
+pub enum OutputMode {
+    PipedAndCollected,
+    Inherited,
+}
+
+impl From<OutputMode> for Stdio {
+    fn from(value: OutputMode) -> Self {
+        match value {
+            OutputMode::PipedAndCollected => Stdio::piped(),
+            OutputMode::Inherited => Stdio::inherit(),
+        }
+    }
+}
+
+#[derive(Debug, Clone)]
+pub struct RunResult {
+    pub exit_status: ExitStatus,
+    pub stdout: String,
+    pub stderr: String,
+}
+
+#[derive(Debug)]
+pub enum FinalRunResult<'c> {
+    Success(CargoCommand<'c>, RunResult),
+    Failed(CargoCommand<'c>, RunResult),
+    CommandError(CargoCommand<'c>, anyhow::Error),
+}
+
+#[derive(Debug)]
+pub enum TestRunError {
+    FileCmpError {
+        expected: String,
+        got: String,
+    },
+    FileError {
+        file: String,
+    },
+    PathConversionError(OsString),
+    CommandError(RunResult),
+    #[allow(dead_code)]
+    IncompatibleCommand,
+}
+
+impl core::fmt::Display for TestRunError {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            TestRunError::FileCmpError { expected, got } => {
+                let patch = create_patch(expected, got);
+                writeln!(f, "Differing output in files.\n")?;
+                let pf = PatchFormatter::new().with_color();
+                writeln!(f, "{}", pf.fmt_patch(&patch))?;
+                write!(
+                    f,
+                    "See flag --overwrite-expected to create/update expected output."
+                )
+            }
+            TestRunError::FileError { file } => {
+                write!(f, "File error on: {file}\nSee flag --overwrite-expected to create/update expected output.")
+            }
+            TestRunError::CommandError(e) => {
+                write!(
+                    f,
+                    "Command failed with exit status {}: {} {}",
+                    e.exit_status, e.stdout, e.stderr
+                )
+            }
+            TestRunError::PathConversionError(p) => {
+                write!(f, "Can't convert path from `OsString` to `String`: {p:?}")
+            }
+            TestRunError::IncompatibleCommand => {
+                write!(f, "Can't run that command in this context")
+            }
+        }
+    }
+}
+
+impl std::error::Error for TestRunError {}

xtask/src/run/mod.rs

@@ -1,45 +1,30 @@
 use std::{
     fs::File,
-    io::Read,
+    io::Write,
     path::PathBuf,
     process::{Command, Stdio},
 };
 
+mod results;
+pub use results::handle_results;
+
+mod data;
+use data::*;
+
 mod iter;
 use iter::{into_iter, CoalescingRunner};
 
 use crate::{
     argument_parsing::{Backends, BuildOrCheck, ExtraArguments, Globals, PackageOpt, TestMetadata},
     cargo_command::{BuildMode, CargoCommand},
-    command_parser, RunResult, TestRunError,
 };
 
-use log::{error, info, Level};
+use log::{error, info};
 
 #[cfg(feature = "rayon")]
 use rayon::prelude::*;
 
-#[derive(Debug, Clone, Copy, PartialEq)]
+use self::results::run_successful;
-pub enum OutputMode {
-    PipedAndCollected,
-    Inherited,
-}
-
-impl From<OutputMode> for Stdio {
-    fn from(value: OutputMode) -> Self {
-        match value {
-            OutputMode::PipedAndCollected => Stdio::piped(),
-            OutputMode::Inherited => Stdio::inherit(),
-        }
-    }
-}
-
-#[derive(Debug)]
-pub enum FinalRunResult<'c> {
-    Success(CargoCommand<'c>, RunResult),
-    Failed(CargoCommand<'c>, RunResult),
-    CommandError(CargoCommand<'c>, anyhow::Error),
-}
 
 fn run_and_convert<'a>(
     (global, command, overwrite): (&Globals, CargoCommand<'a>, bool),
@@ -66,6 +51,68 @@ fn run_and_convert<'a>(
     output
 }
 
+// run example binary `example`
+fn command_parser(
+    glob: &Globals,
+    command: &CargoCommand,
+    overwrite: bool,
+) -> anyhow::Result<RunResult> {
+    let output_mode = if glob.stderr_inherited {
+        OutputMode::Inherited
+    } else {
+        OutputMode::PipedAndCollected
+    };
+
+    match *command {
+        CargoCommand::Qemu { example, .. } | CargoCommand::Run { example, .. } => {
+            let run_file = format!("{example}.run");
+            let expected_output_file = ["rtic", "ci", "expected", &run_file]
+                .iter()
+                .collect::<PathBuf>()
+                .into_os_string()
+                .into_string()
+                .map_err(TestRunError::PathConversionError)?;
+
+            // cargo run <..>
+            let cargo_run_result = run_command(command, output_mode)?;
+
+            // Create a file for the expected output if it does not exist or mismatches
+            if overwrite {
+                let result = run_successful(&cargo_run_result, &expected_output_file);
+                if let Err(e) = result {
+                    // FileError means the file did not exist or was unreadable
+                    error!("Error: {e}");
+                    let mut file_handle = File::create(&expected_output_file).map_err(|_| {
+                        TestRunError::FileError {
+                            file: expected_output_file.clone(),
+                        }
+                    })?;
+                    info!("Flag --overwrite-expected enabled");
+                    info!("Creating/updating file: {expected_output_file}");
+                    file_handle.write_all(cargo_run_result.stdout.as_bytes())?;
+                };
+            } else {
+                run_successful(&cargo_run_result, &expected_output_file)?;
+            };
+
+            Ok(cargo_run_result)
+        }
+        CargoCommand::Format { .. }
+        | CargoCommand::ExampleCheck { .. }
+        | CargoCommand::ExampleBuild { .. }
+        | CargoCommand::Check { .. }
+        | CargoCommand::Build { .. }
+        | CargoCommand::Clippy { .. }
+        | CargoCommand::Doc { .. }
+        | CargoCommand::Test { .. }
+        | CargoCommand::Book { .. }
+        | CargoCommand::ExampleSize { .. } => {
+            let cargo_result = run_command(command, output_mode)?;
+            Ok(cargo_result)
+        }
+    }
+}
+
 /// Cargo command to either build or check
 pub fn cargo<'c>(
     globals: &Globals,
@@ -355,7 +402,7 @@ pub fn build_and_check_size<'c>(
     runner.run_and_coalesce()
 }
 
-pub fn run_command(command: &CargoCommand, stderr_mode: OutputMode) -> anyhow::Result<RunResult> {
+fn run_command(command: &CargoCommand, stderr_mode: OutputMode) -> anyhow::Result<RunResult> {
     log::info!("👟 {command}");
 
     let mut process = Command::new(command.executable());
@@ -391,97 +438,3 @@ pub fn run_command(command: &CargoCommand, stderr_mode: OutputMode) -> anyhow::R
         stderr,
     })
 }
-
-/// Check if `run` was successful.
-/// returns Ok in case the run went as expected,
-/// Err otherwise
-pub fn run_successful(run: &RunResult, expected_output_file: &str) -> Result<(), TestRunError> {
-    let mut file_handle =
-        File::open(expected_output_file).map_err(|_| TestRunError::FileError {
-            file: expected_output_file.to_owned(),
-        })?;
-    let mut expected_output = String::new();
-    file_handle
-        .read_to_string(&mut expected_output)
-        .map_err(|_| TestRunError::FileError {
-            file: expected_output_file.to_owned(),
-        })?;
-
-    if expected_output != run.stdout {
-        Err(TestRunError::FileCmpError {
-            expected: expected_output.clone(),
-            got: run.stdout.clone(),
-        })
-    } else if !run.exit_status.success() {
-        Err(TestRunError::CommandError(run.clone()))
-    } else {
-        Ok(())
-    }
-}
-
-pub fn handle_results(globals: &Globals, results: Vec<FinalRunResult>) -> Result<(), ()> {
-    let errors = results.iter().filter_map(|r| {
-        if let FinalRunResult::Failed(c, r) = r {
-            Some((c, &r.stdout, &r.stderr))
-        } else {
-            None
-        }
-    });
-
-    let successes = results.iter().filter_map(|r| {
-        if let FinalRunResult::Success(c, r) = r {
-            Some((c, &r.stdout, &r.stderr))
-        } else {
-            None
-        }
-    });
-
-    let command_errors = results.iter().filter_map(|r| {
-        if let FinalRunResult::CommandError(c, e) = r {
-            Some((c, e))
-        } else {
-            None
-        }
-    });
-
-    let log_stdout_stderr = |level: Level| {
-        move |(cmd, stdout, stderr): (&CargoCommand, &String, &String)| {
-            let cmd = cmd.as_cmd_string();
-            if !stdout.is_empty() && !stderr.is_empty() {
-                log::log!(level, "\n{cmd}\nStdout:\n{stdout}\nStderr:\n{stderr}");
-            } else if !stdout.is_empty() {
-                log::log!(level, "\n{cmd}\nStdout:\n{}", stdout.trim_end());
-            } else if !stderr.is_empty() {
-                log::log!(level, "\n{cmd}\nStderr:\n{}", stderr.trim_end());
-            }
-        }
-    };
-
-    successes.for_each(|(cmd, stdout, stderr)| {
-        if globals.verbose > 0 {
-            info!("✅ Success: {cmd}\n    {}", cmd.as_cmd_string());
-        } else {
-            info!("✅ Success: {cmd}");
-        }
-
-        log_stdout_stderr(Level::Debug)((cmd, stdout, stderr));
-    });
-
-    errors.clone().for_each(|(cmd, stdout, stderr)| {
-        error!("❌ Failed: {cmd}\n    {}", cmd.as_cmd_string());
-        log_stdout_stderr(Level::Error)((cmd, stdout, stderr));
-    });
-
-    command_errors
-        .clone()
-        .for_each(|(cmd, error)| error!("❌ Failed: {cmd}\n    {}\n{error}", cmd.as_cmd_string()));
-
-    let ecount = errors.count() + command_errors.count();
-    if ecount != 0 {
-        log::error!("{ecount} commands failed.");
-        Err(())
-    } else {
-        info!("🚀🚀🚀 All tasks succeeded 🚀🚀🚀");
-        Ok(())
-    }
-}

xtask/src/run/results.rs

@@ -0,0 +1,122 @@
+use log::{error, info, log, Level};
+
+use crate::{argument_parsing::Globals, cargo_command::CargoCommand};
+
+use super::data::{FinalRunResult, RunResult, TestRunError};
+
+const TARGET: &str = "xtask::results";
+
+/// Check if `run` was successful.
+/// returns Ok in case the run went as expected,
+/// Err otherwise
+pub fn run_successful(run: &RunResult, expected_output_file: &str) -> Result<(), TestRunError> {
+    let file = expected_output_file.to_string();
+
+    let expected_output = std::fs::read(expected_output_file)
+        .map(|d| String::from_utf8(d).map_err(|_| TestRunError::FileError { file: file.clone() }))
+        .map_err(|_| TestRunError::FileError { file })??;
+
+    if expected_output != run.stdout {
+        Err(TestRunError::FileCmpError {
+            expected: expected_output.clone(),
+            got: run.stdout.clone(),
+        })
+    } else if !run.exit_status.success() {
+        Err(TestRunError::CommandError(run.clone()))
+    } else {
+        Ok(())
+    }
+}
+
+pub fn handle_results(globals: &Globals, results: Vec<FinalRunResult>) -> Result<(), ()> {
+    let errors = results.iter().filter_map(|r| {
+        if let FinalRunResult::Failed(c, r) = r {
+            Some((c, &r.stdout, &r.stderr))
+        } else {
+            None
+        }
+    });
+
+    let successes = results.iter().filter_map(|r| {
+        if let FinalRunResult::Success(c, r) = r {
+            Some((c, &r.stdout, &r.stderr))
+        } else {
+            None
+        }
+    });
+
+    let command_errors = results.iter().filter_map(|r| {
+        if let FinalRunResult::CommandError(c, e) = r {
+            Some((c, e))
+        } else {
+            None
+        }
+    });
+
+    let log_stdout_stderr = |level: Level| {
+        move |(cmd, stdout, stderr): (&CargoCommand, &String, &String)| {
+            let cmd = cmd.as_cmd_string();
+            if !stdout.is_empty() && !stderr.is_empty() {
+                log!(
+                    target: TARGET,
+                    level,
+                    "\n{cmd}\nStdout:\n{stdout}\nStderr:\n{stderr}"
+                );
+            } else if !stdout.is_empty() {
+                log!(
+                    target: TARGET,
+                    level,
+                    "\n{cmd}\nStdout:\n{}",
+                    stdout.trim_end()
+                );
+            } else if !stderr.is_empty() {
+                log!(
+                    target: TARGET,
+                    level,
+                    "\n{cmd}\nStderr:\n{}",
+                    stderr.trim_end()
+                );
+            }
+        }
+    };
+
+    successes.for_each(|(cmd, stdout, stderr)| {
+        if globals.verbose > 0 {
+            info!(
+                target: TARGET,
+                "✅ Success: {cmd}\n    {}",
+                cmd.as_cmd_string()
+            );
+        } else {
+            info!(target: TARGET, "✅ Success: {cmd}");
+        }
+
+        log_stdout_stderr(Level::Debug)((cmd, stdout, stderr));
+    });
+
+    errors.clone().for_each(|(cmd, stdout, stderr)| {
+        error!(
+            target: TARGET,
+            "❌ Failed: {cmd}\n    {}",
+            cmd.as_cmd_string()
+        );
+        log_stdout_stderr(Level::Error)((cmd, stdout, stderr));
+    });
+
+    command_errors.clone().for_each(|(cmd, error)| {
+        error!(
+            target: TARGET,
+            "❌ Failed: {cmd}\n    {}\n{error}",
+            cmd.as_cmd_string()
+        )
+    });
+
+    let ecount = errors.count() + command_errors.count();
+    if ecount != 0 {
+        error!(target: TARGET, "{ecount} commands failed.");
+        Err(())
+    } else {
+        info!(target: TARGET, "🚀🚀🚀 All tasks succeeded 🚀🚀🚀");
+        Ok(())
+    }
+}