test(BA-4898): add unit service and component tests for Deployment and ModelServing by HyeockJinKim · Pull Request #9698 · lablup/backend.ai

HyeockJinKim · 2026-03-05T13:54:33Z

Summary

Add unit service tests for 7 Deployment actions (18 test cases) and 3 ModelServing actions (11 test cases)
Add component tests for Deployment lifecycle (8 tests) and ModelServing SDK routes (11 tests)
No modifications to existing test files or component conftest.py

Test plan

pants test tests/unit/manager/services/deployment/test_deployment_crud_actions.py passes
pants test tests/unit/manager/services/model_serving/actions/test_model_serving_crud_actions.py passes
pants test tests/component/deployment/test_deployment_lifecycle.py passes
pants test tests/component/model_serving/test_model_serving_routes.py passes

🤖 Generated with Claude Code

Add test_deployment_crud_actions.py covering: - CreateLegacyDeploymentAction (draft returns info, CHECK_PENDING marking, domain error) - DestroyDeploymentAction (success with DESTROYING, not found, idempotent) - GetReplicaByIdAction (existing data, none for missing, zero-weight inactive) - SearchReplicasAction (pagination, empty result, next page) - SearchAccessTokensAction (token list, empty list) - SyncReplicaAction (CHECK_REPLICA marking, already synced) - GetRevisionByIdAction (existing data, not found) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Add test_model_serving_crud_actions.py with 11 test cases covering: - ModifyEndpointAction: replica count change triggers CHECK_REPLICA, no change skips marking, non-existent endpoint raises error - ForceSyncAction: valid service notifies appproxy, non-existent service raises ModelServiceNotFound, non-owner raises EndpointAccessForbiddenError - DeleteRouteAction: HEALTHY route deletion with session destruction, PROVISIONING raises InvalidAPIParameters, sessionless route skips destroy_session, non-existent route raises RouteNotFound, non-owner raises EndpointAccessForbiddenError Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

…K methods - tests/component/deployment/test_deployment_lifecycle.py: SDK client lifecycle tests (create, update/scale, search routes, destroy, get revision, role-based access) - tests/component/model_serving/test_model_serving_routes.py: SDK client route tests (create, scale, sync, generate token, delete, delete route) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

Adds new unit and component test coverage for Deployment and ModelServing service actions / SDK routes.

Changes:

Added mock-based unit tests for DeploymentService CRUD/replica/revision actions.
Added mock-based unit tests for ModelServingService actions (modify endpoint, force sync, delete route).
Added component tests validating SDK route wiring/serialization and common error cases for Deployment and ModelServing.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
tests/unit/manager/services/model_serving/actions/test_model_serving_crud_actions.py	Adds unit-level action tests for ModelServing processors/service behavior.
tests/unit/manager/services/deployment/test_deployment_crud_actions.py	Adds unit-level action tests for Deployment processors/service behavior.
tests/component/model_serving/test_model_serving_routes.py	Adds SDK-level component tests for legacy model serving client routes.
tests/component/deployment/test_deployment_lifecycle.py	Adds SDK-level component tests for deployment lifecycle endpoints.
changes/9698.test.md	Adds a changelog entry documenting the test additions.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-05T13:57:52Z

tests/unit/manager/services/deployment/test_deployment_crud_actions.py

+    async def test_already_destroying_handled_idempotently(
+        self,
+        processors: DeploymentProcessors,
+        mock_deployment_repository: MagicMock,
+        mock_deployment_controller: MagicMock,
+        endpoint_info: DeploymentInfo,
+        endpoint_id: uuid.UUID,
+    ) -> None:
+        """Already DESTROYING state handled idempotently."""
+        mock_deployment_repository.get_endpoint_info = AsyncMock(return_value=endpoint_info)
+        mock_deployment_controller.destroy_deployment = AsyncMock(return_value=True)
+        mock_deployment_controller.mark_lifecycle_needed = AsyncMock()


This test claims to cover the 'already DESTROYING' lifecycle, but endpoint_info comes from the shared fixture with lifecycle set to EndpointLifecycle.READY. As written, it doesn't exercise the intended branch. Set endpoint_info.state.lifecycle to the DESTROYING value (or create a dedicated fixture for a DESTROYING endpoint) before calling the action.

Copilot · 2026-03-05T13:57:53Z

tests/component/deployment/test_deployment_lifecycle.py

+                ),
+            ),
+        )
+        with pytest.raises(Exception):


Catching Exception here makes the test less precise and may hide unexpected failures (e.g., client-side validation errors vs server/API errors). Prefer asserting the specific expected SDK exception type (similar to other tests in this file using NotFoundError), and/or assert on the error payload/message if available.

Suggested change

with pytest.raises(Exception):

with pytest.raises(NotFoundError):

Copilot · 2026-03-05T13:57:53Z

tests/unit/manager/services/deployment/test_deployment_crud_actions.py

+    async def test_triggers_check_replica_marking(
+        self,
+        processors: DeploymentProcessors,
+        mock_deployment_controller: MagicMock,
+        deployment_id: uuid.UUID,
+    ) -> None:
+        """Replica count mismatch triggers CHECK_REPLICA marking."""
+        mock_deployment_controller.mark_lifecycle_needed = AsyncMock()
+
+        action = SyncReplicaAction(deployment_id=deployment_id)
+        result = await processors.sync_replicas.wait_for_complete(action)
+
+        assert result.success is True
+        mock_deployment_controller.mark_lifecycle_needed.assert_called_once_with(
+            DeploymentLifecycleType.CHECK_REPLICA
+        )
+
+    async def test_already_synced_still_marks(
+        self,
+        processors: DeploymentProcessors,
+        mock_deployment_controller: MagicMock,
+        deployment_id: uuid.UUID,
+    ) -> None:
+        """Already synced state still performs marking."""
+        mock_deployment_controller.mark_lifecycle_needed = AsyncMock()
+
+        action = SyncReplicaAction(deployment_id=deployment_id)
+        result = await processors.sync_replicas.wait_for_complete(action)
+
+        assert result.success is True
+        mock_deployment_controller.mark_lifecycle_needed.assert_called_once_with(
+            DeploymentLifecycleType.CHECK_REPLICA
+        )


These two tests are currently identical in setup and assertions, so they don't validate different behavior. Consider parametrizing a single test (or adding distinct setup/assertions that actually differentiate 'mismatch' vs 'already synced' scenarios) to avoid duplication and improve signal.

Copilot · 2026-03-05T13:57:53Z

tests/unit/manager/services/model_serving/actions/test_model_serving_crud_actions.py

+    async def test_valid_service_returns_success(
+        self,
+        model_serving_processors: ModelServingProcessors,
+        mock_check_user_access: AsyncMock,
+        mock_get_endpoint_access_validation_data: AsyncMock,
+        mock_notify_appproxy: AsyncMock,
+        user_data: UserData,
+        service_id: uuid.UUID,
+    ) -> None:
+        """Valid service_id returns success=true with notify called once."""
+        mock_get_endpoint_access_validation_data.return_value = self._make_validation_data(
+            user_data
+        )


This test injects mock_check_user_access (so the patch is active) but never asserts it was awaited. Adding an assert_awaited() / assert_awaited_once() expectation would make the test actually enforce that the access check happens (and prevent regressions where authorization is accidentally skipped).

Copilot · 2026-03-05T13:57:53Z

tests/unit/manager/services/model_serving/actions/test_model_serving_crud_actions.py

+        mock_deployment_controller: MagicMock,
+        endpoint_id: uuid.UUID,
+    ) -> None:
+        """replica_count change (2->5) returns success=true with CHECK_REPLICA marking."""


The docstring claims a '(2->5)' replica transition, but the test setup doesn't establish an initial replica count anywhere—only that the updater spec marks the replica count as modified. Consider rewording the docstring to match what the test actually verifies (e.g., 'replica_count modified triggers CHECK_REPLICA marking').

Suggested change

"""replica_count change (2->5) returns success=true with CHECK_REPLICA marking."""

"""replica_count marked as modified returns success=true with CHECK_REPLICA marking."""

…tructor Use `replicas`, `group_name`, `domain_name` (actual Pydantic field names) instead of `desired_session_count`, `group`, `domain` (validation aliases that only work during deserialization, not direct construction). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

HyeockJinKim and others added 3 commits March 5, 2026 22:31

Copilot AI review requested due to automatic review settings March 5, 2026 13:54

github-actions bot assigned HyeockJinKim Mar 5, 2026

github-actions bot added the size:XL 500~ LoC label Mar 5, 2026

changelog: add news fragment for PR #9698

9248213

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Copilot AI reviewed Mar 5, 2026

View reviewed changes

HyeockJinKim merged commit 4da1287 into main Mar 5, 2026
23 checks passed

HyeockJinKim deleted the BA-4898 branch March 5, 2026 14:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test(BA-4898): add unit service and component tests for Deployment and ModelServing#9698

test(BA-4898): add unit service and component tests for Deployment and ModelServing#9698
HyeockJinKim merged 5 commits intomainfrom
BA-4898

HyeockJinKim commented Mar 5, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 5, 2026

Uh oh!

Copilot AI Mar 5, 2026

Uh oh!

Copilot AI Mar 5, 2026

Uh oh!

Copilot AI Mar 5, 2026

Uh oh!

Copilot AI Mar 5, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	with pytest.raises(Exception):
	with pytest.raises(NotFoundError):

	"""replica_count change (2->5) returns success=true with CHECK_REPLICA marking."""
	"""replica_count marked as modified returns success=true with CHECK_REPLICA marking."""

Conversation

HyeockJinKim commented Mar 5, 2026

Summary

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants