diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 2683aaf..8026ee7 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -67,7 +67,7 @@ repos: pass_filenames: false verbose: true args: - - "--fail-under=50" + - "--fail-under=100" - "--skip-empty" - "--skip-covered" - "--show-missing" diff --git a/src/eve_mcp/server/helpers.py b/src/eve_mcp/server/helpers.py index 8e1d1e8..018bc95 100644 --- a/src/eve_mcp/server/helpers.py +++ b/src/eve_mcp/server/helpers.py @@ -24,7 +24,9 @@ class NoTagFoundError(Exception): def extract_xml_tag(text, tag): """Extract xml tag from text.""" - if (p2 := text.find(f"")) <= (p1 := text.find(f"<{tag}>")): + p1 = text.find(f"<{tag}>") + p2 = text.find(f"") + if p1 < 0 or p2 <= p1: raise NoTagFoundError(f"no {tag} found in genai response") return text[p1 + len(tag) + 2 : p2] diff --git a/tests/test_server/test_helpers.py b/tests/test_server/test_helpers.py new file mode 100644 index 0000000..47cb671 --- /dev/null +++ b/tests/test_server/test_helpers.py @@ -0,0 +1,321 @@ +"""Tests for the helpers module.""" +# pylint: disable=magic-value-comparison,protected-access + +import importlib +import json +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest + +from eve_mcp.config import SERVER_MODULE as PKG + + +class TestExtractXmlTag: + """Test the extract_xml_tag function.""" + + @staticmethod + def test_extracts_content_between_tags(helpers_mod): + """Returns the substring between opening and closing tags.""" + text = "noise payload trailing" + assert helpers_mod.extract_xml_tag(text, "foo") == "payload" + + @staticmethod + def test_raises_when_tag_missing(helpers_mod): + """Raises NoTagFoundError when the tag is not in the text.""" + with pytest.raises(helpers_mod.NoTagFoundError): + helpers_mod.extract_xml_tag("nothing to see here", "foo") + + @staticmethod + def test_raises_when_only_closing_tag_present(helpers_mod): + """A stray closing tag with no opener is treated as missing.""" + with pytest.raises(helpers_mod.NoTagFoundError): + helpers_mod.extract_xml_tag("garbage ", "foo") + + +class TestExtractTag: + """Test the extract_tag (markdown fence) function.""" + + @staticmethod + def test_extracts_fenced_block(helpers_mod): + """Returns the content between matching markdown fences.""" + text = 'before ```json {"x": 1} ``` after' + assert helpers_mod.extract_tag(text, "json").strip() == '{"x": 1}' + + @staticmethod + def test_extracts_multiline_fenced_block(helpers_mod): + """DOTALL flag means the inner block can span newlines.""" + text = 'pre\n```json\n{\n "x": 1\n}\n```\npost' + out = helpers_mod.extract_tag(text, "json") + assert '"x": 1' in out + + @staticmethod + def test_raises_when_no_fence(helpers_mod): + """Raises NoTagFoundError when the fence is missing.""" + with pytest.raises(helpers_mod.NoTagFoundError): + helpers_mod.extract_tag("plain text", "json") + + +class TestGetEveClient: + """Test the get_eve_client lazy singleton.""" + + @pytest.mark.asyncio + @staticmethod + async def test_raises_when_credentials_missing(helpers_mod, monkeypatch): + """Raises RuntimeError when EVE_EMAIL or EVE_PASSWORD is unset.""" + monkeypatch.delenv("EVE_EMAIL", raising=False) + monkeypatch.delenv("EVE_PASSWORD", raising=False) + with pytest.raises(RuntimeError, match="EVE_EMAIL"): + await helpers_mod.get_eve_client() + + @pytest.mark.asyncio + @staticmethod + async def test_constructs_and_logs_in( + helpers_mod, mock_env + ): # pylint: disable=unused-argument + """Builds an EVEClient, opens HTTP transport, and logs in.""" + fake_client = MagicMock() + fake_client.is_authenticated = MagicMock(return_value=True) + fake_client._ensure_http_client = AsyncMock() + fake_client.login = AsyncMock() + with patch( + f"{PKG}.helpers.EVEClient", return_value=fake_client + ) as ctor: + result = await helpers_mod.get_eve_client() + ctor.assert_called_once_with(base_url="https://eve.example.com") + fake_client._ensure_http_client.assert_awaited_once() + fake_client.login.assert_awaited_once_with( + "test@example.com", "secret" + ) + assert result is fake_client + + @pytest.mark.asyncio + @staticmethod + async def test_returns_cached_authenticated_client( + helpers_mod, mock_env + ): # pylint: disable=unused-argument + """A second call returns the cached singleton without re-login.""" + fake_client = MagicMock() + fake_client.is_authenticated = MagicMock(return_value=True) + fake_client._ensure_http_client = AsyncMock() + fake_client.login = AsyncMock() + with patch( + f"{PKG}.helpers.EVEClient", return_value=fake_client + ) as ctor: + await helpers_mod.get_eve_client() + await helpers_mod.get_eve_client() + ctor.assert_called_once() + fake_client.login.assert_awaited_once() + + +class TestEnsureLoggedIn: # pylint: disable=too-few-public-methods + """Test the ensure_logged_in wrapper.""" + + @pytest.mark.asyncio + @staticmethod + async def test_calls_ensure_authenticated( + helpers_mod, mock_env + ): # pylint: disable=unused-argument + """Returns the cached client and refreshes its auth token.""" + fake_client = MagicMock() + fake_client.is_authenticated = MagicMock(return_value=True) + fake_client._ensure_http_client = AsyncMock() + fake_client.login = AsyncMock() + fake_client.auth = MagicMock() + fake_client.auth.ensure_authenticated = AsyncMock() + with patch(f"{PKG}.helpers.EVEClient", return_value=fake_client): + result = await helpers_mod.ensure_logged_in() + fake_client.auth.ensure_authenticated.assert_awaited_once() + assert result is fake_client + + +def _make_authed_client(stream_events, *, delete_raises=False): + """Build a fake EVEClient with a stubbed stream + post + delete.""" + client = MagicMock() + client.is_authenticated = MagicMock(return_value=True) + client._ensure_http_client = AsyncMock() + client.login = AsyncMock() + client.auth = MagicMock() + client.auth.ensure_authenticated = AsyncMock() + client.post = AsyncMock(return_value={"id": "conv-123"}) + if delete_raises: + client.delete = AsyncMock(side_effect=RuntimeError("nope")) + else: + client.delete = AsyncMock(return_value=None) + + async def fake_stream(*_args, **_kwargs): + for event in stream_events: + yield event + + client.stream = fake_stream + return client + + +class TestQueryEve: + """Test the _query_eve helper.""" + + @pytest.mark.asyncio + @staticmethod + async def test_assembles_tokens_and_sources( + helpers_mod, mock_env + ): # pylint: disable=unused-argument + """Streams TOKEN/SOURCE/FINAL events into the result dict.""" + events = [ + {"type": "token", "content": "Hello "}, + {"type": "token", "content": "world"}, + {"type": "source", "data": {"id": "s1"}}, + {"type": "final"}, + ] + client = _make_authed_client(events) + with patch(f"{PKG}.helpers.EVEClient", return_value=client): + raw = await helpers_mod._query_eve( # pylint: disable=protected-access + "Q?", collections="c1,c2", k=2 + ) + result = json.loads(raw) + assert result["answer"] == "Hello world" + assert result["sources"] == [{"id": "s1"}] + assert result["conversation_id"] == "conv-123" + assert "error" not in result + client.post.assert_awaited_once() + client.delete.assert_awaited_once_with("/conversations/conv-123") + + @pytest.mark.asyncio + @staticmethod + async def test_final_event_content_used_when_no_tokens( + helpers_mod, mock_env + ): # pylint: disable=unused-argument + """If only a FINAL event with content arrives, it is the answer.""" + events = [ + {"type": "final", "content": "complete answer"}, + ] + client = _make_authed_client(events) + with patch(f"{PKG}.helpers.EVEClient", return_value=client): + raw = await helpers_mod._query_eve( # pylint: disable=protected-access + "Q?" + ) + assert json.loads(raw)["answer"] == "complete answer" + + @pytest.mark.asyncio + @staticmethod + async def test_error_event_populates_error_field( + helpers_mod, mock_env + ): # pylint: disable=unused-argument + """An ERROR event short-circuits the stream and is reported.""" + events = [ + {"type": "token", "content": "partial"}, + {"type": "error", "message": "boom"}, + {"type": "token", "content": " ignored"}, + ] + client = _make_authed_client(events) + with patch(f"{PKG}.helpers.EVEClient", return_value=client): + raw = await helpers_mod._query_eve( # pylint: disable=protected-access + "Q?" + ) + result = json.loads(raw) + assert result["error"] == "boom" + assert result["answer"] == "partial" + + @pytest.mark.asyncio + @staticmethod + async def test_delete_failure_is_swallowed( + helpers_mod, mock_env, caplog + ): # pylint: disable=unused-argument + """A failure deleting the conversation is logged, not raised.""" + events = [{"type": "final", "content": "ok"}] + client = _make_authed_client(events, delete_raises=True) + with patch(f"{PKG}.helpers.EVEClient", return_value=client): + raw = await helpers_mod._query_eve( # pylint: disable=protected-access + "Q?" + ) + assert json.loads(raw)["answer"] == "ok" + assert any( + "Failed to delete conversation" in rec.message + for rec in caplog.records + ) + + +class TestExtractFactualityIssues: # pylint: disable=too-few-public-methods + """Test the _extract_factuality_issues helper.""" + + @pytest.mark.asyncio + @staticmethod + async def test_extracts_json_block_from_answer(helpers_mod): + """Returns a payload with the parsed issues string under 'issues'.""" + inner_json = '[{"title": "x"}]' + answer = f"intro ```json {inner_json} ``` outro" + query_payload = json.dumps( + {"answer": answer, "sources": [], "conversation_id": "c"} + ) + with patch( + f"{PKG}.helpers._query_eve", + new=AsyncMock(return_value=query_payload), + ): + raw = await helpers_mod._extract_factuality_issues( # pylint: disable=protected-access + "Q?", "code" + ) + result = json.loads(raw) + assert inner_json in result["issues"] + assert result["conversation_id"] == "c" + + +class TestAssessFactualityIssue: + """Test the _assess_factuality_issue helper.""" + + @pytest.mark.asyncio + @staticmethod + async def test_strips_recommendations_block_from_answer( + helpers_mod, + ): + """Removes from answer; surfaces it + separately.""" + recs = '{"r1": {"explanation": "e", "code_snippet": "x = 1"}}' + answer = ( + "Markdown body. " + f"{recs}" + " trailer" + ) + query_payload = json.dumps( + {"answer": answer, "sources": [], "conversation_id": "c"} + ) + with patch( + f"{PKG}.helpers._query_eve", + new=AsyncMock(return_value=query_payload), + ): + raw = await helpers_mod._assess_factuality_issue( # pylint: disable=protected-access + "Q?", "code", "t", "d", "f", "eq" + ) + result = json.loads(raw) + assert "" in result["answer"] + # The recs payload itself is removed from the answer body. + assert recs not in result["answer"] + assert result["code_recommendations"] == recs + + @pytest.mark.asyncio + @staticmethod + async def test_unwraps_inner_json_fence_in_recommendations( + helpers_mod, + ): + """If wraps a ```json fence, it is unwrapped.""" + inner = '{"r1": {"explanation": "e", "code_snippet": "x = 1"}}' + recs_with_fence = f"```json {inner} ```" + answer = ( + "Body. " + f"{recs_with_fence}" + ) + query_payload = json.dumps( + {"answer": answer, "sources": [], "conversation_id": "c"} + ) + with patch( + f"{PKG}.helpers._query_eve", + new=AsyncMock(return_value=query_payload), + ): + raw = await helpers_mod._assess_factuality_issue( # pylint: disable=protected-access + "Q?", "code", "t", "d", "f", "eq" + ) + result = json.loads(raw) + assert inner in result["code_recommendations"] + + +def test_helpers_module_importable(): + """Sanity: the helpers module imports cleanly under PKG.""" + helpers = importlib.import_module(f"{PKG}.helpers") + assert hasattr(helpers, "_query_eve")