test(ui): drop presence-only and implementation-coupled test assertions - #7065
Open
luizhf42 wants to merge 1 commit into
Open
test(ui): drop presence-only and implementation-coupled test assertions#7065luizhf42 wants to merge 1 commit into
luizhf42 wants to merge 1 commit into
Conversation
Code Review CompleteThe 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 |
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
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.
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
OnlineDothad adata-testidthat existed solely so its own test could find it; the mock and the assertions went together.usePaginatedListStatere-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 — whichpaginatedListParams,usePaginatedListStateanduseDevicesalready cover between them. What each page owns is kept: the status it forces or honours, its tabs, its row-click route, thekey.prefix keeping two lists on the Team page apart, and the page decrement when the last row on a page is deleted.it.each— status chips and badges, error-status to message maps, enabled/disabled form states, and theLicenseBannerseverity table that had every license state written out twice. Genuine conditional branches were kept even where they resemble the cut shapes.MfaEnableDrawerstate cleanup: the test closed the drawer by flipping theopenprop, which the component does not watch — the reset runs fromhandleClose, 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):CreateNamespaceandCreateNamespaceDialogheld byte-identical name state, validation and status-to-message mapping, differing only in theonClose()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-useCloudFormwrapper was inlined.useTableSortand 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 inDevices.test.tsxthat 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)inhandleClosefails the newMfaEnableDrawertest, and replacingonSortChange: () => 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.tsxstill has no page test file (pre-existing — it consumesusePaginatedListStatebut nothing renders it), and colour-class assertions inPlatformBadge/ActiveBadge/FilterBadgewere folded intoit.eachrather than deleted, sinceFilterBadgecarries comments saying they guard a non-lossless inline style.