Update tests of task collection

tests/fixtures_tasks_v2.py

@@ -20,7 +20,7 @@
     _prepare_tasks_metadata,
 )
 from fractal_server.tasks.v2.database_operations import (
-    create_db_task_group_and_tasks,
+    create_db_tasks_and_update_task_group,
 )
 
 
@@ -92,7 +92,7 @@ def fractal_tasks_mock_db(
     default_user_group: UserGroup,
 ) -> dict[str, TaskV2]:
 
-    task_group = create_db_task_group_and_tasks(
+    task_group = create_db_tasks_and_update_task_group(
         task_list=fractal_tasks_mock_collection["task_list"],
         task_group_obj=TaskGroupCreateV2(origin="other", pkg_name="mock"),
         user_id=first_user.id,

tests/v2/03_api/test_api_task_collection.py

@@ -8,7 +8,6 @@
 from packaging.version import Version
 
 from fractal_server.app.models.v2 import CollectionStateV2
-from fractal_server.app.models.v2 import TaskV2
 from fractal_server.app.schemas.v2 import CollectionStatusV2
 from fractal_server.app.schemas.v2 import ManifestV2
 from fractal_server.app.schemas.v2 import TaskCollectCustomV2
@@ -107,9 +106,8 @@ async def test_task_collection_from_wheel(
         assert ".whl[my_extra]" in log
 
         # Check on-disk files
-        full_path = settings.FRACTAL_TASKS_DIR / venv_path
-        assert get_collection_path(full_path).exists()
-        assert get_log_path(full_path).exists()
+        assert get_collection_path(Path(venv_path).parent).exists()
+        assert get_log_path(Path(venv_path).parent).exists()
 
         # Check actual Python version
         python_bin = task_list[0]["command_non_parallel"].split()[0]
@@ -143,29 +141,15 @@ async def test_task_collection_from_wheel(
             if task["command_parallel"] is not None:
                 assert task["args_schema_parallel"] is not None
 
-        # Collect again (already installed)
+        # A second identical collection fails
         res = await client.post(f"{PREFIX}/collect/pip/", json=payload)
-        assert res.status_code == 200
-        state = res.json()
-        data = state["data"]
-        assert data["info"] == "Already installed"
+        assert res.status_code == 422
 
         # Check that *verbose* collection info contains logs
         res = await client.get(f"{PREFIX}/collect/{state_id}/?verbose=true")
         assert res.status_code == 200
         assert res.json()["data"]["log"] is not None
 
-        # Modify a task source (via DB, since endpoint cannot modify source)
-        db_task = await db.get(TaskV2, task_list[0]["id"])
-        db_task.source = "EDITED_SOURCE"
-        await db.commit()
-        await db.close()
-
-        # Collect again, and check that collection fails
-        res = await client.post(f"{PREFIX}/collect/pip/", json=payload)
-        debug(res.json())
-        assert res.status_code == 422
-
 
 async def test_task_collection_from_wheel_non_canonical(
     db,
@@ -227,15 +211,9 @@ async def test_task_collection_from_wheel_non_canonical(
         # Verify how package name is used in source and folders
         assert "fractal_tasks_non_canonical" in task_list[0]["source"]
         python_path, task_path = task_list[0]["command_non_parallel"].split()
-        assert (
-            "FRACTAL_TASKS_DIR/.fractal/fractal-tasks-non-canonical0.0.1"
-            in python_path
-        )
-        assert (
-            "FRACTAL_TASKS_DIR/.fractal/fractal-tasks-non-canonical0.0.1"
-            in task_path
-        )
-        assert "fractal_tasks_non_canonical" in task_path
+        assert "/fractal-tasks-non-canonical/0.0.1" in python_path
+        assert "/fractal-tasks-non-canonical/0.0.1" in task_path
+        assert "fractal-tasks-non-canonical" in task_path
 
         # Check that log were written, even with CRITICAL logging level
         log = data["log"]
@@ -294,7 +272,6 @@ async def test_task_collection_from_pypi(
             json=payload,
         )
         assert res.status_code == 201
-        debug(res.json())
         assert res.json()["data"]["status"] == CollectionStatusV2.PENDING
         state = res.json()
         state_id = state["id"]
@@ -306,16 +283,16 @@ async def test_task_collection_from_pypi(
         res = await client.get(f"{PREFIX}/collect/{state_id}/")
         assert res.status_code == 200
         state = res.json()
+        debug(state)
         data = state["data"]
         task_list = data["task_list"]
         # Check that log were written, even with CRITICAL logging level
         log = data["log"]
         assert log is not None
 
         # Check on-disk files
-        full_path = settings.FRACTAL_TASKS_DIR / venv_path
-        assert get_collection_path(full_path).exists()
-        assert get_log_path(full_path).exists()
+        assert get_collection_path(Path(venv_path).parent).exists()
+        assert get_log_path(Path(venv_path).parent).exists()
 
         # Check actual Python version
         python_bin = task_list[0]["command_non_parallel"].split()[0]
@@ -344,29 +321,17 @@ async def test_task_collection_from_pypi(
             if task["command_parallel"] is not None:
                 assert task["args_schema_parallel"] is not None
 
-        # Collect again (already installed)
+        # Collect again and fail
+        # FIXME: this currently fails because of an existing folder, but it
+        # should fail due to task-group non-duplication contraints
         res = await client.post(f"{PREFIX}/collect/pip/", json=payload)
-        assert res.status_code == 200
-        state = res.json()
-        data = state["data"]
-        assert data["info"] == "Already installed"
+        assert res.status_code == 422
 
         # Check that *verbose* collection info contains logs
         res = await client.get(f"{PREFIX}/collect/{state_id}/?verbose=true")
         assert res.status_code == 200
         assert res.json()["data"]["log"] is not None
 
-        # Modify a task source (via DB, since endpoint cannot modify source)
-        db_task = await db.get(TaskV2, task_list[0]["id"])
-        db_task.source = "EDITED_SOURCE"
-        await db.commit()
-        await db.close()
-
-        # Collect again, and check that collection fails
-        res = await client.post(f"{PREFIX}/collect/pip/", json=payload)
-        debug(res.json())
-        assert res.status_code == 422
-
 
 async def test_read_log_from_file(db, tmp_path, MockCurrentUser, client):
 
@@ -398,8 +363,6 @@ async def test_read_log_from_file(db, tmp_path, MockCurrentUser, client):
 async def test_task_collection_custom(
     client,
     MockCurrentUser,
-    tmp_path: Path,
-    testdata_path,
     task_factory,
     fractal_tasks_mock_collection,
 ):

tests/v2/03_api/test_api_task_collection_failures.py

@@ -1,10 +1,10 @@
-import json
 import logging
-import os
 from pathlib import Path
 
 from devtools import debug  # noqa
+from sqlmodel import select
 
+from fractal_server.app.models.v2 import TaskGroupV2
 from fractal_server.app.schemas.v2 import CollectionStatusV2
 from fractal_server.config import get_settings
 from fractal_server.syringe import Inject
@@ -46,9 +46,6 @@ async def test_failed_API_calls(
             json=dict(package=str("something.whl")),
         )
         assert res.status_code == 422
-        debug(res.json())
-        assert "ends with '.whl'" in str(res.json())
-        assert "is not the absolute path to a wheel file" in str(res.json())
 
     # Package `asd` exists, but it has no wheel file
     async with MockCurrentUser(user_kwargs=dict(is_verified=True)) as user:
@@ -169,69 +166,40 @@ async def test_missing_task_executable(
         assert "fail" in data["log"]
 
 
-async def test_collection_validation_error(
-    client,
+async def test_folder_already_exists(
     MockCurrentUser,
+    client,
     override_settings_factory,
     tmp_path: Path,
     testdata_path: Path,
 ):
     override_settings_factory(FRACTAL_TASKS_DIR=tmp_path)
-    payload = dict(
-        package=(
-            testdata_path.parent
-            / "v2/fractal_tasks_mock/dist"
-            / "fractal_tasks_mock-0.0.1-py3-none-any.whl"
-        ).as_posix()
-    )
-
-    file_dir = tmp_path / ".fractal/fractal-tasks-mock0.0.1"
-    os.makedirs(file_dir, exist_ok=True)
-
-    # Folder exists, but there is no collection.json file
-    async with MockCurrentUser(user_kwargs=dict(is_verified=True)):
-        res = await client.post(
-            f"{PREFIX}/collect/pip/",
-            json=payload,
-        )
-        assert res.status_code == 422
-        assert "FileNotFoundError" in res.json()["detail"]
-
-    # Write an invalid collection.json file
-    file_path = file_dir / "collection.json"
-    # case 1
-    with open(file_path, "w") as f:
-        json.dump([1, 2, 3], f)
-    async with MockCurrentUser(user_kwargs=dict(is_verified=True)):
-        res = await client.post(
-            f"{PREFIX}/collect/pip/",
-            json=payload,
-        )
-        assert res.status_code == 422
-        assert "it's not a Python dictionary" in res.json()["detail"]
-    # case 2
-    with open(file_path, "w") as f:
-        json.dump(dict(foo="bar"), f)
-    async with MockCurrentUser(user_kwargs=dict(is_verified=True)):
-        res = await client.post(
-            f"{PREFIX}/collect/pip/",
-            json=payload,
+    async with MockCurrentUser(user_kwargs=dict(is_verified=True)) as user:
+        # Create the folder in advance
+        expected_path = tmp_path / f"{user.id}/fractal-tasks-mock/0.0.1"
+        expected_path.mkdir(parents=True, exist_ok=True)
+        assert expected_path.exists()
+
+        # Fail because folder already exists
+        payload = dict(
+            package=(
+                testdata_path.parent
+                / "v2/fractal_tasks_mock/dist"
+                / "fractal_tasks_mock-0.0.1-py3-none-any.whl"
+            ).as_posix()
         )
-        assert res.status_code == 422
-        assert "it has no key 'task_list'" in res.json()["detail"]
-    # case 3
-    with open(file_path, "w") as f:
-        json.dump(dict(task_list="foo"), f)
-    async with MockCurrentUser(user_kwargs=dict(is_verified=True)):
         res = await client.post(
             f"{PREFIX}/collect/pip/",
             json=payload,
         )
         assert res.status_code == 422
-        assert "'task_list' is not a Python list" in res.json()["detail"]
+        assert "already exists" in res.json()["detail"]
 
+    # Failed task collection did not remove an existing folder
+    assert expected_path.exists()
 
-async def test_remove_directory(
+
+async def test_failure_cleanup(
     db,
     client,
     MockCurrentUser,
@@ -240,17 +208,15 @@ async def test_remove_directory(
     testdata_path: Path,
 ):
     """
-    This test tries (without success) to reproduce this error
-    https://github.com/fractal-analytics-platform/fractal-server/issues/1234
+    Verify that a failed collection cleans up its folder and TaskGroupV2.
     """
+
     override_settings_factory(
         FRACTAL_TASKS_DIR=tmp_path,
         FRACTAL_LOGGING_LEVEL=logging.CRITICAL,
     )
 
-    DIRECTORY = tmp_path / ".fractal/fractal-tasks-mock0.0.1"
-    assert os.path.isdir(DIRECTORY) is False
-
+    # Valid part of the payload
     payload = dict(
         package=(
             testdata_path.parent
@@ -260,32 +226,32 @@ async def test_remove_directory(
         package_extras="my_extra",
     )
 
-    async with MockCurrentUser(user_kwargs=dict(is_verified=True)):
+    async with MockCurrentUser(user_kwargs=dict(is_verified=True)) as user:
 
+        TASK_GROUP_PATH = tmp_path / str(user.id) / "fractal-tasks-mock/0.0.1"
+        assert not TASK_GROUP_PATH.exists()
+
+        # Endpoint returns correctly, despite invalid `pinned_package_versions`
         res = await client.post(
             f"{PREFIX}/collect/pip/",
             json=dict(
                 **payload, pinned_package_versions={"devtools": "99.99.99"}
             ),
         )
         assert res.status_code == 201
-        assert os.path.isdir(DIRECTORY) is False
+        collection_id = res.json()["id"]
 
-        res = await client.get(f"{PREFIX}/collect/1/")
+        # Background task actually failed
+        res = await client.get(f"{PREFIX}/collect/{collection_id}/")
         assert (
             "No matching distribution found for devtools==99.99.99"
             in res.json()["data"]["log"]
         )
-        assert os.path.isdir(DIRECTORY) is False
 
-        res = await client.post(
-            f"{PREFIX}/collect/pip/",
-            json=dict(
-                **payload, pinned_package_versions={"devtools": "0.0.1"}
-            ),
-        )
-        assert res.status_code == 201
-        assert os.path.isdir(DIRECTORY) is True
+        # Cleanup was performed correctly
+        assert not TASK_GROUP_PATH.exists()
+        res = await db.execute(select(TaskGroupV2))
+        assert len(res.scalars().all()) == 0
 
 
 async def test_invalid_python_version(

tests/v2/06_tasks/test_venv_pip.py

@@ -64,7 +64,9 @@ async def test_pip_install(local_or_remote, tmp_path, testdata_path):
     venv_path = tmp_path / "pkg_folder"
     venv_path.mkdir(exist_ok=True, parents=True)
     await _init_venv_v2(
-        path=venv_path, python_version=PYTHON_VERSION, logger_name=LOGGER_NAME
+        venv_path=venv_path,
+        python_version=PYTHON_VERSION,
+        logger_name=LOGGER_NAME,
     )
 
     # Pip install
@@ -99,7 +101,7 @@ async def test_pip_install_pinned(tmp_path, caplog):
     venv_path.mkdir(exist_ok=True, parents=True)
     pip = venv_path / "venv/bin/pip"
     await _init_venv_v2(
-        path=venv_path, logger_name=LOG, python_version=PYTHON_VERSION
+        venv_path=venv_path, logger_name=LOG, python_version=PYTHON_VERSION
     )
 
     async def _aux(*, pin: Optional[dict[str, str]] = None) -> str:
@@ -192,7 +194,7 @@ async def test_init_venv(
 
     try:
         python_bin = await _init_venv_v2(
-            path=venv_path,
+            venv_path=venv_path,
             logger_name=logger_name,
             python_version=python_version,
         )
@@ -245,7 +247,7 @@ async def test_create_venv_install_package_pip(
 
     # Collect task package
     python_bin, package_root = await _create_venv_install_package_pip(
-        task_pkg=task_pkg, path=tmp_path, logger_name=LOGGER_NAME
+        task_pkg=task_pkg, venv_path=tmp_path, logger_name=LOGGER_NAME
     )
     debug(python_bin)
     debug(package_root)