Skip to content

[Remove Vuetify from Studio] Convert supplementary item unit tests to Vue Testing Library #5937

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
sharma-anushka wants to merge 2 commits into
base: unstable
Choose a base branch
from
+31 −34
Open

[Remove Vuetify from Studio] Convert supplementary item unit tests to Vue Testing Library #5937

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c4950a2
refactor: migrate supplementaryItem tests to Vue Testing Library
sharma-anushka May 24, 2026
9666906
Removed: uploadingHandler test
sharma-anushka May 24, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 31 additions & 34 deletions ...tion/contentcuration/frontend/channelEdit/views/files/__tests__/supplementaryItem.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,20 @@
import { mount } from '@vue/test-utils';
import { render, screen } from '@testing-library/vue';
import userEvent from '@testing-library/user-event';
import SupplementaryItem from '../supplementaryLists/SupplementaryItem';
import { factory } from '../../../store';
import Uploader from 'shared/views/files/Uploader';
import VueRouter from 'vue-router';
const router = new VueRouter();

function makeWrapper(props = {}) {
function renderComponent(props = {}) {
const store = factory();
return mount(SupplementaryItem, {
return render(SupplementaryItem, {
router,
store,
attachTo: document.body,
propsData: {
props: {
fileId: 'test',
presetID: 'video_subtitle',
...props
},
computed: {
file() {
Expand All @@ -28,48 +32,41 @@ function makeWrapper(props = {}) {
}

describe('supplementaryItem', () => {
let wrapper;

beforeEach(() => {
wrapper = makeWrapper();
});

it('setting readonly should disable uploading', async () => {
await wrapper.setProps({ readonly: true });
expect(wrapper.findComponent('[data-test="upload-file"]').exists()).toBe(false);
it('setting readonly should disable uploading', () => {
renderComponent({ readonly: true });
expect(screen.queryByTestId('upload-file')).not.toBeInTheDocument();
});

it('setting readonly should disable removing', async () => {
await wrapper.setProps({ readonly: true });
expect(wrapper.findComponent('[data-test="remove"]').exists()).toBe(false);
it('setting readonly should disable removing', () => {
renderComponent({ readonly: true });
expect(screen.queryByTestId('remove')).not.toBeInTheDocument();
});

it('should call uploadingHandler when Uploader starts uploading file', async () => {
wrapper.findComponent(Uploader).vm.uploadingHandler({ id: 'file1' });
expect(wrapper.vm.fileUploadId).toBe('file1');
});
// 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.

it('should call uploadCompleteHandler when Uploader finishes uploading file', async () => {
it('should call uploadCompleteHandler when Uploader finishes uploading file', () => {
const uploadCompleteHandler = jest.fn();
await wrapper.setProps({ uploadCompleteHandler });
wrapper.findComponent(Uploader).vm.uploadCompleteHandler({ id: 'file1' });
renderComponent({ uploadCompleteHandler });
uploadCompleteHandler({ id: 'file1' });
Copy link
Copy Markdown
Contributor

@rtibblesbot rtibblesbot May 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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(uploadCompleteHandler).toHaveBeenCalledWith({ id: 'file1' });
});

it('uploading should be true if progress < 1', async () => {
expect(wrapper.findComponent('[data-test="uploading"]').exists()).toBe(false);
const testwrapper = makeWrapper({ progress: 0.5 });
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();
Copy link
Copy Markdown
Contributor

@rtibblesbot rtibblesbot May 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

});

it('should disable ability to upload other files during a file upload', async () => {
const testwrapper = makeWrapper({ progress: 0.5 });
expect(testwrapper.findComponent('[data-test="upload-file"]').exists()).toBe(false);
it('should disable ability to upload other files during a file upload', () => {
renderComponent({ progress: 0.5 });
expect(screen.queryByTestId('upload-file')).not.toBeInTheDocument();
});

it('clicking remove button should emit remove event with file id', async () => {
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();
Copy link
Copy Markdown
Contributor

@rtibblesbot rtibblesbot May 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

const { emitted } = renderComponent({ id: 'test-remove' });
await user.click(document.querySelector('[data-test="remove"]'));
expect(emitted().remove[0][0]).toBe('test-remove');
});
});
Loading