Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 61 additions & 0 deletions tests/test_normalize_project.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
"""Tests for project name normalization in MCP server."""

import os

from brainlayer.mcp import normalize_project_name


class TestNormalizeProjectName:
"""Test Claude Code path encoding β†’ clean project name."""

def test_none_returns_none(self):
assert normalize_project_name(None) is None

def test_empty_returns_none(self):
assert normalize_project_name("") is None
assert normalize_project_name(" ") is None
assert normalize_project_name("-") is None

def test_simple_name_passthrough(self):
"""Already-clean names pass through unchanged."""
assert normalize_project_name("golems") == "golems"

def test_claude_code_encoded_path(self):
"""Standard Claude Code path encoding decodes correctly."""
# This test requires the actual filesystem to have ~/Gits/golems
result = normalize_project_name("-Users-etanheyman-Gits-golems")
assert result == "golems"
Comment on lines +26 to +30
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟑 Minor

Misleading comment β€” this test passes on every machine regardless of filesystem.

The comment "This test requires the actual filesystem to have ~/Gits/golems" is incorrect. For the input "-Users-etanheyman-Gits-golems", the implementation tries os.path.isdir("/Users/etanheyman/Gits/golems") and falls back to remaining[0] β€” which is "golems" β€” when the directory is absent. The assertion result == "golems" therefore always passes, not because the directory exists, but due to the fallback. The comment should be corrected to avoid misleading future maintainers.

πŸ› οΈ Proposed fix
     def test_claude_code_encoded_path(self):
         """Standard Claude Code path encoding decodes correctly."""
-        # This test requires the actual filesystem to have ~/Gits/golems
+        # Falls back to the first remaining segment ("golems") when directory isn't found.
         result = normalize_project_name("-Users-etanheyman-Gits-golems")
         assert result == "golems"
πŸ€– Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/test_normalize_project.py` around lines 26 - 30, The test comment is
misleading: update the docstring in test_claude_code_encoded_path to state that
normalize_project_name("-Users-etanheyman-Gits-golems") will return "golems"
even if the filesystem path /Users/etanheyman/Gits/golems does not exist because
normalize_project_name checks os.path.isdir(...) and falls back to the last
segment (remaining[0]); change the comment to reflect this behavior (mention
normalize_project_name and the fallback) instead of claiming the test requires
the actual filesystem to have ~/Gits/golems.


def test_desktop_gits_path(self):
"""Old Desktop/Gits paths decode correctly."""
# Old path β€” directory may not exist, but first segment is returned
result = normalize_project_name("-Users-etanheyman-Desktop-Gits-golems")
# Either finds the dir or falls back to first segment
assert result is not None
Comment on lines +32 to +37
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟑 Minor

Weak assertion in test_desktop_gits_path β€” result is not None doesn't verify decoding.

For "-Users-etanheyman-Desktop-Gits-golems" the fallback in normalize_project_name always returns "golems" (same analysis as test_claude_code_encoded_path), so the assertion can and should be strengthened:

πŸ› οΈ Proposed fix
-        # Either finds the dir or falls back to first segment
-        assert result is not None
+        assert result == "golems"  # Fallback always returns first remaining segment
πŸ“ Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def test_desktop_gits_path(self):
"""Old Desktop/Gits paths decode correctly."""
# Old path β€” directory may not exist, but first segment is returned
result = normalize_project_name("-Users-etanheyman-Desktop-Gits-golems")
# Either finds the dir or falls back to first segment
assert result is not None
def test_desktop_gits_path(self):
"""Old Desktop/Gits paths decode correctly."""
# Old path β€” directory may not exist, but first segment is returned
result = normalize_project_name("-Users-etanheyman-Desktop-Gits-golems")
assert result == "golems" # Fallback always returns first remaining segment
πŸ€– Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/test_normalize_project.py` around lines 32 - 37, The
test.test_desktop_gits_path currently asserts only that
normalize_project_name("-Users-etanheyman-Desktop-Gits-golems") is not None;
strengthen it to verify the actual decoded/fallback value returned by
normalize_project_name is "golems" (same expectation as
test_claude_code_encoded_path). Update the assertion in test_desktop_gits_path
to compare the result to the string "golems" so the test verifies correct
decoding/fallback behavior of normalize_project_name.


def test_compound_name_with_dashes(self):
"""Compound project names with dashes resolve via filesystem check."""
# Only works if the directory actually exists
home = os.path.expanduser("~")
gits_dir = os.path.join(home, "Gits")
if os.path.isdir(gits_dir):
for entry in os.listdir(gits_dir):
if "-" in entry and os.path.isdir(os.path.join(gits_dir, entry)):
# Test that this compound name resolves correctly
encoded = f"-Users-{os.path.basename(home)}-Gits-{entry.replace('-', '-')}"
# The encoding is just dashes, same as the name
# Just verify it doesn't crash
result = normalize_project_name(encoded)
assert result is not None
break
Comment on lines +39 to +53
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟑 Minor

test_compound_name_with_dashes has a no-op string operation and is vacuously true in CI.

Two issues:

  1. No-op: entry.replace('-', '-') on line 48 replaces '-' with '-' and does nothing. This is almost certainly a typo (perhaps intended as entry.replace('/', '-') to simulate a Claude Code path encoding).

  2. Vacuous pass: The entire test body is wrapped in if os.path.isdir(gits_dir): (line 44). On any machine where ~/Gits doesn't exist (all CI runners), the test passes trivially without exercising any logic. Even when ~/Gits exists, the inner loop exits on break at the first matching entry with only assert result is not None β€” not verifying the decoded value.

Use pytest.skip to make the environment dependency explicit, and tighten the assertion:

πŸ› οΈ Proposed fix
     def test_compound_name_with_dashes(self):
         """Compound project names with dashes resolve via filesystem check."""
         home = os.path.expanduser("~")
         gits_dir = os.path.join(home, "Gits")
         if not os.path.isdir(gits_dir):
+            pytest.skip("~/Gits not present on this machine")
         for entry in os.listdir(gits_dir):
             if "-" in entry and os.path.isdir(os.path.join(gits_dir, entry)):
-                encoded = f"-Users-{os.path.basename(home)}-Gits-{entry.replace('-', '-')}"
+                # Claude Code encodes path separators as dashes; the name itself uses dashes
+                encoded = f"-Users-{os.path.basename(home)}-Gits-{entry}"
                 result = normalize_project_name(encoded)
-                assert result is not None
+                assert result == entry, f"Expected {entry!r}, got {result!r}"
                 break
+        else:
+            pytest.skip("No compound-name directories found in ~/Gits")
πŸ€– Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/test_normalize_project.py` around lines 39 - 53, The test
test_compound_name_with_dashes has a no-op entry.replace('-', '-') and silently
skips when ~/Gits is missing; change it to explicitly skip with pytest.skip if
gits_dir doesn't exist, and fix the encode step to transform a name into the
expected encoded form (e.g., replace path separators with '-' as intended)
before calling normalize_project_name; finally tighten the assertion to check
that normalize_project_name(encoded) returns the expected decoded project path
or name (reference normalize_project_name and test_compound_name_with_dashes to
locate the change).


def test_worktree_suffix_stripped(self):
"""Worktree suffixes are stripped."""
assert normalize_project_name("golems-nightshift-1770775282043") == "golems"
assert normalize_project_name("golems-haiku-1770775282043") == "golems"
assert normalize_project_name("golems-worktree-1770775282043") == "golems"

def test_no_gits_segment_returns_none(self):
"""Paths without 'Gits' segment return None."""
result = normalize_project_name("-Users-etanheyman-Documents-stuff")
Comment on lines +11 to +63
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

πŸ› οΈ Refactor suggestion | 🟠 Major

TestNormalizeProjectName substantially duplicates TestProjectNameNormalization in tests/test_phase3_qa.py.

The following test cases are either identical or near-identical across the two files:

test_normalize_project.py test_phase3_qa.py
test_none_returns_none test_none_input
test_empty_returns_none test_empty_string, test_dash_only, test_whitespace
test_simple_name_passthrough test_clean_name_passthrough
test_worktree_suffix_stripped test_worktree_suffix_stripped
test_no_gits_segment_returns_none test_no_gits_segment

Consider consolidating all normalization tests into a single file to reduce maintenance overhead.

πŸ€– Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/test_normalize_project.py` around lines 11 - 63, The
TestNormalizeProjectName test class duplicates many cases already covered by
TestProjectNameNormalization; consolidate by removing the redundant tests (or
the entire TestNormalizeProjectName class) and keep a single canonical test
suite (e.g., TestProjectNameNormalization) that includes the unique cases;
locate the duplicated test methods by name (test_none_returns_none /
test_none_input, test_empty_returns_none /
test_empty_string|test_dash_only|test_whitespace, test_simple_name_passthrough /
test_clean_name_passthrough, test_worktree_suffix_stripped,
test_no_gits_segment_returns_none / test_no_gits_segment) and either move any
unique assertions into the canonical class or delete them, then run the test
suite to ensure no missing imports or references to normalize_project_name
remain.

assert result is None
50 changes: 50 additions & 0 deletions tests/test_paths.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
"""Tests for brainlayer.paths β€” DB path resolution."""

import os
from unittest.mock import patch

from brainlayer.paths import get_db_path


class TestGetDbPath:
"""Test DB path resolution order."""

def test_env_var_override(self, tmp_path):
"""BRAINLAYER_DB env var takes highest priority."""
db_path = tmp_path / "custom.db"
with patch.dict(os.environ, {"BRAINLAYER_DB": str(db_path)}):
assert get_db_path() == db_path

def test_legacy_path_if_exists(self, tmp_path):
"""Legacy zikaron path used when it exists."""
legacy = tmp_path / "zikaron.db"
legacy.touch()
with (
patch.dict(os.environ, {}, clear=True),
patch("brainlayer.paths._LEGACY_DB_PATH", legacy),
patch("brainlayer.paths._CANONICAL_DB_PATH", tmp_path / "brainlayer.db"),
):
# Remove env var if set
os.environ.pop("BRAINLAYER_DB", None)
assert get_db_path() == legacy

def test_canonical_path_fresh_install(self, tmp_path):
"""Canonical path used when no legacy DB exists."""
canonical = tmp_path / "brainlayer" / "brainlayer.db"
legacy = tmp_path / "nonexistent" / "zikaron.db"
with (
patch.dict(os.environ, {}, clear=True),
patch("brainlayer.paths._LEGACY_DB_PATH", legacy),
patch("brainlayer.paths._CANONICAL_DB_PATH", canonical),
):
os.environ.pop("BRAINLAYER_DB", None)
result = get_db_path()
assert result == canonical
assert canonical.parent.exists() # Parent dir created
Comment on lines +15 to +46
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

πŸ› οΈ Refactor suggestion | 🟠 Major

TestGetDbPath largely duplicates TestDBPathResolution in test_phase3_qa.py.

test_env_var_override, test_legacy_path_if_exists, and test_canonical_path_fresh_install are functionally equivalent to TestDBPathResolution.test_env_var_override, test_legacy_path_preferred_over_canonical, and test_canonical_path_when_legacy_missing respectively. Consider consolidating into a single location to reduce maintenance overhead.

πŸ€– Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/test_paths.py` around lines 15 - 46, The three tests in
tests/test_paths.py (test_env_var_override, test_legacy_path_if_exists,
test_canonical_path_fresh_install) duplicate behavior already covered by
TestDBPathResolution tests in test_phase3_qa.py; remove these redundant test
functions from tests/test_paths.py or consolidate them into the existing
TestDBPathResolution class to avoid duplication, keeping a single authoritative
implementation for get_db_path() behavior (env var override, legacy path
precedence, and canonical path creation) and ensuring any fixtures/patches
(patch.dict, patch("brainlayer.paths._LEGACY_DB_PATH"),
patch("brainlayer.paths._CANONICAL_DB_PATH")) remain in the one retained test
location so functionality and assertions (including canonical.parent.exists())
are preserved.


def test_real_db_exists(self):
"""The real production DB exists at the resolved path."""
from brainlayer.paths import DEFAULT_DB_PATH

assert DEFAULT_DB_PATH.exists(), f"DB not found at {DEFAULT_DB_PATH}"
assert DEFAULT_DB_PATH.stat().st_size > 1_000_000, "DB too small β€” might be empty"
Comment on lines +48 to +52
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, find brainlayer/paths.py and check for DEFAULT_DB_PATH
find . -name "paths.py" -type f | grep -E "brainlayer|src"

Repository: EtanHey/brainlayer

Length of output: 87


🏁 Script executed:

# Search for DEFAULT_DB_PATH in the codebase
rg "DEFAULT_DB_PATH" -n --type py

Repository: EtanHey/brainlayer

Length of output: 3522


🏁 Script executed:

# Get the full content of brainlayer/paths.py to verify what's exported
fd -type f -name "paths.py" -exec grep -n "DEFAULT_DB_PATH\|_LEGACY_DB_PATH\|_CANONICAL_DB_PATH\|^[A-Z_]*.*=" {} +

Repository: EtanHey/brainlayer

Length of output: 232


🏁 Script executed:

# Look at the full test_paths.py file to see all tests and context
cat -n tests/test_paths.py

Repository: EtanHey/brainlayer

Length of output: 2371


🏁 Script executed:

# Read the full brainlayer/paths.py file
cat -n src/brainlayer/paths.py

Repository: EtanHey/brainlayer

Length of output: 1711


🏁 Script executed:

# Check for pytest configuration or conftest that might skip tests
find . -name "pytest.ini" -o -name "conftest.py" -o -name "pyproject.toml" | head -10

Repository: EtanHey/brainlayer

Length of output: 78


🏁 Script executed:

# Check CI configuration
find . -name ".github" -type d -o -name ".gitlab-ci.yml" -o -name "*.yml" -path "*/.github/*" | head -20

Repository: EtanHey/brainlayer

Length of output: 130


🏁 Script executed:

# Check the CI workflow configuration
cat -n .github/workflows/ci.yml

Repository: EtanHey/brainlayer

Length of output: 983


🏁 Script executed:

# Check pyproject.toml for pytest configuration
cat -n pyproject.toml | head -100

Repository: EtanHey/brainlayer

Length of output: 3373


test_real_db_exists unconditionally requires a production DB β€” will fail in CI.

This test asserts that DEFAULT_DB_PATH exists and is at least 1 MB in size, with no guards or skip markers. In the CI environment (ubuntu-latest, fresh runner), no legacy database exists and DEFAULT_DB_PATH resolves to ~/.local/share/brainlayer/brainlayer.db, which is created as an empty directory path by get_db_path() but never materialized as an actual database file. The test will fail immediately.

Gate it with @pytest.mark.skipif:

πŸ› οΈ Proposed fix
+    `@pytest.mark.skipif`(
+        not __import__("brainlayer.paths", fromlist=["DEFAULT_DB_PATH"]).DEFAULT_DB_PATH.exists(),
+        reason="Production DB not present on this machine",
+    )
     def test_real_db_exists(self):
         """The real production DB exists at the resolved path."""
         from brainlayer.paths import DEFAULT_DB_PATH
         assert DEFAULT_DB_PATH.exists(), f"DB not found at {DEFAULT_DB_PATH}"
         assert DEFAULT_DB_PATH.stat().st_size > 1_000_000, "DB too small β€” might be empty"

Alternatively, move this to a separate tests/integration/ directory excluded from the default CI pytest run.

πŸ“ Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def test_real_db_exists(self):
"""The real production DB exists at the resolved path."""
from brainlayer.paths import DEFAULT_DB_PATH
assert DEFAULT_DB_PATH.exists(), f"DB not found at {DEFAULT_DB_PATH}"
assert DEFAULT_DB_PATH.stat().st_size > 1_000_000, "DB too small β€” might be empty"
`@pytest.mark.skipif`(
not __import__("brainlayer.paths", fromlist=["DEFAULT_DB_PATH"]).DEFAULT_DB_PATH.exists(),
reason="Production DB not present on this machine",
)
def test_real_db_exists(self):
"""The real production DB exists at the resolved path."""
from brainlayer.paths import DEFAULT_DB_PATH
assert DEFAULT_DB_PATH.exists(), f"DB not found at {DEFAULT_DB_PATH}"
assert DEFAULT_DB_PATH.stat().st_size > 1_000_000, "DB too small β€” might be empty"
πŸ€– Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/test_paths.py` around lines 48 - 52, The test_real_db_exists currently
unconditionally requires a production DB file at DEFAULT_DB_PATH and will fail
in CI; update this test to skip when the DB file is not present or not large
enough by adding a pytest.mark.skipif guard that checks
DEFAULT_DB_PATH.is_file() and DEFAULT_DB_PATH.stat().st_size > 1_000_000 (or use
get_db_path() logic) so the assertion only runs in environments with a real DB,
or move the test into an integration-only suite (e.g., tests/integration/)
excluded from default CI runs.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test asserts production DB exists, fails CI

Medium Severity

test_real_db_exists asserts DEFAULT_DB_PATH.exists() and st_size > 1_000_000, which only passes on the developer's machine where the production database is present. This test will fail in CI or any fresh environment, since no production database exists there.

Fix in CursorΒ Fix in Web

Loading
Loading