Correct issue with generated files not being found (#132)

* Delay sandbox resolution til env build

* fix issue when generated file name has __ in it

* fix coverage hole

source/autograder_platform/Executors/Environment.py

@@ -132,7 +132,7 @@ class ExecutionEnvironment(Generic[ImplEnvironment, ImplResults]):
 def getResults(environment: ExecutionEnvironment[ImplEnvironment, ImplResults]) -> Results[ImplResults]:
     """
     This method gets the results from the environment. 
-    If they aren't populated then an assertion error is raised. 
+    If they aren't populated, then an assertion error is raised.
     Results should be accessed directly from the Results object by their key name.
     :param environment: the execution environment.
     :raises AssertionError: if the results aren't populated.
@@ -162,8 +162,7 @@ def __init__(self):
         # windows doesn't clean out temp files by default (for compatibility reasons),
         # so we are going to be good boys and girls, and delete the *contents* of this folder at the end of a run.
         # however, as it is a different folder each time, we are going to silently fail.
-        self.tempLocation = tempfile.mkdtemp(prefix="autograder_")
-        self.environment = ExecutionEnvironment[ImplEnvironment, ImplResults](sandbox_location=self.tempLocation)
+        self.environment = ExecutionEnvironment[ImplEnvironment, ImplResults]()
         self.dataRoot = "."
 
     def setDataRoot(self: Builder, dataRoot: str) -> Builder:
@@ -216,7 +215,6 @@ def addFile(self: Builder, fileSrc: str, fileDest: str) -> Builder:
             fileSrc = fileSrc[2:]
 
         fileSrc = os.path.join(self.dataRoot, fileSrc)
-        fileDest = os.path.join(self.environment.sandbox_location, fileDest)
 
         self.environment.files[fileSrc] = fileDest
 
@@ -247,6 +245,14 @@ def setImplEnvironment(self: Builder, implEnvironmentBuilder: Type[ImplEnvironme
 
         return self
 
+    def _setAndResolveSandbox(self):
+        tempLocation = tempfile.mkdtemp(prefix="autograder_")
+
+        self.environment.sandbox_location = tempLocation
+
+        for src, dest in self.environment.files.items():
+            self.environment.files[src] = os.path.join(self.environment.sandbox_location, dest)
+
     @staticmethod
     def _validate(environment: ExecutionEnvironment):
         # For now this only validating that the files actually exist
@@ -271,6 +277,9 @@ def build(self) -> ExecutionEnvironment[ImplEnvironment, ImplResults]:
 
         :returns: The build environment
         """
+
+        self._setAndResolveSandbox()
+
         self._validate(self.environment)
 
         return self.environment

source/autograder_platform/Executors/common.py

@@ -35,21 +35,21 @@ def detectFileSystemChanges(inFiles: Iterable[str], directoryToCheck: str) -> Di
 
     outputFiles: Dict[str, str] = {}
 
-    # This ignores sub folders
-    for file in files:
-        if os.path.isdir(file):
+    # This ignores subfolders
+    for path in files:
+        if os.path.isdir(path):
             continue
 
-        if "__" in file:
+        if "__" in os.path.basename(path):
             continue
 
         # ignore hidden files
-        if os.path.basename(file)[0] == ".":
+        if os.path.basename(path)[0] == ".":
             continue
 
-        if file in inFiles:
+        if path in inFiles:
             continue
 
-        outputFiles[os.path.basename(file)] = file
+        outputFiles[os.path.basename(path)] = path
 
     return outputFiles

source/autograder_platform/StudentSubmissionImpl/Python/PythonSubmission.py

@@ -84,12 +84,12 @@ def _addFileToMap(self, path: str, fileType: FileTypeMap) -> None:
         self.discoveredFileMap[fileType].append(path)
 
     def _discoverSubmittedFiles(self, directoryToSearch: str) -> None:
-        pathesToVisit: Iterable[str] = filter(filterSearchResults, os.listdir(directoryToSearch))
+        pathsToVisit: Iterable[str] = filter(filterSearchResults, os.listdir(directoryToSearch))
 
-        if not pathesToVisit:
+        if not pathsToVisit:
             return
 
-        for path in pathesToVisit:
+        for path in pathsToVisit:
             if os.path.isdir(os.path.join(directoryToSearch, path)):
                 self._discoverSubmittedFiles(os.path.join(directoryToSearch, path))
                 continue

tests/platform_tests/impl/python/testFullExecutions.py

@@ -434,27 +434,35 @@ def INJECTED_testClassTest1(test1Cls, expectedValue):
         self.assertEqual(expected, getResults(environment).return_val)
 
     def testVerifySandboxDeletedAfterTests(self):
-        program = "print('OUTPUT stuff')"
-        self.writePythonFile("submission.py", program)
+        expectedContents = "hello"
 
-        self.writePythonFile(".keep", "")
+        program = f"o = open('output.txt', 'w');o.write('{expectedContents}');o.close()"
+        self.writePythonFile("submission.py", program)
 
         submission = PythonSubmission()\
             .setSubmissionRoot(self.PYTHON_PROGRAM_DIRECTORY)\
             .load()\
             .build()\
             .validate()
-        
-        environment = ExecutionEnvironmentBuilder()\
-            .addFile(os.path.join(self.PYTHON_PROGRAM_DIRECTORY, ".keep"), ".keep")\
-            .setTimeout(5)\
-            .build()
-
-        for _ in range(10):
+
+
+        for _ in range(100):
+            environment = ExecutionEnvironmentBuilder() \
+                .setTimeout(2) \
+                .build()
+
             runner = PythonRunnerBuilder(submission)\
                 .setEntrypoint(module=True)\
                 .build()
             Executor.execute(environment, runner)
+            file = getResults(environment).file_out["output.txt"]
+
+            self.assertEqual(expectedContents, file)
+
+            Executor.cleanup(environment)
+
+            self.assertFalse(os.path.exists(environment.sandbox_location))
+
 
 
 

tests/platform_tests/impl/python/testPrograms/submission.py

@@ -0,0 +1 @@
+o = open('output.txt', 'w');o.write('hello');o.close()

tests/platform_tests/impl/python/testPythonSubmission.py

@@ -214,6 +214,44 @@ def testManyNestedMainFiles(self):
                     .build()\
                     .validate()
 
+    def testIgnoreHiddenFiles(self):
+        with open(os.path.join(self.TEST_FILE_DIRECTORY, ".py"), 'w') as w:
+            w.writelines(self.TEST_FILE_MAIN)
+
+        with self.assertRaises(ValidationError):
+            PythonSubmission() \
+                .setSubmissionRoot(self.TEST_FILE_DIRECTORY) \
+                .enableLooseMainMatching()\
+                .load() \
+                .build() \
+                .validate()
+
+    def testIgnorePycacheFiles(self):
+        os.makedirs(os.path.join(self.TEST_FILE_DIRECTORY, "__pycache__"))
+        with open(os.path.join(self.TEST_FILE_DIRECTORY, "__pycache__", "main.pyc"), 'w') as w:
+            w.writelines(self.TEST_FILE_MAIN)
+
+        with self.assertRaises(ValidationError):
+            PythonSubmission() \
+                .setSubmissionRoot(self.TEST_FILE_DIRECTORY) \
+                .enableLooseMainMatching() \
+                .load() \
+                .build() \
+                .validate()
+
+    def testIgnorePathsWithSpaces(self):
+        os.makedirs(os.path.join(self.TEST_FILE_DIRECTORY, "code folder"))
+        with open(os.path.join(self.TEST_FILE_DIRECTORY, "code folder", "main.py"), 'w') as w:
+            w.writelines(self.TEST_FILE_MAIN)
+
+        with self.assertRaises(ValidationError):
+            PythonSubmission() \
+                .setSubmissionRoot(self.TEST_FILE_DIRECTORY) \
+                .load() \
+                .build() \
+                .validate()
+
+
     @patch('sys.stdout', new_callable=StringIO)
     def testDiscoverEntrypointManyPy(self, capturedStdout):
         with open(os.path.join(self.TEST_FILE_DIRECTORY, "main.py"), 'w') as w: