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

reduce usage of 'fixtureWallet' in the integration tests #416

Conversation

KtorZ
Copy link
Member

@KtorZ KtorZ commented Jun 14, 2019

Issue Number

Overview

  • I have replaced some fixtureWallet calls with emptyWallet where a funded wallet wasn't necessary.

Comments

There's a non-significant cost with the use of 'fixtureWallet' since it needs to restore a funded wallet
and wait until restoration. We also only have a limited number of those wallets, so it's better to keep
them for when we actually really need funded wallets. When testing parsing errors or wrong headers, there's no need for demanding a wallet with funds

@KtorZ KtorZ requested a review from piotr-iohk June 14, 2019 14:16
@KtorZ KtorZ self-assigned this Jun 14, 2019
Copy link
Contributor

@piotr-iohk piotr-iohk left a comment

Choose a reason for hiding this comment

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

lgtm 👍

There's a non-significant cost with the use of 'fixtureWallet' since it needs to restore a funded wallet
and wait until restoration. We also only have a limited number of those wallets, so it's better to keep
them for when we actually really need funded wallets. When testing parsing errors or wrong headers, there's
no need for demanding a wallet with funds
@KtorZ KtorZ force-pushed the KtorZ/remove-unnecessary-fixtureWallet-usage branch from a3ee634 to 8d11050 Compare June 14, 2019 15:54
@KtorZ
Copy link
Member Author

KtorZ commented Jun 14, 2019

Merging into #414 to group PRs and avoid multiple merges because of CI issues due to stack major bump today ....

@KtorZ KtorZ changed the base branch from master to KtorZ/368/merge-cardano-launcher June 14, 2019 16:39
@KtorZ KtorZ merged commit d89bed5 into KtorZ/368/merge-cardano-launcher Jun 14, 2019
@KtorZ KtorZ deleted the KtorZ/remove-unnecessary-fixtureWallet-usage branch June 14, 2019 16:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants