-
Notifications
You must be signed in to change notification settings - Fork 988
Valueset Create Playwright Tests #14948
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Valueset Create Playwright Tests #14948
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds two slug-length constants and a new Playwright test suite that exercises ValueSet creation workflows: full-field creation, include/exclude rules, system selection, preview interactions, auto-generated slug behavior, and slug length validation (5–25 characters). Changes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds comprehensive Playwright test coverage for the ValueSet creation functionality, addressing issue #14304. The tests validate form validation, code verification, preview functionality, and list display.
Key Changes:
- Added end-to-end tests for ValueSet creation workflow with validation of all form fields and preview functionality
- Added slug validation tests to ensure length constraints (5-25 characters) are enforced
- Added constants for slug length validation boundaries
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| tests/admin/valueset/valuesetCreate.spec.ts | Implements comprehensive test suite for ValueSet creation including field validation, code verification, preview dialog, and list display verification |
| tests/admin/valueset/valuesetConstants.ts | Adds MIN_SLUG_LENGTH and MAX_SLUG_LENGTH constants to support slug validation tests |
| page.getByRole("textbox", { name: "Slug *" }), | ||
| ); | ||
| await expect(errorMessage).toBeVisible(); | ||
| await expect(errorMessage).toHaveText(/atleast \d+.*atmost 25/i); |
Copilot
AI
Dec 26, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The regex pattern matches 'atleast' and 'atmost', but the correct English phrases are 'at least' (two words) and 'at most' (two words). If this is matching actual error message text, the error message itself may need correction. Update the test to match the corrected error message format.
| await page.getByRole("button", { name: /save/i }).click(); | ||
|
|
||
| await expect(errorMessage).toBeVisible(); | ||
| await expect(errorMessage).toHaveText(/atleast \d+.*atmost 25/i); |
Copilot
AI
Dec 26, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The regex pattern matches 'atleast' and 'atmost', but the correct English phrases are 'at least' (two words) and 'at most' (two words). If this is matching actual error message text, the error message itself may need correction. Update the test to match the corrected error message format.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 8
📜 Review details
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
tests/admin/valueset/valuesetConstants.tstests/admin/valueset/valuesetCreate.spec.ts
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx}: Write concise, technical TypeScript code with accurate examples
Use TypeScript for all code; prefer interfaces over types
Avoid enums; use maps instead
Use TanStack Query for data fetching from the API along with query and mutate utilities for the queryFn and mutationFn
Use raviger for routing
**/*.{ts,tsx}: Use TypeScript with strict mode and ES2022 target
Useinterfacefor defining object types in TypeScript
Avoid explicitanytype in TypeScript
Use proper nullability annotations in TypeScript types
Use dedicated error handlers and TypeScript strict null checks
**/*.{ts,tsx}: Use TypeScript for all new code
Prefer interfaces over types for object definitions in TypeScript
Avoid usinganytype; use proper type definitions in TypeScript
Use type inference where possible in TypeScript
Use TanStack Query for API data management
Prefer React Context for global state management
**/*.{ts,tsx}: Use TanStack Query with thequeryandmutateutilities from@/Utils/request/
Use appropriate query keys following the resource pattern for TanStack Query
Leverage TanStack Query built-in features for pagination and debouncing
Implement proper error handling using the global error handler for TanStack Query operations
UseuseQueryhook withqueryKeyandqueryFnparameters, wherequeryFnwraps the API route with thequery()utility
UseuseMutationhook withmutationFnparameter wrapping API routes with themutate()utility, and implementonSuccesscallbacks to invalidate related queries
Support path parameters in TanStack Query using thepathParamsoption in query/mutate utilities
Support query parameters in TanStack Query using thequeryParamsoption in query/mutate utilities
Use thesilent: trueoption to suppress global error notifications when custom error handling is needed
**/*.{ts,tsx}: Use TypeScript with strict mode and ES2022 target
Useinterfacefor defining object types
Avoid explicitanytypes and maint...
Files:
tests/admin/valueset/valuesetConstants.tstests/admin/valueset/valuesetCreate.spec.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx,js,jsx}: Use functional and declarative programming patterns; avoid classes
Prefer iteration and modularization over code duplication
Use descriptive variable names with auxiliary verbs (e.g., isLoading, hasError)
Use the "function" keyword for pure functions
Avoid unnecessary curly braces in conditionals; use concise syntax for simple statements
**/*.{ts,tsx,js,jsx}: Use path aliases for imports from src (@/*)
Use double quotes for strings
Semicolons are required at end of statements
Order imports as: 3rd-party → library → CAREUI → UI → components → hooks → utils → relative
Use PascalCase for component and class names
Use camelCase for variable and function names
**/*.{ts,tsx,js,jsx}: Use path aliases@/*for imports from src directory
Use double quotes for strings
Require semicolons at end of statements
Order imports: 3rd-party → library → CAREUI → UI → components → hooks → utils → relative
Use PascalCase for component and class names
Use camelCase for variable and function names
Files:
tests/admin/valueset/valuesetConstants.tstests/admin/valueset/valuesetCreate.spec.ts
**/*
📄 CodeRabbit inference engine (CLAUDE.md)
Use 2-space indentation
Files:
tests/admin/valueset/valuesetConstants.tstests/admin/valueset/valuesetCreate.spec.ts
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/02-coding-standards.mdc)
Use camelCase for utility file names (e.g.,
useAuth.ts)
Files:
tests/admin/valueset/valuesetConstants.tstests/admin/valueset/valuesetCreate.spec.ts
**/*.{tsx,ts}
📄 CodeRabbit inference engine (.cursor/rules/04-ui-components.mdc)
**/*.{tsx,ts}: Use Shadcn UI components as the primary component library
Follow the component documentation for proper usage
Customize components using Tailwind CSS
UsenavigateanduseRedirectfrom raviger for navigation and redirects
Use Tailwind's responsive classes and follow mobile-first approach
Implement proper ARIA attributes for accessibility
Ensure keyboard navigation works in components
Define component props using TypeScript interfaces
Use translation keys fromsrc/Locale/for internationalization
Support RTL languages in components
Use proper date/time formatting for multiple language support
Files:
tests/admin/valueset/valuesetConstants.tstests/admin/valueset/valuesetCreate.spec.ts
**/*.{ts,tsx,js,jsx,json,css,scss,html}
📄 CodeRabbit inference engine (AGENTS.md)
Use 2-space indentation
Files:
tests/admin/valueset/valuesetConstants.tstests/admin/valueset/valuesetCreate.spec.ts
**/*.spec.ts
📄 CodeRabbit inference engine (.cursor/rules/02-coding-standards.mdc)
**/*.spec.ts: Write E2E tests in Playwright following https://playwright.dev/docs/writing-tests
Write meaningful test descriptions
Files:
tests/admin/valueset/valuesetCreate.spec.ts
🧠 Learnings (18)
📓 Common learnings
Learnt from: yash-learner
Repo: ohcnetwork/care_fe PR: 14108
File: tests/facility/settings/activityDefinitions/activityDefinitionCreate.spec.ts:86-147
Timestamp: 2025-11-20T09:48:26.486Z
Learning: In tests/facility/settings/activityDefinitions/activityDefinitionCreate.spec.ts, detailed verification of selected values (e.g., code from value sets) in the detail view is intentionally deferred to the list spec, as the creation test focuses on the happy path and basic verification only.
Learnt from: AdityaJ2305
Repo: ohcnetwork/care_fe PR: 13197
File: src/pages/Facility/settings/observationDefinition/ObservationDefinitionForm.tsx:470-495
Timestamp: 2025-09-16T20:46:59.537Z
Learning: In PR #13197, AdityaJ2305 indicated that aria-invalid and ref forwarding improvements for ValueSetSelect components can be deferred to a separate PR, considering them not critical for the current focus/scroll fix implementation.
Learnt from: CR
Repo: ohcnetwork/care_fe PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T13:50:10.786Z
Learning: Playwright E2E tests required for critical healthcare workflows and must be run before deployment
Learnt from: CR
Repo: ohcnetwork/care_fe PR: 0
File: .github/instructions/careui.instructions.md:0-0
Timestamp: 2025-11-25T13:50:46.407Z
Learning: Applies to src/CAREUI/**/*.{test,spec}.{ts,tsx} : Implement comprehensive testing in CAREUI components including accessibility and medical workflow scenarios
📚 Learning: 2025-11-25T13:50:54.683Z
Learnt from: CR
Repo: ohcnetwork/care_fe PR: 0
File: .github/instructions/common.instructions.md:0-0
Timestamp: 2025-11-25T13:50:54.683Z
Learning: Applies to src/common/**/constants.tsx : Define core application configuration including RESULTS_PER_PAGE_LIMIT for API pagination in constants.tsx
Applied to files:
tests/admin/valueset/valuesetConstants.ts
📚 Learning: 2025-09-29T14:07:50.923Z
Learnt from: GSMPRANEETH
Repo: ohcnetwork/care_fe PR: 13750
File: src/components/Questionnaire/MedicationValueSetSelect.tsx:240-254
Timestamp: 2025-09-29T14:07:50.923Z
Learning: The codebase uses src/common/constants.tsx for application-wide constants like RESULTS_PER_PAGE_LIMIT, and follows uppercase naming convention for constants.
Applied to files:
tests/admin/valueset/valuesetConstants.ts
📚 Learning: 2025-11-25T13:53:40.623Z
Learnt from: CR
Repo: ohcnetwork/care_fe PR: 0
File: .github/instructions/utils.instructions.md:0-0
Timestamp: 2025-11-25T13:53:40.623Z
Learning: Applies to src/Utils/**/*.{ts,tsx} : Define constants in appropriate utility files with type-safe and well-documented values
Applied to files:
tests/admin/valueset/valuesetConstants.ts
📚 Learning: 2025-11-25T13:51:41.208Z
Learnt from: CR
Repo: ohcnetwork/care_fe PR: 0
File: .github/instructions/lib.instructions.md:0-0
Timestamp: 2025-11-25T13:51:41.208Z
Learning: Applies to src/lib/**/*.{ts,tsx} : Handle medical coding systems with SNOMED CT and LOINC standards
Applied to files:
tests/admin/valueset/valuesetConstants.ts
📚 Learning: 2025-11-20T09:48:26.486Z
Learnt from: yash-learner
Repo: ohcnetwork/care_fe PR: 14108
File: tests/facility/settings/activityDefinitions/activityDefinitionCreate.spec.ts:86-147
Timestamp: 2025-11-20T09:48:26.486Z
Learning: In tests/facility/settings/activityDefinitions/activityDefinitionCreate.spec.ts, detailed verification of selected values (e.g., code from value sets) in the detail view is intentionally deferred to the list spec, as the creation test focuses on the happy path and basic verification only.
Applied to files:
tests/admin/valueset/valuesetCreate.spec.ts
📚 Learning: 2025-11-25T13:54:09.978Z
Learnt from: CR
Repo: ohcnetwork/care_fe PR: 0
File: .cursor/rules/02-coding-standards.mdc:0-0
Timestamp: 2025-11-25T13:54:09.978Z
Learning: Applies to **/*.spec.ts : Write E2E tests in Playwright following https://playwright.dev/docs/writing-tests
Applied to files:
tests/admin/valueset/valuesetCreate.spec.ts
📚 Learning: 2025-11-25T13:53:40.623Z
Learnt from: CR
Repo: ohcnetwork/care_fe PR: 0
File: .github/instructions/utils.instructions.md:0-0
Timestamp: 2025-11-25T13:53:40.623Z
Learning: Applies to src/Utils/**/*.{ts,tsx} : Create test helpers and fixtures as separate utility files; keep test utilities separate from production code
Applied to files:
tests/admin/valueset/valuesetCreate.spec.ts
📚 Learning: 2025-11-25T13:52:33.081Z
Learnt from: CR
Repo: ohcnetwork/care_fe PR: 0
File: .github/instructions/providers.instructions.md:0-0
Timestamp: 2025-11-25T13:52:33.081Z
Learning: Applies to src/Providers/**/*.{test,spec}.{ts,tsx} : Test provider state management and updates with comprehensive unit tests
Applied to files:
tests/admin/valueset/valuesetCreate.spec.ts
📚 Learning: 2025-11-25T13:53:40.623Z
Learnt from: CR
Repo: ohcnetwork/care_fe PR: 0
File: .github/instructions/utils.instructions.md:0-0
Timestamp: 2025-11-25T13:53:40.623Z
Learning: Applies to src/Utils/**/*.{ts,tsx} : Provide utilities for common testing scenarios in test utility files
Applied to files:
tests/admin/valueset/valuesetCreate.spec.ts
📚 Learning: 2025-11-25T13:50:46.407Z
Learnt from: CR
Repo: ohcnetwork/care_fe PR: 0
File: .github/instructions/careui.instructions.md:0-0
Timestamp: 2025-11-25T13:50:46.407Z
Learning: Applies to src/CAREUI/**/*.{test,spec}.{ts,tsx} : Implement comprehensive testing in CAREUI components including accessibility and medical workflow scenarios
Applied to files:
tests/admin/valueset/valuesetCreate.spec.ts
📚 Learning: 2025-11-25T13:51:07.680Z
Learnt from: CR
Repo: ohcnetwork/care_fe PR: 0
File: .github/instructions/config-files.instructions.md:0-0
Timestamp: 2025-11-25T13:51:07.680Z
Learning: Applies to playwright.config.ts : Maintain `playwright.config.ts` as the Playwright testing configuration
Applied to files:
tests/admin/valueset/valuesetCreate.spec.ts
📚 Learning: 2025-11-25T13:54:09.978Z
Learnt from: CR
Repo: ohcnetwork/care_fe PR: 0
File: .cursor/rules/02-coding-standards.mdc:0-0
Timestamp: 2025-11-25T13:54:09.978Z
Learning: Applies to **/*.spec.ts : Write meaningful test descriptions
Applied to files:
tests/admin/valueset/valuesetCreate.spec.ts
📚 Learning: 2025-11-25T13:52:33.081Z
Learnt from: CR
Repo: ohcnetwork/care_fe PR: 0
File: .github/instructions/providers.instructions.md:0-0
Timestamp: 2025-11-25T13:52:33.081Z
Learning: Applies to src/Providers/**/*.{test,spec}.{ts,tsx} : Validate audit logging and compliance features in provider tests
Applied to files:
tests/admin/valueset/valuesetCreate.spec.ts
📚 Learning: 2025-11-20T09:26:50.157Z
Learnt from: yash-learner
Repo: ohcnetwork/care_fe PR: 14108
File: tests/facility/settings/activityDefinitions/activityDefinitionCreate.spec.ts:91-94
Timestamp: 2025-11-20T09:26:50.157Z
Learning: In tests/facility/settings/activityDefinitions/activityDefinitionCreate.spec.ts, the hard-coded bounds for faker.number.int (e.g., 0-4, 0-2) when selecting itemIndex are intentionally based on stable backend seed data that is guaranteed to be available in the test environment, making the approach valid and not flaky.
Applied to files:
tests/admin/valueset/valuesetCreate.spec.ts
📚 Learning: 2024-12-04T18:58:47.241Z
Learnt from: kihan2518B
Repo: ohcnetwork/care_fe PR: 8956
File: cypress/e2e/facility_spec/FacilityCreation.cy.ts:261-261
Timestamp: 2024-12-04T18:58:47.241Z
Learning: In `cypress/e2e/facility_spec/FacilityCreation.cy.ts`, the test case titled "Create a new facility with no bed and doctor capacity" includes steps to select bed types and specializations before validation. This is done intentionally to verify that error messages are shown properly when no capacity is specified.
Applied to files:
tests/admin/valueset/valuesetCreate.spec.ts
📚 Learning: 2025-01-15T11:05:16.663Z
Learnt from: nihal467
Repo: ohcnetwork/care_fe PR: 9986
File: cypress/e2e/users_spec/user_creation.cy.ts:43-55
Timestamp: 2025-01-15T11:05:16.663Z
Learning: UI verification for user creation in the users list should be implemented only after search functionality is available in the UI to avoid flaky tests. Currently, the test focuses on API verification only.
Applied to files:
tests/admin/valueset/valuesetCreate.spec.ts
📚 Learning: 2025-11-25T13:51:23.408Z
Learnt from: CR
Repo: ohcnetwork/care_fe PR: 0
File: .github/instructions/context.instructions.md:0-0
Timestamp: 2025-11-25T13:51:23.408Z
Learning: Applies to src/context/**/*.{test,spec}.{ts,tsx} : Validate emergency override scenarios in context tests
Applied to files:
tests/admin/valueset/valuesetCreate.spec.ts
🧬 Code graph analysis (1)
tests/admin/valueset/valuesetCreate.spec.ts (4)
tests/helper/utils.ts (1)
expectedSlug(6-8)tests/admin/valueset/valuesetConstants.ts (10)
STATUS_OPTIONS(93-93)SYSTEM_OPTIONS(91-91)VALID_SNOMED_CODES(1-12)SNOMED_CODE_NAME(14-25)VALID_LOINC_CODES(27-38)LOINC_CODE_NAME(40-51)VALID_UCUM_CODES(53-63)UCUM_CODE_NAME(65-75)MIN_SLUG_LENGTH(95-95)MAX_SLUG_LENGTH(96-96)tests/helper/ui.ts (2)
closeAnyOpenPopovers(47-61)expectToast(345-352)tests/helper/error.ts (1)
getFieldErrorMessage(16-18)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: build
- GitHub Check: Test
- GitHub Check: CodeQL-Build
🔇 Additional comments (2)
tests/admin/valueset/valuesetConstants.ts (1)
95-96: LGTM! Well-defined constants for slug validation.The constants are appropriately named, exported, and will support consistent slug validation testing across the test suite.
tests/admin/valueset/valuesetCreate.spec.ts (1)
118-130: No changes needed. The Status field has a default value ofValueSetStatus.ACTIVE(line 380 of ValueSetForm.tsx), so tests that don't explicitly select a status will use this default when submitting the form. Tests 2 and 3 don't attempt to save, making Status selection irrelevant. Test 4 successfully saves with the default Status value.
| const displayValue = await page | ||
| .getByRole("textbox", { name: "Unverified" }) | ||
| .inputValue(); | ||
| expect(displayValue).toContain(codeName); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potentially fragile selector using "Unverified" after code verification.
After clicking "Verify code" and confirming success, the textbox label might change from "Unverified" to something else (e.g., "Verified" or a code-specific label), making this selector brittle.
Consider using a more stable selector such as a test-id or a selector that doesn't depend on verification state, or verify that the label remains "Unverified" in the UI even after successful verification.
🤖 Prompt for AI Agents
In tests/admin/valueset/valuesetCreate.spec.ts around lines 78 to 81, the test
uses getByRole("textbox", { name: "Unverified" }) which is brittle because the
textbox label can change after verification; replace this with a stable selector
(preferably a data-testid on the input) or select the textbox by its
position/aria-label that does not depend on verification state, then read its
value and assert it contains codeName; if adding a test-id is not possible,
locate the textbox via a surrounding fieldset/label text that is stable or by
input[name|id] and use that instead.
| await previewDialog.getByRole("combobox").click(); | ||
|
|
||
| await expect(page.getByText(codeName)).toBeVisible(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use dialog-scoped selectors for more robust assertions.
Lines 95 and 97 use selectors that could be fragile:
- Line 95:
previewDialog.getByRole("combobox")lacks a specific name, which could fail if multiple comboboxes exist in the dialog. - Line 97:
page.getByText(codeName)checks the entire page instead of the dialog, which could produce false positives ifcodeNameappears elsewhere.
🔎 Recommended improvements
- await previewDialog.getByRole("combobox").click();
+ await previewDialog.getByRole("combobox", { name: "Select code" }).click(); // or appropriate accessible name
- await expect(page.getByText(codeName)).toBeVisible();
+ await expect(previewDialog.getByText(codeName)).toBeVisible();Note: Adjust the combobox name to match the actual accessible name in the UI.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| await previewDialog.getByRole("combobox").click(); | |
| await expect(page.getByText(codeName)).toBeVisible(); | |
| await previewDialog.getByRole("combobox", { name: "Select code" }).click(); // or appropriate accessible name | |
| await expect(previewDialog.getByText(codeName)).toBeVisible(); |
🤖 Prompt for AI Agents
In tests/admin/valueset/valuesetCreate.spec.ts around lines 95 to 97, the test
uses non-specific selectors (previewDialog.getByRole("combobox") without a name
and page.getByText(codeName)), which can be fragile and cause false positives;
update the combobox selection to use the dialog-scoped selector with its
accessible name (e.g., previewDialog.getByRole("combobox", { name:
"COMBOBOX_NAME" })) and change the page-wide text assertion to a dialog-scoped
assertion (e.g., previewDialog.getByText(codeName)) so both selectors are scoped
to the dialog and unambiguous.
| expect(slugValue).toBeTruthy(); | ||
| expect(slugValue.length).toBeGreaterThan(0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Remove redundant slug value check.
Checking both toBeTruthy() and length > 0 is redundant since any string with length > 0 is already truthy.
🔎 Simplified check
const slugValue = await slugField.inputValue();
- expect(slugValue).toBeTruthy();
- expect(slugValue.length).toBeGreaterThan(0);
+ expect(slugValue.length).toBeGreaterThan(0);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| expect(slugValue).toBeTruthy(); | |
| expect(slugValue.length).toBeGreaterThan(0); | |
| const slugValue = await slugField.inputValue(); | |
| expect(slugValue.length).toBeGreaterThan(0); |
🤖 Prompt for AI Agents
In tests/admin/valueset/valuesetCreate.spec.ts around lines 124 to 125, the test
redundantly asserts both expect(slugValue).toBeTruthy() and
expect(slugValue.length).toBeGreaterThan(0); remove one of these redundant
checks — keep the explicit length assertion
(expect(slugValue.length).toBeGreaterThan(0)) or, if you prefer truthiness, keep
expect(slugValue).toBeTruthy() — and delete the other assertion so the test only
verifies the slug once.
|
|
||
| const expectedSlugValue = expectedSlug(name); | ||
|
|
||
| expect(slugValue).toContain(expectedSlugValue); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Consider using toBe for precise slug matching.
Using toContain instead of toBe could miss cases where the slug has extra characters or formatting. If the slug should exactly match expectedSlug(name), use toBe for a stricter assertion.
🔎 More precise assertion
- expect(slugValue).toContain(expectedSlugValue);
+ expect(slugValue).toBe(expectedSlugValue);Use toContain only if additional slug processing (e.g., uniqueness suffixes) is expected.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| expect(slugValue).toContain(expectedSlugValue); | |
| expect(slugValue).toBe(expectedSlugValue); |
🤖 Prompt for AI Agents
In tests/admin/valueset/valuesetCreate.spec.ts around line 129, the test
currently uses expect(slugValue).toContain(expectedSlugValue); which allows
extra characters in the slug; replace this with a strict equality assertion
using expect(slugValue).toBe(expectedSlugValue) so the test fails if the slug
differs or contains unexpected suffixes, unless the code intentionally appends
uniqueness or formatting — in that case leave toContain and document the
expectation.
| const displayValue = await page | ||
| .getByRole("textbox", { name: "Unverified" }) | ||
| .inputValue(); | ||
| expect(displayValue).toContain(codeName); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potentially fragile selector using "Unverified" after code verification.
Same issue as in the first test: after code verification succeeds, the textbox label might change, making this selector unreliable.
Consider using a selector that doesn't depend on verification state.
🤖 Prompt for AI Agents
In tests/admin/valueset/valuesetCreate.spec.ts around lines 152-155, the test is
using page.getByRole("textbox", { name: "Unverified" }) which is brittle because
the label changes after verification; replace this locator with one that does
not depend on the verification state — e.g. target the textbox by a stable
attribute (data-testid or input id/aria-label/placeholder that is invariant), or
add a data-testid to the input in the component and use
page.getByTestId("...").inputValue() so the assertion no longer relies on the
"Unverified" label.
| await previewDialog.getByRole("combobox").click(); | ||
|
|
||
| await expect(page.getByText(codeName)).not.toBeVisible(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Scope selectors to the preview dialog for accurate assertions.
Lines 169-171 have the same selector issues as the first test:
- Line 169: Unspecified combobox could be fragile.
- Line 171:
page.getByText(codeName).not.toBeVisible()checks the entire page, which could produce false negatives ifcodeNameis hidden in the dialog but visible elsewhere on the page.
🔎 Recommended improvements
- await previewDialog.getByRole("combobox").click();
+ await previewDialog.getByRole("combobox", { name: "Select code" }).click(); // use appropriate accessible name
- await expect(page.getByText(codeName)).not.toBeVisible();
+ await expect(previewDialog.getByText(codeName)).not.toBeVisible();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| await previewDialog.getByRole("combobox").click(); | |
| await expect(page.getByText(codeName)).not.toBeVisible(); | |
| await previewDialog.getByRole("combobox", { name: "Select code" }).click(); | |
| await expect(previewDialog.getByText(codeName)).not.toBeVisible(); |
🤖 Prompt for AI Agents
tests/admin/valueset/valuesetCreate.spec.ts lines 169-171: the selectors are too
global — scope them to the previewDialog to avoid false negatives; replace the
unspecified combobox selector with a scoped one (e.g.,
previewDialog.getByRole("combobox", { name: /.../ }) or at minimum
previewDialog.getByRole("combobox").click()) and change
page.getByText(codeName).not.toBeVisible() to
previewDialog.getByText(codeName).not.toBeVisible() so the assertion only checks
the dialog contents.
| page.getByRole("textbox", { name: "Slug *" }), | ||
| ); | ||
| await expect(errorMessage).toBeVisible(); | ||
| await expect(errorMessage).toHaveText(/atleast \d+.*atmost 25/i); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Consider whether hard-coded "25" in error message check needs updating.
The error message regex hard-codes "25" instead of referencing MAX_SLUG_LENGTH. While this is checking UI text (not validation logic), if MAX_SLUG_LENGTH changes, this regex would need manual updating.
This is acceptable as-is since error messages are typically fixed strings, but consider documenting that this test expects the UI to display "atmost 25" in the error message.
Note: The spelling "atleast" (vs. "at least") should match the actual UI text. Verify this is intentional.
🤖 Prompt for AI Agents
In tests/admin/valueset/valuesetCreate.spec.ts around line 192, the regex
asserts "atmost 25" with a hard-coded 25; replace the literal with the project
constant (MAX_SLUG_LENGTH) so the expectation follows any future changes, e.g.,
build the regex/string using that constant imported from the module that defines
it; alternatively, if you intend this test to lock UI wording, add a brief
comment above the assertion stating the test intentionally expects the fixed
text "atmost 25" and verify the spelling "atleast"/"at least" matches the UI
before leaving it hard-coded.
🎭 Playwright Test ResultsStatus: ✅ Passed
📊 Detailed results are available in the playwright-final-report artifact. Run: #4178 |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| let name: string; | ||
| let slug: string; | ||
| let status: string; | ||
| let description: string; | ||
| let code: string; | ||
| let system: string; | ||
| let codeName: string; | ||
|
|
||
| test.describe("ValueSet Create", () => { |
Copilot
AI
Jan 2, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test variables are declared at module scope but only initialized in beforeEach. This pattern can lead to test interdependencies if tests run in parallel or if a test fails before beforeEach completes. Consider moving these declarations inside the describe block or using a factory function to generate fresh test data for each test.
| let name: string; | |
| let slug: string; | |
| let status: string; | |
| let description: string; | |
| let code: string; | |
| let system: string; | |
| let codeName: string; | |
| test.describe("ValueSet Create", () => { | |
| test.describe("ValueSet Create", () => { | |
| let name: string; | |
| let slug: string; | |
| let status: string; | |
| let description: string; | |
| let code: string; | |
| let system: string; | |
| let codeName: string; |
| case "UCUM": | ||
| code = faker.helpers.arrayElement(VALID_UCUM_CODES); | ||
| codeName = UCUM_CODE_NAME[code]; | ||
| break; |
Copilot
AI
Jan 2, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The switch statement lacks a default case, which means if system is randomly selected to a value outside ['SNOMED', 'LOINC', 'UCUM'], the variables code and codeName will remain undefined. This will cause test failures in tests that use these variables. Add a default case that throws an error or ensure the random selection is properly constrained.
| break; | |
| break; | |
| default: | |
| throw new Error(`Unsupported system: ${system}`); |
| page.getByRole("textbox", { name: "Slug *" }), | ||
| ); | ||
| await expect(errorMessage).toBeVisible(); | ||
| await expect(errorMessage).toHaveText(/atleast \d+.*atmost 25/i); |
Copilot
AI
Jan 2, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Corrected spelling of 'atleast' to 'at least' in the error message pattern.
| await page.getByRole("button", { name: /save/i }).click(); | ||
|
|
||
| await expect(errorMessage).toBeVisible(); | ||
| await expect(errorMessage).toHaveText(/atleast \d+.*atmost 25/i); |
Copilot
AI
Jan 2, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Corrected spelling of 'atleast' to 'at least' in the error message pattern.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (7)
tests/admin/valueset/valuesetCreate.spec.ts (7)
78-81: Potentially fragile selector using "Unverified" after code verification.After clicking "Verify code" and confirming success, the textbox label might change from "Unverified" to something else, making this selector unreliable. Consider using a more stable selector such as a data-testid or an input-specific attribute that doesn't depend on verification state.
95-97: Use dialog-scoped selectors for more robust assertions.Line 95 uses an unspecified combobox selector that could fail if multiple comboboxes exist. Line 97 checks the entire page instead of the dialog, which could produce false positives if
codeNameappears elsewhere.🔎 Recommended improvements
- await previewDialog.getByRole("combobox").click(); + await previewDialog.getByRole("combobox", { name: "Select code" }).click(); - await expect(page.getByText(codeName)).toBeVisible(); + await expect(previewDialog.getByText(codeName)).toBeVisible();Note: Adjust the combobox name to match the actual accessible name in the UI.
124-125: Remove redundant slug value check.Checking both
toBeTruthy()andlength > 0is redundant since any string with length > 0 is already truthy.🔎 Simplified check
const slugValue = await slugField.inputValue(); - expect(slugValue).toBeTruthy(); - expect(slugValue.length).toBeGreaterThan(0); + expect(slugValue.length).toBeGreaterThan(0);
129-129: Consider usingtoBefor precise slug matching.Using
toContaininstead oftoBecould miss cases where the slug has extra characters. If the slug should exactly matchexpectedSlug(name), usetoBefor stricter validation.🔎 More precise assertion
- expect(slugValue).toContain(expectedSlugValue); + expect(slugValue).toBe(expectedSlugValue);Use
toContainonly if additional slug processing (e.g., uniqueness suffixes) is expected.
152-155: Potentially fragile selector using "Unverified" after code verification.Same issue as in the first test: after code verification succeeds, the textbox label might change, making this selector unreliable. Consider using a selector that doesn't depend on verification state.
169-171: Scope selectors to the preview dialog for accurate assertions.Line 169 uses an unspecified combobox that could be fragile. Line 171 checks the entire page instead of the dialog, which could produce false negatives if
codeNameis hidden in the dialog but visible elsewhere.🔎 Recommended improvements
- await previewDialog.getByRole("combobox").click(); + await previewDialog.getByRole("combobox", { name: "Select code" }).click(); - await expect(page.getByText(codeName)).not.toBeVisible(); + await expect(previewDialog.getByText(codeName)).not.toBeVisible();
192-202: Hard-coded "25" and potential spelling issue in error message regex.Lines 192 and 202 hard-code "25" in the regex instead of using
MAX_SLUG_LENGTH. Additionally, the regex matches "atleast" and "atmost" (one word), but the correct English phrases are "at least" and "at most" (two words). If this matches actual error message text, the error message itself may need correction.
📜 Review details
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
tests/admin/valueset/valuesetCreate.spec.ts
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx}: Write concise, technical TypeScript code with accurate examples
Use TypeScript for all code; prefer interfaces over types
Avoid enums; use maps instead
Use TanStack Query for data fetching from the API along with query and mutate utilities for the queryFn and mutationFn
Use raviger for routing
**/*.{ts,tsx}: Use TypeScript with strict mode and ES2022 target
Useinterfacefor defining object types in TypeScript
Avoid explicitanytype in TypeScript
Use proper nullability annotations in TypeScript types
Use dedicated error handlers and TypeScript strict null checks
**/*.{ts,tsx}: Use TypeScript for all new code
Prefer interfaces over types for object definitions in TypeScript
Avoid usinganytype; use proper type definitions in TypeScript
Use type inference where possible in TypeScript
Use TanStack Query for API data management
Prefer React Context for global state management
**/*.{ts,tsx}: Use TanStack Query with thequeryandmutateutilities from@/Utils/request/
Use appropriate query keys following the resource pattern for TanStack Query
Leverage TanStack Query built-in features for pagination and debouncing
Implement proper error handling using the global error handler for TanStack Query operations
UseuseQueryhook withqueryKeyandqueryFnparameters, wherequeryFnwraps the API route with thequery()utility
UseuseMutationhook withmutationFnparameter wrapping API routes with themutate()utility, and implementonSuccesscallbacks to invalidate related queries
Support path parameters in TanStack Query using thepathParamsoption in query/mutate utilities
Support query parameters in TanStack Query using thequeryParamsoption in query/mutate utilities
Use thesilent: trueoption to suppress global error notifications when custom error handling is needed
**/*.{ts,tsx}: Use TypeScript with strict mode and ES2022 target
Useinterfacefor defining object types
Avoid explicitanytypes and maint...
Files:
tests/admin/valueset/valuesetCreate.spec.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx,js,jsx}: Use functional and declarative programming patterns; avoid classes
Prefer iteration and modularization over code duplication
Use descriptive variable names with auxiliary verbs (e.g., isLoading, hasError)
Use the "function" keyword for pure functions
Avoid unnecessary curly braces in conditionals; use concise syntax for simple statements
**/*.{ts,tsx,js,jsx}: Use path aliases for imports from src (@/*)
Use double quotes for strings
Semicolons are required at end of statements
Order imports as: 3rd-party → library → CAREUI → UI → components → hooks → utils → relative
Use PascalCase for component and class names
Use camelCase for variable and function names
**/*.{ts,tsx,js,jsx}: Use path aliases@/*for imports from src directory
Use double quotes for strings
Require semicolons at end of statements
Order imports: 3rd-party → library → CAREUI → UI → components → hooks → utils → relative
Use PascalCase for component and class names
Use camelCase for variable and function names
Files:
tests/admin/valueset/valuesetCreate.spec.ts
**/*
📄 CodeRabbit inference engine (CLAUDE.md)
Use 2-space indentation
Files:
tests/admin/valueset/valuesetCreate.spec.ts
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/02-coding-standards.mdc)
Use camelCase for utility file names (e.g.,
useAuth.ts)
Files:
tests/admin/valueset/valuesetCreate.spec.ts
**/*.spec.ts
📄 CodeRabbit inference engine (.cursor/rules/02-coding-standards.mdc)
**/*.spec.ts: Write E2E tests in Playwright following https://playwright.dev/docs/writing-tests
Write meaningful test descriptions
Files:
tests/admin/valueset/valuesetCreate.spec.ts
**/*.{tsx,ts}
📄 CodeRabbit inference engine (.cursor/rules/04-ui-components.mdc)
**/*.{tsx,ts}: Use Shadcn UI components as the primary component library
Follow the component documentation for proper usage
Customize components using Tailwind CSS
UsenavigateanduseRedirectfrom raviger for navigation and redirects
Use Tailwind's responsive classes and follow mobile-first approach
Implement proper ARIA attributes for accessibility
Ensure keyboard navigation works in components
Define component props using TypeScript interfaces
Use translation keys fromsrc/Locale/for internationalization
Support RTL languages in components
Use proper date/time formatting for multiple language support
Files:
tests/admin/valueset/valuesetCreate.spec.ts
**/*.{ts,tsx,js,jsx,json,css,scss,html}
📄 CodeRabbit inference engine (AGENTS.md)
Use 2-space indentation
Files:
tests/admin/valueset/valuesetCreate.spec.ts
🧠 Learnings (26)
📓 Common learnings
Learnt from: yash-learner
Repo: ohcnetwork/care_fe PR: 14108
File: tests/facility/settings/activityDefinitions/activityDefinitionCreate.spec.ts:86-147
Timestamp: 2025-11-20T09:48:26.486Z
Learning: In tests/facility/settings/activityDefinitions/activityDefinitionCreate.spec.ts, detailed verification of selected values (e.g., code from value sets) in the detail view is intentionally deferred to the list spec, as the creation test focuses on the happy path and basic verification only.
Learnt from: AdityaJ2305
Repo: ohcnetwork/care_fe PR: 13197
File: src/pages/Facility/settings/observationDefinition/ObservationDefinitionForm.tsx:470-495
Timestamp: 2025-09-16T20:46:59.537Z
Learning: In PR #13197, AdityaJ2305 indicated that aria-invalid and ref forwarding improvements for ValueSetSelect components can be deferred to a separate PR, considering them not critical for the current focus/scroll fix implementation.
📚 Learning: 2025-11-20T09:48:26.486Z
Learnt from: yash-learner
Repo: ohcnetwork/care_fe PR: 14108
File: tests/facility/settings/activityDefinitions/activityDefinitionCreate.spec.ts:86-147
Timestamp: 2025-11-20T09:48:26.486Z
Learning: In tests/facility/settings/activityDefinitions/activityDefinitionCreate.spec.ts, detailed verification of selected values (e.g., code from value sets) in the detail view is intentionally deferred to the list spec, as the creation test focuses on the happy path and basic verification only.
Applied to files:
tests/admin/valueset/valuesetCreate.spec.ts
📚 Learning: 2025-11-25T13:54:09.978Z
Learnt from: CR
Repo: ohcnetwork/care_fe PR: 0
File: .cursor/rules/02-coding-standards.mdc:0-0
Timestamp: 2025-11-25T13:54:09.978Z
Learning: Applies to **/*.spec.ts : Write E2E tests in Playwright following https://playwright.dev/docs/writing-tests
Applied to files:
tests/admin/valueset/valuesetCreate.spec.ts
📚 Learning: 2025-11-25T13:50:46.407Z
Learnt from: CR
Repo: ohcnetwork/care_fe PR: 0
File: .github/instructions/careui.instructions.md:0-0
Timestamp: 2025-11-25T13:50:46.407Z
Learning: Applies to src/CAREUI/**/*.{test,spec}.{ts,tsx} : Implement comprehensive testing in CAREUI components including accessibility and medical workflow scenarios
Applied to files:
tests/admin/valueset/valuesetCreate.spec.ts
📚 Learning: 2025-11-25T13:53:40.623Z
Learnt from: CR
Repo: ohcnetwork/care_fe PR: 0
File: .github/instructions/utils.instructions.md:0-0
Timestamp: 2025-11-25T13:53:40.623Z
Learning: Applies to src/Utils/**/*.{ts,tsx} : Create test helpers and fixtures as separate utility files; keep test utilities separate from production code
Applied to files:
tests/admin/valueset/valuesetCreate.spec.ts
📚 Learning: 2025-11-25T13:54:09.978Z
Learnt from: CR
Repo: ohcnetwork/care_fe PR: 0
File: .cursor/rules/02-coding-standards.mdc:0-0
Timestamp: 2025-11-25T13:54:09.978Z
Learning: Applies to **/*.spec.ts : Write meaningful test descriptions
Applied to files:
tests/admin/valueset/valuesetCreate.spec.ts
📚 Learning: 2025-11-25T13:52:33.081Z
Learnt from: CR
Repo: ohcnetwork/care_fe PR: 0
File: .github/instructions/providers.instructions.md:0-0
Timestamp: 2025-11-25T13:52:33.081Z
Learning: Applies to src/Providers/**/*.{test,spec}.{ts,tsx} : Test provider state management and updates with comprehensive unit tests
Applied to files:
tests/admin/valueset/valuesetCreate.spec.ts
📚 Learning: 2025-11-20T09:26:50.157Z
Learnt from: yash-learner
Repo: ohcnetwork/care_fe PR: 14108
File: tests/facility/settings/activityDefinitions/activityDefinitionCreate.spec.ts:91-94
Timestamp: 2025-11-20T09:26:50.157Z
Learning: In tests/facility/settings/activityDefinitions/activityDefinitionCreate.spec.ts, the hard-coded bounds for faker.number.int (e.g., 0-4, 0-2) when selecting itemIndex are intentionally based on stable backend seed data that is guaranteed to be available in the test environment, making the approach valid and not flaky.
Applied to files:
tests/admin/valueset/valuesetCreate.spec.ts
📚 Learning: 2025-11-25T13:53:40.623Z
Learnt from: CR
Repo: ohcnetwork/care_fe PR: 0
File: .github/instructions/utils.instructions.md:0-0
Timestamp: 2025-11-25T13:53:40.623Z
Learning: Applies to src/Utils/**/*.{ts,tsx} : Provide utilities for common testing scenarios in test utility files
Applied to files:
tests/admin/valueset/valuesetCreate.spec.ts
📚 Learning: 2024-12-04T18:58:47.241Z
Learnt from: kihan2518B
Repo: ohcnetwork/care_fe PR: 8956
File: cypress/e2e/facility_spec/FacilityCreation.cy.ts:261-261
Timestamp: 2024-12-04T18:58:47.241Z
Learning: In `cypress/e2e/facility_spec/FacilityCreation.cy.ts`, the test case titled "Create a new facility with no bed and doctor capacity" includes steps to select bed types and specializations before validation. This is done intentionally to verify that error messages are shown properly when no capacity is specified.
Applied to files:
tests/admin/valueset/valuesetCreate.spec.ts
📚 Learning: 2025-11-25T13:51:23.408Z
Learnt from: CR
Repo: ohcnetwork/care_fe PR: 0
File: .github/instructions/context.instructions.md:0-0
Timestamp: 2025-11-25T13:51:23.408Z
Learning: Applies to src/context/**/*.{test,spec}.{ts,tsx} : Verify audit trail completeness in context tests
Applied to files:
tests/admin/valueset/valuesetCreate.spec.ts
📚 Learning: 2024-12-05T04:20:20.231Z
Learnt from: nihal467
Repo: ohcnetwork/care_fe PR: 9301
File: cypress/e2e/facility_spec/FacilityLocation.cy.ts:220-220
Timestamp: 2024-12-05T04:20:20.231Z
Learning: In `FacilityLocation.cy.ts`, ensure not to suggest adding success message verifications if they are already present in the test steps.
Applied to files:
tests/admin/valueset/valuesetCreate.spec.ts
📚 Learning: 2024-11-14T10:29:07.447Z
Learnt from: Jacobjeevan
Repo: ohcnetwork/care_fe PR: 9080
File: cypress/e2e/users_spec/UsersManage.cy.ts:129-131
Timestamp: 2024-11-14T10:29:07.447Z
Learning: In the Cypress test files (e.g., `cypress/e2e/users_spec/UsersManage.cy.ts`), it is acceptable to use hardcoded English error messages, as testing is conducted in English.
Applied to files:
tests/admin/valueset/valuesetCreate.spec.ts
📚 Learning: 2025-01-14T09:22:13.878Z
Learnt from: rajku-dev
Repo: ohcnetwork/care_fe PR: 9887
File: src/components/Users/CreateUserForm.tsx:93-93
Timestamp: 2025-01-14T09:22:13.878Z
Learning: The suggestion to use GENDER_TYPES.map(g => g.id) for gender enum validation in CreateUserForm.tsx was incorrect and caused an error.
Applied to files:
tests/admin/valueset/valuesetCreate.spec.ts
📚 Learning: 2024-11-15T05:22:27.912Z
Learnt from: AnveshNalimela
Repo: ohcnetwork/care_fe PR: 8930
File: cypress/pageobject/Users/UserCreation.ts:3-5
Timestamp: 2024-11-15T05:22:27.912Z
Learning: In the `clickProfileName()` method of the `UserCreationPage` class, the profile name is dynamic and cannot be verified using a static text. Therefore, `cy.get("#user-profile-name").click();` should be used instead of `cy.verifyAndClickElement`.
Applied to files:
tests/admin/valueset/valuesetCreate.spec.ts
📚 Learning: 2025-01-15T11:05:16.663Z
Learnt from: nihal467
Repo: ohcnetwork/care_fe PR: 9986
File: cypress/e2e/users_spec/user_creation.cy.ts:43-55
Timestamp: 2025-01-15T11:05:16.663Z
Learning: UI verification for user creation in the users list should be implemented only after search functionality is available in the UI to avoid flaky tests. Currently, the test focuses on API verification only.
Applied to files:
tests/admin/valueset/valuesetCreate.spec.ts
📚 Learning: 2024-11-14T10:28:32.680Z
Learnt from: Jacobjeevan
Repo: ohcnetwork/care_fe PR: 9080
File: cypress/pageobject/Users/ManageUserPage.ts:181-188
Timestamp: 2024-11-14T10:28:32.680Z
Learning: In the Cypress tests for user management (`cypress/pageobject/Users/ManageUserPage.ts`), the method `verifyDoctorQualification()` is intended to verify qualifications specifically for the Doctor role.
Applied to files:
tests/admin/valueset/valuesetCreate.spec.ts
📚 Learning: 2025-01-19T14:12:10.781Z
Learnt from: AdityaJ2305
Repo: ohcnetwork/care_fe PR: 10054
File: src/components/Users/CreateUserForm.tsx:278-297
Timestamp: 2025-01-19T14:12:10.781Z
Learning: When using Cypress with React forms that have real-time validation, use `should('be.visible')` before typing and add `{ force: true }` option or break down type commands into smaller chunks with intermediate assertions to handle DOM detachment issues.
Applied to files:
tests/admin/valueset/valuesetCreate.spec.ts
📚 Learning: 2024-11-18T10:46:56.270Z
Learnt from: Jacobjeevan
Repo: ohcnetwork/care_fe PR: 9145
File: cypress/e2e/facility_spec/FacilityHomepage.cy.ts:229-317
Timestamp: 2024-11-18T10:46:56.270Z
Learning: In `cypress/e2e/facility_spec/FacilityHomepage.cy.ts`, when verifying the bed capacity badge reflection, the badge remains the same during multiple actions and only updates after a bed is attached to a patient during consultation. Therefore, it's appropriate to keep the test case consolidated rather than splitting it into smaller tests.
Applied to files:
tests/admin/valueset/valuesetCreate.spec.ts
📚 Learning: 2024-11-14T08:09:58.453Z
Learnt from: Jacobjeevan
Repo: ohcnetwork/care_fe PR: 9080
File: cypress/pageobject/Users/ManageUserPage.ts:67-97
Timestamp: 2024-11-14T08:09:58.453Z
Learning: In Cypress test files (`cypress/pageobject/Users/ManageUserPage.ts`), when methods are called to check input provided earlier in the test, adding TypeScript interfaces, input validation, and custom error messages is unnecessary.
Applied to files:
tests/admin/valueset/valuesetCreate.spec.ts
📚 Learning: 2025-11-25T13:52:19.758Z
Learnt from: CR
Repo: ohcnetwork/care_fe PR: 0
File: .github/instructions/pages.instructions.md:0-0
Timestamp: 2025-11-25T13:52:19.758Z
Learning: Applies to src/pages/**/*.{ts,tsx} : Use proper ARIA labels and roles in page components
Applied to files:
tests/admin/valueset/valuesetCreate.spec.ts
📚 Learning: 2025-11-25T13:52:19.758Z
Learnt from: CR
Repo: ohcnetwork/care_fe PR: 0
File: .github/instructions/pages.instructions.md:0-0
Timestamp: 2025-11-25T13:52:19.758Z
Learning: Applies to src/pages/**/*.{ts,tsx} : Add appropriate `data-cy` attributes for E2E tests to page components
Applied to files:
tests/admin/valueset/valuesetCreate.spec.ts
📚 Learning: 2025-11-25T13:51:23.408Z
Learnt from: CR
Repo: ohcnetwork/care_fe PR: 0
File: .github/instructions/context.instructions.md:0-0
Timestamp: 2025-11-25T13:51:23.408Z
Learning: Applies to src/context/**/*.{test,spec}.{ts,tsx} : Validate emergency override scenarios in context tests
Applied to files:
tests/admin/valueset/valuesetCreate.spec.ts
📚 Learning: 2024-11-18T10:48:08.500Z
Learnt from: Jacobjeevan
Repo: ohcnetwork/care_fe PR: 9145
File: cypress/e2e/patient_spec/PatientConsultationCreation.cy.ts:93-94
Timestamp: 2024-11-18T10:48:08.500Z
Learning: In `cypress/e2e/patient_spec/PatientConsultationCreation.cy.ts`, bed capacity verification assertions after patient admission are already being performed elsewhere, so adding them here is unnecessary.
Applied to files:
tests/admin/valueset/valuesetCreate.spec.ts
📚 Learning: 2024-11-20T23:25:06.602Z
Learnt from: Jacobjeevan
Repo: ohcnetwork/care_fe PR: 9145
File: src/common/validation.tsx:41-41
Timestamp: 2024-11-20T23:25:06.602Z
Learning: When updating the name validation pattern in `src/common/validation.tsx`, avoid partial internationalization by not adding specific non-Latin Unicode ranges unless we're prepared to support all non-Latin scripts.
Applied to files:
tests/admin/valueset/valuesetCreate.spec.ts
📚 Learning: 2025-11-25T13:52:33.081Z
Learnt from: CR
Repo: ohcnetwork/care_fe PR: 0
File: .github/instructions/providers.instructions.md:0-0
Timestamp: 2025-11-25T13:52:33.081Z
Learning: Applies to src/Providers/**/*.{test,spec}.{ts,tsx} : Validate audit logging and compliance features in provider tests
Applied to files:
tests/admin/valueset/valuesetCreate.spec.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: build
- GitHub Check: Test
- GitHub Check: CodeQL-Build
🔇 Additional comments (1)
tests/admin/valueset/valuesetCreate.spec.ts (1)
29-212: Verify test coverage against PR objectives.The PR objectives mention three test cases:
- Include rules with concept-only entries ✓ (covered in test 1)
- Include rules with filters ✗ (not covered)
- Both exclude and include rules together ✗ (test 3 only covers exclude rules)
Consider whether additional tests are needed to cover filter-based rules and the combination of both include and exclude rules in a single ValueSet.
Based on learnings, creation tests may focus on the happy path with detailed verification deferred to list specs. Please confirm if the current coverage is sufficient or if additional test cases should be added in a follow-up.
| switch (system) { | ||
| case "SNOMED": | ||
| code = faker.helpers.arrayElement(VALID_SNOMED_CODES); | ||
| codeName = SNOMED_CODE_NAME[code]; | ||
| break; | ||
| case "LOINC": | ||
| code = faker.helpers.arrayElement(VALID_LOINC_CODES); | ||
| codeName = LOINC_CODE_NAME[code]; | ||
| break; | ||
| case "UCUM": | ||
| code = faker.helpers.arrayElement(VALID_UCUM_CODES); | ||
| codeName = UCUM_CODE_NAME[code]; | ||
| break; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a default case to handle unexpected system values.
The switch statement lacks a default case. If SYSTEM_OPTIONS contains a system value not handled by the existing cases, code and codeName will remain undefined, leading to confusing test failures.
🔎 Proposed fix
switch (system) {
case "SNOMED":
code = faker.helpers.arrayElement(VALID_SNOMED_CODES);
codeName = SNOMED_CODE_NAME[code];
break;
case "LOINC":
code = faker.helpers.arrayElement(VALID_LOINC_CODES);
codeName = LOINC_CODE_NAME[code];
break;
case "UCUM":
code = faker.helpers.arrayElement(VALID_UCUM_CODES);
codeName = UCUM_CODE_NAME[code];
break;
+ default:
+ throw new Error(`Unsupported system: ${system}`);
}🤖 Prompt for AI Agents
In tests/admin/valueset/valuesetCreate.spec.ts around lines 37 to 50, the switch
on system misses a default branch so code and codeName can remain undefined for
unexpected SYSTEM_OPTIONS values; add a default case that assigns safe fallback
values (e.g., pick from a general valid codes array or set code/codeName to a
known placeholder) or throw a clear error to fail fast, ensuring the test either
recovers with valid data or reports a descriptive failure.
Proposed Changes
Fixes #14304
Tagging: @ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.