From 65f1f4c1b7f8de50c6f9462a39c98dcd3a73b4cc Mon Sep 17 00:00:00 2001 From: datdenkikniet Date: Sun, 16 Apr 2023 11:26:23 +0200 Subject: [PATCH] Also separate all results and data --- xtask/src/main.rs | 126 +------------------------- xtask/src/run/data.rs | 87 ++++++++++++++++++ xtask/src/run/mod.rs | 191 +++++++++++++++------------------------ xtask/src/run/results.rs | 122 +++++++++++++++++++++++++ 4 files changed, 283 insertions(+), 243 deletions(-) create mode 100644 xtask/src/run/data.rs create mode 100644 xtask/src/run/results.rs diff --git a/xtask/src/main.rs b/xtask/src/main.rs index 1712d712a2..30c3da0503 100644 --- a/xtask/src/main.rs +++ b/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::{ - error::Error, - ffi::OsString, - fs::File, - io::prelude::*, - path::{Path, PathBuf}, - process::ExitStatus, - str, -}; +use std::{path::Path, 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 { - 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::() - .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) - } - } -} diff --git a/xtask/src/run/data.rs b/xtask/src/run/data.rs new file mode 100644 index 0000000000..eacd72cbad --- /dev/null +++ b/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 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 {} diff --git a/xtask/src/run/mod.rs b/xtask/src/run/mod.rs index daa6256361..501849d734 100644 --- a/xtask/src/run/mod.rs +++ b/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)] -pub enum OutputMode { - PipedAndCollected, - Inherited, -} - -impl From 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), -} +use self::results::run_successful; 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 { + 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::() + .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 { +fn run_command(command: &CargoCommand, stderr_mode: OutputMode) -> anyhow::Result { 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) -> 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(()) - } -} diff --git a/xtask/src/run/results.rs b/xtask/src/run/results.rs new file mode 100644 index 0000000000..072cbcfd4d --- /dev/null +++ b/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) -> 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(()) + } +}