Compare commits
15
Commits
e39fc96ac1
..
v1.0.0
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
2f19fcc972 | ||
|
|
3bd65895ec | ||
|
|
5311d49fa6 | ||
|
|
703bb9521f | ||
|
|
8e47ebf4c6 | ||
|
|
9a800bf553 | ||
|
|
8c34534187 | ||
|
|
b1210bddf2 | ||
|
|
39b104383c | ||
|
|
beb2b5128e | ||
|
|
ce062be411 | ||
|
|
a381d45650 | ||
|
|
5c8cd841b6 | ||
|
|
34632980ba | ||
|
|
5393efc6cd |
@@ -16,8 +16,14 @@ jobs:
|
||||
|
||||
- name: Check PR title when source files change
|
||||
env:
|
||||
PR_TITLE: ${{ github.event.pull_request.title }}
|
||||
API_URL: ${{ vars.API_URL }}
|
||||
REPO_OWNER: ${{ github.repository_owner }}
|
||||
REPO_NAME: ${{ github.event.repository.name }}
|
||||
PR_NUMBER: ${{ github.event.pull_request.number }}
|
||||
CI_RUNNER_TOKEN: ${{ secrets.CI_RUNNER_TOKEN }}
|
||||
run: |
|
||||
git fetch origin "${{ github.base_ref }}"
|
||||
PR_TITLE=$(scripts/ci/fetch-pr-title.sh)
|
||||
echo "Live PR title: ${PR_TITLE}"
|
||||
git diff --name-only "origin/${{ github.base_ref }}...HEAD" \
|
||||
| scripts/ci/check-pr-title.sh "$PR_TITLE"
|
||||
|
||||
@@ -28,9 +28,11 @@ jobs:
|
||||
|
||||
- name: Run unit tests
|
||||
run: |
|
||||
# --cov-fail-under=0: partial coverage only; floor is checked in coverage-report.
|
||||
uv run pytest tests/unit/ -m "not integration" \
|
||||
--cov=python_repositories \
|
||||
--cov-report=
|
||||
--cov-report= \
|
||||
--cov-fail-under=0
|
||||
|
||||
- name: Upload unit coverage
|
||||
uses: https://github.com/christopherHX/gitea-upload-artifact@v4
|
||||
@@ -64,9 +66,11 @@ jobs:
|
||||
|
||||
- name: Run integration tests
|
||||
run: |
|
||||
# --cov-fail-under=0: partial coverage only; floor is checked in coverage-report.
|
||||
uv run pytest -m integration \
|
||||
--cov=python_repositories \
|
||||
--cov-report=
|
||||
--cov-report= \
|
||||
--cov-fail-under=0
|
||||
|
||||
- name: Upload integration coverage
|
||||
uses: https://github.com/christopherHX/gitea-upload-artifact@v4
|
||||
@@ -77,6 +81,7 @@ jobs:
|
||||
compression-level: 0
|
||||
|
||||
coverage-report:
|
||||
# Merges unit + integration coverage and enforces fail_under from pyproject.toml.
|
||||
needs: [unit-tests, integration-tests]
|
||||
runs-on: ubuntu-latest
|
||||
if: github.event_name == 'pull_request' || github.event_name == 'push'
|
||||
@@ -111,9 +116,24 @@ jobs:
|
||||
|
||||
- name: Combine coverage report
|
||||
run: |
|
||||
# --fail-under=0 so the full report is always written before the floor check.
|
||||
uv run coverage combine coverage-unit/.coverage coverage-integration/.coverage
|
||||
uv run coverage report -m --include='python_repositories/*' > coverage.txt
|
||||
cat coverage.txt
|
||||
uv run coverage report -m --include='python_repositories/*' --fail-under=0 > coverage.txt
|
||||
|
||||
- name: Show coverage report
|
||||
run: cat coverage.txt
|
||||
|
||||
- name: Enforce coverage floor
|
||||
run: |
|
||||
# Reads fail_under from pyproject.toml; only combined coverage is evaluated here.
|
||||
FAIL_UNDER=$(python3 -c "import tomllib; print(tomllib.load(open('pyproject.toml', 'rb'))['tool']['coverage']['report']['fail_under'])")
|
||||
echo "Checking combined coverage against ${FAIL_UNDER}% floor..."
|
||||
if uv run coverage report --fail-under="$FAIL_UNDER" --include='python_repositories/*'; then
|
||||
echo "Coverage floor met."
|
||||
else
|
||||
echo "::error::Combined coverage is below the ${FAIL_UNDER}% floor"
|
||||
exit 1
|
||||
fi
|
||||
|
||||
- name: Post coverage summary to PR
|
||||
if: github.event_name == 'pull_request'
|
||||
|
||||
@@ -148,7 +148,14 @@ uv run pytest -v # full suite (requires Doc
|
||||
|
||||
Integration tests are marked with `@pytest.mark.integration` and require Docker (testcontainers). Run unit tests alone for quick local feedback.
|
||||
|
||||
CI runs unit and integration tests in parallel with coverage, then merges `.coverage` artifacts in a follow-up job (via [christopherhx/gitea-\*-artifact@v4](https://github.com/christopherHX/gitea-upload-artifact) for Gitea 1.26 compatibility).
|
||||
CI runs unit and integration tests in parallel with coverage, then merges `.coverage` artifacts in a follow-up job (via [christopherhx/gitea-\*-artifact@v4](https://github.com/christopherHX/gitea-upload-artifact) for Gitea 1.26 compatibility). Combined coverage must be at least **90%**; the floor is set by [`fail_under` in `pyproject.toml`](pyproject.toml#L45-L48) and enforced after merging unit and integration coverage, not on unit-only runs.
|
||||
|
||||
To check coverage locally (requires Docker for the full suite):
|
||||
|
||||
```bash
|
||||
uv run pytest --cov=python_repositories --cov-report=
|
||||
uv run coverage report
|
||||
```
|
||||
|
||||
`pre-commit` is included in the dev dependency group. `uv sync` installs the CLI, but git does not run hooks until you install them with `pre-commit install` (one time per clone). After that, commits run the checks defined in [`.pre-commit-config.yaml`](.pre-commit-config.yaml) (ruff, mypy, pyupgrade, prettier, and general file hygiene).
|
||||
|
||||
|
||||
+9
-1
@@ -1,6 +1,6 @@
|
||||
[project]
|
||||
name = "python-repositories"
|
||||
version = "0.5.0"
|
||||
version = "1.0.0"
|
||||
description = "Various python repository interfaces exposed as a python package."
|
||||
authors = [
|
||||
{ name = "Brian Bjarke Jensen", email = "[email protected]" }
|
||||
@@ -39,6 +39,14 @@ markers = [
|
||||
"integration: tests requiring Docker containers (deselect with '-m \"not integration\"')",
|
||||
]
|
||||
|
||||
[tool.coverage.run]
|
||||
source = ["python_repositories"]
|
||||
|
||||
[tool.coverage.report]
|
||||
fail_under = 100
|
||||
show_missing = true
|
||||
precision = 2
|
||||
|
||||
[tool.uv.sources]
|
||||
python-utils = { index = "gitea" }
|
||||
|
||||
|
||||
@@ -152,6 +152,7 @@ class MinioAdapter(ObjectRepositoryInterface, ConnectionAwareAdapter):
|
||||
assert self._client is not None and self._bucket_name is not None
|
||||
# Get data from bucket
|
||||
# N.B. bucket name is set when connecting
|
||||
response = None
|
||||
try:
|
||||
response = self._client.get_object(
|
||||
bucket_name=self._bucket_name,
|
||||
@@ -171,11 +172,12 @@ class MinioAdapter(ObjectRepositoryInterface, ConnectionAwareAdapter):
|
||||
self.logger.warning(
|
||||
f"Object '{object_name}' not found in bucket '{self._bucket_name}'"
|
||||
)
|
||||
else:
|
||||
self.logger.error(repr(exc))
|
||||
except Exception as exc: # pylint: disable=broad-except
|
||||
self.logger.error(repr(exc))
|
||||
return None
|
||||
raise
|
||||
finally:
|
||||
if response is not None:
|
||||
response.close()
|
||||
response.release_conn()
|
||||
|
||||
def delete(self, object_name: str) -> None:
|
||||
"""Delete an object from the Minio bucket."""
|
||||
|
||||
@@ -9,7 +9,11 @@ class ObjectRepositoryInterface(ABC):
|
||||
|
||||
@abstractmethod
|
||||
def get(self, object_name: str) -> BytesIO | None:
|
||||
"""Get an object by name."""
|
||||
"""Get an object by name.
|
||||
|
||||
Returns None when the object does not exist. Raises ConnectionError when
|
||||
not connected. Other backend errors propagate to the caller.
|
||||
"""
|
||||
...
|
||||
|
||||
@abstractmethod
|
||||
|
||||
Executable
+16
@@ -0,0 +1,16 @@
|
||||
#!/usr/bin/env bash
|
||||
set -euo pipefail
|
||||
|
||||
# Fetch the current PR title from the Gitea API.
|
||||
# Requires: API_URL, REPO_OWNER, REPO_NAME, PR_NUMBER, CI_RUNNER_TOKEN
|
||||
|
||||
: "${API_URL:?API_URL is required}"
|
||||
: "${REPO_OWNER:?REPO_OWNER is required}"
|
||||
: "${REPO_NAME:?REPO_NAME is required}"
|
||||
: "${PR_NUMBER:?PR_NUMBER is required}"
|
||||
: "${CI_RUNNER_TOKEN:?CI_RUNNER_TOKEN is required}"
|
||||
|
||||
curl -sf \
|
||||
"${API_URL}/repos/${REPO_OWNER}/${REPO_NAME}/pulls/${PR_NUMBER}" \
|
||||
-H "Authorization: token ${CI_RUNNER_TOKEN}" \
|
||||
| python3 -c 'import json, sys; print(json.load(sys.stdin)["title"])'
|
||||
@@ -218,10 +218,8 @@ def test_should_log_warning_when_getting_nonexistent_object(
|
||||
)
|
||||
|
||||
|
||||
def test_should_log_error_when_getting_with_s3error_other_than_no_such_key(
|
||||
caplog: pytest.LogCaptureFixture,
|
||||
) -> None:
|
||||
"""Test that the MinioAdapter logs an error for unhandled S3 errors."""
|
||||
def test_should_reraise_s3error_other_than_no_such_key() -> None:
|
||||
"""Test that the MinioAdapter re-raises unhandled S3 errors."""
|
||||
mock_client = MagicMock(spec=Minio)
|
||||
mock_client.bucket_exists.return_value = True
|
||||
other_s3error = S3Error(
|
||||
@@ -236,25 +234,20 @@ def test_should_log_error_when_getting_with_s3error_other_than_no_such_key(
|
||||
)
|
||||
mock_client.get_object.side_effect = other_s3error
|
||||
adapter = MinioAdapter(config=TEST_MINIO_CONFIG, client=mock_client)
|
||||
with caplog.at_level("ERROR"):
|
||||
result = adapter.get("missing-object")
|
||||
assert result is None
|
||||
assert repr(other_s3error) in caplog.text
|
||||
with pytest.raises(S3Error) as exc_info:
|
||||
adapter.get("missing-object")
|
||||
assert exc_info.value.code == "UnhandledError"
|
||||
|
||||
|
||||
def test_should_log_error_when_getting_with_general_exception(
|
||||
caplog: pytest.LogCaptureFixture,
|
||||
) -> None:
|
||||
"""Test that the MinioAdapter logs an error on general exceptions during get."""
|
||||
def test_should_reraise_general_exception() -> None:
|
||||
"""Test that the MinioAdapter re-raises general exceptions during get."""
|
||||
mock_client = MagicMock(spec=Minio)
|
||||
mock_client.bucket_exists.return_value = True
|
||||
general_exception = Exception("General failure")
|
||||
mock_client.get_object.side_effect = general_exception
|
||||
adapter = MinioAdapter(config=TEST_MINIO_CONFIG, client=mock_client)
|
||||
with caplog.at_level("ERROR"):
|
||||
result = adapter.get("missing-object")
|
||||
assert result is None
|
||||
assert repr(general_exception) in caplog.text
|
||||
with pytest.raises(Exception, match="General failure"):
|
||||
adapter.get("missing-object")
|
||||
|
||||
|
||||
def test_should_put_data(
|
||||
|
||||
@@ -117,3 +117,24 @@ def test_adapters_subpackage_lazy_import_succeeds() -> None:
|
||||
from python_repositories.adapters import RedisAdapter
|
||||
|
||||
assert RedisAdapter.__name__ == "RedisAdapter"
|
||||
|
||||
|
||||
def test_adapters_dir_exposes_lazy_exports() -> None:
|
||||
"""dir(adapters) includes lazy adapter names for tab completion."""
|
||||
import python_repositories.adapters as adapters
|
||||
|
||||
assert {"RedisAdapter", "MinioAdapter"}.issubset(set(dir(adapters)))
|
||||
|
||||
|
||||
def test_adapters_getattr_raises_for_unknown() -> None:
|
||||
"""Unknown adapter names raise AttributeError."""
|
||||
import python_repositories.adapters as adapters
|
||||
|
||||
with pytest.raises(AttributeError, match="has no attribute 'NoSuchAdapter'"):
|
||||
_ = adapters.NoSuchAdapter
|
||||
|
||||
|
||||
def test_top_level_dir_exposes_lazy_exports() -> None:
|
||||
"""dir(python_repositories) includes lazy adapter names for tab completion."""
|
||||
assert "RedisAdapter" in dir(python_repositories)
|
||||
assert "MinioAdapter" in dir(python_repositories)
|
||||
|
||||
@@ -5,7 +5,8 @@ from __future__ import annotations
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
import pytest
|
||||
from minio import Minio
|
||||
from minio import Minio, S3Error
|
||||
from urllib3.response import BaseHTTPResponse
|
||||
|
||||
from python_repositories.adapters.minio_adapter import MinioAdapter
|
||||
from python_repositories.interfaces import ObjectRepositoryInterface
|
||||
@@ -53,3 +54,129 @@ def test_raises_when_client_provided_without_config() -> None:
|
||||
mock_client = MagicMock(spec=Minio)
|
||||
with pytest.raises(ValueError, match="config is required"):
|
||||
MinioAdapter(client=mock_client)
|
||||
|
||||
|
||||
def test_connect_with_injected_client_succeeds() -> None:
|
||||
mock_client = MagicMock(spec=Minio)
|
||||
adapter = MinioAdapter(config=TEST_MINIO_CONFIG, client=mock_client)
|
||||
|
||||
adapter.connect()
|
||||
|
||||
mock_client.list_buckets.assert_called_once()
|
||||
|
||||
|
||||
def test_connect_with_injected_client_raises_on_failure() -> None:
|
||||
mock_client = MagicMock(spec=Minio)
|
||||
mock_client.list_buckets.side_effect = Exception("connection lost")
|
||||
adapter = MinioAdapter(config=TEST_MINIO_CONFIG, client=mock_client)
|
||||
|
||||
with pytest.raises(ConnectionError, match="Could not connect to Minio"):
|
||||
adapter.connect()
|
||||
|
||||
|
||||
def test_connect_disconnects_before_reconnect(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
stale_client = MagicMock(spec=Minio)
|
||||
new_client = MagicMock(spec=Minio)
|
||||
adapter = MinioAdapter(config=TEST_MINIO_CONFIG)
|
||||
adapter._client = stale_client
|
||||
adapter._bucket_name = None
|
||||
|
||||
monkeypatch.setattr(
|
||||
"python_repositories.adapters.minio_adapter.minio.Minio",
|
||||
lambda *args, **kwargs: new_client,
|
||||
)
|
||||
|
||||
adapter.connect()
|
||||
|
||||
assert adapter._client is new_client
|
||||
new_client.list_buckets.assert_called_once()
|
||||
|
||||
|
||||
def test_get_closes_response_on_success() -> None:
|
||||
"""get() must close and release the get_object HTTP response."""
|
||||
mock_client = MagicMock(spec=Minio)
|
||||
mock_client.bucket_exists.return_value = True
|
||||
mock_response = MagicMock()
|
||||
mock_response.read.side_effect = [b"data", b""]
|
||||
mock_client.get_object.return_value = mock_response
|
||||
adapter = MinioAdapter(config=TEST_MINIO_CONFIG, client=mock_client)
|
||||
|
||||
result = adapter.get("some-object")
|
||||
|
||||
assert result is not None
|
||||
assert result.read() == b"data"
|
||||
mock_response.close.assert_called_once()
|
||||
mock_response.release_conn.assert_called_once()
|
||||
|
||||
|
||||
def test_get_closes_response_when_read_fails() -> None:
|
||||
"""get() must close and release the response even if read() raises."""
|
||||
mock_client = MagicMock(spec=Minio)
|
||||
mock_client.bucket_exists.return_value = True
|
||||
mock_response = MagicMock()
|
||||
mock_response.read.side_effect = OSError("connection reset")
|
||||
mock_client.get_object.return_value = mock_response
|
||||
adapter = MinioAdapter(config=TEST_MINIO_CONFIG, client=mock_client)
|
||||
|
||||
with pytest.raises(OSError, match="connection reset"):
|
||||
adapter.get("some-object")
|
||||
|
||||
mock_response.close.assert_called_once()
|
||||
mock_response.release_conn.assert_called_once()
|
||||
|
||||
|
||||
def test_get_returns_none_for_no_such_key() -> None:
|
||||
"""get() returns None when the object does not exist."""
|
||||
mock_client = MagicMock(spec=Minio)
|
||||
mock_client.bucket_exists.return_value = True
|
||||
mock_client.get_object.side_effect = S3Error(
|
||||
MagicMock(spec=BaseHTTPResponse),
|
||||
"NoSuchKey",
|
||||
"",
|
||||
"",
|
||||
"",
|
||||
"",
|
||||
bucket_name="test-bucket",
|
||||
object_name="missing-object",
|
||||
)
|
||||
adapter = MinioAdapter(config=TEST_MINIO_CONFIG, client=mock_client)
|
||||
|
||||
result = adapter.get("missing-object")
|
||||
|
||||
assert result is None
|
||||
|
||||
|
||||
def test_get_reraises_other_s3_errors() -> None:
|
||||
"""get() re-raises S3 errors other than NoSuchKey."""
|
||||
mock_client = MagicMock(spec=Minio)
|
||||
mock_client.bucket_exists.return_value = True
|
||||
other_s3error = S3Error(
|
||||
MagicMock(spec=BaseHTTPResponse),
|
||||
"AccessDenied",
|
||||
"",
|
||||
"",
|
||||
"",
|
||||
"",
|
||||
bucket_name="test-bucket",
|
||||
object_name="some-object",
|
||||
)
|
||||
mock_client.get_object.side_effect = other_s3error
|
||||
adapter = MinioAdapter(config=TEST_MINIO_CONFIG, client=mock_client)
|
||||
|
||||
with pytest.raises(S3Error) as exc_info:
|
||||
adapter.get("some-object")
|
||||
|
||||
assert exc_info.value.code == "AccessDenied"
|
||||
|
||||
|
||||
def test_get_reraises_general_exception_from_get_object() -> None:
|
||||
"""get() re-raises unexpected exceptions from get_object."""
|
||||
mock_client = MagicMock(spec=Minio)
|
||||
mock_client.bucket_exists.return_value = True
|
||||
mock_client.get_object.side_effect = Exception("General failure")
|
||||
adapter = MinioAdapter(config=TEST_MINIO_CONFIG, client=mock_client)
|
||||
|
||||
with pytest.raises(Exception, match="General failure"):
|
||||
adapter.get("some-object")
|
||||
|
||||
@@ -69,3 +69,48 @@ def test_subclass_custom_env_var_name(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
monkeypatch.setenv("CUSTOM_REDIS_URI", "redis://custom:6379")
|
||||
adapter = CustomEnvRedisAdapter()
|
||||
assert adapter._config.uri == "redis://custom:6379"
|
||||
|
||||
|
||||
def test_connect_with_injected_client_succeeds_when_ping_ok() -> None:
|
||||
mock_client = MagicMock(spec=redis.Redis)
|
||||
mock_client.ping.return_value = True
|
||||
adapter = RedisAdapter(config=TEST_REDIS_CONFIG, client=mock_client)
|
||||
|
||||
adapter.connect()
|
||||
|
||||
mock_client.ping.assert_called_once()
|
||||
|
||||
|
||||
def test_connect_with_injected_client_raises_when_ping_false() -> None:
|
||||
mock_client = MagicMock(spec=redis.Redis)
|
||||
mock_client.ping.return_value = False
|
||||
adapter = RedisAdapter(config=TEST_REDIS_CONFIG, client=mock_client)
|
||||
|
||||
with pytest.raises(ConnectionError, match="Could not connect to Redis"):
|
||||
adapter.connect()
|
||||
|
||||
|
||||
def test_connect_with_injected_client_raises_on_redis_error() -> None:
|
||||
mock_client = MagicMock(spec=redis.Redis)
|
||||
mock_client.ping.side_effect = redis.ConnectionError("connection lost")
|
||||
adapter = RedisAdapter(config=TEST_REDIS_CONFIG, client=mock_client)
|
||||
|
||||
with pytest.raises(ConnectionError, match="Could not connect to Redis"):
|
||||
adapter.connect()
|
||||
|
||||
|
||||
def test_connect_closes_existing_non_injected_client(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
stale_client = MagicMock(spec=redis.Redis)
|
||||
new_client = MagicMock(spec=redis.Redis)
|
||||
new_client.ping.return_value = True
|
||||
adapter = RedisAdapter(config=TEST_REDIS_CONFIG)
|
||||
adapter._client = stale_client
|
||||
|
||||
monkeypatch.setattr("redis.Redis.from_url", lambda *args, **kwargs: new_client)
|
||||
|
||||
adapter.connect()
|
||||
|
||||
stale_client.close.assert_called_once()
|
||||
assert adapter._client is new_client
|
||||
|
||||
Reference in New Issue
Block a user