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

Stake pools join quit stub tests #962

Merged
merged 5 commits into from
Nov 7, 2019

Conversation

piotr-iohk
Copy link
Contributor

@piotr-iohk piotr-iohk commented Nov 5, 2019

Issue Number

#895

Overview

  • Added some preliminary tests for join/quit stake pool endpoints...

Comments

@piotr-iohk piotr-iohk self-assigned this Nov 5, 2019
@piotr-iohk piotr-iohk added this to the List Stake Pool milestone Nov 5, 2019
@piotr-iohk piotr-iohk force-pushed the piotr/stake-pools-join-quit-stub-tests branch from 4bdd25b to 5484aec Compare November 6, 2019 19:40
@piotr-iohk piotr-iohk changed the title wip: Stake pools join quit stub tests Stake pools join quit stub tests Nov 6, 2019
@piotr-iohk
Copy link
Contributor Author

Addressed review comments and added few more cases.

Copy link
Member

@jonathanknowles jonathanknowles left a comment

Choose a reason for hiding this comment

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

Looks good to me! I've made a comment regarding a formatting suggestion.

r2 <- joinStakePool ctx p2 (w, "Secure Passprase")
expectResponseCode HTTP.status501 r2

it "STAKE_POOLS_JOIN_01 - I definitely can quit and join another" $ \ctx -> do
Copy link
Member

Choose a reason for hiding this comment

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

This line is only slightly over the line length limit, but I hope you can forgive me for using it to illustrate a point. 😄

Of course we want to make the description long enough that the test is self-documenting. However, the line length limit seemingly imposes a constraint on the description length.

Though perhaps there's a way to avoid the above issue. How about using a format similar to the following:

    it "STAKE_POOLS_JOIN_01: \
        \I definitely can quit and join another."
        $ \ctx -> do
            (_, p1:p2:_) <- eventually $
                unsafeRequest @[ApiStakePool] ctx listStakePoolsEp Empty
            w <- emptyWallet ctx
            ...

This format has the advantage of scaling nicely even when the description gets longer:

    it "LAUNCH_MISSILES_01: \
        \When initiating a missile attack, rockets are fired in such a way \
        \as to prevent excessive collateral damage to neighbouring friendly \
        \countries."
        $ \ctx -> do
            (_, p1:p2:_) <- eventually $
                unsafeRequest @[ApiStakePool] ctx listStakePoolsEp Empty
            w <- emptyWallet ctx
            ...

@piotr-iohk What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jonathanknowles agreed and fixed. :)

Copy link
Contributor

@paweljakubas paweljakubas 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 is going to be more tests checking pool id is present, valid in #970

@piotr-iohk
Copy link
Contributor Author

bors r+

iohk-bors bot added a commit that referenced this pull request Nov 7, 2019
962: Stake pools join quit stub tests r=piotr-iohk a=piotr-iohk

# Issue Number

#895

# Overview

<!-- Detail in a few bullet points the work accomplished in this PR -->

- [ ] Added some preliminary tests for join/quit stake pool endpoints...


# Comments

<!-- Additional comments or screenshots to attach if any -->

<!-- 
Don't forget to:

 ✓ Self-review your changes to make sure nothing unexpected slipped through
 ✓ Assign yourself to the PR
 ✓ Assign one or several reviewer(s)
 ✓ Once created, link this PR to its corresponding ticket
 ✓ Acknowledge any changes required to the Wiki
-->


Co-authored-by: Piotr Stachyra <[email protected]>
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Nov 7, 2019

Build succeeded

@iohk-bors iohk-bors bot merged commit a392603 into master Nov 7, 2019
@KtorZ KtorZ deleted the piotr/stake-pools-join-quit-stub-tests branch November 7, 2019 11:59
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.

4 participants