Skip to content

Add unit tests for exception handling#151

Draft
Beeram12 wants to merge 1 commit intoXortexAI:mainfrom
Beeram12:feature/unit-tests
Draft

Add unit tests for exception handling#151
Beeram12 wants to merge 1 commit intoXortexAI:mainfrom
Beeram12:feature/unit-tests

Conversation

@Beeram12
Copy link
Copy Markdown

@Beeram12 Beeram12 commented May 5, 2026

fixes #147

  1. This adds unit tests for exception.py .

@github-actions github-actions Bot added the tests label May 5, 2026
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request unignores the tests/ directory and introduces a new test suite for the project's custom exception hierarchy. The feedback focuses on improving test accuracy by fixing a typo and directly verifying attributes, adhering to PEP 8 standards for None comparisons, and ensuring complete coverage of the exception hierarchy by adding a test for VectorStoreValidationError.

Comment thread tests/test_exception.py
Comment on lines +30 to +32
def test_mssg_attribute_is_accessible(self):
err = XMemError("hello")
assert str(err) == "hello"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The test name test_mssg_attribute_is_accessible contains a typo (mssg) and the implementation redundantly checks the string representation instead of the actual message attribute. It should be updated to verify the attribute directly.

Suggested change
def test_mssg_attribute_is_accessible(self):
err = XMemError("hello")
assert str(err) == "hello"
def test_message_attribute_is_accessible(self):
err = XMemError("hello")
assert err.message == "hello"

Comment thread tests/test_exception.py

def test_operation_is_none_by_default(self):
err = XMemError("message")
assert err.operation == None
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

According to PEP 8, comparisons to singletons like None should always be done with is or is not, never the equality operators.

Suggested change
assert err.operation == None
assert err.operation is None
References
  1. Comparisons to singletons like None should always be done with 'is' or 'is not', never the equality operators. (link)

Comment thread tests/test_exception.py
Comment on lines +147 to +149
def test_validation_error_is_not_a_vector_store_error(self):
err = ValidationError("bad input")
assert not isinstance(err, VectorStoreError)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The VectorStoreValidationError class is imported but not included in the hierarchy tests. It's important to verify that it correctly inherits from VectorStoreError to ensure the exception hierarchy is functioning as expected.

Suggested change
def test_validation_error_is_not_a_vector_store_error(self):
err = ValidationError("bad input")
assert not isinstance(err, VectorStoreError)
def test_vector_store_validation_error_is_caught_as_vector_store_error(self):
with pytest.raises(VectorStoreError):
raise VectorStoreValidationError("invalid metadata")
def test_validation_error_is_not_a_vector_store_error(self):
err = ValidationError("bad input")
assert not isinstance(err, VectorStoreError)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Phase 1: Foundation and Core Unit Tests

1 participant