-
Notifications
You must be signed in to change notification settings - Fork 260
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
test(api): api package test + few adjustments #518
Conversation
🦋 Changeset detectedLatest commit: 3d68a63 The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
@@ -0,0 +1,27 @@ | |||
/* eslint-disable no-console */ |
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.
I think this test should be in the packages/integration
package.
packages/api/src/test/setup.ts
Outdated
} | ||
} | ||
|
||
const mockServer = setupServer(mockEndpointWeights, mockGetContractEvents); |
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.
I think this is not the best way how it could be tested. We already have integration tests, so I think we don't need a next integration test package. You can here mock responses by jest.spyOn(axios, 'request')
instead using the msw
server.
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.
good suggestion. on it
const callSpy = jest.fn(async () => await resolver.next()); | ||
const result = await callSpy(); |
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 is unnecessary. jest.fn()
should be used only in case, when you need to test some logic inside a testing class/function.
packages/api/src/test/setup.ts
Outdated
}; | ||
}[]; | ||
|
||
export class MockApi extends ApiModule { |
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.
I think this is not the best way to test ApiResolver. Tests should be small as possible. Here you want to test ApiResolver, but you have additionally config/modules etc. If you had an error in modules, you will get an error in ApiResolver tests. That is wrong.
I recommend move this test to packages/api/src/Resolver.test.ts
, and test only ApiResolver without modules.
name: API package test
New Pull Request
Checklist
Issue Description
Related issue: #
FILL_THIS_OUT
Solution Description