Skip to content
This repository has been archived by the owner on Apr 15, 2019. It is now read-only.

Adding more tests for register component - Closes #839 #848

Merged
merged 6 commits into from
Oct 10, 2017

Conversation

ginacontrino
Copy link
Contributor

What's the Problem?
The test coverage of the register component was poor + the component needed some refactoring #839

What did you do?

  • Added a login util to get rid of duplicated code
  • Tested the utils file
  • Added tests for the register component

How can I test this?

  • Run tests
  • Also check in the browser that the login and registration still work as expected

screen shot 2017-10-10 at 09 07 43

@ginacontrino ginacontrino changed the base branch from development to 1.2.0 October 10, 2017 09:03
Copy link
Contributor

@slaweet slaweet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most looks good, just one detail commented on below

it('should call activePeerSet with testnet if network index is testnet', () => {
const props = wrapper.find('Passphrase').props();

const loginData = stub(Utils, 'getLoginData');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please move the stub creation to beforeEach block and the stub restore to afterEach block. It avoids some code duplication and makes tests more stable because restore gets called even if there is some error in the middle of the test and no other test is affected. WIth restore inside the test case, in case of error in the test, the stub is not restored and the next test fails too because it is trying to stub something that already is a stub.

@ginacontrino ginacontrino merged commit 979a6e3 into 1.2.0 Oct 10, 2017
@ginacontrino ginacontrino deleted the 839-register-tests branch October 10, 2017 10:36
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants