[Remove Vuetify from Studio] Convert supplementary item unit tests to Vue Testing Library#5937
Conversation
|
👋 Hi @sharma-anushka, 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 |
rtibblesbot
left a comment
rtibblesbot
left a comment
There was a problem hiding this comment.
Migration is mostly clean, but the uploadCompleteHandler test needs to be removed before merge.
CI failures are at the "Use Node.js" infrastructure step and are not caused by this PR. No UI files changed — Phase 3 skipped.
- blocking:
uploadCompleteHandlertest is a tautology — see inline - suggestion:
document.querySelectoron lines 58 and 69 — see inline - praise: Clean VTL migration — see inline
@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly
How was this generated?
Reviewed the pull request diff checking for:
- Correctness: bugs, edge cases, undocumented behavior, resource leaks, hardcoded values
- Design: unnecessary complexity, naming, readability, comment accuracy, redundant state
- Architecture: duplicated concerns, minimal interfaces, composition over inheritance
- Testing: behavior-based assertions, mocks only at hard boundaries, accurate coverage
- Completeness: missing dependencies, unupdated usages, i18n, accessibility, security
- Principles: DRY (same reason to change), SRP, Rule of Three (no premature abstraction)
- Checked CI status and linked issue acceptance criteria
- For UI changes: inspected screenshots for layout, visual completeness, and consistency
| await wrapper.setProps({ uploadCompleteHandler }); | ||
| wrapper.findComponent(Uploader).vm.uploadCompleteHandler({ id: 'file1' }); | ||
| renderComponent({ uploadCompleteHandler }); | ||
| uploadCompleteHandler({ id: 'file1' }); |
There was a problem hiding this comment.
blocking: This test is a tautology — it calls the mock directly (uploadCompleteHandler({ id: 'file1' })) and then asserts the mock was called. The component is not involved at all: removing :uploadCompleteHandler="uploadCompleteHandler" from SupplementaryItem.vue would not cause this test to fail.
Like the uploadingHandler test that was correctly removed, this test cannot be meaningfully migrated to VTL's user-behavior model (there's no user gesture that triggers upload completion). Remove it rather than keep dead coverage.
| expect(testwrapper.findComponent('[data-test="uploading"]').exists()).toBe(true); | ||
| it('uploading should be true if progress < 1', () => { | ||
| renderComponent({ progress: 0.5 }); | ||
| expect(document.querySelector('[data-test="uploading"]')).toBeInTheDocument(); |
There was a problem hiding this comment.
suggestion: Use screen.queryByTestId('uploading') instead of document.querySelector('[data-test="uploading"]'). The acceptance criteria require screen queries, and screen methods give better failure messages (they report what was in the DOM when the element wasn't found). Same pattern applies on line 69 — use screen.getByTestId('remove') as the click target.
| const wrapper = makeWrapper({ id: 'test-remove' }); | ||
| await wrapper.findComponent('[data-test="remove"]').trigger('click'); | ||
| expect(wrapper.emitted('remove')[0][0]).toBe('test-remove'); | ||
| const user = userEvent.setup(); |
There was a problem hiding this comment.
praise: Using userEvent.setup() + user.click() here is correct VTL practice — it dispatches real pointer and keyboard events rather than a synthetic trigger, giving more realistic interaction coverage.
|
👋 Hi @sharma-anushka, 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. |
2 participants
Summary
Refactor channelEdit/views/files/tests/supplementaryItem.spec.js test suite to use Vue Testing Library (VTL)
Important note
Removed: uploadingHandler test - was implementation-detail focused and doesn't translate to VTL's user-behavior model. Covered by 'uploading should be true if progress < 1' test instead.
References
closes #5791
Reviewer guidance
Ran
pnpm test channelEdit/views/files/__tests__/supplementaryItem.spec.jsAI usage
I used AI to migrate tests, however all changes were carefully reviewed.