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.

Sign up for GitHub

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

Jump to bottom

Unify success post-condition for performSelection. #2968

Merged
merged 9 commits into from
Oct 12, 2021
Merged

Unify success post-condition for performSelection. #2968

merged 9 commits into from
Oct 12, 2021

Conversation

jonathanknowles
Copy link
Contributor

@jonathanknowles jonathanknowles commented Oct 12, 2021 •
edited
Loading

Issue Number

ADP-1037

Summary

This PR:

  • adds coverage to verify that selections returned by performSelection respect the SelectionLimit returned by computeSelectionLimit.
  • unifies the various post conditions for performSelection into a single post condition.

Details

This PR follows the pattern used in the Migration.Selection module:

  • all post conditions are unified into a single post condition:
    >>> verifySelection cs ps s == SelectionCorrect
  • the comment for performSelection refers to this single post condition.

This pattern has the following advantages:

  • we can avoid a violation of the DRY principle: we only need to state the properties we expect of a Selection in a single place. (The alternative is to repeat these properties in both the comment for performSelection and within CoinSelectionSpec itself.)
  • every failure condition has a record type that includes a detailed description of why the failure has occurred.
  • every failure condition is automatically pretty-printed by the test suite.

Example Test Failure

example-test-failure

@jonathanknowles
Add property prop_performSelection_onSuccess_selectionLimitRespected.
6bca834 
This tests that the selection limit is respected for successful selections.
@jonathanknowles
Add verifySelection function to verify the correctness of a selection.
9e75591 
We also add a property `prop_performSelection_onSuccess_isCorrect`.

We will use this property to replace the other correctness properties,
step by step, in later commits.
@jonathanknowles
Add verification failure case SelectionCollateralInsufficient.
98d44d0
@jonathanknowles
Add verification failure case SelectionCollateralUnsuitable.
ffdd565
@jonathanknowles
Add verification failure case SelectionLimitExceeded.
4f91ebd
@jonathanknowles
Add verification failure case SelectionDeltaInvalid.
e353d57
@jonathanknowles
Add type VerifySelectionProperty.
0905890
@jonathanknowles
Simplify description of post-condition for performSelection
0353a0b
@jonathanknowles
Simplify prop_performSelection_onSuccess.
fdf8209 
Since there is now only one property to check on success, we can
simplify the infrastructure around testing successful selections.
@jonathanknowles jonathanknowles self-assigned this Oct 12, 2021
@jonathanknowles
Copy link
Contributor Author

bors r+

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Oct 12, 2021

Build succeeded:

@iohk-bors iohk-bors bot merged commit 79b58b3 into master Oct 12, 2021
@iohk-bors iohk-bors bot deleted the jonathanknowles/adp-1037/unified-success-property branch October 12, 2021 07:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sevanspowell sevanspowell sevanspowell approved these changes

Assignees

@jonathanknowles jonathanknowles

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jonathanknowles @sevanspowell