-
Notifications
You must be signed in to change notification settings - Fork 19
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
feat: jest testing #29
Conversation
Coverage Report (0%)
|
Unused dependencies (4)
Unused devDependencies (6)
Unlisted dependencies (10)
Unlisted binaries (4)
Unused exports (1)
|
Looks like there's a lot of conflicts. Please handle those. |
Indeed. PR still on a draft though. We need to make a choice if we want to keep |
# Conflicts: # .gitignore # bun.lockb # package.json # src/github/types/env.ts # tests/__mocks__/db.ts # tests/main.test.ts
What do you want to do about the failing typecheck CI? If you can't find a way to work with Bun then we can switch back to yarn 1.x This union type stuff is really complicated to deal with @whilefoo rfc |
tests/main.test.ts
Outdated
})); | ||
|
||
class WebhooksMocked { | ||
// eslint-disable-next-line @typescript-eslint/naming-convention |
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.
Maybe just add this directive once at the top of the file?
I wonder if its too noisy to add this comment on every pull. I like how it was originally implemented, with inline warnings and errors on the file view instead. @gitcoindev perhaps you can look into logging all the errors in the form of annotations on the files view?
|
I agree. I will have a look at configuration and try to change this into annotations only. |
I looked into it. The 'bun:test' is a correct module but somehow I cannot get TypeScript to understand it. I read the docs, checked everything and it should work. Execution of the code also works fine, I am very confused of the reason why it complains. I might just silence the error there. |
Just the |
I removed it but it seems Knip is still complaining about it somehow. |
I'm not sure I guess we can gamble and merge. |
Resolves #26