test: add unit tests for permission hooks, utilities, and service classes#491
Merged
kaviththiranga merged 2 commits intoopenchoreo:mainfrom Apr 8, 2026
Merged
Conversation
…sses
Add 37 new test files covering:
- 30 permission hooks (all untested variants: org-level, entity-scoped,
multi-permission, composite, and dynamic kind-based)
- 2 pure utility functions (humanizeTitle, isFromSourceComponent)
- 3 service classes (OpenChoreoFetchApi, OpenChoreoPermissionApi, ObserverUrlCache)
Signed-off-by: Kavith Lokuhewage <kaviththiranga@gmail.com>
kaviththiranga
requested review from
jhivandb and
sameerajayasoma
as code owners
📝 WalkthroughWalkthroughAdds numerous Jest test suites across packages and plugins, covering API clients, permission hooks, catalog processors, observability caching, scaffolder actions, authorization rules/services, and various utilities. All changes are new test files; no production code or public API declarations were modified. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (29)
plugins/openchoreo/src/utils/componentUtils.test.ts (1)
6-33: Optional: extract a small entity factory to reduce repeated setup.The repeated entity literals and
as unknown as Entitycasts make the tests slightly noisy; a tiny helper would improve readability and future additions.♻️ Proposed refactor
import { Entity } from '@backstage/catalog-model'; import { isFromSourceComponent } from './componentUtils'; describe('isFromSourceComponent', () => { + const makeEntity = (spec?: Record<string, unknown>): Entity => + ({ + apiVersion: 'backstage.io/v1alpha1', + kind: 'Component', + metadata: { name: 'test' }, + ...(spec !== undefined ? { spec } : {}), + } as Entity); + it('returns true when entity has workflow in spec', () => { - const entity = { - apiVersion: 'backstage.io/v1alpha1', - kind: 'Component', - metadata: { name: 'test' }, - spec: { workflow: 'build-and-deploy' }, - } as unknown as Entity; + const entity = makeEntity({ workflow: 'build-and-deploy' }); expect(isFromSourceComponent(entity)).toBe(true); }); it('returns false when entity has no workflow in spec', () => { - const entity = { - apiVersion: 'backstage.io/v1alpha1', - kind: 'Component', - metadata: { name: 'test' }, - spec: { type: 'service' }, - } as unknown as Entity; + const entity = makeEntity({ type: 'service' }); expect(isFromSourceComponent(entity)).toBe(false); }); it('returns false when spec is undefined', () => { - const entity = { - apiVersion: 'backstage.io/v1alpha1', - kind: 'Component', - metadata: { name: 'test' }, - } as unknown as Entity; + const entity = makeEntity(); expect(isFromSourceComponent(entity)).toBe(false); }); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/openchoreo/src/utils/componentUtils.test.ts` around lines 6 - 33, Extract a small test helper (e.g., makeEntity or createComponentEntity) to centralize the repeated entity construction used in componentUtils.test.ts so tests calling isFromSourceComponent can pass only the differing fields; update the three tests to use this helper instead of repeating the literal objects and the "as unknown as Entity" cast, ensuring the helper returns a correctly typed Entity and accepts overrides for metadata and spec.packages/design-system/src/components/RjsfTemplates/utils.test.ts (1)
14-25: Consider parameterizing similar assertions withit.each.This would reduce repetition and make future case additions simpler.
Refactor sketch
describe('humanizeTitle', () => { - it('converts simple camelCase to title case', () => { - expect(humanizeTitle('myField')).toBe('My Field'); - expect(humanizeTitle('firstName')).toBe('First Name'); - }); - - it('handles numbers in camelCase', () => { - expect(humanizeTitle('field2Name')).toBe('Field 2Name'); - }); - - it('handles single word', () => { - expect(humanizeTitle('name')).toBe('Name'); - }); + it.each([ + ['myField', 'My Field'], + ['firstName', 'First Name'], + ['field2Name', 'Field 2Name'], + ['name', 'Name'], + ])('humanizes %s -> %s', (input, expected) => { + expect(humanizeTitle(input)).toBe(expected); + }); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/design-system/src/components/RjsfTemplates/utils.test.ts` around lines 14 - 25, Tests in utils.test.ts repeat similar assertions for humanizeTitle; replace the three separate it blocks with a single parameterized test using it.each to iterate input/expected pairs, e.g., provide an array of cases for 'myField' -> 'My Field', 'firstName' -> 'First Name', 'field2Name' -> 'Field 2Name', 'name' -> 'Name' and assert humanizeTitle(input) equals expected; update or remove the individual it(...) blocks and ensure the test description and table include all current cases so future additions are a single row change.plugins/openchoreo-react/src/hooks/useModeState.test.ts (2)
68-99: Limited test coverage fortype: 'file'path.The
setMode,cleanupIndex, andcleanupContainertests all usetype: 'env'. If the hook implementation has branching logic based on thetypeparameter that affects these operations, consider adding at least one test per function fortype: 'file'to ensure parity.Also applies to: 101-144
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/openchoreo-react/src/hooks/useModeState.test.ts` around lines 68 - 99, Tests only cover the 'env' branch of useModeState; add at least one test case per operation (setMode, cleanupIndex, cleanupContainer and getMode) that constructs the hook with useModeState({ type: 'file', ... }) and exercises the same behaviors asserted for 'env' to ensure branch parity. Specifically, create a renderHook call passing type: 'file' (and an appropriate initialContainers/initialFiles payload), call result.current.setMode(...) and assert result.current.getMode(...), then add cases that invoke cleanupIndex and cleanupContainer on file-typed data and assert expected state changes (mirroring the existing 'env' assertions) so the file branch is covered.
113-126: Consider verifying the vacated index after shifting.The test correctly validates that indices shift down, but it doesn't verify that the original index 2 position now falls back to the default
'plain'mode. Adding this assertion would confirm the shift logic is complete and no stale data remains.🔧 Suggested additional assertion
expect(result.current.getMode('main', 0)).toBe('secret'); expect(result.current.getMode('main', 1)).toBe('plain'); + // Verify index 2 is now default (no stale mode from pre-shift state) + expect(result.current.getMode('main', 2)).toBe('plain'); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/openchoreo-react/src/hooks/useModeState.test.ts` around lines 113 - 126, The test should also assert that the vacated highest index now returns the default mode; update the test that uses useModeState to add an assertion after calling cleanupIndex('main', 0) that result.current.getMode('main', 2') (the former index 2 slot) equals 'plain'. This ensures the cleanupIndex shifting (and any fallback/default logic in getMode) leaves no stale data; reference the useModeState hook and its methods setMode, cleanupIndex, and getMode when making the change.plugins/openchoreo-react/src/hooks/useEntityAnnotation.test.ts (2)
8-93: Add mock reset per test for stronger isolation.The current suite works, but adding
beforeEachreset prevents hidden coupling as this file grows.♻️ Suggested update
const mockUseEntity = jest.fn(); jest.mock('@backstage/plugin-catalog-react', () => ({ useEntity: () => mockUseEntity(), })); + +beforeEach(() => { + mockUseEntity.mockReset(); +});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/openchoreo-react/src/hooks/useEntityAnnotation.test.ts` around lines 8 - 93, Add a per-test mock reset to improve isolation: in the test file add a beforeEach that resets the mocked function (mockUseEntity) before each test (e.g., call mockUseEntity.mockReset() or mockUseEntity.mockClear()); this ensures all specs that use useEntity (via the jest.mock wrapper and mockUseEntity) start from a clean mock state and prevents cross-test coupling.
38-41: Avoidas anyin the missing-annotations case.This test can stay fully typed by constructing the same entity shape without
annotations, which reduces type-blind spots.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/openchoreo-react/src/hooks/useEntityAnnotation.test.ts` around lines 38 - 41, Replace the ad-hoc "as any" in the "returns undefined when annotations object is missing" test by returning a properly typed entity object from mockUseEntity that simply omits the annotations field (e.g., mockUseEntity.mockReturnValue({ entity: { metadata: { name: 'test' } } as Entity)); in other words construct the same entity shape using the real entity type used in this test file (Entity or the test's concrete type) rather than casting to any so the test remains fully typed while still lacking annotations.plugins/openchoreo-react/src/hooks/useProjectPermission.test.ts (2)
19-25: Consider assertingcanCreateduring loading state.The test verifies
loadingandcreateDeniedTooltipbut omitscanCreate. Documenting the expected value ofcanCreatewhile loading would make the test more complete and guard against regressions.♻️ Optional enhancement
it('returns loading=true during permission check', () => { mockUsePermission.mockReturnValue({ allowed: false, loading: true }); const { result } = renderHook(() => useProjectPermission()); expect(result.current.loading).toBe(true); expect(result.current.createDeniedTooltip).toBe(''); + // Assert canCreate reflects the underlying allowed state or a safe default + expect(result.current.canCreate).toBe(false); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/openchoreo-react/src/hooks/useProjectPermission.test.ts` around lines 19 - 25, The test "returns loading=true during permission check" currently asserts loading and createDeniedTooltip but omits canCreate; update the test for useProjectPermission to also assert the expected canCreate value during the loading state (given mockUsePermission.mockReturnValue({ allowed: false, loading: true }) assert result.current.canCreate is false) so the test documents and protects the expected behavior of useProjectPermission during loading.
4-7: AddbeforeEachto reset the mock between tests.Without resetting
mockUsePermission, call history accumulates across tests. This could cause thetoHaveBeenCalledWithassertion (line 39) to pass based on calls from earlier tests rather than the current test's invocation.♻️ Suggested fix
const mockUsePermission = jest.fn(); jest.mock('@backstage/plugin-permission-react', () => ({ usePermission: (...args: any[]) => mockUsePermission(...args), })); describe('useProjectPermission', () => { + beforeEach(() => { + mockUsePermission.mockReset(); + }); + it('returns canCreate=true when allowed', () => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/openchoreo-react/src/hooks/useProjectPermission.test.ts` around lines 4 - 7, The mockUsePermission mock is not being cleared between tests, causing accumulated call history to leak assertions; add a beforeEach that resets/clears mockUsePermission (e.g., call mockUsePermission.mockClear() or mockUsePermission.mockReset()) so usePermission calls in one test don't affect another, ensuring assertions like toHaveBeenCalledWith only see the current test's invocations.packages/app/src/apis/OpenChoreoPermissionApi.test.ts (1)
136-161: Consider adding an emptyitemsarray edge case.The error handling and response extraction tests are solid. Optionally, consider adding a test for when the API returns an empty
itemsarray, as accessingitems[0]would beundefined.it('handles empty items array gracefully', async () => { mockFetch.mockResolvedValue({ ok: true, json: () => Promise.resolve({ items: [] }), }); const api = createApi(); const result = await api.authorize({ permission: { name: 'test.read' } }); expect(result).toBeUndefined(); // or assert expected behavior });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/app/src/apis/OpenChoreoPermissionApi.test.ts` around lines 136 - 161, Add a unit test covering the edge case where the API returns ok: true but an empty items array so api.authorize’s extraction of items[0] doesn’t break; in the existing test file add a case using mockFetch.mockResolvedValue to return { ok: true, json: () => Promise.resolve({ items: [] }) }, call createApi() and await api.authorize({ permission: { name: 'test.read' } }), and assert the expected behavior (e.g., result is undefined or whatever authorize() should return for empty items) to ensure api.authorize handles empty items arrays gracefully.plugins/openchoreo-react/src/hooks/useResourceDefinitionPermission.test.ts (1)
43-46: Consider verifying allusePermissioncalls for consistency.
toHaveBeenCalledWithchecks if any call matches, but the hook likely callsusePermissiontwice (for update and delete). Lines 57-59 iterate all calls for the cluster-scoped test—you could apply the same pattern here for consistency.Optional: Verify all calls have resourceRef
- // Verify resourceRef is passed for resource-scoped kind - expect(mockUsePermission).toHaveBeenCalledWith( - expect.objectContaining({ resourceRef: 'component:default/test' }), - ); + // Verify resourceRef is passed for all permission checks + for (const call of mockUsePermission.mock.calls) { + expect(call[0]).toHaveProperty('resourceRef', 'component:default/test'); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/openchoreo-react/src/hooks/useResourceDefinitionPermission.test.ts` around lines 43 - 46, The test currently asserts a single match with expect(mockUsePermission).toHaveBeenCalledWith(...), but the hook calls usePermission multiple times; update the assertion to iterate all calls on mockUsePermission (e.g., inspect mockUsePermission.mock.calls) and assert each call's first argument contains the expected resourceRef ('component:default/test') so both permission calls (update and delete) are verified consistently; reference mockUsePermission and the usePermission calls in the test when making the change.plugins/openchoreo-react/src/hooks/useTraitsPermission.test.ts (1)
4-60: Add mock reset hook for test isolation.The test file lacks a
beforeEachhook to reset mocks between test cases. While each test explicitly sets mock return values, themockUsePermissionmock at module scope persists across tests. Addingjest.clearAllMocks()inbeforeEachensures proper test isolation and prevents any subtle test-order coupling.♻️ Suggested patch
const mockUsePermission = jest.fn(); @@ describe('useTraitsPermission', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + it('returns canViewTraits=true when allowed', () => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/openchoreo-react/src/hooks/useTraitsPermission.test.ts` around lines 4 - 60, Add a beforeEach that calls jest.clearAllMocks() to ensure test isolation: reset the module-scope mockUsePermission before each test so previous return values or call history don't leak between cases; place the beforeEach near the top of the test file (before the describe or inside it) so that useTraitsPermission tests always start with a cleared mock state.plugins/permission-backend-module-openchoreo-policy/src/rules/matchesCatalogEntityCapability.test.ts (1)
400-433: Consider using structural assertions instead of string matching fortoQuerytests.The
JSON.stringify+toContain/not.toContainpattern is fragile and can produce false positives or miss regressions if the JSON structure changes. For example, the assertion on line 430-432 depends on exact JSON key formatting.Consider using recursive object matching or extracting specific filter values:
💡 Example of more robust assertion
- it('builds path-based filter using PROJECT_ID annotation for system kind', () => { - const result = toQuery({ - system: { - action: 'project:view', - allowedPaths: ['ns/acme/project/proj-uuid'], - deniedPaths: [], - }, - }) as any; - - const serialized = JSON.stringify(result); - expect(serialized).toContain(CHOREO_ANNOTATIONS.PROJECT_ID); - expect(serialized).toContain('proj-uuid'); - // Should NOT use the PROJECT annotation for System entities - expect(serialized).not.toContain( - `metadata.annotations.${CHOREO_ANNOTATIONS.PROJECT}"`, - ); - }); + it('builds path-based filter using PROJECT_ID annotation for system kind', () => { + const result = toQuery({ + system: { + action: 'project:view', + allowedPaths: ['ns/acme/project/proj-uuid'], + deniedPaths: [], + }, + }) as any; + + const serialized = JSON.stringify(result); + const projectIdKey = `metadata.annotations.${CHOREO_ANNOTATIONS.PROJECT_ID}`; + const projectKey = `metadata.annotations.${CHOREO_ANNOTATIONS.PROJECT}`; + + expect(serialized).toContain(projectIdKey); + expect(serialized).toContain('proj-uuid'); + // Should NOT use the PROJECT annotation for System entities + expect(serialized).not.toContain(projectKey); + });The current implementation works correctly; this is a maintainability suggestion for when the filter structure evolves.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/permission-backend-module-openchoreo-policy/src/rules/matchesCatalogEntityCapability.test.ts` around lines 400 - 433, Replace fragile JSON string matching in the tests that call toQuery with structural assertions: instead of JSON.stringify(result) + toContain/toContain.not, parse and traverse the returned query object from toQuery and assert presence/absence of specific filter objects or paths using the CHOREO_ANNOTATIONS constants (e.g., check that some filter uses metadata.annotations[CHOREO_ANNOTATIONS.PROJECT] === 'foo' and namespace === 'acme' for the component case, and that for the system case there is a filter using metadata.annotations[CHOREO_ANNOTATIONS.PROJECT_ID] === 'proj-uuid' and no filter referencing metadata.annotations[CHOREO_ANNOTATIONS.PROJECT]); update the two it(...) blocks that reference toQuery and CHOREO_ANNOTATIONS to use these structural checks so the tests are robust to formatting changes.plugins/scaffolder-backend-module-openchoreo/src/actions/environment.test.ts (2)
129-138: Missing test case for authz-enabled path with valid token.The other action tests (
project.test.tsandnamespace.test.ts) include a test verifying that the action proceeds successfully when authz is enabled and a valid token is provided. This file only tests the failure case (no token).➕ Add missing test case for consistency
it('throws when authz enabled and no token', async () => { const action = createEnvironmentAction( buildConfig({ authzEnabled: true }), mockCatalog as any, ); await expect(action.handler(buildCtx() as any)).rejects.toThrow( /User authentication token not available/, ); }); + + it('proceeds with token when authz enabled', async () => { + mockPOST.mockResolvedValueOnce(successResponse()); + const action = createEnvironmentAction( + buildConfig({ authzEnabled: true }), + mockCatalog as any, + ); + await action.handler( + buildCtx({ secrets: { OPENCHOREO_USER_TOKEN: 'tkn' } }) as any, + ); + expect(mockPOST).toHaveBeenCalled(); + }); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/scaffolder-backend-module-openchoreo/src/actions/environment.test.ts` around lines 129 - 138, Add a positive test that mirrors the other modules: when createEnvironmentAction is created with buildConfig({ authzEnabled: true }) and the handler is invoked with a context containing a valid user token, the action should resolve successfully (not throw). Update the test suite in environment.test.ts to call createEnvironmentAction(...) and then call action.handler(buildCtx({ token: '<valid-token>' }) as any) and assert it resolves (or returns expected result) to cover the authz-enabled success path alongside the existing failure test.
89-100: Good test forClusterDataPlanekind detection.Consider also adding a test case verifying the default
dataplane:prefix behavior (line 47 usesdataplane:my-ns/default-dp) to document what kind it resolves to (likelyDataPlane).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/scaffolder-backend-module-openchoreo/src/actions/environment.test.ts` around lines 89 - 100, Add a test that asserts the default "dataplane:" prefix resolves to the expected kind (e.g., "DataPlane"): create an additional it block similar to the existing ClusterDataPlane test that uses createEnvironmentAction and buildCtx with input { dataPlaneRef: 'dataplane:my-ns/default-dp' }, call action.handler(ctx) and then inspect mockPOST.mock.calls[0][1].body to expect body.spec.dataPlaneRef.kind toBe 'DataPlane' and name toBe 'default-dp' to document and verify the default resolution behavior.plugins/scaffolder-backend-module-openchoreo/src/actions/namespace.test.ts (1)
212-232: Consider simplifying the input setup for this test case.The current approach spreads defaults then manually overwrites with
undefined. A cleaner alternative:♻️ Suggested simplification
- it('omits display-name and description annotations when those inputs are not provided', async () => { - mockPOST.mockResolvedValueOnce(successResponse('my-ns')); - const action = createNamespaceAction( - buildConfig(), - mockImmediateCatalog as any, - ); - const ctx = buildCtx({ input: { namespaceName: 'my-ns' } }); - // wipe out optional defaults on the test input - ctx.input.displayName = undefined; - ctx.input.description = undefined; - - await action.handler(ctx as any); + it('omits display-name and description annotations when those inputs are not provided', async () => { + mockPOST.mockResolvedValueOnce(successResponse('my-ns')); + const action = createNamespaceAction( + buildConfig(), + mockImmediateCatalog as any, + ); + const ctx = buildCtx({ + input: { + namespaceName: 'my-ns', + displayName: undefined, + description: undefined, + }, + }); + + await action.handler(ctx as any);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/scaffolder-backend-module-openchoreo/src/actions/namespace.test.ts` around lines 212 - 232, The test "omits display-name and description annotations when those inputs are not provided" currently creates a ctx with defaults then sets ctx.input.displayName and ctx.input.description to undefined; instead, construct the test context input without those keys to avoid manual undefined mutations — e.g., use buildCtx({ input: { namespaceName: 'my-ns' } }) (the ctx variable used by action.handler and the createNamespaceAction call) so mockPOST, body inspection, and assertions remain unchanged.plugins/openchoreo-react/src/hooks/useWorkflowPermission.test.ts (1)
9-42: AddbeforeEachto clear mocks between tests.Same as the previous file—add mock cleanup for test isolation.
Suggested fix
describe('useWorkflowPermission', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + it('returns canCreate=true when allowed', () => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/openchoreo-react/src/hooks/useWorkflowPermission.test.ts` around lines 9 - 42, Tests for useWorkflowPermission rely on mockUsePermission and don't reset state between cases; add a beforeEach that clears/reset mocks to ensure isolation. Update the test suite for useWorkflowPermission by adding a beforeEach block that calls jest.clearAllMocks() (or mockUsePermission.mockReset()/mockClear()) before each it, so mockUsePermission and any other jest mocks are cleared prior to calling renderHook/useWorkflowPermission in each test.plugins/openchoreo-react/src/hooks/useClusterTraitCreatePermission.test.ts (1)
9-42: AddbeforeEachto clear mocks between tests.Suggested fix
describe('useClusterTraitCreatePermission', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + it('returns canCreate=true when allowed', () => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/openchoreo-react/src/hooks/useClusterTraitCreatePermission.test.ts` around lines 9 - 42, Add a beforeEach that clears/reset mocks so each test starts clean: add a beforeEach block in the useClusterTraitCreatePermission test suite that calls jest.clearAllMocks() (and optionally mockUsePermission.mockReset() if you want to fully reset that spy) to ensure mockUsePermission and any other jest mocks are cleared between tests before calling renderHook or asserting calls.plugins/openchoreo-react/src/hooks/useDeploymentPipelinePermission.test.ts (1)
9-42: AddbeforeEachto clear mocks between tests.Suggested fix
describe('useDeploymentPipelinePermission', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + it('returns canCreate=true when allowed', () => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/openchoreo-react/src/hooks/useDeploymentPipelinePermission.test.ts` around lines 9 - 42, Tests share mocked state causing flaky results; add a beforeEach that resets/clears the mockUsePermission mock before each test. Specifically, in the describe('useDeploymentPipelinePermission', ...) block add a beforeEach(() => { mockUsePermission.mockReset() } or mockClear()) so mockUsePermission is reset before each it and all invocations and return values are isolated for useDeploymentPipelinePermission tests.plugins/openchoreo-react/src/hooks/useClusterWorkflowPermission.test.ts (1)
9-42: AddbeforeEachto clear mocks between tests.Suggested fix
describe('useClusterWorkflowPermission', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + it('returns canCreate=true when allowed', () => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/openchoreo-react/src/hooks/useClusterWorkflowPermission.test.ts` around lines 9 - 42, Tests share mocked state across cases causing cross-test interference; add a beforeEach that clears/reset mocks before each test (e.g., call jest.clearAllMocks() or mockUsePermission.mockClear()/mockReset()) in the test suite for useClusterWorkflowPermission so mockUsePermission is reset between tests and each it block starts with a clean mock state.plugins/openchoreo-react/src/hooks/useWorkloadUpdatePermission.test.ts (1)
23-55: AddbeforeEachto clear mocks between tests.The
mockUsePermissionfunction retains its mock state across tests, which can lead to test pollution if execution order changes or if a test fails mid-way. Other similar test files in this PR (e.g.,useConfigureAndDeployPermission.test.ts) includebeforeEach(() => { jest.clearAllMocks(); }).Suggested fix
describe('useWorkloadUpdatePermission', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + it('returns canUpdateWorkload=true when allowed', () => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/openchoreo-react/src/hooks/useWorkloadUpdatePermission.test.ts` around lines 23 - 55, Add a beforeEach that clears Jest mocks so mockUsePermission's state is reset between tests: inside the top-level describe('useWorkloadUpdatePermission') block add beforeEach(() => { jest.clearAllMocks(); }) so every test (including those calling mockUsePermission and renderHook(() => useWorkloadUpdatePermission())) starts with a clean mock state and avoids cross-test pollution.plugins/openchoreo-react/src/hooks/useClusterComponentTypePermission.test.ts (1)
9-42: AddbeforeEachto clear mocks between tests.Suggested fix
describe('useClusterComponentTypePermission', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + it('returns canCreate=true when allowed', () => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/openchoreo-react/src/hooks/useClusterComponentTypePermission.test.ts` around lines 9 - 42, Add a beforeEach to reset mocks so tests are isolated: in the test suite for useClusterComponentTypePermission call jest.clearAllMocks() (or mockUsePermission.mockReset()/mockRestore() as appropriate) in a beforeEach block placed inside the describe; ensure the beforeEach runs before each it, referencing mockUsePermission and the renderHook(() => useClusterComponentTypePermission()) tests so previous mockReturnValue calls are cleared between tests.plugins/openchoreo-react/src/hooks/useProjects.test.ts (1)
79-85: Strengthen the API-error test to prove the failure path executed.At Line 82, the
[]assertion can pass from initial state even if fetch never ran. Assert the fetch call happened before validating the fallback result.Proposed test hardening
it('returns empty array on API error', async () => { mockGetEntities.mockRejectedValue(new Error('API fail')); const { result } = renderHook(() => useProjects()); - await waitFor(() => { - expect(result.current).toEqual([]); - }); + await waitFor(() => expect(mockGetEntities).toHaveBeenCalledTimes(1)); + expect(result.current).toEqual([]); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/openchoreo-react/src/hooks/useProjects.test.ts` around lines 79 - 85, The test for useProjects' API error path currently only asserts the fallback [] which could be the initial state; update the test to first assert that mockGetEntities (the mocked fetch used by useProjects) was called/rejected before asserting result.current equals [] by waiting for or expecting mockGetEntities to have been invoked (e.g., using expect(mockGetEntities).toHaveBeenCalled() or waiting on the mock call) and then check result.current, ensuring the failure path in useProjects ran; reference useProjects, mockGetEntities, renderHook and waitFor when making the change.plugins/catalog-backend-module-openchoreo/src/processors/processors.test.ts (1)
98-112: Harden relation tests by asserting exact emission count.Using only
toHaveBeenCalledWithcan still pass when extra unintended relations are emitted. AddtoHaveBeenCalledTimes(...)where the expected relation set is fixed.♻️ Suggested hardening pattern
expect(emit).toHaveBeenCalledWith( processingResult.relation({ source: { kind: 'environment', namespace: 'my-ns', name: 'dev' }, target: { kind: 'domain', namespace: 'my-ns', name: 'my-ns' }, type: RELATION_PART_OF, }), ); expect(emit).toHaveBeenCalledWith( processingResult.relation({ source: { kind: 'domain', namespace: 'my-ns', name: 'my-ns' }, target: { kind: 'environment', namespace: 'my-ns', name: 'dev' }, type: RELATION_HAS_PART, }), ); + expect(emit).toHaveBeenCalledTimes(2);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/catalog-backend-module-openchoreo/src/processors/processors.test.ts` around lines 98 - 112, The test currently only checks that specific relations were emitted using emit and processingResult.relation (RELATION_PART_OF and RELATION_HAS_PART) but doesn't assert the total number of emissions, allowing extra unintended calls to pass; add an assertion using expect(emit).toHaveBeenCalledTimes(2) (or the exact expected count for this scenario) before/alongside the toHaveBeenCalledWith checks to ensure only the two relations (processingResult.relation with RELATION_PART_OF and processingResult.relation with RELATION_HAS_PART) were emitted.plugins/openchoreo-react/src/hooks/useClusterRoleMappingPermissions.test.ts (1)
14-51: Include a mixed-result case to validate permission-to-output wiring.The current cases use identical mocked responses for all
usePermissioncalls. Add one scenario with different per-call values so incorrect mapping between cluster role-mapping permissions andcan*fields is detectable.Suggested test addition
+ it('maps mixed permission responses to the correct flags', () => { + mockUsePermission + .mockReturnValueOnce({ allowed: true, loading: false }) + .mockReturnValueOnce({ allowed: false, loading: false }) + .mockReturnValueOnce({ allowed: true, loading: false }) + .mockReturnValueOnce({ allowed: false, loading: true }); + + const { result } = renderHook(() => useClusterRoleMappingPermissions()); + + expect(result.current.canView).toBe(true); + expect(result.current.canCreate).toBe(false); + expect(result.current.canUpdate).toBe(true); + expect(result.current.canDelete).toBe(false); + expect(result.current.loading).toBe(true); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/openchoreo-react/src/hooks/useClusterRoleMappingPermissions.test.ts` around lines 14 - 51, Add a new test in useClusterRoleMappingPermissions.test.ts that verifies per-call mapping by making mockUsePermission return different results for each permission call (use mockImplementationOnce or similar to sequence responses) and then renderHook(() => useClusterRoleMappingPermissions()) to assert that canView/canCreate/canUpdate/canDelete and their corresponding deniedTooltip fields reflect those distinct mocked values; reference the mockUsePermission mock and the useClusterRoleMappingPermissions hook to locate where to add this mixed-result scenario.plugins/openchoreo-react/src/hooks/useRolePermissions.test.ts (1)
14-51: Strengthen coverage with a non-uniform permission response test.Using the same mocked response for all calls can miss incorrect permission wiring inside
useRolePermissions. Add one mixed-response test that validates eachcan*output independently.Suggested test addition
+ it('maps mixed permission responses to the correct flags', () => { + mockUsePermission + .mockReturnValueOnce({ allowed: true, loading: false }) + .mockReturnValueOnce({ allowed: false, loading: false }) + .mockReturnValueOnce({ allowed: true, loading: false }) + .mockReturnValueOnce({ allowed: false, loading: true }); + + const { result } = renderHook(() => useRolePermissions()); + + expect(result.current.canView).toBe(true); + expect(result.current.canCreate).toBe(false); + expect(result.current.canUpdate).toBe(true); + expect(result.current.canDelete).toBe(false); + expect(result.current.loading).toBe(true); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/openchoreo-react/src/hooks/useRolePermissions.test.ts` around lines 14 - 51, Add a new test that ensures useRolePermissions correctly maps distinct permission responses by using mockUsePermission.mockReturnValueOnce to return different {allowed, loading} combinations for each sequential permission check, then renderHook(() => useRolePermissions()) and assert each output property (canView, canCreate, canUpdate, canDelete) and corresponding denied tooltip (viewDeniedTooltip, createDeniedTooltip, updateDeniedTooltip, deleteDeniedTooltip) matches the mocked responses; reference the existing test helpers mockUsePermission and the hook useRolePermissions to locate where to add the test and use the can* and *DeniedTooltip properties for assertions.plugins/openchoreo-react/src/hooks/useRoleMappingPermissions.test.ts (1)
14-51: Add one mixed-permission scenario to verify per-flag mapping.All three behavior tests currently return the same mocked result for every
usePermissioncall, so a wrong internal mapping between permission calls andcan*outputs can still pass. Add one non-uniform mocked sequence.Suggested test addition
+ it('maps mixed permission responses to the correct flags', () => { + mockUsePermission + .mockReturnValueOnce({ allowed: true, loading: false }) // view + .mockReturnValueOnce({ allowed: false, loading: false }) // create + .mockReturnValueOnce({ allowed: true, loading: false }) // update + .mockReturnValueOnce({ allowed: false, loading: true }); // delete + + const { result } = renderHook(() => useRoleMappingPermissions()); + + expect(result.current.canView).toBe(true); + expect(result.current.canCreate).toBe(false); + expect(result.current.canUpdate).toBe(true); + expect(result.current.canDelete).toBe(false); + expect(result.current.loading).toBe(true); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/openchoreo-react/src/hooks/useRoleMappingPermissions.test.ts` around lines 14 - 51, Add a test case in useRoleMappingPermissions.test that mocks mockUsePermission to return a different allowed/loading pair for each sequential call (e.g., first call allowed:true, second false, third true, fourth false) and then renderHook(() => useRoleMappingPermissions()) to assert that the resulting flags (result.current.canView, canCreate, canUpdate, canDelete) map one-to-one to those mocked calls and that the corresponding denied tooltip fields (viewDeniedTooltip, createDeniedTooltip, updateDeniedTooltip, deleteDeniedTooltip) reflect the per-flag allowed state; this ensures the internal mapping in useRoleMappingPermissions correctly ties each usePermission invocation to its respective can* output.plugins/openchoreo-react/src/hooks/useClusterRolePermissions.test.ts (1)
14-51: Add one mixed-permission test to improve mapping confidence.All current state tests return identical mock values for each permission call. A mixed-response case would verify that each cluster-role permission maps to the correct
can*/tooltip output.Suggested test addition
+ it('maps mixed permission responses to the correct flags', () => { + mockUsePermission + .mockReturnValueOnce({ allowed: true, loading: false }) + .mockReturnValueOnce({ allowed: false, loading: false }) + .mockReturnValueOnce({ allowed: true, loading: false }) + .mockReturnValueOnce({ allowed: false, loading: true }); + + const { result } = renderHook(() => useClusterRolePermissions()); + + expect(result.current.canView).toBe(true); + expect(result.current.canCreate).toBe(false); + expect(result.current.canUpdate).toBe(true); + expect(result.current.canDelete).toBe(false); + expect(result.current.loading).toBe(true); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/openchoreo-react/src/hooks/useClusterRolePermissions.test.ts` around lines 14 - 51, Add a new test that verifies mixed permission mapping by configuring mockUsePermission to return different values for each sequential call (use mockUsePermission.mockImplementationOnce) to simulate e.g. view: allowed true, create: allowed false loading false, update: allowed true loading false, delete: allowed false loading true; render the hook via renderHook(() => useClusterRolePermissions()) and assert each resulting flag (canView/canCreate/canUpdate/canDelete) and tooltip/loading outputs (viewDeniedTooltip, createDeniedTooltip, updateDeniedTooltip, deleteDeniedTooltip, loading) match the corresponding mocked responses; reference the hook useClusterRolePermissions and the mock function mockUsePermission when adding the test.plugins/openchoreo-react/src/hooks/useNamespacePermission.test.ts (1)
14-41: Add a mixed view/create case to catch mapping regressions.Current behavior tests mock identical outcomes for both
usePermissioncalls. Add one non-uniform scenario so swapped/miswiredcanViewandcanCreatelogic is caught.Suggested test addition
+ it('maps mixed permission responses to the correct fields', () => { + mockUsePermission + .mockReturnValueOnce({ allowed: true, loading: false }) // view + .mockReturnValueOnce({ allowed: false, loading: false }); // create + + const { result } = renderHook(() => useNamespacePermission()); + + expect(result.current.canView).toBe(true); + expect(result.current.canCreate).toBe(false); + expect(result.current.viewDeniedTooltip).toBe(''); + expect(result.current.createDeniedTooltip).toBeTruthy(); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/openchoreo-react/src/hooks/useNamespacePermission.test.ts` around lines 14 - 41, Add a test to verify mixed permission results to catch swapped logic: in the tests for useNamespacePermission, add a case where mockUsePermission is configured with mockReturnValueOnce twice (one call returns {allowed: true, loading: false} and the other returns {allowed: false, loading: false}) so the first call (view) is allowed and the second (create) is denied, then renderHook(() => useNamespacePermission()) and assert result.current.canView is true, result.current.canCreate is false, result.current.viewDeniedTooltip is '' and result.current.createDeniedTooltip is truthy; use mockReturnValueOnce on mockUsePermission to control ordering and ensure the mapping between usePermission calls and canView/canCreate is correct.plugins/openchoreo-observability/src/api/ObserverUrlCache.test.ts (1)
112-119: Add an explicit invariant test forrcaAgentUrlwithoutobserverUrl.You already test missing
observerUrlwith{}. Add one explicit case with onlyrcaAgentUrlto lock the contract and prevent regressions in this specific path.🧪 Suggested additional test
it('throws when observerUrl is missing from response', async () => { mockFetchApi.fetch.mockResolvedValue(mockOkResponse({})); const cache = createCache(); await expect(cache.resolveUrls('ns1', 'dev')).rejects.toThrow( 'Observability is not enabled for this component', ); }); + + it('throws when rcaAgentUrl is present without observerUrl', async () => { + mockFetchApi.fetch.mockResolvedValue( + mockOkResponse({ rcaAgentUrl: 'http://rca' }), + ); + const cache = createCache(); + + await expect(cache.resolveUrls('ns1', 'dev')).rejects.toThrow( + 'Observability is not enabled for this component', + ); + });Based on learnings: in
plugins/openchoreo-observability/src/api/ObserverUrlCache.ts, URL resolution should requireobserverUrlwheneverrcaAgentUrlis present.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/openchoreo-observability/src/api/ObserverUrlCache.test.ts` around lines 112 - 119, Add a new unit test case in ObserverUrlCache.test.ts that specifically asserts the contract where rcaAgentUrl without observerUrl is treated as missing observerUrl: after setting mockFetchApi.fetch.mockResolvedValue(mockOkResponse({ rcaAgentUrl: 'https://rca.example' })), call createCache() and assert that cache.resolveUrls('ns1', 'dev') rejects with the same 'Observability is not enabled for this component' error; this locks the behavior in the ObserverUrlCache.resolveUrls path (and related resolution logic) to require observerUrl whenever rcaAgentUrl is present.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: dad361ac-d53f-4a47-b4f1-cad9adcdab51
📒 Files selected for processing (50)
packages/app/src/apis/OpenChoreoFetchApi.test.tspackages/app/src/apis/OpenChoreoPermissionApi.test.tspackages/design-system/src/components/RjsfTemplates/utils.test.tsplugins/catalog-backend-module-openchoreo/src/processors/processors.test.tsplugins/openchoreo-observability/src/api/ObserverUrlCache.test.tsplugins/openchoreo-react/src/hooks/useAlertsPermission.test.tsplugins/openchoreo-react/src/hooks/useClusterComponentTypePermission.test.tsplugins/openchoreo-react/src/hooks/useClusterRoleMappingPermissions.test.tsplugins/openchoreo-react/src/hooks/useClusterRolePermissions.test.tsplugins/openchoreo-react/src/hooks/useClusterTraitCreatePermission.test.tsplugins/openchoreo-react/src/hooks/useClusterWorkflowPermission.test.tsplugins/openchoreo-react/src/hooks/useComponentCreatePermission.test.tsplugins/openchoreo-react/src/hooks/useComponentTypePermission.test.tsplugins/openchoreo-react/src/hooks/useComponentUpdatePermission.test.tsplugins/openchoreo-react/src/hooks/useComponentWorkflowPermission.test.tsplugins/openchoreo-react/src/hooks/useConfigureAndDeployPermission.test.tsplugins/openchoreo-react/src/hooks/useDeploymentPipelinePermission.test.tsplugins/openchoreo-react/src/hooks/useEntityAnnotation.test.tsplugins/openchoreo-react/src/hooks/useEnvironmentReadPermission.test.tsplugins/openchoreo-react/src/hooks/useIncidentsPermission.test.tsplugins/openchoreo-react/src/hooks/useLogsPermission.test.tsplugins/openchoreo-react/src/hooks/useMetricsPermission.test.tsplugins/openchoreo-react/src/hooks/useModeState.test.tsplugins/openchoreo-react/src/hooks/useNamespacePermission.test.tsplugins/openchoreo-react/src/hooks/useOpenChoreoFeatures.test.tsplugins/openchoreo-react/src/hooks/useProjectPermission.test.tsplugins/openchoreo-react/src/hooks/useProjectUpdatePermission.test.tsplugins/openchoreo-react/src/hooks/useProjects.test.tsplugins/openchoreo-react/src/hooks/useRcaPermission.test.tsplugins/openchoreo-react/src/hooks/useRcaUpdatePermission.test.tsplugins/openchoreo-react/src/hooks/useReleaseBindingPermission.test.tsplugins/openchoreo-react/src/hooks/useResourceDefinitionPermission.test.tsplugins/openchoreo-react/src/hooks/useRoleMappingPermissions.test.tsplugins/openchoreo-react/src/hooks/useRolePermissions.test.tsplugins/openchoreo-react/src/hooks/useScopedComponentCreatePermission.test.tsplugins/openchoreo-react/src/hooks/useScopedProjectCreatePermission.test.tsplugins/openchoreo-react/src/hooks/useTracesPermission.test.tsplugins/openchoreo-react/src/hooks/useTraitCreatePermission.test.tsplugins/openchoreo-react/src/hooks/useTraitsPermission.test.tsplugins/openchoreo-react/src/hooks/useUndeployPermission.test.tsplugins/openchoreo-react/src/hooks/useWorkflowPermission.test.tsplugins/openchoreo-react/src/hooks/useWorkloadUpdatePermission.test.tsplugins/openchoreo/src/utils/componentUtils.test.tsplugins/permission-backend-module-openchoreo-policy/src/rules/matchesCapability.test.tsplugins/permission-backend-module-openchoreo-policy/src/rules/matchesCatalogEntityCapability.test.tsplugins/permission-backend-module-openchoreo-policy/src/services/AuthzProfileCache.test.tsplugins/permission-backend-module-openchoreo-policy/src/services/AuthzProfileService.test.tsplugins/scaffolder-backend-module-openchoreo/src/actions/environment.test.tsplugins/scaffolder-backend-module-openchoreo/src/actions/namespace.test.tsplugins/scaffolder-backend-module-openchoreo/src/actions/project.test.ts
plugins/catalog-backend-module-openchoreo/src/processors/processors.test.ts
Show resolved
Hide resolved
plugins/catalog-backend-module-openchoreo/src/processors/processors.test.ts
Show resolved
Hide resolved
Comment on lines
+61
to
+82
| it('re-fetches after TTL expires', async () => { | ||
| const now = Date.now(); | ||
| jest.spyOn(Date, 'now').mockReturnValue(now); | ||
|
|
||
| mockFetchApi.fetch.mockResolvedValue( | ||
| mockOkResponse({ observerUrl: 'http://observer-v1' }), | ||
| ); | ||
| const cache = createCache(); | ||
| await cache.resolveUrls('ns1', 'dev'); | ||
|
|
||
| // Advance past 5-minute TTL | ||
| (Date.now as jest.Mock).mockReturnValue(now + 6 * 60 * 1000); | ||
| mockFetchApi.fetch.mockResolvedValue( | ||
| mockOkResponse({ observerUrl: 'http://observer-v2' }), | ||
| ); | ||
| const result = await cache.resolveUrls('ns1', 'dev'); | ||
|
|
||
| expect(result.observerUrl).toBe('http://observer-v2'); | ||
| expect(mockFetchApi.fetch).toHaveBeenCalledTimes(2); | ||
|
|
||
| jest.restoreAllMocks(); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify Date mocking cleanup strategy in this test file.
rg -n "spyOn\\(Date, 'now'\\)|restoreAllMocks\\(|afterEach\\(" plugins/openchoreo-observability/src/api/ObserverUrlCache.test.tsRepository: openchoreo/backstage-plugins
Length of output: 155
🏁 Script executed:
cat -n plugins/openchoreo-observability/src/api/ObserverUrlCache.test.tsRepository: openchoreo/backstage-plugins
Length of output: 5115
Move jest.restoreAllMocks() to an afterEach hook to prevent Date.now spy leakage on early test failure.
If the test fails before line 81, the spy cleanup won't execute, polluting subsequent tests. This should run unconditionally after every test in this describe block.
💡 Suggested fix
beforeEach(() => {
jest.clearAllMocks();
mockDiscoveryApi.getBaseUrl.mockResolvedValue('http://localhost/api/obs');
});
+
+afterEach(() => {
+ jest.restoreAllMocks();
+});
@@
it('re-fetches after TTL expires', async () => {
@@
- jest.restoreAllMocks();
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/openchoreo-observability/src/api/ObserverUrlCache.test.ts` around
lines 61 - 82, The Date.now spy is restored only at the end of this test which
can leak if the test fails early; move the cleanup into a shared afterEach hook
so it always runs: remove the inline jest.restoreAllMocks() from the 're-fetches
after TTL expires' test and add an afterEach(() => jest.restoreAllMocks()) in
the same describe/block that defines mockFetchApi/createCache/resolveUrls so
Date.now spy and other mocks are unconditionally restored after every test.
Comment on lines
+4
to
+7
| const mockUsePermission = jest.fn(); | ||
| jest.mock('@backstage/plugin-permission-react', () => ({ | ||
| usePermission: (...args: any[]) => mockUsePermission(...args), | ||
| })); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether Jest globally clears/resets mocks (which affects this test's determinism).
echo "== Jest config files =="
fd -i 'jest.config.*'
echo
echo "== clearMocks/resetMocks/restoreMocks in repo =="
rg -n --iglob '*jest*' --iglob 'package.json' '"clearMocks"|"resetMocks"|"restoreMocks"|clearMocks|resetMocks|restoreMocks'
echo
echo "== Local test lifecycle hooks in this file =="
rg -n -C2 'beforeEach|afterEach|mockReset|mockClear' plugins/openchoreo-react/src/hooks/useReleaseBindingPermission.test.tsRepository: openchoreo/backstage-plugins
Length of output: 192
🏁 Script executed:
cat -n plugins/openchoreo-react/src/hooks/useReleaseBindingPermission.test.tsRepository: openchoreo/backstage-plugins
Length of output: 1873
🏁 Script executed:
cat -n plugins/openchoreo-react/src/hooks/useReleaseBindingPermission.ts && echo -e "\n---\n" && rg -A5 '"jest"' plugins/openchoreo-react/package.jsonRepository: openchoreo/backstage-plugins
Length of output: 1133
🏁 Script executed:
cat plugins/openchoreo-react/package.json | grep -A20 '"jest"'Repository: openchoreo/backstage-plugins
Length of output: 54
🏁 Script executed:
find . -maxdepth 3 -name 'jest.config.*' -o -name 'package.json' -path '*/root*' | head -5 && echo "---" && cat package.json | jq '.jest // empty' 2>/dev/null || cat package.json | grep -A20 jestRepository: openchoreo/backstage-plugins
Length of output: 75
🏁 Script executed:
head -100 package.json | grep -A15 '"jest"'Repository: openchoreo/backstage-plugins
Length of output: 54
🏁 Script executed:
cat package.json | head -200Repository: openchoreo/backstage-plugins
Length of output: 1892
🏁 Script executed:
cat plugins/openchoreo-react/package.json | grep -A20 '"jest"' || echo "No jest config found in openchoreo-react package.json"Repository: openchoreo/backstage-plugins
Length of output: 125
Stabilize mock isolation for the resourceRef assertion.
The shared mockUsePermission at module level (line 4) accumulates call history across tests without per-test reset. Line 36's toHaveBeenCalledWith assertion can match against prior calls, reducing test clarity and resilience to future changes. Add per-test reset and assert single invocation.
Suggested fix
const mockUsePermission = jest.fn();
jest.mock('@backstage/plugin-permission-react', () => ({
usePermission: (...args: any[]) => mockUsePermission(...args),
}));
describe('useReleaseBindingPermission', () => {
+ beforeEach(() => {
+ mockUsePermission.mockReset();
+ });
+
it('returns canViewBindings=true when allowed', () => {
mockUsePermission.mockReturnValue({ allowed: true, loading: false });
const { result } = renderHook(() => useReleaseBindingPermission());
@@
it('does not pass resourceRef to usePermission', () => {
mockUsePermission.mockReturnValue({ allowed: true, loading: false });
renderHook(() => useReleaseBindingPermission());
+ expect(mockUsePermission).toHaveBeenCalledTimes(1);
expect(mockUsePermission).toHaveBeenCalledWith(
expect.not.objectContaining({ resourceRef: expect.anything() }),
);
});
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/openchoreo-react/src/hooks/useReleaseBindingPermission.test.ts`
around lines 4 - 7, The test uses a module-level mockUsePermission that retains
call history across tests; add a per-test reset by calling
mockUsePermission.mockReset() (or mockClear()) in a beforeEach to isolate
invocations, and strengthen the assertion in the test that checks resourceRef by
adding expect(mockUsePermission).toHaveBeenCalledTimes(1) before
expect(mockUsePermission).toHaveBeenCalledWith(...) so you validate a single,
fresh call to usePermission (mockUsePermission) and ensure the resourceRef
argument is asserted against that single invocation.
Signed-off-by: Kavith Lokuhewage <kaviththiranga@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@plugins/openchoreo-react/src/hooks/useClusterTraitCreatePermission.test.ts`:
- Around line 43-45: The current assertion uses expect.not.objectContaining({
resourceRef: expect.anything() }) which can pass when resourceRef exists but is
undefined; instead retrieve the first call argument to mockUsePermission (e.g.,
mockUsePermission.mock.calls[0][0]) and assert that it does not have the
property resourceRef using expect(firstArg).not.toHaveProperty('resourceRef');
replace the toHaveBeenCalledWith(...) line with this explicit property absence
check on the first call argument to ensure the property is truly absent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c34185d8-0b88-4591-8310-710855756ad6
📒 Files selected for processing (29)
plugins/catalog-backend-module-openchoreo/src/processors/processors.test.tsplugins/openchoreo-react/src/hooks/useAlertsPermission.test.tsplugins/openchoreo-react/src/hooks/useClusterComponentTypePermission.test.tsplugins/openchoreo-react/src/hooks/useClusterTraitCreatePermission.test.tsplugins/openchoreo-react/src/hooks/useClusterWorkflowPermission.test.tsplugins/openchoreo-react/src/hooks/useComponentCreatePermission.test.tsplugins/openchoreo-react/src/hooks/useComponentTypePermission.test.tsplugins/openchoreo-react/src/hooks/useComponentUpdatePermission.test.tsplugins/openchoreo-react/src/hooks/useComponentWorkflowPermission.test.tsplugins/openchoreo-react/src/hooks/useDeployPermission.test.tsplugins/openchoreo-react/src/hooks/useDeploymentPipelinePermission.test.tsplugins/openchoreo-react/src/hooks/useEnvironmentPermission.test.tsplugins/openchoreo-react/src/hooks/useEnvironmentReadPermission.test.tsplugins/openchoreo-react/src/hooks/useIncidentsPermission.test.tsplugins/openchoreo-react/src/hooks/useLogsPermission.test.tsplugins/openchoreo-react/src/hooks/useMetricsPermission.test.tsplugins/openchoreo-react/src/hooks/useProjectPermission.test.tsplugins/openchoreo-react/src/hooks/useProjectUpdatePermission.test.tsplugins/openchoreo-react/src/hooks/useRcaPermission.test.tsplugins/openchoreo-react/src/hooks/useRcaUpdatePermission.test.tsplugins/openchoreo-react/src/hooks/useReleaseBindingPermission.test.tsplugins/openchoreo-react/src/hooks/useScopedComponentCreatePermission.test.tsplugins/openchoreo-react/src/hooks/useScopedProjectCreatePermission.test.tsplugins/openchoreo-react/src/hooks/useTracesPermission.test.tsplugins/openchoreo-react/src/hooks/useTraitCreatePermission.test.tsplugins/openchoreo-react/src/hooks/useTraitsPermission.test.tsplugins/openchoreo-react/src/hooks/useUndeployPermission.test.tsplugins/openchoreo-react/src/hooks/useWorkflowPermission.test.tsplugins/openchoreo-react/src/hooks/useWorkloadUpdatePermission.test.ts
✅ Files skipped from review due to trivial changes (13)
- plugins/openchoreo-react/src/hooks/useEnvironmentPermission.test.ts
- plugins/openchoreo-react/src/hooks/useWorkloadUpdatePermission.test.ts
- plugins/openchoreo-react/src/hooks/useComponentWorkflowPermission.test.ts
- plugins/openchoreo-react/src/hooks/useDeploymentPipelinePermission.test.ts
- plugins/openchoreo-react/src/hooks/useRcaPermission.test.ts
- plugins/openchoreo-react/src/hooks/useAlertsPermission.test.ts
- plugins/openchoreo-react/src/hooks/useRcaUpdatePermission.test.ts
- plugins/openchoreo-react/src/hooks/useComponentTypePermission.test.ts
- plugins/openchoreo-react/src/hooks/useProjectUpdatePermission.test.ts
- plugins/openchoreo-react/src/hooks/useProjectPermission.test.ts
- plugins/openchoreo-react/src/hooks/useScopedComponentCreatePermission.test.ts
- plugins/openchoreo-react/src/hooks/useTracesPermission.test.ts
- plugins/openchoreo-react/src/hooks/useMetricsPermission.test.ts
🚧 Files skipped from review as they are similar to previous changes (14)
- plugins/openchoreo-react/src/hooks/useComponentUpdatePermission.test.ts
- plugins/openchoreo-react/src/hooks/useTraitsPermission.test.ts
- plugins/openchoreo-react/src/hooks/useClusterWorkflowPermission.test.ts
- plugins/openchoreo-react/src/hooks/useClusterComponentTypePermission.test.ts
- plugins/openchoreo-react/src/hooks/useUndeployPermission.test.ts
- plugins/openchoreo-react/src/hooks/useTraitCreatePermission.test.ts
- plugins/openchoreo-react/src/hooks/useScopedProjectCreatePermission.test.ts
- plugins/openchoreo-react/src/hooks/useLogsPermission.test.ts
- plugins/openchoreo-react/src/hooks/useWorkflowPermission.test.ts
- plugins/openchoreo-react/src/hooks/useEnvironmentReadPermission.test.ts
- plugins/openchoreo-react/src/hooks/useReleaseBindingPermission.test.ts
- plugins/openchoreo-react/src/hooks/useComponentCreatePermission.test.ts
- plugins/openchoreo-react/src/hooks/useIncidentsPermission.test.ts
- plugins/catalog-backend-module-openchoreo/src/processors/processors.test.ts
Comment on lines
+43
to
+45
| expect(mockUsePermission).toHaveBeenCalledWith( | ||
| expect.not.objectContaining({ resourceRef: expect.anything() }), | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find similar matcher usage that can miss `resourceRef: undefined`.
rg -n --type=ts -C2 "not\\.objectContaining\\(\\{\\s*resourceRef:\\s*expect\\.anything\\(\\)\\s*\\}\\)"Repository: openchoreo/backstage-plugins
Length of output: 6698
🏁 Script executed:
cat -n plugins/openchoreo-react/src/hooks/useClusterTraitCreatePermission.tsRepository: openchoreo/backstage-plugins
Length of output: 1874
Strengthen the resourceRef assertion to avoid false negatives.
The test uses expect.not.objectContaining({ resourceRef: expect.anything() }), which passes if resourceRef is absent OR if it's undefined. Use expect(firstArg).not.toHaveProperty('resourceRef') instead to ensure the property is definitively not present.
Suggested patch
import { renderHook } from '@testing-library/react';
+import { openchoreoClusterTraitCreatePermission } from '@openchoreo/backstage-plugin-common';
import { useClusterTraitCreatePermission } from './useClusterTraitCreatePermission';
@@
it('does not pass resourceRef to usePermission', () => {
mockUsePermission.mockReturnValue({ allowed: true, loading: false });
renderHook(() => useClusterTraitCreatePermission());
- expect(mockUsePermission).toHaveBeenCalledWith(
- expect.not.objectContaining({ resourceRef: expect.anything() }),
- );
+ expect(mockUsePermission).toHaveBeenCalledTimes(1);
+ const firstArg = mockUsePermission.mock.calls[0][0];
+ expect(firstArg).toEqual(
+ expect.objectContaining({
+ permission: openchoreoClusterTraitCreatePermission,
+ }),
+ );
+ expect(firstArg).not.toHaveProperty('resourceRef');
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/openchoreo-react/src/hooks/useClusterTraitCreatePermission.test.ts`
around lines 43 - 45, The current assertion uses expect.not.objectContaining({
resourceRef: expect.anything() }) which can pass when resourceRef exists but is
undefined; instead retrieve the first call argument to mockUsePermission (e.g.,
mockUsePermission.mock.calls[0][0]) and assert that it does not have the
property resourceRef using expect(firstArg).not.toHaveProperty('resourceRef');
replace the toHaveBeenCalledWith(...) line with this explicit property absence
check on the first call argument to ensure the property is truly absent.
LakshanSS
reviewed
Apr 7, 2026
| @@ -0,0 +1,47 @@ | |||
| import { renderHook } from '@testing-library/react'; | |||
Contributor
There was a problem hiding this comment.
Nit: These ~10 simple permission hook tests (useClusterComponentTypePermission, useClusterTraitCreatePermission, useClusterWorkflowPermission, useComponentCreatePermission, useComponentTypePermission, useComponentWorkflowPermission, useDeploymentPipelinePermission, useProjectPermission, useTraitCreatePermission, useWorkflowPermission) are nearly identical. Only the hook name and import differ.
stefinie123
approved these changes
Apr 7, 2026
LakshanSS
approved these changes
Apr 7, 2026
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
3 participants
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.
Add 37 new test files covering:
Summary by CodeRabbit