Skip to content

Commit d2bd1b6

Browse files
author
Bogdan-Marius-Catanus
committed
test: improve coverage for leader identification feature
- Add test for health check exception handling when is_leader() fails - Add tests for GatewayService initialization with invalid/None port values - Add comprehensive tests for is_leader() method covering JSON metadata, legacy UUID, and edge cases - Fix is_leader() to return False when Redis is unavailable (was incorrectly returning True) - All tests passing: 15,833 passed (9 new tests added) Addresses coverage gaps identified in diff-cover report for lines: - mcpgateway/main.py: 10346, 10348 (exception handling) - mcpgateway/services/gateway_service.py: 478-479 (port conversion), 4133-4152 (is_leader logic) Signed-off-by: Bogdan-Marius-Catanus <bogdan-marius.catanus@ibm.com>
1 parent 0557e3b commit d2bd1b6

4 files changed

Lines changed: 157 additions & 9 deletions

File tree

.secrets.baseline

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
"files": "^.secrets.baseline$",
44
"lines": null
55
},
6-
"generated_at": "2026-04-01T14:06:09Z",
6+
"generated_at": "2026-04-01T14:37:32Z",
77
"plugins_used": [
88
{
99
"name": "AWSKeyDetector"

mcpgateway/services/gateway_service.py

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -478,12 +478,7 @@ def __init__(self) -> None:
478478
except (ValueError, TypeError):
479479
port_value = 0
480480

481-
self._instance_metadata = {
482-
"instance_id": str(uuid.uuid4()),
483-
"port": port_value,
484-
"pid": os.getpid(),
485-
"hostname": socket.gethostname()
486-
}
481+
self._instance_metadata = {"instance_id": str(uuid.uuid4()), "port": port_value, "pid": os.getpid(), "hostname": socket.gethostname()}
487482
self._instance_id = self._instance_metadata["instance_id"]
488483

489484
# Leader election settings from config
@@ -4131,8 +4126,8 @@ async def is_leader(self) -> bool:
41314126
bool: True if this instance holds the leader lock, False otherwise.
41324127
"""
41334128
if not self._redis_client or not hasattr(self, "_leader_key"):
4134-
# Fallback to file lock for non-Redis setups
4135-
return True
4129+
# Redis unavailable - cannot verify leadership in cluster mode
4130+
return False
41364131

41374132
try:
41384133
current_leader_raw = await self._redis_client.get(self._leader_key)

tests/unit/mcpgateway/services/test_gateway_service_redis_leadership.py

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -380,3 +380,129 @@ async def test_skips_cancel_when_old_task_done(self):
380380
await service._run_follower_election("test@example.com") # pylint: disable=protected-access
381381

382382
done_task.cancel.assert_not_called()
383+
384+
385+
386+
class TestGatewayServiceInitialization:
387+
"""Tests for GatewayService initialization with instance metadata."""
388+
389+
def test_init_with_invalid_port(self):
390+
"""Test GatewayService initialization with invalid port value."""
391+
with patch("mcpgateway.services.gateway_service.settings") as mock_settings:
392+
mock_settings.port = "invalid"
393+
mock_settings.platform_admin_email = "admin@test.com"
394+
395+
service = GatewayService()
396+
397+
# Should default to 0 when port conversion fails
398+
assert service._instance_metadata["port"] == 0
399+
assert "instance_id" in service._instance_metadata
400+
assert "pid" in service._instance_metadata
401+
assert "hostname" in service._instance_metadata
402+
403+
def test_init_with_none_port(self):
404+
"""Test GatewayService initialization with None port value."""
405+
with patch("mcpgateway.services.gateway_service.settings") as mock_settings:
406+
mock_settings.port = None
407+
mock_settings.platform_admin_email = "admin@test.com"
408+
409+
service = GatewayService()
410+
411+
# Should default to 0 when port is None
412+
assert service._instance_metadata["port"] == 0
413+
assert "instance_id" in service._instance_metadata
414+
assert "pid" in service._instance_metadata
415+
assert "hostname" in service._instance_metadata
416+
417+
def test_init_with_valid_port(self):
418+
"""Test GatewayService initialization with valid port value."""
419+
with patch("mcpgateway.services.gateway_service.settings") as mock_settings:
420+
mock_settings.port = "8080"
421+
mock_settings.platform_admin_email = "admin@test.com"
422+
423+
service = GatewayService()
424+
425+
# Should convert port to int
426+
assert service._instance_metadata["port"] == 8080
427+
assert "instance_id" in service._instance_metadata
428+
assert "pid" in service._instance_metadata
429+
assert "hostname" in service._instance_metadata
430+
431+
432+
class TestIsLeaderMethod:
433+
"""Tests for is_leader() method."""
434+
435+
@pytest.mark.asyncio
436+
async def test_is_leader_with_json_metadata(self):
437+
"""Test is_leader() correctly parses JSON metadata from Redis."""
438+
import json
439+
440+
redis_mock = AsyncMock()
441+
metadata = {
442+
"instance_id": "test-id-123",
443+
"port": 8001,
444+
"pid": 12345,
445+
"hostname": "test-host"
446+
}
447+
redis_mock.get = AsyncMock(return_value=json.dumps(metadata))
448+
449+
service = _make_service(redis_client=redis_mock, instance_id="test-id-123")
450+
451+
result = await service.is_leader()
452+
453+
assert result is True
454+
redis_mock.get.assert_called_once_with("test:leader")
455+
456+
@pytest.mark.asyncio
457+
async def test_is_leader_with_legacy_uuid(self):
458+
"""Test is_leader() handles legacy UUID-only format."""
459+
redis_mock = AsyncMock()
460+
redis_mock.get = AsyncMock(return_value="test-id-123") # Plain UUID string
461+
462+
service = _make_service(redis_client=redis_mock, instance_id="test-id-123")
463+
464+
result = await service.is_leader()
465+
466+
assert result is True
467+
redis_mock.get.assert_called_once_with("test:leader")
468+
469+
@pytest.mark.asyncio
470+
async def test_is_leader_returns_false_when_not_leader(self):
471+
"""Test is_leader() returns False when instance is not the leader."""
472+
import json
473+
474+
redis_mock = AsyncMock()
475+
metadata = {
476+
"instance_id": "other-id-456",
477+
"port": 8002,
478+
"pid": 67890,
479+
"hostname": "other-host"
480+
}
481+
redis_mock.get = AsyncMock(return_value=json.dumps(metadata))
482+
483+
service = _make_service(redis_client=redis_mock, instance_id="test-id-123")
484+
485+
result = await service.is_leader()
486+
487+
assert result is False
488+
489+
@pytest.mark.asyncio
490+
async def test_is_leader_returns_false_when_no_leader(self):
491+
"""Test is_leader() returns False when no leader exists in Redis."""
492+
redis_mock = AsyncMock()
493+
redis_mock.get = AsyncMock(return_value=None)
494+
495+
service = _make_service(redis_client=redis_mock, instance_id="test-id-123")
496+
497+
result = await service.is_leader()
498+
499+
assert result is False
500+
501+
@pytest.mark.asyncio
502+
async def test_is_leader_returns_false_when_redis_unavailable(self):
503+
"""Test is_leader() returns False when Redis client is unavailable."""
504+
service = _make_service(redis_client=None, instance_id="test-id-123")
505+
506+
result = await service.is_leader()
507+
508+
assert result is False

tests/unit/mcpgateway/test_main.py

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -499,6 +499,33 @@ def close(self):
499499
assert response["status"] == "unhealthy"
500500
assert session.invalidate_called is True
501501

502+
@pytest.mark.asyncio
503+
async def test_health_check_leader_exception(self):
504+
"""Test health check when is_leader() raises an exception."""
505+
# First-Party
506+
from mcpgateway import main as mcpgateway_main
507+
508+
class DummySession:
509+
def execute(self, *_args, **_kwargs):
510+
pass
511+
512+
def commit(self):
513+
pass
514+
515+
def close(self):
516+
pass
517+
518+
session = DummySession()
519+
520+
# Mock gateway_service to raise exception on is_leader()
521+
with patch("mcpgateway.main.SessionLocal", return_value=session):
522+
with patch("mcpgateway.main.gateway_service") as mock_gateway:
523+
mock_gateway.is_leader.side_effect = Exception("Redis connection failed")
524+
response = await mcpgateway_main.healthcheck()
525+
526+
assert response["status"] == "healthy"
527+
assert response["is_leader"] is False # Should fallback to False on exception
528+
502529
@pytest.mark.asyncio
503530
async def test_ready_check_db_error(self):
504531
"""Test readiness check error path with rollback failure."""

0 commit comments

Comments
 (0)