Skip to content
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

Migrate to Vue Testing Library #510

Open
EshaanAgg opened this issue Dec 21, 2023 · 21 comments · May be fixed by #897
Open

Migrate to Vue Testing Library #510

EshaanAgg opened this issue Dec 21, 2023 · 21 comments · May be fixed by #897
Assignees
Labels
help wanted Open source contributors welcome type: proposal New feature or request

Comments

@EshaanAgg
Copy link
Contributor

EshaanAgg commented Dec 21, 2023

Blocked by

#439

Context

Vue Testing Library is a lightweight wrapper that builds on top of the DOM Testing Library by adding APIs for working with Vue components. It is built on top of @vue/test-utils, which is currently used in the KDS.

As per the documentation, Vue Testing Library does three things:

  1. Re-exports query utilities and helpers from the DOM Testing Library.
  2. Hides @vue/test-utils methods that conflict with Testing Library Guiding Principle.
  3. Tweaks some methods from both sources.

Product

KDS

The Value Add

I personally advocate for VTL because of two major reasons:

  1. Most of the products (OSS or otherwise) are migrating toward the Testing Library (or its framework wrappers like React Testing Library etc.), and thus the community support is much more. The libraries are actively maintained, and using them leads to similar-looking tests irrespective of the framework of your choice. We can also then install @testing-library/jest-dom to use the custom Jest matchers for the DOM, enabling us to write better and more expressive tests.
  2. The Testing Library is based on the principle that we should only test for user interactions, and never for the internal state of components, and thus hides some API exports that allow us to do so. This, in my opinion, again leads to better tests, that check for the functionality of the components and are not heavily invested in the internal implementation of the state and hooks of the component, as it should be for a component library.

Possible Tradeoffs

Refactoring the existing tests is additional work, but it would also provide a good foundation for the future if we want to invest heavily in testing.

PS

I would be happy to migrate an existing test to VTL to provide a proof of concept for the approach.

@MisRob MisRob added the type: proposal New feature or request label Jan 9, 2024
@MisRob
Copy link
Member

MisRob commented Jan 9, 2024

Hi @EshaanAgg, thank you for the proposal, I think you've raised some good points here. We will discuss this.

@MisRob
Copy link
Member

MisRob commented Jan 26, 2024

Hi @EshaanAgg, we'd be really happy to see the library in action as we believe that the basic idea and all its many features would be really helpful. It'd be great to see a proof of concept here in the KDS repository. We'd then looked at it, few of us tried it too, and then considered if we'd like to migrate fully (it's a part of larger discussion of the whole ecosystem of testing tools in all our products). From what I can say right now, it looks promising. Please let me know if you're still interested. Thanks so much for raising such a valuable suggestion, personally I'm really impressed by the library.

@MisRob
Copy link
Member

MisRob commented Jan 26, 2024

@EshaanAgg By the way, do you have any experience with migrating larger test suites? Here in KDS, it wouldn't be such a big deal to refactor everything if such a decision is made, but for example in Kolibri we have hundreds of unit tests. So in case we'd want to use it widely, I wonder what would be a good strategy to approach it. I assume that many tests may fail because of

2. Hides @vue/test-utils methods that are in conflict with Testing Library Guiding Principle.
3. Tweaks some methods from both sources.

By any chance, is there a way to limit Vue Testing Library usage just to new test suites?

@EshaanAgg
Copy link
Contributor Author

EshaanAgg commented Jan 27, 2024

Hey! I'm really glad that you liked the suggestion! Also, I would be totally interested in migrating a component or two in KDS to VTL, so that the team can review it and see how it fits into the ecosystem here. We can then work on adopting it partially or completely as the team decides! I'll get started on it!

@EshaanAgg By the way, do you have any experience with migrating larger test suites? Here in KDS, it wouldn't be such a big deal to refactor everything if such a decision is made, but for example in Kolibri we have hundreds of unit tests. So in case we'd want to use it widely, I wonder what would be a good strategy to approach it. I assume that many tests may fail because of

2. Hides @vue/test-utils methods that are in conflict with Testing Library Guiding Principle.
3. Tweaks some methods from both sources.

I myself, have not migrated any large test suites, but have seen many large OpenSource organizations do so. One example is the Jaegar UI project by CNCF. You can look at a few PRs here to see how we do not need to migrate the whole test suite simultaneously, and what the incremental migration process may look like (it's for React Testing Library, but the core spirit is the same).

On a high level, the adoption process would look like:

  • Add VTL and its required helpers to the codebase alongside enzyme.
  • Configure the test runners for VTL.
  • Migrate the already existing tests to use the same instead of enzyme.
  • Remove the libraries and the configuration related to enzyme when the migration is complete!

By any chance, is there a way to limit Vue Testing Library usage just to new test suites?

Yup totally. As I mentioned, it's totally our call as to which testing library we want to use in which components!

@EshaanAgg
Copy link
Contributor Author

@MisRob Tried adding VTL to the project, but the bottleneck seems to be the node 10 requirement. Even the earliest library versions require installing at least node 14. I see that issue #439 has already been created in the same direction. Is there any way I can help with it?

@MisRob
Copy link
Member

MisRob commented Jan 30, 2024

Ah, that's unfortunate. Thanks @EshaanAgg. I am going to find out more about #439 and will reach out back to you.

@MisRob
Copy link
Member

MisRob commented Jan 30, 2024

@EshaanAgg And thanks for explaining more about the migration, that's great news.

@MisRob
Copy link
Member

MisRob commented Jan 30, 2024

So @EshaanAgg, in regards to blocking #439, it's possible that @rtibbles will open some sub-tasks and labels them as help wanted, but overall we will need to deal with it internally. Thanks for asking.

@MisRob
Copy link
Member

MisRob commented Feb 2, 2024

@EshaanAgg We had an idea that we could experiment with this in Kolibri which uses Node 18. There are lots of test scenarios and many have troubles that the library could help with. If you'd be still interested in preparing a proof of concept for a couple of test files, but in Kolibri, please let me know.

@EshaanAgg
Copy link
Contributor Author

Hey! Even I had been looking for an opportunity to contribute to Kolibri recently, so this falls right into that ballpark. I'll open an issue in Kolibri regarding the same, and refer this there. Are there any files that you want me to take on first or do any work?

@MisRob
Copy link
Member

MisRob commented Feb 2, 2024

@EshaanAgg Wonderful. I think you don't need to be opening a new issue in Kolibri and just reference this one which I've just assigned you.

Are there any files that you want me to take on first or do any work?

Feel free to have a look around and choose. Some are more complex and some less. One idea I had that it may be good to refactor at least one file that uses Vue store, router, and perhaps some mocks too, so we can see how to best transition tricky parts? I also wanted to note that depending on the test suite you have open, you are definitely not required to follow exactly the existing scenarios - I'm pretty sure that in some tests we test internals, use lots of global states, etc. and we'd ideally move away from this over time. So if needed, if it's simpler you could also just look at what's being tested and come up with a different test for the corresponding part of a workflow.

If you needed some help with choosing a test scenario, you can let me know.

@MisRob
Copy link
Member

MisRob commented Dec 11, 2024

This is now unblocked :)!

@MisRob
Copy link
Member

MisRob commented Dec 11, 2024

@EshaanAgg I unassigned you because it appeared to be a stale assignment but if you're still around and interested to get back to it, we'll welcome it

@RONAK-AI647
Copy link
Contributor

@MisRob Can you assign it to me , i will research the issue till Learning Equality closed!!! If @EshaanAgg agrees.

@AlexVelezLl
Copy link
Member

Hey @RONAK-AI647! I see that you currently have some other issues assigned, so lets make sure to close them before jumping into this one 🤗.

@Pandaa007
Copy link

Hello @MisRob I would really like to work on this issue. Can you please assign it to me?

@MisRob
Copy link
Member

MisRob commented Jan 14, 2025

Hi @Pandaa007, thank you. I will assign you. I recommend closely studying @EshaanAgg's similar pull request in another repository learningequality/kolibri#11833

@Pandaa007
Copy link

Hello @MisRob thank you for assigning me this issue. I look forward to start working and get back with updates.

@akolson
Copy link
Member

akolson commented Jan 15, 2025

Hi @Pandaa007!

Please be sure to let us know in case you have any questions or comments. We look forward to collaborating with you too.

Thank you!

@Pandaa007 Pandaa007 linked a pull request Jan 15, 2025 that will close this issue
8 tasks
@Pandaa007
Copy link

Hello @MisRob I've made a PR please review it and let me know if any changes are required.

@MisRob
Copy link
Member

MisRob commented Jan 16, 2025

Thanks @Pandaa007, we will have a look!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Open source contributors welcome type: proposal New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants