From 611f9d8722593430d82187aebee9db5cc6952da1 Mon Sep 17 00:00:00 2001 From: mo8it Date: Sat, 1 Jun 2024 21:48:15 +0200 Subject: [PATCH] Check that all solutions run successfully --- src/app_state.rs | 41 ++++++++------- src/cmd.rs | 4 +- src/dev/check.rs | 57 ++++++++++++++------ src/exercise.rs | 126 +++++++++++++++++++++++++++++---------------- src/info_file.rs | 19 ++++++- src/run.rs | 4 +- src/watch/state.rs | 4 +- 7 files changed, 169 insertions(+), 86 deletions(-) diff --git a/src/app_state.rs b/src/app_state.rs index c7c090f6..e9a5b109 100644 --- a/src/app_state.rs +++ b/src/app_state.rs @@ -11,7 +11,7 @@ use std::{ use crate::{ clear_terminal, embedded::EMBEDDED_FILES, - exercise::{Exercise, OUTPUT_CAPACITY}, + exercise::{Exercise, RunnableExercise, OUTPUT_CAPACITY}, info_file::ExerciseInfo, DEBUG_PROFILE, }; @@ -40,6 +40,25 @@ struct CargoMetadata { target_directory: PathBuf, } +pub fn parse_target_dir() -> Result { + // Get the target directory from Cargo. + let metadata_output = Command::new("cargo") + .arg("metadata") + .arg("-q") + .arg("--format-version") + .arg("1") + .arg("--no-deps") + .stdin(Stdio::null()) + .stderr(Stdio::inherit()) + .output() + .context(CARGO_METADATA_ERR)? + .stdout; + + serde_json::de::from_slice::(&metadata_output) + .context("Failed to read the field `target_directory` from the `cargo metadata` output") + .map(|metadata| metadata.target_directory) +} + pub struct AppState { current_exercise_ind: usize, exercises: Vec, @@ -104,23 +123,7 @@ impl AppState { exercise_infos: Vec, final_message: String, ) -> Result<(Self, StateFileStatus)> { - // Get the target directory from Cargo. - let metadata_output = Command::new("cargo") - .arg("metadata") - .arg("-q") - .arg("--format-version") - .arg("1") - .arg("--no-deps") - .stdin(Stdio::null()) - .stderr(Stdio::inherit()) - .output() - .context(CARGO_METADATA_ERR)? - .stdout; - let target_dir = serde_json::de::from_slice::(&metadata_output) - .context( - "Failed to read the field `target_directory` from the `cargo metadata` output", - )? - .target_directory; + let target_dir = parse_target_dir()?; let exercises = exercise_infos .into_iter() @@ -381,7 +384,7 @@ impl AppState { write!(writer, "Running {exercise} ... ")?; writer.flush()?; - let success = exercise.run(&mut output, &self.target_dir)?; + let success = exercise.run_exercise(&mut output, &self.target_dir)?; if !success { writeln!(writer, "{}\n", "FAILED".red())?; diff --git a/src/cmd.rs b/src/cmd.rs index b914ed88..6092f531 100644 --- a/src/cmd.rs +++ b/src/cmd.rs @@ -35,7 +35,7 @@ pub fn run_cmd(mut cmd: Command, description: &str, output: &mut Vec) -> Res pub struct CargoCmd<'a> { pub subcommand: &'a str, pub args: &'a [&'a str], - pub exercise_name: &'a str, + pub bin_name: &'a str, pub description: &'a str, /// RUSTFLAGS="-A warnings" pub hide_warnings: bool, @@ -65,7 +65,7 @@ impl<'a> CargoCmd<'a> { .arg("always") .arg("-q") .arg("--bin") - .arg(self.exercise_name) + .arg(self.bin_name) .args(self.args); if self.hide_warnings { diff --git a/src/dev/check.rs b/src/dev/check.rs index 59352e96..6a3597cc 100644 --- a/src/dev/check.rs +++ b/src/dev/check.rs @@ -2,12 +2,14 @@ use anyhow::{anyhow, bail, Context, Error, Result}; use std::{ cmp::Ordering, fs::{self, read_dir, OpenOptions}, - io::Read, + io::{self, Read, Write}, path::{Path, PathBuf}, }; use crate::{ + app_state::parse_target_dir, cargo_toml::{append_bins, bins_start_end_ind, BINS_BUFFER_CAPACITY}, + exercise::{RunnableExercise, OUTPUT_CAPACITY}, info_file::{ExerciseInfo, InfoFile}, CURRENT_FORMAT_VERSION, DEBUG_PROFILE, }; @@ -17,6 +19,29 @@ fn forbidden_char(input: &str) -> Option { input.chars().find(|c| !c.is_alphanumeric() && *c != '_') } +// Check that the Cargo.toml file is up-to-date. +fn check_cargo_toml( + exercise_infos: &[ExerciseInfo], + current_cargo_toml: &str, + exercise_path_prefix: &[u8], +) -> Result<()> { + let (bins_start_ind, bins_end_ind) = bins_start_end_ind(current_cargo_toml)?; + + let old_bins = ¤t_cargo_toml.as_bytes()[bins_start_ind..bins_end_ind]; + let mut new_bins = Vec::with_capacity(BINS_BUFFER_CAPACITY); + append_bins(&mut new_bins, exercise_infos, exercise_path_prefix); + + if old_bins != new_bins { + if DEBUG_PROFILE { + bail!("The file `dev/Cargo.toml` is outdated. Please run `cargo run -- dev update` to update it"); + } + + bail!("The file `Cargo.toml` is outdated. Please run `rustlings dev update` to update it"); + } + + Ok(()) +} + // Check the info of all exercises and return their paths in a set. fn check_info_file_exercises(info_file: &InfoFile) -> Result> { let mut names = hashbrown::HashSet::with_capacity(info_file.exercises.len()); @@ -136,24 +161,20 @@ fn check_exercises(info_file: &InfoFile) -> Result<()> { Ok(()) } -// Check that the Cargo.toml file is up-to-date. -fn check_cargo_toml( - exercise_infos: &[ExerciseInfo], - current_cargo_toml: &str, - exercise_path_prefix: &[u8], -) -> Result<()> { - let (bins_start_ind, bins_end_ind) = bins_start_end_ind(current_cargo_toml)?; +fn check_solutions(info_file: &InfoFile) -> Result<()> { + let target_dir = parse_target_dir()?; + let mut output = Vec::with_capacity(OUTPUT_CAPACITY); - let old_bins = ¤t_cargo_toml.as_bytes()[bins_start_ind..bins_end_ind]; - let mut new_bins = Vec::with_capacity(BINS_BUFFER_CAPACITY); - append_bins(&mut new_bins, exercise_infos, exercise_path_prefix); + for exercise_info in &info_file.exercises { + let success = exercise_info.run_solution(&mut output, &target_dir)?; + if !success { + io::stderr().write_all(&output)?; - if old_bins != new_bins { - if DEBUG_PROFILE { - bail!("The file `dev/Cargo.toml` is outdated. Please run `cargo run -- dev update` to update it"); + bail!( + "Failed to run the solution of the exercise {}", + exercise_info.name, + ); } - - bail!("The file `Cargo.toml` is outdated. Please run `rustlings dev update` to update it"); } Ok(()) @@ -161,7 +182,6 @@ fn check_cargo_toml( pub fn check() -> Result<()> { let info_file = InfoFile::parse()?; - check_exercises(&info_file)?; // A hack to make `cargo run -- dev check` work when developing Rustlings. if DEBUG_PROFILE { @@ -176,6 +196,9 @@ pub fn check() -> Result<()> { check_cargo_toml(&info_file.exercises, ¤t_cargo_toml, b"")?; } + check_exercises(&info_file)?; + check_solutions(&info_file)?; + println!("\nEverything looks fine!"); Ok(()) diff --git a/src/exercise.rs b/src/exercise.rs index 4bc37cd7..b6adc141 100644 --- a/src/exercise.rs +++ b/src/exercise.rs @@ -17,6 +17,35 @@ use crate::{ /// The initial capacity of the output buffer. pub const OUTPUT_CAPACITY: usize = 1 << 14; +// Run an exercise binary and append its output to the `output` buffer. +// Compilation must be done before calling this method. +fn run_bin(bin_name: &str, output: &mut Vec, target_dir: &Path) -> Result { + writeln!(output, "{}", "Output".underlined())?; + + // 7 = "/debug/".len() + let mut bin_path = PathBuf::with_capacity(target_dir.as_os_str().len() + 7 + bin_name.len()); + bin_path.push(target_dir); + bin_path.push("debug"); + bin_path.push(bin_name); + + let success = run_cmd(Command::new(&bin_path), &bin_path.to_string_lossy(), output)?; + + if !success { + // This output is important to show the user that something went wrong. + // Otherwise, calling something like `exit(1)` in an exercise without further output + // leaves the user confused about why the exercise isn't done yet. + writeln!( + output, + "{}", + "The exercise didn't run successfully (nonzero exit code)" + .bold() + .red(), + )?; + } + + Ok(success) +} + /// See `info_file::ExerciseInfo` pub struct Exercise { pub dir: Option<&'static str>, @@ -30,39 +59,25 @@ pub struct Exercise { } impl Exercise { - // Run the exercise's binary and append its output to the `output` buffer. - // Compilation should be done before calling this method. - fn run_bin(&self, output: &mut Vec, target_dir: &Path) -> Result { - writeln!(output, "{}", "Output".underlined())?; - - // 7 = "/debug/".len() - let mut bin_path = - PathBuf::with_capacity(target_dir.as_os_str().len() + 7 + self.name.len()); - bin_path.push(target_dir); - bin_path.push("debug"); - bin_path.push(self.name); - - let success = run_cmd(Command::new(&bin_path), &bin_path.to_string_lossy(), output)?; - - if !success { - // This output is important to show the user that something went wrong. - // Otherwise, calling something like `exit(1)` in an exercise without further output - // leaves the user confused about why the exercise isn't done yet. - writeln!( - output, - "{}", - "The exercise didn't run successfully (nonzero exit code)" - .bold() - .red(), - )?; - } - - Ok(success) + pub fn terminal_link(&self) -> StyledContent> { + style(TerminalFileLink(self.path)).underlined().blue() } +} - /// Compile, check and run the exercise. - /// The output is written to the `output` buffer after clearing it. - pub fn run(&self, output: &mut Vec, target_dir: &Path) -> Result { +impl Display for Exercise { + fn fmt(&self, f: &mut Formatter) -> fmt::Result { + self.path.fmt(f) + } +} + +pub trait RunnableExercise { + fn name(&self) -> &str; + fn strict_clippy(&self) -> bool; + fn test(&self) -> bool; + + // Compile, check and run the exercise or its solution (depending on `bin_name´). + // The output is written to the `output` buffer after clearing it. + fn run(&self, bin_name: &str, output: &mut Vec, target_dir: &Path) -> Result { output.clear(); // Developing the official Rustlings. @@ -71,7 +86,7 @@ impl Exercise { let build_success = CargoCmd { subcommand: "build", args: &[], - exercise_name: self.name, + bin_name, description: "cargo build …", hide_warnings: false, target_dir, @@ -87,7 +102,7 @@ impl Exercise { output.clear(); // `--profile test` is required to also check code with `[cfg(test)]`. - let clippy_args: &[&str] = if self.strict_clippy { + let clippy_args: &[&str] = if self.strict_clippy() { &["--profile", "test", "--", "-D", "warnings"] } else { &["--profile", "test"] @@ -95,7 +110,7 @@ impl Exercise { let clippy_success = CargoCmd { subcommand: "clippy", args: clippy_args, - exercise_name: self.name, + bin_name, description: "cargo clippy …", hide_warnings: false, target_dir, @@ -107,14 +122,14 @@ impl Exercise { return Ok(false); } - if !self.test { - return self.run_bin(output, target_dir); + if !self.test() { + return run_bin(bin_name, output, target_dir); } let test_success = CargoCmd { subcommand: "test", args: &["--", "--color", "always", "--show-output"], - exercise_name: self.name, + bin_name, description: "cargo test …", // Hide warnings because they are shown by Clippy. hide_warnings: true, @@ -124,18 +139,43 @@ impl Exercise { } .run()?; - let run_success = self.run_bin(output, target_dir)?; + let run_success = run_bin(bin_name, output, target_dir)?; Ok(test_success && run_success) } - pub fn terminal_link(&self) -> StyledContent> { - style(TerminalFileLink(self.path)).underlined().blue() + /// Compile, check and run the exercise. + /// The output is written to the `output` buffer after clearing it. + #[inline] + fn run_exercise(&self, output: &mut Vec, target_dir: &Path) -> Result { + self.run(self.name(), output, target_dir) + } + + /// Compile, check and run the exercise's solution. + /// The output is written to the `output` buffer after clearing it. + fn run_solution(&self, output: &mut Vec, target_dir: &Path) -> Result { + let name = self.name(); + let mut bin_name = String::with_capacity(name.len()); + bin_name.push_str(name); + bin_name.push_str("_sol"); + + self.run(&bin_name, output, target_dir) } } -impl Display for Exercise { - fn fmt(&self, f: &mut Formatter) -> fmt::Result { - self.path.fmt(f) +impl RunnableExercise for Exercise { + #[inline] + fn name(&self) -> &str { + self.name + } + + #[inline] + fn strict_clippy(&self) -> bool { + self.strict_clippy + } + + #[inline] + fn test(&self) -> bool { + self.test } } diff --git a/src/info_file.rs b/src/info_file.rs index 5ea487fc..f226f735 100644 --- a/src/info_file.rs +++ b/src/info_file.rs @@ -2,7 +2,7 @@ use anyhow::{bail, Context, Error, Result}; use serde::Deserialize; use std::{fs, io::ErrorKind}; -use crate::embedded::EMBEDDED_FILES; +use crate::{embedded::EMBEDDED_FILES, exercise::RunnableExercise}; /// Deserialized from the `info.toml` file. #[derive(Deserialize)] @@ -75,6 +75,23 @@ impl ExerciseInfo { } } +impl RunnableExercise for ExerciseInfo { + #[inline] + fn name(&self) -> &str { + &self.name + } + + #[inline] + fn strict_clippy(&self) -> bool { + self.strict_clippy + } + + #[inline] + fn test(&self) -> bool { + self.test + } +} + /// The deserialized `info.toml` file. #[derive(Deserialize)] pub struct InfoFile { diff --git a/src/run.rs b/src/run.rs index 36899b91..899d0a94 100644 --- a/src/run.rs +++ b/src/run.rs @@ -4,14 +4,14 @@ use std::io::{self, Write}; use crate::{ app_state::{AppState, ExercisesProgress}, - exercise::OUTPUT_CAPACITY, + exercise::{RunnableExercise, OUTPUT_CAPACITY}, terminal_link::TerminalFileLink, }; pub fn run(app_state: &mut AppState) -> Result<()> { let exercise = app_state.current_exercise(); let mut output = Vec::with_capacity(OUTPUT_CAPACITY); - let success = exercise.run(&mut output, app_state.target_dir())?; + let success = exercise.run_exercise(&mut output, app_state.target_dir())?; let mut stdout = io::stdout().lock(); stdout.write_all(&output)?; diff --git a/src/watch/state.rs b/src/watch/state.rs index 14c3f015..dd43c566 100644 --- a/src/watch/state.rs +++ b/src/watch/state.rs @@ -8,7 +8,7 @@ use std::io::{self, StdoutLock, Write}; use crate::{ app_state::{AppState, ExercisesProgress}, clear_terminal, - exercise::OUTPUT_CAPACITY, + exercise::{RunnableExercise, OUTPUT_CAPACITY}, progress_bar::progress_bar, terminal_link::TerminalFileLink, }; @@ -54,7 +54,7 @@ impl<'a> WatchState<'a> { let success = self .app_state .current_exercise() - .run(&mut self.output, self.app_state.target_dir())?; + .run_exercise(&mut self.output, self.app_state.target_dir())?; if success { self.done_status = if let Some(solution_path) = self.app_state.current_solution_path()? {