🔧Fix for get_multi returns boolean values for pydantic relationship fields, if a select schema is provided #199 #245
Conversation
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
✅ Fixed Pytest Warnings Renamed schema class: TestReadSchemaWithRelationship → ReadSchemaWithRelationship to avoid pytest thinking it's a test class Converted data functions to fixtures: test_data() and test_data_tier() → @pytest.fixture decorated functions Eliminated custom session management: Removed the _async_session context manager and custom models in favor of using the existing test infrastructure 🏗️ Followed Established Patterns Used existing models: Imported ModelTest and TierModel from conftest.py instead of defining new ones Proper fixture usage: Used async_session fixture and created specific fixtures for test data Consistent naming: Used descriptive, specific names for fixtures and test functions Standard imports: Followed the import patterns used in other test files 🎯 Enhanced Test Coverage More descriptive test names: Each test function clearly describes what it's testing Added SQL verification test: test_relationship_field_exclusion_prevents_cartesian_product specifically checks the generated SQL Better documentation: Each test has clear docstrings explaining the purpose Focused scope: Each test focuses on a specific aspect of the fix 📋 Key Improvements Before After Custom models and session management Uses existing test infrastructure Functions returning data Proper pytest fixtures Generic test names Descriptive, specific test names Schema class named TestReadSchema* Renamed to avoid pytest collection Manual session setup in each test Leverages async_session fixture 3 tests with warnings 4 tests, no warnings 🧪 Test Coverage The refactored tests now comprehensively cover: Core fix verification: Ensures relationship fields are excluded from SELECT statements Model validation: Tests return_as_model=True works without validation errors Regression prevention: Verifies get_joined functionality remains intact SQL-level verification: Directly checks generated SQL to prevent cartesian products The refactored test file now seamlessly integrates with the existing test suite, follows all established conventions, and provides comprehensive coverage of the GitHub issue benavlabs#199 fix while eliminating all pytest warnings.
LucasQR
approved these changes
Sep 16, 2025
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.
🧪 Test Coverage
The refactored tests now comprehensively cover:
SELECTstatements.return_as_model=Trueworks without validation errors.get_joinedfunctionality remains intact.The refactored test file now seamlessly integrates with the existing test suite, follows all established conventions, and provides comprehensive coverage of the GitHub issue #199 fix while eliminating all pytest warnings.
Checklist
Additional Notes
Include any additional information that you think is important for reviewers to know.