refactor(test): replace SimpleNamespace with real model instances in test_snippet.py#37679
Open
lavkeshdwivedi wants to merge 3 commits into
Open
Conversation
SQLAlchemy's CursorResult.rowcount can return None for some DBAPI drivers/operations. Five bulk-delete methods used result.rowcount directly in += or as a return value, causing: TypeError: unsupported operand type(s) for +=: 'int' and 'NoneType' Apply the `result.rowcount or 0` pattern already used in sibling methods (delete_by_runs, count_by_runs) across all five affected sites: - sqlalchemy_api_workflow_node_execution_repository.py - delete_expired_executions - delete_executions_by_app - delete_executions_by_ids - sqlalchemy_api_workflow_run_repository.py - delete_runs_by_ids - delete_runs_by_app Fixes langgenius#37643
Covers all five affected methods to guard against regression: - delete_expired_executions, delete_executions_by_app, delete_executions_by_ids (node execution repository) - delete_runs_by_ids, delete_runs_by_app (run repository) Each test mocks the DBAPI session to return rowcount=None and asserts the method returns 0 instead of raising TypeError.
…test_snippet.py Fixes the pyrefly type errors reported in langgenius#37604. SimpleNamespace objects passed where Workflow, Account, and Tag are expected do not satisfy the type checker. Replace with real ORM instances so pyrefly can verify the fields being accessed actually exist on the model. Also replace SimpleNamespace session stubs with unittest.mock.Mock, which is the standard pattern in the rest of the test suite. Refs langgenius#37604
dosubot
Bot
added
size:S
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
1 participant
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.
Summary
Addresses #37604.
test_snippet.pyusedSimpleNamespaceto stand in forWorkflow,Account, andTagmodel instances.pyreflyflags these as type errors becauseSimpleNamespacedoes not satisfy the model's type annotations.Changes
Workflow(graph=...)replacesSimpleNamespace(graph=...)Account()with.id/.nameset replacesSimpleNamespace(id=..., name=...)Tag()with.idset replacesSimpleNamespace(id=...)Mock(...)replacesSimpleNamespace(...)for session stubs (consistent with the rest of the test suite)Why real instances, not
Mock(spec=...)Per maintainer guidance in #37604: real class instances, not mocks. Real instances let pyrefly verify that the attributes being accessed (
.graph,.name,.id) are actual declared fields on the model.Test plan
uv run pytest api/tests/unit_tests/models/test_snippet.pypassesuv run pyrefly check api/tests/unit_tests/models/test_snippet.pyreports no SimpleNamespace-related errors