Fix path issues and point parsing (#90)

* Fix point parsing regex

Points with a '.' were not parsed

* Fix path issues, improve error handling/logging

* Tests for point parsing

* Fix checkstyle

* Replace RuntimeExceptions with RunResults

tmc-langs-qmake/src/main/java/fi/helsinki/cs/tmc/langs/qmake/QmakePlugin.java

@@ -31,6 +31,7 @@
 import java.nio.charset.StandardCharsets;
 import java.nio.file.FileVisitResult;
 import java.nio.file.Files;
+import java.nio.file.LinkOption;
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.nio.file.SimpleFileVisitor;
@@ -49,9 +50,9 @@ public final class QmakePlugin extends AbstractLanguagePlugin {
 
     private static final Path TMC_TEST_RESULTS = Paths.get("tmc_test_results.xml");
 
-    // Finds pattern 'POINT(exercise_name, 1)'
+    // Finds pattern 'POINT(exercise_name, 1.1)'
     private static final Pattern POINT_PATTERN
-            = Pattern.compile("POINT\\(\\s*(\\w+),\\s*(\\w+)\\s*\\)\\s*;");
+            = Pattern.compile("POINT\\(\\s*(\\w+),\\s*([^\\s|\\)]+)\\s*\\)\\s*;");
     // Pattern to find comments
     private static final Pattern COMMENT_PATTERN
             = Pattern.compile("(^[^\"\\r\\n]*\\/\\*{1,2}.*?\\*\\/"
@@ -79,8 +80,9 @@ public String getPluginName() {
      * Resolve the exercise .pro file from exercise directory. The file should
      * be named after the directory.
      */
-    private Path getProFile(Path basePath) {
-        return Paths.get(basePath.toString() + "/" + basePath.getFileName() + ".pro");
+    private Path getProFile(Path basePath) throws IOException {
+        Path fullPath = basePath.toRealPath(LinkOption.NOFOLLOW_LINKS);
+        return fullPath.resolve(fullPath.getFileName() + ".pro");
     }
 
     @Override
@@ -101,7 +103,11 @@ public Optional<ExerciseDesc> scanExercise(Path path, String exerciseName) {
 
     @Override
     public boolean isExerciseTypeCorrect(Path path) {
-        return Files.isRegularFile(getProFile(path));
+        try {
+            return Files.isRegularFile(getProFile(path));
+        } catch (IOException e) {
+            return false;
+        }
     }
 
     /**
@@ -121,31 +127,45 @@ protected StudentFilePolicy getStudentFilePolicy(Path projectPath) {
 
     @Override
     public RunResult runTests(Path path) {
-        Path shadowDir = makeShadowBuildDir(path);
+        Path fullPath;
+        try {
+            fullPath = path.toRealPath(LinkOption.NOFOLLOW_LINKS);
+        } catch (IOException e) {
+            log.error("Exercise directory not found", e);
+            return filledFailure(Status.GENERIC_ERROR, "Exercise directory not found");
+        }
+
+        Path shadowDir;
+        try {
+            shadowDir = makeShadowBuildDir(fullPath);
+        } catch (IOException e) {
+            log.error("Preparing exercise failed", e);
+            return filledFailure(Status.GENERIC_ERROR, "Could not create build directory");
+        }
 
         try {
             ProcessResult qmakeBuild = buildWithQmake(shadowDir);
             if (qmakeBuild.statusCode != 0) {
                 log.error("Building project with qmake failed: {}", qmakeBuild.errorOutput);
-                return filledFailure(qmakeBuild.errorOutput);
+                return filledFailure(Status.COMPILE_FAILED, qmakeBuild.errorOutput);
             }
         } catch (IOException | InterruptedException e) {
             log.error("Building project with qmake failed", e);
-            throw new QmakeBuildException(e);
+            return filledFailure(Status.GENERIC_ERROR, "Building project with qmake failed");
         }
 
         try {
             ProcessResult makeBuild = buildWithMake(shadowDir);
             if (makeBuild.statusCode != 0) {
                 log.error("Building project with make failed: {}", makeBuild.errorOutput);
-                return filledFailure(makeBuild.errorOutput);
+                return filledFailure(Status.COMPILE_FAILED, makeBuild.errorOutput);
             }
         } catch (IOException | InterruptedException e) {
             log.error("Building project with make failed", e);
-            throw new QmakeBuildException(e);
+            return filledFailure(Status.GENERIC_ERROR, "Building project with make failed");
         }
 
-        Path testResults = path.resolve(TMC_TEST_RESULTS);
+        Path testResults = shadowDir.resolve(TMC_TEST_RESULTS);
 
         String target = "check";
         String config = "TESTARGS=-o " + testResults.toString() + ",xml";
@@ -158,11 +178,11 @@ public RunResult runTests(Path path) {
 
             if (!Files.exists(testResults)) {
                 log.error("Failed to get test output at {}", testResults);
-                return filledFailure(testRun.output);
+                return filledFailure(Status.GENERIC_ERROR, testRun.output);
             }
         } catch (IOException | InterruptedException e) {
             log.error("Testing with make check failed", e);
-            throw new QmakeBuildException(e);
+            return filledFailure(Status.GENERIC_ERROR, "Testing with make check failed");
         }
 
         QTestResultParser parser = new QTestResultParser();
@@ -185,11 +205,18 @@ public Map<File, List<ValidationError>> getValidationErrors() {
         };
     }
 
-    private Path makeShadowBuildDir(Path dir) {
-        File buildDir = dir.resolve("build").toFile();
+    private Path makeShadowBuildDir(Path dir) throws IOException {
+        Path shadowPath = dir.resolve("build");
+        if (Files.exists(shadowPath)) {
+            log.info("Shadow dir already exists at {}", shadowPath);
+            return shadowPath;
+        }
+
+        File buildDir = shadowPath.toFile();
+
         log.info("Making shadow build dir to {}", buildDir.toPath());
         if (!buildDir.mkdirs()) {
-            throw new RuntimeException(
+            throw new IOException(
                     "Unable to create shadow build directory: "
                     + buildDir.toPath());
         }
@@ -231,13 +258,13 @@ private ProcessResult run(String[] command, Path dir) throws IOException, Interr
         return runner.call();
     }
 
-    private RunResult filledFailure(String output) {
+    private RunResult filledFailure(Status status, String output) {
         byte[] errorOutput = output.getBytes(StandardCharsets.UTF_8);
         ImmutableMap<String, byte[]> logs
                 = new ImmutableMap.Builder()
                 .put(SpecialLogs.COMPILER_OUTPUT, errorOutput)
                 .<String, byte[]>build();
-        return new RunResult(Status.COMPILE_FAILED, ImmutableList.<TestResult>of(), logs);
+        return new RunResult(status, ImmutableList.<TestResult>of(), logs);
     }
 
     /**

tmc-langs-qmake/src/test/java/fi/helsinki/cs/tmc/langs/qmake/QmakePluginTest.java

@@ -7,6 +7,7 @@
 import fi.helsinki.cs.tmc.langs.domain.ExerciseDesc;
 import fi.helsinki.cs.tmc.langs.domain.RunResult;
 import fi.helsinki.cs.tmc.langs.domain.TestDesc;
+import fi.helsinki.cs.tmc.langs.domain.TestResult;
 import fi.helsinki.cs.tmc.langs.utils.TestUtils;
 
 import com.google.common.base.Optional;
@@ -20,6 +21,7 @@ public class QmakePluginTest {
     private final QmakePlugin qmakePlugin;
 
     public QmakePluginTest() {
+        TestUtils.skipIfNotAvailable("qmake");
         qmakePlugin = new QmakePlugin();
     }
 
@@ -49,6 +51,37 @@ public void testQtTestsPassingWithMultipleLib() {
                 runTests("passing_multiple_lib"));
     }
 
+    @Test
+    public void testQtTestsPassingWithMultipleLibSamePoints() {
+        RunResult result = runTests(TestUtils.getPath(getClass(),
+                "passing_multiple_lib_same_point"));
+        assertEquals("Tests passing with multiple libraries and same points",
+                RunResult.Status.PASSED, result.status);
+
+        assertEquals(3, result.testResults.size());
+
+        TestResult test1 = result.testResults.get(0);
+        assertEquals(test1.getName(), "test_function_one_here");
+        assertTrue(test1.isSuccessful());
+        assertEquals(2, test1.points.size());
+        assertEquals("testPoint1", test1.points.get(0));
+        assertEquals("testPoint1.1", test1.points.get(1));
+
+        TestResult test2 = result.testResults.get(1);
+        assertEquals(test2.getName(), "test_function_two_here");
+        assertTrue(test2.isSuccessful());
+        assertEquals(2, test2.points.size());
+        assertEquals("testPoint1", test2.points.get(0));
+        assertEquals("testPoint1.2", test2.points.get(1));
+
+        TestResult test3 = result.testResults.get(2);
+        assertEquals(test3.getName(), "test_function_two_here_2");
+        assertTrue(test3.isSuccessful());
+        assertEquals(2, test3.points.size());
+        assertEquals("testPoint1", test3.points.get(0));
+        assertEquals("testPoint1.3", test3.points.get(1));
+    }
+
     @Test
     public void testQtBuildFailingWithSingleLib() {
         assertEquals("Student source not compiling with compiling single lib",
@@ -58,9 +91,24 @@ public void testQtBuildFailingWithSingleLib() {
 
     @Test
     public void testQtFailingSingleLib() {
+        RunResult result = runTests(TestUtils.getPath(getClass(), "failing_single_lib"));
         assertEquals("Tests failing with single library",
                 RunResult.Status.TESTS_FAILED,
-                runTests("failing_single_lib"));
+                result.status);
+
+        assertEquals(2, result.testResults.size());
+
+        TestResult test1 = result.testResults.get(0);
+        assertEquals(test1.getName(), "test_function_one_here");
+        assertTrue(test1.isSuccessful());
+        assertEquals(1, test1.points.size());
+        assertEquals("1", test1.points.get(0));
+
+        TestResult test2 = result.testResults.get(1);
+        assertEquals(test2.getName(), "test_function_two_here");
+        assertFalse(test2.isSuccessful());
+        assertEquals(1, test2.points.size());
+        assertEquals("2", test2.points.get(0));
     }
 
     @Test
@@ -127,6 +175,38 @@ public void testScanExerciseWithPassingSamePoints() {
         assertEquals("2", test2.points.get(0));
     }
 
+    @Test
+    public void testScanExerciseWithPartialPoints() {
+        Optional<ExerciseDesc> optional = scanExercise("passing_multiple_lib_same_point");
+        assertTrue(optional.isPresent());
+
+        ExerciseDesc desc = optional.get();
+
+        assertEquals("passing_multiple_lib_same_point", desc.name);
+
+        assertEquals(3, desc.tests.size());
+        TestDesc test1 = desc.tests.get(2);
+        TestDesc test2 = desc.tests.get(0);
+        TestDesc test3 = desc.tests.get(1);
+
+        assertEquals("test_function_one_here", test1.name);
+        assertEquals("test_function_two_here", test2.name);
+        assertEquals("test_function_two_here_2", test3.name);
+
+        assertEquals(2, test1.points.size());
+        assertEquals("testPoint1", test1.points.get(0));
+        assertEquals("testPoint1.1", test1.points.get(1));
+
+        assertEquals(2, test2.points.size());
+        assertEquals("testPoint1", test2.points.get(0));
+        assertEquals("testPoint1.2", test2.points.get(1));
+
+        assertEquals(2, test2.points.size());
+        assertEquals("testPoint1", test3.points.get(0));
+        assertEquals("testPoint1.3", test3.points.get(1));
+
+    }
+
     @Test
     public void testScanExerciseWithFailingSamePoints() {
         Optional<ExerciseDesc> optional = scanExercise("failing_single_lib_same_point");
@@ -179,6 +259,10 @@ public void testScanExerciseWithNonexistentProject() {
         assertFalse(optional.isPresent());
     }
 
+    private RunResult runTests(Path testExercise) {
+        return qmakePlugin.runTests(testExercise);
+    }
+
     private RunResult.Status runTests(String testExercise) {
         return qmakePlugin.runTests(TestUtils.getPath(getClass(), testExercise)).status;
     }

tmc-langs-qmake/src/test/resources/passing_multiple_lib_same_point/passing_multiple_lib_same_point.pro

@@ -1,12 +1,12 @@
 TEMPLATE = subdirs
 SUBDIRS += \
-	test_case_app \
-	test_case_test_runner \
+        src \
+        test_case_test_runner \
         test_case_lib \
         test_case_lib2
 
-        test_case_app.depends = test_case_lib \
-                                test_case_lib2
+        src.depends = test_case_lib \
+                      test_case_lib2
         test_case_test_runner.depends = test_case_lib \
                                         test_case_lib2
 

tmc-langs-qmake/src/test/resources/passing_multiple_lib_same_point/test_case_test_runner/test_case_test_runner.cpp

@@ -15,7 +15,8 @@ void test_case_test_runner::test_function_one_here() {
 
     test_case_lib test_case;
 
-    POINT(test_function_one_here, 1);
+    POINT(test_function_one_here, testPoint1);
+    POINT(test_function_one_here, testPoint1.1);
     QVERIFY(!strcmp(test_case.piece_of_string(), "Hello, world!"));
 
 }
@@ -24,15 +25,17 @@ void test_case_test_runner::test_function_two_here() {
 
     test_case_lib2 test_case;
 
-     POINT(test_function_two_here, 2);
-     QVERIFY(test_case.adding_ints(666, 1337) == 2003);
+    POINT(test_function_two_here, testPoint1);
+    POINT(test_function_two_here, testPoint1.2);
+    QVERIFY(test_case.adding_ints(666, 1337) == 2003);
 
 }
 
 void test_case_test_runner::test_function_two_here_2() {
 
     test_case_lib2 test_case;
 
-    POINT(test_function_two_here_2, 1);
+    POINT(test_function_two_here_2, testPoint1);
+    POINT(test_function_two_here_2, testPoint1.3);
     QVERIFY(test_case.adding_ints(-341, 428) == 87);
 }