Graylog2 / Graylog2/graylog2-server

Collectors frontend tests: mock hooks per file and stop stubbing sibling components

Open
#27,324 0 comments 0 reactions 0 assignees View on GitHub
bug
Dominant language
Java
Stars
8.1k
Forks
1.1k
Avg merge
1d 20h
Merged PRs (30d)
217

Description

## Motivation

Review of #27252 (https://github.com/Graylog2/graylog2-server/pull/27252#discussion_r3935079085 and #discussion_r3934950686) flagged two patterns in the collectors test suite that hide runtime errors and make tests brittle:

1. Mocking the hooks index (`components/collectors/hooks`) instead of individual hook files.
2. Replacing collectors-owned components with stubs, so an error inside the real component is never caught.

Both patterns are module-wide, not specific to that PR, so they are addressed here in one pass. Rules: `graylog2-web-interface/CONTRIBUTING.md`, sections "Mocking" and "Assertions and Queries".

## Current state (46 test files, 126 `jest.mock` calls)

### Hook mocking, three coexisting styles

Whole-index automock, `jest.mock('../hooks')`, 5 files:
- components/collectors/settings/CollectorsSettings.test.tsx
- components/collectors/deployment/DeployTab.test.tsx
- components/collectors/overview/onboarding/IngestEndpointStrip.test.tsx
- components/collectors/overview/FirstOnboarding.test.tsx
- pages/CollectorsOverviewPage.test.tsx

Index factory mock (partial module, every unlisted hook becomes `undefined`), 11 files:
- components/collectors/fleets/FleetDetail.test.tsx
- components/collectors/fleets/CollectorsFleets.test.tsx
- components/collectors/instances/BulkActions.test.tsx
- components/collectors/instances/CollectorsInstances.test.tsx
- components/collectors/instances/InstanceDetailDrawer.test.tsx
- components/collectors/overview/RecentActivity.test.tsx
- components/collectors/overview/CollectorsOverview.test.tsx
- pages/CollectorsDeploymentPage.test.tsx
- pages/CollectorsFleetsPage.test.tsx
- pages/CollectorsInstancesPage.test.tsx
- pages/CollectorsFleetDetailPage.test.tsx

Per-hook-file mock (target style), roughly 20 files already do this, e.g. TokenStep, EnrollingHostsList, ReassignFleetModal, InstanceActions, EnrollmentTokenList, ConnectionSuccess.

### Stubs of collectors-owned components (12 stubs, 11 files)

| Test | Stubbed component(s) |
|---|---|
| components/collectors/overview/CollectorsOverview.test.tsx | RecentActivity, FirstOnboarding |
| components/collectors/overview/FirstOnboarding.test.tsx | WaitingForConnection |
| components/collectors/fleets/FleetDetail.test.tsx | InstanceDetailDrawer |
| components/collectors/instances/BulkActions.test.tsx | ReassignFleetModal |
| components/collectors/instances/InstanceActions.test.tsx | ReassignFleetModal |
| pages/CollectorsOnboardingInstancePage.test.tsx | ConnectionSuccess |
| pages/CollectorsDeploymentPage.test.tsx | DeployTab, EnrollmentTokenList |
| pages/CollectorsFleetsPage.test.tsx | CollectorsFleets |
| pages/CollectorsInstancesPage.test.tsx | CollectorsInstances |
| pages/CollectorsFleetDetailPage.test.tsx | FleetDetail |
| pages/CollectorsOverviewPage.test.tsx | CollectorsOverview |

### Stubs of shared components (out of scope unless agreed otherwise)

- `components/common/PaginatedEntityTable`: FleetDetail, CollectorsInstances, CollectorsFleets tests
- `components/common/CreateButton`: CollectorsFleetsPage test (precedent: PipelinesOverviewPage.test.tsx)
- `components/common/Tooltip` passthrough: DeployTab, InstallCommand, FirstOnboarding tests

## Proposed work

### Part 1: hook mocks (mechanical)

- Replace all 16 index mocks with per-file mocks, e.g. `jest.mock('components/collectors/hooks/useCollectorsConfig')`. The index re-exports resolve to the mocked module, so components importing from `../hooks` need no change.
- Where a factory currently lists hooks, mock each listed hook's file with automock and set return values via `asMock(...)` in `beforeEach`.
- Reuse `components/collectors/testing/{fixtures,mockMutations,mockPermissions}.ts` for return values.

### Part 2: component stubs (needs a rule first)

Proposed rule, to be confirmed in review:
- A component test renders its real children and mocks their hooks. No stubs of collectors-owned components.
- A page test that only asserts the permission/redirect gate may stub the page content, since the content has its on the page renders for real.

Under that rule:
- Remove: RecentActivity, FirstOnboarding (in CollectorsOverview.test), WaitingForConnection, InstanceDetailDrawer, ReassignFleetModal (x2), ConnectionSuccess stubs. Mock their hooks instead.
- Keep, with a comment stating the rule: page-content stubs in CollectorsDeploymentPage, CollectorsFleetsPage, CollectorsInstancesPage, CollectorsFleetDetailPage, CollectorsOverviewPage tests.
- Decide separately whether `PaginatedEntityTable` stubs stay; it is stubbed in other modules too.

## Acceptance

- No `jest.mock('<...>/hooks')` or `jest.mock('components/collectors/hooks', ...)` left in `src/components/collectors*`.
- Component tests stub no collectors-owned components.
- Remaining page-content stubs carry a one-line comment referencing this issue.
- `yarn test src/components/collectors src/pages/Collectors` green; `yarn tsc` and eslint clean.

## Notes

- Removing the strip stub in #27252 already exposed one wrong assertion the stub had hidden (configured cluster without input shows "No ingest input exists", not the confirm title). Expect more of these.
- Related: #27322 (strip not shown for a failed input) may add another consumer of IngestEndpointStrip

Contributor guide

Open the contributing guide

Research direction

Start with graylog2-web-interface/CONTRIBUTING.md sections “Mocking” and “Assertions and Queries”, then review the listed collector tests and existing per-hook examples. Run the collector test command to identify failures while replacing index mocks and collectors-owned component stubs. Done means the stated mock patterns and stubs are removed or documented, and the collector tests, tsc, and eslint pass.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
frontend, testing
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.