feat(system-e2e): Add test for admin delegations #17536
Conversation
WalkthroughThis pull request introduces an end-to-end test for the admin portal's delegation functionality and adds a data test ID to a button in the delegation admin interface. The changes focus on improving testability by adding a specific test identifier to the "Add" button and creating a Playwright test to verify the button's presence and visibility in the admin delegations screen. Changes
Sequence DiagramsequenceDiagram
participant Test as E2E Test
participant Browser as Browser Context
participant AdminPortal as Admin Portal Page
Test->>Browser: Initialize session
Browser->>AdminPortal: Navigate to delegations page
Test->>AdminPortal: Verify "Add" button presence
AdminPortal-->>Test: Confirm button visibility
Possibly related PRs
Suggested labels
Suggested reviewers
Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
apps/system-e2e/src/tests/islandis/admin-portal/smoke/admin-delegations.spec.ts (2)
24-28: Enhance test coverage and description.The test case could be more comprehensive and descriptive:
- The test name should better describe the scenario being tested
- Additional assertions should verify the button's functionality
- test('should see "Add" button', async () => { + test('should display and enable "Add" button for admin with write access', async () => { const page = await context.newPage() await page.goto('/stjornbord/delegation-admin') - await expect(page.getByText('Skrá nýtt umboð')).toBeVisible() + const addButton = page.getByTestId('create-delegation-button') + await expect(addButton).toBeVisible() + await expect(addButton).toBeEnabled() + await expect(addButton).toHaveText('Skrá nýtt umboð') })
11-22: Consider adding negative test cases.The test suite only covers the happy path. Consider adding test cases for:
- Admin without write access
- Unauthenticated users
- Session timeout scenarios
libs/portals/admin/delegation-admin/src/screens/Root.tsx (1)
65-65: LGTM! Consider using a constant for the test ID.The addition of the test ID is good. To improve maintainability, consider extracting test IDs into a constants file that can be shared between the component and tests.
// testIds.ts export const DELEGATION_TEST_IDS = { CREATE_BUTTON: 'create-delegation-button', } as const;Then in the component:
- dataTestId="create-delegation-button" + dataTestId={DELEGATION_TEST_IDS.CREATE_BUTTON}
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/system-e2e/src/tests/islandis/admin-portal/smoke/admin-delegations.spec.ts(1 hunks)libs/portals/admin/delegation-admin/src/screens/Root.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
apps/system-e2e/src/tests/islandis/admin-portal/smoke/admin-delegations.spec.ts (1)
Pattern apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
libs/portals/admin/delegation-admin/src/screens/Root.tsx (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
📓 Learnings (1)
apps/system-e2e/src/tests/islandis/admin-portal/smoke/admin-delegations.spec.ts (1)
Learnt from: saevarma
PR: island-is/island.is#16257
File: apps/services/auth/admin-api/src/app/v2/delegations/test/delegation-admin.auth.spec.ts:178-201
Timestamp: 2024-11-12T15:15:20.157Z
Learning: In `apps/services/auth/admin-api/src/app/v2/delegations/test/delegation-admin.auth.spec.ts`, ensure that test case names accurately describe the scenario being tested, particularly in cases involving invalid request signatures.
apps/system-e2e/src/tests/islandis/admin-portal/smoke/admin-delegations.spec.ts
Show resolved
Hide resolved
Datadog ReportAll test runs ✅ 9 Total Test Services: 0 Failed, 9 Passed Test Services
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #17536 +/- ##
=======================================
Coverage 35.61% 35.61%
=======================================
Files 7015 7015
Lines 150339 150339
Branches 42981 42981
=======================================
Hits 53541 53541
Misses 96798 96798 Continue to review full report in Codecov by Sentry.
|
...
Attach a link to issue if relevant
What
Specify what you're trying to achieve
Why
Specify why you need to achieve this
Screenshots / Gifs
Attach Screenshots / Gifs to help reviewers understand the scope of the pull request
Checklist:
Summary by CodeRabbit
New Features
Tests