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

Fill "How to add automated tests to your plugin" page #754

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

s-blu
Copy link

@s-blu s-blu commented Jan 13, 2025

Edited

Hello,

I created a guide how to integrate jest to the obsidian example plugin and how to deal with the obsidian API when running code in a test environment to fill in the stub page "How to add automated tests to your plugin".

I tested the guide via a Windows Sandbox but would very appreciate if somebody else undertakes the steps to make sure that it's working as expected.

I was unsure what to do with https://publish.obsidian.md/hub/04+-+Guides%2C+Workflows%2C+%26+Courses/Guides/How+to+test+plugin+code+that+uses+Obsidian+APIs and if I should update it, as well. I'd love your input here. "Issue 13" of the obsidian API is not available anymore and the links under "Other people's suggestions" are not reachable. However, I am not quite aware if the guide now provide enough guidance to remove the section altogether or what's the best course of action is.

If you have pointers to improve reading flow or wording, it'd be very appreciated. Furthermore I have no experience with Obsidian Publish and do not know if I should have avoided any kind of syntax - I only controlled the look and feel in Obsidian itself.
Also, since this is my first contribution, I hope it was correct to change the tag to "evergreen".

Regards

Added

Checklist

Copy link
Contributor

@mProjectsCode mProjectsCode left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. I have marked some things I noticed while reading through it.


```ts
// myplugin.ts
import { Notice } from "obsidian";
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe use explicit import type { Notice } from "obsidian"; notation. Adding "verbatimModuleSyntax": true, to the TS config is a good idea anyways.

Copy link
Author

Choose a reason for hiding this comment

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

You have a point, but I still would like to keep it this way. If one uses a obsidian dependency, i.e. Notice, VSC creates an auto-import like this. While a type import would be cleaner, it is not necessary to use them to get the tests running and I'd like to only use and mention mandatory steps to keep the complexity lower.

Copy link
Contributor

Choose a reason for hiding this comment

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

That is fair. Though without explicit type imports, you are at the mercy of whatever the TS transpiler in this setup does.

@s-blu s-blu requested a review from mProjectsCode January 13, 2025 21:03
Copy link
Contributor

@mProjectsCode mProjectsCode left a comment

Choose a reason for hiding this comment

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

Noticed a few more small things, mostly with the code examples around inconsistent code style.

@s-blu
Copy link
Author

s-blu commented Jan 14, 2025

Noticed a few more small things, mostly with the code examples around inconsistent code style.

Sorry for that, I should have invested 5 minutes to configure prettier in the example plugin after all .. :) If I might ask you to have a look again? Thanks for your time!

@s-blu s-blu requested a review from mProjectsCode January 14, 2025 18:31
Copy link
Contributor

@mProjectsCode mProjectsCode left a comment

Choose a reason for hiding this comment

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

Almost there XD

Copy link
Contributor

@mProjectsCode mProjectsCode left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants