Improve the runner

src/app_state.rs

@@ -1,19 +1,18 @@
 use anyhow::{bail, Context, Error, Result};
-use serde::Deserialize;
 use std::{
     fs::{self, File},
     io::{Read, StdoutLock, Write},
-    path::{Path, PathBuf},
+    path::Path,
     process::{Command, Stdio},
     thread,
 };
 
 use crate::{
     clear_terminal,
+    cmd::CmdRunner,
     embedded::EMBEDDED_FILES,
     exercise::{Exercise, RunnableExercise},
     info_file::ExerciseInfo,
-    DEBUG_PROFILE,
 };
 
 const STATE_FILE_NAME: &str = ".rustlings-state.txt";
@@ -34,31 +33,6 @@ pub enum StateFileStatus {
     NotRead,
 }
 
-// Parses parts of the output of `cargo metadata`.
-#[derive(Deserialize)]
-struct CargoMetadata {
-    target_directory: PathBuf,
-}
-
-pub fn parse_target_dir() -> Result<PathBuf> {
-    // 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::<CargoMetadata>(&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<Exercise>,
@@ -68,8 +42,7 @@ pub struct AppState {
     // Preallocated buffer for reading and writing the state file.
     file_buf: Vec<u8>,
     official_exercises: bool,
-    // Cargo's target directory.
+    cmd_runner: CmdRunner,
-    target_dir: PathBuf,
 }
 
 impl AppState {
@@ -123,7 +96,7 @@ impl AppState {
         exercise_infos: Vec<ExerciseInfo>,
         final_message: String,
     ) -> Result<(Self, StateFileStatus)> {
-        let target_dir = parse_target_dir()?;
+        let cmd_runner = CmdRunner::build()?;
 
         let exercises = exercise_infos
             .into_iter()
@@ -157,7 +130,7 @@ impl AppState {
             final_message,
             file_buf: Vec::with_capacity(2048),
             official_exercises: !Path::new("info.toml").exists(),
-            target_dir,
+            cmd_runner,
         };
 
         let state_file_status = slf.update_from_file();
@@ -186,8 +159,8 @@ impl AppState {
     }
 
     #[inline]
-    pub fn target_dir(&self) -> &Path {
+    pub fn cmd_runner(&self) -> &CmdRunner {
-        &self.target_dir
+        &self.cmd_runner
     }
 
     // Write the state file.
@@ -336,7 +309,7 @@ impl AppState {
     /// Official exercises: Dump the solution file form the binary and return its path.
     /// Third-party exercises: Check if a solution file exists and return its path in that case.
     pub fn current_solution_path(&self) -> Result<Option<String>> {
-        if DEBUG_PROFILE {
+        if cfg!(debug_assertions) {
             return Ok(None);
         }
 
@@ -386,7 +359,7 @@ impl AppState {
                 .iter_mut()
                 .map(|exercise| {
                     s.spawn(|| {
-                        let success = exercise.run_exercise(None, &self.target_dir)?;
+                        let success = exercise.run_exercise(None, &self.cmd_runner)?;
                         exercise.done = success;
                         Ok::<_, Error>(success)
                     })
@@ -434,10 +407,6 @@ impl AppState {
     }
 }
 
-const CARGO_METADATA_ERR: &str = "Failed to run the command `cargo metadata …`
-Did you already install Rust?
-Try running `cargo --version` to diagnose the problem.";
-
 const RERUNNING_ALL_EXERCISES_MSG: &[u8] = b"
 All exercises seem to be done.
 Recompiling and running all exercises to make sure that all of them are actually done.
@@ -490,7 +459,7 @@ mod tests {
             final_message: String::new(),
             file_buf: Vec::new(),
             official_exercises: true,
-            target_dir: PathBuf::new(),
+            cmd_runner: CmdRunner::build().unwrap(),
         };
 
         let mut assert = |done: [bool; 3], expected: [Option<usize>; 3]| {

src/cmd.rs

@@ -1,13 +1,14 @@
 use anyhow::{Context, Result};
+use serde::Deserialize;
 use std::{
     io::Read,
-    path::Path,
+    path::PathBuf,
     process::{Command, Stdio},
 };
 
 /// Run a command with a description for a possible error and append the merged stdout and stderr.
 /// The boolean in the returned `Result` is true if the command's exit status is success.
-pub fn run_cmd(mut cmd: Command, description: &str, output: Option<&mut Vec<u8>>) -> Result<bool> {
+fn run_cmd(mut cmd: Command, description: &str, output: Option<&mut Vec<u8>>) -> Result<bool> {
     let spawn = |mut cmd: Command| {
         // NOTE: The closure drops `cmd` which prevents a pipe deadlock.
         cmd.stdin(Stdio::null())
@@ -45,50 +46,107 @@ pub fn run_cmd(mut cmd: Command, description: &str, output: Option<&mut Vec<u8>>
         .map(|status| status.success())
 }
 
-pub struct CargoCmd<'a> {
+// Parses parts of the output of `cargo metadata`.
-    pub subcommand: &'a str,
+#[derive(Deserialize)]
-    pub args: &'a [&'a str],
+struct CargoMetadata {
-    pub bin_name: &'a str,
+    target_directory: PathBuf,
-    pub description: &'a str,
-    /// RUSTFLAGS="-A warnings"
-    pub hide_warnings: bool,
-    /// Added as `--target-dir` if `Self::dev` is true.
-    pub target_dir: &'a Path,
-    /// The output buffer to append the merged stdout and stderr.
-    pub output: Option<&'a mut Vec<u8>>,
-    /// true while developing Rustlings.
-    pub dev: bool,
 }
 
-impl<'a> CargoCmd<'a> {
+pub struct CmdRunner {
-    /// Run `cargo SUBCOMMAND --bin EXERCISE_NAME … ARGS`.
+    target_dir: PathBuf,
-    pub fn run(self) -> Result<bool> {
+}
+
+impl CmdRunner {
+    pub fn build() -> Result<Self> {
+        // 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::<CargoMetadata>(&metadata_output)
+            .context("Failed to read the field `target_directory` from the `cargo metadata` output")
+            .map(|metadata| metadata.target_directory)?;
+
+        Ok(Self { target_dir })
+    }
+
+    pub fn cargo<'out>(
+        &self,
+        subcommand: &str,
+        bin_name: &str,
+        output: Option<&'out mut Vec<u8>>,
+    ) -> CargoSubcommand<'out> {
         let mut cmd = Command::new("cargo");
-        cmd.arg(self.subcommand);
+        cmd.arg(subcommand).arg("-q").arg("--bin").arg(bin_name);
 
         // A hack to make `cargo run` work when developing Rustlings.
-        if self.dev {
+        #[cfg(debug_assertions)]
         cmd.arg("--manifest-path")
             .arg("dev/Cargo.toml")
             .arg("--target-dir")
-                .arg(self.target_dir);
+            .arg(&self.target_dir);
+
+        if output.is_some() {
+            cmd.arg("--color").arg("always");
         }
 
-        cmd.arg("--color")
+        CargoSubcommand { cmd, output }
-            .arg("always")
-            .arg("-q")
-            .arg("--bin")
-            .arg(self.bin_name)
-            .args(self.args);
-
-        if self.hide_warnings {
-            cmd.env("RUSTFLAGS", "-A warnings");
     }
 
-        run_cmd(cmd, self.description, self.output)
+    /// The boolean in the returned `Result` is true if the command's exit status is success.
+    pub fn run_debug_bin(&self, bin_name: &str, output: Option<&mut Vec<u8>>) -> Result<bool> {
+        // 7 = "/debug/".len()
+        let mut bin_path =
+            PathBuf::with_capacity(self.target_dir.as_os_str().len() + 7 + bin_name.len());
+        bin_path.push(&self.target_dir);
+        bin_path.push("debug");
+        bin_path.push(bin_name);
+
+        run_cmd(Command::new(&bin_path), &bin_path.to_string_lossy(), output)
     }
 }
 
+pub struct CargoSubcommand<'out> {
+    cmd: Command,
+    output: Option<&'out mut Vec<u8>>,
+}
+
+impl<'out> CargoSubcommand<'out> {
+    #[inline]
+    pub fn args<'arg, I>(&mut self, args: I) -> &mut Self
+    where
+        I: IntoIterator<Item = &'arg str>,
+    {
+        self.cmd.args(args);
+        self
+    }
+
+    /// RUSTFLAGS="-A warnings"
+    #[inline]
+    pub fn hide_warnings(&mut self) -> &mut Self {
+        self.cmd.env("RUSTFLAGS", "-A warnings");
+        self
+    }
+
+    /// The boolean in the returned `Result` is true if the command's exit status is success.
+    #[inline]
+    pub fn run(self, description: &str) -> Result<bool> {
+        run_cmd(self.cmd, description, self.output)
+    }
+}
+
+const CARGO_METADATA_ERR: &str = "Failed to run the command `cargo metadata …`
+Did you already install Rust?
+Try running `cargo --version` to diagnose the problem.";
+
 #[cfg(test)]
 mod tests {
     use super::*;

src/dev.rs

@@ -2,8 +2,6 @@ use anyhow::{bail, Context, Result};
 use clap::Subcommand;
 use std::path::PathBuf;
 
-use crate::DEBUG_PROFILE;
-
 mod check;
 mod new;
 mod update;
@@ -32,7 +30,7 @@ impl DevCommands {
     pub fn run(self) -> Result<()> {
         match self {
             Self::New { path, no_git } => {
-                if DEBUG_PROFILE {
+                if cfg!(debug_assertions) {
                     bail!("Disabled in the debug build");
                 }
 

src/dev/check.rs

@@ -12,11 +12,11 @@ use std::{
 };
 
 use crate::{
-    app_state::parse_target_dir,
     cargo_toml::{append_bins, bins_start_end_ind, BINS_BUFFER_CAPACITY},
+    cmd::CmdRunner,
     exercise::{RunnableExercise, OUTPUT_CAPACITY},
     info_file::{ExerciseInfo, InfoFile},
-    CURRENT_FORMAT_VERSION, DEBUG_PROFILE,
+    CURRENT_FORMAT_VERSION,
 };
 
 // Find a char that isn't allowed in the exercise's `name` or `dir`.
@@ -37,8 +37,8 @@ fn check_cargo_toml(
     append_bins(&mut new_bins, exercise_infos, exercise_path_prefix);
 
     if old_bins != new_bins {
-        if DEBUG_PROFILE {
+        if cfg!(debug_assertions) {
-            bail!("The file `dev/Cargo.toml` is outdated. Please run `cargo run -- dev update` to update it");
+            bail!("The file `dev/Cargo.toml` is outdated. Please run `cargo run -- dev update` to update it. Then run `cargo run -- dev check` again");
         }
 
         bail!("The file `Cargo.toml` is outdated. Please run `rustlings dev update` to update it. Then run `rustlings dev check` again");
@@ -162,7 +162,7 @@ fn check_unexpected_files(
     Ok(())
 }
 
-fn check_exercises_unsolved(info_file: &InfoFile, target_dir: &Path) -> Result<()> {
+fn check_exercises_unsolved(info_file: &InfoFile, cmd_runner: &CmdRunner) -> Result<()> {
     let error_occurred = AtomicBool::new(false);
 
     println!(
@@ -184,7 +184,7 @@ fn check_exercises_unsolved(info_file: &InfoFile, target_dir: &Path) -> Result<(
                     error_occurred.store(true, atomic::Ordering::Relaxed);
                 };
 
-                match exercise_info.run_exercise(None, target_dir) {
+                match exercise_info.run_exercise(None, cmd_runner) {
                     Ok(true) => error(b"Already solved!"),
                     Ok(false) => (),
                     Err(e) => error(e.to_string().as_bytes()),
@@ -200,7 +200,7 @@ fn check_exercises_unsolved(info_file: &InfoFile, target_dir: &Path) -> Result<(
     Ok(())
 }
 
-fn check_exercises(info_file: &InfoFile, target_dir: &Path) -> Result<()> {
+fn check_exercises(info_file: &InfoFile, cmd_runner: &CmdRunner) -> Result<()> {
     match info_file.format_version.cmp(&CURRENT_FORMAT_VERSION) {
         Ordering::Less => bail!("`format_version` < {CURRENT_FORMAT_VERSION} (supported version)\nPlease migrate to the latest format version"),
         Ordering::Greater => bail!("`format_version` > {CURRENT_FORMAT_VERSION} (supported version)\nTry updating the Rustlings program"),
@@ -210,10 +210,14 @@ fn check_exercises(info_file: &InfoFile, target_dir: &Path) -> Result<()> {
     let info_file_paths = check_info_file_exercises(info_file)?;
     check_unexpected_files("exercises", &info_file_paths)?;
 
-    check_exercises_unsolved(info_file, target_dir)
+    check_exercises_unsolved(info_file, cmd_runner)
 }
 
-fn check_solutions(require_solutions: bool, info_file: &InfoFile, target_dir: &Path) -> Result<()> {
+fn check_solutions(
+    require_solutions: bool,
+    info_file: &InfoFile,
+    cmd_runner: &CmdRunner,
+) -> Result<()> {
     let paths = Mutex::new(hashbrown::HashSet::with_capacity(info_file.exercises.len()));
     let error_occurred = AtomicBool::new(false);
 
@@ -243,7 +247,7 @@ fn check_solutions(require_solutions: bool, info_file: &InfoFile, target_dir: &P
                 }
 
                 let mut output = Vec::with_capacity(OUTPUT_CAPACITY);
-                match exercise_info.run_solution(Some(&mut output), target_dir) {
+                match exercise_info.run_solution(Some(&mut output), cmd_runner) {
                     Ok(true) => {
                         paths.lock().unwrap().insert(PathBuf::from(path));
                     }
@@ -266,8 +270,8 @@ fn check_solutions(require_solutions: bool, info_file: &InfoFile, target_dir: &P
 pub fn check(require_solutions: bool) -> Result<()> {
     let info_file = InfoFile::parse()?;
 
+    if cfg!(debug_assertions) {
         // A hack to make `cargo run -- dev check` work when developing Rustlings.
-    if DEBUG_PROFILE {
         check_cargo_toml(
             &info_file.exercises,
             include_str!("../../dev-Cargo.toml"),
@@ -279,9 +283,9 @@ pub fn check(require_solutions: bool) -> Result<()> {
         check_cargo_toml(&info_file.exercises, &current_cargo_toml, b"")?;
     }
 
-    let target_dir = parse_target_dir()?;
+    let cmd_runner = CmdRunner::build()?;
-    check_exercises(&info_file, &target_dir)?;
+    check_exercises(&info_file, &cmd_runner)?;
-    check_solutions(require_solutions, &info_file, &target_dir)?;
+    check_solutions(require_solutions, &info_file, &cmd_runner)?;
 
     println!("\nEverything looks fine!");
 

src/dev/update.rs

@@ -4,7 +4,6 @@ use std::fs;
 use crate::{
     cargo_toml::updated_cargo_toml,
     info_file::{ExerciseInfo, InfoFile},
-    DEBUG_PROFILE,
 };
 
 // Update the `Cargo.toml` file.
@@ -27,7 +26,7 @@ pub fn update() -> Result<()> {
     let info_file = InfoFile::parse()?;
 
     // A hack to make `cargo run -- dev update` work when developing Rustlings.
-    if DEBUG_PROFILE {
+    if cfg!(debug_assertions) {
         update_cargo_toml(
             &info_file.exercises,
             include_str!("../../dev-Cargo.toml"),

src/exercise.rs

@@ -3,38 +3,25 @@ use ratatui::crossterm::style::{style, StyledContent, Stylize};
 use std::{
     fmt::{self, Display, Formatter},
     io::Write,
-    path::{Path, PathBuf},
-    process::Command,
 };
 
-use crate::{
+use crate::{cmd::CmdRunner, terminal_link::TerminalFileLink};
-    cmd::{run_cmd, CargoCmd},
-    in_official_repo,
-    terminal_link::TerminalFileLink,
-    DEBUG_PROFILE,
-};
 
 /// 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, mut output: Option<&mut Vec<u8>>, target_dir: &Path) -> Result<bool> {
+fn run_bin(
+    bin_name: &str,
+    mut output: Option<&mut Vec<u8>>,
+    cmd_runner: &CmdRunner,
+) -> Result<bool> {
     if let Some(output) = output.as_deref_mut() {
         writeln!(output, "{}", "Output".underlined())?;
     }
 
-    // 7 = "/debug/".len()
+    let success = cmd_runner.run_debug_bin(bin_name, output.as_deref_mut())?;
-    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.as_deref_mut(),
-    )?;
 
     if let Some(output) = output {
         if !success {
@@ -89,26 +76,20 @@ pub trait RunnableExercise {
         &self,
         bin_name: &str,
         mut output: Option<&mut Vec<u8>>,
-        target_dir: &Path,
+        cmd_runner: &CmdRunner,
     ) -> Result<bool> {
-        if let Some(output) = output.as_deref_mut() {
+        let output_is_none = if let Some(output) = output.as_deref_mut() {
             output.clear();
-        }
+            false
+        } else {
+            true
+        };
 
-        // Developing the official Rustlings.
+        let mut build_cmd = cmd_runner.cargo("build", bin_name, output.as_deref_mut());
-        let dev = DEBUG_PROFILE && in_official_repo();
+        if output_is_none {
-
+            build_cmd.hide_warnings();
-        let build_success = CargoCmd {
-            subcommand: "build",
-            args: &[],
-            bin_name,
-            description: "cargo build …",
-            hide_warnings: output.is_none(),
-            target_dir,
-            output: output.as_deref_mut(),
-            dev,
         }
-        .run()?;
+        let build_success = build_cmd.run("cargo build …")?;
         if !build_success {
             return Ok(false);
         }
@@ -118,45 +99,33 @@ pub trait RunnableExercise {
             output.clear();
         }
 
+        let mut clippy_cmd = cmd_runner.cargo("clippy", bin_name, output.as_deref_mut());
+
         // `--profile test` is required to also check code with `[cfg(test)]`.
-        let clippy_args: &[&str] = if self.strict_clippy() {
+        if self.strict_clippy() {
-            &["--profile", "test", "--", "-D", "warnings"]
+            clippy_cmd.args(["--profile", "test", "--", "-D", "warnings"]);
         } else {
-            &["--profile", "test"]
+            clippy_cmd.args(["--profile", "test"]);
-        };
-        let clippy_success = CargoCmd {
-            subcommand: "clippy",
-            args: clippy_args,
-            bin_name,
-            description: "cargo clippy …",
-            hide_warnings: false,
-            target_dir,
-            output: output.as_deref_mut(),
-            dev,
         }
-        .run()?;
+
+        let clippy_success = clippy_cmd.run("cargo clippy …")?;
         if !clippy_success {
             return Ok(false);
         }
 
         if !self.test() {
-            return run_bin(bin_name, output.as_deref_mut(), target_dir);
+            return run_bin(bin_name, output.as_deref_mut(), cmd_runner);
         }
 
-        let test_success = CargoCmd {
+        let mut test_cmd = cmd_runner.cargo("test", bin_name, output.as_deref_mut());
-            subcommand: "test",
+        if !output_is_none {
-            args: &["--", "--color", "always", "--show-output"],
+            test_cmd.args(["--", "--color", "always", "--show-output"]);
-            bin_name,
+        }
-            description: "cargo test …",
         // Hide warnings because they are shown by Clippy.
-            hide_warnings: true,
+        test_cmd.hide_warnings();
-            target_dir,
+        let test_success = test_cmd.run("cargo test …")?;
-            output: output.as_deref_mut(),
-            dev,
-        }
-        .run()?;
 
-        let run_success = run_bin(bin_name, output.as_deref_mut(), target_dir)?;
+        let run_success = run_bin(bin_name, output, cmd_runner)?;
 
         Ok(test_success && run_success)
     }
@@ -164,19 +133,19 @@ pub trait RunnableExercise {
     /// Compile, check and run the exercise.
     /// The output is written to the `output` buffer after clearing it.
     #[inline]
-    fn run_exercise(&self, output: Option<&mut Vec<u8>>, target_dir: &Path) -> Result<bool> {
+    fn run_exercise(&self, output: Option<&mut Vec<u8>>, cmd_runner: &CmdRunner) -> Result<bool> {
-        self.run(self.name(), output, target_dir)
+        self.run(self.name(), output, cmd_runner)
     }
 
     /// Compile, check and run the exercise's solution.
     /// The output is written to the `output` buffer after clearing it.
-    fn run_solution(&self, output: Option<&mut Vec<u8>>, target_dir: &Path) -> Result<bool> {
+    fn run_solution(&self, output: Option<&mut Vec<u8>>, cmd_runner: &CmdRunner) -> Result<bool> {
         let name = self.name();
         let mut bin_name = String::with_capacity(name.len() + 4);
         bin_name.push_str(name);
         bin_name.push_str("_sol");
 
-        self.run(&bin_name, output, target_dir)
+        self.run(&bin_name, output, cmd_runner)
     }
 }
 

src/main.rs

@@ -24,22 +24,6 @@ mod terminal_link;
 mod watch;
 
 const CURRENT_FORMAT_VERSION: u8 = 1;
-const DEBUG_PROFILE: bool = {
-    #[allow(unused_assignments, unused_mut)]
-    let mut debug_profile = false;
-
-    #[cfg(debug_assertions)]
-    {
-        debug_profile = true;
-    }
-
-    debug_profile
-};
-
-// The current directory is the official Rustligns repository.
-fn in_official_repo() -> bool {
-    Path::new("dev/rustlings-repo.txt").exists()
-}
 
 fn clear_terminal(stdout: &mut StdoutLock) -> io::Result<()> {
     stdout.write_all(b"\x1b[H\x1b[2J\x1b[3J")
@@ -89,7 +73,7 @@ enum Subcommands {
 fn main() -> Result<()> {
     let args = Args::parse();
 
-    if !DEBUG_PROFILE && in_official_repo() {
+    if cfg!(not(debug_assertions)) && Path::new("dev/rustlings-repo.txt").exists() {
         bail!("{OLD_METHOD_ERR}");
     }
 

src/run.rs

@@ -11,7 +11,7 @@ use crate::{
 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_exercise(Some(&mut output), app_state.target_dir())?;
+    let success = exercise.run_exercise(Some(&mut output), app_state.cmd_runner())?;
 
     let mut stdout = io::stdout().lock();
     stdout.write_all(&output)?;

src/watch/state.rs

@@ -54,7 +54,7 @@ impl<'a> WatchState<'a> {
         let success = self
             .app_state
             .current_exercise()
-            .run_exercise(Some(&mut self.output), self.app_state.target_dir())?;
+            .run_exercise(Some(&mut self.output), self.app_state.cmd_runner())?;
         if success {
             self.done_status =
                 if let Some(solution_path) = self.app_state.current_solution_path()? {