-
Notifications
You must be signed in to change notification settings - Fork 79
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
fix: Re-request wallet stats #2947
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Pull Request Test Coverage Report for Build 6433283166
💛 - Coveralls |
src/modules/worlds/sagas.spec.ts
Outdated
describe('when there is an address in the store', () => { | ||
it('should put the request action to fetch worlds wallet stats with the stored address', () => { | ||
return expectSaga(worldsSaga) | ||
.provide([[select(getAddress), address]]) |
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.
As expectSaga
runs the saga until completion and, when testing handleActionsThatTriggerFetchWorldsWalletStatsRequest
, the fetchWorldsWalletStatsRequest
action is put and, taking into consideration that the sagas also handles that action, the fetchWorldsWalletStatsRequest
should be ran as well. To prevent this, we can mock the put:
.provide([[select(getAddress), address]]) | |
.provide([[select(getAddress), address], [put(fetchWorldsWalletStatsRequest(address)), undefined]]) |
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'm certain that is unnecessary due to the nature of sagas.
fetchWorldsWalletStatsRequest
will be handled by the saga yes, but it will not proceed on any undeclared effect.
For example, it will run and stop were said in the code:
function* handlefetchWorldsWalletStatsRequest(action: FetchWalletWorldsStatsRequestAction) {
const { address } = action.payload
try {
// Stops here given that the call is not declared in the test
const stats: WorldsWalletStats | null = yield call([WorldsAPIContent, WorldsAPIContent.fetchWalletStats], address)
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.
Besides if I set the request put in the .provide, it will not call the .put that comes afterwards, making the test not declaratively useful given that the things tested are what comes after the .provide
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'm mistaken with this, updated it per your suggestion
ba5bc90
to
1642322
Compare
On clear deployment and deployment success