Merge pull request #211 from rage/msg

Fix serialization issue and improve error message

Author: Heliozoa

Diff for: Cargo.lock

@@ -22,7 +22,7 @@ authors = [
 edition = "2021"
 license = "MIT OR Apache-2.0"
 rust-version = "1.70.0"
-version = "0.33.0"
+version = "0.34.0"
 
 [workspace.dependencies]
 mooc-langs-api = { git = "https://github.com/rage/secret-project-331.git", rev = "778a5820a8b7422cbf9f1c786da437833ced5ae9" }
@@ -39,7 +39,7 @@ tmc-langs-util = { path = "crates/tmc-langs-util" }
 tmc-mooc-client = { path = "crates/tmc-mooc-client" }
 tmc-server-mock = { path = "crates/helpers/tmc-server-mock" }
 tmc-testmycode-client = { path = "crates/tmc-testmycode-client" }
-ts-rs = { git = "https://github.com/Heliozoa/ts-rs.git", rev = "07712bf04007472aeeb065091261b3b64c019381" }
+ts-rs = { git = "https://github.com/Heliozoa/ts-rs.git", rev = "a08135ffad38a8fce2b9501b344d96e609f2c46f" }
 
 # [patch.'https://github.com/Heliozoa/ts-rs.git']
 # ts-rs = { path = "../ts-rs/ts-rs" }

Diff for: Cargo.toml

@@ -12,7 +12,7 @@ tmc-langs-util.workspace = true
 
 dirs = "5.0.1"
 flate2 = "1.0.22"
-j4rs = "=0.16.0" # specific version to match the jar
+j4rs = "=0.17.1" # specific version to match the jar
 log = "0.4.14"
 once_cell = "1.9.0"
 serde = { version = "1.0.136", features = ["derive"] }

Diff for: crates/plugins/java/Cargo.toml

@@ -29,7 +29,7 @@ const SEPARATOR: &str = ":";
 const TMC_JUNIT_RUNNER_BYTES: &[u8] = include_bytes!("../deps/tmc-junit-runner-0.2.8.jar");
 const TMC_CHECKSTYLE_RUNNER_BYTES: &[u8] =
     include_bytes!("../deps/tmc-checkstyle-runner-3.0.3-20200520.064542-3.jar");
-const J4RS_BYTES: &[u8] = include_bytes!("../deps/j4rs-0.16.0-jar-with-dependencies.jar");
+const J4RS_BYTES: &[u8] = include_bytes!("../deps/j4rs-0.17.1-jar-with-dependencies.jar");
 
 struct JvmWrapper {
     jvm: Jvm,

Diff for: crates/plugins/java/deps/j4rs-0.16.0-jar-with-dependencies.jar renamed to crates/plugins/java/deps/j4rs-0.17.1-jar-with-dependencies.jar

@@ -163,52 +163,61 @@ impl LanguagePlugin for MakePlugin {
             });
         }
 
+        // try to clean old log file if any
+        let base_test_path = path.join("test");
+        let valgrind_log_path = base_test_path.join("valgrind.log");
+        let _ = file_util::remove_file(&valgrind_log_path);
+
         // try to run valgrind
         let mut ran_valgrind = true;
         let valgrind_run = self.run_tests_with_valgrind(path, true);
         let output = match valgrind_run {
             Ok(output) => output,
-            Err(error) => match error {
-                MakeError::Tmc(TmcError::Command(command_error)) => {
-                    match command_error {
-                        CommandError::Popen(_, PopenError::IoError(io_error))
-                        | CommandError::FailedToRun(_, PopenError::IoError(io_error))
-                            if io_error.kind() == io::ErrorKind::PermissionDenied =>
-                        {
-                            // failed due to lacking permissions, try to clean and rerun
-                            self.clean(path)?;
-                            match self.run_tests_with_valgrind(path, false) {
-                                Ok(output) => output,
-                                Err(err) => {
-                                    log::error!(
+            Err(error) => {
+                if let Ok(valgrind_log) = file_util::read_file_to_string_lossy(&valgrind_log_path) {
+                    log::warn!("Failed to run valgrind but a valgrind.log exists: {valgrind_log}");
+                }
+                match error {
+                    MakeError::Tmc(TmcError::Command(command_error)) => {
+                        match command_error {
+                            CommandError::Popen(_, PopenError::IoError(io_error))
+                            | CommandError::FailedToRun(_, PopenError::IoError(io_error))
+                                if io_error.kind() == io::ErrorKind::PermissionDenied =>
+                            {
+                                // failed due to lacking permissions, try to clean and rerun
+                                self.clean(path)?;
+                                match self.run_tests_with_valgrind(path, false) {
+                                    Ok(output) => output,
+                                    Err(err) => {
+                                        log::error!(
                                         "Running with valgrind failed after trying to clean! {}",
                                         err
                                     );
-                                    ran_valgrind = false;
-                                    log::info!("Running without valgrind");
-                                    self.run_tests_with_valgrind(path, false)?
+                                        ran_valgrind = false;
+                                        log::info!("Running without valgrind");
+                                        self.run_tests_with_valgrind(path, false)?
+                                    }
                                 }
                             }
-                        }
-                        _ => {
-                            ran_valgrind = false;
-                            log::info!("Running without valgrind");
-                            self.run_tests_with_valgrind(path, false)?
+                            _ => {
+                                ran_valgrind = false;
+                                log::info!("Running without valgrind");
+                                self.run_tests_with_valgrind(path, false)?
+                            }
                         }
                     }
+                    MakeError::RunningTestsWithValgrind(..) => {
+                        ran_valgrind = false;
+                        log::info!("Running without valgrind");
+                        self.run_tests_with_valgrind(path, false)?
+                    }
+                    err => {
+                        log::warn!("unexpected error {:?}", err);
+                        return Err(err.into());
+                    }
                 }
-                MakeError::RunningTestsWithValgrind(..) => {
-                    ran_valgrind = false;
-                    log::info!("Running without valgrind");
-                    self.run_tests_with_valgrind(path, false)?
-                }
-                err => {
-                    log::warn!("unexpected error {:?}", err);
-                    return Err(err.into());
-                }
-            },
+            }
         };
-        let base_test_path = path.join("test");
 
         // fails on valgrind by default
         let fail_on_valgrind_error = match TmcProjectYml::load_or_default(path) {
@@ -218,8 +227,7 @@ impl LanguagePlugin for MakePlugin {
 
         // valgrind logs are only interesting if fail on valgrind error is on
         let valgrind_log = if ran_valgrind && fail_on_valgrind_error {
-            let valgrind_path = base_test_path.join("valgrind.log");
-            Some(ValgrindLog::from(&valgrind_path)?)
+            Some(ValgrindLog::from(&valgrind_log_path)?)
         } else {
             None
         };

Diff for: crates/plugins/java/src/lib.rs

@@ -25,7 +25,7 @@ serde = "1.0.136"
 serde_json = "1.0.78"
 tempfile = "3.3.0"
 thiserror = "1.0.30"
-toml = "0.7.2"
+toml = "0.8.2"
 url = "2.2.2"
 uuid = { version = "1.3.4", features = ["v4"] }
 walkdir = "2.3.2"

Diff for: crates/plugins/make/src/plugin.rs

@@ -20,7 +20,7 @@ serde_path_to_error = "0.1.7"
 serde_yaml = "0.9.10"
 tempfile = "3.3.0"
 thiserror = "1.0.30"
-toml = "0.7.2"
+toml = "0.8.2"
 type-map = "0.5.0"
 walkdir = "2.3.2"
 

Diff for: crates/tmc-langs-cli/Cargo.toml

@@ -38,15 +38,15 @@ shellwords = "1.1.0"
 tar = "0.4.38"
 tempfile = "3.3.0"
 thiserror = "1.0.30"
-toml = "0.7.2"
+toml = "0.8.2"
 url = "2.2.2"
 uuid = { version = "1.3.4", features = ["v4"] }
 walkdir = "2.3.2"
 zip = "0.6.3"
 zstd = "0.12.3"
 
 [target.'cfg(unix)'.dependencies]
-nix = "0.26.2"
+nix = { version = "0.27.1", features = ["fs"] }
 
 [dev-dependencies]
 chrono = "0.4.19"

Diff for: crates/tmc-langs-util/Cargo.toml

@@ -126,11 +126,9 @@ impl TestMyCodeClient {
             return Err(TestMyCodeClientError::AlreadyAuthenticated);
         }
 
-        let auth_url = self
-            .0
-            .root_url
-            .join("/oauth/token")
-            .map_err(|e| TestMyCodeClientError::UrlParse("oauth/token".to_string(), e))?;
+        let auth_url = self.0.root_url.join("/oauth/token").map_err(|e| {
+            TestMyCodeClientError::UrlParse(self.0.root_url.to_string() + "/oauth/token", e)
+        })?;
 
         let credentials = api_v8::get_credentials(self, client_name)?;
 

Diff for: crates/tmc-langs/Cargo.toml

@@ -437,7 +437,7 @@ pub struct SubmissionFeedbackQuestion {
     pub kind: SubmissionFeedbackKind,
 }
 
-#[derive(Debug, PartialEq, Eq, JsonSchema)]
+#[derive(Debug, Clone, Copy, PartialEq, Eq, JsonSchema)]
 #[cfg_attr(feature = "ts-rs", derive(ts_rs::TS))]
 pub enum SubmissionFeedbackKind {
     Text,
@@ -449,7 +449,22 @@ impl<'de> Deserialize<'de> for SubmissionFeedbackKind {
     where
         D: Deserializer<'de>,
     {
-        deserializer.deserialize_string(SubmissionFeedbackKindVisitor {})
+        let wrapper = SubmissionFeedbackKindWrapper::deserialize(deserializer)?;
+        let kind = match wrapper {
+            SubmissionFeedbackKindWrapper::String(string) => match string {
+                SubmissionFeedbackKindString::Text => Self::Text,
+                SubmissionFeedbackKindString::IntRange { lower, upper } => {
+                    Self::IntRange { lower, upper }
+                }
+            },
+            SubmissionFeedbackKindWrapper::Derived(derived) => match derived {
+                SubmissionFeedbackKindDerived::Text => Self::Text,
+                SubmissionFeedbackKindDerived::IntRange { lower, upper } => {
+                    Self::IntRange { lower, upper }
+                }
+            },
+        };
+        Ok(kind)
     }
 }
 
@@ -458,19 +473,53 @@ impl Serialize for SubmissionFeedbackKind {
     where
         S: Serializer,
     {
-        let s = match self {
-            Self::Text => "text".to_string(),
-            Self::IntRange { lower, upper } => format!("intrange[{lower}..{upper}]"),
+        let derived = match self {
+            Self::Text => SubmissionFeedbackKindDerived::Text,
+            Self::IntRange { lower, upper } => SubmissionFeedbackKindDerived::IntRange {
+                lower: *lower,
+                upper: *upper,
+            },
         };
-        serializer.serialize_str(&s)
+        derived.serialize(serializer)
+    }
+}
+
+// wraps the two stringly typed and rusty versions of the kind
+#[derive(Debug, Clone, Copy, Deserialize)]
+#[serde(untagged)]
+enum SubmissionFeedbackKindWrapper {
+    Derived(SubmissionFeedbackKindDerived),
+    String(SubmissionFeedbackKindString),
+}
+
+// uses derived serde impls
+#[derive(Debug, Clone, Copy, Deserialize, Serialize)]
+enum SubmissionFeedbackKindDerived {
+    Text,
+    IntRange { lower: u32, upper: u32 },
+}
+
+// the stringly typed "text" or "intrange" that comes from the server
+#[derive(Debug, Clone, Copy)]
+enum SubmissionFeedbackKindString {
+    Text,
+    IntRange { lower: u32, upper: u32 },
+}
+
+impl<'de> Deserialize<'de> for SubmissionFeedbackKindString {
+    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
+    where
+        D: Deserializer<'de>,
+    {
+        deserializer.deserialize_string(SubmissionFeedbackKindStringVisitor {})
     }
 }
 
-struct SubmissionFeedbackKindVisitor {}
+struct SubmissionFeedbackKindStringVisitor {}
 
 // parses "text" into Text, and "intrange[x..y]" into IntRange {lower: x, upper: y}
-impl<'de> Visitor<'de> for SubmissionFeedbackKindVisitor {
-    type Value = SubmissionFeedbackKind;
+impl<'de> Visitor<'de> for SubmissionFeedbackKindStringVisitor {
+    type Value = SubmissionFeedbackKindString;
 
     fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result {
         formatter.write_str("\"text\" or \"intrange[x..y]\"")
@@ -485,7 +534,7 @@ impl<'de> Visitor<'de> for SubmissionFeedbackKindVisitor {
             Lazy::new(|| Regex::new(r#"intrange\[(\d+)\.\.(\d+)\]"#).unwrap());
 
         if value == "text" {
-            Ok(SubmissionFeedbackKind::Text)
+            Ok(SubmissionFeedbackKindString::Text)
         } else if let Some(captures) = RANGE.captures(value) {
             let lower = &captures[1];
             let lower = u32::from_str(lower).map_err(|e| {
@@ -495,7 +544,7 @@ impl<'de> Visitor<'de> for SubmissionFeedbackKindVisitor {
             let upper = u32::from_str(upper).map_err(|e| {
                 E::custom(format!("error parsing intrange upper bound {upper}: {e}"))
             })?;
-            Ok(SubmissionFeedbackKind::IntRange { lower, upper })
+            Ok(SubmissionFeedbackKindString::IntRange { lower, upper })
         } else {
             Err(E::custom("expected \"text\" or \"intrange[x..y]\""))
         }
@@ -601,7 +650,7 @@ mod test {
     }
 
     #[test]
-    fn feedback_kind_de() {
+    fn feedback_kind_de_server() {
         init();
 
         let text = serde_json::json!("text");
@@ -619,17 +668,31 @@ mod test {
         }
     }
 
+    #[test]
+    fn feedback_kind_de_rust() {
+        init();
+
+        let original = SubmissionFeedbackKind::Text;
+        let json = serde_json::to_string(&original).unwrap();
+        let deserialized: SubmissionFeedbackKind = deserialize::json_from_str(&json).unwrap();
+        assert_eq!(deserialized, original);
+
+        let original = SubmissionFeedbackKind::IntRange { lower: 1, upper: 5 };
+        let json = serde_json::to_string(&original).unwrap();
+        let deserialized: SubmissionFeedbackKind = deserialize::json_from_str(&json).unwrap();
+        assert_eq!(deserialized, original);
+    }
+
     #[test]
     fn feedback_kind_se() {
         init();
-        use serde_json::Value;
 
-        let text = SubmissionFeedbackKind::Text;
-        let text = serde_json::to_value(&text).unwrap();
-        assert_eq!(text, Value::String("text".to_string()));
+        let original = SubmissionFeedbackKind::Text;
+        let json = serde_json::to_string(&original).unwrap();
+        assert_eq!(json, r#""Text""#);
 
-        let range = SubmissionFeedbackKind::IntRange { lower: 1, upper: 5 };
-        let range = serde_json::to_value(&range).unwrap();
-        assert_eq!(range, Value::String("intrange[1..5]".to_string()));
+        let original = SubmissionFeedbackKind::IntRange { lower: 1, upper: 5 };
+        let json = serde_json::to_string(&original).unwrap();
+        assert_eq!(json, r#"{"IntRange":{"lower":1,"upper":5}}"#);
     }
 }

Diff for: crates/tmc-testmycode-client/src/client.rs

@@ -4,7 +4,7 @@ CMD="${*:-cargo build && cp /build/target/debug/tmc-langs-cli /build/out/}"
 export DOCKER_BUILDKIT=1
 docker build . -f docker/Dockerfile -t tmc-langs-rust
 if [ "$CMD" = "interactive" ]; then
-    docker run -it --rm -v "$PWD":/build/out tmc-langs-rust bash
+    docker run --ulimit nofile=1024:1024 -it --rm -v "$PWD":/build/out tmc-langs-rust bash
 else
-    docker run --rm -v "$PWD":/build/out tmc-langs-rust bash -c "$CMD"
+    docker run --ulimit nofile=1024:1024 --rm -v "$PWD":/build/out tmc-langs-rust bash -c "$CMD"
 fi;