Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Comments

[Remove Vuetify from Studio] Convert content library filter bar unit tests to Vue Testing Library#5653

Open
sharma-anushka wants to merge 9 commits intolearningequality:unstable from
sharma-anushka:fix-issue-5649
Open

[Remove Vuetify from Studio] Convert content library filter bar unit tests to Vue Testing Library #5653
sharma-anushka wants to merge 9 commits intolearningequality:unstable from
sharma-anushka:fix-issue-5649

Conversation

@sharma-anushka
Copy link
Contributor

@sharma-anushka sharma-anushka commented Jan 20, 2026

Summary

Converted Content Library (CatalogFilterBar) unit tests from Vue Test Utils to Vue Testing Library.

Key Changes

  • Replaced mount() and wrapper.vm usage with Vue Testing Library’s render() and semantic queries
  • Removed direct calls to component methods (resetKeywords, resetCoach, removeLanguage)
  • Tests now simulate real user interactions (clicking buttons and filter chips)
  • Router state is validated through actual navigation updates instead of manual mutation
  • Eliminated assertions on internal computed state (currentFilters)
  • Improved test resilience by asserting only user-observable behavior

Test Coverage

  • Clear all filters via UI interaction
  • Remove text-based filters (keywords)
  • Remove boolean-based filters (coach)
  • Remove list-based filters incrementally (languages)
  • Ensure unrelated query parameters remain unaffected

References

closes #5649

Verification

Ran pnpm test -- catalogFilterBar.spec.js

Pasted Graphic

Copy link

👋 Thanks for contributing!

We will assign a reviewer within the next two weeks. In the meantime, please ensure that:

  • You ran pre-commit locally
  • All issue requirements are satisfied
  • The contribution is aligned with our Contributing guidelines. Pay extra attention to Using generative AI. Pull requests that don't follow the guidelines will be closed.

We'll be in touch! 😊

Copy link
Member

MisRob commented Jan 20, 2026

Hi @sharma-anushka - Files Changed tab is showing 56 changed files which I believe wasn't intended. Can you have a look?

Copy link
Contributor Author

Yes @MisRob, I think I have forgotten to drop the changes from the other issue, Ill soon revert with only the intended changes. Sorry for the inconvenience.

Copy link
Member

MisRob commented Jan 20, 2026

No problem @sharma-anushka , can happen. Thanks!

Copy link
Contributor Author

sharma-anushka commented Jan 20, 2026 •
edited
Loading

Hi @MisRob, Ive dropped the changes from the issue #5645. You may have a look.

@MisRob MisRob changed the title (削除) Fix issue 5649 (削除ここまで) (追記) [Remove Vuetify from Studio] Convert content library filter bar unit tests to Vue Testing Library (追記ここまで) Jan 21, 2026
Copy link
Member

MisRob commented Jan 21, 2026 •
edited
Loading

Hi @sharma-anushka, thank you! Before we assign a maintainer, I will first invite the community.
sharma-anushka reacted with heart emoji

Copy link
Member

MisRob commented Jan 21, 2026 •
edited
Loading

📢 ✨ Community Review guidance for both authors and reviewers.

Copy link
Contributor

AadarshM07 commented Jan 22, 2026 •
edited
Loading

Hi @sharma-anushka 🖐️ ,
I tested the current implementation, and it works really well. I’m noting down a few considerations I found. Sorry if I’m mistaken anywhere anywhere, you can always clarify!

  • The tests currently import a component createLocalVue from @vue/test-utils. To fully migrate away from @vue/test-utils, we should replace createLocalVue by passing plugins directly to the render options or using the configureVue hook.
  • The code contains document.body.innerHTML = '' in afterEach. The @testing-library/vue library automatically cleans up the DOM after every test. As of this, i think manual cleanup here is not needed and might even interfere with VTL’s cleanup process
  • Using .v-chip__close and .v-chip relies on Vuetify implementation. This could be risky, especially since there is an ongoing effort in Studio to remove Vuetify. To ensure tests don’t break later, it would be better to target elements by adding data-test attribute, which is already widely used in the codebase.

Copy link
Contributor Author

Hi @AadarshM07. Thankyou for the considerations. I will make the changes and revert soon.
AadarshM07 reacted with thumbs up emoji

Copy link
Contributor Author

Hi @AadarshM07 Ive made the changes you suggested. I hope this is okay now. Thankyou again for the review.
AadarshM07 reacted with thumbs up emoji

Copy link
Contributor Author

Hi @ozer550, Ive made the changes according to the community review. If theres any other change required, let me know.

Copy link
Member

@ozer550 ozer550 left a comment

Choose a reason for hiding this comment

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

Its almost there, just couple of questions and changes that came to my mind.

import CatalogFilterBar from '../CatalogFilterBar';

const store = factory();
Vue.use(Vuex);
Copy link
Member

@ozer550 ozer550 Feb 3, 2026

Choose a reason for hiding this comment

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

I think this is redundant as we already have it registered in jest_config/setup.js .


await waitFor(() => {
expect(router.currentRoute.query.keywords).toBeUndefined();
expect(router.currentRoute.query.coach).toBeTruthy();
Copy link
Member

@ozer550 ozer550 Feb 3, 2026

Choose a reason for hiding this comment

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

Instead of toBeTruthy can we use something more precise like toBe ?

import CatalogFilterBar from '../CatalogFilterBar';

const store = factory();
Vue.use(Vuex);
Copy link
Member

@ozer550 ozer550 Feb 3, 2026

Choose a reason for hiding this comment

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

This might be redundant we already import this in jest_config/setup.js.

sharma-anushka reacted with thumbs up emoji
let wrapper;
beforeEach(() => {
wrapper = makeWrapper();
router
Copy link
Member

@ozer550 ozer550 Feb 3, 2026

Choose a reason for hiding this comment

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

Router is set up in both makeWrapper() and beforeEach() is there a specific reason for this?

Copy link
Contributor Author

@sharma-anushka sharma-anushka Feb 9, 2026

Choose a reason for hiding this comment

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

Hi, No. Thankyou for catching it, I have removed it now.

async function closeChipByText(user, text) {
const chip = await screen.findByText(text);

const closeButton = chip.closest('[data-test^="filter-chip"]').querySelector('i');
Copy link
Member

@ozer550 ozer550 Feb 3, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Should we rely on better semantic tag than querySelector('i')?

Copy link
Contributor Author

Hi @ozer550, apologies for the delay. I have refactored it according to you feedback. You may have a look. Thanks.

Copy link
Member

@ozer550 ozer550 left a comment

Choose a reason for hiding this comment

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

First pass looks good, just one small comment that can be removed. Also, a small suggestion its helpful to add commit messages that are more clearer and that might help reviewers to understand the progression of the PR. Personal commit messages are usually not very informative.

const store = factory();

return render(CatalogFilterBar, {
// localVue,
Copy link
Member

Choose a reason for hiding this comment

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

We can remove this comment!

Copy link
Contributor Author

Hi @ozer550, Ive removed the unwanted comment, will keep commit messages clearer going forward.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ozer550 ozer550 Awaiting requested review from ozer550

Requested changes must be addressed to merge this pull request.

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Remove Vuetify from Studio] Convert content library filter bar unit tests to Vue Testing Library

AltStyle によって変換されたページ (->オリジナル) /