CONSOLE-5176: Refactor RTL Tests from fireEvent to userEvent#16294
Open
cajieh wants to merge 1 commit intoopenshift:mainfrom
Open
CONSOLE-5176: Refactor RTL Tests from fireEvent to userEvent#16294cajieh wants to merge 1 commit intoopenshift:mainfrom
cajieh wants to merge 1 commit intoopenshift:mainfrom
Conversation
openshift-ci-robot
added
the
jira/valid-reference
Contributor
|
@cajieh: This pull request references CONSOLE-5176 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set. DetailsIn response to this: Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
openshift-ci
bot
added
component/core
cajieh
force-pushed
the
refactor-rtl-ests-fire-event-user-event
branch
from
c8f15f8 to
2b14b5d
Compare
Contributor
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cajieh The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
Contributor
📝 WalkthroughWalkthroughThis pull request systematically migrates test suites across the OpenShift Console frontend from 🚥 Pre-merge checks | ✅ 10✅ Passed checks (10 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
|
@cajieh: This pull request references CONSOLE-5176 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/public/components/modals/__tests__/impersonate-user-modal-integration.spec.tsx (1)
236-247:⚠️ Potential issue | 🟠 MajorAvoid conditional no-op in deselection assertion path.
Line 237 gates the core behavior under
if (closeButtons.length > 0), so the test can pass even when deselection UI is broken. Assert presence, then always execute deselect path.Proposed fix
const closeButtons = document.querySelectorAll('.pf-v6-c-label__actions button'); - if (closeButtons.length > 0) { - await act(async () => { - await user.click(closeButtons[0]); - }); - - await waitFor(() => { - const chips = document.querySelectorAll('.pf-v6-c-label'); - expect(chips.length).toBe(0); - }); - } + expect(closeButtons.length).toBeGreaterThan(0); + await user.click(closeButtons[0]); + + await waitFor(() => { + const chips = document.querySelectorAll('.pf-v6-c-label'); + expect(chips.length).toBe(0); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/public/components/modals/__tests__/impersonate-user-modal-integration.spec.tsx` around lines 236 - 247, The test currently short-circuits deselection when no close button exists; change the flow to explicitly assert the presence of the close button and then always run the deselect path: locate the closeButtons using document.querySelectorAll('.pf-v6-c-label__actions button') (as assigned to closeButtons), add an expectation like expect(closeButtons.length).toBeGreaterThan(0) to fail fast if UI is broken, then always run the act(async () => await user.click(closeButtons[0])) and the subsequent waitFor that asserts chips length is 0 so the test cannot pass silently when deselection UI is missing.
🧹 Nitpick comments (15)
frontend/public/components/utils/__tests__/kebab.spec.tsx (1)
19-67: Optional: reduce repeateduserEvent.setup()boilerplate.You can trim repetition by initializing
userin abeforeEachinside eachdescribeblock.♻️ Small cleanup example
describe('KebabItem', () => { + let user: ReturnType<typeof userEvent.setup>; + beforeEach(() => { + user = userEvent.setup(); + }); + it('should disable option without callback / href (i.e. option does nothing)', async () => { - const user = userEvent.setup(); const nothingOption = { ...mockOption, href: undefined }; const trackOnClick = jest.fn(); const renderItem = render(<KebabItem onClick={trackOnClick} option={nothingOption} />); await user.click(renderItem.getByRole('menuitem'));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/public/components/utils/__tests__/kebab.spec.tsx` around lines 19 - 67, Refactor the repeated userEvent.setup() calls by creating a shared `let user: ReturnType<typeof userEvent.setup>` and initializing it in a beforeEach within each describe block that contains the tests (e.g., the describe for KebabItem_ and the describe for KebabItemAccessReview_); then remove the per-test `const user = userEvent.setup()` lines so tests that call user.click(...) use the beforeEach-initialized `user` instead (update any imports/types if needed and keep test logic in KebabItem, KebabItemAccessReview_, and related mocks unchanged).frontend/packages/dev-console/src/components/user-preferences/__tests__/SecureRouteFields.spec.tsx (1)
60-72: Nice consistency in the second dropdown test; consider a small helper to reduce duplication.Both dropdown-behavior tests repeat the same setup/click flow; extracting a tiny helper would improve maintainability.
♻️ Optional refactor
+const openInsecureTrafficDropdown = async () => { + const user = userEvent.setup(); + await user.click(screen.getByTestId('insecure-traffic')); +}; it('should not show Allow option in Insecure traffic dropdown if TLS termination is Passthrough', async () => { - const user = userEvent.setup(); mockUsePreferredRoutingOptions.mockReturnValue([ { secure: true, tlsTermination: 'passthrough', insecureTraffic: 'Redirect', }, () => {}, true, ]); render(<SecureRouteFields />); - const inSecureTraffic = screen.getByTestId('insecure-traffic'); - await user.click(inSecureTraffic); + await openInsecureTrafficDropdown(); it('should show Allow, None and Redirect options in Insecure traffic dropdown if TLS termination is not Passthrough', async () => { - const user = userEvent.setup(); mockUsePreferredRoutingOptions.mockReturnValue([ { secure: true, tlsTermination: 'edge', insecureTraffic: 'Redirect', }, () => {}, true, ]); render(<SecureRouteFields />); - const inSecureTraffic = screen.getByTestId('insecure-traffic'); - await user.click(inSecureTraffic); + await openInsecureTrafficDropdown();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/packages/dev-console/src/components/user-preferences/__tests__/SecureRouteFields.spec.tsx` around lines 60 - 72, Extract a small test helper to encapsulate the repeated setup/click flow used in SecureRouteFields tests: move the userEvent.setup(), mockUsePreferredRoutingOptions.mockReturnValue([...]), render(<SecureRouteFields />) and the await user.click(screen.getByTestId('insecure-traffic')) into a helper function (e.g., renderAndClickInsecureTraffic or setupSecureRouteFields) and replace the duplicated blocks in both dropdown-behavior tests with calls to that helper; keep references to mockUsePreferredRoutingOptions, SecureRouteFields, and the 'insecure-traffic' testId so the behavior remains identical.frontend/packages/operator-lifecycle-manager/src/components/modals/__tests__/installplan-approval-modal.spec.tsx (1)
107-118: Decouple this test from shared fixture defaults.Line 108 relies on
testInstallPlan’s current default approval. If that fixture ever defaults toManual, Line 116 won’t change state and Line 118 may not exercise submit due to disabled Save.Suggested hardening
it('calls k8sUpdate with updated install plan when form is submitted', async () => { const user = userEvent.setup(); const installPlan = _.cloneDeep(testInstallPlan); + installPlan.spec.approval = InstallPlanApproval.Automatic; mockModelFor.mockReturnValue(InstallPlanModel);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/packages/operator-lifecycle-manager/src/components/modals/__tests__/installplan-approval-modal.spec.tsx` around lines 107 - 118, The test depends on the shared fixture testInstallPlan default approval; make the test explicit by mutating the cloned installPlan before rendering so its approval is not already "Manual" (e.g. set installPlan.spec.approval = 'Automatic' or the appropriate ApprovalAutomatic constant) so that clicking the Manual radio (InstallPlanApprovalModal) actually changes state and enables the Save button; keep using the cloned installPlan and the same installPlanApprovalModalProps when calling renderWithProviders.frontend/public/components/modals/__tests__/configure-update-strategy-modal.spec.tsx (1)
69-88: Deduplicate the RollingUpdate test harness.The same harness is defined twice; extracting a shared helper will reduce drift and keep these tests easier to maintain.
♻️ Proposed refactor
+ const renderRollingUpdateInputsHarness = () => { + const RollingUpdateInputsHarness = () => { + const [maxUnavailable, setMaxUnavailable] = useState('25%'); + const [maxSurge, setMaxSurge] = useState('25%'); + return ( + <ConfigureUpdateStrategy + {...mockProps} + strategyType="RollingUpdate" + maxUnavailable={maxUnavailable} + maxSurge={maxSurge} + onChangeMaxUnavailable={(v) => { + setMaxUnavailable(String(v)); + mockProps.onChangeMaxUnavailable(v); + }} + onChangeMaxSurge={(v) => { + setMaxSurge(String(v)); + mockProps.onChangeMaxSurge(v); + }} + /> + ); + }; + + renderWithProviders(<RollingUpdateInputsHarness />); + }; it('calls onChangeMaxUnavailable when input changes', async () => { const user = userEvent.setup(); - const RollingUpdateInputsHarness = () => { ... }; - renderWithProviders(<RollingUpdateInputsHarness />); + renderRollingUpdateInputsHarness(); const maxUnavailableInput = screen.getByTestId('max-unavailable-input'); await user.clear(maxUnavailableInput); await user.type(maxUnavailableInput, '50%'); await waitFor(() => { expect(mockProps.onChangeMaxUnavailable).toHaveBeenLastCalledWith('50%'); }); }); it('calls onChangeMaxSurge when input changes', async () => { const user = userEvent.setup(); - const RollingUpdateInputsHarness = () => { ... }; - renderWithProviders(<RollingUpdateInputsHarness />); + renderRollingUpdateInputsHarness(); const maxSurgeInput = screen.getByTestId('max-surge-input'); await user.clear(maxSurgeInput); await user.type(maxSurgeInput, '1'); await waitFor(() => { expect(mockProps.onChangeMaxSurge).toHaveBeenLastCalledWith('1'); }); });Also applies to: 105-124
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/public/components/modals/__tests__/configure-update-strategy-modal.spec.tsx` around lines 69 - 88, The RollingUpdateInputsHarness component is duplicated; extract it into a single reusable helper used by both tests to avoid drift: create one shared RollingUpdateInputsHarness function that initializes maxUnavailable and maxSurge via useState, calls setMaxUnavailable/setMaxSurge and mockProps.onChangeMaxUnavailable/onChangeMaxSurge in the same way, and render ConfigureUpdateStrategy with strategyType="RollingUpdate" and the same props; replace both inline duplicates (the blocks using ConfigureUpdateStrategy with maxUnavailable/maxSurge and onChange handlers) with calls to this shared helper so both tests use the identical component.frontend/packages/console-shared/src/components/getting-started/__tests__/GettingStartedCard.spec.tsx (2)
60-61: Make click-handler assertions strict.Lines 61 and 78 only assert “called”. Using exact call counts better detects accidental double invocation.
Stricter interaction assertions
- expect(onClick).toHaveBeenCalled(); + expect(onClick).toHaveBeenCalledTimes(1);Also applies to: 77-78
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/packages/console-shared/src/components/getting-started/__tests__/GettingStartedCard.spec.tsx` around lines 60 - 61, The test currently asserts only that the click handler was called; change these to strict call-count assertions so accidental double-invocations are caught—replace expect(onClick).toHaveBeenCalled() with expect(onClick).toHaveBeenCalledTimes(1) for the user.click() interaction on screen.getByTestId('item link-1') and the other similar interaction (the second occurrence around lines 77-78), ensuring both click assertions check for exactly one invocation of the onClick mock.
32-35: Use a semantic heading assertion for the title.Line 34 uses
getByText, which won’t catch heading-level regressions. Prefer querying by role/level for stronger a11y coverage.Refactor assertion
- expect(screen.getByText('Test Card')).toBeInTheDocument(); + expect(screen.getByRole('heading', { level: 3, name: 'Test Card' })).toBeInTheDocument();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/packages/console-shared/src/components/getting-started/__tests__/GettingStartedCard.spec.tsx` around lines 32 - 35, Replace the title assertion that uses screen.getByText('Test Card') in the GettingStartedCard test with a semantic heading query to catch heading-level regressions; specifically, change it to use screen.getByRole('heading', { name: 'Test Card', level: 2 }) (or the correct heading level used by the GettingStartedCard component) so the test asserts both accessible role and label instead of plain text.frontend/packages/container-security/src/components/__tests__/ImageVulnerabilityToggleGroup.spec.tsx (1)
32-39: Use an async query after the click to reduce flake risk.After the
await user.click()at line 37, UI updates can be asynchronous. The synchronousscreen.getByTextat line 39 can race with these updates in slower environments. Useawait screen.findByText()instead for resilience.Proposed change
- expect(screen.getByText(/No.*app dependency.*vulnerabilities/i)).toBeVisible(); + expect(await screen.findByText(/No.*app dependency.*vulnerabilities/i)).toBeVisible();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/packages/container-security/src/components/__tests__/ImageVulnerabilityToggleGroup.spec.tsx` around lines 32 - 39, In the ImageVulnerabilityToggleGroup test "should render empty state when switching to App dependency with no vulnerabilities", replace the synchronous assertion that uses screen.getByText after await user.click(appDependencyButton) with an asynchronous query (await screen.findByText) so the test waits for the UI update; update the assertion to await screen.findByText(/No.*app dependency.*vulnerabilities/i) and assert visibility on the resolved element.frontend/packages/console-app/src/components/modals/resource-limits/__tests__/ResourceLimitsModal.spec.tsx (2)
64-87: Add one negative-path test for Save disablement guard.Current coverage verifies submit/cancel happy paths, but not the guard driven by
errors/isSubmitting. A focused case prevents regressions in modal submit protection.Suggested test addition
+ it('does not submit when Save is disabled due to validation errors', async () => { + const user = userEvent.setup(); + const handleSubmit = jest.fn(); + renderWithProviders( + <ResourceLimitsModal + {...formProps} + handleSubmit={handleSubmit} + errors={{ cpuLimit: 'Required' }} + isSubmitting={false} + />, + ); + + const saveButton = screen.getByRole('button', { name: 'Save' }); + expect(saveButton).toBeDisabled(); + await user.click(saveButton); + expect(handleSubmit).not.toHaveBeenCalled(); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/packages/console-app/src/components/modals/resource-limits/__tests__/ResourceLimitsModal.spec.tsx` around lines 64 - 87, Add a negative-path test to ResourceLimitsModal that verifies the Save button is disabled and does not call handleSubmit when the form is guarded by validation or submission state: render <ResourceLimitsModal {...formProps} /> with formProps adjusted to include a non-empty errors object or isSubmitting: true, assert screen.getByRole('button', { name: 'Save' }) is disabled (or has aria-disabled) and then attempt a user.click and expect formProps.handleSubmit not toHaveBeenCalled; use the existing formProps, userEvent.setup(), and screen helpers to mirror the other tests.
22-61: Usesatisfiesinstead ofas Propsto enforce stricter type checking and prevent unused fixture fields.At Line 61,
as Propsbypasses excess-property checks, allowing the largeresourcefixture block (Lines 26–60) to persist even if the component'sResourceLimitsModalPropsinterface doesn't require it. The component only declares{ cancel?: () => void }, soresourceis extraneous.Replace the type assertion with
satisfies ResourceLimitsModalProps. This will catch excess properties at compile time and prevent fixture drift:Refactor
- formProps = { + formProps = { ...formikFormProps, isSubmitting: false, cancel: jest.fn(), - resource: { - apiVersion: 'apps/v1', - kind: 'Deployment', - metadata: { - name: 'xyz-deployment', - }, - spec: { - selector: { - matchLabels: { - app: 'hello-openshift', - }, - }, - replicas: 1, - template: { - metadata: { - labels: { - app: 'hello-openshift', - }, - }, - spec: { - containers: [ - { - name: 'hello-openshift', - image: 'openshift/hello-openshift', - ports: [ - { - containerPort: 8080, - }, - ], - }, - ], - }, - }, - }, - }, - } as Props; + } satisfies ResourceLimitsModalProps;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/packages/console-app/src/components/modals/resource-limits/__tests__/ResourceLimitsModal.spec.tsx` around lines 22 - 61, The test uses a wide fixture typed with "as Props" which suppresses excess-property checks; replace the trailing "as Props" on the formProps object with "satisfies ResourceLimitsModalProps" (reference: formProps and ResourceLimitsModalProps) so TypeScript will flag extraneous fields like the large resource block and force you to either remove or trim it; ensure the cancel property remains (cancel: jest.fn()) and run typechecking to fix any reported excess properties.frontend/public/components/modals/__tests__/impersonate-user-modal-integration.spec.tsx (1)
64-70: Remove manualact()wrappers aroundrender()and user interaction calls.
@testing-library/reactv16 +@testing-library/user-eventv14.6.1 handle async operations internally. Manualact()wrapping is redundant, adds noise, and makes tests harder to maintain. RTL'srender()already handles the necessaryact()wrapping, anduserEventmethods integrate seamlessly without explicit wrappers.Applies to: Lines 64–70, 96–102, 110–112, 121–123, 142–148, 157–159, 167–169, 179–181, 202–208, 216–218, 225–227, 238–240, 261–267, 272–274, 282–284, 296–302, 307–309, 317–319, 332–338, 370–376, 379–385, 396–402 in
frontend/public/components/modals/__tests__/impersonate-user-modal-integration.spec.tsx🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/public/components/modals/__tests__/impersonate-user-modal-integration.spec.tsx` around lines 64 - 70, Remove the manual act(...) wrappers around render(...) and userEvent calls in the ImpersonateUserModal tests: find occurrences of act(async () => { render(...); }) and act(async () => { await userEvent.*(...) }) in impersonate-user-modal-integration.spec.tsx and replace them with direct calls to render(...) and await userEvent.*(...) respectively (keep async/await on userEvent where needed); rely on `@testing-library/react` and `@testing-library/user-event` to handle act for you and do not import or use act around render, user interactions, or assertions involving the ImpersonateUserModal, mockStore, onClose, and onImpersonate test helpers.frontend/packages/knative-plugin/src/components/sink-source/__tests__/SinkSourceModal.spec.tsx (2)
12-18: Keep theButtonmock complete to preserve secondary button test coverage.The component renders a cancel button (line 67 of SinkSourceModal.tsx) with
type="button", but the current mock returnsnullfor non-submit buttons. This gaps test visibility: the cancel button's existence, visibility, andonClickhandler cannot be asserted in tests, creating regression risk for secondary actions.♻️ Proposed refactor
- Button: jest.fn(({ children, type, ...props }) => - type === 'submit' ? ( - <button type="submit" {...props}> - {children} - </button> - ) : null, - ), + Button: jest.fn(({ children, type = 'button', ...props }) => ( + <button type={type} {...props}> + {children} + </button> + )),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/packages/knative-plugin/src/components/sink-source/__tests__/SinkSourceModal.spec.tsx` around lines 12 - 18, The Button mock in SinkSourceModal.spec.tsx currently only renders when type === 'submit', hiding the modal's cancel button; update the mock implementation of Button (jest.fn(({ children, type, ...props }) => ...)) so it returns a real <button> element for non-submit types as well (e.g., when type === 'button' or undefined) forwarding children and props so tests can assert the cancel button's presence and onClick; ensure the mock still renders the submit button as before and preserves prop forwarding to keep all existing assertions working with SinkSourceModal's cancel button.
49-53: Consider enhancing therequestSubmitpolyfill to support thesubmitterparameter and dispatchSubmitEventinstead of plainEvent.The current polyfill works for this test's assertions, but the enhanced version ensures compatibility with form handlers that inspect
event.submitterto determine which button triggered submission—behavior that@testing-library/user-eventv14 expects when simulating submit button clicks. The improvement is backward-compatible (falls back to plainEventin unsupported environments) and aligns with HTML spec semantics.Enhanced polyfill for future-proof form testing
- HTMLFormElement.prototype.requestSubmit = function requestSubmitPolyfill( - this: HTMLFormElement, - ) { - this.dispatchEvent(new Event('submit', { bubbles: true, cancelable: true })); - }; + HTMLFormElement.prototype.requestSubmit = function requestSubmitPolyfill( + this: HTMLFormElement, + submitter?: HTMLElement | null, + ) { + const event = + typeof SubmitEvent === 'function' + ? new SubmitEvent('submit', { + bubbles: true, + cancelable: true, + submitter: submitter ?? undefined, + }) + : new Event('submit', { bubbles: true, cancelable: true }); + this.dispatchEvent(event); + };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/packages/knative-plugin/src/components/sink-source/__tests__/SinkSourceModal.spec.tsx` around lines 49 - 53, Update the requestSubmit polyfill (HTMLFormElement.prototype.requestSubmit / requestSubmitPolyfill) to accept an optional submitter parameter and dispatch a SubmitEvent with the submitter set when available; if the environment lacks SubmitEvent support, fall back to creating a plain Event but attach a submitter property so handlers relying on event.submitter (and tests using `@testing-library/user-event` v14) still work. Ensure you preserve bubbles/cancelable flags and maintain backward compatibility by only using SubmitEvent when supported.frontend/packages/console-shared/src/components/formik-fields/__tests__/NumberSpinnerField.spec.tsx (1)
6-20: Use role-based queries (getByRole) instead of test IDs for core interactive elements to strengthen a11y signal in tests.The input field and buttons should be queryable by role since PatternFly's
NumberInputrenders a proper<input type="number">(which has thespinbuttonrole) and buttons with accessible labels. Replace thegetByTestId('number-spinner-field')calls withgetByRole('spinbutton')and thegetByTestId('Increment'/'Decrement')calls withgetByRole('button', { name: /increment|decrement/i })to better detect accessibility regressions while keeping test IDs as a fallback for edge cases.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/packages/console-shared/src/components/formik-fields/__tests__/NumberSpinnerField.spec.tsx` around lines 6 - 20, Update the tests to use role-based queries: replace the getInput helper (currently using screen.getByTestId('number-spinner-field')) to use screen.getByRole('spinbutton') and, in the button checks inside the "Number Spinner Field" suite where screen.getByTestId('Increment') and 'Decrement' are used, replace them with screen.getByRole('button', { name: /increment/i }) and screen.getByRole('button', { name: /decrement/i }); keep using mockFormikRenderer(<NumberSpinnerField name="spinnerField" />) as-is and retain testId queries only as a fallback if role queries fail.frontend/packages/console-shared/src/components/formik-fields/key-value-file-input-field/__tests__/KeyValueFileInputField.spec.tsx (2)
46-49: Drop unnecessaryactwrapper arounduserEvent.At Line 46, wrapping awaited
user.click/user.tabinactis redundant with modernuserEventand can conflict withtesting-library/no-unnecessary-act.Proposed diff
-import { screen, render, act, waitFor } from '@testing-library/react'; +import { screen, render, waitFor } from '@testing-library/react'; @@ - await act(async () => { - await user.click(KeyField); - await user.tab(); - }); + await user.click(KeyField); + await user.tab();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/packages/console-shared/src/components/formik-fields/key-value-file-input-field/__tests__/KeyValueFileInputField.spec.tsx` around lines 46 - 49, Remove the unnecessary act wrapper around userEvent calls in the test: replace the block using act(async () => { await user.click(KeyField); await user.tab(); }) with direct awaited user calls (await user.click(KeyField); await user.tab();). Update the test in KeyValueFileInputField.spec.tsx where KeyField and the user helper are used, removing the import/usage of act for that interaction to satisfy testing-library/no-unnecessary-act and rely on userEvent's built-in async handling.
82-85: Prefer direct async queries overwaitFor+queryBy*for these cases.Lines 82/99/118 can be simplified for clearer intent and less polling noise: use direct
queryByTestIdfor absence andfindBy*/findAllBy*for presence/count.Proposed diff
- await waitFor(() => { - const removeButton = screen.queryByTestId('remove-key-value-button'); - expect(removeButton).toBeNull(); - }); + expect(screen.queryByTestId('remove-key-value-button')).toBeNull(); @@ - await waitFor(() => { - const removeButton = screen.queryByTestId('remove-key-value-button'); - expect(removeButton).not.toBeNull(); - }); + expect(await screen.findByTestId('remove-key-value-button')).not.toBeNull(); @@ - await waitFor(() => { - const keyValuePair = screen.queryAllByTestId('key-value-pair'); - expect(keyValuePair).toHaveLength(2); - }); + expect(await screen.findAllByTestId('key-value-pair')).toHaveLength(2);Also applies to: 99-102, 118-121
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/packages/console-shared/src/components/formik-fields/key-value-file-input-field/__tests__/KeyValueFileInputField.spec.tsx` around lines 82 - 85, Replace the waitFor + queryByTestId patterns with direct async queries: for assertions that something is absent (currently using await waitFor(() => { const removeButton = screen.queryByTestId('remove-key-value-button'); expect(removeButton).toBeNull(); })) remove the waitFor and just use const removeButton = screen.queryByTestId('remove-key-value-button'); expect(removeButton).toBeNull(); and for assertions that wait for presence or count (the other two occurrences) replace waitFor + queryBy*/getAllBy* with await screen.findByTestId('...') or await screen.findAllByTestId('...') respectively (use findByTestId for single-element presence and findAllByTestId for count checks) so tests use direct async queries instead of polling with waitFor.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@frontend/packages/console-shared/src/components/formik-fields/__tests__/NumberSpinnerField.spec.tsx`:
- Around line 10-47: The tests for NumberSpinnerField were narrowed to only type
and click flows and need to restore behavioral coverage: update
NumberSpinnerField.spec.tsx to re-add assertions that helper
text/aria-describedby wiring works (render with helperText prop and assert
getInput().getAttribute('aria-describedby') points to the helper element and
that the helper text is present), restore min/max behavior checks by rendering
NumberSpinnerField/NumberSpinner with min and max props and asserting values
clamp at min/max when typing, clearing, or using Increment/Decrement, and verify
step behavior and default value handling (render with initial value or empty
state and assert expected initial value like '0'); use existing helpers
(mockFormikRenderer, getInput, screen.getByTestId('Increment'|'Decrement')) and
keep async userEvent flows in act/waitFor as in other tests.
In
`@frontend/packages/console-shared/src/components/getting-started/__tests__/GettingStartedCard.spec.tsx`:
- Around line 7-94: Tests for GettingStartedCard lost coverage of key behaviors:
conditional rendering branches and external-link hardening attributes. Update
the test suite for GettingStartedCard to (1) assert that external links (links
with external: true) render rel="noopener noreferrer" and target="_blank" (use
the link with id 'link-2'), (2) add assertions for conditional rendering paths
such as when a link has loading: true vs has href/onClick (use 'loading-link'
and the existing onClick cases), and (3) verify that internal vs external link
markup differs as expected; reference test utilities like renderWithProviders,
screen.getByTestId('item link-2'), screen.getByTestId('item loading-link'), and
the moreLink case to locate elements.
In
`@frontend/packages/knative-plugin/src/components/traffic-splitting/__tests__/TrafficSplittingModal.spec.tsx`:
- Around line 15-21: The mocked Button in TrafficSplittingModal.spec.tsx
currently returns null for non-submit types which hides the Cancel button from
tests; update the mock (the jest.fn for Button) to render a real <button> for
all types (or at least when type === 'button') preserving children and props so
Cancel/actions are present, e.g., return a button element that spreads props and
renders children; run the tests and update any assertions to check for the
Cancel button rendered by TrafficSplittingModal.tsx (the modal component that
renders Save type="submit" and Cancel type="button").
In
`@frontend/packages/webterminal-plugin/src/components/cloud-shell/__tests__/MultiTabbedTerminal.spec.tsx`:
- Around line 21-31: The helper clickMultipleTimes currently captures a single
HTMLElement and repeatedly clicks it, which can target a detached node when the
"Add new tab" button disappears; update clickMultipleTimes to accept a getter
(e.g., () => HTMLElement | null) or a selector and, inside the loop, query the
current element each iteration (using the provided getter or DOM query) and bail
out if it returns null, then perform the await act(() =>
user.click(currentElement)) only when the element is present; update callers in
the tests to pass a fresh-querying getter (or selector) instead of a captured
element so clicks stop gracefully when the button is removed.
---
Outside diff comments:
In
`@frontend/public/components/modals/__tests__/impersonate-user-modal-integration.spec.tsx`:
- Around line 236-247: The test currently short-circuits deselection when no
close button exists; change the flow to explicitly assert the presence of the
close button and then always run the deselect path: locate the closeButtons
using document.querySelectorAll('.pf-v6-c-label__actions button') (as assigned
to closeButtons), add an expectation like
expect(closeButtons.length).toBeGreaterThan(0) to fail fast if UI is broken,
then always run the act(async () => await user.click(closeButtons[0])) and the
subsequent waitFor that asserts chips length is 0 so the test cannot pass
silently when deselection UI is missing.
---
Nitpick comments:
In
`@frontend/packages/console-app/src/components/modals/resource-limits/__tests__/ResourceLimitsModal.spec.tsx`:
- Around line 64-87: Add a negative-path test to ResourceLimitsModal that
verifies the Save button is disabled and does not call handleSubmit when the
form is guarded by validation or submission state: render <ResourceLimitsModal
{...formProps} /> with formProps adjusted to include a non-empty errors object
or isSubmitting: true, assert screen.getByRole('button', { name: 'Save' }) is
disabled (or has aria-disabled) and then attempt a user.click and expect
formProps.handleSubmit not toHaveBeenCalled; use the existing formProps,
userEvent.setup(), and screen helpers to mirror the other tests.
- Around line 22-61: The test uses a wide fixture typed with "as Props" which
suppresses excess-property checks; replace the trailing "as Props" on the
formProps object with "satisfies ResourceLimitsModalProps" (reference: formProps
and ResourceLimitsModalProps) so TypeScript will flag extraneous fields like the
large resource block and force you to either remove or trim it; ensure the
cancel property remains (cancel: jest.fn()) and run typechecking to fix any
reported excess properties.
In
`@frontend/packages/console-shared/src/components/formik-fields/__tests__/NumberSpinnerField.spec.tsx`:
- Around line 6-20: Update the tests to use role-based queries: replace the
getInput helper (currently using screen.getByTestId('number-spinner-field')) to
use screen.getByRole('spinbutton') and, in the button checks inside the "Number
Spinner Field" suite where screen.getByTestId('Increment') and 'Decrement' are
used, replace them with screen.getByRole('button', { name: /increment/i }) and
screen.getByRole('button', { name: /decrement/i }); keep using
mockFormikRenderer(<NumberSpinnerField name="spinnerField" />) as-is and retain
testId queries only as a fallback if role queries fail.
In
`@frontend/packages/console-shared/src/components/formik-fields/key-value-file-input-field/__tests__/KeyValueFileInputField.spec.tsx`:
- Around line 46-49: Remove the unnecessary act wrapper around userEvent calls
in the test: replace the block using act(async () => { await
user.click(KeyField); await user.tab(); }) with direct awaited user calls (await
user.click(KeyField); await user.tab();). Update the test in
KeyValueFileInputField.spec.tsx where KeyField and the user helper are used,
removing the import/usage of act for that interaction to satisfy
testing-library/no-unnecessary-act and rely on userEvent's built-in async
handling.
- Around line 82-85: Replace the waitFor + queryByTestId patterns with direct
async queries: for assertions that something is absent (currently using await
waitFor(() => { const removeButton =
screen.queryByTestId('remove-key-value-button');
expect(removeButton).toBeNull(); })) remove the waitFor and just use const
removeButton = screen.queryByTestId('remove-key-value-button');
expect(removeButton).toBeNull(); and for assertions that wait for presence or
count (the other two occurrences) replace waitFor + queryBy*/getAllBy* with
await screen.findByTestId('...') or await screen.findAllByTestId('...')
respectively (use findByTestId for single-element presence and findAllByTestId
for count checks) so tests use direct async queries instead of polling with
waitFor.
In
`@frontend/packages/console-shared/src/components/getting-started/__tests__/GettingStartedCard.spec.tsx`:
- Around line 60-61: The test currently asserts only that the click handler was
called; change these to strict call-count assertions so accidental
double-invocations are caught—replace expect(onClick).toHaveBeenCalled() with
expect(onClick).toHaveBeenCalledTimes(1) for the user.click() interaction on
screen.getByTestId('item link-1') and the other similar interaction (the second
occurrence around lines 77-78), ensuring both click assertions check for exactly
one invocation of the onClick mock.
- Around line 32-35: Replace the title assertion that uses
screen.getByText('Test Card') in the GettingStartedCard test with a semantic
heading query to catch heading-level regressions; specifically, change it to use
screen.getByRole('heading', { name: 'Test Card', level: 2 }) (or the correct
heading level used by the GettingStartedCard component) so the test asserts both
accessible role and label instead of plain text.
In
`@frontend/packages/container-security/src/components/__tests__/ImageVulnerabilityToggleGroup.spec.tsx`:
- Around line 32-39: In the ImageVulnerabilityToggleGroup test "should render
empty state when switching to App dependency with no vulnerabilities", replace
the synchronous assertion that uses screen.getByText after await
user.click(appDependencyButton) with an asynchronous query (await
screen.findByText) so the test waits for the UI update; update the assertion to
await screen.findByText(/No.*app dependency.*vulnerabilities/i) and assert
visibility on the resolved element.
In
`@frontend/packages/dev-console/src/components/user-preferences/__tests__/SecureRouteFields.spec.tsx`:
- Around line 60-72: Extract a small test helper to encapsulate the repeated
setup/click flow used in SecureRouteFields tests: move the userEvent.setup(),
mockUsePreferredRoutingOptions.mockReturnValue([...]), render(<SecureRouteFields
/>) and the await user.click(screen.getByTestId('insecure-traffic')) into a
helper function (e.g., renderAndClickInsecureTraffic or setupSecureRouteFields)
and replace the duplicated blocks in both dropdown-behavior tests with calls to
that helper; keep references to mockUsePreferredRoutingOptions,
SecureRouteFields, and the 'insecure-traffic' testId so the behavior remains
identical.
In
`@frontend/packages/knative-plugin/src/components/sink-source/__tests__/SinkSourceModal.spec.tsx`:
- Around line 12-18: The Button mock in SinkSourceModal.spec.tsx currently only
renders when type === 'submit', hiding the modal's cancel button; update the
mock implementation of Button (jest.fn(({ children, type, ...props }) => ...))
so it returns a real <button> element for non-submit types as well (e.g., when
type === 'button' or undefined) forwarding children and props so tests can
assert the cancel button's presence and onClick; ensure the mock still renders
the submit button as before and preserves prop forwarding to keep all existing
assertions working with SinkSourceModal's cancel button.
- Around line 49-53: Update the requestSubmit polyfill
(HTMLFormElement.prototype.requestSubmit / requestSubmitPolyfill) to accept an
optional submitter parameter and dispatch a SubmitEvent with the submitter set
when available; if the environment lacks SubmitEvent support, fall back to
creating a plain Event but attach a submitter property so handlers relying on
event.submitter (and tests using `@testing-library/user-event` v14) still work.
Ensure you preserve bubbles/cancelable flags and maintain backward compatibility
by only using SubmitEvent when supported.
In
`@frontend/packages/operator-lifecycle-manager/src/components/modals/__tests__/installplan-approval-modal.spec.tsx`:
- Around line 107-118: The test depends on the shared fixture testInstallPlan
default approval; make the test explicit by mutating the cloned installPlan
before rendering so its approval is not already "Manual" (e.g. set
installPlan.spec.approval = 'Automatic' or the appropriate ApprovalAutomatic
constant) so that clicking the Manual radio (InstallPlanApprovalModal) actually
changes state and enables the Save button; keep using the cloned installPlan and
the same installPlanApprovalModalProps when calling renderWithProviders.
In
`@frontend/public/components/modals/__tests__/configure-update-strategy-modal.spec.tsx`:
- Around line 69-88: The RollingUpdateInputsHarness component is duplicated;
extract it into a single reusable helper used by both tests to avoid drift:
create one shared RollingUpdateInputsHarness function that initializes
maxUnavailable and maxSurge via useState, calls setMaxUnavailable/setMaxSurge
and mockProps.onChangeMaxUnavailable/onChangeMaxSurge in the same way, and
render ConfigureUpdateStrategy with strategyType="RollingUpdate" and the same
props; replace both inline duplicates (the blocks using ConfigureUpdateStrategy
with maxUnavailable/maxSurge and onChange handlers) with calls to this shared
helper so both tests use the identical component.
In
`@frontend/public/components/modals/__tests__/impersonate-user-modal-integration.spec.tsx`:
- Around line 64-70: Remove the manual act(...) wrappers around render(...) and
userEvent calls in the ImpersonateUserModal tests: find occurrences of act(async
() => { render(...); }) and act(async () => { await userEvent.*(...) }) in
impersonate-user-modal-integration.spec.tsx and replace them with direct calls
to render(...) and await userEvent.*(...) respectively (keep async/await on
userEvent where needed); rely on `@testing-library/react` and
`@testing-library/user-event` to handle act for you and do not import or use act
around render, user interactions, or assertions involving the
ImpersonateUserModal, mockStore, onClose, and onImpersonate test helpers.
In `@frontend/public/components/utils/__tests__/kebab.spec.tsx`:
- Around line 19-67: Refactor the repeated userEvent.setup() calls by creating a
shared `let user: ReturnType<typeof userEvent.setup>` and initializing it in a
beforeEach within each describe block that contains the tests (e.g., the
describe for KebabItem_ and the describe for KebabItemAccessReview_); then
remove the per-test `const user = userEvent.setup()` lines so tests that call
user.click(...) use the beforeEach-initialized `user` instead (update any
imports/types if needed and keep test logic in KebabItem,
KebabItemAccessReview_, and related mocks unchanged).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: ec8ee7a1-e993-44d6-b331-0e57c249dbf7
📒 Files selected for processing (57)
frontend/packages/console-app/src/__tests__/hooks/useCSPViolationDetector.spec.tsxfrontend/packages/console-app/src/components/modals/resource-limits/__tests__/ResourceLimitsModal.spec.tsxfrontend/packages/console-shared/src/components/alerts/__tests__/SwitchToYAMLAlert.spec.tsxfrontend/packages/console-shared/src/components/editor/__tests__/CodeEditorToolbar.spec.tsxfrontend/packages/console-shared/src/components/form-utils/__tests__/FlexForm.spec.tsxfrontend/packages/console-shared/src/components/form-utils/__tests__/FormFooter.spec.tsxfrontend/packages/console-shared/src/components/formik-fields/__tests__/NumberSpinnerField.spec.tsxfrontend/packages/console-shared/src/components/formik-fields/key-value-file-input-field/__tests__/KeyValueFileInputField.spec.tsxfrontend/packages/console-shared/src/components/formik-fields/multi-column-field/__tests__/MultiColumnFieldFooter.spec.tsxfrontend/packages/console-shared/src/components/getting-started/__tests__/GettingStartedCard.spec.tsxfrontend/packages/console-shared/src/components/getting-started/__tests__/RestoreGettingStartedButton.spec.tsxfrontend/packages/console-shared/src/components/modals/__tests__/FetchProgressModal.spec.tsxfrontend/packages/console-shared/src/components/modals/__tests__/TextInputModal.spec.tsxfrontend/packages/console-shared/src/components/progressive-list/__tests__/ProgressiveList.spec.tsxfrontend/packages/console-shared/src/components/toast/__tests__/ToastProvider.spec.tsxfrontend/packages/console-shared/src/test-utils/unit-test-utils.tsxfrontend/packages/container-security/src/components/__tests__/ImageVulnerabilityToggleGroup.spec.tsxfrontend/packages/dev-console/src/components/deployments/__tests__/AdvancedSection.spec.tsxfrontend/packages/dev-console/src/components/deployments/__tests__/DeploymentForm.spec.tsxfrontend/packages/dev-console/src/components/deployments/__tests__/DeploymentStrategySection.spec.tsxfrontend/packages/dev-console/src/components/deployments/__tests__/EnvironmentVariablesSection.spec.tsxfrontend/packages/dev-console/src/components/deployments/__tests__/ImagesSection.spec.tsxfrontend/packages/dev-console/src/components/deployments/__tests__/PauseRolloutsSection.spec.tsxfrontend/packages/dev-console/src/components/user-preferences/__tests__/SecureRouteFields.spec.tsxfrontend/packages/knative-plugin/src/components/overview/__tests__/RevisionsOverviewList.spec.tsxfrontend/packages/knative-plugin/src/components/sink-source/__tests__/SinkSourceModal.spec.tsxfrontend/packages/knative-plugin/src/components/traffic-splitting/__tests__/TrafficSplittingModal.spec.tsxfrontend/packages/operator-lifecycle-manager-v1/src/components/cluster-extension/__tests__/ClusterExtensionForm.spec.tsxfrontend/packages/operator-lifecycle-manager/src/components/__tests__/install-plan.spec.tsxfrontend/packages/operator-lifecycle-manager/src/components/descriptors/spec/__tests__/resource-requirements.spec.tsxfrontend/packages/operator-lifecycle-manager/src/components/modals/__tests__/installplan-approval-modal.spec.tsxfrontend/packages/operator-lifecycle-manager/src/components/modals/__tests__/subscription-channel-modal.spec.tsxfrontend/packages/operator-lifecycle-manager/src/components/modals/__tests__/uninstall-operator-modal.spec.tsxfrontend/packages/topology/src/__tests__/TopologySideBar.spec.tsxfrontend/packages/topology/src/components/export-app/__tests__/ExportApplicationModal.spec.tsxfrontend/packages/webterminal-plugin/src/components/cloud-shell/__tests__/MinimizeRestoreButton.spec.tsxfrontend/packages/webterminal-plugin/src/components/cloud-shell/__tests__/MultiTabbedTerminal.spec.tsxfrontend/public/components/__tests__/storage-class-form.spec.tsxfrontend/public/components/cluster-settings/__tests__/basicauth-idp-form.spec.tsxfrontend/public/components/cluster-settings/__tests__/github-idp-form.spec.tsxfrontend/public/components/cluster-settings/__tests__/gitlab-idp-form.spec.tsxfrontend/public/components/cluster-settings/__tests__/google-idp-form.spec.tsxfrontend/public/components/cluster-settings/__tests__/htpasswd-idp-form.spec.tsxfrontend/public/components/cluster-settings/__tests__/keystone-idp-form.spec.tsxfrontend/public/components/cluster-settings/__tests__/ldap-idp-form.spec.tsxfrontend/public/components/cluster-settings/__tests__/openid-idp-form.spec.tsxfrontend/public/components/cluster-settings/__tests__/request-header-idp-form.spec.tsxfrontend/public/components/cluster-settings/__tests__/test-utils.tsfrontend/public/components/factory/__tests__/list-page.spec.tsxfrontend/public/components/modals/__tests__/configure-update-strategy-modal.spec.tsxfrontend/public/components/modals/__tests__/impersonate-user-modal-integration.spec.tsxfrontend/public/components/modals/__tests__/impersonate-user-modal.spec.tsxfrontend/public/components/modals/__tests__/replace-code-modal.spec.tsxfrontend/public/components/utils/__tests__/download-button.spec.tsxfrontend/public/components/utils/__tests__/kebab.spec.tsxfrontend/public/components/utils/__tests__/name-value-editor.spec.tsxfrontend/public/components/utils/__tests__/single-typeahead-dropdown.spec.tsx
Comment on lines
+10
to
47
| describe('Number Spinner Field', () => { | ||
| it('should render input field with type number', () => { | ||
| mockFormikRenderer(<NumberSpinnerField name="spinnerField" />); | ||
| const input = getInput(); | ||
| expect(input.type).toEqual('number'); | ||
| }); | ||
|
|
||
| it('displays 0 when initial value is empty', () => { | ||
| mockFormikRenderer(<NumberSpinnerField name="spinnerField" />, { spinnerField: '' }); | ||
| expect(getInput()).toHaveValue(0); | ||
| it('should render input with increment decrement button', () => { | ||
| mockFormikRenderer(<NumberSpinnerField name="spinnerField" />); | ||
| screen.getByTestId('Increment'); | ||
| screen.getByTestId('Decrement'); | ||
| }); | ||
|
|
||
| it('updates value when typing in input field', async () => { | ||
| const user = userEvent.setup(); | ||
| it('should only put numbers in input field', async () => { | ||
| mockFormikRenderer(<NumberSpinnerField name="spinnerField" />, { spinnerField: '' }); | ||
|
|
||
| await user.clear(getInput()); | ||
| await user.type(getInput(), '42'); | ||
|
|
||
| expect(getInput()).toHaveValue(42); | ||
| }); | ||
|
|
||
| it('increments value when increment button is clicked', async () => { | ||
| const user = userEvent.setup(); | ||
| mockFormikRenderer(<NumberSpinnerField name="spinnerField" />, { spinnerField: 0 }); | ||
| expect(getInput().value).toEqual('0'); | ||
|
|
||
| await user.click(screen.getByRole('button', { name: /increment/i })); | ||
|
|
||
| expect(getInput()).toHaveValue(1); | ||
| }); | ||
|
|
||
| it('decrements value when decrement button is clicked', async () => { | ||
| const user = userEvent.setup(); | ||
| mockFormikRenderer(<NumberSpinnerField name="spinnerField" />, { spinnerField: 5 }); | ||
|
|
||
| await user.click(screen.getByRole('button', { name: /decrement/i })); | ||
|
|
||
| expect(getInput()).toHaveValue(4); | ||
| }); | ||
|
|
||
| it('associates help text with input via aria-describedby', () => { | ||
| mockFormikRenderer(<NumberSpinnerField name="spinnerField" helpText="Helpful description" />); | ||
| expect(getInput()).toHaveAttribute('aria-describedby'); | ||
| }); | ||
|
|
||
| it('input is accessible by spinbutton role', () => { | ||
| mockFormikRenderer(<NumberSpinnerField name="spinnerField" label="Replicas" />); | ||
| expect(screen.getByRole('spinbutton')).toBeVisible(); | ||
| }); | ||
|
|
||
| it('does not decrement below min when value equals min', async () => { | ||
| const user = userEvent.setup(); | ||
| mockFormikRenderer(<NumberSpinnerField name="spinnerField" min={3} />, { | ||
| spinnerField: 3, | ||
| }); | ||
|
|
||
| await user.click(screen.getByRole('button', { name: /decrement/i })); | ||
|
|
||
| expect(getInput()).toHaveValue(3); | ||
| }); | ||
|
|
||
| it('does not increment above max when value equals max', async () => { | ||
| const user = userEvent.setup(); | ||
| mockFormikRenderer(<NumberSpinnerField name="spinnerField" max={7} />, { | ||
| spinnerField: 7, | ||
| await act(async () => { | ||
| const user = userEvent.setup(); | ||
| await user.clear(getInput()); | ||
| await user.type(getInput(), '12'); | ||
| }); | ||
|
|
||
| await user.click(screen.getByRole('button', { name: /increment/i })); | ||
|
|
||
| expect(getInput()).toHaveValue(7); | ||
| await waitFor(() => expect(getInput().value).toEqual('12')); | ||
| }); | ||
|
|
||
| it('increments value when ArrowUp is pressed on the input', async () => { | ||
| it('should increment or decrement value based on clicked button', async () => { | ||
| const user = userEvent.setup(); | ||
| mockFormikRenderer(<NumberSpinnerField name="spinnerField" />, { spinnerField: 2 }); | ||
|
|
||
| await user.click(getInput()); | ||
| await user.keyboard('{ArrowUp}'); | ||
|
|
||
| expect(getInput()).toHaveValue(3); | ||
| }); | ||
|
|
||
| it('decrements value when ArrowDown is pressed on the input', async () => { | ||
| const user = userEvent.setup(); | ||
| mockFormikRenderer(<NumberSpinnerField name="spinnerField" />, { spinnerField: 4 }); | ||
|
|
||
| await user.click(getInput()); | ||
| await user.keyboard('{ArrowDown}'); | ||
| mockFormikRenderer(<NumberSpinnerField name="spinnerField" />, { spinnerField: '' }); | ||
|
|
||
| expect(getInput()).toHaveValue(3); | ||
| expect(getInput().value).toEqual('0'); | ||
| await user.click(screen.getByTestId('Increment')); | ||
| await user.click(screen.getByTestId('Increment')); | ||
| expect(getInput().value).toEqual('2'); | ||
| await user.click(screen.getByTestId('Decrement')); | ||
| expect(getInput().value).toEqual('1'); | ||
| }); |
Contributor
There was a problem hiding this comment.
Please restore dropped behavior coverage, not just interaction API parity.
Line 10–47 now validates only a narrow path (type + click flow). This refactor removed checks for component behaviors that are part of NumberSpinnerField/NumberSpinner contract (e.g., aria-describedby/helper text wiring and min/max-related behavior). Since PR scope is interaction API migration, this test-surface reduction is a regression risk.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@frontend/packages/console-shared/src/components/formik-fields/__tests__/NumberSpinnerField.spec.tsx`
around lines 10 - 47, The tests for NumberSpinnerField were narrowed to only
type and click flows and need to restore behavioral coverage: update
NumberSpinnerField.spec.tsx to re-add assertions that helper
text/aria-describedby wiring works (render with helperText prop and assert
getInput().getAttribute('aria-describedby') points to the helper element and
that the helper text is present), restore min/max behavior checks by rendering
NumberSpinnerField/NumberSpinner with min and max props and asserting values
clamp at min/max when typing, clearing, or using Increment/Decrement, and verify
step behavior and default value handling (render with initial value or empty
state and assert expected initial value like '0'); use existing helpers
(mockFormikRenderer, getInput, screen.getByTestId('Increment'|'Decrement')) and
keep async userEvent flows in act/waitFor as in other tests.
Comment on lines
7
to
94
| describe('GettingStartedCard', () => { | ||
| const defaultProps: GettingStartedCardProps = { | ||
| id: 'test-card', | ||
| title: 'Test Card', | ||
| description: 'This is a test card.', | ||
| links: [ | ||
| { id: 'link-1', title: 'Internal Link', href: '/internal' }, | ||
| { id: 'link-2', title: 'External Link', href: 'https://example.com', external: true }, | ||
| { | ||
| id: 'link-1', | ||
| title: 'Internal Link', | ||
| href: '/internal', | ||
| }, | ||
| { | ||
| id: 'link-2', | ||
| title: 'External Link', | ||
| href: 'https://example.com', | ||
| external: true, | ||
| }, | ||
| ], | ||
| moreLink: { id: 'more-link', title: 'More Info', href: '/more' }, | ||
| moreLink: { | ||
| id: 'more-link', | ||
| title: 'More Info', | ||
| href: '/more', | ||
| }, | ||
| }; | ||
|
|
||
| it('renders card title, description, and icon', async () => { | ||
| const TestIcon = () => <div>TestIcon</div>; | ||
| renderWithProviders(<GettingStartedCard {...defaultProps} icon={<TestIcon />} />); | ||
| await waitFor(() => { | ||
| expect(screen.getByRole('heading', { name: /Test Card/i, level: 3 })).toBeVisible(); | ||
| }); | ||
| expect(screen.getByText('This is a test card.')).toBeVisible(); | ||
| expect(screen.getByText('TestIcon')).toBeVisible(); | ||
| }); | ||
|
|
||
| it('renders internal links with arrow icon', async () => { | ||
| renderWithProviders(<GettingStartedCard {...defaultProps} />); | ||
| const internalLink = await screen.findByTestId('item link-1'); | ||
| expect(within(internalLink).getByText('Internal Link')).toBeVisible(); | ||
| expect(within(internalLink).getByText('ArrowRightIcon')).toBeVisible(); | ||
| }); | ||
|
|
||
| it('renders external links with external link icon and target blank', async () => { | ||
| it('renders title and description', () => { | ||
| renderWithProviders(<GettingStartedCard {...defaultProps} />); | ||
| const externalLink = await screen.findByTestId('item link-2'); | ||
| expect(within(externalLink).getByText('External Link')).toBeVisible(); | ||
| expect(within(externalLink).getByText('ExternalLinkAltIcon')).toBeVisible(); | ||
| expect(externalLink).toHaveAttribute('target', '_blank'); | ||
| expect(externalLink).toHaveAttribute('rel', 'noopener noreferrer'); | ||
| expect(screen.getByText('Test Card')).toBeInTheDocument(); | ||
| expect(screen.getByText('This is a test card.')).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| it('renders moreLink as internal link or button based on props', async () => { | ||
| it('renders all links', () => { | ||
| renderWithProviders(<GettingStartedCard {...defaultProps} />); | ||
| expect(await screen.findByRole('link', { name: 'More Info' })).toHaveAttribute('href', '/more'); | ||
| expect(screen.getByTestId('item link-1')).toBeInTheDocument(); | ||
| expect(screen.getByTestId('item link-2')).toBeInTheDocument(); | ||
| expect(screen.getByTestId('item more-link')).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| it('renders moreLink as button when onClick is provided and calls handler', async () => { | ||
| it('calls onClick for internal link', async () => { | ||
| const user = userEvent.setup(); | ||
| const onClick = jest.fn(); | ||
| const props = { ...defaultProps, moreLink: { id: 'more-link', title: 'More Info', onClick } }; | ||
| const props = { | ||
| ...defaultProps, | ||
| links: [ | ||
| { | ||
| id: 'link-1', | ||
| title: 'Internal Link', | ||
| href: '/internal', | ||
| onClick, | ||
| }, | ||
| ], | ||
| }; | ||
| renderWithProviders(<GettingStartedCard {...props} />); | ||
| await user.click(await screen.findByRole('button', { name: 'More Info' })); | ||
| expect(onClick).toHaveBeenCalledTimes(1); | ||
| await user.click(screen.getByTestId('item link-1')); | ||
| expect(onClick).toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it('calls onClick for internal link when clicked', async () => { | ||
| it('calls onClick for moreLink', async () => { | ||
| const user = userEvent.setup(); | ||
| const onClick = jest.fn(); | ||
| const props = { | ||
| ...defaultProps, | ||
| links: [{ id: 'link-1', title: 'Internal Link', href: '/internal', onClick }], | ||
| moreLink: { | ||
| id: 'more-link', | ||
| title: 'More Info', | ||
| href: '/more', | ||
| onClick, | ||
| }, | ||
| }; | ||
| renderWithProviders(<GettingStartedCard {...props} />); | ||
| await user.click(await screen.findByTestId('item link-1')); | ||
| expect(onClick).toHaveBeenCalledTimes(1); | ||
| await user.click(screen.getByTestId('item more-link')); | ||
| expect(onClick).toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it('renders skeleton for loading links', async () => { | ||
| const props = { ...defaultProps, links: [{ id: 'loading-link', loading: true }] }; | ||
| renderWithProviders(<GettingStartedCard {...props} />); | ||
| expect(await screen.findByTestId('getting-started-skeleton')).toBeVisible(); | ||
| }); | ||
|
|
||
| it('does not render links section when links array is empty', async () => { | ||
| const props = { ...defaultProps, links: [] }; | ||
| renderWithProviders(<GettingStartedCard {...props} />); | ||
| await waitFor(() => { | ||
| expect(screen.queryByRole('list')).not.toBeInTheDocument(); | ||
| }); | ||
| }); | ||
|
|
||
| it('does not render moreLink when not provided', async () => { | ||
| const props = { ...defaultProps, moreLink: undefined }; | ||
| renderWithProviders(<GettingStartedCard {...props} />); | ||
| await waitFor(() => { | ||
| expect(screen.queryByRole('link', { name: 'More Info' })).not.toBeInTheDocument(); | ||
| }); | ||
| }); | ||
|
|
||
| it('applies custom title color when provided', async () => { | ||
| const props = { ...defaultProps, titleColor: '#ff0000' }; | ||
| it('renders skeleton for loading links', () => { | ||
| const props = { | ||
| ...defaultProps, | ||
| links: [ | ||
| { | ||
| id: 'loading-link', | ||
| loading: true, | ||
| }, | ||
| ], | ||
| }; | ||
| renderWithProviders(<GettingStartedCard {...props} />); | ||
| expect(await screen.findByRole('heading', { name: 'Test Card' })).toHaveStyle({ | ||
| color: '#ff0000', | ||
| }); | ||
| expect(screen.getByTestId('getting-started-skeleton')).toBeInTheDocument(); | ||
| }); | ||
| }); |
Contributor
There was a problem hiding this comment.
Preserve behavioral coverage; this currently narrows it beyond the migration scope.
This suite now validates mostly presence/clicks, but it no longer guards key behaviors of GettingStartedCard (external-link hardening attrs and conditional rendering branches). That weakens regression detection for security and UX-critical paths.
Suggested test additions to restore critical coverage
it('renders all links', () => {
renderWithProviders(<GettingStartedCard {...defaultProps} />);
- expect(screen.getByTestId('item link-1')).toBeInTheDocument();
- expect(screen.getByTestId('item link-2')).toBeInTheDocument();
+ expect(screen.getByTestId('item link-1')).toBeInTheDocument();
+ const external = screen.getByTestId('item link-2');
+ expect(external).toBeInTheDocument();
+ expect(external).toHaveAttribute('target', '_blank');
+ expect(external).toHaveAttribute('rel', expect.stringContaining('noopener'));
+ expect(external).toHaveAttribute('rel', expect.stringContaining('noreferrer'));
expect(screen.getByTestId('item more-link')).toBeInTheDocument();
});
+
+it('does not render list when links are empty', () => {
+ renderWithProviders(<GettingStartedCard {...defaultProps} links={[]} />);
+ expect(screen.queryByTestId('item link-1')).not.toBeInTheDocument();
+});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| describe('GettingStartedCard', () => { | |
| const defaultProps: GettingStartedCardProps = { | |
| id: 'test-card', | |
| title: 'Test Card', | |
| description: 'This is a test card.', | |
| links: [ | |
| { id: 'link-1', title: 'Internal Link', href: '/internal' }, | |
| { id: 'link-2', title: 'External Link', href: 'https://example.com', external: true }, | |
| { | |
| id: 'link-1', | |
| title: 'Internal Link', | |
| href: '/internal', | |
| }, | |
| { | |
| id: 'link-2', | |
| title: 'External Link', | |
| href: 'https://example.com', | |
| external: true, | |
| }, | |
| ], | |
| moreLink: { id: 'more-link', title: 'More Info', href: '/more' }, | |
| moreLink: { | |
| id: 'more-link', | |
| title: 'More Info', | |
| href: '/more', | |
| }, | |
| }; | |
| it('renders card title, description, and icon', async () => { | |
| const TestIcon = () => <div>TestIcon</div>; | |
| renderWithProviders(<GettingStartedCard {...defaultProps} icon={<TestIcon />} />); | |
| await waitFor(() => { | |
| expect(screen.getByRole('heading', { name: /Test Card/i, level: 3 })).toBeVisible(); | |
| }); | |
| expect(screen.getByText('This is a test card.')).toBeVisible(); | |
| expect(screen.getByText('TestIcon')).toBeVisible(); | |
| }); | |
| it('renders internal links with arrow icon', async () => { | |
| renderWithProviders(<GettingStartedCard {...defaultProps} />); | |
| const internalLink = await screen.findByTestId('item link-1'); | |
| expect(within(internalLink).getByText('Internal Link')).toBeVisible(); | |
| expect(within(internalLink).getByText('ArrowRightIcon')).toBeVisible(); | |
| }); | |
| it('renders external links with external link icon and target blank', async () => { | |
| it('renders title and description', () => { | |
| renderWithProviders(<GettingStartedCard {...defaultProps} />); | |
| const externalLink = await screen.findByTestId('item link-2'); | |
| expect(within(externalLink).getByText('External Link')).toBeVisible(); | |
| expect(within(externalLink).getByText('ExternalLinkAltIcon')).toBeVisible(); | |
| expect(externalLink).toHaveAttribute('target', '_blank'); | |
| expect(externalLink).toHaveAttribute('rel', 'noopener noreferrer'); | |
| expect(screen.getByText('Test Card')).toBeInTheDocument(); | |
| expect(screen.getByText('This is a test card.')).toBeInTheDocument(); | |
| }); | |
| it('renders moreLink as internal link or button based on props', async () => { | |
| it('renders all links', () => { | |
| renderWithProviders(<GettingStartedCard {...defaultProps} />); | |
| expect(await screen.findByRole('link', { name: 'More Info' })).toHaveAttribute('href', '/more'); | |
| expect(screen.getByTestId('item link-1')).toBeInTheDocument(); | |
| expect(screen.getByTestId('item link-2')).toBeInTheDocument(); | |
| expect(screen.getByTestId('item more-link')).toBeInTheDocument(); | |
| }); | |
| it('renders moreLink as button when onClick is provided and calls handler', async () => { | |
| it('calls onClick for internal link', async () => { | |
| const user = userEvent.setup(); | |
| const onClick = jest.fn(); | |
| const props = { ...defaultProps, moreLink: { id: 'more-link', title: 'More Info', onClick } }; | |
| const props = { | |
| ...defaultProps, | |
| links: [ | |
| { | |
| id: 'link-1', | |
| title: 'Internal Link', | |
| href: '/internal', | |
| onClick, | |
| }, | |
| ], | |
| }; | |
| renderWithProviders(<GettingStartedCard {...props} />); | |
| await user.click(await screen.findByRole('button', { name: 'More Info' })); | |
| expect(onClick).toHaveBeenCalledTimes(1); | |
| await user.click(screen.getByTestId('item link-1')); | |
| expect(onClick).toHaveBeenCalled(); | |
| }); | |
| it('calls onClick for internal link when clicked', async () => { | |
| it('calls onClick for moreLink', async () => { | |
| const user = userEvent.setup(); | |
| const onClick = jest.fn(); | |
| const props = { | |
| ...defaultProps, | |
| links: [{ id: 'link-1', title: 'Internal Link', href: '/internal', onClick }], | |
| moreLink: { | |
| id: 'more-link', | |
| title: 'More Info', | |
| href: '/more', | |
| onClick, | |
| }, | |
| }; | |
| renderWithProviders(<GettingStartedCard {...props} />); | |
| await user.click(await screen.findByTestId('item link-1')); | |
| expect(onClick).toHaveBeenCalledTimes(1); | |
| await user.click(screen.getByTestId('item more-link')); | |
| expect(onClick).toHaveBeenCalled(); | |
| }); | |
| it('renders skeleton for loading links', async () => { | |
| const props = { ...defaultProps, links: [{ id: 'loading-link', loading: true }] }; | |
| renderWithProviders(<GettingStartedCard {...props} />); | |
| expect(await screen.findByTestId('getting-started-skeleton')).toBeVisible(); | |
| }); | |
| it('does not render links section when links array is empty', async () => { | |
| const props = { ...defaultProps, links: [] }; | |
| renderWithProviders(<GettingStartedCard {...props} />); | |
| await waitFor(() => { | |
| expect(screen.queryByRole('list')).not.toBeInTheDocument(); | |
| }); | |
| }); | |
| it('does not render moreLink when not provided', async () => { | |
| const props = { ...defaultProps, moreLink: undefined }; | |
| renderWithProviders(<GettingStartedCard {...props} />); | |
| await waitFor(() => { | |
| expect(screen.queryByRole('link', { name: 'More Info' })).not.toBeInTheDocument(); | |
| }); | |
| }); | |
| it('applies custom title color when provided', async () => { | |
| const props = { ...defaultProps, titleColor: '#ff0000' }; | |
| it('renders skeleton for loading links', () => { | |
| const props = { | |
| ...defaultProps, | |
| links: [ | |
| { | |
| id: 'loading-link', | |
| loading: true, | |
| }, | |
| ], | |
| }; | |
| renderWithProviders(<GettingStartedCard {...props} />); | |
| expect(await screen.findByRole('heading', { name: 'Test Card' })).toHaveStyle({ | |
| color: '#ff0000', | |
| }); | |
| expect(screen.getByTestId('getting-started-skeleton')).toBeInTheDocument(); | |
| }); | |
| }); | |
| describe('GettingStartedCard', () => { | |
| const defaultProps: GettingStartedCardProps = { | |
| id: 'test-card', | |
| title: 'Test Card', | |
| description: 'This is a test card.', | |
| links: [ | |
| { | |
| id: 'link-1', | |
| title: 'Internal Link', | |
| href: '/internal', | |
| }, | |
| { | |
| id: 'link-2', | |
| title: 'External Link', | |
| href: 'https://example.com', | |
| external: true, | |
| }, | |
| ], | |
| moreLink: { | |
| id: 'more-link', | |
| title: 'More Info', | |
| href: '/more', | |
| }, | |
| }; | |
| it('renders title and description', () => { | |
| renderWithProviders(<GettingStartedCard {...defaultProps} />); | |
| expect(screen.getByText('Test Card')).toBeInTheDocument(); | |
| expect(screen.getByText('This is a test card.')).toBeInTheDocument(); | |
| }); | |
| it('renders all links', () => { | |
| renderWithProviders(<GettingStartedCard {...defaultProps} />); | |
| expect(screen.getByTestId('item link-1')).toBeInTheDocument(); | |
| const external = screen.getByTestId('item link-2'); | |
| expect(external).toBeInTheDocument(); | |
| expect(external).toHaveAttribute('target', '_blank'); | |
| expect(external).toHaveAttribute('rel', expect.stringContaining('noopener')); | |
| expect(external).toHaveAttribute('rel', expect.stringContaining('noreferrer')); | |
| expect(screen.getByTestId('item more-link')).toBeInTheDocument(); | |
| }); | |
| it('calls onClick for internal link', async () => { | |
| const user = userEvent.setup(); | |
| const onClick = jest.fn(); | |
| const props = { | |
| ...defaultProps, | |
| links: [ | |
| { | |
| id: 'link-1', | |
| title: 'Internal Link', | |
| href: '/internal', | |
| onClick, | |
| }, | |
| ], | |
| }; | |
| renderWithProviders(<GettingStartedCard {...props} />); | |
| await user.click(screen.getByTestId('item link-1')); | |
| expect(onClick).toHaveBeenCalled(); | |
| }); | |
| it('calls onClick for moreLink', async () => { | |
| const user = userEvent.setup(); | |
| const onClick = jest.fn(); | |
| const props = { | |
| ...defaultProps, | |
| moreLink: { | |
| id: 'more-link', | |
| title: 'More Info', | |
| href: '/more', | |
| onClick, | |
| }, | |
| }; | |
| renderWithProviders(<GettingStartedCard {...props} />); | |
| await user.click(screen.getByTestId('item more-link')); | |
| expect(onClick).toHaveBeenCalled(); | |
| }); | |
| it('renders skeleton for loading links', () => { | |
| const props = { | |
| ...defaultProps, | |
| links: [ | |
| { | |
| id: 'loading-link', | |
| loading: true, | |
| }, | |
| ], | |
| }; | |
| renderWithProviders(<GettingStartedCard {...props} />); | |
| expect(screen.getByTestId('getting-started-skeleton')).toBeInTheDocument(); | |
| }); | |
| it('does not render list when links are empty', () => { | |
| renderWithProviders(<GettingStartedCard {...defaultProps} links={[]} />); | |
| expect(screen.queryByTestId('item link-1')).not.toBeInTheDocument(); | |
| }); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@frontend/packages/console-shared/src/components/getting-started/__tests__/GettingStartedCard.spec.tsx`
around lines 7 - 94, Tests for GettingStartedCard lost coverage of key
behaviors: conditional rendering branches and external-link hardening
attributes. Update the test suite for GettingStartedCard to (1) assert that
external links (links with external: true) render rel="noopener noreferrer" and
target="_blank" (use the link with id 'link-2'), (2) add assertions for
conditional rendering paths such as when a link has loading: true vs has
href/onClick (use 'loading-link' and the existing onClick cases), and (3) verify
that internal vs external link markup differs as expected; reference test
utilities like renderWithProviders, screen.getByTestId('item link-2'),
screen.getByTestId('item loading-link'), and the moreLink case to locate
elements.
Comment on lines
+15
to
+21
| Button: jest.fn(({ children, type, ...props }) => | ||
| type === 'submit' ? ( | ||
| <button type="submit" {...props}> | ||
| {children} | ||
| </button> | ||
| ) : null, | ||
| ), |
Contributor
There was a problem hiding this comment.
Mocked Button hides the Cancel action and weakens test fidelity.
At Lines 15-21, non-submit PatternFly buttons are rendered as null. In frontend/packages/knative-plugin/src/components/traffic-splitting/TrafficSplittingModal.tsx (Lines 34-46), the modal renders both Save (type="submit") and Cancel (type="button"). This mock can mask regressions around modal actions and button presence.
Proposed fix
- Button: jest.fn(({ children, type, ...props }) =>
- type === 'submit' ? (
- <button type="submit" {...props}>
- {children}
- </button>
- ) : null,
- ),
+ Button: jest.fn(({ children, type = 'button', ...props }) => (
+ <button type={type} {...props}>
+ {children}
+ </button>
+ )),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| Button: jest.fn(({ children, type, ...props }) => | |
| type === 'submit' ? ( | |
| <button type="submit" {...props}> | |
| {children} | |
| </button> | |
| ) : null, | |
| ), | |
| Button: jest.fn(({ children, type = 'button', ...props }) => ( | |
| <button type={type} {...props}> | |
| {children} | |
| </button> | |
| )), |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@frontend/packages/knative-plugin/src/components/traffic-splitting/__tests__/TrafficSplittingModal.spec.tsx`
around lines 15 - 21, The mocked Button in TrafficSplittingModal.spec.tsx
currently returns null for non-submit types which hides the Cancel button from
tests; update the mock (the jest.fn for Button) to render a real <button> for
all types (or at least when type === 'button') preserving children and props so
Cancel/actions are present, e.g., return a button element that spreads props and
renders children; run the tests and update any assertions to check for the
Cancel button rendered by TrafficSplittingModal.tsx (the modal component that
renders Save type="submit" and Cancel type="button").
Comment on lines
+21
to
31
| const clickMultipleTimes = async ( | ||
| user: ReturnType<typeof userEvent.setup>, | ||
| element: HTMLElement, | ||
| times: number, | ||
| ) => { | ||
| for (let i = 0; i < times; i++) { | ||
| // eslint-disable-next-line no-await-in-loop | ||
| await act(async () => { | ||
| fireEvent.click(element); | ||
| await user.click(element); | ||
| }); | ||
| } |
Contributor
There was a problem hiding this comment.
Avoid repeated clicks on a potentially detached “Add new tab” element.
At Line 29, clickMultipleTimes keeps clicking the same captured node. In the callers (Line 89, Line 101), once max tabs are reached, the add button can disappear, and later user.click calls may target a detached element and fail.
🔧 Suggested fix
-const clickMultipleTimes = async (
- user: ReturnType<typeof userEvent.setup>,
- element: HTMLElement,
- times: number,
-) => {
+const clickMultipleTimes = async (
+ user: ReturnType<typeof userEvent.setup>,
+ getElement: () => HTMLElement | null,
+ times: number,
+) => {
for (let i = 0; i < times; i++) {
- // eslint-disable-next-line no-await-in-loop
- await act(async () => {
- await user.click(element);
- });
+ const element = getElement();
+ expect(element).toBeTruthy();
+ // eslint-disable-next-line no-await-in-loop
+ await user.click(element!);
}
};
@@
-const addTerminalButton = multiTabTerminalWrapper.getByLabelText('Add new tab');
-await clickMultipleTimes(user, addTerminalButton, 8);
+await clickMultipleTimes(
+ user,
+ () => multiTabTerminalWrapper.queryByLabelText('Add new tab'),
+ 7,
+);
@@
-const addTerminalButton = multiTabTerminalWrapper.getByLabelText('Add new tab');
-await clickMultipleTimes(user, addTerminalButton, 8);
+await clickMultipleTimes(
+ user,
+ () => multiTabTerminalWrapper.queryByLabelText('Add new tab'),
+ 7,
+);Also applies to: 89-89, 101-101
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@frontend/packages/webterminal-plugin/src/components/cloud-shell/__tests__/MultiTabbedTerminal.spec.tsx`
around lines 21 - 31, The helper clickMultipleTimes currently captures a single
HTMLElement and repeatedly clicks it, which can target a detached node when the
"Add new tab" button disappears; update clickMultipleTimes to accept a getter
(e.g., () => HTMLElement | null) or a selector and, inside the loop, query the
current element each iteration (using the provided getter or DOM query) and bail
out if it returns null, then perform the await act(() =>
user.click(currentElement)) only when the element is present; update callers in
the tests to pass a fresh-querying getter (or selector) instead of a captured
element so clicks stop gracefully when the button is removed.
cajieh
force-pushed
the
refactor-rtl-ests-fire-event-user-event
branch
from
2b14b5d to
3a0f805
Compare
Contributor
Author
|
/test e2e-gcp-console |
Contributor
|
@cajieh: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
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
3 participants
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.
Summary by CodeRabbit