plugin refactor

Author: Heliozoa

plugins/csharp/src/error.rs

@@ -1,24 +1,10 @@
 //! Error type for C#
 use std::path::PathBuf;
-use std::process::ExitStatus;
 use thiserror::Error;
-use tmc_langs_framework::{error::CommandNotFound, zip, TmcError};
+use tmc_langs_framework::{error::CommandError, error::FileIo, zip, TmcError};
 
 #[derive(Debug, Error)]
 pub enum CSharpError {
-    #[error("Failed to create file {0}")]
-    CreateFile(PathBuf, #[source] std::io::Error),
-    #[error("Failed to write file {0}")]
-    WriteFile(PathBuf, #[source] std::io::Error),
-    #[error("Failed to read file {0}")]
-    ReadFile(PathBuf, #[source] std::io::Error),
-    #[error("Failed to remove file {0}")]
-    RemoveFile(PathBuf, #[source] std::io::Error),
-    #[error("Failed to create dir {0}")]
-    CreateDir(PathBuf, #[source] std::io::Error),
-    #[error("Failed to remove dir {0}")]
-    RemoveDir(PathBuf, #[source] std::io::Error),
-
     #[error("Failed to parse exercise description at {0}")]
     ParseExerciseDesc(PathBuf, #[source] serde_json::Error),
     #[error("Failed to parse test results at {0}")]
@@ -30,13 +16,13 @@ pub enum CSharpError {
     MissingBootstrapDll(PathBuf),
     #[error("Error while handling tmc-csharp-runner zip")]
     Zip(#[source] zip::result::ZipError),
-    #[error("Failed to run {0}")]
-    RunFailed(&'static str, #[source] std::io::Error),
-    #[error("Command {0} failed with return code {1}")]
-    CommandFailed(&'static str, ExitStatus),
 
     #[error("Command not found")]
-    CommandNotFound(#[from] CommandNotFound),
+    Command(#[from] CommandError),
+    #[error("File IO error")]
+    FileIo(#[from] FileIo),
+    #[error("Error")]
+    Tmc(#[from] TmcError),
 }
 
 impl From<CSharpError> for TmcError {

plugins/csharp/src/plugin.rs

@@ -10,6 +10,8 @@ use tmc_langs_framework::{
     domain::{
         ExerciseDesc, RunResult, RunStatus, Strategy, TestDesc, TestResult, ValidationResult,
     },
+    error::FileIo,
+    io::file_util,
     plugin::Language,
     zip::ZipArchive,
     LanguagePlugin, TmcError,
@@ -18,8 +20,8 @@ use tmc_langs_framework::{
 use std::collections::HashMap;
 use std::env;
 use std::ffi::{OsStr, OsString};
-use std::fs::{self, File};
-use std::io::{BufReader, Cursor, Read, Seek, Write};
+use std::fs::File;
+use std::io::{BufReader, Cursor, Read, Seek};
 use std::path::{Path, PathBuf};
 use std::time::Duration;
 use walkdir::WalkDir;
@@ -44,18 +46,13 @@ impl CSharpPlugin {
             if file.is_file() {
                 let target_file_path = target.join(file.sanitized_name());
                 if let Some(parent) = target_file_path.parent() {
-                    fs::create_dir_all(&parent)
-                        .map_err(|e| CSharpError::CreateDir(target_file_path.clone(), e))?;
+                    file_util::create_dir_all(&parent)?;
                 }
-                let mut target_file = File::create(&target_file_path)
-                    .map_err(|e| CSharpError::CreateFile(target_file_path.clone(), e))?;
                 let bytes: Vec<u8> = file
                     .bytes()
                     .collect::<Result<Vec<_>, _>>()
-                    .map_err(|e| CSharpError::ReadFile(target_file_path.clone(), e))?;
-                target_file
-                    .write_all(&bytes)
-                    .map_err(|e| CSharpError::WriteFile(target_file_path, e))?;
+                    .map_err(|e| FileIo::FileRead(target_file_path.clone(), e))?;
+                file_util::write_to_file(&mut bytes.as_slice(), target_file_path)?;
             }
         }
         Ok(())
@@ -94,8 +91,7 @@ impl CSharpPlugin {
 
     /// Parses the test results JSON file at the path argument.
     fn parse_test_results(test_results_path: &Path) -> Result<RunResult, CSharpError> {
-        let test_results = File::open(test_results_path)
-            .map_err(|e| CSharpError::ReadFile(test_results_path.to_path_buf(), e))?;
+        let test_results = file_util::open_file(test_results_path)?;
         let test_results: Vec<CSTestResult> = serde_json::from_reader(test_results)
             .map_err(|e| CSharpError::ParseTestResults(test_results_path.to_path_buf(), e))?;
 
@@ -122,14 +118,13 @@ impl LanguagePlugin for CSharpPlugin {
     // checks the directories in src for csproj files
     fn is_exercise_type_correct(path: &Path) -> bool {
         WalkDir::new(path.join("src"))
-            .min_depth(2)
             .max_depth(2)
             .into_iter()
             .filter_map(|e| e.ok())
             .any(|e| e.path().extension() == Some(&OsString::from("csproj")))
     }
 
-    /// Finds .csproj files and checks whether they are in a X/src/ directory, returning X if so.
+    /// Finds any directory X which contains a X/src/*.csproj file.
     fn find_project_dir_in_zip<R: Read + Seek>(
         zip_archive: &mut ZipArchive<R>,
     ) -> Result<PathBuf, TmcError> {
@@ -155,21 +150,16 @@ impl LanguagePlugin for CSharpPlugin {
 
     // runs --generate-points-file and parses the generated .tmc_available_points.json
     fn scan_exercise(&self, path: &Path, exercise_name: String) -> Result<ExerciseDesc, TmcError> {
-        let mut command = TmcCommand::new("dotnet");
+        let mut command = TmcCommand::new("dotnet".to_string());
         command
             .current_dir(path)
             .arg(Self::get_bootstrap_path()?)
             .arg("--generate-points-file");
-        let output = command.output()?;
-        log::trace!("stdout: {}", String::from_utf8_lossy(&output.stdout));
-        log::debug!("stderr: {}", String::from_utf8_lossy(&output.stderr));
-        if !output.status.success() {
-            return Err(CSharpError::CommandFailed("dotnet", output.status).into());
-        }
+        command.output_checked()?;
 
         let exercise_desc_json_path = path.join(".tmc_available_points.json");
         let exercise_desc_json = File::open(&exercise_desc_json_path)
-            .map_err(|e| CSharpError::ReadFile(exercise_desc_json_path.clone(), e))?;
+            .map_err(|e| FileIo::FileRead(exercise_desc_json_path.clone(), e))?;
         let json: HashMap<String, Vec<String>> =
             serde_json::from_reader(BufReader::new(exercise_desc_json))
                 .map_err(|e| CSharpError::ParseExerciseDesc(exercise_desc_json_path, e))?;
@@ -192,10 +182,9 @@ impl LanguagePlugin for CSharpPlugin {
     ) -> Result<RunResult, TmcError> {
         let test_results_path = path.join(".tmc_test_results.json");
         if test_results_path.exists() {
-            fs::remove_file(&test_results_path)
-                .map_err(|e| CSharpError::RemoveFile(test_results_path.clone(), e))?;
+            file_util::remove_file(&test_results_path)?;
         }
-        let mut command = TmcCommand::new("dotnet");
+        let mut command = TmcCommand::new("dotnet".to_string());
         command
             .current_dir(path)
             .arg(Self::get_bootstrap_path()?)
@@ -257,16 +246,14 @@ impl LanguagePlugin for CSharpPlugin {
     fn clean(&self, path: &Path) -> Result<(), TmcError> {
         let test_results_path = path.join(".tmc_test_results.json");
         if test_results_path.exists() {
-            fs::remove_file(&test_results_path)
-                .map_err(|e| CSharpError::RemoveFile(test_results_path.clone(), e))?;
+            file_util::remove_file(&test_results_path)?;
         }
         for entry in WalkDir::new(path).into_iter().filter_map(|e| e.ok()) {
             let file_name = entry.path().file_name();
             if file_name == Some(&OsString::from("bin"))
                 || file_name == Some(&OsString::from("obj"))
             {
-                fs::remove_dir_all(entry.path())
-                    .map_err(|e| CSharpError::RemoveDir(entry.path().to_path_buf(), e))?;
+                file_util::remove_dir_all(entry.path())?;
             }
         }
         Ok(())
@@ -276,6 +263,7 @@ impl LanguagePlugin for CSharpPlugin {
 #[cfg(test)]
 mod test {
     use super::*;
+    use std::fs;
     use std::sync::Once;
     use tempfile::TempDir;
 

plugins/java/src/ant.rs

@@ -7,14 +7,13 @@ use super::{error::JavaError, plugin::JavaPlugin, CompileResult, TestRun, SEPARA
 use j4rs::Jvm;
 use policy::AntStudentFilePolicy;
 use std::env;
-use std::fs::{self, File};
-use std::io::Write;
 use std::path::Path;
 use std::process::Stdio;
 use std::time::Duration;
 use tmc_langs_framework::{
     command::TmcCommand,
     domain::{ExerciseDesc, RunResult, ValidationResult},
+    io::file_util,
     plugin::{Language, LanguagePlugin},
     TmcError,
 };
@@ -36,7 +35,7 @@ impl AntPlugin {
 
     fn get_ant_executable(&self) -> &'static str {
         if cfg!(windows) {
-            if let Ok(status) = TmcCommand::new("ant")
+            if let Ok(status) = TmcCommand::new("ant".to_string())
                 .arg("-version")
                 .stdout(Stdio::piped())
                 .stderr(Stdio::piped())
@@ -61,13 +60,8 @@ impl AntPlugin {
 
         // TODO: don't traverse symlinks
         if !runner_path.exists() {
-            fs::create_dir_all(&runner_dir).map_err(|e| JavaError::DirCreate(runner_dir, e))?;
             log::debug!("writing tmc-junit-runner to {}", runner_path.display());
-            let mut target_file = File::create(&runner_path)
-                .map_err(|e| JavaError::FileCreate(runner_path.clone(), e))?;
-            target_file
-                .write_all(JUNIT_RUNNER_ARCHIVE)
-                .map_err(|e| JavaError::JarWrite(runner_path, e))?;
+            file_util::write_to_file(&mut JUNIT_RUNNER_ARCHIVE, &runner_path)?;
         } else {
             log::debug!("already exists");
         }
@@ -118,34 +112,21 @@ impl LanguagePlugin for AntPlugin {
         log::debug!("Cleaning project at {}", path.display());
 
         let stdout_path = path.join("build_log.txt");
-        let stdout = File::create(&stdout_path)
-            .map_err(|e| JavaError::FileCreate(stdout_path.clone(), e))?;
+        let stdout = file_util::create_file(&stdout_path)?;
         let stderr_path = path.join("build_errors.txt");
-        let stderr = File::create(&stderr_path)
-            .map_err(|e| JavaError::FileCreate(stderr_path.clone(), e))?;
+        let stderr = file_util::create_file(&stderr_path)?;
 
         let ant_exec = self.get_ant_executable();
-        let mut command = TmcCommand::new(ant_exec);
+        let mut command = TmcCommand::new(ant_exec.to_string());
         command
             .arg("clean")
             .stdout(stdout)
             .stderr(stderr)
             .current_dir(path);
-        let output = command.output()?;
-
-        if output.status.success() {
-            fs::remove_file(&stdout_path).map_err(|e| JavaError::FileRemove(stdout_path, e))?;
-            fs::remove_file(&stderr_path).map_err(|e| JavaError::FileRemove(stderr_path, e))?;
-            Ok(())
-        } else {
-            Err(JavaError::FailedCommand(
-                "ant clean".to_string(),
-                output.status,
-                String::from_utf8_lossy(&output.stdout).into_owned(),
-                String::from_utf8_lossy(&output.stderr).into_owned(),
-            )
-            .into())
-        }
+        command.output_checked()?;
+        file_util::remove_file(&stdout_path)?;
+        file_util::remove_file(&stderr_path)?;
+        Ok(())
     }
 }
 
@@ -192,27 +173,18 @@ impl JavaPlugin for AntPlugin {
     fn build(&self, project_root_path: &Path) -> Result<CompileResult, JavaError> {
         log::info!("Building project at {}", project_root_path.display());
 
-        let stdout_path = project_root_path.join("build_log.txt");
-        let mut stdout = File::create(&stdout_path)
-            .map_err(|e| JavaError::FileCreate(stdout_path.clone(), e))?;
-        let stderr_path = project_root_path.join("build_errors.txt");
-        let mut stderr = File::create(&stderr_path)
-            .map_err(|e| JavaError::FileCreate(stderr_path.clone(), e))?;
-
         // TODO: don't require ant in path?
         let ant_exec = self.get_ant_executable();
-        let mut command = TmcCommand::new(ant_exec);
+        let mut command = TmcCommand::new(ant_exec.to_string());
         command.arg("compile-test").current_dir(project_root_path);
         let output = command.output()?;
 
         log::debug!("stdout: {}", String::from_utf8_lossy(&output.stdout));
         log::debug!("stderr: {}", String::from_utf8_lossy(&output.stderr));
-        stdout
-            .write_all(&output.stdout)
-            .map_err(|e| JavaError::FileWrite(stdout_path, e))?;
-        stderr
-            .write_all(&output.stderr)
-            .map_err(|e| JavaError::FileWrite(stderr_path, e))?;
+        let stdout_path = project_root_path.join("build_log.txt");
+        let stderr_path = project_root_path.join("build_errors.txt");
+        file_util::write_to_file(&mut output.stdout.as_slice(), stdout_path)?;
+        file_util::write_to_file(&mut output.stderr.as_slice(), stderr_path)?;
 
         Ok(CompileResult {
             status_code: output.status,
@@ -270,7 +242,7 @@ impl JavaPlugin for AntPlugin {
         }
 
         log::debug!("java args {} in {}", arguments.join(" "), path.display());
-        let mut command = TmcCommand::new("java");
+        let mut command = TmcCommand::new("java".to_string());
         command.current_dir(path).args(arguments);
         let output = command.output()?;
 
@@ -286,6 +258,7 @@ impl JavaPlugin for AntPlugin {
 #[cfg(not(target_os = "macos"))] // ant is not installed on github's macos-latest image
 mod test {
     use super::*;
+    use std::fs::{self, File};
     use tempfile::{tempdir, TempDir};
     use tmc_langs_framework::domain::Strategy;
     use tmc_langs_framework::zip::ZipArchive;

plugins/java/src/error.rs

@@ -2,9 +2,11 @@
 
 use std::io;
 use std::path::PathBuf;
-use std::process::ExitStatus;
 use thiserror::Error;
-use tmc_langs_framework::TmcError;
+use tmc_langs_framework::{
+    error::{CommandError, FileIo},
+    TmcError,
+};
 
 #[derive(Error, Debug)]
 pub enum JavaError {
@@ -14,43 +16,23 @@ pub enum JavaError {
     NoMvnClassPath,
     #[error("{0} did not contain a valid exercise")]
     InvalidExercise(PathBuf),
-    #[error("Failed to run {0}")]
-    FailedToRun(String, #[source] std::io::Error),
-    #[error(
-        r"Command '{0}' exited with a exit status {1}
-#### STDOUT ####
-{2}
-#### STDERR ####
-{3}"
-    )]
-    FailedCommand(String, ExitStatus, String, String),
     #[error("Failed to write temporary .jar file {0}")]
     JarWrite(PathBuf, #[source] io::Error),
-    #[error("Failed to create file at {0}")]
-    FileCreate(PathBuf, #[source] io::Error),
-    #[error("Failed to write to file at {0}")]
-    FileWrite(PathBuf, #[source] io::Error),
-    #[error("Failed to read to file at {0}")]
-    FileRead(PathBuf, #[source] io::Error),
-    #[error("Failed to remove file at {0}")]
-    FileRemove(PathBuf, #[source] io::Error),
-    #[error("Failed to create directory at {0}")]
-    DirCreate(PathBuf, #[source] io::Error),
     #[error("Failed to create temporary directory at")]
     TempDir(#[source] io::Error),
     #[error("Failed to find home directory")]
     HomeDir,
-    #[error("Failed to copy file from {0} to {1}")]
-    FileCopy(PathBuf, PathBuf, #[source] std::io::Error),
     #[error("Failed to find cache directory")]
     CacheDir,
     #[error("Failed to compile")]
-    Compilation(Vec<u8>),
+    Compilation { stdout: String, stderr: String },
 
     #[error(transparent)]
     Json(#[from] serde_json::Error),
-    #[error(transparent)]
-    Inner(#[from] TmcError),
+    #[error("Failed to run command")]
+    Command(#[from] CommandError),
+    #[error("Error")]
+    Tmc(#[from] TmcError),
 }
 
 impl From<JavaError> for TmcError {
@@ -59,6 +41,12 @@ impl From<JavaError> for TmcError {
     }
 }
 
+impl From<FileIo> for JavaError {
+    fn from(err: FileIo) -> JavaError {
+        JavaError::Tmc(TmcError::FileIo(err))
+    }
+}
+
 impl<T> Into<Result<T, TmcError>> for JavaError {
     fn into(self) -> Result<T, TmcError> {
         Err(TmcError::Plugin(Box::new(self)))