[Remove Vuetify from Studio] Convert Content Library unit tests to Vue Testing Library

[vtushar06]

Summary

Converted Content Library (CatalogList) unit tests from Vue Test Utils to Vue Testing Library, focusing on user-observable behavior and removing implementation detail testing.

Key Changes:

• Replaced mount() with render() and Vue Testing Library semantic queries
• Removed 2 unused mock functions (mockDownloadChannelsCSV, mockDownloadChannelsPDF)
• Consolidated 4 duplicate tests into 2 focused tests (10 total tests, all passing)
• Simplified test setup using real Vuex store (component uses mapActions/mapGetters)
• All tests now check visible outcomes: text content, button state, navigation

Test Coverage (10 tests):

• Initial load: Results display, download button visibility, search action
• Selection mode: Enter/exit selection, toolbar visibility, selection count
• Channel selection: Select-all checkbox and count display
• Search/filtering: Query parameter changes trigger search, results update
• Download workflow: Select button availability
• Offline/loading states tested through button disabled state

Verification:

Ran npm test -- catalogList.spec.js
It passed all 15 tests

…

References

Closes: #5527

…

Reviewer guidance

Tests focus on rendered content + business logic (VTL pattern)
No internal state access (removed tests on excluded array, component methods)

[MisRob]

Thank you @vtushar06, within next two weeks, we will assign a reviewer.

[vtushar06]

Hii @MisRob, I made some changes as you mentioned in my other PR about testing user-observable behavior instead of implementation details.
Please let me know If any more changes are required.

[vtushar06]

@MisRob ,I applied the approach from Issue #5474 and systematically removed stubs that weren't needed.
The remaining 4 stubs are minimal and necessary:

• ChannelItem: isolates component under test
• Checkbox: needed for selection mode testing
• BottomBar: needed for toolbar testing
• CatalogFilters: minimal stub

Tests now focus on user-visible behavior, Now PR is ready for review.

[MisRob]

@vtushar06

• Stubs that you added because of data-testid are not needed - just add test ID on actual components. Not that it'd be wrong, but it will be simpler approach for now, and more aligned with other tests.
• As for checkbox stub, you're basically re-implementing some parts of checkbox logic in your stub, and then based on it test that everything works well. But, in actual user experience, this implementation is not used. So again, this results in tests that actually don't reflect on what's really happening. Instead, just click the actual checkbox as user.

Stubs can be useful on some occasions, but I would generally recommend that until you get a grasp on them, it will be simpler if you always attempt not to use them at all. Only add a stub after you tried all other approaches, or if not using a stub in results in more complex or significantly slower tests. In majority of cases, you won't need them.

[vtushar06]

Hi @MisRob,
Updated tests to use semantic queries (getByText) instead of test IDs. Can we move forward with this, as I have gone through merged PRs from their I thought this can be good approach for out changes.

[vtushar06]

Hii @MisRob, This PR is now ready for review, please assign someone to review this and let me know If any more changes required.

[marcellamaki]

Hi @vtushar06 - thanks for your work on this! You've made a good start (and I can see that you've already applied some feedback from Misha regarding simplification and removing stubs) on a challenging task, and much of the work here is looking good :)

I know that in some other conversations with Misha, you've discussed the value of using getByText to confirm the rendering something in the DOM, and that can be a really useful approach. Here, however, you've added a mock for the translations, which I think adds unnecessary complexity, and potentially also brittleness. I added one particular inline comment just for more detail.

Could you try an approach that uses either strings without having to mock them, or another simple check to confirm rendering? That will help keep the test suite as simple as possible.

Two ideas for how to think about it, and you may want to apply these differently to different scenarios:

• Is the string specifically what needs to be tested that it's rendered? If so, is there a way to do so without a mock?
• If the string is a "proxy" of some type of display or changed state in the UX, is there another way to confirm that a particular (div, button, etc.) is rendered in a way that wouldn't require mocking a string? (this might require for example adding a test id on a component, and then checking that instead).

Misha and I will both be away starting next week for end of year holidays, but I will be happy to re-review or answer any more questions in January. Happy new year!

[vtushar06]

Hi @marcellamaki, Thank you for the detailed feedback

I've updated the tests to remove the translation mocks entirely. Now the tests use regex patterns to match the actual rendered text from the component's $trs translations:

Changes I made Removed all $tr mocks and Using screen.getByText(/results found/i) to match actual rendered text
The component already translates text via $tr(), so the tests now verify the actual rendered output without any mocking complexity.

Ready for re-review when you're back in January. Happy holidays! 🎉

[Copilot]

Pull request overview

This PR converts the Content Library (CatalogList) unit tests from Vue Test Utils to Vue Testing Library, shifting from implementation-detail testing to user-observable behavior testing. The migration simplifies the test setup by using a real Vuex store and consolidates duplicate tests.

Key Changes:

• Replaced Vue Test Utils mount() with Vue Testing Library render() and semantic queries
• Consolidated duplicate tests from 15 to 10 focused tests
• Removed unused mock functions (downloadChannelsCSV, downloadChannelsPDF)

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[vtushar06]

Hii @marcellamaki, can we continue resolving this one, once you are free, please let me know. Thanks

[marcellamaki]

Hi @vtushar06 -- thank you for the updates here. This has removed a lot of the complexity! I am still a bit confused about why you've removed all of the data-test references and replaced them with string references. This is pretty similar to Misha's previous point:

just add test ID on actual components. Not that it'd be wrong, but it will be simpler approach for now, and more aligned with other tests
and your response here:
Updated tests to use semantic queries (getByText) instead of test IDs

we use data-test more broadly on the component to check if it renders, so it's just more consistent. Is there a particular reason you were using the semantic queries? I do think it would be better than using stubs, but I am having a hard time following why some of the cases you're not able to add a test id to the component itself, and then reference that. Were you having issues with state or rendering that added complexity?

It isn't that it's wrong, I just think if there are ways to continue to simplify a bit more, that would be ideal. Thank you! And please ask follow up questions if I am not being clear :)

[vtushar06]

Hi @marcellamaki,
Thank you for the patience,
Previously I used getByText (e.g., finding the button by its label) to strictly follow the Vue Testing Library "testing what the user sees."
Now,I see your point that this makes the tests brittle if translations change, I also noticed that while newer PRs (like #5556) use data-testid, this specific legacy component (CatalogList.vue) relies on data-test.
Now to ensure consistency without modifying the component code unnecessarily, I will not rename the attributes in CatalogList.vue. Instead, I am configuring the test file to recognize data-test as the ID:

configure({ testIdAttribute: 'data-test' });

This allows me to use stable queries like screen.getByTestId('select') for interactions, while keeping the component code clean. I've updated the PR with these changes too,
Please let me know do you prefer configuration or should I make change in vue file.

[MisRob]

I don't know if it was the best decision, but I guide people to simply use VTL's default data-testid - at least for now. Rather than re-configuring the attribute in just some files - eventually that would result in lots of inconsistencies and having to cleanup.

In long-term, it'd be best to choose just one and configure it globally for all tests - but I don't think that's in the scope of this work. If you'd be interested in it @vtushar06, you can do it, but it'd be a follow-up and before that I would need to check in with the team about which of the two we want.

[MisRob]

^ this concerns only naming choice
(and if you will wish to fix our id naming globally as a follow up, then whatever approach will be okay in the affected files in this PR ;)

Please still resolve Marcella's suggestion.

[vtushar06]

Thanks for guidance, @MisRob, That makes sense — keeping the approach consistent across files now definitely help in figuring things later.

Absolutely! I'm interested in the follow-up work. I'll wait for you to check in with the team about the preferred approach, and then I can get started on the global configuration. Thanks for the opportunity!

Also, I'll get Marcella's suggestion resolved right now.

[vtushar06]

And I do not know why linting is not getting fixed even though I ran lint command twice, can you help me with this.
Thanks