fix: correct assert syntax in test_empty_model_suffix to prevent false positives
Why this change is needed: The test file contained assert statements using tuple syntax `assert (condition, message)`, which Python interprets as asserting a non-empty tuple (always True). This meant the tests were passing even when the actual conditions failed, creating a false sense of test coverage. Additionally, there were unused imports (pytest, patch, MagicMock) that needed cleanup. How it solves it: - Fixed assert statements on lines 61-63 and 105-109 to use correct syntax: `assert condition, message` instead of `assert (condition, message)` - Removed unused imports to satisfy linter requirements - Applied automatic formatting via ruff-format and ruff Impact: - Tests now correctly validate the empty model suffix behavior - Prevents false positive test results that could hide bugs - Passes all pre-commit hooks (F631 error resolved) Testing: - Verified with `uv run pre-commit run --all-files` - all checks pass - Assert statements now properly fail when conditions are not met
This commit is contained in:
parent
42df825d30
commit
7d0c356702
1 changed files with 31 additions and 29 deletions
|
|
@ -5,8 +5,8 @@ This test module verifies that both storage backends gracefully handle
|
||||||
the case when _generate_collection_suffix() returns an empty string.
|
the case when _generate_collection_suffix() returns an empty string.
|
||||||
"""
|
"""
|
||||||
|
|
||||||
import pytest
|
from unittest.mock import Mock
|
||||||
from unittest.mock import Mock, patch, MagicMock
|
|
||||||
from lightrag.base import BaseVectorStorage
|
from lightrag.base import BaseVectorStorage
|
||||||
from lightrag.utils import EmbeddingFunc
|
from lightrag.utils import EmbeddingFunc
|
||||||
|
|
||||||
|
|
@ -58,10 +58,9 @@ class TestEmptyModelSuffix:
|
||||||
# 2. table_name doesn't have trailing underscore
|
# 2. table_name doesn't have trailing underscore
|
||||||
# 3. table_name equals the base table name
|
# 3. table_name equals the base table name
|
||||||
assert storage.model_suffix == "", "model_suffix should be empty"
|
assert storage.model_suffix == "", "model_suffix should be empty"
|
||||||
assert (
|
assert not storage.table_name.endswith(
|
||||||
not storage.table_name.endswith("_"),
|
"_"
|
||||||
f"table_name should not have trailing underscore: {storage.table_name}",
|
), f"table_name should not have trailing underscore: {storage.table_name}"
|
||||||
)
|
|
||||||
|
|
||||||
# Expected base table name
|
# Expected base table name
|
||||||
expected_base = namespace_to_table_name("chunks")
|
expected_base = namespace_to_table_name("chunks")
|
||||||
|
|
@ -102,13 +101,12 @@ class TestEmptyModelSuffix:
|
||||||
# 1. model_suffix is empty
|
# 1. model_suffix is empty
|
||||||
# 2. final_namespace doesn't have trailing underscore
|
# 2. final_namespace doesn't have trailing underscore
|
||||||
# 3. final_namespace equals the legacy namespace
|
# 3. final_namespace equals the legacy namespace
|
||||||
assert storage._generate_collection_suffix() == "", (
|
|
||||||
"model_suffix should be empty"
|
|
||||||
)
|
|
||||||
assert (
|
assert (
|
||||||
not storage.final_namespace.endswith("_"),
|
storage._generate_collection_suffix() == ""
|
||||||
f"final_namespace should not have trailing underscore: {storage.final_namespace}",
|
), "model_suffix should be empty"
|
||||||
)
|
assert not storage.final_namespace.endswith(
|
||||||
|
"_"
|
||||||
|
), f"final_namespace should not have trailing underscore: {storage.final_namespace}"
|
||||||
assert storage.final_namespace == storage.legacy_namespace, (
|
assert storage.final_namespace == storage.legacy_namespace, (
|
||||||
f"final_namespace should fallback to legacy_namespace when model_suffix is empty. "
|
f"final_namespace should fallback to legacy_namespace when model_suffix is empty. "
|
||||||
f"Expected: {storage.legacy_namespace}, Got: {storage.final_namespace}"
|
f"Expected: {storage.legacy_namespace}, Got: {storage.final_namespace}"
|
||||||
|
|
@ -127,7 +125,9 @@ class TestEmptyModelSuffix:
|
||||||
|
|
||||||
# Create a proper embedding function with model_name
|
# Create a proper embedding function with model_name
|
||||||
embedding_func = EmbeddingFunc(
|
embedding_func = EmbeddingFunc(
|
||||||
embedding_dim=1536, func=dummy_embedding_func, model_name="text-embedding-ada-002"
|
embedding_dim=1536,
|
||||||
|
func=dummy_embedding_func,
|
||||||
|
model_name="text-embedding-ada-002",
|
||||||
)
|
)
|
||||||
|
|
||||||
# Setup global_config
|
# Setup global_config
|
||||||
|
|
@ -154,16 +154,18 @@ class TestEmptyModelSuffix:
|
||||||
# 1. model_suffix is not empty
|
# 1. model_suffix is not empty
|
||||||
# 2. table_name has correct format
|
# 2. table_name has correct format
|
||||||
assert storage.model_suffix != "", "model_suffix should not be empty"
|
assert storage.model_suffix != "", "model_suffix should not be empty"
|
||||||
assert "_" in storage.table_name, "table_name should contain underscore as separator"
|
assert (
|
||||||
|
"_" in storage.table_name
|
||||||
|
), "table_name should contain underscore as separator"
|
||||||
|
|
||||||
# Expected format: base_table_model_suffix
|
# Expected format: base_table_model_suffix
|
||||||
expected_base = namespace_to_table_name("chunks")
|
expected_base = namespace_to_table_name("chunks")
|
||||||
expected_model_id = embedding_func.get_model_identifier()
|
expected_model_id = embedding_func.get_model_identifier()
|
||||||
expected_table_name = f"{expected_base}_{expected_model_id}"
|
expected_table_name = f"{expected_base}_{expected_model_id}"
|
||||||
|
|
||||||
assert storage.table_name == expected_table_name, (
|
assert (
|
||||||
f"table_name format incorrect. Expected: {expected_table_name}, Got: {storage.table_name}"
|
storage.table_name == expected_table_name
|
||||||
)
|
), f"table_name format incorrect. Expected: {expected_table_name}, Got: {storage.table_name}"
|
||||||
|
|
||||||
def test_qdrant_collection_name_with_valid_suffix(self):
|
def test_qdrant_collection_name_with_valid_suffix(self):
|
||||||
"""
|
"""
|
||||||
|
|
@ -177,7 +179,9 @@ class TestEmptyModelSuffix:
|
||||||
|
|
||||||
# Create a proper embedding function with model_name
|
# Create a proper embedding function with model_name
|
||||||
embedding_func = EmbeddingFunc(
|
embedding_func = EmbeddingFunc(
|
||||||
embedding_dim=1536, func=dummy_embedding_func, model_name="text-embedding-ada-002"
|
embedding_dim=1536,
|
||||||
|
func=dummy_embedding_func,
|
||||||
|
model_name="text-embedding-ada-002",
|
||||||
)
|
)
|
||||||
|
|
||||||
# Setup global_config
|
# Setup global_config
|
||||||
|
|
@ -200,17 +204,17 @@ class TestEmptyModelSuffix:
|
||||||
# 2. final_namespace has correct format
|
# 2. final_namespace has correct format
|
||||||
model_suffix = storage._generate_collection_suffix()
|
model_suffix = storage._generate_collection_suffix()
|
||||||
assert model_suffix != "", "model_suffix should not be empty"
|
assert model_suffix != "", "model_suffix should not be empty"
|
||||||
assert "_" in storage.final_namespace, (
|
assert (
|
||||||
"final_namespace should contain underscore as separator"
|
"_" in storage.final_namespace
|
||||||
)
|
), "final_namespace should contain underscore as separator"
|
||||||
|
|
||||||
# Expected format: lightrag_vdb_namespace_model_suffix
|
# Expected format: lightrag_vdb_namespace_model_suffix
|
||||||
expected_model_id = embedding_func.get_model_identifier()
|
expected_model_id = embedding_func.get_model_identifier()
|
||||||
expected_collection_name = f"lightrag_vdb_chunks_{expected_model_id}"
|
expected_collection_name = f"lightrag_vdb_chunks_{expected_model_id}"
|
||||||
|
|
||||||
assert storage.final_namespace == expected_collection_name, (
|
assert (
|
||||||
f"final_namespace format incorrect. Expected: {expected_collection_name}, Got: {storage.final_namespace}"
|
storage.final_namespace == expected_collection_name
|
||||||
)
|
), f"final_namespace format incorrect. Expected: {expected_collection_name}, Got: {storage.final_namespace}"
|
||||||
|
|
||||||
def test_suffix_generation_fallback_chain(self):
|
def test_suffix_generation_fallback_chain(self):
|
||||||
"""
|
"""
|
||||||
|
|
@ -221,7 +225,6 @@ class TestEmptyModelSuffix:
|
||||||
2. Global config fallback: global_config["embedding_func"].get_model_identifier()
|
2. Global config fallback: global_config["embedding_func"].get_model_identifier()
|
||||||
3. Final fallback: return empty string
|
3. Final fallback: return empty string
|
||||||
"""
|
"""
|
||||||
from lightrag.base import BaseVectorStorage
|
|
||||||
|
|
||||||
# Create a concrete implementation for testing
|
# Create a concrete implementation for testing
|
||||||
class TestStorage(BaseVectorStorage):
|
class TestStorage(BaseVectorStorage):
|
||||||
|
|
@ -288,7 +291,6 @@ class TestEmptyModelSuffix:
|
||||||
global_config={},
|
global_config={},
|
||||||
embedding_func=mock_embedding_func,
|
embedding_func=mock_embedding_func,
|
||||||
)
|
)
|
||||||
assert storage._generate_collection_suffix() == "", (
|
assert (
|
||||||
"Should return empty string when no model_identifier available"
|
storage._generate_collection_suffix() == ""
|
||||||
)
|
), "Should return empty string when no model_identifier available"
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue