diff --git a/tests/fixtures_tasks_v2.py b/tests/fixtures_tasks_v2.py index c404b6332d..0e5de4ae45 100644 --- a/tests/fixtures_tasks_v2.py +++ b/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, diff --git a/tests/v2/03_api/test_api_task_collection.py b/tests/v2/03_api/test_api_task_collection.py index 2a244161eb..cf526885bb 100644 --- a/tests/v2/03_api/test_api_task_collection.py +++ b/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,6 +283,7 @@ 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 @@ -313,9 +291,8 @@ async def test_task_collection_from_pypi( 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, ): diff --git a/tests/v2/03_api/test_api_task_collection_failures.py b/tests/v2/03_api/test_api_task_collection_failures.py index 463738258b..0a94e1b89a 100644 --- a/tests/v2/03_api/test_api_task_collection_failures.py +++ b/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,8 +226,12 @@ 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( @@ -269,23 +239,19 @@ async def test_remove_directory( ), ) 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( diff --git a/tests/v2/06_tasks/test_venv_pip.py b/tests/v2/06_tasks/test_venv_pip.py index 81d828cb38..074ebcb36f 100644 --- a/tests/v2/06_tasks/test_venv_pip.py +++ b/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)