From 2fc807a8644595acc3a9eae270c0c0c245e5627c Mon Sep 17 00:00:00 2001 From: Douglas Lowe <10961945+douglowe@users.noreply.github.com> Date: Mon, 14 Jul 2025 22:55:03 +0100 Subject: [PATCH 1/6] minio util tests --- tests/test_minio.py | 304 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 304 insertions(+) create mode 100644 tests/test_minio.py diff --git a/tests/test_minio.py b/tests/test_minio.py new file mode 100644 index 0000000..18938e1 --- /dev/null +++ b/tests/test_minio.py @@ -0,0 +1,304 @@ +import os +import tempfile +import json +import pytest +from io import BytesIO +from minio import Minio +from minio.error import S3Error +from unittest.mock import MagicMock +from unittest import mock + + +@pytest.fixture +def mock_minio_response(): + response = MagicMock() + response.data.decode.return_value = json.dumps({"status": "valid"}) + return response + +# @pytest.fixture +# def mock_minio_response(): +# mock_response = mock.Mock() +# return mock_response +# mock_response.data.decode.return_value = json.dumps({"status": "valid"}) + +# Testing function: get_minio_client_and_bucket + +def test_get_minio_client_success(monkeypatch): + # Set required env vars + monkeypatch.setenv("MINIO_ENDPOINT", "localhost:9000") + monkeypatch.setenv("MINIO_ROOT_USER", "admin") + monkeypatch.setenv("MINIO_ROOT_PASSWORD", "password123") + monkeypatch.setenv("MINIO_BUCKET_NAME", "test-bucket") + + from app.utils.minio_utils import get_minio_client_and_bucket + client, bucket = get_minio_client_and_bucket() + + assert isinstance(client, Minio) + assert bucket == "test-bucket" + assert client._base_url.host == "localhost:9000" + + +def test_get_minio_client_missing_bucket_name(monkeypatch): + # Set all except MINIO_BUCKET_NAME + monkeypatch.setenv("MINIO_ENDPOINT", "localhost:9000") + monkeypatch.setenv("MINIO_ROOT_USER", "admin") + monkeypatch.setenv("MINIO_ROOT_PASSWORD", "password123") + monkeypatch.setenv("MINIO_BUCKET_NAME", "") + + from app.utils.minio_utils import get_minio_client_and_bucket + with pytest.raises(ValueError, match="MINIO_BUCKET_NAME is not set"): + get_minio_client_and_bucket() + + +# Testing function: get_validation_status_from_minio + +def test_successful_retrieval(mocker, mock_minio_response): + mock_client = MagicMock() + mock_bucket = "test-bucket" + mock_client.get_object.return_value = mock_minio_response + mocker.patch("app.utils.minio_utils.get_minio_client_and_bucket", return_value=(mock_client, mock_bucket)) + + from app.utils.minio_utils import get_validation_status_from_minio + result = get_validation_status_from_minio("crate123") + + assert result == {"status": "valid"} + mock_minio_response.close.assert_called_once() + mock_minio_response.release_conn.assert_called_once() + + +def test_s3_error_raised(mocker): + mock_client = MagicMock() + mock_bucket = "test-bucket" + mock_client.get_object.side_effect = S3Error( + code="S3 error", + message=None, + resource=None, + request_id=None, + host_id=None, + response=None + ) + mocker.patch("app.utils.minio_utils.get_minio_client_and_bucket", return_value=(mock_client, mock_bucket)) + + from app.utils.minio_utils import get_validation_status_from_minio + with pytest.raises(S3Error): + get_validation_status_from_minio("crate123") + + +def test_value_error_raised(mocker): + mocker.patch("app.utils.minio_utils.get_minio_client_and_bucket", side_effect=ValueError("Missing env var")) + + from app.utils.minio_utils import get_validation_status_from_minio + with pytest.raises(ValueError): + get_validation_status_from_minio("crate123") + + +def test_generic_exception_raised(mocker): + mock_client = MagicMock() + mock_bucket = "test-bucket" + mock_client.get_object.side_effect = Exception("Unexpected failure") + mocker.patch("app.utils.minio_utils.get_minio_client_and_bucket", return_value=(mock_client, mock_bucket)) + + from app.utils.minio_utils import get_validation_status_from_minio + with pytest.raises(Exception): + get_validation_status_from_minio("crate123") + + +# Testing function: fetch_ro_crate_from_minio + +@mock.patch("app.utils.minio_utils.load_dotenv") +@mock.patch("app.utils.minio_utils.get_minio_client_and_bucket") +@mock.patch("app.utils.minio_utils.tempfile.mkdtemp") +@mock.patch("app.utils.minio_utils.os.path.join", side_effect=os.path.join) +def test_fetch_ro_crate_success(mock_join, mock_mkdtemp, mock_get_client, mock_load_dotenv): + # Setup mocks + mock_minio_client = mock.Mock() + mock_get_client.return_value = (mock_minio_client, "test-bucket") + mock_mkdtemp.return_value = "/tmp/testdir" + + crate_id = "test_crate" + expected_path = os.path.join("/tmp/testdir", f"{crate_id}.zip") + + from app.utils.minio_utils import fetch_ro_crate_from_minio + + result = fetch_ro_crate_from_minio(crate_id) + + mock_load_dotenv.assert_called_once() + mock_get_client.assert_called_once() + mock_minio_client.fget_object.assert_called_once_with( + "test-bucket", f"{crate_id}.zip", expected_path + ) + + assert result == expected_path + + +@mock.patch("app.utils.minio_utils.load_dotenv") +@mock.patch("app.utils.minio_utils.get_minio_client_and_bucket") +@mock.patch("app.utils.minio_utils.tempfile.mkdtemp", return_value="/tmp/testdir") +def test_fetch_ro_crate_s3_error(mock_mkdtemp, mock_get_client, mock_load_dotenv): + mock_minio_client = mock.Mock() + mock_get_client.return_value = (mock_minio_client, "test-bucket") + + crate_id = "bad_crate" + mock_minio_client.fget_object.side_effect = S3Error( + code="S3 error", + message=None, + resource=None, + request_id=None, + host_id=None, + response=None + ) + + from app.utils.minio_utils import fetch_ro_crate_from_minio + with pytest.raises(S3Error): + fetch_ro_crate_from_minio(crate_id) + + +@mock.patch("app.utils.minio_utils.load_dotenv") +@mock.patch("app.utils.minio_utils.get_minio_client_and_bucket", side_effect=ValueError("Missing config")) +def test_fetch_ro_crate_value_error(mock_get_client, mock_load_dotenv): + from app.utils.minio_utils import fetch_ro_crate_from_minio + with pytest.raises(ValueError): + fetch_ro_crate_from_minio("any_crate") + + +@mock.patch("app.utils.minio_utils.load_dotenv") +@mock.patch("app.utils.minio_utils.get_minio_client_and_bucket") +@mock.patch("app.utils.minio_utils.tempfile.mkdtemp", return_value="/tmp/testdir") +def test_fetch_ro_crate_unexpected_error(mock_mkdtemp, mock_get_client, mock_load_dotenv): + mock_minio_client = mock.Mock() + mock_get_client.return_value = (mock_minio_client, "test-bucket") + mock_minio_client.fget_object.side_effect = RuntimeError("Unexpected failure") + + from app.utils.minio_utils import fetch_ro_crate_from_minio + with pytest.raises(RuntimeError): + fetch_ro_crate_from_minio("any_crate") + + +# Testing function: update_validation_status_in_minio + +@mock.patch("app.utils.minio_utils.load_dotenv") +@mock.patch("app.utils.minio_utils.get_minio_client_and_bucket") +def test_update_validation_status_success(mock_get_client, mock_load_dotenv): + mock_minio_client = mock.Mock() + mock_get_client.return_value = (mock_minio_client, "test-bucket") + + crate_id = "crate123" + validation_status = json.dumps({"status": "valid", "errors": []}) + + from app.utils.minio_utils import update_validation_status_in_minio + update_validation_status_in_minio(crate_id, validation_status) + + expected_object_name = f"{crate_id}/validation_status.txt" + expected_data = json.dumps(json.loads(validation_status), indent=None).encode("utf-8") + + mock_minio_client.put_object.assert_called_once() + args, kwargs = mock_minio_client.put_object.call_args + + # FIXME: Original suggested test expected 4 values in args, but returned only 2. + # Solution was to check both args and kwargs for the 'data' and 'length' objects. + # Do we need to chose one format of call_args for our tests, or is this ambiguity okay? + bucket_name = args[0] if args else kwargs["bucket_name"] + object_name = args[1] if len(args) > 1 else kwargs["object_name"] + actual_data_stream = args[2] if len(args) > 2 else kwargs["data"] + length = args[3] if len(args) > 3 else kwargs["length"] + + assert bucket_name == "test-bucket" + assert object_name == expected_object_name + assert isinstance(actual_data_stream, BytesIO) + actual_data_stream.seek(0) + assert actual_data_stream.read() == expected_data + assert length == len(expected_data) + assert kwargs["content_type"] == "application/json" + + +@mock.patch("app.utils.minio_utils.load_dotenv") +@mock.patch("app.utils.minio_utils.get_minio_client_and_bucket") +def test_update_validation_status_s3_error(mock_get_client, mock_load_dotenv): + mock_minio_client = mock.Mock() + mock_get_client.return_value = (mock_minio_client, "test-bucket") + mock_minio_client.put_object.side_effect = S3Error( + code="S3 error", + message=None, + resource=None, + request_id=None, + host_id=None, + response=None + ) + + from app.utils.minio_utils import update_validation_status_in_minio + with pytest.raises(S3Error): + update_validation_status_in_minio("crate123", json.dumps({"status": "valid"})) + + +@mock.patch("app.utils.minio_utils.load_dotenv") +@mock.patch("app.utils.minio_utils.get_minio_client_and_bucket", side_effect=ValueError("Missing env vars")) +def test_update_validation_status_value_error(mock_get_client, mock_load_dotenv): + from app.utils.minio_utils import update_validation_status_in_minio + with pytest.raises(ValueError): + update_validation_status_in_minio("crate123", json.dumps({"status": "valid"})) + + +@mock.patch("app.utils.minio_utils.load_dotenv") +@mock.patch("app.utils.minio_utils.get_minio_client_and_bucket") +def test_update_validation_status_unexpected_error(mock_get_client, mock_load_dotenv): + mock_minio_client = mock.Mock() + mock_get_client.return_value = (mock_minio_client, "test-bucket") + mock_minio_client.put_object.side_effect = RuntimeError("Unexpected failure") + + from app.utils.minio_utils import update_validation_status_in_minio + with pytest.raises(RuntimeError): + update_validation_status_in_minio("crate123", json.dumps({"status": "valid"})) + + +# Testing function: get_validation_status_from_minio + +@mock.patch("app.utils.minio_utils.get_minio_client_and_bucket") +def test_get_validation_status_success(mock_get_client, mock_minio_response): + mock_minio_client = mock.Mock() + mock_minio_client.get_object.return_value = mock_minio_response + mock_get_client.return_value = (mock_minio_client, "test-bucket") + + crate_id = "crate123" + from app.utils.minio_utils import get_validation_status_from_minio + result = get_validation_status_from_minio(crate_id) + + assert result == {"status": "valid"} + mock_minio_client.get_object.assert_called_once_with("test-bucket", f"{crate_id}/validation_status.txt") + mock_minio_response.close.assert_called_once() + mock_minio_response.release_conn.assert_called_once() + + +@mock.patch("app.utils.minio_utils.get_minio_client_and_bucket") +def test_get_validation_status_s3_error(mock_get_client): + mock_minio_client = mock.Mock() + mock_minio_client.get_object.side_effect = S3Error( + code="S3 error", + message=None, + resource=None, + request_id=None, + host_id=None, + response=None + ) + mock_get_client.return_value = (mock_minio_client, "test-bucket") + + from app.utils.minio_utils import get_validation_status_from_minio + with pytest.raises(S3Error): + get_validation_status_from_minio("crate123") + + +@mock.patch("app.utils.minio_utils.get_minio_client_and_bucket", side_effect=ValueError("Missing config")) +def test_get_validation_status_value_error(mock_get_client): + from app.utils.minio_utils import get_validation_status_from_minio + with pytest.raises(ValueError): + get_validation_status_from_minio("crate123") + + +@mock.patch("app.utils.minio_utils.get_minio_client_and_bucket") +def test_get_validation_status_generic_exception(mock_get_client): + mock_minio_client = mock.Mock() + mock_minio_client.get_object.side_effect = RuntimeError("Unexpected failure") + mock_get_client.return_value = (mock_minio_client, "test-bucket") + + from app.utils.minio_utils import get_validation_status_from_minio + with pytest.raises(RuntimeError): + get_validation_status_from_minio("crate123") From b68727338b963480a7dec378089193a3263fae3b Mon Sep 17 00:00:00 2001 From: Douglas Lowe <10961945+douglowe@users.noreply.github.com> Date: Thu, 17 Jul 2025 14:20:00 +0100 Subject: [PATCH 2/6] remove unneeded commented code --- tests/test_minio.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/tests/test_minio.py b/tests/test_minio.py index 18938e1..0da8a0b 100644 --- a/tests/test_minio.py +++ b/tests/test_minio.py @@ -15,12 +15,6 @@ def mock_minio_response(): response.data.decode.return_value = json.dumps({"status": "valid"}) return response -# @pytest.fixture -# def mock_minio_response(): -# mock_response = mock.Mock() -# return mock_response -# mock_response.data.decode.return_value = json.dumps({"status": "valid"}) - # Testing function: get_minio_client_and_bucket def test_get_minio_client_success(monkeypatch): From 5fb3ea9f8b7c9db1a0836bd3e61d5bc228ab81ee Mon Sep 17 00:00:00 2001 From: Douglas Lowe <10961945+douglowe@users.noreply.github.com> Date: Thu, 17 Jul 2025 14:27:00 +0100 Subject: [PATCH 3/6] add unit tests to GH workflow --- .github/workflows/unit_tests.yml | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 .github/workflows/unit_tests.yml diff --git a/.github/workflows/unit_tests.yml b/.github/workflows/unit_tests.yml new file mode 100644 index 0000000..d53568a --- /dev/null +++ b/.github/workflows/unit_tests.yml @@ -0,0 +1,28 @@ +name: Unit Tests + +on: + pull_request: + branches: [ develop ] + +jobs: + test: + runs-on: ubuntu-latest + + steps: + - name: Checkout code + uses: actions/checkout@v4 + + - name: Set up Python + uses: actions/setup-python@v5 + with: + python-version: '3.11' + + - name: Install dependencies + run: | + python -m pip install --upgrade pip + pip install -r requirements.txt + pip install pytest + + - name: Run tests (excluding integration tests) + run: | + pytest --ignore=tests/test_integration.py From 36cb6bd87f80c9499168a364c8299a277c97782c Mon Sep 17 00:00:00 2001 From: Douglas Lowe <10961945+douglowe@users.noreply.github.com> Date: Thu, 17 Jul 2025 14:46:24 +0100 Subject: [PATCH 4/6] try running pytest as python module --- .github/workflows/unit_tests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/unit_tests.yml b/.github/workflows/unit_tests.yml index d53568a..2912dd2 100644 --- a/.github/workflows/unit_tests.yml +++ b/.github/workflows/unit_tests.yml @@ -25,4 +25,4 @@ jobs: - name: Run tests (excluding integration tests) run: | - pytest --ignore=tests/test_integration.py + python -m pytest --ignore=tests/test_integration.py From c12ade7fea120149fce14e4db0b5f217e1c79838 Mon Sep 17 00:00:00 2001 From: Douglas Lowe <10961945+douglowe@users.noreply.github.com> Date: Thu, 17 Jul 2025 14:54:58 +0100 Subject: [PATCH 5/6] add pytest-mock to pip install --- .github/workflows/unit_tests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/unit_tests.yml b/.github/workflows/unit_tests.yml index 2912dd2..7c0a996 100644 --- a/.github/workflows/unit_tests.yml +++ b/.github/workflows/unit_tests.yml @@ -21,7 +21,7 @@ jobs: run: | python -m pip install --upgrade pip pip install -r requirements.txt - pip install pytest + pip install pytest pytest-mock - name: Run tests (excluding integration tests) run: | From 2572150c96a8b91e51978bf45498713a627461ce Mon Sep 17 00:00:00 2001 From: Douglas Lowe <10961945+douglowe@users.noreply.github.com> Date: Thu, 17 Jul 2025 15:01:14 +0100 Subject: [PATCH 6/6] remove partial option for validating json --- app/ro_crates/routes/post_routes.py | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/app/ro_crates/routes/post_routes.py b/app/ro_crates/routes/post_routes.py index 2b98d1a..9f9f909 100644 --- a/app/ro_crates/routes/post_routes.py +++ b/app/ro_crates/routes/post_routes.py @@ -5,25 +5,27 @@ # Copyright (c) 2025 eScience Lab, The University of Manchester from apiflask import APIBlueprint, Schema -from apiflask.fields import Integer, String -from flask import request, Response +from apiflask.fields import String +from flask import Response from app.services.validation_service import ( - queue_ro_crate_validation_task, + queue_ro_crate_validation_task, queue_ro_crate_metadata_validation_task ) post_routes_bp = APIBlueprint("post_routes", __name__) + class ValidateData(Schema): crate_id = String(required=True) profile_name = String(required=False) webhook_url = String(required=False) + class ValidateJSON(Schema): crate_json = String(required=True) profile_name = String(required=False) - + @post_routes_bp.post("/validate_by_id") @post_routes_bp.input(ValidateData(partial=True), location='json') @@ -59,8 +61,9 @@ def validate_ro_crate_from_id(json_data) -> tuple[Response, int]: return queue_ro_crate_validation_task(crate_id, profile_name, webhook_url) + @post_routes_bp.post("/validate_by_id_no_webhook") -@post_routes_bp.input(ValidateData(partial=True), location='json') # -> json_data +@post_routes_bp.input(ValidateData(partial=True), location='json') # -> json_data def validate_ro_crate_from_id_no_webhook(json_data) -> tuple[Response, int]: """ Endpoint to validate an RO-Crate using its ID from MinIO. @@ -90,7 +93,7 @@ def validate_ro_crate_from_id_no_webhook(json_data) -> tuple[Response, int]: @post_routes_bp.post("/validate_metadata") -@post_routes_bp.input(ValidateJSON(partial=True), location='json') # -> json_data +@post_routes_bp.input(ValidateJSON(partial=False), location='json') # -> json_data def validate_ro_crate_metadata(json_data) -> tuple[Response, int]: """ Endpoint to validate an RO-Crate JSON file uploaded to the Service. @@ -117,4 +120,3 @@ def validate_ro_crate_metadata(json_data) -> tuple[Response, int]: profile_name = None return queue_ro_crate_metadata_validation_task(crate_json, profile_name) -