CONSOLE-5176: Refactor RTL Tests from fireEvent to userEvent#16265
CONSOLE-5176: Refactor RTL Tests from fireEvent to userEvent#16265cajieh wants to merge 1 commit intoopenshift:mainfrom
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@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 "4.22.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
the
do-not-merge/work-in-progress
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: cajieh The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
openshift-ci
bot
added
component/dev-console
cajieh
changed the title
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
|
/retest |
|
@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 "4.22.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. |
📝 WalkthroughWalkthroughA systematic migration across the test suite to replace ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/packages/dev-console/src/components/deployments/__tests__/DeploymentForm.spec.tsx (1)
152-156:⚠️ Potential issue | 🟠 MajorMove disabled/progress assertions inside
waitForto improve test determinism.The assertions on
saveButton.hasAttribute('disabled')andsaveButton.querySelector('.pf-v6-c-button__progress')should be wrapped inwaitFor()to ensure the async state updates have settled. This aligns with@testing-librarybest practices when testing async state changes and reduces test flakiness.Suggested fix
await user.click(saveButton); - expect(saveButton.hasAttribute('disabled')).toBeTruthy(); - expect(saveButton.querySelector('.pf-v6-c-button__progress')).not.toBeNull(); - await waitFor(() => { - expect(handleSubmit).toHaveBeenCalledTimes(1); - }); + await waitFor(() => { + expect(saveButton.hasAttribute('disabled')).toBeTruthy(); + expect(saveButton.querySelector('.pf-v6-c-button__progress')).not.toBeNull(); + expect(handleSubmit).toHaveBeenCalledTimes(1); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/packages/dev-console/src/components/deployments/__tests__/DeploymentForm.spec.tsx` around lines 152 - 156, Move the two post-click assertions into the asynchronous waitFor to avoid flakiness: after calling user.click(saveButton) keep the click, then wrap both expect(saveButton.hasAttribute('disabled')).toBeTruthy() and expect(saveButton.querySelector('.pf-v6-c-button__progress')).not.toBeNull() inside a single await waitFor(() => { ... }) block so the saveButton disabled state and the .pf-v6-c-button__progress element are asserted only after async updates settle; reference the saveButton variable and the progress selector when making the change.
🧹 Nitpick comments (3)
frontend/packages/webterminal-plugin/src/components/cloud-shell/__tests__/MultiTabbedTerminal.spec.tsx (1)
21-30: Avoid reusing a potentially stale DOM node in repeated clicks
clickMultipleTimescaptures oneHTMLElementand reuses it across async clicks. Around re-renders/removal boundaries (like max tabs), this can interact with a detached node and make the test brittle.Suggested refactor
const clickMultipleTimes = async ( user: ReturnType<typeof userEvent.setup>, - element: HTMLElement, + getElement: () => HTMLElement | null, times: number, ) => { for (let i = 0; i < times; i++) { + const element = getElement(); + if (!element) { + break; + } // eslint-disable-next-line no-await-in-loop await act(async () => { await user.click(element); }); } }; @@ - await clickMultipleTimes(user, addTerminalButton, 8); + await clickMultipleTimes(user, () => multiTabTerminalWrapper.queryByLabelText('Add new tab'), 8); @@ - await clickMultipleTimes(user, addTerminalButton, 8); + await clickMultipleTimes(user, () => multiTabTerminalWrapper.queryByLabelText('Add new tab'), 8);Also applies to: 89-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 - 30, The helper clickMultipleTimes reuses a captured HTMLElement across async clicks which can become detached after re-renders; change its signature (the function clickMultipleTimes) to accept a getter (e.g., elementGetter: () => HTMLElement | null or a query callback) instead of a single HTMLElement, and inside the loop call the getter each iteration and assert the returned element is not null before awaiting user.click on it; update all call sites (including the other instance referenced around the second occurrence) to pass a function that queries the DOM for the current node so each click uses a fresh, non-stale element.frontend/packages/knative-plugin/src/components/traffic-splitting/__tests__/TrafficSplittingModal.spec.tsx (1)
40-45: Scope therequestSubmitpolyfill to avoid brittle global mutation.This overrides
HTMLFormElement.prototype.requestSubmitunconditionally and never restores it. Guard it and restore after tests to keep test behavior isolated and future-proof.Proposed patch
+let originalRequestSubmit: typeof HTMLFormElement.prototype.requestSubmit; + // jsdom does not implement requestSubmit; user-event uses it for type="submit" clicks beforeAll(() => { - HTMLFormElement.prototype.requestSubmit = function requestSubmitPolyfill(this: HTMLFormElement) { - this.dispatchEvent(new Event('submit', { bubbles: true, cancelable: true })); - }; + originalRequestSubmit = HTMLFormElement.prototype.requestSubmit; + if (!HTMLFormElement.prototype.requestSubmit) { + HTMLFormElement.prototype.requestSubmit = function requestSubmitPolyfill(this: HTMLFormElement) { + this.dispatchEvent(new Event('submit', { bubbles: true, cancelable: true })); + }; + } }); + +afterAll(() => { + HTMLFormElement.prototype.requestSubmit = originalRequestSubmit; +});As per coding guidelines, "Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity."
🤖 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 40 - 45, The global requestSubmit polyfill set in the beforeAll hook unconditionally mutates HTMLFormElement.prototype and never restores it; modify the test setup so beforeAll first saves the original HTMLFormElement.prototype.requestSubmit (e.g., const _orig = HTMLFormElement.prototype.requestSubmit), only assigns the polyfill if the original is undefined, and add an afterAll that restores HTMLFormElement.prototype.requestSubmit = _orig; keep the polyfill implementation as the existing requestSubmitPolyfill function and use the same event dispatch logic so test behavior remains identical while avoiding brittle global mutation.frontend/packages/knative-plugin/src/components/sink-source/__tests__/SinkSourceModal.spec.tsx (1)
43-48: Apply the same polyfill lifecycle guard here (beforeAll+afterAll).This file has the same unconditional global
requestSubmitoverride; make it conditional and restore original implementation after tests.As per coding guidelines, "Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity."
🤖 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 43 - 48, The test file unconditionally overrides HTMLFormElement.prototype.requestSubmit in the beforeAll block; make it conditional and restore the original after tests by saving the original requestSubmit, only assigning the polyfill if it is undefined, and restoring the saved value in an afterAll cleanup. Update the existing beforeAll/requestSubmitPolyfill usage and add a matching afterAll that references the saved originalRequestSubmit to return behavior to its prior state.
🤖 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/operator-lifecycle-manager-v1/src/components/cluster-extension/__tests__/ClusterExtensionForm.spec.tsx`:
- Around line 80-97: The test "should auto-generate namespace and service
account names based on ClusterExtension name" only asserts metadata.name but
must also verify the auto-generated namespace and serviceAccount fields; update
the assertion that checks mockOnChange (in the spec using ClusterExtensionForm
and mockOnChange) to expect objectContaining metadata.namespace and the service
account field (e.g., spec.serviceAccountName or metadata.serviceAccount
depending on impl) with the expected auto-generated values or patterns derived
from 'my-operator' so the test actually verifies namespace/serviceAccount
generation.
In
`@frontend/packages/topology/src/components/export-app/__tests__/ExportApplicationModal.spec.tsx`:
- Around line 146-156: The test "should call k8sKill with correct data on click
of cancel button when export app is in progress" is clicking the wrong element;
change the user click target from the 'export-restart-btn' test id to the cancel
button test id (e.g., 'export-cancel-btn') so the cancel path in
ExportApplicationModal is exercised for the exportData with completed=false;
update any related assertions that expect k8sKill to be called after rendering
via renderWithProviders and using the existing exportData/mockExportData setup.
In `@frontend/public/components/factory/__tests__/list-page.spec.tsx`:
- Around line 61-73: The test for TextFilter is asserting on onChange
immediately even though userEvent.type triggers multiple change events; update
the assertions to wait for the final call by using waitFor and assert on
toHaveBeenLastCalledWith (or check the last call) instead of
toHaveBeenCalledTimes(1) and toHaveBeenCalledWith; locate the test around the
TextFilter render and user.type invocation and replace the immediate count/check
with a waitFor that verifies onChange was called and that its last call received
the expected event and 'test-value' argument.
---
Outside diff comments:
In
`@frontend/packages/dev-console/src/components/deployments/__tests__/DeploymentForm.spec.tsx`:
- Around line 152-156: Move the two post-click assertions into the asynchronous
waitFor to avoid flakiness: after calling user.click(saveButton) keep the click,
then wrap both expect(saveButton.hasAttribute('disabled')).toBeTruthy() and
expect(saveButton.querySelector('.pf-v6-c-button__progress')).not.toBeNull()
inside a single await waitFor(() => { ... }) block so the saveButton disabled
state and the .pf-v6-c-button__progress element are asserted only after async
updates settle; reference the saveButton variable and the progress selector when
making the change.
---
Nitpick comments:
In
`@frontend/packages/knative-plugin/src/components/sink-source/__tests__/SinkSourceModal.spec.tsx`:
- Around line 43-48: The test file unconditionally overrides
HTMLFormElement.prototype.requestSubmit in the beforeAll block; make it
conditional and restore the original after tests by saving the original
requestSubmit, only assigning the polyfill if it is undefined, and restoring the
saved value in an afterAll cleanup. Update the existing
beforeAll/requestSubmitPolyfill usage and add a matching afterAll that
references the saved originalRequestSubmit to return behavior to its prior
state.
In
`@frontend/packages/knative-plugin/src/components/traffic-splitting/__tests__/TrafficSplittingModal.spec.tsx`:
- Around line 40-45: The global requestSubmit polyfill set in the beforeAll hook
unconditionally mutates HTMLFormElement.prototype and never restores it; modify
the test setup so beforeAll first saves the original
HTMLFormElement.prototype.requestSubmit (e.g., const _orig =
HTMLFormElement.prototype.requestSubmit), only assigns the polyfill if the
original is undefined, and add an afterAll that restores
HTMLFormElement.prototype.requestSubmit = _orig; keep the polyfill
implementation as the existing requestSubmitPolyfill function and use the same
event dispatch logic so test behavior remains identical while avoiding brittle
global mutation.
In
`@frontend/packages/webterminal-plugin/src/components/cloud-shell/__tests__/MultiTabbedTerminal.spec.tsx`:
- Around line 21-30: The helper clickMultipleTimes reuses a captured HTMLElement
across async clicks which can become detached after re-renders; change its
signature (the function clickMultipleTimes) to accept a getter (e.g.,
elementGetter: () => HTMLElement | null or a query callback) instead of a single
HTMLElement, and inside the loop call the getter each iteration and assert the
returned element is not null before awaiting user.click on it; update all call
sites (including the other instance referenced around the second occurrence) to
pass a function that queries the DOM for the current node so each click uses a
fresh, non-stale element.
🪄 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), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 10332720-6b23-4352-9c07-f1212708b573
📒 Files selected for processing (58)
.claude/skills/gen-rtl-test/SKILL.mdfrontend/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
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.
Files:
frontend/public/components/cluster-settings/__tests__/basicauth-idp-form.spec.tsxfrontend/public/components/cluster-settings/__tests__/htpasswd-idp-form.spec.tsxfrontend/packages/console-shared/src/components/alerts/__tests__/SwitchToYAMLAlert.spec.tsxfrontend/packages/container-security/src/components/__tests__/ImageVulnerabilityToggleGroup.spec.tsxfrontend/packages/console-shared/src/components/getting-started/__tests__/RestoreGettingStartedButton.spec.tsxfrontend/public/components/cluster-settings/__tests__/google-idp-form.spec.tsxfrontend/packages/console-app/src/components/modals/resource-limits/__tests__/ResourceLimitsModal.spec.tsxfrontend/packages/webterminal-plugin/src/components/cloud-shell/__tests__/MinimizeRestoreButton.spec.tsxfrontend/public/components/cluster-settings/__tests__/keystone-idp-form.spec.tsxfrontend/packages/dev-console/src/components/deployments/__tests__/ImagesSection.spec.tsxfrontend/packages/console-shared/src/components/editor/__tests__/CodeEditorToolbar.spec.tsxfrontend/packages/operator-lifecycle-manager/src/components/modals/__tests__/uninstall-operator-modal.spec.tsxfrontend/packages/topology/src/__tests__/TopologySideBar.spec.tsxfrontend/packages/operator-lifecycle-manager/src/components/__tests__/install-plan.spec.tsxfrontend/public/components/cluster-settings/__tests__/ldap-idp-form.spec.tsxfrontend/packages/dev-console/src/components/deployments/__tests__/EnvironmentVariablesSection.spec.tsxfrontend/packages/topology/src/components/export-app/__tests__/ExportApplicationModal.spec.tsxfrontend/public/components/cluster-settings/__tests__/openid-idp-form.spec.tsxfrontend/packages/dev-console/src/components/deployments/__tests__/AdvancedSection.spec.tsxfrontend/packages/console-shared/src/components/getting-started/__tests__/GettingStartedCard.spec.tsxfrontend/packages/console-shared/src/components/form-utils/__tests__/FormFooter.spec.tsxfrontend/packages/console-shared/src/components/progressive-list/__tests__/ProgressiveList.spec.tsxfrontend/packages/console-app/src/__tests__/hooks/useCSPViolationDetector.spec.tsxfrontend/packages/dev-console/src/components/deployments/__tests__/DeploymentStrategySection.spec.tsxfrontend/public/components/cluster-settings/__tests__/request-header-idp-form.spec.tsxfrontend/public/components/utils/__tests__/kebab.spec.tsxfrontend/public/components/cluster-settings/__tests__/github-idp-form.spec.tsxfrontend/public/components/modals/__tests__/replace-code-modal.spec.tsxfrontend/packages/knative-plugin/src/components/overview/__tests__/RevisionsOverviewList.spec.tsxfrontend/packages/dev-console/src/components/user-preferences/__tests__/SecureRouteFields.spec.tsxfrontend/packages/operator-lifecycle-manager/src/components/modals/__tests__/subscription-channel-modal.spec.tsxfrontend/packages/console-shared/src/components/formik-fields/__tests__/NumberSpinnerField.spec.tsxfrontend/packages/console-shared/src/test-utils/unit-test-utils.tsxfrontend/public/components/utils/__tests__/name-value-editor.spec.tsxfrontend/packages/console-shared/src/components/formik-fields/multi-column-field/__tests__/MultiColumnFieldFooter.spec.tsxfrontend/public/components/modals/__tests__/configure-update-strategy-modal.spec.tsxfrontend/packages/knative-plugin/src/components/traffic-splitting/__tests__/TrafficSplittingModal.spec.tsxfrontend/public/components/modals/__tests__/impersonate-user-modal.spec.tsxfrontend/packages/console-shared/src/components/form-utils/__tests__/FlexForm.spec.tsxfrontend/public/components/factory/__tests__/list-page.spec.tsxfrontend/packages/console-shared/src/components/formik-fields/key-value-file-input-field/__tests__/KeyValueFileInputField.spec.tsxfrontend/packages/knative-plugin/src/components/sink-source/__tests__/SinkSourceModal.spec.tsxfrontend/packages/operator-lifecycle-manager/src/components/descriptors/spec/__tests__/resource-requirements.spec.tsxfrontend/packages/console-shared/src/components/toast/__tests__/ToastProvider.spec.tsxfrontend/packages/dev-console/src/components/deployments/__tests__/DeploymentForm.spec.tsxfrontend/packages/webterminal-plugin/src/components/cloud-shell/__tests__/MultiTabbedTerminal.spec.tsxfrontend/packages/operator-lifecycle-manager/src/components/modals/__tests__/installplan-approval-modal.spec.tsxfrontend/packages/operator-lifecycle-manager-v1/src/components/cluster-extension/__tests__/ClusterExtensionForm.spec.tsxfrontend/public/components/cluster-settings/__tests__/gitlab-idp-form.spec.tsxfrontend/packages/console-shared/src/components/modals/__tests__/TextInputModal.spec.tsxfrontend/packages/console-shared/src/components/modals/__tests__/FetchProgressModal.spec.tsxfrontend/public/components/utils/__tests__/single-typeahead-dropdown.spec.tsxfrontend/public/components/cluster-settings/__tests__/test-utils.tsfrontend/public/components/modals/__tests__/impersonate-user-modal-integration.spec.tsxfrontend/packages/dev-console/src/components/deployments/__tests__/PauseRolloutsSection.spec.tsxfrontend/public/components/__tests__/storage-class-form.spec.tsxfrontend/public/components/utils/__tests__/download-button.spec.tsx
🔇 Additional comments (56)
frontend/packages/topology/src/__tests__/TopologySideBar.spec.tsx (2)
1-2: Good RTL migration import update.Switching to
userEventimport is aligned with current RTL interaction best practices and keeps the test API consistent with async user-driven events.
25-26: Async click flow is correctly implemented.Using
asynctest +userEvent.setup()+await user.click(...)is the right pattern, and the assertion remains clear and stable for this interaction.Also applies to: 34-35
frontend/packages/container-security/src/components/__tests__/ImageVulnerabilityToggleGroup.spec.tsx (2)
1-2:userEventimport migration looks correct.This aligns the test with the async interaction API used in the updated test body.
32-38: Async interaction update is implemented correctly.Using
asynctest +userEvent.setup()+await user.click(...)is the right RTL pattern and improves interaction realism/reliability.frontend/packages/webterminal-plugin/src/components/cloud-shell/__tests__/MinimizeRestoreButton.spec.tsx (2)
1-2: Nice cleanup:userEventimport aligns with RTL interaction best practices.
Good shift away from low-level event dispatching here.
38-38: Async interaction flow is correctly implemented.
UsinguserEvent.setup()plusawait user.click(...)for both button paths makes these assertions more realistic and stable.Also applies to: 54-54, 71-71
frontend/packages/dev-console/src/components/user-preferences/__tests__/SecureRouteFields.spec.tsx (2)
1-2: Clean test-library migration import update.The import changes are scoped and aligned with the PR objective to move from
fireEventtouserEvent.
39-52: Good async interaction handling in both dropdown tests.Using
userEvent.setup()withawait user.click(...)and querying the element withgetByTestIdbefore interaction makes these tests more deterministic and maintainable.Also applies to: 60-73
frontend/public/components/modals/__tests__/replace-code-modal.spec.tsx (1)
36-69: SoliduserEventmigration in interaction tests.These async click updates are consistent and improve test reliability versus synthetic sync events, with assertions preserved across all modal actions.
frontend/packages/console-app/src/__tests__/hooks/useCSPViolationDetector.spec.tsx (2)
1-1: Correct removal offireEventimport.The import change is appropriate since
fireEventis no longer used. The test now uses nativedocument.dispatchEvent()for CSP violation events, which is more explicit and suitable for security events.
81-81: Nativedocument.dispatchEvent()is the right approach for CSP violation events.Switching from RTL's
fireEvent(document, testEvent)to nativedocument.dispatchEvent(testEvent)is appropriate here. CSP violation events are security events on the document, not user interactions. Using the native DOM API:
- Makes the test intent clearer
- Removes unnecessary abstraction for non-interactive events
- Properly simulates how the browser dispatches these events
The continued use of
act()ensures React state updates are batched correctly.Also applies to: 92-92, 107-107, 122-122
frontend/packages/webterminal-plugin/src/components/cloud-shell/__tests__/MultiTabbedTerminal.spec.tsx (2)
39-41: Good userEvent setup for fake timers
beforeEachinitialization withuserEvent.setup({ advanceTimers: jest.advanceTimersByTime })is the right pattern for async user interactions under fake timers.
103-118: Nice improvement: re-querying close-tab controls per interactionRefreshing the close-tab list before each click is more robust than relying on previously indexed elements during stateful tab updates.
frontend/packages/knative-plugin/src/components/overview/__tests__/RevisionsOverviewList.spec.tsx (1)
133-146: GooduserEventasync migration for click flow.
userEvent.setup()+await user.click(...)is the right pattern here, and the assertion still verifies the intended modal launcher behavior.frontend/packages/knative-plugin/src/components/traffic-splitting/__tests__/TrafficSplittingModal.spec.tsx (1)
68-72: Nice conversion to user-driven submit behavior.Clicking the Save button via
userEventis a stronger end-user interaction path than direct form submit dispatch.frontend/packages/knative-plugin/src/components/sink-source/__tests__/SinkSourceModal.spec.tsx (1)
82-86: GooduserEventsubmit-path update.This now exercises the form submit through the Save button, which better matches real interaction behavior.
frontend/packages/dev-console/src/components/deployments/__tests__/ImagesSection.spec.tsx (1)
3-4: SoliduserEventmigration in this spec.The updates consistently use
userEvent.setup()with awaited clicks, and the async assertions remain properly guarded withwaitFor.Also applies to: 45-45, 53-53, 62-62, 78-78, 95-95, 121-121, 138-138, 154-154
frontend/packages/dev-console/src/components/deployments/__tests__/EnvironmentVariablesSection.spec.tsx (1)
2-3: Good conversion to async user interactions.This migration is consistent and correctly awaited across add/select/delete flows.
Also applies to: 57-57, 60-60, 74-74, 79-79, 84-84, 89-89, 96-96, 99-99
frontend/packages/dev-console/src/components/deployments/__tests__/PauseRolloutsSection.spec.tsx (1)
1-2: Looks good.
userEvent.setup()andawait user.click(...)are applied correctly for this checkbox flow.Also applies to: 9-9, 18-18
frontend/packages/operator-lifecycle-manager/src/components/modals/__tests__/subscription-channel-modal.spec.tsx (1)
1-2: Clean async migration for modal channel-change tests.Awaited
user.clickusage and async test conversion are correctly applied.Also applies to: 87-87, 91-91, 94-94, 111-111, 115-115, 118-118, 132-133, 140-140
frontend/packages/operator-lifecycle-manager/src/components/modals/__tests__/uninstall-operator-modal.spec.tsx (1)
1-2: Nicely done on the interaction updates here.The uninstall flows now use
userEventwith proper awaiting and preserve existing async assertions.Also applies to: 80-80, 84-84, 104-104, 108-108, 133-133, 139-139, 147-147, 151-151
frontend/packages/operator-lifecycle-manager/src/components/__tests__/install-plan.spec.tsx (1)
1-2: This conversion is correct and stable.
await user.click(...)is the right replacement for this approval-path interaction.Also applies to: 311-311, 313-313
frontend/packages/operator-lifecycle-manager/src/components/modals/__tests__/installplan-approval-modal.spec.tsx (1)
1-2: Great improvement to user-realistic submit behavior.Switching from direct form submission events to clicking the Save button is a stronger, behavior-faithful test path.
Also applies to: 82-82, 88-88, 90-90, 107-107, 116-116, 118-118, 135-135, 138-138
frontend/packages/operator-lifecycle-manager/src/components/descriptors/spec/__tests__/resource-requirements.spec.tsx (1)
1-2: ExcellentuserEventmigration in this spec.Clearing/typing inputs and submitting via awaited clicks is correctly implemented and better matches real interaction timing.
Also applies to: 65-65, 70-78, 81-81, 162-162, 165-165
frontend/packages/operator-lifecycle-manager-v1/src/components/cluster-extension/__tests__/ClusterExtensionForm.spec.tsx (1)
117-232: Good asyncuserEventmigration in interaction-heavy testsThe updated
await user.*calls pluswaitFor/toHaveBeenLastCalledWithare a solid fit for per-keystrokeonChangebehavior and reduce flakiness across these cases.frontend/public/components/modals/__tests__/impersonate-user-modal-integration.spec.tsx (1)
57-409: Migration looks soundThe conversion to awaited
userEventinteractions preserves test behavior while matching async UI updates (waitFor) in the right places.frontend/public/components/modals/__tests__/impersonate-user-modal.spec.tsx (1)
111-581: SolidfireEvent→userEventconversionAsync user interactions are awaited consistently, and callback assertions remain properly guarded with
waitForwhere needed.frontend/public/components/modals/__tests__/configure-update-strategy-modal.spec.tsx (1)
65-129: Nice update for incremental input eventsUsing awaited
user.clear/user.typewithtoHaveBeenLastCalledWithis the right adaptation for userEvent-driven change sequences..claude/skills/gen-rtl-test/SKILL.md (1)
514-583: Guidance update is aligned and actionableThe revised rules/examples consistently reflect async
userEventusage and the recommended post-interaction waiting patterns.Also applies to: 998-1035, 1485-1492, 1616-1617
frontend/packages/topology/src/components/export-app/__tests__/ExportApplicationModal.spec.tsx (1)
95-139: Good interaction migration on create/restart flowsAwaited
user.clickusage is correctly applied and keeps call assertions deterministic for these action paths.frontend/packages/dev-console/src/components/deployments/__tests__/AdvancedSection.spec.tsx (1)
37-127: Looks goodThis is a clean
userEventmigration with proper async handling and unchanged behavioral assertions.frontend/packages/dev-console/src/components/deployments/__tests__/DeploymentStrategySection.spec.tsx (1)
21-191: Well-executed migrationAwaited
userEventclicks are applied consistently across dropdown/option interactions, and the existing async assertions remain stable.frontend/packages/dev-console/src/components/deployments/__tests__/DeploymentForm.spec.tsx (1)
2-3: GooduserEventmigration with async interactions.Using
userEvent.setup()+ awaitedclick/clear/typeis a solid update and makes these tests closer to real user behavior.Also applies to: 106-151, 162-191
frontend/public/components/cluster-settings/__tests__/google-idp-form.spec.tsx (1)
25-26: Async helper usage is correctly applied.These updates properly await field-verification helpers, which is the right pattern after moving interactions to async
userEventflows and should improve test stability.Also applies to: 35-36, 43-44, 52-53
frontend/packages/console-shared/src/components/alerts/__tests__/SwitchToYAMLAlert.spec.tsx (1)
1-2: Good migration fromfireEventto awaiteduserEventclick.The async test flow is correctly implemented and avoids timing issues common with synchronous click simulation.
Also applies to: 46-47, 52-52
frontend/public/components/cluster-settings/__tests__/ldap-idp-form.spec.tsx (1)
85-86: Async conversion for list-field assertions looks solid.Awaiting
verifyIDPListInputFieldsin each case is the correct change and should prevent false pass/fail behavior from unresolved async interactions.Also applies to: 95-96, 105-106, 115-116
frontend/public/components/cluster-settings/__tests__/openid-idp-form.spec.tsx (1)
91-92: Nice update to awaited async field-list verification.This is consistent with
userEventsemantics and improves determinism of these claim/scope interaction tests.Also applies to: 101-102, 111-112, 129-130
frontend/public/components/utils/__tests__/kebab.spec.tsx (1)
1-2:Kebabinteraction tests were migrated cleanly touserEvent.Using awaited clicks across these cases is the right move and keeps authorization/option-behavior assertions stable under async event processing.
Also applies to: 18-19, 23-23, 26-27, 31-31, 34-35, 39-39, 51-52, 63-63, 66-67, 78-78
frontend/packages/console-shared/src/components/progressive-list/__tests__/ProgressiveList.spec.tsx (1)
2-3: ProgressiveList click test migration is correct.Awaiting the click before asserting callback/state effects is the correct sequencing for async RTL interactions.
Also applies to: 53-54, 72-72
frontend/public/components/cluster-settings/__tests__/request-header-idp-form.spec.tsx (1)
43-44: Async helper adoption is applied consistently here.Awaiting both single-input and list-input helper calls is the right update and should reduce flaky behavior in this suite.
Also applies to: 58-59, 68-69, 89-90, 98-99, 108-109, 117-118, 126-127
frontend/public/components/cluster-settings/__tests__/github-idp-form.spec.tsx (1)
43-44: GitHub IDP test updates look good and consistent.The async/await conversion around helper-driven interactions is correctly implemented and aligned with
userEvent-based test behavior.Also applies to: 53-54, 61-62, 70-71, 86-86, 101-101, 109-109, 124-124
frontend/packages/console-shared/src/components/getting-started/__tests__/GettingStartedCard.spec.tsx (1)
1-2: Good user-event migration for click flows.Async
userEvent.setup()+await user.click(...)is correctly applied in both onClick tests and keeps behavior assertions intact.Also applies to: 45-46, 60-60, 64-65, 77-77
frontend/packages/console-shared/src/components/toast/__tests__/ToastProvider.spec.tsx (1)
2-3: Interaction updates are solid and less flaky.The awaited
user.clickmigration is consistent across button/link/close paths, and the anchor existence check before clicking is a good guard.Also applies to: 74-74, 102-102, 145-145, 175-176, 186-186, 209-209
frontend/public/components/cluster-settings/__tests__/gitlab-idp-form.spec.tsx (1)
41-42: Nice async alignment with shared field helpers.Converting these field checks to
async+await verifyInputField(...)is correct and improves test reliability.Also applies to: 51-52, 61-62, 69-70
frontend/packages/console-shared/src/components/editor/__tests__/CodeEditorToolbar.spec.tsx (1)
1-2: Correct conversion to user-event in dispatch test.The async test setup and awaited click are correctly implemented for this interaction path.
Also applies to: 52-53, 57-57
frontend/packages/console-shared/src/components/form-utils/__tests__/FormFooter.spec.tsx (1)
1-2: Handler-click test migration is clean.Using
userEvent.setup()and awaiting all three clicks is the right pattern for stable callback assertions.Also applies to: 85-86, 90-92
frontend/packages/console-shared/src/components/modals/__tests__/FetchProgressModal.spec.tsx (1)
1-2: Good coverage-preserving user-event conversion.The cancel and close interaction paths were migrated correctly with awaited clicks and retained async assertions.
Also applies to: 167-167, 182-182, 199-200, 209-209, 318-318, 341-341
frontend/public/components/utils/__tests__/download-button.spec.tsx (1)
1-2: Looks good for async click behavior.
userEventis applied correctly in both scenarios and preserves the intended fetch/downloading assertions.Also applies to: 35-35, 44-44, 52-52, 71-71
frontend/packages/console-shared/src/components/formik-fields/key-value-file-input-field/__tests__/KeyValueFileInputField.spec.tsx (1)
2-3: Touched/blur and add-entry interactions are migrated correctly.Using
user.click+user.tabfor validation andawait user.clickfor add-entry keeps these tests realistic and reliable.Also applies to: 45-49, 115-116
frontend/packages/console-shared/src/test-utils/unit-test-utils.tsx (1)
6-7: Solid migration of shared input helper touserEvent.The updated interaction path is async-safe and better reflects real user behavior while keeping assertions stable.
Also applies to: 187-190
frontend/packages/console-shared/src/components/form-utils/__tests__/FlexForm.spec.tsx (1)
1-2: Form submission tests look correct afteruserEventconversion.The tests now await user interactions and still validate submit behavior clearly.
Also applies to: 26-39, 54-67
frontend/packages/console-shared/src/components/formik-fields/__tests__/NumberSpinnerField.spec.tsx (1)
1-2:NumberSpinnerFieldtests are migrated cleanly touserEvent.The updated typing/click flows preserve behavior and improve realism of interaction simulation.
Also applies to: 29-31, 37-46
frontend/public/components/__tests__/storage-class-form.spec.tsx (1)
40-43: These test timing adjustments look good.Using the shared helper directly with explicit presence checks keeps the tests readable and reliable.
Also applies to: 48-53
frontend/public/components/utils/__tests__/single-typeahead-dropdown.spec.tsx (1)
1-2: Comprehensive and correctfireEvent→userEventmigration here.Keyboard and combobox interaction paths are now closer to real usage while preserving existing assertions.
Also applies to: 34-52, 62-79, 88-108, 123-143, 161-190, 198-226
frontend/public/components/cluster-settings/__tests__/test-utils.ts (1)
179-194: Async conversion ofverifyIDPListInputFieldsis well done.The helper now uses realistic user input operations while maintaining clear structural assertions for add/remove behavior.
Also applies to: 210-243
frontend/packages/console-shared/src/components/modals/__tests__/TextInputModal.spec.tsx (1)
1-2:TextInputModaltest migration is consistent and reliable.The async
userEventflows cover click, typing, and Enter submission behavior cleanly.Also applies to: 37-43, 48-55, 59-65, 71-79, 89-96, 107-125, 157-163, 176-183, 189-194
| it('should auto-generate namespace and service account names based on ClusterExtension name', async () => { | ||
| const user = userEvent.setup(); | ||
| renderWithProviders(<ClusterExtensionForm formData={{}} onChange={mockOnChange} />); | ||
|
|
||
| const nameInput = screen.getByLabelText('Name'); | ||
| fireEvent.change(nameInput, { target: { value: 'my-operator' } }); | ||
| const nameInput = screen.getByRole('textbox', { name: /^Name$/i }); | ||
| await user.clear(nameInput); | ||
| await user.type(nameInput, 'my-operator'); | ||
|
|
||
| await waitFor(() => expect(nameInput).toHaveValue('my-operator')); | ||
| // Should call onChange with auto-generated names | ||
| expect(mockOnChange).toHaveBeenCalledWith( | ||
| expect.objectContaining({ | ||
| metadata: expect.objectContaining({ name: 'my-operator' }), | ||
| }), | ||
| ); | ||
| await waitFor(() => { | ||
| expect(mockOnChange).toHaveBeenLastCalledWith( | ||
| expect.objectContaining({ | ||
| metadata: expect.objectContaining({ name: 'my-operator' }), | ||
| }), | ||
| ); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Test intent regressed: namespace/serviceAccount auto-generation is no longer verified
On Line 80 the test promises namespace/service account auto-generation, but the assertion block on Line 91 only checks metadata.name. This weakens coverage for the exact behavior this test is named for.
Proposed fix
await waitFor(() => {
expect(mockOnChange).toHaveBeenLastCalledWith(
expect.objectContaining({
metadata: expect.objectContaining({ name: 'my-operator' }),
+ spec: expect.objectContaining({
+ namespace: 'my-operator',
+ serviceAccount: expect.objectContaining({
+ name: 'my-operator-service-account',
+ }),
+ }),
}),
);
});📝 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.
| it('should auto-generate namespace and service account names based on ClusterExtension name', async () => { | |
| const user = userEvent.setup(); | |
| renderWithProviders(<ClusterExtensionForm formData={{}} onChange={mockOnChange} />); | |
| const nameInput = screen.getByLabelText('Name'); | |
| fireEvent.change(nameInput, { target: { value: 'my-operator' } }); | |
| const nameInput = screen.getByRole('textbox', { name: /^Name$/i }); | |
| await user.clear(nameInput); | |
| await user.type(nameInput, 'my-operator'); | |
| await waitFor(() => expect(nameInput).toHaveValue('my-operator')); | |
| // Should call onChange with auto-generated names | |
| expect(mockOnChange).toHaveBeenCalledWith( | |
| expect.objectContaining({ | |
| metadata: expect.objectContaining({ name: 'my-operator' }), | |
| }), | |
| ); | |
| await waitFor(() => { | |
| expect(mockOnChange).toHaveBeenLastCalledWith( | |
| expect.objectContaining({ | |
| metadata: expect.objectContaining({ name: 'my-operator' }), | |
| }), | |
| ); | |
| }); | |
| }); | |
| it('should auto-generate namespace and service account names based on ClusterExtension name', async () => { | |
| const user = userEvent.setup(); | |
| renderWithProviders(<ClusterExtensionForm formData={{}} onChange={mockOnChange} />); | |
| const nameInput = screen.getByRole('textbox', { name: /^Name$/i }); | |
| await user.clear(nameInput); | |
| await user.type(nameInput, 'my-operator'); | |
| await waitFor(() => expect(nameInput).toHaveValue('my-operator')); | |
| // Should call onChange with auto-generated names | |
| await waitFor(() => { | |
| expect(mockOnChange).toHaveBeenLastCalledWith( | |
| expect.objectContaining({ | |
| metadata: expect.objectContaining({ name: 'my-operator' }), | |
| spec: expect.objectContaining({ | |
| namespace: 'my-operator', | |
| serviceAccount: expect.objectContaining({ | |
| name: 'my-operator-service-account', | |
| }), | |
| }), | |
| }), | |
| ); | |
| }); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@frontend/packages/operator-lifecycle-manager-v1/src/components/cluster-extension/__tests__/ClusterExtensionForm.spec.tsx`
around lines 80 - 97, The test "should auto-generate namespace and service
account names based on ClusterExtension name" only asserts metadata.name but
must also verify the auto-generated namespace and serviceAccount fields; update
the assertion that checks mockOnChange (in the spec using ClusterExtensionForm
and mockOnChange) to expect objectContaining metadata.namespace and the service
account field (e.g., spec.serviceAccountName or metadata.serviceAccount
depending on impl) with the expected auto-generated values or patterns derived
from 'my-operator' so the test actually verifies namespace/serviceAccount
generation.
| it('should call k8sKill with correct data on click of cancel button when export app is in progress', async () => { | ||
| const user = userEvent.setup(); | ||
| const exportData = _.cloneDeep(mockExportData); | ||
| exportData.status.completed = false; | ||
| renderWithProviders( | ||
| <ExportApplicationModal name="my-export" namespace="my-app" exportResource={exportData} />, | ||
| ); | ||
|
|
||
| await act(async () => { | ||
| fireEvent.click(screen.getByTestId('export-restart-btn')); | ||
| await user.click(screen.getByTestId('export-restart-btn')); | ||
| }); |
There was a problem hiding this comment.
Cancel-flow test clicks the wrong button
On Line 146 the test states it validates cancel behavior, but Line 155 clicks export-restart-btn. That means the cancel path is not actually exercised.
Proposed fix
- await user.click(screen.getByTestId('export-restart-btn'));
+ await user.click(screen.getByTestId('export-cancel-btn'));📝 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.
| it('should call k8sKill with correct data on click of cancel button when export app is in progress', async () => { | |
| const user = userEvent.setup(); | |
| const exportData = _.cloneDeep(mockExportData); | |
| exportData.status.completed = false; | |
| renderWithProviders( | |
| <ExportApplicationModal name="my-export" namespace="my-app" exportResource={exportData} />, | |
| ); | |
| await act(async () => { | |
| fireEvent.click(screen.getByTestId('export-restart-btn')); | |
| await user.click(screen.getByTestId('export-restart-btn')); | |
| }); | |
| it('should call k8sKill with correct data on click of cancel button when export app is in progress', async () => { | |
| const user = userEvent.setup(); | |
| const exportData = _.cloneDeep(mockExportData); | |
| exportData.status.completed = false; | |
| renderWithProviders( | |
| <ExportApplicationModal name="my-export" namespace="my-app" exportResource={exportData} />, | |
| ); | |
| await act(async () => { | |
| await user.click(screen.getByTestId('export-cancel-btn')); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@frontend/packages/topology/src/components/export-app/__tests__/ExportApplicationModal.spec.tsx`
around lines 146 - 156, The test "should call k8sKill with correct data on click
of cancel button when export app is in progress" is clicking the wrong element;
change the user click target from the 'export-restart-btn' test id to the cancel
button test id (e.g., 'export-cancel-btn') so the cancel path in
ExportApplicationModal is exercised for the exportData with completed=false;
update any related assertions that expect k8sKill to be called after rendering
via renderWithProviders and using the existing exportData/mockExportData setup.
| it('calls onChange with event and new value when input changes', async () => { | ||
| const user = userEvent.setup(); | ||
| const onChange = jest.fn(); | ||
|
|
||
| renderWithProviders(<TextFilter onChange={onChange} defaultValue="" />); | ||
|
|
||
| const input = screen.getByRole('textbox'); | ||
| fireEvent.change(input, { target: { value: 'test-value' } }); | ||
| await user.type(input, 'test-value'); | ||
|
|
||
| expect(onChange).toHaveBeenCalled(); | ||
| expect(onChange).toHaveBeenCalledTimes(1); | ||
| expect(onChange).toHaveBeenCalledWith(expect.any(Object), 'test-value'); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Locate TextFilter implementation =="
fd -i 'text-filter.(ts|tsx|js|jsx)' frontend
echo
echo "== Show onChange-related logic in TextFilter implementation(s) =="
fd -i 'text-filter.(ts|tsx|js|jsx)' frontend | while read -r f; do
echo "--- $f ---"
rg -n -C3 'onChange|deboun|setTimeout|thrott|useDebounce' "$f" || true
done
echo
echo "== Show the potentially brittle assertions in this test file =="
rg -n -C2 'user\.type\(|toHaveBeenCalledTimes\(1\)|toHaveBeenCalledWith\(expect\.any\(Object\),\s*'"'"'test-value'"'"'\)' frontend/public/components/factory/__tests__/list-page.spec.tsxRepository: openshift/console
Length of output: 631
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== TextFilter component implementation =="
cat -n frontend/public/components/factory/text-filter.tsx
echo
echo "== Also check if there's any wrapping debounce hook used elsewhere =="
rg -n 'TextFilter|useDebounce|withDebounce' frontend/public/components/factory/ -A 2 -B 2Repository: openshift/console
Length of output: 6618
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Check other TextFilter tests or usage patterns =="
rg -B 5 -A 10 'TextFilter.*onChange' frontend/public/components/factory/__tests__/list-page.spec.tsx | head -80
echo
echo "== Check if waitFor is used in other parts of this test file =="
rg -n 'waitFor|screen\.' frontend/public/components/factory/__tests__/list-page.spec.tsx | head -30
echo
echo "== Check PatternFly TextInput usage patterns for onChange in console codebase =="
rg -l 'TextInput.*onChange|onChange.*TextInput' frontend --type ts --type tsx | head -5 | xargs -I {} rg -B 2 -A 2 'onChange' {} 2>/dev/null | head -60Repository: openshift/console
Length of output: 3327
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Check if this is a new test or modified test =="
git log -p --follow -S 'toHaveBeenCalledTimes(1)' -- frontend/public/components/factory/__tests__/list-page.spec.tsx | head -100
echo
echo "== Check if there are other 'user.type' tests with call count assertions =="
rg -B 5 -A 3 'user\.type.*await' frontend --type ts --type tsx | grep -A 8 'toHaveBeenCalledTimes' | head -40
echo
echo "== Search for PatternFly TextInput onChange behavior in codebase =="
rg 'TextInput.*onChange|onChange.*value' frontend --type ts --type tsx -C 2 | head -60Repository: openshift/console
Length of output: 4208
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Find ClusterExtensionForm tests =="
fd -i 'cluster.*extension.*form' frontend --type ts --type tsx | grep -i test | head -3
echo
echo "== Show the waitFor pattern from ClusterExtensionForm =="
find frontend -name '*cluster*extension*form*.spec.tsx' -o -name '*cluster*extension*form*.test.tsx' 2>/dev/null | head -1 | xargs cat 2>/dev/null | grep -B 5 -A 5 'toHaveBeenLastCalledWith' | head -40Repository: openshift/console
Length of output: 285
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Search for toHaveBeenLastCalledWith pattern in tests =="
rg 'toHaveBeenLastCalledWith' frontend --follow | head -20
echo
echo "== Search for waitFor with user.type pattern =="
rg -B 3 'user\.type' frontend | grep -B 5 'waitFor' | head -40
echo
echo "== Search for any ClusterExtensionForm test file =="
find frontend -name '*[Cc]luster*[Ee]xtension*' 2>/dev/null | head -5Repository: openshift/console
Length of output: 3640
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Show waitFor + user.type pattern from ClusterExtensionForm =="
rg -B 10 -A 3 'toHaveBeenLastCalledWith' 'frontend/packages/operator-lifecycle-manager-v1/src/components/cluster-extension/__tests__/ClusterExtensionForm.spec.tsx' | head -80
echo
echo "== Also search for waitFor import in that file =="
rg 'import.*waitFor|from.*testing-library' 'frontend/packages/operator-lifecycle-manager-v1/src/components/cluster-extension/__tests__/ClusterExtensionForm.spec.tsx' | head -10Repository: openshift/console
Length of output: 3197
user.type() triggers multiple onChange events—wrap assertion in waitFor()
The test follows the commit message's own acknowledgment: "userEvent.type() triggers onChange for each character typed." The assertion at lines 71–72 uses toHaveBeenCalledTimes(1) without waitFor(), which contradicts the pattern applied to similar tests in the same commit (e.g., ClusterExtensionForm). Replace toHaveBeenCalledTimes(1) and toHaveBeenCalledWith() with toHaveBeenLastCalledWith() wrapped in waitFor() to ensure the assertion waits for the final callback.
Suggested test assertion update
it('calls onChange with event and new value when input changes', async () => {
const user = userEvent.setup();
const onChange = jest.fn();
renderWithProviders(<TextFilter onChange={onChange} defaultValue="" />);
const input = screen.getByRole('textbox');
await user.type(input, 'test-value');
- expect(onChange).toHaveBeenCalled();
- expect(onChange).toHaveBeenCalledTimes(1);
- expect(onChange).toHaveBeenCalledWith(expect.any(Object), 'test-value');
+ await waitFor(() => {
+ expect(onChange).toHaveBeenLastCalledWith(expect.any(Object), 'test-value');
+ });
});-import { screen } from '@testing-library/react';
+import { screen, waitFor } from '@testing-library/react';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/public/components/factory/__tests__/list-page.spec.tsx` around lines
61 - 73, The test for TextFilter is asserting on onChange immediately even
though userEvent.type triggers multiple change events; update the assertions to
wait for the final call by using waitFor and assert on toHaveBeenLastCalledWith
(or check the last call) instead of toHaveBeenCalledTimes(1) and
toHaveBeenCalledWith; locate the test around the TextFilter render and user.type
invocation and replace the immediate count/check with a waitFor that verifies
onChange was called and that its last call received the expected event and
'test-value' argument.
|
/retest |
- ExportApplicationModal: Fix wrong button clicked in cancel test (line 155) - Test name says 'cancel button' but was clicking 'export-restart-btn' - Changed to correctly click 'export-cancel-btn' - configure-update-strategy-modal: Wrap onChange assertions in waitFor() - userEvent.type() triggers onChange for each character - Assertions need to wait for final onChange call - Added waitFor() around onChangeMaxUnavailable and onChangeMaxSurge - Added test harness with local state to properly test async updates - Removed problematic beforeEach that rendered for all tests Addresses CodeRabbit major findings from PR openshift#16265
- installplan-approval-modal: Add Manual radio click before Save - Save button stays disabled when selected strategy matches resource - Must change strategy first to enable Save and test submit flow - list-page (TextFilter): Wrap onChange assertion in waitFor() - userEvent.type() triggers onChange per character - Use toHaveBeenLastCalledWith instead of separate count check - name-value-editor: Add test harness and wrap assertions in waitFor() - Created NameValueEditorHarness to maintain state for controlled inputs - Wraps all mockUpdate assertions in waitFor() - Ensures typed values match controlled input state All changes address async timing issues with userEvent identified in PR openshift#16265
- Created ClusterExtensionFormWithState harness to maintain form state - Mirrors parent state updates so controlled fields reflect typed values - Simplified version input test to use user.type instead of multiple keyboard calls - Ensures all userEvent interactions work with properly controlled inputs Completes async userEvent fixes for PR openshift#16265
- Added assertions for spec.namespace (should match name) - Added assertions for spec.serviceAccount.name (should be name + '-service-account') - Fully addresses test name promise to verify auto-generation - Resolves CodeRabbit major finding #1 for ClusterExtensionForm Addresses CodeRabbit review comment on PR openshift#16265
|
/test all |
cajieh
force-pushed
the
update-claude-skills-rtl-best-practices
branch
3 times, most recently
from
6f8257e to
6ee4ffc
Compare
cajieh
force-pushed
the
update-claude-skills-rtl-best-practices
branch
from
6ee4ffc to
b4f9c59
Compare
|
/retest |
|
@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. |
2 participants
Summary by CodeRabbit
Tests
Documentation