Merge pull request #354 from christianlouis/copilot/fix-ci-pipeline-test-failures
fix(tests): resolve CI test failures from asyncio event loop destruction and settings singleton reload
This commit is contained in:
+2
-2
@@ -11,7 +11,7 @@ from sqlalchemy.orm import Session
|
|||||||
|
|
||||||
from app.config import settings
|
from app.config import settings
|
||||||
from app.database import get_db
|
from app.database import get_db
|
||||||
from app.utils.input_validation import validate_setting_key
|
from app.utils.input_validation import validate_setting_key, validate_setting_key_format
|
||||||
from app.utils.settings_service import (
|
from app.utils.settings_service import (
|
||||||
SETTING_METADATA,
|
SETTING_METADATA,
|
||||||
delete_setting_from_db,
|
delete_setting_from_db,
|
||||||
@@ -99,7 +99,7 @@ async def get_setting(key: str, request: Request, db: DbSession, admin: AdminUse
|
|||||||
Get a specific setting by key.
|
Get a specific setting by key.
|
||||||
Admin only.
|
Admin only.
|
||||||
"""
|
"""
|
||||||
validate_setting_key(key)
|
validate_setting_key_format(key)
|
||||||
try:
|
try:
|
||||||
# Get current value
|
# Get current value
|
||||||
value = getattr(settings, key, None)
|
value = getattr(settings, key, None)
|
||||||
|
|||||||
@@ -126,6 +126,31 @@ def validate_task_id(task_id: str) -> str:
|
|||||||
return task_id
|
return task_id
|
||||||
|
|
||||||
|
|
||||||
|
def validate_setting_key_format(key: str) -> str:
|
||||||
|
"""
|
||||||
|
Validate that *key* has a valid setting key format (alphanumeric + underscore,
|
||||||
|
starting with a letter). Does **not** check whether the key exists in the
|
||||||
|
``SETTING_METADATA`` registry — use this for read-only lookups where an
|
||||||
|
unknown key should return ``None`` rather than a 404 error.
|
||||||
|
|
||||||
|
Args:
|
||||||
|
key: The setting key supplied by the client.
|
||||||
|
|
||||||
|
Returns:
|
||||||
|
The validated setting key (unchanged).
|
||||||
|
|
||||||
|
Raises:
|
||||||
|
HTTPException 400: If the key contains invalid characters.
|
||||||
|
"""
|
||||||
|
if not _SETTING_KEY_RE.match(key):
|
||||||
|
logger.warning(f"Invalid setting key format rejected: {key!r}")
|
||||||
|
raise HTTPException(
|
||||||
|
status_code=status.HTTP_400_BAD_REQUEST,
|
||||||
|
detail="Invalid setting key format",
|
||||||
|
)
|
||||||
|
return key
|
||||||
|
|
||||||
|
|
||||||
def validate_setting_key(key: str) -> str:
|
def validate_setting_key(key: str) -> str:
|
||||||
"""
|
"""
|
||||||
Validate that *key* is a syntactically valid setting key.
|
Validate that *key* is a syntactically valid setting key.
|
||||||
|
|||||||
@@ -226,7 +226,7 @@ class TestListCredentials:
|
|||||||
for key in SETTING_METADATA:
|
for key in SETTING_METADATA:
|
||||||
setattr(mock_settings, key, None)
|
setattr(mock_settings, key, None)
|
||||||
|
|
||||||
result = asyncio.get_event_loop().run_until_complete(list_credentials(mock_request, mock_db, mock_admin))
|
result = asyncio.run(list_credentials(mock_request, mock_db, mock_admin))
|
||||||
|
|
||||||
returned_keys = {c["key"] for c in result["credentials"]}
|
returned_keys = {c["key"] for c in result["credentials"]}
|
||||||
sensitive_keys = {k for k, v in SETTING_METADATA.items() if v.get("sensitive")}
|
sensitive_keys = {k for k, v in SETTING_METADATA.items() if v.get("sensitive")}
|
||||||
@@ -247,7 +247,7 @@ class TestListCredentials:
|
|||||||
with patch("app.api.settings.settings") as mock_settings:
|
with patch("app.api.settings.settings") as mock_settings:
|
||||||
mock_settings.openai_api_key = "sk-env-key"
|
mock_settings.openai_api_key = "sk-env-key"
|
||||||
|
|
||||||
result = asyncio.get_event_loop().run_until_complete(list_credentials(mock_request, mock_db, mock_admin))
|
result = asyncio.run(list_credentials(mock_request, mock_db, mock_admin))
|
||||||
|
|
||||||
openai_entry = next(c for c in result["credentials"] if c["key"] == "openai_api_key")
|
openai_entry = next(c for c in result["credentials"] if c["key"] == "openai_api_key")
|
||||||
assert openai_entry["source"] == "db"
|
assert openai_entry["source"] == "db"
|
||||||
@@ -268,7 +268,7 @@ class TestListCredentials:
|
|||||||
with patch("app.api.settings.settings") as mock_settings:
|
with patch("app.api.settings.settings") as mock_settings:
|
||||||
mock_settings.openai_api_key = "sk-env-key"
|
mock_settings.openai_api_key = "sk-env-key"
|
||||||
|
|
||||||
result = asyncio.get_event_loop().run_until_complete(list_credentials(mock_request, mock_db, mock_admin))
|
result = asyncio.run(list_credentials(mock_request, mock_db, mock_admin))
|
||||||
|
|
||||||
openai_entry = next(c for c in result["credentials"] if c["key"] == "openai_api_key")
|
openai_entry = next(c for c in result["credentials"] if c["key"] == "openai_api_key")
|
||||||
assert openai_entry["source"] == "env"
|
assert openai_entry["source"] == "env"
|
||||||
@@ -289,7 +289,7 @@ class TestListCredentials:
|
|||||||
with patch("app.api.settings.settings") as mock_settings:
|
with patch("app.api.settings.settings") as mock_settings:
|
||||||
mock_settings.openai_api_key = None
|
mock_settings.openai_api_key = None
|
||||||
|
|
||||||
result = asyncio.get_event_loop().run_until_complete(list_credentials(mock_request, mock_db, mock_admin))
|
result = asyncio.run(list_credentials(mock_request, mock_db, mock_admin))
|
||||||
|
|
||||||
openai_entry = next(c for c in result["credentials"] if c["key"] == "openai_api_key")
|
openai_entry = next(c for c in result["credentials"] if c["key"] == "openai_api_key")
|
||||||
assert openai_entry["configured"] is False
|
assert openai_entry["configured"] is False
|
||||||
@@ -311,7 +311,7 @@ class TestListCredentials:
|
|||||||
# Most keys will be None, one will be set via db_settings mock
|
# Most keys will be None, one will be set via db_settings mock
|
||||||
mock_settings.openai_api_key = "sk-key"
|
mock_settings.openai_api_key = "sk-key"
|
||||||
|
|
||||||
result = asyncio.get_event_loop().run_until_complete(list_credentials(mock_request, mock_db, mock_admin))
|
result = asyncio.run(list_credentials(mock_request, mock_db, mock_admin))
|
||||||
|
|
||||||
assert result["total"] == len(result["credentials"])
|
assert result["total"] == len(result["credentials"])
|
||||||
assert result["configured_count"] + result["unconfigured_count"] == result["total"]
|
assert result["configured_count"] + result["unconfigured_count"] == result["total"]
|
||||||
@@ -331,7 +331,7 @@ class TestListCredentials:
|
|||||||
mock_admin = {"is_admin": True}
|
mock_admin = {"is_admin": True}
|
||||||
|
|
||||||
with pytest.raises(HTTPException) as exc_info:
|
with pytest.raises(HTTPException) as exc_info:
|
||||||
asyncio.get_event_loop().run_until_complete(list_credentials(mock_request, mock_db, mock_admin))
|
asyncio.run(list_credentials(mock_request, mock_db, mock_admin))
|
||||||
assert exc_info.value.status_code == 500
|
assert exc_info.value.status_code == 500
|
||||||
|
|
||||||
def test_list_credentials_endpoint_requires_admin(self, client):
|
def test_list_credentials_endpoint_requires_admin(self, client):
|
||||||
@@ -354,7 +354,7 @@ class TestListCredentials:
|
|||||||
with patch("app.api.settings.settings") as mock_settings:
|
with patch("app.api.settings.settings") as mock_settings:
|
||||||
mock_settings.openai_api_key = None
|
mock_settings.openai_api_key = None
|
||||||
|
|
||||||
result = asyncio.get_event_loop().run_until_complete(list_credentials(mock_request, mock_db, mock_admin))
|
result = asyncio.run(list_credentials(mock_request, mock_db, mock_admin))
|
||||||
|
|
||||||
for cred in result["credentials"]:
|
for cred in result["credentials"]:
|
||||||
assert "key" in cred
|
assert "key" in cred
|
||||||
|
|||||||
+4
-10
@@ -169,12 +169,9 @@ def test_cors_parse_comma_separated_origins():
|
|||||||
original = os.environ.get("CORS_ALLOWED_ORIGINS")
|
original = os.environ.get("CORS_ALLOWED_ORIGINS")
|
||||||
os.environ["CORS_ALLOWED_ORIGINS"] = "https://app.example.com,https://admin.example.com"
|
os.environ["CORS_ALLOWED_ORIGINS"] = "https://app.example.com,https://admin.example.com"
|
||||||
try:
|
try:
|
||||||
from importlib import reload
|
from app.config import Settings
|
||||||
|
|
||||||
import app.config as config_module
|
test_settings = Settings(
|
||||||
|
|
||||||
reload(config_module)
|
|
||||||
test_settings = config_module.Settings(
|
|
||||||
database_url="sqlite:///:memory:",
|
database_url="sqlite:///:memory:",
|
||||||
redis_url="redis://localhost:6379/0",
|
redis_url="redis://localhost:6379/0",
|
||||||
openai_api_key="test-key",
|
openai_api_key="test-key",
|
||||||
@@ -204,12 +201,9 @@ def test_cors_single_origin_string_to_list():
|
|||||||
original = os.environ.get("CORS_ALLOWED_ORIGINS")
|
original = os.environ.get("CORS_ALLOWED_ORIGINS")
|
||||||
os.environ["CORS_ALLOWED_ORIGINS"] = "https://app.example.com"
|
os.environ["CORS_ALLOWED_ORIGINS"] = "https://app.example.com"
|
||||||
try:
|
try:
|
||||||
from importlib import reload
|
from app.config import Settings
|
||||||
|
|
||||||
import app.config as config_module
|
test_settings = Settings(
|
||||||
|
|
||||||
reload(config_module)
|
|
||||||
test_settings = config_module.Settings(
|
|
||||||
database_url="sqlite:///:memory:",
|
database_url="sqlite:///:memory:",
|
||||||
redis_url="redis://localhost:6379/0",
|
redis_url="redis://localhost:6379/0",
|
||||||
openai_api_key="test-key",
|
openai_api_key="test-key",
|
||||||
|
|||||||
Reference in New Issue
Block a user