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

Add unit test to check updates of decentralizationLevel #1736

Merged
merged 5 commits into from
Jun 12, 2020

Conversation

rvl
Copy link
Contributor

@rvl rvl commented Jun 10, 2020

Issue Number

#1693 can be moved to "QA" column once this is merged.

Overview

  • Enhances the "unit test" to check that protocol parameters can be updated to include decentralization level.
  • Removes "todo" comment from NETWORK_PARAMS - Able to fetch network parameters integration test case. The decentralisationParam in the shelley integration test genesis is 1.0 ⇒ 0% decentralised.

-- | A value that is not the same as what's in the test data genesis.
halfD :: DecentralizationLevel
halfD = DecentralizationLevel p
where Right p = mkPercentage (1/2)
Copy link
Member

Choose a reason for hiding this comment

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

It is sort of weird to have this unit test in the byron package; the decentralization level is something that exists only in Shelley.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK I moved it to the shelley package.

@@ -6,7 +6,7 @@ protocolParams:
protocolVersion:
minor: 0
major: 0
decentralisationParam: 1
decentralisationParam: 1 # means 0% decentralised
Copy link
Member

Choose a reason for hiding this comment

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

Weird that github shows this diff, whereas master decentralisationParam is actually 0.25 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well github doesn't show merge conflicts. It can only show a diff to the merge base.

@rvl rvl force-pushed the rvl/1693/d-param-test-case branch 2 times, most recently from 734dd56 to 84528b0 Compare June 12, 2020 11:45
@rvl rvl force-pushed the rvl/1693/d-param-test-case branch from 84528b0 to 92bf608 Compare June 12, 2020 11:50
Copy link
Member

@KtorZ KtorZ left a comment

Choose a reason for hiding this comment

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

bors r+

@KtorZ
Copy link
Member

KtorZ commented Jun 12, 2020

bors r-

@KtorZ
Copy link
Member

KtorZ commented Jun 12, 2020

bors r+

iohk-bors bot added a commit that referenced this pull request Jun 12, 2020
1736: Add unit test to check updates of decentralizationLevel r=KtorZ a=rvl

### Issue Number

#1693 can be moved to "QA" column once this is merged.

### Overview

- Enhances the "unit test" to check that protocol parameters can be updated to include decentralization level.
- Removes "todo" comment from `NETWORK_PARAMS - Able to fetch network parameters` integration test case. The `decentralisationParam` in the shelley integration test genesis is 1.0 ⇒ 0% decentralised.


Co-authored-by: Rodney Lorrimar <[email protected]>
Co-authored-by: KtorZ <[email protected]>
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Jun 12, 2020

Build failed

@KtorZ
Copy link
Member

KtorZ commented Jun 12, 2020

  test/unit/Cardano/Wallet/Shelley/NetworkSpec.hs:50:5: 
  1) Cardano.Wallet.Shelley.Network.getTxParameters Correct values are queried
       uncaught exception: ProcessHasExited
       ProcessDidNotStart "cardano-node" cardano-node: createProcess: runInteractiveProcess: exec: does not exist (No such file or directory)

Ho. We need to provision unit tests with cardano-node as well.

@rvl
Copy link
Contributor Author

rvl commented Jun 12, 2020

bors r+

@KtorZ KtorZ added the IMPROVEMENT Mark a PR as an improvement, for auto-generated CHANGELOG label Jun 12, 2020
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Jun 12, 2020

@iohk-bors iohk-bors bot merged commit 2787bbd into master Jun 12, 2020
@iohk-bors iohk-bors bot deleted the rvl/1693/d-param-test-case branch June 12, 2020 14:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
IMPROVEMENT Mark a PR as an improvement, for auto-generated CHANGELOG
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants