Fixes #39439 - replace AuditsPage tests with RTL testing

[andreilakatos]

Fixes #39439

Summary

Replaces brittle snapshot-based tests for AuditsPage with React Testing Library (RTL) behavioral tests, and fixes the shared renderWithStoreAndI18n / renderWithI18n helpers so they can be used as the standard test setup going forward.

Changes

AuditsPage tests (AuditsPage.test.js)

• Removed snapshot tests and the 311-line AuditsPage.test.js.snap file.
• Added RTL tests that assert on rendered output instead of component trees:
  • Happy path with audit rows, documentation link, search input, and host data
  • Empty data state (no audit rows, data list still present)
  • Loading state (no data list, no error)
  • Error state with a generic message
  • Error state with a "no audits" message (restores coverage dropped during the migration)
• Uses rtlHelpers.renderWithStoreAndI18n() for Redux + i18n wrapping.
• Centralizes the i18n readiness wait in renderAuditsPage() via await intl.ready, so individual tests can use synchronous RTL queries afterward.

Fixtures (AuditsPage.fixtures.js)

• Adds default audits and itemCount to auditsPageProps, since they are required by AuditsTable and should not need to be repeated in every test.

RTL test helpers (rtlTestHelpers.js)

• Fixes renderWithI18n() and renderWithStoreAndI18n(), which previously treated i18nProviderWrapperFactory() as a React component:

  // Before (broken)
  const IntlWrapper = i18nProviderWrapperFactory(mockDate, timezone);
  React.createElement(IntlWrapper, {}, component);

[ofedoren]

Review: Fixes #39439 - replace AuditsPage tests with RTL testing

Good direction — snapshot tests were brittle and the 311-line snap file was pure noise. A few things to address:

Use rtlHelpers.renderWithStoreAndI18n() instead of manual i18n setup

The current approach:

const I18nProvider = i18nProviderWrapperFactory(new Date(), 'UTC')(
  ({ children }) => children
);

works accidentally — i18nProviderWrapperFactory returns an HOC, not a provider. It also introduces an async intl.ready gate, which is why every test needs await screen.findByText('Audits') even though AuditsPage renders synchronously.

rtlHelpers.renderWithStoreAndI18n() already handles both Redux and i18n wrapping and is the established pattern in this codebase. Using it would simplify renderAuditsPage and likely remove the need for async queries.

Prefer RTL queries over document.getElementById

expect(document.getElementById('audits-empty-table')).toBeInTheDocument();

AuditsLoadingPage only renders a <div id="..."> with skeletons inside — no text, roles, or labels — so there's no clean RTL query available. Two options:

1. Add data-testid="audits-loading" to AuditsLoadingPage and use screen.getByTestId()
2. Assert the absence of the data list instead: expect(screen.queryByRole('list', { name: 'Audits data list' })).not.toBeInTheDocument() (which the test already does)

Option 2 requires no component changes.

Dropped test scenario

The old "render audits page w/empty audits" fixture was { hasError: true, message: 'no audits' } — an error state. The new "renders audits page with no audit rows" test uses { hasError: false, hasData: true, audits: [], itemCount: 0 } — a legitimate empty-data state, which is a different withRenderHandler branch. Both are worth testing; the error-with-"no audits" case is currently missing.

Consider adding audits and itemCount to the default fixture

auditsPageProps lacks audits and itemCount, which are required props for AuditsTable. The happy-path test adds them manually, but the loading and error tests rely on withRenderHandler short-circuiting before AuditsTable mounts. Adding sensible defaults to auditsPageProps would make the tests less coupled to the HOC branching order.

---

Generated by Claude Opus 4.6

[andreilakatos]

Done @ofedoren
Had to fix renderWithStoreAndI18n though.

[ofedoren]

Re-review after update

All four points from the previous review are addressed — renderWithStoreAndI18n adopted, document.getElementById removed, dropped error scenario restored, defaults added to fixture. The rtlTestHelpers.js fix is also correct — the old renderWithI18n and renderWithStoreAndI18n were passing the HOC function directly to React.createElement instead of calling it with a component first. Two remaining items:

intl.ready handling should move into renderWithStoreAndI18n

The test imports intl directly and wraps every render with:

await act(async () => {
  await intl.ready;
});

Since renderWithStoreAndI18n now correctly uses the i18n wrapper (which gates on intl.ready internally), this async wait is a concern of the helper, not of each individual test. Moving the act + intl.ready await into renderWithStoreAndI18n (and making it async) would prevent every future test from having to repeat this boilerplate.

Two error tests cover the same code path

'renders audits page with empty audits error' and 'renders audits page with error message' both pass { hasError: true, message: { type: 'error', text: '...' } } — they hit the same withRenderHandler branch and the same EmptyPage rendering with different text strings. One of these is enough; consider dropping the duplicate and keeping the one with the more descriptive name.

---

Generated by Claude Opus 4.6

[Lukshio]

lgtm from my side, any objections @ofedoren ?

[MariaAga]

Tests look good to me, @andreilakatos Can you squash please?

[Lukshio]

lgtm

[Lukshio]

Thanks @andreilakatos