-
Notifications
You must be signed in to change notification settings - Fork 962
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
Add testing setup #79
Conversation
Thanks for the PR. I think it's great to have a sample plugin with testing framework integrated for people to use, but I would rather we don't further complicate the official sample plugin template. In this case, I think it's best if we have your fork as a standalone fork, where people can choose to use if they'd like to write tests for their plugins. If you'd like to maintain the template for longer term we can look into adding links to your repo in our plugin developer docs. |
@lishid thanks for the quick review! This seems a highly requested feature given the thumbs up in #37. Please consider opening up the PR again pending discussions in the feature request. Please note that this is by far the simplest implementation of everything I tried out. If you look at how others solve testing obsidian plugins ( |
I want to chime in here to say thanks to @hjonasson and the Obsidian maintainers, but also to ask the maintainers to reconsider adding this functionality to the sample plugin. As someone who is contributing to an Obsidian plugin to the first time, it took me a while to find this PR. Before that, I looked at various other Obsidian plugins of varying popularity, and they all have fairly different, often quite complicated test setups. I think the reason for this is that currently, everyone is reinventing the wheel when it comes to testing their Obsidian plugins. Having an officially blessed way of writing tests would be amazing because
🚀 |
Thanks @x3ro ! This was my feeling exactly. Both about everyone needing to reinvent it for each plugin, and most setups being more complicated. I (biasedly) think this PR or a similar one would be a great addition to the sample plugin. |
After what just happened with toggle-list being disabled, adding simple Unit tests for side effects (like deleting global hot keys) would be super easy and valuable to the community. @lishid you should deffo reconsider adding testing and linting configs. They don’t need to be run/mandated but should at the very least be available in this project. I’ve managed and maintained several large scale APIs and I will tell you that this will improve your team’s velocity and your product’s value. Mid to long term it will just also serve to make team-Obsidian look better, and can be part of your gate-keeping criteria. Those are some pretty high-value wins, and easy. I would also, additionally, standardise on formatting ala AirBnB as there are a few other bugs/problems hidden in some plugins by inconsistent formatting and other code-readability issues. Obviously some of the plugins are written by novice software engineers (which is a great testament to Obsidian’s API and this template!) and you don’t want to hinder creation, but helping them to not make mistakes only adds value to your core value proposition - instead of damaging it like Obsidian’s reaction to toggle-list did, which was unfortunate. Trust me, you want to get ahead of this now before it becomes a really big headache. |
I am writing my own plugin and wanted to add tests. I get around the closed nature of
obsidian
by using Jest's manual mocks. I added three tests to establish a pattern for testingPlugin
s.Closes #37
PS: When researching if this was a solved problem, I came across
obsidian-full-calendar
which has much more detailed mock implementations. I leave that link here for posterity if someone wants more detailed mocks.