-
Notifications
You must be signed in to change notification settings - Fork 54
test(mcp): sample-project fixture + assertion contract (T3 #650) #677
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
a598a74
b7cbbb0
286059b
42b794b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -49,4 +49,5 @@ where = ["."] | |
| dev = [ | ||
| "pytest>=9.0.2", | ||
| "anyio>=4.0,<5.0", | ||
| "pyyaml>=6.0.3", | ||
| ] | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,145 @@ | ||
| """Shared fixtures for the MCP test suite. | ||
|
|
||
| Every per-tool integration test from T4 onward reuses the | ||
| ``indexed_fixture`` fixture below: it indexes ``fixtures/sample_project`` | ||
| into a uniquely named FalkorDB graph once per test session and yields a | ||
| descriptor (project name, branch, graph name) the test can pass straight | ||
| to the MCP tool under test. | ||
|
|
||
| The integration fixture is opt-in — it requires a reachable FalkorDB | ||
| (see ``api/graph.py``) and the optional language analyzers. Tests that | ||
| only need the static contract (counts, named callers / callees / paths) | ||
| can depend on ``expected_contract`` alone, which is pure-Python and | ||
| always available. | ||
| """ | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| import os | ||
| import uuid | ||
| from dataclasses import dataclass | ||
| from pathlib import Path | ||
| from typing import Any | ||
|
|
||
| import pytest | ||
| import yaml | ||
|
|
||
|
|
||
| FIXTURE_DIR = Path(__file__).parent / "fixtures" | ||
| SAMPLE_PROJECT = FIXTURE_DIR / "sample_project" | ||
| EXPECTED_PATH = FIXTURE_DIR / "expected.yaml" | ||
|
|
||
|
|
||
| # --------------------------------------------------------------------------- | ||
| # Pure-Python contract (no FalkorDB required) | ||
| # --------------------------------------------------------------------------- | ||
|
|
||
|
|
||
| @dataclass(frozen=True) | ||
| class IndexedFixture: | ||
| """Descriptor for an indexed fixture graph.""" | ||
|
|
||
| project: str | ||
| branch: str | ||
| graph_name: str | ||
| path: Path | ||
|
|
||
|
|
||
| @pytest.fixture(scope="session") | ||
| def expected_contract() -> dict[str, Any]: | ||
| """Load ``fixtures/expected.yaml`` once per session.""" | ||
| with EXPECTED_PATH.open() as fh: | ||
| return yaml.safe_load(fh) | ||
|
|
||
|
|
||
| @pytest.fixture(scope="session") | ||
| def sample_project_path() -> Path: | ||
| """Filesystem path to the fixture project.""" | ||
| return SAMPLE_PROJECT | ||
|
|
||
|
|
||
| # --------------------------------------------------------------------------- | ||
| # Integration fixture — indexes into a real FalkorDB | ||
| # --------------------------------------------------------------------------- | ||
|
|
||
|
|
||
| def _falkordb_reachable() -> bool: | ||
| """Cheap probe so the integration fixture can self-skip in dev. | ||
|
|
||
| Uses the falkordb-py client (rather than a raw TCP socket) so the | ||
| check exercises the same connection settings the rest of the test | ||
| suite uses, and surfaces auth / protocol failures — not just | ||
| "something is listening on that port". | ||
| """ | ||
| try: | ||
| from falkordb import FalkorDB | ||
|
|
||
| host = os.getenv("FALKORDB_HOST", "localhost") | ||
| port = int(os.getenv("FALKORDB_PORT", 6379)) | ||
| db = FalkorDB(host=host, port=port, socket_timeout=1) | ||
| return bool(db.connection.ping()) | ||
| except Exception: | ||
| return False | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
|
|
||
| @pytest.fixture(scope="session") | ||
| def indexed_fixture(sample_project_path: Path): | ||
| """Index the sample project into a unique per-session graph. | ||
|
|
||
| Each test session creates a new graph named | ||
| ``code:sample_project:test-<uuid>`` so parallel CI shards never | ||
| contend on the same graph. The graph is deleted on teardown so | ||
| long-lived FalkorDB deployments (developer machines, shared CI) | ||
| don't accumulate orphan ``test-*`` graphs across runs. | ||
|
|
||
| Set ``MCP_KEEP_TEST_GRAPHS=1`` to skip teardown — useful for | ||
| post-mortem debugging when a test fails and you want to poke at | ||
| the graph by hand. | ||
|
|
||
| Uses :class:`api.analyzers.SourceAnalyzer` directly (instead of | ||
| ``Project.from_local_repository``) so the fixture doesn't need to | ||
| be a git repository — analyzing a plain directory is exactly the | ||
| code path the ``index_repo`` MCP tool exercises for non-git | ||
| folders. | ||
| """ | ||
|
|
||
| if not _falkordb_reachable(): | ||
| pytest.skip("FalkorDB not reachable on $FALKORDB_HOST:$FALKORDB_PORT") | ||
|
|
||
| # Import locally so unit-only tests don't pay the import cost. | ||
| from api.analyzers.source_analyzer import SourceAnalyzer | ||
| from api.graph import Graph | ||
|
|
||
| project_name = sample_project_path.name # "sample_project" | ||
| branch = f"test-{uuid.uuid4().hex[:8]}" | ||
| graph = Graph(project_name, branch=branch) | ||
|
|
||
| analyzer = SourceAnalyzer() | ||
| # Belt-and-suspenders: jedi/multilspy used to create venv/ inside the | ||
| # fixture during resolution, which polluted the tree with hundreds of | ||
| # site-packages files. Tree-sitter doesn't do this, but we still pass | ||
| # an explicit ignore list so a stray venv on a contributor's machine | ||
| # can never break the exact-count contract. | ||
| analyzer.analyze_local_folder( | ||
| str(sample_project_path), | ||
| graph, | ||
| ignore=["venv", "__pycache__", ".venv"], | ||
| ) | ||
|
|
||
| yield IndexedFixture( | ||
| project=project_name, | ||
| branch=branch, | ||
| graph_name=graph.name, | ||
| path=sample_project_path, | ||
| ) | ||
|
Comment on lines
+85
to
+134
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done in 42b794b — Preserved the post-mortem-debugging path via an opt-in env var: set |
||
|
|
||
| # Teardown — drop the graph so we don't leak ``test-<uuid>`` graphs | ||
| # across runs. Opt out with MCP_KEEP_TEST_GRAPHS=1 when debugging. | ||
| if os.getenv("MCP_KEEP_TEST_GRAPHS") == "1": | ||
| return | ||
| try: | ||
| graph.delete() | ||
| except Exception: | ||
| # Best-effort cleanup: never fail a test session because | ||
| # teardown couldn't reach FalkorDB. | ||
| pass | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,56 @@ | ||
| # MCP fixture assertion contract. | ||
| # | ||
| # Anything declared here is treated as load-bearing by tests/mcp/*. | ||
| # Counts are EXACT (not minimums): the fixture is pinned to a Python-only | ||
| # tree the deterministic tree-sitter analyzer fully covers, so there is | ||
| # no analyzer-availability skew to hedge against. | ||
| # Named-symbol assertions are also exact. | ||
|
|
||
| # The graph is named after the fixture directory (per Project.from_local_repository). | ||
| project_name: sample_project | ||
|
|
||
| # Exact counts produced by the Python tree-sitter analyzer over the | ||
| # fixture as committed. If you add/remove a fixture source file, update | ||
| # these numbers to match. | ||
| counts: | ||
| File: 5 # entrypoint.py, service.py, repo.py, db.py, __init__.py | ||
| Class: 3 # BaseRepo, UserRepo, OrderRepo | ||
| Function: 6 # entrypoint, service, db, BaseRepo.repo, UserRepo.repo, OrderRepo.repo | ||
|
|
||
| # Named callers / callees (used by T5 — get_callers / get_callees). | ||
| # Verified against the live graph: assertions are exact, not "at least". | ||
| calls: | ||
| service: | ||
| callers: ["entrypoint"] | ||
| # service() instantiates UserRepo + OrderRepo and calls .repo() on each; | ||
| # the analyzer emits CALLS edges to both the class constructors and | ||
| # the repo method (one per subclass, so `repo` appears twice). | ||
| callees: ["OrderRepo", "UserRepo", "repo", "repo"] | ||
| entrypoint: | ||
| callers: [] | ||
| callees: ["service"] | ||
| db: | ||
| # db() is called from BaseRepo.repo (the subclasses delegate via super()). | ||
| callers: ["repo"] | ||
| callees: [] | ||
|
|
||
| # Known paths between two named symbols (used by T7 — find_path). | ||
| # Exact path count per (source, dest) pair. | ||
| # | ||
| # entrypoint -> service -> { UserRepo.repo, OrderRepo.repo } -> db | ||
| # yields two distinct paths through the class hierarchy. | ||
| paths: | ||
| - source: entrypoint | ||
| dest: db | ||
| paths_count: 2 | ||
|
|
||
| # Prefix-search hits (used by T8 — search_code). | ||
| # Verified against api.auto_complete.prefix_search. | ||
| search_prefixes: | ||
| ent: | ||
| must_include: ["entrypoint"] | ||
| serv: | ||
| must_include: ["service"] | ||
| Base: | ||
| # Case-insensitive prefix match against Searchable.name. | ||
| must_include: ["BaseRepo"] |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| # Keep the fixture deterministic: jedi/multilspy historically auto-created | ||
| # a venv/ here during indexing, which polluted the tree with hundreds of | ||
| # stdlib + site-packages files and broke the exact-count contract. | ||
| venv/ | ||
| __pycache__/ | ||
| *.pyc |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| # Sample fixture project for the MCP test suite | ||
|
|
||
| This directory is consumed by `tests/mcp/conftest.py::indexed_fixture`. Every | ||
| MCP tool ticket from T4 onward asserts against the assertions declared in | ||
| `expected.yaml`. | ||
|
|
||
| ## Canonical Python call graph | ||
|
|
||
| ``` | ||
| entrypoint() -> service() -> {UserRepo,OrderRepo}.repo() -> db() | ||
| ``` | ||
|
Comment on lines
+9
to
+11
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add fence languages to satisfy markdownlint. The two fenced blocks should declare a language (for these, Suggested patch-```
+```text
entrypoint() -> service() -> {UserRepo,OrderRepo}.repo() -> db()@@ Also applies to: 15-19 🧰 Tools🪛 markdownlint-cli2 (0.22.1)[warning] 9-9: Fenced code blocks should have a language specified (MD040, fenced-code-language) 🤖 Prompt for AI Agents |
||
|
|
||
| Plus a small class hierarchy: | ||
|
|
||
| ``` | ||
| BaseRepo | ||
| ├── UserRepo | ||
| └── OrderRepo | ||
| ``` | ||
|
|
||
| ## Why three languages? (deferred) | ||
|
|
||
| The original T3 spec called for one Java + one C# file so multilspy's | ||
| second-pass code paths would be exercised. In practice both analyzers | ||
| demand a real Maven / .NET project layout at the **root** of the indexed | ||
| tree, which would make this fixture awkward to co-host with the Python | ||
| sample. The multilingual coverage is therefore deferred to a follow-up | ||
| ticket (likely T16, which already pulls in additional languages). | ||
|
|
||
| T4-T8 only need Python, which this fixture covers in full. | ||
|
|
||
| ## Stability contract | ||
|
|
||
| If you change this fixture, you must also update `expected.yaml`. Tests | ||
| read counts and named symbols directly from that file so the assertion | ||
| contract stays in lock-step. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| """Marks ``sample_project/python`` as a package so IMPORTS edges resolve.""" |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| """Bottom of the canonical call chain.""" | ||
|
|
||
|
|
||
| def db() -> str: | ||
| """Leaf function — entrypoint -> service -> repo -> db.""" | ||
| return "db" |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| """Entrypoint for the MCP test fixture project. | ||
|
|
||
| Call graph (must match ``expected.yaml``): | ||
|
|
||
| entrypoint() -> service() -> repo() -> db() | ||
| """ | ||
|
|
||
| from .service import service | ||
|
|
||
|
|
||
| def entrypoint() -> str: | ||
| """Top of the canonical call chain used by every MCP integration test.""" | ||
| return service() | ||
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| entrypoint() |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| """Repository layer for the MCP fixture project. | ||
|
|
||
| Exercises a small class hierarchy: ``BaseRepo`` <- ``UserRepo`` / ``OrderRepo``. | ||
| """ | ||
|
|
||
| from .db import db | ||
|
|
||
|
|
||
| class BaseRepo: | ||
| """Base class so the analyzer emits an EXTENDS edge.""" | ||
|
|
||
| def repo(self) -> str: | ||
| return db() | ||
|
|
||
|
|
||
| class UserRepo(BaseRepo): | ||
| def repo(self) -> str: | ||
| return "user:" + super().repo() | ||
|
|
||
|
|
||
| class OrderRepo(BaseRepo): | ||
| def repo(self) -> str: | ||
| return "order:" + super().repo() |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| """Service layer for the MCP fixture project.""" | ||
|
|
||
| from .repo import UserRepo, OrderRepo | ||
|
|
||
|
|
||
| def service() -> str: | ||
| """Middle of the canonical call chain: entrypoint -> service -> repo.""" | ||
| users = UserRepo() | ||
| orders = OrderRepo() | ||
| return users.repo() + ":" + orders.repo() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
Repository: FalkorDB/code-graph
Length of output: 267
🏁 Script executed:
Repository: FalkorDB/code-graph
Length of output: 1315
🏁 Script executed:
Repository: FalkorDB/code-graph
Length of output: 263
🏁 Script executed:
Repository: FalkorDB/code-graph
Length of output: 840
🏁 Script executed:
Repository: FalkorDB/code-graph
Length of output: 3109
🏁 Script executed:
Repository: FalkorDB/code-graph
Length of output: 9010
Add
pyyamlto the.[test]extra to satisfy MCP teststests/mcp/conftest.pyimportsyamlat module import time, butpyyaml>=6.0.3is only listed under[dependency-groups].dev(not under[project.optional-dependencies].test). Test installs usingpip install -e ".[test]"can fail withModuleNotFoundError: yaml. Addpyyaml>=6.0.3to[project.optional-dependencies].test(you can keep it indevtoo).Suggested patch
[project.optional-dependencies] test = [ "pytest>=9.0.2,<10.0.0", "ruff>=0.11.0,<1.0.0", "httpx>=0.28.0,<1.0.0", "anyio>=4.0,<5.0", + "pyyaml>=6.0.3", ]🤖 Prompt for AI Agents