Merge branch 'master' of github.com:rage/tmc-langs-rust

Author: Heliozoa

plugins/java/src/maven.rs

@@ -228,27 +228,28 @@ impl JavaPlugin for MavenPlugin {
 #[cfg(test)]
 #[cfg(not(target_os = "macos"))] // issues with maven dependencies
 mod test {
+    //! Maven doesn't like being run in parallel, at least on Windows.
+    //! For now the tests access the MavenPlugin with a function that locks a mutex.
+
     use super::super::{TestCase, TestCaseStatus};
     use super::*;
     use std::fs::{self, File};
+    use std::sync::{Mutex, MutexGuard};
     use tempfile::{tempdir, TempDir};
     use tmc_langs_framework::domain::Strategy;
     use tmc_langs_framework::zip::ZipArchive;
     use walkdir::WalkDir;
 
-    #[cfg(windows)]
-    use std::sync::Once;
-    #[cfg(windows)]
-    static INIT_MAVEN: Once = Once::new();
+    lazy_static::lazy_static! {
+        static ref MAVEN_LOCK: Mutex<()> = Mutex::new(());
+    }
 
     fn init() {
         let _ = env_logger::builder().is_test(true).try_init();
+    }
 
-        // initializes maven in a synchronized manner for all tests
-        #[cfg(windows)]
-        INIT_MAVEN.call_once(|| {
-            MavenPlugin::new().expect("failed to instantiate maven");
-        });
+    fn get_maven() -> (MavenPlugin, MutexGuard<'static, ()>) {
+        (MavenPlugin::new().unwrap(), MAVEN_LOCK.lock().unwrap())
     }
 
     fn copy_test_dir(path: &str) -> TempDir {
@@ -273,13 +274,12 @@ mod test {
     }
 
     #[test]
-    #[cfg(not(target_os = "windows"))] // CI problems
     fn gets_project_class_path() {
         init();
 
         let temp_dir = copy_test_dir("tests/data/maven_exercise");
         let test_path = temp_dir.path();
-        let plugin = MavenPlugin::new().unwrap();
+        let (plugin, _lock) = get_maven();
         let class_path = plugin.get_project_class_path(test_path).unwrap();
         log::debug!("{}", class_path);
         let expected = format!("{0}junit{0}", std::path::MAIN_SEPARATOR);
@@ -295,19 +295,18 @@ mod test {
 
         let temp_dir = copy_test_dir("tests/data/maven_exercise");
         let test_path = temp_dir.path();
-        let plugin = MavenPlugin::new().unwrap();
+        let (plugin, _lock) = get_maven();
         let compile_result = plugin.build(test_path).unwrap();
         assert!(compile_result.status_code.success());
     }
 
     #[test]
-    #[cfg(not(target_os = "windows"))] // CI problems
     fn creates_run_result_file() {
         init();
 
         let temp_dir = copy_test_dir("tests/data/maven_exercise");
         let test_path = temp_dir.path();
-        let plugin = MavenPlugin::new().unwrap();
+        let (plugin, _lock) = get_maven();
         let compile_result = plugin.build(test_path).unwrap();
         let test_run = plugin
             .create_run_result_file(test_path, compile_result)
@@ -338,7 +337,7 @@ mod test {
 
         let temp_dir = copy_test_dir("tests/data/maven_exercise");
         let test_path = temp_dir.path();
-        let plugin = MavenPlugin::new().unwrap();
+        let (plugin, _lock) = get_maven();
         let exercises = plugin
             .scan_exercise(&test_path, "test".to_string(), &mut vec![])
             .unwrap();
@@ -357,7 +356,7 @@ mod test {
 
         let temp_dir = copy_test_dir("tests/data/maven_exercise");
         let test_path = temp_dir.path();
-        let plugin = MavenPlugin::new().unwrap();
+        let (plugin, _lock) = get_maven();
         let checkstyle_result = plugin
             .check_code_style(test_path, Language::from_639_3("fin").unwrap())
             .unwrap()

plugins/python3/src/policy.rs

@@ -22,17 +22,21 @@ impl StudentFilePolicy for Python3StudentFilePolicy {
     }
 
     fn is_student_source_file(&self, path: &Path) -> bool {
-        // .py files in exercise root, and all non-pyc or __pycache__ files in src
-        match path.parent() {
-            Some(parent) => {
-                parent == OsStr::new("src")
-                    && path.extension() != Some(OsStr::new("pyc"))
-                    && !path
-                        .components()
-                        .any(|c| c.as_os_str() == OsStr::new("__pycache__"))
-            }
-            None => path.extension() == Some(OsStr::new("py")),
-        }
+        // all non-pyc or __pycache__ files in src are student source files
+        let in_src = path.starts_with("src");
+        let is_cache_file = path.extension() == Some(OsStr::new("pyc"))
+            || path
+                .components()
+                .any(|c| c.as_os_str() == OsStr::new("__pycache__"));
+
+        // .py files in exercise root are student source files
+        let is_in_project_root = match path.parent() {
+            Some(s) => s.as_os_str().is_empty(),
+            None => true,
+        };
+        let is_py_file = path.extension() == Some(OsStr::new("py"));
+
+        in_src && !is_cache_file || is_in_project_root && is_py_file
     }
 }
 

tmc-langs-framework/src/io/tmc_zip.rs

@@ -21,19 +21,20 @@ pub fn zip<P: StudentFilePolicy>(policy: P, root_directory: &Path) -> Result<Vec
         .filter_entry(|e| !contains_tmcnosubmit(e))
         .filter_map(|e| e.ok())
     {
-        log::trace!("processing {:?}", entry.path());
+        log::trace!("processing {}", entry.path().display());
         if policy.is_student_file(entry.path(), &root_directory, &tmc_project_yml)? {
             let path = root_directory
                 .parent()
                 .map(|p| entry.path().strip_prefix(p).unwrap())
                 .unwrap_or_else(|| entry.path());
             if entry.path().is_dir() {
                 log::trace!("adding directory {}", path.display());
-                writer.add_directory(path.to_string_lossy(), FileOptions::default())?;
+                writer
+                    .add_directory(path_to_zip_compatible_string(path), FileOptions::default())?;
             } else {
                 let bytes = file_util::read_file(entry.path())?;
                 log::trace!("writing file {}", path.display());
-                writer.start_file(path.to_string_lossy(), FileOptions::default())?;
+                writer.start_file(path_to_zip_compatible_string(path), FileOptions::default())?;
                 writer
                     .write_all(&bytes)
                     .map_err(|e| TmcError::ZipWrite(path.to_path_buf(), e))?;
@@ -44,6 +45,18 @@ pub fn zip<P: StudentFilePolicy>(policy: P, root_directory: &Path) -> Result<Vec
     Ok(cursor.into_inner())
 }
 
+// ensures the / separator is used
+fn path_to_zip_compatible_string(path: &Path) -> String {
+    let mut string = String::new();
+    for component in path.components() {
+        if !string.is_empty() {
+            string.push('/');
+        }
+        string.push_str(&*component.as_os_str().to_string_lossy());
+    }
+    string
+}
+
 // todo: remove
 /// Finds a project directory in the given zip and unzips it according to the given student policy. Also cleans unnecessary non-student files.
 ///
@@ -210,10 +223,10 @@ fn contains_tmcnosubmit(entry: &DirEntry) -> bool {
 
 #[cfg(test)]
 mod test {
-    /*
     use super::*;
     use crate::policy::EverythingIsStudentFilePolicy;
     use std::collections::HashSet;
+    use std::fs::{self, *};
     use tempfile::tempdir;
 
     fn init() {
@@ -245,12 +258,9 @@ mod test {
         File::create(missing_file_path).unwrap();
 
         let path = temp.path().join("exercise-name");
-        let zipped = zip(
-            Box::new(EverythingIsStudentFilePolicy::new(path.clone())),
-            &path,
-        )
-        .unwrap();
+        let zipped = zip(EverythingIsStudentFilePolicy::new(path.clone()), &path).unwrap();
         let mut archive = ZipArchive::new(Cursor::new(zipped)).unwrap();
+        assert!(archive.len() > 0);
         for i in 0..archive.len() {
             log::debug!("{:?}", archive.by_index(i).unwrap().name());
         }
@@ -319,5 +329,21 @@ mod test {
         .unwrap();
         assert!(temp.path().join("src").exists());
     }
-    */
+
+    #[cfg(windows)]
+    #[test]
+    fn windows_paths_get_converted() {
+        let win_path = PathBuf::from(r"tests\data\dir");
+        let zipped = zip(
+            EverythingIsStudentFilePolicy::new(win_path.clone()),
+            &win_path,
+        )
+        .unwrap();
+        let mut ziparch = ZipArchive::new(Cursor::new(zipped)).unwrap();
+        assert!(ziparch.len() > 0);
+        for i in 0..ziparch.len() {
+            let file = ziparch.by_index(i).unwrap();
+            assert!(file.name().chars().find(|c| c == &'\\').is_none())
+        }
+    }
 }

tmc-langs-framework/src/policy.rs

@@ -41,7 +41,6 @@ pub trait StudentFilePolicy {
         let root_canon = project_root_path
             .canonicalize()
             .map_err(|e| TmcError::Canonicalize(project_root_path.to_path_buf(), e))?;
-        log::debug!("{} {}", path_canon.display(), root_canon.display());
 
         // the project root path should be considered a student file
         if path_canon == root_canon {

tmc-langs-util/tests/data/course_refresher/valid_exercises/ex1/test/test.py

@@ -1,2 +1,2 @@
 @points("test_point")
-@points("ex_and_test_point")
+@points("ex_and_test_point")