Merge pull request #225 from cs50/run_dir

check50.internal.run_root_dir

check50/__main__.py

@@ -369,22 +369,20 @@ def main():
             checks_file = (internal.check_dir / config["checks"]).resolve()
 
             # Have lib50 decide which files to include
-            included = lib50.files(config.get("files"))[0]
-
+            included_files = lib50.files(config.get("files"))[0]
 
             # Create a working_area (temp dir) named - with all included student files
-            with lib50.working_area(included, name='-') as working_area, \
+            with CheckRunner(checks_file, included_files) as check_runner, \
                     contextlib.redirect_stdout(LoggerWriter(LOGGER, logging.INFO)), \
                     contextlib.redirect_stderr(LoggerWriter(LOGGER, logging.INFO)):
 
-                check_results = CheckRunner(checks_file).run(included, working_area, args.target)
+                check_results = check_runner.run(args.target)
                 results = {
                     "slug": internal.slug,
                     "results": [attr.asdict(result) for result in check_results],
                     "version": __version__
                 }
 
-
     LOGGER.debug(results)
 
     # Render output

check50/_api.py

@@ -252,7 +252,7 @@ def stdout(self, output=None, str_output=None, regex=True, timeout=3):
         if output == EOF:
             log(_("checking for EOF..."))
         else:
-            output = output.replace("\n", "\r\n")
+            output = str(output).replace("\n", "\r\n")
             log(_("checking for output \"{}\"...").format(str_output))
 
         try:

check50/internal.py

@@ -16,9 +16,15 @@
 #: Directory containing the check and its associated files
 check_dir = None
 
-#: Temporary directory in which check is being run
+#: Temporary directory in which the current check is being run
 run_dir = None
 
+#: Temporary directory that is the root (parent) of all run_dir(s)
+run_root_dir = None
+
+#: Directory check50 was run from
+student_dir = None
+
 #: Boolean that indicates if a check is currently running
 check_running = False
 

check50/runner.py

@@ -15,6 +15,7 @@
 import traceback
 
 import attr
+import lib50
 
 from . import internal, __version__
 from ._api import log, Failure, _copy, _log, _data
@@ -126,16 +127,16 @@ def decorator(check):
         check._check_dependency = dependency
 
         @functools.wraps(check)
-        def wrapper(checks_root, dependency_state):
+        def wrapper(run_root_dir, dependency_state):
             # Result template
             result = CheckResult.from_check(check)
             # Any shared (returned) state
             state = None
 
             try:
                 # Setup check environment, copying disk state from dependency
-                internal.run_dir = checks_root / check.__name__
-                src_dir = checks_root / (dependency.__name__ if dependency else "-")
+                internal.run_dir = run_root_dir / check.__name__
+                src_dir = run_root_dir / (dependency.__name__ if dependency else "-")
                 shutil.copytree(src_dir, internal.run_dir)
                 os.chdir(internal.run_dir)
 
@@ -166,33 +167,11 @@ def wrapper(checks_root, dependency_state):
 
 
 class CheckRunner:
-    def __init__(self, checks_path):
-        # TODO: Naming the module "checks" is arbitray. Better name?
-        self.checks_spec = importlib.util.spec_from_file_location("checks", checks_path)
-
-        # Clear check_names, import module, then save check_names. Not thread safe.
-        # Ideally, there'd be a better way to extract declaration order than @check mutating global state,
-        # but there are a lot of subtleties with using `inspect` or similar here
-        _check_names.clear()
-        check_module = importlib.util.module_from_spec(self.checks_spec)
-        self.checks_spec.loader.exec_module(check_module)
-        self.check_names = _check_names.copy()
-        _check_names.clear()
+    def __init__(self, checks_path, included_files):
+        self.checks_path = checks_path
+        self.included_files = included_files
 
-        # Grab all checks from the module
-        checks = inspect.getmembers(check_module, lambda f: hasattr(f, "_check_dependency"))
-
-        # Map each check to tuples containing the names of the checks that depend on it
-        self.dependency_graph = collections.defaultdict(set)
-        for name, check in checks:
-            dependency = None if check._check_dependency is None else check._check_dependency.__name__
-            self.dependency_graph[dependency].add(name)
-
-        # Map each check name to its description
-        self.check_descriptions = {name: check.__doc__ for name, check in checks}
-
-
-    def run(self, files, working_area, targets=None):
+    def run(self, targets=None):
         """
         Run checks concurrently.
         Returns a list of CheckResults ordered by declaration order of the checks in the imported module
@@ -203,7 +182,6 @@ def run(self, files, working_area, targets=None):
         # Ensure that dictionary is ordered by check declaration order (via self.check_names)
         # NOTE: Requires CPython 3.6. If we need to support older versions of Python, replace with OrderedDict.
         results = {name: None for name in self.check_names}
-        checks_root = working_area.parent
 
         try:
             max_workers = int(os.environ.get("CHECK50_WORKERS"))
@@ -212,7 +190,7 @@ def run(self, files, working_area, targets=None):
 
         with futures.ProcessPoolExecutor(max_workers=max_workers) as executor:
             # Start all checks that have no dependencies
-            not_done = set(executor.submit(run_check(name, self.checks_spec, checks_root))
+            not_done = set(executor.submit(run_check(name, self.checks_spec))
                            for name in graph[None])
             not_passed = []
 
@@ -226,7 +204,7 @@ def run(self, files, working_area, targets=None):
                         # Dispatch dependent checks
                         for child_name in graph[result.name]:
                             not_done.add(executor.submit(
-                                run_check(child_name, self.checks_spec, checks_root, state)))
+                                run_check(child_name, self.checks_spec, state)))
                     else:
                         not_passed.append(result.name)
 
@@ -291,6 +269,53 @@ def _skip_children(self, check_name, results):
                 self._skip_children(name, results)
 
 
+    def __enter__(self):
+        # Remember the student's directory
+        internal.student_dir = Path.cwd()
+
+        # Set up a temp dir for the checks
+        self._working_area_manager = lib50.working_area(self.included_files, name='-')
+        internal.run_root_dir = self._working_area_manager.__enter__().parent
+
+        # Change current working dir to the temp dir
+        self._cd_manager = lib50.cd(internal.run_root_dir)
+        self._cd_manager.__enter__()
+
+        # TODO: Naming the module "checks" is arbitray. Better name?
+        self.checks_spec = importlib.util.spec_from_file_location("checks", self.checks_path)
+
+        # Clear check_names, import module, then save check_names. Not thread safe.
+        # Ideally, there'd be a better way to extract declaration order than @check mutating global state,
+        # but there are a lot of subtleties with using `inspect` or similar here
+        _check_names.clear()
+        check_module = importlib.util.module_from_spec(self.checks_spec)
+        self.checks_spec.loader.exec_module(check_module)
+        self.check_names = _check_names.copy()
+        _check_names.clear()
+
+        # Grab all checks from the module
+        checks = inspect.getmembers(check_module, lambda f: hasattr(f, "_check_dependency"))
+
+        # Map each check to tuples containing the names of the checks that depend on it
+        self.dependency_graph = collections.defaultdict(set)
+        for name, check in checks:
+            dependency = None if check._check_dependency is None else check._check_dependency.__name__
+            self.dependency_graph[dependency].add(name)
+
+        # Map each check name to its description
+        self.check_descriptions = {name: check.__doc__ for name, check in checks}
+
+        return self
+
+
+    def __exit__(self, type, value, tb):
+        # Destroy the temporary directory for the checks
+        self._working_area_manager.__exit__(type, value, tb)
+
+        # cd back to the directory check50 was called from
+        self._cd_manager.__exit__(type, value, tb)
+
+
 class run_check:
     """
     Check job that runs in a separate process.
@@ -303,14 +328,15 @@ class run_check:
     CROSS_PROCESS_ATTRIBUTES = (
         "internal.check_dir",
         "internal.slug",
+        "internal.student_dir",
+        "internal.run_root_dir",
         "sys.excepthook",
         "__version__"
     )
 
-    def __init__(self, check_name, spec, checks_root, state=None):
+    def __init__(self, check_name, spec, state=None):
         self.check_name = check_name
         self.spec = spec
-        self.checks_root = checks_root
         self.state = state
         self.attribute_values = tuple(eval(name) for name in self.CROSS_PROCESS_ATTRIBUTES)
 
@@ -333,6 +359,6 @@ def __call__(self):
         self.spec.loader.exec_module(mod)
         internal.check_running = True
         try:
-            return getattr(mod, self.check_name)(self.checks_root, self.state)
+            return getattr(mod, self.check_name)(internal.run_root_dir, self.state)
         finally:
             internal.check_running = False

tests/check50_tests.py

@@ -412,5 +412,13 @@ def test_traceback(self):
         process.expect("foo")
 
 
+class TestInternalDirectories(Base):
+    def test_directories_exist(self):
+        with open("foo.py", "w") as f:
+            f.write(os.getcwd())
+
+        process = pexpect.spawn(f"check50 --dev {CHECKS_DIRECTORY}/internal_directories")
+        process.expect_exact(":)")
+
 if __name__ == "__main__":
     unittest.main()

tests/checks/internal_directories/.cs50.yaml

@@ -0,0 +1 @@
+check50: true

tests/checks/internal_directories/__init__.py

@@ -0,0 +1,20 @@
+import pathlib
+import check50
+import check50.internal as internal
+
+@check50.check()
+def foo():
+    """directories exist"""
+    assert internal.run_dir.resolve() == pathlib.Path.cwd()
+    assert internal.run_dir.parent == internal.run_root_dir
+
+assert internal.run_root_dir.exists()
+assert internal.run_root_dir.resolve() == pathlib.Path.cwd()
+
+assert internal.student_dir.exists()
+with open(internal.student_dir / "foo.py") as f:
+    student_dir = pathlib.Path(f.read().strip())
+assert internal.student_dir == student_dir
+
+assert internal.check_dir.exists()
+assert internal.check_dir.name == "internal_directories"