fix: from review

selfprivacy_api/jobs/nix_collect_garbage.py

@@ -6,25 +6,30 @@ from selfprivacy_api.utils.huey import huey
 
 from selfprivacy_api.jobs import JobStatus, Jobs, Job
 
+class ShellException(Exception):
+    """Custom exception for shell-related errors."""
+    pass
 
 COMPLETED_WITH_ERROR = (
-    "Error occured, please report this to the support chat."
+    "Error occurred, please report this to the support chat."
 )
 RESULT_WAS_NOT_FOUND_ERROR = "We are sorry, garbage collection result was not found. " \
     "Something went wrong, please report this to the support chat."
 CLEAR_COMPLETED = "Garbage collection completed."
 
 
-def delete_old_gens_and_print_dead() -> str:
+def delete_old_gens_and_return_dead_report() -> str:
     subprocess.run(
         ["nix-env", "-p", "/nix/var/nix/profiles/system", "--delete-generations old"],
         check=False,
     )
 
-    return subprocess.check_output(["nix-store", "--gc", "--print-dead"]).decode(
+    result = subprocess.check_output(["nix-store", "--gc", "--print-dead"]).decode(
         "utf-8"
     )
 
+    return " " if result is None else result
+
 
 def run_nix_collect_garbage() -> Iterable[bytes]:
     process = subprocess.Popen(
@@ -44,12 +49,7 @@ def parse_line(job: Job, line: str) -> Job:
     match = re.search(pattern, line)
 
     if match is None:
-        Jobs.update(
+        raise ShellException("nix returned gibberish output")
-            job=job,
-            status=JobStatus.ERROR,
-            status_text=COMPLETED_WITH_ERROR,
-            error=RESULT_WAS_NOT_FOUND_ERROR,
-        )
 
     else:
         Jobs.update(
@@ -103,17 +103,18 @@ def calculate_and_clear_dead_paths(job: Job):
     )
 
     dead_packages, package_equal_to_percent = get_dead_packages(
-        delete_old_gens_and_print_dead()
+        delete_old_gens_and_return_dead_report()
     )
 
     if dead_packages == 0:
+
         Jobs.update(
             job=job,
             status=JobStatus.FINISHED,
             status_text="Nothing to clear",
             result="System is clear",
         )
-        return
+        return True
 
     Jobs.update(
         job=job,
@@ -123,7 +124,15 @@ def calculate_and_clear_dead_paths(job: Job):
     )
 
     stream = run_nix_collect_garbage()
+    try:
         process_stream(job, stream, dead_packages)
+    except ShellException as error:
+        Jobs.update(
+            job=job,
+            status=JobStatus.ERROR,
+            status_text=COMPLETED_WITH_ERROR,
+            error=RESULT_WAS_NOT_FOUND_ERROR,
+        )
 
 
 def start_nix_collect_garbage() -> Job:
@@ -132,5 +141,7 @@ def start_nix_collect_garbage() -> Job:
         name="Collect garbage",
         description="Cleaning up unused packages",
     )
-    calculate_and_clear_dead_paths(job=job)
+    task_handle = calculate_and_clear_dead_paths(job=job)
+    result = task_handle(blocking=True)
+    assert result
     return job

tests/test_graphql/test_nix_collect_garbage.py

@@ -5,6 +5,7 @@
 import pytest
 
 from selfprivacy_api.jobs import JobStatus, Jobs, Job
+from tests.test_graphql.common import get_data, assert_errorcode, assert_ok
 
 from selfprivacy_api.jobs.nix_collect_garbage import (
     get_dead_packages,
@@ -12,6 +13,7 @@ from selfprivacy_api.jobs.nix_collect_garbage import (
     CLEAR_COMPLETED,
     COMPLETED_WITH_ERROR,
     RESULT_WAS_NOT_FOUND_ERROR,
+    ShellException,
 )
 
 OUTPUT_PRINT_DEAD = """
@@ -53,10 +55,17 @@ log_event = []
 
 
 @pytest.fixture
-def mock_delete_old_gens_and_print_dead(mocker):
+def mock_delete_old_gens_and_return_dead_report(mocker):
-    mock = mocker.patch("selfprivacy_api.jobs.nix_collect_garbage.delete_old_gens_and_print_dead", autospec=True, return_value=None)
+    mock = mocker.patch("selfprivacy_api.jobs.nix_collect_garbage.delete_old_gens_and_return_dead_report", autospec=True, return_value=OUTPUT_PRINT_DEAD)
     return mock
 
+
+@pytest.fixture
+def mock_run_nix_collect_garbage_returns_gibberish(mocker):
+    mock = mocker.patch("selfprivacy_api.jobs.nix_collect_garbage.run_nix_collect_garbage", autospec=True, return_value=b" oiuojkhjkhjkhjkhkjh")
+    return mock
+
+
 # ---
 
 
@@ -83,10 +92,9 @@ def test_parse_line_with_blank_line():
         description="description",
     )
 
+    with pytest.raises(ShellException):
         output = parse_line(job, txt)
-    assert output.error == RESULT_WAS_NOT_FOUND_ERROR
+    
-    assert output.status_text == COMPLETED_WITH_ERROR
-    assert output.status == JobStatus.ERROR
 
 
 def test_get_dead_packages():
@@ -124,7 +132,7 @@ mutation CollectGarbage {
 """
 
 
-def test_graphql_nix_collect_garbage(authorized_client, mock_delete_old_gens_and_print_dead):
+def test_graphql_nix_collect_garbage(authorized_client, mock_delete_old_gens_and_return_dead_report):
     response = authorized_client.post(
         "/graphql",
         json={
@@ -132,10 +140,20 @@ def test_graphql_nix_collect_garbage(authorized_client, mock_delete_old_gens_and
         },
     )
 
-    assert response.status_code == 200
+    output = get_data(response)["system"]["nixCollectGarbage"]
-    assert response.json().get("data") is not None
+    assert_ok(output)
-    assert response.json()["data"]["system"]["nixCollectGarbage"]["message"] is not None
+    assert output["job"] is not None
-    assert response.json()["data"]["system"]["nixCollectGarbage"]["success"] is True
-    assert response.json()["data"]["system"]["nixCollectGarbage"]["code"] == 200
 
-    assert response.json()["data"]["system"]["nixCollectGarbage"]["job"] is not None
+
+def test_graphql_nix_collect_garbage_with_problems(authorized_client, mock_run_nix_collect_garbage_returns_gibberish):
+    response = authorized_client.post(
+        "/graphql",
+        json={
+            "query": RUN_NIX_COLLECT_GARBAGE_MUTATION,
+        },
+    )
+
+    output = get_data(response)["system"]["nixCollectGarbage"]
+    assert_ok(output)
+    assert output["job"] is not None
+    assert output["job"]["status"] == "ERROR"