Merge pull request #86 from rage/scanfix

Fix some exercise points being incorrectly given

Author: Heliozoa

plugins/csharp/deps/tmc-csharp-runner-1.1.1.zip

@@ -1,6 +1,7 @@
 //! Contains the CSTestResult type that models the C# test runner result.
 
 use serde::Deserialize;
+use std::collections::HashSet;
 use tmc_langs_framework::domain::TestResult;
 
 /// Test result from the C# test runner.
@@ -14,14 +15,15 @@ pub struct CSTestResult {
     pub error_stack_trace: Vec<String>,
 }
 
-impl From<CSTestResult> for TestResult {
-    fn from(test_result: CSTestResult) -> Self {
+impl CSTestResult {
+    pub fn into_test_result(mut self, failed_points: &HashSet<String>) -> TestResult {
+        self.points.retain(|point| !failed_points.contains(point));
         TestResult {
-            name: test_result.name,
-            successful: test_result.passed,
-            message: test_result.message,
-            exception: test_result.error_stack_trace,
-            points: test_result.points,
+            name: self.name,
+            successful: self.passed,
+            message: self.message,
+            exception: self.error_stack_trace,
+            points: self.points,
         }
     }
 }

plugins/csharp/deps/tmc-csharp-runner-1.1.zip

@@ -2,7 +2,7 @@
 
 use crate::policy::CSharpStudentFilePolicy;
 use crate::{cs_test_result::CSTestResult, CSharpError};
-use std::collections::HashMap;
+use std::collections::{HashMap, HashSet};
 use std::env;
 use std::ffi::{OsStr, OsString};
 use std::io::{BufReader, Cursor, Read, Seek};
@@ -24,7 +24,7 @@ use tmc_langs_framework::{
 };
 use walkdir::WalkDir;
 
-const TMC_CSHARP_RUNNER: &[u8] = include_bytes!("../deps/tmc-csharp-runner-1.1.zip");
+const TMC_CSHARP_RUNNER: &[u8] = include_bytes!("../deps/tmc-csharp-runner-1.1.1.zip");
 
 #[derive(Default)]
 pub struct CSharpPlugin {}
@@ -142,15 +142,19 @@ impl CSharpPlugin {
             .map_err(|e| CSharpError::ParseTestResults(test_results_path.to_path_buf(), e))?;
 
         let mut status = RunStatus::Passed;
+        let mut failed_points = HashSet::new();
         for test_result in &test_results {
             if !test_result.passed {
-                log::info!("C# tests failed");
                 status = RunStatus::TestsFailed;
-                break;
+                failed_points.extend(test_result.points.iter().cloned());
             }
         }
+
         // convert the parsed C# test results into TMC test results
-        let test_results = test_results.into_iter().map(|t| t.into()).collect();
+        let test_results = test_results
+            .into_iter()
+            .map(|t| t.into_test_result(&failed_points))
+            .collect();
         Ok(RunResult {
             status,
             test_results,
@@ -697,4 +701,21 @@ mod test {
         let res = CSharpPlugin::points_parser("@  pOiNtS  (  \"  1  \"  )  ").unwrap();
         assert_eq!(res.1, "1");
     }
+
+    #[test]
+    #[ignore = "requires newer version of C# runner that always includes all points in the tests"]
+    fn doesnt_give_points_unless_all_relevant_exercises_pass() {
+        init();
+
+        let temp = dir_to_temp("tests/data/partially-passing");
+        let plugin = CSharpPlugin::new();
+        let results = plugin.run_tests(temp.path(), &mut vec![]).unwrap();
+        assert_eq!(results.status, RunStatus::TestsFailed);
+        let mut got_point = false;
+        for test in results.test_results {
+            got_point = got_point || test.points.contains(&"1.2".to_string());
+            assert!(!test.points.contains(&"1".to_string()));
+            assert!(!test.points.contains(&"1.1".to_string()));
+        }
+    }
 }

plugins/csharp/src/cs_test_result.rs

@@ -0,0 +1,20 @@
+using System;
+
+namespace TestProject
+{
+    public class Program
+    {
+        public static bool ReturnTrue => true;
+
+        public static bool ReturnNotInput(bool input) => !input;
+        public static string ReturnInputString(string input) => input;
+
+        public static void Main(string[] args)
+        {
+			//BEGIN SOLUTION
+            Console.WriteLine("Hello Home");
+			//END SOLUTION
+			//STUB: Console.WriteLine("Stub");
+        }
+    }
+}

plugins/csharp/src/plugin.rs

@@ -0,0 +1,8 @@
+<Project Sdk="Microsoft.NET.Sdk">
+
+  <PropertyGroup>
+    <OutputType>Exe</OutputType>
+    <TargetFramework>net5.0</TargetFramework>
+  </PropertyGroup>
+
+</Project>

plugins/csharp/tests/data/partially-passing/src/TestProject/Program.cs

@@ -0,0 +1,43 @@
+using System;
+using Xunit;
+using TestProject;
+using TestMyCode.CSharp.API.Attributes;
+
+namespace TestProjectTests
+{
+    [Points("1")]
+    public class ProgramTest
+    {
+        [Fact]
+        [Points("1.1")]
+        public void TestReturnsTrue()
+        {
+            Assert.True(false);
+        }
+
+        [Fact]
+        [Points("1.1")]
+        public void ReturnsNotInput()
+        {
+            Assert.True(true);
+        }
+
+        [Fact]
+        [Points("1.2")]
+        public void ReturnsString()
+        {
+            Assert.True(true);
+        }
+
+        [Fact]
+        public void TestForClassPoint()
+        {
+            Assert.True(true);
+        }
+
+        public void NotAPointTest()
+        {
+            Assert.True(false);
+        }
+    }
+}

plugins/csharp/tests/data/partially-passing/src/TestProject/TestProject.csproj

@@ -0,0 +1,18 @@
+<Project Sdk="Microsoft.NET.Sdk">
+
+  <PropertyGroup>
+    <TargetFramework>net5.0</TargetFramework>
+    <IsPackable>false</IsPackable>
+  </PropertyGroup>
+
+  <ItemGroup>
+    <PackageReference Include="Microsoft.NET.Test.Sdk" Version="16.8.0" />
+    <PackageReference Include="TestMyCode.CSharp.API" Version="1.1" />
+    <PackageReference Include="xunit" Version="2.4.1" />
+    <PackageReference Include="xunit.runner.visualstudio" Version="2.4.3" />
+  </ItemGroup>
+
+  <ItemGroup>
+    <ProjectReference Include="../../src/TestProject/TestProject.csproj" />
+  </ItemGroup>
+</Project>

plugins/csharp/tests/data/partially-passing/test/TestProjectTests/ProgramTest.cs

@@ -4,7 +4,7 @@ use crate::error::PythonError;
 use crate::policy::Python3StudentFilePolicy;
 use crate::python_test_result::PythonTestResult;
 use lazy_static::lazy_static;
-use std::collections::HashMap;
+use std::collections::{HashMap, HashSet};
 use std::env;
 use std::io::BufReader;
 use std::path::{Path, PathBuf};
@@ -176,17 +176,20 @@ impl Python3Plugin {
         let test_results: Vec<PythonTestResult> =
             serde_json::from_reader(BufReader::new(results_file))
                 .map_err(|e| PythonError::Deserialize(test_results_json.to_path_buf(), e))?;
-        let test_results: Vec<TestResult> = test_results
-            .into_iter()
-            .map(PythonTestResult::into_test_result)
-            .collect();
 
         let mut status = RunStatus::Passed;
+        let mut failed_points = HashSet::new();
         for result in &test_results {
-            if !result.successful {
+            if !result.passed {
                 status = RunStatus::TestsFailed;
+                failed_points.extend(result.points.iter().cloned());
             }
         }
+
+        let test_results: Vec<TestResult> = test_results
+            .into_iter()
+            .map(|r| r.into_test_result(&failed_points))
+            .collect();
         Ok(RunResult::new(status, test_results, logs))
     }
 }
@@ -253,7 +256,21 @@ impl LanguagePlugin for Python3Plugin {
                 if test_results_json.exists() {
                     file_util::remove_file(&test_results_json)?;
                 }
-                Ok(parse_res?)
+
+                let mut run_result = parse_res?;
+
+                // remove points associated with any failed tests
+                let mut failed_points = HashSet::new();
+                for test_result in &run_result.test_results {
+                    if !test_result.successful {
+                        failed_points.extend(test_result.points.iter().cloned());
+                    }
+                }
+                for test_result in &mut run_result.test_results {
+                    test_result.points.retain(|p| !failed_points.contains(p));
+                }
+
+                Ok(run_result)
             }
             Err(PythonError::Tmc(TmcError::Command(CommandError::TimeOut {
                 stdout,
@@ -531,10 +548,10 @@ class TestFailing(unittest.TestCase):
 
         let plugin = Python3Plugin::new();
         let run_result = plugin.run_tests(temp_dir.path(), &mut vec![]).unwrap();
+        log::debug!("{:#?}", run_result);
         assert_eq!(run_result.status, RunStatus::TestsFailed);
         assert_eq!(run_result.test_results[0].name, "TestFailing: test_func");
         assert!(!run_result.test_results[0].successful);
-        assert!(run_result.test_results[0].points.contains(&"1.1".into()));
         assert!(run_result.test_results[0].message.starts_with("'a' != 'b'"));
         assert!(!run_result.test_results[0].exception.is_empty());
         assert_eq!(run_result.test_results.len(), 1);
@@ -562,10 +579,10 @@ class TestErroring(unittest.TestCase):
 
         let plugin = Python3Plugin::new();
         let run_result = plugin.run_tests(temp_dir.path(), &mut vec![]).unwrap();
+        log::debug!("{:#?}", run_result);
         assert_eq!(run_result.status, RunStatus::TestsFailed);
         assert_eq!(run_result.test_results[0].name, "TestErroring: test_func");
         assert!(!run_result.test_results[0].successful);
-        assert!(run_result.test_results[0].points.contains(&"1.1".into()));
         assert_eq!(
             run_result.test_results[0].message,
             "name 'doSomethingIllegal' is not defined"
@@ -708,4 +725,42 @@ class TestErroring(unittest.TestCase):
         let res = Python3Plugin::find_project_dir_in_zip(&mut zip);
         assert!(res.is_err());
     }
+
+    #[test]
+    fn doesnt_give_points_unless_all_relevant_exercises_pass() {
+        init();
+
+        let temp_dir = temp_with_tmc();
+        file_to(&temp_dir, "test/__init__.py", "");
+        file_to(
+            &temp_dir,
+            "test/test_file.py",
+            r#"
+import unittest
+from tmc import points
+
+@points('1')
+class TestClass(unittest.TestCase):
+    @points('1.1', '1.2')
+    def test_func1(self):
+        self.assertTrue(False)
+
+    @points('1.1', '1.3')
+    def test_func2(self):
+        self.assertTrue(True)
+"#,
+        );
+
+        let plugin = Python3Plugin::new();
+        let results = plugin.run_tests(temp_dir.path(), &mut vec![]).unwrap();
+        assert_eq!(results.status, RunStatus::TestsFailed);
+        let mut got_point = false;
+        for test in results.test_results {
+            got_point = got_point || test.points.contains(&"1.3".to_string());
+            assert!(!test.points.contains(&"1".to_string()));
+            assert!(!test.points.contains(&"1.1".to_string()));
+            assert!(!test.points.contains(&"1.2".to_string()));
+        }
+        assert!(got_point);
+    }
 }

plugins/csharp/tests/data/partially-passing/test/TestProjectTests/TestProjectTests.csproj

@@ -1,4 +1,5 @@
 use serde::{Deserialize, Serialize};
+use std::collections::HashSet;
 use tmc_langs_framework::domain::TestResult;
 
 #[derive(Debug, Deserialize, Serialize)]
@@ -12,7 +13,8 @@ pub struct PythonTestResult {
 }
 
 impl PythonTestResult {
-    pub fn into_test_result(self) -> TestResult {
+    pub fn into_test_result(mut self, failed_points: &HashSet<String>) -> TestResult {
+        self.points.retain(|point| !failed_points.contains(point));
         TestResult {
             name: parse_test_name(self.name),
             successful: self.passed,