test: Add Playwright visual tests

[prajwalnl]

Type of Change

• Bugfix
• New feature
• Enhancement
• Refactoring
• Dependency updates
• Documentation
• CI/CD

Description

Add visual tests for orchestrator product.

Motivation and Context

Increase visual testing coverage.

How did you test it?

Ran tests locally and verified all tests pass in CI.

Where to test it?

• INTEG
• SANDBOX
• PROD

Backend Dependency

• Yes
• No

Backend PR URL:

Feature Flag

• New feature flag added
• Existing feature flag updated
• No feature flag changes

Feature flag name(s):

Checklist

• I ran npm run re:build
• I reviewed submitted code
• I added unit tests for my changes where possible

[semanticdiff-com]

Review changes with  

[XyneSpaces]

Review Summary

Classification: Test coverage expansion — Playwright visual tests for global search, connectors, and operations
Risk Level: Low (test-only changes, no production code modifications)
Files Changed: 8 modified TypeScript files, 26 renamed/added snapshot files

---

⚠️ Warning — Missing Test Descriptions

Location: PR Description template

The PR body has empty sections for:

• Description
• Motivation and Context
• How did you test it?

Fix: Even for test-only PRs, please complete the description with:

1. What areas of the UI are being visually tested
2. Why these specific test scenarios were chosen (coverage gaps being filled)
3. Confirmation that visual baselines were generated on the reference environment

---

⚠️ Warning — Commented Out Code in PayinConnector Tests

Location: playwright-tests/e2e/4-connectors/payinConnector.spec.ts

The test code relies heavily on XPath-like locators (.locator("div").filter({ hasText: /^Select PM Authentication Processor$/ })) and nth(1) indexing. While this may work, these selectors are fragile and may break on UI layout changes.

Fix: Consider adding data-testid attributes to the underlying components for more stable selectors:

// Instead of:
await page.locator('div').filter({ hasText: /^Select PM Authentication Processor$/ }).nth(1)).toBeVisible();

// Prefer:
await page.getByTestId('pm-authenticator-select').toBeVisible();

---

💡 Suggestion — Magic Numbers Should Be Named Constants

Location: playwright-tests/e2e/4-connectors/payinConnector.spec.ts lines 3-5

The timeout value 60000 is repeated across multiple tests.

Fix: Define a constant at the describe block level:

const CONNECTOR_SETUP_TIMEOUT = 60000;

test("should setup Stripe connector...", async ({ page }) => {
  test.setTimeout(CONNECTOR_SETUP_TIMEOUT);
  // ...
});

---

💡 Suggestion — Incomplete Test Flow Verification

Location: playwright-tests/e2e/4-connectors/payinConnector.spec.ts line 219+

The test "should setup Stripe connector without Plaid and Bank Debit PMT" opens the PM Authenticator dialog but doesn't select an authenticator before clicking Proceed. The test passes, but this may indicate the flow allows proceeding without selection, or the test is incomplete.

Fix: Clarify in comments whether proceeding without selection is the expected behavior, or add assertions verifying the disabled state of controls.

---

✅ Positive Aspects

• ✅ Good use of helper function generateMerchantName() for identifiable test data
• ✅ Comprehensive coverage for global search "View N results" pagination
• ✅ Proper mocking of customer details and search APIs
• ✅ Visual tests use maxDiffPixelRatio: 0.01 for reasonable tolerance
• ✅ Refactoring from generateDateTimeString to generateMerchantName improves test data traceability

---

Pre-existing Issues (unchanged by this PR)

None introduced.

---

Verdict: 💬 Comment — Clean test additions. Please complete PR description and consider data-testid attributes for fragile selectors.

[XyneSpaces]

Review Summary

Classification: Test Infrastructure Enhancement — Playwright visual tests expansion
Risk Level: Low-Medium (test-only changes, but touches connector auth patterns)
Files Changed: 8 test files (+728/-35 lines)

---

✅ Tier 1: High-Signal Checks

1. Naming Consistency

• ✅ generateMerchantName() follows existing convention in helper.ts
• ✅ playwright_ prefix is consistently applied across all test files
• ✅ Mock naming follows established patterns (VIEW_RESULTS_*, MOCK_*_RESPONSE)

2. Reuse Existing Utilities

• ✅ Uses existing generateUniqueEmail(), signupUser(), loginUI() helpers
• ✅ Leverages existing mockPaymentFilters and ompLineage utilities
• ✅ Properly imports page objects from established patterns (HomePage, PaymentConnector)

3. Error Handling & Async Robustness

• ⚠️ Missing explicit error handling in payinConnector.spec.ts: The test function testConnectorWithPlaid performs multiple UI interactions without intermediate verification that previous steps succeeded (lines with await page.click() chains). If an early click fails silently, subsequent assertions on later screens may give misleading errors.

---

✅ Tier 2: Medium-Signal Checks

4. Analytics/Event Tracking

• ✅ N/A for test files (no mixpanel events being added in this diff)

5. Feature Flag Gating

• ✅ N/A for test files (testing feature-flagged behavior via API mocks, not gating tests themselves)

6. useEffect Skepticism

• ✅ N/A — No React component changes in this PR

7. Open/Import Discipline

• ✅ Test imports are clean and consistent
• ✅ No unnecessary imports detected

8. Tailwind/MUI Correctness

• ✅ N/A — No styling changes in this PR

9. Conditional Logic Validation

• ✅ Test conditions properly structured with clear branching for auth/no-auth scenarios

10. Magic Values

• ✅ Constants extracted: VIEW_RESULTS_COUNT = 11, PREVIEW_TABLE_PAGE_SIZE = 10
• ✅ Password and URLs come from environment variables with sensible defaults

---

⚠️ Concerns Identified

1. Test Data Cleanup — Medium Risk

The payinConnector.spec.ts creates real connectors via createStripeConnectorAPI() but doesn't appear to have teardown logic in the diff. Tests that leave connector configurations behind can pollute the test environment over time.

Suggestion: Add afterEach or afterAll hook to clean up created connectors using DELETE /account/:accountId/connectors/:connectorId.

2. Plaid Authentication Token — Security Question

The connector setup tests Bank Debit with Plaid authenticator. Confirm that:

• No real Plaid credentials are hardcoded
• Any sandbox/mock tokens used are explicitly marked as test-only

3. Flaky Selector Pattern — Minor

In globalSearch.spec.ts, the mock response structure relies on specific indices:

const VIEW_RESULTS_DISPUTE_HIT = {...};

The test assumes this exact structure matches the backend contract. If the search response format changes, these tests will break without clear indication of what's wrong.

Suggestion: Add a schema validation step or document the expected response structure in comments.

---

💡 Suggestions

1. Add connector cleanup: Ensure payin connector tests clean up after themselves
2. Document mock structure: Add brief comment explaining why 11 results triggers "View X results" link
3. Consider test parallelization: The new connector tests appear isolated per test case — verify they can run in parallel

---

Pre-existing Issues (unchanged by this PR)

None introduced.

---

Verdict: ⚠️ Comment — Minor concerns around test cleanup and documentation. Consider addressing before merge.