Merge pull request #218 from rage/duplicate-yaml-keys

Fix duplicate keys in .tmcproject.yml after merging

Author: Redande

Cargo.lock

@@ -22,7 +22,7 @@ authors = [
 edition = "2024"
 license = "MIT OR Apache-2.0"
 rust-version = "1.85.0"
-version = "0.39.1"
+version = "0.39.2"
 
 [workspace.dependencies]
 mooc-langs-api = { git = "https://github.com/rage/secret-project-331.git", rev = "24179d597e5f4120649be50b903a9a4e544ea77c" }

Cargo.toml

@@ -137,7 +137,8 @@ impl TmcProjectYml {
     /// Saves the TmcProjectYml to the given directory.
     pub fn save_to_dir(&self, dir: &Path) -> Result<(), TmcError> {
         let config_path = Self::path_in_dir(dir);
-        let mut lock = Lock::file(&config_path, LockOptions::WriteCreate)?;
+        // It is important to truncate the file here, when we save the merged tmcproject.yml files, the exercise folder can already contain a .tmcproject.yml file. If we don't truncate the file before writing, all the merged values will be appended to the file, and duplicate keys will make the file invalid YAML.
+        let mut lock = Lock::file(&config_path, LockOptions::WriteTruncate)?;
         let mut guard = lock.lock()?;
         serde_yaml::to_writer(guard.get_file_mut(), &self)?;
         Ok(())
@@ -374,4 +375,39 @@ mod test {
         let tpy = TmcProjectYml::load(temp.path()).unwrap().unwrap();
         assert_eq!(tpy.tests_timeout_ms, Some(1234));
     }
+
+    #[test]
+    fn saves_truncates_file_not_appends() {
+        init();
+
+        let temp = tempfile::tempdir().unwrap();
+
+        // First save
+        let first = TmcProjectYml {
+            tests_timeout_ms: Some(1000),
+            ..Default::default()
+        };
+        first.save_to_dir(temp.path()).unwrap();
+
+        // Second save with a different value to ensure old contents are not kept
+        let second = TmcProjectYml {
+            tests_timeout_ms: Some(2000),
+            ..Default::default()
+        };
+        second.save_to_dir(temp.path()).unwrap();
+
+        // Read raw YAML and ensure tests_timeout_ms occurs only once
+        let yaml_path = TmcProjectYml::path_in_dir(temp.path());
+        let yaml = std::fs::read_to_string(&yaml_path).unwrap();
+        let occurrences = yaml.matches("tests_timeout_ms").count();
+        assert_eq!(
+            occurrences, 1,
+            "YAML should contain the key only once: {}",
+            yaml
+        );
+
+        // And the file is still valid YAML after the second save
+        let parsed = TmcProjectYml::load(temp.path()).unwrap().unwrap();
+        assert_eq!(parsed.tests_timeout_ms, Some(2000));
+    }
 }

crates/tmc-langs-framework/src/tmc_project_yml.rs

@@ -712,4 +712,41 @@ mod test {
         assert_eq!(tpyb.tests_timeout_ms, Some(1234));
         assert_eq!(tpyb.fail_on_valgrind_error, Some(false));
     }
+
+    #[test]
+    fn merges_tmcproject_configs_exercise_overrides_root() {
+        init();
+
+        let temp = tempfile::tempdir().unwrap();
+        let exercise_path = PathBuf::from("exercise");
+        let exercise_dir = temp.path().join(&exercise_path);
+        file_util::create_dir(&exercise_dir).unwrap();
+
+        // Root config has tests_timeout_ms: 1000
+        let root = TmcProjectYml {
+            tests_timeout_ms: Some(1000),
+            fail_on_valgrind_error: Some(true),
+            ..Default::default()
+        };
+
+        // Exercise config has tests_timeout_ms: 2000 (should override root)
+        let exercise_config = TmcProjectYml {
+            tests_timeout_ms: Some(2000),
+            ..Default::default()
+        };
+        exercise_config.save_to_dir(&exercise_dir).unwrap();
+
+        let exercise_dirs = vec![exercise_path];
+
+        let dirs_configs =
+            get_and_merge_tmcproject_configs(Some(root), temp.path(), exercise_dirs).unwrap();
+
+        let (_, merged_config) = &dirs_configs[0];
+        let merged_config = merged_config.as_ref().unwrap();
+
+        // Exercise values should override root values when both are present
+        assert_eq!(merged_config.tests_timeout_ms, Some(2000));
+        // Root values should be inherited when exercise doesn't have that field
+        assert_eq!(merged_config.fail_on_valgrind_error, Some(true));
+    }
 }