enzyme -> RTL: convert the CodeEditor component suites - #464
Merged
Conversation
Migrate components/CodeEditor (CodeEditor, VariablesDetail, VariablesField) off enzyme/mountWithContexts onto renderWithContexts (React Testing Library). react-ace is opaque under jsdom, so editor structure/mode/ readOnly/validation are asserted via DOM proxies; behaviour is otherwise preserved.
Contributor
There was a problem hiding this comment.
⚠️ Not ready to approve
The RTL conversions significantly reduce meaningful coverage of value propagation and onChange wiring (especially for CodeEditor/VariablesField), and key behaviors can be preserved by mocking react-ace/CodeEditor instead of relying on jsdom’s incomplete Ace DOM.
Pull request overview
Migrates the CodeEditor component test suites from Enzyme (mountWithContexts) to React Testing Library (renderWithContexts) as part of the ongoing enzyme → RTL conversion effort in the UI codebase.
Changes:
- Replaced Enzyme mounting/simulation with RTL
renderWithContexts+userEventinteractions across CodeEditor-related suites. - Updated assertions to use DOM-observable behavior (toggle state, modal presence, helper-text errors) given jsdom limitations with
react-ace. - Added in-test documentation explaining which former value/onChange assertions were dropped due to
react-acenot surfacing value changes to the DOM under jsdom.
File summaries
| File | Description |
|---|---|
| awx/ui/src/components/CodeEditor/VariablesField.test.js | Converted VariablesField tests to RTL; updated assertions around mode toggling, errors, tooltip, modal, and Formik submit. |
| awx/ui/src/components/CodeEditor/VariablesDetail.test.js | Converted VariablesDetail tests to RTL; asserted mode via toggle button state and reduced value-format assertions due to Ace DOM limits. |
| awx/ui/src/components/CodeEditor/CodeEditor.test.js | Converted CodeEditor tests to RTL; now asserts DOM artifacts (editor container/textarea/id/readonly) instead of Ace props/onChange. |
Copilot's findings
- Files reviewed: 3/3 changed files
- Comments generated: 4
Note
Your feedback helps us improve the quality of this feature.
Please use 👍 or 👎 to tell us whether this assessment is correct.
Comment on lines
+20
to
+37
| it('should render the ace editor in the requested mode', () => { | ||
| const onChange = jest.fn(); | ||
| const wrapper = mountWithContexts( | ||
| <CodeEditor value={'---\nfoo: bar'} onChange={onChange} mode="yaml" /> | ||
| const { container } = renderWithContexts( | ||
| <CodeEditor | ||
| id="code" | ||
| value={'---\nfoo: bar'} | ||
| onChange={onChange} | ||
| mode="yaml" | ||
| /> | ||
| ); | ||
| const aceEditor = wrapper.find('AceEditor'); | ||
| expect(aceEditor.prop('mode')).toEqual('yaml'); | ||
| expect(aceEditor.prop('setOptions').readOnly).toEqual(false); | ||
| expect(aceEditor.prop('value')).toEqual('---\nfoo: bar'); | ||
| }); | ||
|
|
||
| it('should trigger onChange prop', () => { | ||
| debounce.mockImplementation((fn) => fn); | ||
| const onChange = jest.fn(); | ||
| const wrapper = mountWithContexts( | ||
| <CodeEditor value="---" onChange={onChange} mode="yaml" /> | ||
| ); | ||
| const aceEditor = wrapper.find('AceEditor'); | ||
| aceEditor.prop('onChange')('newvalue'); | ||
| expect(onChange).toHaveBeenCalledWith('newvalue'); | ||
| // editor container + hidden textarea are rendered | ||
| expect(container.querySelector('.ace_editor')).toBeInTheDocument(); | ||
| const textarea = container.querySelector('textarea'); | ||
| expect(textarea).toBeInTheDocument(); | ||
| // CodeEditor copies its `id` prop onto the editor textarea | ||
| expect(textarea).toHaveAttribute('id', 'code'); | ||
| // not read only -> textarea is editable | ||
| expect(textarea).not.toHaveAttribute('readonly'); |
Comment on lines
+131
to
+155
| @@ -180,91 +145,54 @@ describe('VariablesField', () => { | |||
| )} | |||
| </Formik> | |||
| ); | |||
| await act(async () => { | |||
| wrapper.find('CodeEditor').invoke('onChange')('---\nnewval: changed'); | |||
| wrapper.find('form').simulate('submit'); | |||
| }); | |||
|
|
|||
| expect(handleSubmit).toHaveBeenCalled(); | |||
| await user.click(screen.getByText('Submit')); | |||
| await waitFor(() => expect(handleSubmit).toHaveBeenCalled()); | |||
| expect(handleSubmit.mock.calls[0][0]).toEqual({ | |||
| variables: '---\nnewval: changed', | |||
| variables: '---\nfoo: bar\n', | |||
| }); | |||
| // NOTE: original first drove the CodeEditor's onChange to change the value | |||
| // before submitting; ace edits are not observable/drivable under jsdom, so | |||
| // this asserts submission of the (unchanged) initial field value instead. | |||
|
|
||
| test('should update value if prop changes', () => { | ||
| const wrapper = mountWithContexts( | ||
| test('should update mode when prop changes', async () => { |
| import { renderWithContexts } from '../../../testUtils/rtlContexts'; | ||
| import VariablesDetail from './VariablesDetail'; | ||
|
|
||
| jest.mock('../../api'); |
Mock react-ace in CodeEditor to assert the mode/value/readOnly/onChange contract; mock CodeEditor in VariablesField to drive an edit and assert the submitted value changes; rename the VariablesDetail mode-preservation test and drop its unused api mock.
Contributor
Author
|
Thanks for the review. Pushed a commit: mock react-ace in the CodeEditor test to assert the mode, value, readOnly, and onChange contract, mock CodeEditor in VariablesField to drive an edit and assert the submitted value changes, rename the misleading VariablesDetail mode test, and drop its unused api mock. |
cigamit
approved these changes
Jun 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
SUMMARY
Converts the CodeEditor component test suite (
components/CodeEditor) from enzyme to React Testing Library, continuing the enzyme → RTL migration (components, one directory per PR).Files migrated off
mountWithContexts/enzyme ontorenderWithContexts:CodeEditor,VariablesDetail,VariablesField.ISSUE TYPE
COMPONENT NAME
ADDITIONAL INFORMATION
npm testforcomponents/CodeEditor: 3 suites, 15 tests, all passing. ESLint clean (--no-ignore). No production code changed — test-only.react-ace under jsdom: the editor renders a
.ace_editorcontainer + hidden<textarea>(which carries theidandreadOnly), but the controlled value lives in ace's internal model — the textarea stays empty andfireEvent.changedoes not fireonChange. Coverage preserved via DOM proxies: editor mode (yaml/json) via the activeMultiButtonTogglebutton, validation via.pf-m-errorhelper text, the help tooltip via the Popover button, modal-expand viarole="dialog"+ a second editor mounting, and Formik submit via a real submit click.A few assertions that required reading or driving the ace value have no DOM equivalent under jsdom and are folded (each noted in-file): CodeEditor "trigger onChange"; VariablesDetail JSON-format/empty-default value checks; VariablesField "retain edited yaml"/JSON-format. The mode/conversion paths are still exercised; only the exact ace value text is unobservable. (This matches how react-ace value assertions were handled in the already-converted screen suites.)