refactor csharp

Author: Heliozoa

plugins/csharp/src/cs_test_result.rs

@@ -0,0 +1,47 @@
+//! Contains the CSTestResult type that models the C# test runner result.
+
+use serde::Deserialize;
+use tmc_langs_framework::domain::TestResult;
+
+/// Test result from the C# test runner
+#[derive(Debug, Deserialize)]
+#[serde(rename_all = "PascalCase")]
+pub struct CSTestResult {
+    pub name: String,
+    pub passed: bool,
+    pub message: String,
+    pub points: Vec<String>,
+    pub error_stack_trace: Vec<String>,
+}
+
+impl From<CSTestResult> for TestResult {
+    fn from(test_result: CSTestResult) -> Self {
+        TestResult {
+            name: test_result.name,
+            successful: test_result.passed,
+            message: test_result.message,
+            exception: test_result.error_stack_trace,
+            points: test_result.points,
+        }
+    }
+}
+
+#[cfg(test)]
+mod test {
+    use super::*;
+
+    #[test]
+    fn deserializes() {
+        let s = r#"
+{
+    "Name": "n",
+    "Passed": true,
+    "Message": "m",
+    "Points": ["p1", "p2"],
+    "ErrorStackTrace": ["e1", "e2"]
+}
+"#;
+
+        let _cstr: CSTestResult = serde_json::from_str(s).unwrap();
+    }
+}

plugins/csharp/src/error.rs

@@ -1,4 +1,4 @@
-//! Error type for C#
+//! Error type for the crate.
 
 use std::path::PathBuf;
 use thiserror::Error;

plugins/csharp/src/lib.rs

@@ -1,34 +1,10 @@
-//! Language plugin for C#
+//! TMC language plugin for C#.
 
+mod cs_test_result;
 mod error;
 mod plugin;
 mod policy;
 
 pub use self::error::CSharpError;
 pub use self::plugin::CSharpPlugin;
 pub use self::policy::CSharpStudentFilePolicy;
-
-use serde::Deserialize;
-use tmc_langs_framework::domain::TestResult;
-
-#[derive(Debug, Deserialize)]
-#[serde(rename_all = "PascalCase")]
-struct CSTestResult {
-    name: String,
-    passed: bool,
-    message: String,
-    points: Vec<String>,
-    error_stack_trace: Vec<String>,
-}
-
-impl From<CSTestResult> for TestResult {
-    fn from(test_result: CSTestResult) -> Self {
-        TestResult {
-            name: test_result.name,
-            successful: test_result.passed,
-            message: test_result.message,
-            exception: test_result.error_stack_trace,
-            points: test_result.points,
-        }
-    }
-}

plugins/csharp/src/plugin.rs

@@ -1,8 +1,8 @@
-//! Implementation of LanguagePlugin for C#
+//! An implementation of LanguagePlugin for C#.
 
 use crate::policy::CSharpStudentFilePolicy;
 
-use crate::{CSTestResult, CSharpError};
+use crate::{cs_test_result::CSTestResult, CSharpError};
 use std::collections::HashMap;
 use std::env;
 use std::ffi::{OsStr, OsString};
@@ -33,7 +33,7 @@ impl CSharpPlugin {
     }
 
     /// Extracts the bundled tmc-csharp-runner to the given path.
-    fn extract_runner(target: &Path) -> Result<(), CSharpError> {
+    fn extract_runner_to_dir(target: &Path) -> Result<(), CSharpError> {
         log::debug!("extracting C# runner to {}", target.display());
         const TMC_CSHARP_RUNNER: &[u8] = include_bytes!("../tmc-csharp-runner-1.0.1.zip");
 
@@ -58,12 +58,14 @@ impl CSharpPlugin {
     }
 
     /// Returns the directory of the TMC C# runner, writing it to the cache dir if it doesn't exist there yet.
-    fn get_runner_dir() -> Result<PathBuf, CSharpError> {
+    ///
+    /// NOTE: May cause issues if called concurrently.
+    fn get_or_init_runner_dir() -> Result<PathBuf, CSharpError> {
         match dirs::cache_dir() {
             Some(cache_dir) => {
                 let runner_dir = cache_dir.join("tmc").join("tmc-csharp-runner");
                 if !runner_dir.exists() {
-                    Self::extract_runner(&runner_dir)?;
+                    Self::extract_runner_to_dir(&runner_dir)?;
                 }
                 Ok(runner_dir)
             }
@@ -77,7 +79,7 @@ impl CSharpPlugin {
             log::debug!("using bootstrap path TMC_CSHARP_BOOTSTRAP_PATH={}", var);
             Ok(PathBuf::from(var))
         } else {
-            let runner_path = Self::get_runner_dir()?;
+            let runner_path = Self::get_or_init_runner_dir()?;
             let bootstrap_path = runner_path.join("TestMyCode.CSharp.Bootstrap.dll");
             if bootstrap_path.exists() {
                 log::debug!("found boostrap dll at {}", bootstrap_path.display());
@@ -139,9 +141,12 @@ impl LanguagePlugin for CSharpPlugin {
             let file_path = Path::new(file.name());
 
             if file_path.extension() == Some(OsStr::new("csproj")) {
+                // check parent of parent of csproj file for src
                 if let Some(csproj_parent) = file_path.parent().and_then(Path::parent) {
                     if csproj_parent.file_name() == Some(OsStr::new("src")) {
+                        // get parent of src
                         if let Some(src_parent) = csproj_parent.parent() {
+                            // skip if any part of the path is __MACOSX
                             if src_parent.components().any(|p| p.as_os_str() == "__MACOSX") {
                                 continue;
                             }
@@ -276,8 +281,9 @@ impl LanguagePlugin for CSharpPlugin {
         }
         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"))
+            if entry.path().is_dir()
+                && (file_name == Some(&OsString::from("bin"))
+                    || file_name == Some(&OsString::from("obj")))
             {
                 file_util::remove_dir_all(entry.path())?;
             }
@@ -331,7 +337,7 @@ mod test {
     fn init() {
         let _ = env_logger::builder().is_test(true).try_init();
         INIT_RUNNER.call_once(|| {
-            let _ = CSharpPlugin::get_runner_dir().unwrap();
+            let _ = CSharpPlugin::get_or_init_runner_dir().unwrap();
         });
     }
 

plugins/csharp/src/policy.rs

@@ -1,4 +1,4 @@
-//! Student file policy for C#
+//! Student file policy for the C# plugin.
 
 use std::path::{Path, PathBuf};
 use tmc_langs_framework::StudentFilePolicy;
@@ -14,11 +14,12 @@ impl CSharpStudentFilePolicy {
         }
     }
 
-    /// Goes up directories until a bin or obj directory is found
-    fn is_child_of_binary_dir(&self, path: &Path) -> bool {
-        for ancestor in path.ancestors() {
+    /// Goes up directories until a bin or obj directory is found, either indicating that the path is in a binary directory.
+    fn is_child_of_binary_dir(path: &Path) -> bool {
+        // checks each parent directory for bin or obj
+        for ancestor in path.ancestors().skip(1) {
             if let Some(file_name) = ancestor.file_name() {
-                if ancestor.is_dir() && (file_name == "bin" || file_name == "obj") {
+                if file_name == "bin" || file_name == "obj" {
                     return true;
                 }
             }
@@ -28,12 +29,31 @@ impl CSharpStudentFilePolicy {
 }
 
 impl StudentFilePolicy for CSharpStudentFilePolicy {
-    // false for files in bin or obj directories, true for other files in src
+    // false for files in bin or obj directories, true for other files in src.
     fn is_student_source_file(&self, path: &Path) -> bool {
-        path.starts_with("src") && !self.is_child_of_binary_dir(path)
+        path.starts_with("src") && !Self::is_child_of_binary_dir(path)
     }
 
     fn get_config_file_parent_path(&self) -> &Path {
         &self.config_file_parent_path
     }
 }
+
+#[cfg(test)]
+mod test {
+    use super::*;
+
+    #[test]
+    fn file_in_binary_dir_is_not_student_file() {
+        let policy = CSharpStudentFilePolicy::new(PathBuf::new());
+        assert!(!policy.is_student_source_file(Path::new("src/bin/any/file")));
+        assert!(!policy.is_student_source_file(Path::new("obj/any/src/file")));
+    }
+
+    #[test]
+    fn file_in_src_is_student_file() {
+        let policy = CSharpStudentFilePolicy::new(PathBuf::new());
+        assert!(policy.is_student_source_file(Path::new("src/file")));
+        assert!(policy.is_student_source_file(Path::new("src/any/file")));
+    }
+}