refactor: migrate workflow run repository tests from mocks to …#33837
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly refactors the testing approach for the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request is a great improvement, migrating the workflow run repository tests from mocks to integration tests using testcontainers. This provides much higher confidence by testing against a real database instance. The new tests are well-structured and cover a good range of scenarios for pagination and counting. I have a couple of minor suggestions to improve code clarity and test precision.
api/tests/test_containers_integration_tests/repositories/test_workflow_run_repository.py
Outdated
Show resolved
Hide resolved
api/tests/test_containers_integration_tests/repositories/test_workflow_run_repository.py
Outdated
Show resolved
Hide resolved
|
@asukaminato0721 Could you please review my PR and share any feedback? |
Pyrefly Diffbase → PR--- /tmp/pyrefly_base.txt 2026-03-21 02:26:55.723376227 +0000
+++ /tmp/pyrefly_pr.txt 2026-03-21 02:26:46.307374742 +0000
@@ -913,6 +913,8 @@
--> tests/test_containers_integration_tests/repositories/test_sqlalchemy_api_workflow_node_execution_repository.py:122:70
ERROR Generator function should return `Generator` [bad-return]
--> tests/test_containers_integration_tests/repositories/test_sqlalchemy_api_workflow_run_repository.py:112:56
+ERROR Generator function should return `Generator` [bad-return]
+ --> tests/test_containers_integration_tests/repositories/test_workflow_run_repository.py:88:56
ERROR Argument `Literal['active']` is not assignable to parameter `status` with type `AccountStatus | SQLCoreOperations[AccountStatus]` in function `models.account.Account.__init__` [bad-argument-type]
--> tests/test_containers_integration_tests/services/dataset_service_update_delete.py:40:20
ERROR Argument `Literal['normal']` is not assignable to parameter `status` with type `SQLCoreOperations[TenantStatus] | TenantStatus` in function `models.account.Tenant.__init__` [bad-argument-type]
@@ -5535,8 +5537,6 @@
--> tests/unit_tests/repositories/test_sqlalchemy_execution_extra_content_repository.py:97:23
ERROR Argument `_FakeSessionMaker` is not assignable to parameter `session_maker` with type `sessionmaker[Session]` in function `repositories.sqlalchemy_execution_extra_content_repository.SQLAlchemyExecutionExtraContentRepository.__init__` [bad-argument-type]
--> tests/unit_tests/repositories/test_sqlalchemy_execution_extra_content_repository.py:159:23
-ERROR Cannot instantiate `DifyAPISQLAlchemyWorkflowRunRepository` because the following members are abstract: `save` [bad-instantiation]
- --> tests/unit_tests/repositories/test_workflow_run_repository.py:24:54
ERROR Argument `Literal['workflow-run']` is not assignable to parameter `value` with type `SQLCoreOperations[WorkflowNodeExecutionTriggeredFrom] | WorkflowNodeExecutionTriggeredFrom` in function `sqlalchemy.orm.base.Mapped.__set__` [bad-argument-type]
--> tests/unit_tests/repositories/workflow_node_execution/test_sqlalchemy_repository.py:295:31
ERROR Argument `Literal['account']` is not assignable to parameter `value` with type `CreatorUserRole | SQLCoreOperations[CreatorUserRole]` in function `sqlalchemy.orm.base.Mapped.__set__` [bad-argument-type]
|
Pyrefly Diffbase → PR--- /tmp/pyrefly_base.txt 2026-03-21 02:39:45.772627325 +0000
+++ /tmp/pyrefly_pr.txt 2026-03-21 02:39:36.794655587 +0000
@@ -27,9 +27,9 @@
ERROR Object of class `MissingRouter` has no attribute `get` [missing-attribute]
--> controllers/console/ping.py:10:2
ERROR Object of class `MissingRouter` has no attribute `get` [missing-attribute]
- --> controllers/console/setup.py:40:2
+ --> controllers/console/setup.py:39:2
ERROR Object of class `MissingRouter` has no attribute `post` [missing-attribute]
- --> controllers/console/setup.py:65:2
+ --> controllers/console/setup.py:64:2
ERROR Object of class `MissingRouter` has no attribute `get` [missing-attribute]
--> controllers/console/version.py:30:2
ERROR `result` may be uninitialized [unbound-name]
@@ -913,6 +913,8 @@
--> tests/test_containers_integration_tests/repositories/test_sqlalchemy_api_workflow_node_execution_repository.py:122:70
ERROR Generator function should return `Generator` [bad-return]
--> tests/test_containers_integration_tests/repositories/test_sqlalchemy_api_workflow_run_repository.py:112:56
+ERROR Generator function should return `Generator` [bad-return]
+ --> tests/test_containers_integration_tests/repositories/test_workflow_run_repository.py:89:56
ERROR Argument `Literal['active']` is not assignable to parameter `status` with type `AccountStatus | SQLCoreOperations[AccountStatus]` in function `models.account.Account.__init__` [bad-argument-type]
--> tests/test_containers_integration_tests/services/dataset_service_update_delete.py:40:20
ERROR Argument `Literal['normal']` is not assignable to parameter `status` with type `SQLCoreOperations[TenantStatus] | TenantStatus` in function `models.account.Tenant.__init__` [bad-argument-type]
@@ -5535,8 +5537,6 @@
--> tests/unit_tests/repositories/test_sqlalchemy_execution_extra_content_repository.py:97:23
ERROR Argument `_FakeSessionMaker` is not assignable to parameter `session_maker` with type `sessionmaker[Session]` in function `repositories.sqlalchemy_execution_extra_content_repository.SQLAlchemyExecutionExtraContentRepository.__init__` [bad-argument-type]
--> tests/unit_tests/repositories/test_sqlalchemy_execution_extra_content_repository.py:159:23
-ERROR Cannot instantiate `DifyAPISQLAlchemyWorkflowRunRepository` because the following members are abstract: `save` [bad-instantiation]
- --> tests/unit_tests/repositories/test_workflow_run_repository.py:24:54
ERROR Argument `Literal['workflow-run']` is not assignable to parameter `value` with type `SQLCoreOperations[WorkflowNodeExecutionTriggeredFrom] | WorkflowNodeExecutionTriggeredFrom` in function `sqlalchemy.orm.base.Mapped.__set__` [bad-argument-type]
--> tests/unit_tests/repositories/workflow_node_execution/test_sqlalchemy_repository.py:295:31
ERROR Argument `Literal['account']` is not assignable to parameter `value` with type `CreatorUserRole | SQLCoreOperations[CreatorUserRole]` in function `sqlalchemy.orm.base.Mapped.__set__` [bad-argument-type]
|
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request is a great improvement, replacing mocked unit tests for the workflow run repository with comprehensive integration tests using testcontainers. The new tests cover various scenarios including pagination, filtering, error handling, and tenant isolation, which increases confidence in the repository's implementation. I've found a couple of areas for minor improvements to enhance test robustness and code clarity.
api/tests/test_containers_integration_tests/repositories/test_workflow_run_repository.py
Outdated
Show resolved
Hide resolved
api/tests/test_containers_integration_tests/repositories/test_workflow_run_repository.py
Outdated
Show resolved
Hide resolved
api/tests/test_containers_integration_tests/repositories/test_workflow_run_repository.py
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
This PR migrates workflow run repository tests from mock-based unit tests to real database integration tests backed by Testcontainers, aligning coverage with actual SQLAlchemy/DB behavior for pagination and count queries.
Changes:
- Added a new Testcontainers integration test suite for
get_paginated_workflow_runsandget_workflow_runs_count(pagination, cursor behavior, tenant isolation, status/time_range filters). - Removed the prior mock-based unit test module for the same repository behaviors.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| api/tests/unit_tests/repositories/test_workflow_run_repository.py | Removes mock-based unit tests that asserted query behavior via patched sessions. |
| api/tests/test_containers_integration_tests/repositories/test_workflow_run_repository.py | Adds DB-backed integration tests covering pagination and count queries with real enum binding/validation behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
api/tests/test_containers_integration_tests/repositories/test_workflow_run_repository.py
Outdated
Show resolved
Hide resolved
api/tests/test_containers_integration_tests/repositories/test_workflow_run_repository.py
Outdated
Show resolved
Hide resolved
api/tests/test_containers_integration_tests/repositories/test_workflow_run_repository.py
Show resolved
Hide resolved
api/tests/test_containers_integration_tests/repositories/test_workflow_run_repository.py
Show resolved
Hide resolved
Pyrefly Diffbase → PR--- /tmp/pyrefly_base.txt 2026-03-21 04:32:15.486916081 +0000
+++ /tmp/pyrefly_pr.txt 2026-03-21 04:32:06.415885219 +0000
@@ -913,6 +913,8 @@
--> tests/test_containers_integration_tests/repositories/test_sqlalchemy_api_workflow_node_execution_repository.py:122:70
ERROR Generator function should return `Generator` [bad-return]
--> tests/test_containers_integration_tests/repositories/test_sqlalchemy_api_workflow_run_repository.py:112:56
+ERROR Generator function should return `Generator` [bad-return]
+ --> tests/test_containers_integration_tests/repositories/test_workflow_run_repository.py:89:56
ERROR Argument `Literal['active']` is not assignable to parameter `status` with type `AccountStatus | SQLCoreOperations[AccountStatus]` in function `models.account.Account.__init__` [bad-argument-type]
--> tests/test_containers_integration_tests/services/dataset_service_update_delete.py:40:20
ERROR Argument `Literal['normal']` is not assignable to parameter `status` with type `SQLCoreOperations[TenantStatus] | TenantStatus` in function `models.account.Tenant.__init__` [bad-argument-type]
@@ -5535,8 +5537,6 @@
--> tests/unit_tests/repositories/test_sqlalchemy_execution_extra_content_repository.py:97:23
ERROR Argument `_FakeSessionMaker` is not assignable to parameter `session_maker` with type `sessionmaker[Session]` in function `repositories.sqlalchemy_execution_extra_content_repository.SQLAlchemyExecutionExtraContentRepository.__init__` [bad-argument-type]
--> tests/unit_tests/repositories/test_sqlalchemy_execution_extra_content_repository.py:159:23
-ERROR Cannot instantiate `DifyAPISQLAlchemyWorkflowRunRepository` because the following members are abstract: `save` [bad-instantiation]
- --> tests/unit_tests/repositories/test_workflow_run_repository.py:24:54
ERROR Argument `Literal['workflow-run']` is not assignable to parameter `value` with type `SQLCoreOperations[WorkflowNodeExecutionTriggeredFrom] | WorkflowNodeExecutionTriggeredFrom` in function `sqlalchemy.orm.base.Mapped.__set__` [bad-argument-type]
--> tests/unit_tests/repositories/workflow_node_execution/test_sqlalchemy_repository.py:295:31
ERROR Argument `Literal['account']` is not assignable to parameter `value` with type `CreatorUserRole | SQLCoreOperations[CreatorUserRole]` in function `sqlalchemy.orm.base.Mapped.__set__` [bad-argument-type]
|
Part of #32454
Changes
api/tests/test_containers_integration_tests/repositories/test_workflow_run_repository.py— 11 integration tests covering pagination (no-filter, status filter,has_more, cursor-based, invalid cursor, tenant isolation) and count queries (grouped counts, status filter, invalid status, time range, combined filters)api/tests/unit_tests/repositories/test_workflow_run_repository.py— replaced by the integration tests aboveScreenshots
Checklist
make lintandmake type-check(backend) andcd web && npx lint-staged(frontend) to appease the lint gods