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

Fix unit test coverage - Closes #1764 #1832

Merged
merged 18 commits into from Apr 11, 2019

Conversation

Projects
None yet
4 participants
@Efefefef
Copy link
Contributor

commented Mar 20, 2019

What issue have I solved?

#1764

How have I implemented/fixed it?

Some unit tests are straight forward, but some seemed impossible to make the assertion work, so they contain TODO comments explaining that.

Happy to hear If anyone has some suggestions on what else to try to make those assertions work.

How has this been tested?

Run
npm run test

Review checklist

@Efefefef Efefefef changed the base branch from development to 1.14.0 Mar 20, 2019

@Efefefef Efefefef self-assigned this Mar 20, 2019

@slaweet slaweet changed the base branch from 1.14.0 to 1.15.0 Mar 22, 2019

Efefefef added some commits Mar 19, 2019

@Efefefef Efefefef assigned slaweet and unassigned Efefefef Mar 25, 2019

@Efefefef Efefefef force-pushed the 1764-fix-coverage branch from 742fb11 to 8d6c67a Mar 26, 2019

@Efefefef Efefefef assigned Efefefef and unassigned slaweet Mar 26, 2019

@slaweet slaweet modified the milestone: Version 1.15.0 Mar 29, 2019

@slaweet slaweet assigned slaweet and unassigned Efefefef Apr 2, 2019

@slaweet slaweet changed the base branch from 1.15.0 to 1.16.0 Apr 5, 2019

slaweet added some commits Apr 8, 2019

@slaweet slaweet force-pushed the 1764-fix-coverage branch from a1bab2e to 6d51611 Apr 9, 2019

@slaweet slaweet force-pushed the 1764-fix-coverage branch from 6d51611 to 0c60209 Apr 9, 2019

slaweet and others added some commits Apr 9, 2019

@slaweet slaweet requested a review from massao Apr 10, 2019

@massao
Copy link
Contributor

left a comment

Just some comments, but everything else seems to be fine.

*/
// TODO the assertion below should be replaced with the assertion above, but it doesn't work
expect(dispatch).to.have.been.calledWith();
});

This comment has been minimized.

Copy link
@massao

massao Apr 10, 2019

Contributor

In both tests the problem since you have the async function liskAPIClientSet, invoking the async function accountApi.getAccount, is the reason why the dispatch is not being called correctly, it's needed to update the functions and the test to wait for the api call being rejected as expected.

Something like this could work:

actions/peers.js

...
// eslint-disable-next-line max-statements
export const login = async (dispatch, getState, data, config) => {
...
export const liskAPIClientSet = data =>
  async (dispatch, getState) => { // eslint-disable-line max-statements
...

And making the tests asynchronous too.

actions/peers.test.js

it('...', async () => {
  ...
  await liskAPIClientSet('....');
  ...
})
Show resolved Hide resolved src/actions/search.test.js Outdated
Fix assertions in search actions tests
... by use of async-await

@slaweet slaweet force-pushed the 1764-fix-coverage branch from f459972 to 70d285e Apr 10, 2019

Fix assertions in peers actions test
... by use of async-await

@slaweet slaweet requested a review from massao Apr 10, 2019

@massao

massao approved these changes Apr 10, 2019

Copy link
Contributor

left a comment

🥇

@slaweet slaweet added the ready label Apr 11, 2019

@slaweet slaweet merged commit 76ca336 into 1.16.0 Apr 11, 2019

4 checks passed

Jenkins e2e tests e2e tests passed
Details
Jenkins test deployment Commit was deployed to test
Details
continuous-integration/jenkins/pr-merge This commit looks good
Details
security/snyk - package.json (LiskHQ) No manifest changes detected

@slaweet slaweet deleted the 1764-fix-coverage branch Apr 11, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.