-
Notifications
You must be signed in to change notification settings - Fork 282
[Remove Vuetify from Studio] Convert content library filter bar unit tests to Vue Testing Library #5653
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.
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
base: unstable
Are you sure you want to change the base?
[Remove Vuetify from Studio] Convert content library filter bar unit tests to Vue Testing Library #5653
Conversation
|
👋 Thanks for contributing! We will assign a reviewer within the next two weeks. In the meantime, please ensure that:
We'll be in touch! 😊 |
|
Hi @sharma-anushka - Files Changed tab is showing 56 changed files which I believe wasn't intended. Can you have a look? |
|
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. |
|
No problem @sharma-anushka , can happen. Thanks! |
eb8fe63 to
b2192f3
Compare
b2192f3 to
53dd04d
Compare
|
Hi @sharma-anushka, thank you! Before we assign a maintainer, I will first invite the community. |
|
📢 ✨ Community Review guidance for both authors and reviewers. |
|
Hi @sharma-anushka 🖐️ ,
|
|
Hi @AadarshM07. Thankyou for the considerations. I will make the changes and revert soon. |
|
Hi @AadarshM07 Ive made the changes you suggested. I hope this is okay now. Thankyou again for the review. |
|
Hi @ozer550, Ive made the changes according to the community review. If theres any other change required, let me know. |
ozer550
left a comment
There was a problem hiding this 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); |
There was a problem hiding this comment.
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(); |
There was a problem hiding this comment.
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); |
There was a problem hiding this comment.
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.
| let wrapper; | ||
| beforeEach(() => { | ||
| wrapper = makeWrapper(); | ||
| router |
There was a problem hiding this comment.
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?
| async function closeChipByText(user, text) { | ||
| const chip = await screen.findByText(text); | ||
|
|
||
| const closeButton = chip.closest('[data-test^="filter-chip"]').querySelector('i'); |
There was a problem hiding this comment.
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')?
Summary
Converted Content Library (CatalogFilterBar) unit tests from Vue Test Utils to Vue Testing Library.
Key Changes
mount()andwrapper.vmusage with Vue Testing Library’srender()and semantic queriesresetKeywords,resetCoach,removeLanguage)currentFilters)Test Coverage
References
closes #5649
Verification
Ran pnpm test -- catalogFilterBar.spec.js