redis/jobs-coverage (#24)

selfprivacy_api/jobs/__init__.py

@@ -67,10 +67,9 @@ class Jobs:
         """
         Reset the jobs list.
         """
-        r = RedisPool().get_connection()
         jobs = Jobs.get_jobs()
         for job in jobs:
-            r.delete(redis_key_from_uuid(job.uid))
+            Jobs.remove(job)
 
     @staticmethod
     def add(
@@ -99,7 +98,7 @@ class Jobs:
             result=None,
         )
         r = RedisPool().get_connection()
-        store_job_as_hash(r, redis_key_from_uuid(job.uid), job)
+        _store_job_as_hash(r, _redis_key_from_uuid(job.uid), job)
         return job
 
     @staticmethod
@@ -115,8 +114,10 @@ class Jobs:
         Remove a job from the jobs list.
         """
         r = RedisPool().get_connection()
-        key = redis_key_from_uuid(job_uuid)
-        r.delete(key)
+        key = _redis_key_from_uuid(job_uuid)
+        if (r.exists(key)):
+            r.delete(key)
+            return True
         return False
 
     @staticmethod
@@ -149,9 +150,9 @@ class Jobs:
             job.finished_at = datetime.datetime.now()
 
         r = RedisPool().get_connection()
-        key = redis_key_from_uuid(job.uid)
+        key = _redis_key_from_uuid(job.uid)
         if r.exists(key):
-            store_job_as_hash(r, key, job)
+            _store_job_as_hash(r, key, job)
             if status in (JobStatus.FINISHED, JobStatus.ERROR):
                 r.expire(key, JOB_EXPIRATION_SECONDS)
 
@@ -163,9 +164,9 @@ class Jobs:
         Get a job from the jobs list.
         """
         r = RedisPool().get_connection()
-        key = redis_key_from_uuid(uid)
+        key = _redis_key_from_uuid(uid)
         if r.exists(key):
-            return job_from_hash(r, key)
+            return _job_from_hash(r, key)
         return None
 
     @staticmethod
@@ -175,7 +176,7 @@ class Jobs:
         """
         r = RedisPool().get_connection()
         jobs = r.keys("jobs:*")
-        return [job_from_hash(r, job_key) for job_key in jobs]
+        return [_job_from_hash(r, job_key) for job_key in jobs]
 
     @staticmethod
     def is_busy() -> bool:
@@ -183,16 +184,16 @@ class Jobs:
         Check if there is a job running.
         """
         for job in Jobs.get_jobs():
-            if job["status"] == JobStatus.RUNNING.value:
+            if job.status == JobStatus.RUNNING:
                 return True
         return False
 
 
-def redis_key_from_uuid(uuid):
+def _redis_key_from_uuid(uuid):
     return "jobs:" + str(uuid)
 
 
-def store_job_as_hash(r, redis_key, model):
+def _store_job_as_hash(r, redis_key, model):
     for key, value in model.dict().items():
         if isinstance(value, uuid.UUID):
             value = str(value)
@@ -203,7 +204,7 @@ def store_job_as_hash(r, redis_key, model):
         r.hset(redis_key, key, str(value))
 
 
-def job_from_hash(r, redis_key):
+def _job_from_hash(r, redis_key):
     if r.exists(redis_key):
         job_dict = r.hgetall(redis_key)
         for date in [

tests/test_jobs.py

@@ -6,30 +6,65 @@ from selfprivacy_api.jobs import Jobs, JobStatus
 import selfprivacy_api.jobs as jobsmodule
 
 
-def test_jobs(authorized_client, jobs_file, shared_datadir):
-    jobs = Jobs()
-    jobs.reset()
-    assert jobs.get_jobs() == []
+def test_add_reset(jobs_with_one_job):
+    jobs_with_one_job.reset()
+    assert jobs_with_one_job.get_jobs() == []
 
-    test_job = jobs.add(
-        type_id="test",
-        name="Test job",
-        description="This is a test job.",
-        status=JobStatus.CREATED,
-        status_text="Status text",
-        progress=0,
-    )
+
+def test_minimal_update(jobs_with_one_job):
+    jobs = jobs_with_one_job
+    test_job = jobs_with_one_job.get_jobs()[0]
+
+    jobs.update(job=test_job, status=JobStatus.ERROR)
 
     assert jobs.get_jobs() == [test_job]
 
+
+def test_remove_by_uid(jobs_with_one_job):
+    test_job = jobs_with_one_job.get_jobs()[0]
+    uid_str = str(test_job.uid)
+
+    assert jobs_with_one_job.remove_by_uid(uid_str)
+    assert jobs_with_one_job.get_jobs() == []
+    assert not jobs_with_one_job.remove_by_uid(uid_str)
+
+
+def test_remove_update_nonexistent(jobs_with_one_job):
+    test_job = jobs_with_one_job.get_jobs()[0]
+
+    jobs_with_one_job.remove(test_job)
+    assert jobs_with_one_job.get_jobs() == []
+
+    result = jobs_with_one_job.update(job=test_job, status=JobStatus.ERROR)
+    assert result == test_job  # even though we might consider changing this behavior
+
+
+def test_remove_get_nonexistent(jobs_with_one_job):
+    test_job = jobs_with_one_job.get_jobs()[0]
+    uid_str = str(test_job.uid)
+    assert jobs_with_one_job.get_job(uid_str) == test_job
+
+    jobs_with_one_job.remove(test_job)
+
+    assert jobs_with_one_job.get_job(uid_str) is None
+
+
+def test_jobs(jobs_with_one_job):
+    jobs = jobs_with_one_job
+    test_job = jobs_with_one_job.get_jobs()[0]
+    assert not jobs.is_busy()
+
     jobs.update(
         job=test_job,
+        name="Write Tests",
+        description="An oddly satisfying experience",
         status=JobStatus.RUNNING,
         status_text="Status text",
         progress=50,
     )
 
     assert jobs.get_jobs() == [test_job]
+    assert jobs.is_busy()
 
     backup = jobsmodule.JOB_EXPIRATION_SECONDS
     jobsmodule.JOB_EXPIRATION_SECONDS = 0
@@ -46,18 +81,23 @@ def test_jobs(authorized_client, jobs_file, shared_datadir):
 
 
 @pytest.fixture
-def mock_subprocess_run(mocker):
-    mock = mocker.patch("subprocess.run", autospec=True)
-    return mock
+def jobs():
+    j = Jobs()
+    j.reset()
+    assert j.get_jobs() == []
+    yield j
+    j.reset()
 
 
 @pytest.fixture
-def mock_shutil_move(mocker):
-    mock = mocker.patch("shutil.move", autospec=True)
-    return mock
-
-
-@pytest.fixture
-def mock_shutil_chown(mocker):
-    mock = mocker.patch("shutil.chown", autospec=True)
-    return mock
+def jobs_with_one_job(jobs):
+    test_job = jobs.add(
+        type_id="test",
+        name="Test job",
+        description="This is a test job.",
+        status=JobStatus.CREATED,
+        status_text="Status text",
+        progress=0,
+    )
+    assert jobs.get_jobs() == [test_job]
+    return jobs