Fix JFM tests (#424)

* fix JFM tests

* pre-commit

* add minor comment

Author: dlqqq

jupyter_scheduler/job_files_manager.py

@@ -52,16 +52,11 @@ def __init__(
 
     def generate_filepaths(self):
         """A generator that produces filepaths"""
-        output_dir = self.output_dir
-        if not os.path.exists(output_dir):
-            os.makedirs(output_dir)
-
         output_formats = self.output_formats + ["input"]
 
         for output_format in output_formats:
             input_filepath = self.staging_paths[output_format]
-            output_filename = self.output_filenames[output_format]
-            output_filepath = os.path.join(output_dir, output_filename)
+            output_filepath = os.path.join(self.output_dir, self.output_filenames[output_format])
             if not os.path.exists(output_filepath) or self.redownload:
                 yield input_filepath, output_filepath
 
@@ -74,9 +69,15 @@ def download_tar(self, archive_format: str = "tar"):
                 tar.extractall(self.output_dir, filter="data")
 
     def download(self):
+        # ensure presence of staging paths
         if not self.staging_paths:
             return
 
+        # ensure presence of output dir
+        output_dir = self.output_dir
+        if not os.path.exists(output_dir):
+            os.makedirs(output_dir)
+
         if "tar" in self.staging_paths:
             self.download_tar()
         elif "tar.gz" in self.staging_paths:

jupyter_scheduler/tests/test_job_files_manager.py

@@ -2,15 +2,14 @@
 import os
 import shutil
 import tarfile
-import tempfile
+import time
 from pathlib import Path
 from unittest.mock import patch
 
 import pytest
 
 from jupyter_scheduler.job_files_manager import Downloader, JobFilesManager
 from jupyter_scheduler.models import DescribeJob, JobFile
-from jupyter_scheduler.scheduler import BaseScheduler
 
 
 async def test_copy_from_staging():
@@ -68,6 +67,9 @@ async def test_copy_from_staging():
 def clear_outputs_dir():
     yield
     shutil.rmtree(OUTPUTS_DIR)
+    # rmtree() is not synchronous; wait until it has finished running
+    while os.path.isdir(OUTPUTS_DIR):
+        time.sleep(0.01)
 
 
 @pytest.mark.parametrize(
@@ -76,9 +78,9 @@ def clear_outputs_dir():
         (
             ["ipynb", "html"],
             {
-                "ipynb": "helloworld-out.ipynb",
-                "html": "helloworld-out.html",
-                "input": "helloworld-input.ipynb",
+                "ipynb": "job-1/helloworld-out.ipynb",
+                "html": "job-1/helloworld-out.html",
+                "input": "job-1/helloworld-input.ipynb",
             },
             {
                 "ipynb": os.path.join(HERE, "test_staging_dir", "job-1", "helloworld-1.ipynb"),
@@ -91,9 +93,9 @@ def clear_outputs_dir():
         (
             ["ipynb", "html"],
             {
-                "ipynb": "helloworld-out.ipynb",
-                "html": "helloworld-out.html",
-                "input": "helloworld-input.ipynb",
+                "ipynb": "job-2/helloworld-1.ipynb",
+                "html": "job-2/helloworld-1.html",
+                "input": "job-2/helloworld.ipynb",
             },
             {
                 "tar.gz": os.path.join(HERE, "test_staging_dir", "job-2", "helloworld.tar.gz"),
@@ -120,10 +122,13 @@ def test_downloader_download(
 
     assert os.path.exists(output_dir)
     for format in output_formats:
+        # get path to output file corresponding to this format
         out_filepath = os.path.join(output_dir, output_filenames[format])
 
+        # assert each output file exists
         assert os.path.exists(out_filepath)
 
+        # assert integrity of each output file
         if "tar.gz" in staging_paths:
             with tarfile.open(staging_paths["tar.gz"]) as tar:
                 input_file = tar.extractfile(member=staging_paths[format])