Convert assessment item editor unit tests to Vue Testing Library#6038
Convert assessment item editor unit tests to Vue Testing Library#6038vtushar06 wants to merge 1 commit into
Conversation
|
👋 Hi @vtushar06, thanks for contributing! For the review process to begin, please verify that the following is satisfied:
Also check that issue requirements are satisfied & you ran Pull requests that don't follow the guidelines will be closed. Reviewer assignment can take up to 2 weeks. |
|
📢✨ Before we assign a reviewer, we'll turn on |
🔵 Review postedLast updated: 2026-07-11 07:00 UTC |
rtibblesbot
left a comment
rtibblesbot
left a comment
There was a problem hiding this comment.
PR #6038 cleanly migrates AssessmentItemEditor.spec.js from @vue/test-utils to Vue Testing Library + userEvent. The conversion is genuinely behavioral — it drives real clicks/inputs and asserts on emitted payloads rather than reaching into vm, and replacing the shared listeners mock with per-test emitted() removes cross-test mutable state. CI passing. No UI source changed, so Phase 3 (visual QA) not applicable.
Non-blocking findings:
- suggestion (inline, L87): dropped the single→multiple-selection type-change case, a coverage regression vs the old four-case suite.
- suggestion (inline, L54):
openQuestionEditorselects the textbox bytagName, brittle vs the mock's rendering. - nitpick (inline, L176): lone
fireEvent.clickon the radio is inconsistent with the file'suserEventstyle. - nitpick — L65 test title says "and hints" but asserts none (hints render collapsed); trim the title or assert the collapsed section.
@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly
How was this generated?
Ran a phased review pipeline over the pull request diff:
- Classified the diff to select review passes (core, frontend, backend) and whether manual QA was required
- Core review pass checked correctness, design, architecture, testing, completeness, and DRY/SRP/Rule-of-Three principles
- Specialized frontend/backend review passes applied framework-specific lenses where those files changed
- For UI changes: manual QA and an accessibility audit against a live dev server, when available
- Checked CI status and linked issue acceptance criteria
- Synthesized one review from those passes and chose the verdict from the findings, CI status, and QA evidence
| }); | ||
| }); | ||
| }); | ||
| describe('changing the question type', () => { |
There was a problem hiding this comment.
suggestion: The old suite exercised four type transitions (single, multiple, true/false, input); this block covers only single choice, true/false, and numeric input. The single→multiple-selection case (asserting answers are preserved) was dropped — a distinct branch of the kind-change logic, so this is a real coverage regression, not just a rename. Consider adding a fourth case mirroring the others to keep the conversion behavior-preserving. If the omission is intentional, ignore.
| const openQuestionEditor = async user => { | ||
| await user.click(screen.getByTestId('questionText')); | ||
| // Both the type dropdown and the answers expose textboxes, so target the question editor's. | ||
| return screen.getAllByRole('textbox').find(el => el.tagName === 'TEXTAREA'); |
There was a problem hiding this comment.
suggestion: Selecting the textbox by el.tagName === 'TEXTAREA' keys off the mock's rendering rather than anything semantic — if the mock ever rendered a contenteditable div (as a real rich-text editor does) this silently returns undefined. Scoping via within(...).getByRole('textbox') around the question editor would be more robust; the edit-mode TipTapEditor carries no data-test, so a fully clean scope may need a test id added in production code. Acceptable as-is — flagging so the fragility is a conscious choice.
| ]; | ||
| // Selecting the second answer's correctness control makes it the correct one. | ||
| const radios = screen.getAllByRole('radio'); | ||
| await fireEvent.click(radios[1]); |
There was a problem hiding this comment.
nitpick: Most tests click via user.click(...), but this drops to fireEvent.click(radios[1]). fireEvent.update for v-model values is idiomatic and fine to keep, but prefer await user.click(radios[1]) here so the interaction style is uniform.
Summary
Converts
AssessmentItemEditor.spec.jsfrom@vue/test-utilsto Vue Testing Library (VTL), rewriting the suite to reflect how a user actually interacts with the question editor.Changes:
mount/shallowMount+wrapper.vm/findComponent/vm.$emitwithrender,screenqueries, anduserEvent@vue/test-utilsimportsMirrors the existing VTL pattern in the sibling
HintsEditor.spec.js.Screen recording:
References
Closes #5814
Sub-issue of #5789
Reviewer guidance
pnpm test AssessmentItemEditor— 8 tests pass