Skip to content

test(ui): drop presence-only and implementation-coupled test assertions - #7065

Open
luizhf42 wants to merge 1 commit into
feat/ui/orvalfrom
test/ui/remove-low-value-tests
Open

test(ui): drop presence-only and implementation-coupled test assertions#7065
luizhf42 wants to merge 1 commit into
feat/ui/orvalfrom
test/ui/remove-low-value-tests

Conversation

@luizhf42

@luizhf42 luizhf42 commented Sep 9, 2026

Copy link
Copy Markdown
Member

What

Cuts the console's unit suite from 2939 to 2346 tests by removing assertions that check an element exists rather than that the code does something, and extracts one duplicated form controller that was blocking a deletion.

Why

E2E coverage is coming, and it subsumes the "does this element render" layer these tests spent most of their assertions on. Keeping both means paying twice for the same signal, and the unit copy is the one that breaks on markup changes without catching bugs.

Changes

  • presence-only assertions: removed static checks on headings, column headers, and buttons the very next test clicks. A leaf presentational component's presence test is its contract, so those stayed — the cut is for pages and containers where rendering is incidental to behaviour.
  • assertions against mocked children: tests that mocked a child and then asserted the mock rendered were verifying the mock, not the component. OnlineDot had a data-testid that existed solely so its own test could find it; the mock and the assertions went together.
  • usePaginatedListState re-tested through the DOM: every page backed by the hook re-tested it — page and search hydrated from the URL, defaults, invalid values falling back, sort toggling, clicking Next — which paginatedListParams, usePaginatedListState and useDevices already cover between them. What each page owns is kept: the status it forces or honours, its tabs, its row-click route, the key. prefix keeping two lists on the Team page apart, and the page decrement when the last row on a page is deleted.
  • merged rather than deleted: tests sharing a render were folded into one, and cases differing only in a literal became it.each — status chips and badges, error-status to message maps, enabled/disabled form states, and the LicenseBanner severity table that had every license state written out twice. Genuine conditional branches were kept even where they resemble the cut shapes.
  • MfaEnableDrawer state cleanup: the test closed the drawer by flipping the open prop, which the component does not watch — the reset runs from handleClose, off the Cancel button. It therefore asserted step 2 was still showing, under a name promising a reset. It now cancels the way a user does and asserts the rewind to step 1.
  • useNamespaceCreateForm (the one production change): CreateNamespace and CreateNamespaceDialog held byte-identical name state, validation and status-to-message mapping, differing only in the onClose() after success, so neither file's error tests could go without leaving live code uncovered. Both now share the hook, one hook test covers the status table, and each component keeps only what it owns. Its single-use CloudForm wrapper was inlined.
  • useTableSort and sort/pagination wiring: the hook had no test at all, and nothing asserted a page wires a column header to it. Added a hook test for the toggle rules, plus one page-level test in Devices.test.tsx that pages forward, sorts, and asserts the outgoing request — one file, not repeated per page.

Testing

Two behavioural claims here were previously green and wrong, so both were mutation-checked rather than trusted: breaking setStep(1) in handleClose fails the new MfaEnableDrawer test, and replacing onSortChange: () => setPage(1) with a no-op fails the new devices test. Worth re-running if you touch either.

The deliberate gaps, so they're a decision and not an oversight: install-keys/index.tsx still has no page test file (pre-existing — it consumes usePaginatedListState but nothing renders it), and colour-class assertions in PlatformBadge/ActiveBadge/FilterBadge were folded into it.each rather than deleted, since FilterBadge carries comments saying they guard a non-lossless inline style.

@luizhf42 luizhf42 self-assigned this Sep 9, 2026
@luizhf42
luizhf42 requested a review from a team as a code owner September 9, 2026 21:25
@luizhf42
luizhf42 added this pull request to stack #7066 September 9, 2026 21:25
@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown

Code Review Complete

The automated review ran but did not post an updated summary — this usually means no new issues were found since the previous review. If you've pushed changes and want a fresh pass, comment /review.

View job

@otavio otavio added this to the 0.27.1 milestone Sep 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants