test(BA-4897): add unit service + component tests for PermissionController and Domain by HyeockJinKim · Pull Request #9697 · lablup/backend.ai

HyeockJinKim · 2026-03-05T13:47:25Z

Summary

Add unit service tests for PermissionController (21 actions: CreateRole, GetRoleDetail, UpdateRole, DeleteRole, PurgeRole, AssignRole, RevokeRole, SearchRoles, SearchUsersAssignedToRole, CreatePermission, DeletePermission, SearchPermissions, CreateObjectPermission, DeleteObjectPermission, SearchObjectPermissions, UpdateRolePermissions, SearchEntities, GetEntityTypes, SearchElementAssociations, and more)
Add unit service tests for Domain (9 actions: CreateDomain, GetDomain, SearchDomains, ModifyDomain, DeleteDomain, PurgeDomain, CreateDomainNode, ModifyDomainNode, SearchRGDomains)
Add RBAC role lifecycle component tests (create → assign → verify → revoke → verify → delete, scope/entity discovery, pagination and filtering)

Test plan

pants test tests/unit/manager/services/permission_controller::
pants test tests/unit/manager/services/domain::
pants test tests/component/rbac::

Resolves BA-4897

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

Copilot

Pull request overview

Adds automated test coverage for RBAC/Domain service layers and RBAC REST API behavior to support BA-4897.

Changes:

Introduces unit tests for PermissionControllerService methods using a mocked repository layer.
Introduces unit tests for DomainService methods (including input validation and error propagation).
Adds a component-level RBAC role lifecycle test suite covering create/assign/revoke/delete/purge flows plus pagination/filtering checks.

Reviewed changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
tests/unit/manager/services/permission_controller/test_permission_controller_service.py	New unit tests for `PermissionControllerService` (mocked repository interactions and action results).
tests/unit/manager/services/domain/test_domain_service.py	New unit tests for `DomainService`, including domain name validation and repository delegation.
tests/unit/manager/services/domain/init.py	Adds package marker for the new domain service unit test package.
tests/unit/manager/services/domain/BUILD	Adds Pants target to run the new domain service unit tests.
tests/component/rbac/test_rbac_role_lifecycle.py	Adds component tests validating end-to-end RBAC role lifecycle and search/pagination behaviors via SDK v2.
changes/9697.test.md	Adds changelog entry documenting the new tests.
DEVIATION.md	Documents deviations/alternatives from the original BA-4897 testing plan.

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

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

tests/component/rbac/test_rbac_role_lifecycle.py

+    async def test_revoke_nonexistent_assignment_raises_error(
+        self,
+        admin_registry: BackendAIClientRegistry,
+        role_factory: RoleFactory,
+    ) -> None:
+        created = await role_factory()
+        with pytest.raises(Exception):
+            await admin_registry.rbac.revoke_role(
+                RevokeRoleRequest(
+                    user_id=uuid.uuid4(),
+                    role_id=created.role.id,
+                )
+            )
+


pytest.raises(Exception) is too broad here and can hide unexpected failures. The RBAC client maps HTTP 400 to ai.backend.client.v2.exceptions.InvalidRequestError, so this test should assert that specific exception (or another concrete expected exception type).

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

DEVIATION.md

+| Item | Type | Reason / Alternative |
+|------|------|----------------------|
+| CheckPermissionAction component test | Alternative applied | `check_permission` is not exposed via REST API or client SDK v2. Lifecycle test verifies assignment/revocation via `search_assigned_users` instead. |
+| Scope chain (GLOBAL/DOMAIN/USER inheritance through AUTO edges) | Not implemented | Scope chain inheritance is internal service/repository behavior not testable at the HTTP API layer. Scope type discovery is tested instead. |
+| Domain + permission integration (cascade behavior) | Alternative applied | Permission CRUD APIs (create_permission, update_role_permissions) are not exposed via REST API. Domain creation requires separate route registration not available in RBAC conftest. Role lifecycle with soft-delete exclusion from search is tested as alternative. |


The markdown table rows start with ||, which creates an empty leading column and renders oddly. Use a standard markdown table format (| Item | Type | Reason / Alternative | and | --- | --- | --- |) for consistent rendering.

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

tests/unit/manager/services/permission_controller/test_permission_controller_service.py

+        assert result.data.created_at is not None
+        assert result.data.updated_at is not None
+        assert result.data.created_at <= now or result.data.created_at >= now
+


The assertion created_at <= now or created_at >= now is tautologically true and does not validate anything. Replace it with a meaningful check (e.g., assert the timestamp is timezone-aware, within a reasonable delta of now, or that created_at == updated_at when created).

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

tests/unit/manager/services/permission_controller/test_permission_controller_service.py

@@ -0,0 +1,1042 @@
+"""
+Unit tests for PermissionControllerService.
+Tests all 21 service methods using mocked repository layer.


This module docstring claims it "Tests all 21 service methods", but this file does not include coverage for get_scope_types and search_scopes (those appear to be in a different test module). Please reword the docstring to avoid stating full coverage in this file, or add the missing tests here and remove the separate module.

Suggested change

Tests all 21 service methods using mocked repository layer.

Covers service methods using a mocked repository layer.

…tions) Add comprehensive unit tests for all 21 PermissionControllerService methods covering role CRUD, assignment/revocation, permission management, search operations, entity types, and element associations. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

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

- Full lifecycle: create -> assign -> verify -> revoke -> verify -> delete - Create -> update -> purge lifecycle with verification - Soft-deleted roles excluded from search - Assignment audit trail (granted_at tracking) - Multiple roles assigned independently to same user - Scope and entity type discovery tests - Search pagination and status filtering - Error handling (nonexistent role, nonexistent assignment) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Soft delete sets role status to DELETED but search_roles doesn't auto-exclude deleted roles. Updated test to use statuses=[ACTIVE] filter and verify DELETED status appears with correct filter. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

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

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

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

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

github-actions bot assigned HyeockJinKim Mar 5, 2026

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

HyeockJinKim added a commit that referenced this pull request Mar 5, 2026

changelog: add news fragment for PR #9697

3c8c014

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

Copilot started reviewing on behalf of HyeockJinKim March 5, 2026 13:48 View session

Copilot AI reviewed Mar 5, 2026

View reviewed changes

HyeockJinKim and others added 7 commits March 6, 2026 00:29

test(BA-4897): add unit service tests for Domain (9 actions)

76bf621

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

changelog: add news fragment for PR #9697

42bd86f

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

fix(BA-4897): add type annotations to SearchResult variables for mypy…

bb42796

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

chore(BA-4897): remove DEVIATION.md worker artifact

8b63e3e

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

HyeockJinKim force-pushed the BA-4897 branch from d5fc42a to 8b63e3e Compare March 5, 2026 15:29

HyeockJinKim merged commit 26da025 into main Mar 5, 2026
23 checks passed

HyeockJinKim deleted the BA-4897 branch March 5, 2026 15:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test(BA-4897): add unit service + component tests for PermissionController and Domain#9697

test(BA-4897): add unit service + component tests for PermissionController and Domain#9697
HyeockJinKim merged 7 commits intomainfrom
BA-4897

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!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	Tests all 21 service methods using mocked repository layer.

	Covers service methods using a mocked repository layer.

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!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants