-
Couldn't load subscription status.
- Fork 149
chore: add type-safe ESLint #119
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
Conversation
1631038 to
003fe9c
Compare
003fe9c to
7164709
Compare
| try { | ||
| await integration.mcpClient().callTool({ name, arguments: arg }); | ||
| expect.fail("Expected an error to be thrown"); | ||
| throw new Error("Expected an error to be thrown"); |
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 was not working correctly so we had the false impression that extra fields are problematic with our zod setup; they're not so I removed places where we were using that as an invalid arg scenario
| "strictNullChecks": true, | ||
| "esModuleInterop": true, | ||
| "types": ["node"], | ||
| "types": ["node", "jest"], |
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 just seems to be the best way to get VSCode to stop complaining about the types once in a while but I think can work without too as long as others are represented
Pull Request Test Coverage Report for Build 14665235990Details
💛 - Coveralls |
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.
Nice!
| expect.fail("Expected an error to be thrown"); | ||
| throw new Error("Expected an error to be thrown"); | ||
| } catch (error) { | ||
| expect((error as Error).message).not.toEqual("Expected an error to be thrown"); |
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.
Isn't the assertion just below sufficient? I don't imagine Error is an instance of McpError.
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.
it is but then the error message is more confusing (it just says expected McpError got Error which originally made me think it was erroring differently instead of not erroring at all). I want to differentiate an external error from the intentional "did not end up erroring" error
c239276 to
140830c
Compare
Fixes #113
Largely did the minimum viable actions needed to get the linting to pass.
Also added
expectDefinedso we don't need to keep asserting our types manually.