Skip to content

refactor(test): replace SimpleNamespace with real Site model in test_fields.py#37680

Open
lavkeshdwivedi wants to merge 11 commits into
langgenius:mainfrom
lavkeshdwivedi:refactor/test-fields-real-site-model
Open

refactor(test): replace SimpleNamespace with real Site model in test_fields.py#37680
lavkeshdwivedi wants to merge 11 commits into
langgenius:mainfrom
lavkeshdwivedi:refactor/test-fields-real-site-model

Conversation

@lavkeshdwivedi

Copy link
Copy Markdown

Summary

Addresses #37604.

test_fields.py used SimpleNamespace to stand in for the ORM Site model passed to Site.model_validate(). pyrefly flags this as a type error since SimpleNamespace doesn't satisfy the Site type annotation.

Changes

  • Replace both SimpleNamespace(...) site stubs with models.model.Site(...) instances
  • Site has a custom_disclaimer property backed by _custom_disclaimer, so all fields accessed by the Pydantic Site serializer are correctly typed
  • Remove the now-unused SimpleNamespace import

Why this works

models.model.Site has mapped_column declarations for all fields the Pydantic Site model reads (title, icon_type, icon, default_language, show_workflow_steps, etc.). The custom_disclaimer column is exposed via a @property, so from_attributes=True reads it correctly.

Test plan

  • uv run pytest api/tests/unit_tests/controllers/common/test_fields.py passes
  • uv run pyrefly check api/tests/unit_tests/controllers/common/test_fields.py reports no SimpleNamespace-related errors

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
…fields.py

Fixes pyrefly type errors from langgenius#37604.

Pydantic's model_validate receives a real models.model.Site ORM instance
instead of a SimpleNamespace. This satisfies the type checker since Site
has all the required attributes (title, icon_type, icon, etc.) as mapped
columns, and custom_disclaimer is exposed via a property on the model.

Also removes the unused SimpleNamespace import.

Refs langgenius#37604
@dosubot dosubot Bot added size:S This PR changes 10-29 lines, ignoring generated files. refactor labels Jun 20, 2026
@github-actions

github-actions Bot commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

Pyrefly Diff

base → PR
--- /tmp/pyrefly_base.txt	2026-06-28 14:42:55.455623001 +0000
+++ /tmp/pyrefly_pr.txt	2026-06-28 14:42:43.699535853 +0000
@@ -6553,18 +6553,26 @@
   --> tests/unit_tests/oss/tencent_cos/test_tencent_cos.py:17:9
 ERROR Class member `TestVolcengineTos.setup_method` overrides a member in a parent class but is missing an `@override` decorator [missing-override-decorator]
   --> tests/unit_tests/oss/volcengine_tos/test_volcengine_tos.py:17:9
+ERROR Class member `_ConcreteNodeRepo.save` overrides a member in a parent class but is missing an `@override` decorator [missing-override-decorator]
+  --> tests/unit_tests/repositories/test_sqlalchemy_api_workflow_node_execution_repository.py:15:9
+ERROR Class member `_ConcreteNodeRepo.save_execution_data` overrides a member in a parent class but is missing an `@override` decorator [missing-override-decorator]
+  --> tests/unit_tests/repositories/test_sqlalchemy_api_workflow_node_execution_repository.py:18:9
+ERROR Class member `_ConcreteNodeRepo.get_by_workflow_execution` overrides a member in a parent class but is missing an `@override` decorator [missing-override-decorator]
+  --> tests/unit_tests/repositories/test_sqlalchemy_api_workflow_node_execution_repository.py:21:9
+ERROR Class member `_ConcreteRunRepo.save` overrides a member in a parent class but is missing an `@override` decorator [missing-override-decorator]
+  --> tests/unit_tests/repositories/test_sqlalchemy_api_workflow_run_repository.py:18:9
 ERROR Argument `SimpleNamespace` is not assignable to parameter `reason_model` with type `WorkflowPauseReason` in function `repositories.sqlalchemy_api_workflow_run_repository._build_human_input_required_reason` [bad-argument-type]
-  --> tests/unit_tests/repositories/test_sqlalchemy_api_workflow_run_repository.py:38:9
+  --> tests/unit_tests/repositories/test_sqlalchemy_api_workflow_run_repository.py:48:9
 ERROR Argument `SimpleNamespace` is not assignable to parameter `form_model` with type `HumanInputForm | None` in function `repositories.sqlalchemy_api_workflow_run_repository._build_human_input_required_reason` [bad-argument-type]
-  --> tests/unit_tests/repositories/test_sqlalchemy_api_workflow_run_repository.py:39:9
+  --> tests/unit_tests/repositories/test_sqlalchemy_api_workflow_run_repository.py:49:9
 ERROR Argument `list[SimpleNamespace]` is not assignable to parameter `recipients` with type `Sequence[HumanInputFormRecipient]` in function `repositories.sqlalchemy_api_workflow_run_repository._build_human_input_required_reason` [bad-argument-type]
-  --> tests/unit_tests/repositories/test_sqlalchemy_api_workflow_run_repository.py:40:9
+  --> tests/unit_tests/repositories/test_sqlalchemy_api_workflow_run_repository.py:50:9
 ERROR Argument `SimpleNamespace` is not assignable to parameter `reason_model` with type `WorkflowPauseReason` in function `repositories.sqlalchemy_api_workflow_run_repository._build_human_input_required_reason` [bad-argument-type]
-  --> tests/unit_tests/repositories/test_sqlalchemy_api_workflow_run_repository.py:54:9
+  --> tests/unit_tests/repositories/test_sqlalchemy_api_workflow_run_repository.py:64:9
 ERROR Argument `SimpleNamespace` is not assignable to parameter `form_model` with type `HumanInputForm | None` in function `repositories.sqlalchemy_api_workflow_run_repository._build_human_input_required_reason` [bad-argument-type]
-  --> tests/unit_tests/repositories/test_sqlalchemy_api_workflow_run_repository.py:55:9
+  --> tests/unit_tests/repositories/test_sqlalchemy_api_workflow_run_repository.py:65:9
 ERROR Argument `list[SimpleNamespace]` is not assignable to parameter `recipients` with type `Sequence[HumanInputFormRecipient]` in function `repositories.sqlalchemy_api_workflow_run_repository._build_human_input_required_reason` [bad-argument-type]
-  --> tests/unit_tests/repositories/test_sqlalchemy_api_workflow_run_repository.py:56:9
+  --> tests/unit_tests/repositories/test_sqlalchemy_api_workflow_run_repository.py:66:9
 ERROR Object of class `object` has no attribute `prompt` [missing-attribute]
    --> tests/unit_tests/services/agent/test_agent_services.py:959:12
 ERROR Cannot index into `object` [bad-index]

@github-actions

github-actions Bot commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

Pyrefly Type Coverage

Metric Base PR Delta
Type coverage 51.48% 51.49% +0.01%
Strict coverage 51.00% 51.01% +0.01%
Typed symbols 30,824 30,838 +14
Untyped symbols 29,325 29,325 0
Modules 2931 2932 +1

@lavkeshdwivedi

Copy link
Copy Markdown
Author

Heads up on overlap with #37678 and #37679

This PR shares 4 files with two other open PRs:

File #37678 #37679 #37680
sqlalchemy_api_workflow_node_execution_repository.py
sqlalchemy_api_workflow_run_repository.py
test_sqlalchemy_api_workflow_node_execution_repository.py
test_sqlalchemy_api_workflow_run_repository.py

Whichever of the three merges first, the others will need a rebase. #37678 is a bug fix (None rowcount crash) with no Pyrefly regressions, so merging that one first would give this PR and #37679 a clean base to rebase onto.

Also noting: the Pyrefly diff for this PR introduces two new bad-instantiation errors in the repository test files that aren't present in the base — worth confirming those are expected or addressing before merge.

@lavkeshdwivedi

Copy link
Copy Markdown
Author

Fixed the two bad-instantiation pyrefly errors in a new commit (same fix as #37678 and #37679 — these branches share the repository test files).

Root cause: DifyAPISQLAlchemyWorkflowNodeExecutionRepository and DifyAPISQLAlchemyWorkflowRunRepository inherit abstract methods (save, save_execution_data, get_by_workflow_execution) from their Protocol base classes that aren't implemented in the concrete service-layer classes. Pyrefly correctly flags direct instantiation.

Fix: added _ConcreteNodeRepo and _ConcreteRunRepo stub subclasses in the test files.

@lavkeshdwivedi

Copy link
Copy Markdown
Author

Friendly bump - happy to address any review feedback.

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

Labels

refactor size:S This PR changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants