feat: add E2E Playwright tests for billing settings pages#16230
Open
pruthvirajpatil2024 wants to merge 1 commit intoohcnetwork:developfrom
Open
feat: add E2E Playwright tests for billing settings pages#16230pruthvirajpatil2024 wants to merge 1 commit intoohcnetwork:developfrom
pruthvirajpatil2024 wants to merge 1 commit intoohcnetwork:developfrom
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
|
Contributor
WalkthroughA new test file is added for facility billing settings providing end-to-end coverage for search/filter functionality across three table-based pages and edit/save flows for two configuration pages. Tests use authenticated context, role-based locators, and localized strings from en.json. Changes
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/facility/settings/billing/billingSetting.spec.ts`:
- Around line 191-210: Capture the current invoice expression text from the
read-only view before entering edit mode, then assert the textbox (obtained via
page.getByRole("textbox", { name: en.invoice_number_expression }) assigned to
input) is prefilled with that exact value after clicking the Edit button; after
clicking Cancel assert the read-only heading/field again shows the original
captured expression. Apply the same change to the similar test that checks
cancel behavior (the second test for invoice_number_expression) so both verify
preservation of the stored expression rather than only presence of
headings/buttons.
- Around line 152-166: Capture the original billing value before editing in the
test "modifying values and saving shows success toast" (e.g., read current value
from page.getByLabel(en.max_applicable_discounts) into a variable), perform the
fill/click/asserts as written, and then add a finally block that restores the
original value by refilling the saved original and clicking the save button (use
the same selectors: page.getByLabel(en.max_applicable_discounts),
page.getByRole("button", { name: en.save })). Apply the same pattern to the
other save test (the one around lines 212-232) so both tests always revert
backend state even on failures.
- Around line 126-179: The tests lack a state oracle: in the "shows current
values in read-only view on navigation" and subsequent tests (functions
referencing en.max_applicable_discounts, en.applicability_order, maxInput, and
en.discount_configuration_saved) assert only visibility; update them to read the
current displayed values (capture the read-only text for
max_applicable_discounts and applicability_order), assert those match the
expected seed/state, then when clicking Edit assert the inputs (maxInput and the
applicability_order input) are pre-filled with those exact values; for the save
test, choose a new value deterministically different from the current value
(e.g., read current value and increment or toggle) before filling and saving,
then assert the read-only view now shows the new value and the success toast
(en.discount_configuration_saved); for the cancel test, record the original
read-only values, change inputs, click cancel, and assert the read-only view
still shows the original values and the inputs are hidden.
- Around line 29-35: The beforeEach hooks (test.beforeEach) are each waiting up
to the global 60s Playwright timeout (via .waitFor with timeout: 60000) which
exhausts the test budget; add a longer test timeout for this spec by calling
test.setTimeout(...) near the top of the file (before the beforeEach blocks) or
increase timeouts on the specific hooks/tests; update the file to call
test.setTimeout(120000) (or another suitable value) before the test.beforeEach
usages so the beforeEach waits don't consume the entire test timeout (also apply
the same change for the other beforeEach blocks referenced).
🪄 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: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0bd45c5d-b35e-439a-b87c-db1e3d65adbe
📒 Files selected for processing (1)
tests/facility/settings/billing/billingSetting.spec.ts
Comment on lines
+29
to
+35
| test.beforeEach(async ({ page }) => { | ||
| await page.goto(`/facility/${facilityId}${route}`); | ||
| // Wait for the search input to appear — proves auth + facility data loaded | ||
| await page | ||
| .getByPlaceholder(placeholder) | ||
| .waitFor({ state: "visible", timeout: 60000 }); | ||
| }); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
fd 'playwright\.config\.(ts|js|mjs|cjs)$' . -x sh -c '
printf "\n--- %s ---\n" "$1"
sed -n "1,220p" "$1"
' sh {}
printf "\n--- timeout usage in tests/facility/settings/billing/billingSetting.spec.ts ---\n"
sed -n '1,260p' tests/facility/settings/billing/billingSetting.spec.ts \
| nl -ba \
| rg -n 'setTimeout|waitFor\(|timeout:'Repository: ohcnetwork/care_fe
Length of output: 2995
🏁 Script executed:
#!/bin/bash
# Check for waitFor and setTimeout in the spec file
echo "=== Looking for waitFor calls with timeout values ==="
rg 'waitFor\(' tests/facility/settings/billing/billingSetting.spec.ts -A 2 -B 1
echo ""
echo "=== Looking for test.setTimeout calls ==="
rg 'test\.setTimeout|test\.use' tests/facility/settings/billing/billingSetting.spec.tsRepository: ohcnetwork/care_fe
Length of output: 535
Increase the test timeout before the beforeEach hooks consume the entire budget.
The global Playwright timeout is 60 seconds. Each beforeEach hook waits up to 60 seconds, exhausting the entire timeout before any assertions run. This spec fails to override the timeout.
test.use({ storageState: "tests/.auth/user.json" });
+test.setTimeout(120000);Also applies to: lines 116-124, 183-189
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/facility/settings/billing/billingSetting.spec.ts` around lines 29 - 35,
The beforeEach hooks (test.beforeEach) are each waiting up to the global 60s
Playwright timeout (via .waitFor with timeout: 60000) which exhausts the test
budget; add a longer test timeout for this spec by calling test.setTimeout(...)
near the top of the file (before the beforeEach blocks) or increase timeouts on
the specific hooks/tests; update the file to call test.setTimeout(120000) (or
another suitable value) before the test.beforeEach usages so the beforeEach
waits don't consume the entire test timeout (also apply the same change for the
other beforeEach blocks referenced).
Comment on lines
+126
to
+179
| test("shows current values in read-only view on navigation", async ({ | ||
| page, | ||
| }) => { | ||
| await expect(page.getByRole("button", { name: en.edit })).toBeVisible(); | ||
| await expect( | ||
| page.getByText(en.max_applicable_discounts, { exact: true }), | ||
| ).toBeVisible(); | ||
| await expect( | ||
| page.getByText(en.applicability_order, { exact: true }), | ||
| ).toBeVisible(); | ||
| }); | ||
|
|
||
| test("clicking Edit shows pre-filled input fields", async ({ page }) => { | ||
| await page.getByRole("button", { name: en.edit }).click(); | ||
|
|
||
| const maxInput = page.getByLabel(en.max_applicable_discounts); | ||
| await expect(maxInput).toBeVisible(); | ||
|
|
||
| // Value must be a whole number pre-filled from current config | ||
| await expect(maxInput).toHaveValue(/^\d+$/); | ||
|
|
||
| await expect(page.getByLabel(en.applicability_order)).toBeVisible(); | ||
| await expect(page.getByRole("button", { name: en.save })).toBeVisible(); | ||
| await expect(page.getByRole("button", { name: en.cancel })).toBeVisible(); | ||
| }); | ||
|
|
||
| test("modifying values and saving shows success toast", async ({ page }) => { | ||
| await page.getByRole("button", { name: en.edit }).click(); | ||
|
|
||
| const maxInput = page.getByLabel(en.max_applicable_discounts); | ||
| const newValue = String(faker.number.int({ min: 1, max: 10 })); | ||
| await maxInput.fill(newValue); | ||
|
|
||
| await page.getByRole("button", { name: en.save }).click(); | ||
|
|
||
| await expect( | ||
| page.getByText(en.discount_configuration_saved, { exact: true }), | ||
| ).toBeVisible({ timeout: 5000 }); | ||
| // Read-only view restored | ||
| await expect(page.getByRole("button", { name: en.edit })).toBeVisible(); | ||
| }); | ||
|
|
||
| test("cancelling edit restores original values", async ({ page }) => { | ||
| await page.getByRole("button", { name: en.edit }).click(); | ||
|
|
||
| const maxInput = page.getByLabel(en.max_applicable_discounts); | ||
| await maxInput.fill("999"); | ||
|
|
||
| await page.getByRole("button", { name: en.cancel }).click(); | ||
|
|
||
| // Edit form must be gone — edit button back, inputs hidden | ||
| await expect(page.getByRole("button", { name: en.edit })).toBeVisible(); | ||
| await expect(maxInput).not.toBeVisible(); | ||
| }); |
Contributor
There was a problem hiding this comment.
Discount Configuration needs a real state oracle.
This suite mostly checks labels and mode switches. It never proves that the read-only view shows the current values, that applicability_order is prefilled correctly, that save actually persisted a different value, or that cancel restored the previous one. A blank read-only section or a no-op save can still pass here, especially because the random 1..10 can equal the current value.
♻️ Suggested pattern
test("modifying values and saving shows success toast", async ({ page }) => {
await page.getByRole("button", { name: en.edit }).click();
const maxInput = page.getByLabel(en.max_applicable_discounts);
- const newValue = String(faker.number.int({ min: 1, max: 10 }));
+ const originalValue = await maxInput.inputValue();
+ const newValue = originalValue === "1" ? "2" : "1";
await maxInput.fill(newValue);
await page.getByRole("button", { name: en.save }).click();
await expect(
page.getByText(en.discount_configuration_saved, { exact: true }),
).toBeVisible({ timeout: 5000 });
- // Read-only view restored
- await expect(page.getByRole("button", { name: en.edit })).toBeVisible();
+ await page.getByRole("button", { name: en.edit }).click();
+ await expect(page.getByLabel(en.max_applicable_discounts)).toHaveValue(newValue);
});
test("cancelling edit restores original values", async ({ page }) => {
await page.getByRole("button", { name: en.edit }).click();
const maxInput = page.getByLabel(en.max_applicable_discounts);
- await maxInput.fill("999");
+ const originalValue = await maxInput.inputValue();
+ const draftValue = originalValue === "999" ? "998" : "999";
+ await maxInput.fill(draftValue);
await page.getByRole("button", { name: en.cancel }).click();
- // Edit form must be gone — edit button back, inputs hidden
- await expect(page.getByRole("button", { name: en.edit })).toBeVisible();
- await expect(maxInput).not.toBeVisible();
+ await page.getByRole("button", { name: en.edit }).click();
+ await expect(page.getByLabel(en.max_applicable_discounts)).toHaveValue(originalValue);
});Do the same for applicability_order, not just its visibility.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/facility/settings/billing/billingSetting.spec.ts` around lines 126 -
179, The tests lack a state oracle: in the "shows current values in read-only
view on navigation" and subsequent tests (functions referencing
en.max_applicable_discounts, en.applicability_order, maxInput, and
en.discount_configuration_saved) assert only visibility; update them to read the
current displayed values (capture the read-only text for
max_applicable_discounts and applicability_order), assert those match the
expected seed/state, then when clicking Edit assert the inputs (maxInput and the
applicability_order input) are pre-filled with those exact values; for the save
test, choose a new value deterministically different from the current value
(e.g., read current value and increment or toggle) before filling and saving,
then assert the read-only view now shows the new value and the success toast
(en.discount_configuration_saved); for the cancel test, record the original
read-only values, change inputs, click cancel, and assert the read-only view
still shows the original values and the inputs are hidden.
Comment on lines
+152
to
+166
| test("modifying values and saving shows success toast", async ({ page }) => { | ||
| await page.getByRole("button", { name: en.edit }).click(); | ||
|
|
||
| const maxInput = page.getByLabel(en.max_applicable_discounts); | ||
| const newValue = String(faker.number.int({ min: 1, max: 10 })); | ||
| await maxInput.fill(newValue); | ||
|
|
||
| await page.getByRole("button", { name: en.save }).click(); | ||
|
|
||
| await expect( | ||
| page.getByText(en.discount_configuration_saved, { exact: true }), | ||
| ).toBeVisible({ timeout: 5000 }); | ||
| // Read-only view restored | ||
| await expect(page.getByRole("button", { name: en.edit })).toBeVisible(); | ||
| }); |
Contributor
There was a problem hiding this comment.
Restore the settings you persist in the save-path tests.
tests/support/facilityId.ts:1-43 reuses a shared facility, so both save tests leave backend state changed for retries and later specs. If one of these tests fails after saving, the retry starts from already-edited billing settings. Restore the original values in a finally block after the success assertions.
Also applies to: 212-232
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/facility/settings/billing/billingSetting.spec.ts` around lines 152 -
166, Capture the original billing value before editing in the test "modifying
values and saving shows success toast" (e.g., read current value from
page.getByLabel(en.max_applicable_discounts) into a variable), perform the
fill/click/asserts as written, and then add a finally block that restores the
original value by refilling the saved original and clicking the save button (use
the same selectors: page.getByLabel(en.max_applicable_discounts),
page.getByRole("button", { name: en.save })). Apply the same pattern to the
other save test (the one around lines 212-232) so both tests always revert
backend state even on failures.
Comment on lines
+191
to
+210
| test("shows current expression in read-only view on navigation", async ({ | ||
| page, | ||
| }) => { | ||
| await expect(page.getByRole("button", { name: en.edit })).toBeVisible(); | ||
| await expect( | ||
| page.getByRole("heading", { name: en.invoice_number_expression }).first(), | ||
| ).toBeVisible(); | ||
| }); | ||
|
|
||
| test("clicking Edit shows pre-filled input field", async ({ page }) => { | ||
| await page.getByRole("button", { name: en.edit }).click(); | ||
|
|
||
| const input = page.getByRole("textbox", { | ||
| name: en.invoice_number_expression, | ||
| }); | ||
| await expect(input).toBeVisible(); | ||
| await expect(input).toBeEnabled(); | ||
| await expect(page.getByRole("button", { name: en.save })).toBeVisible(); | ||
| await expect(page.getByRole("button", { name: en.cancel })).toBeVisible(); | ||
| }); |
Contributor
There was a problem hiding this comment.
Assert the existing invoice expression, not just the heading.
The current checks only prove that the section renders and that a draft value disappears. If cancel blanks the stored expression instead of restoring it, this suite still passes. Capture the original expression, assert the textbox is prefilled with it, and after cancel verify that same value is shown again.
🧪 Minimal improvement
test("clicking Edit shows pre-filled input field", async ({ page }) => {
await page.getByRole("button", { name: en.edit }).click();
const input = page.getByRole("textbox", {
name: en.invoice_number_expression,
});
await expect(input).toBeVisible();
await expect(input).toBeEnabled();
+ await expect(input).toHaveValue(/\S+/);
await expect(page.getByRole("button", { name: en.save })).toBeVisible();
await expect(page.getByRole("button", { name: en.cancel })).toBeVisible();
});
test("cancelling edit restores original expression", async ({ page }) => {
await page.getByRole("button", { name: en.edit }).click();
const input = page.getByRole("textbox", {
name: en.invoice_number_expression,
});
+ const originalExpression = await input.inputValue();
await input.fill("TEMP_EXPR_XYZ");
await page.getByRole("button", { name: en.cancel }).click();
await expect(page.getByRole("button", { name: en.edit })).toBeVisible();
await expect(input).not.toBeVisible();
await expect(page.getByText("TEMP_EXPR_XYZ")).not.toBeVisible();
+ await expect(page.getByText(originalExpression, { exact: true })).toBeVisible();
});Also applies to: 234-249
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/facility/settings/billing/billingSetting.spec.ts` around lines 191 -
210, Capture the current invoice expression text from the read-only view before
entering edit mode, then assert the textbox (obtained via
page.getByRole("textbox", { name: en.invoice_number_expression }) assigned to
input) is prefilled with that exact value after clicking the Edit button; after
clicking Cancel assert the read-only heading/field again shows the original
captured expression. Apply the same change to the similar test that checks
cancel behavior (the second test for invoice_number_expression) so both verify
preservation of the stored expression rather than only presence of
headings/buttons.
Jacobjeevan
requested changes
Mar 31, 2026
Contributor
Jacobjeevan
left a comment
Jacobjeevan
left a comment
There was a problem hiding this comment.
Self-review - do compare with our other tests suites.
| (await firstDataRow.getByRole("cell").first().textContent()) ?? ""; | ||
| const searchTerm = cellText.trim().slice(0, 4); | ||
| if (!searchTerm) { | ||
| throw new Error( |
Contributor
There was a problem hiding this comment.
why? does it make sense to throw an error here?
| }); | ||
| } | ||
|
|
||
| searchFilterSuite( |
Contributor
There was a problem hiding this comment.
refer to how other tests are handled; write tests that call a common search function, rather than writing a common fn that wraps tests.
| import { expect, test } from "@playwright/test"; | ||
| import { getFacilityId } from "tests/support/facilityId"; | ||
|
|
||
| import en from "@/public/locale/en.json"; |
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
2 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.
Proposed Changes
Fixes #16208
en.jsoni18n keys — no hardcoded English literals in assertions{ exact: true }to prevent strict-mode violations from DOM-nesting multi-matchestest.setTimeout(120000)set at file level to givebeforeEach(navigation +waitFor) sufficient budget within the global 60 s test timeoutsearchFilterSuite()helper eliminates duplication across the three identical table-page suitesTagging:
@ohcnetwork/care-fe-code-reviewersMerge Checklist