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

More test coverage of selection errors. #2979

Merged
merged 7 commits into from
Oct 20, 2021

Conversation

jonathanknowles
Copy link
Contributor

Issue Number

ADP-1037

Summary

This PR adds more test coverage of selection errors.

These now all have the following pattern:

- verify<X>            : name of function that verifies property <X>
- Verify<X>Failure     : name of outer constructor for verification failure
- Verify<X>FailureInfo : name of inner constructor for verification failure

The use of `Failure` and `FailureInfo` avoids confusion with the term
`Error`, which will appear only in the names of verification functions
for `SelectionError` values.
In cases where it's possible for multiple values to fail verification, our
current verification functions go to the effort of pattern matching on and
only returning the first such value that failed verification.

However, we can simplify the code by just returning all values that fail
verification.

If callers only wish to report the first value, then callers still have
the option to do this.
We will eventually have the following type synonyms:

- VerifySelection
- VerifySelectionError

Shorter names are worth having here, to make these type synonyms less
verbose.
In this change we add pattern matches for the various types of errors
that we want to verify.
This commit adds coverage checks for the different failure conditions of
`performSelection`.

The coverage checks are rather numerous, so we also extract them out into a
separate function where they can be self-contained.
When testing a `SelectionLimitReachedError`, we don't necessarily have
access to the adjusted `outputsToCover` value, as `prepareOutputs` may
have adjusted the original outputs, which may give rise to a different
value of `SelectionLimit`.

So we add `outputsToCover` to the error record type to make testing easier.
@jonathanknowles jonathanknowles force-pushed the jonathanknowles/adp-1037/test-coin-selection-7 branch from ba93cb6 to 0c56da8 Compare October 20, 2021 04:37
@jonathanknowles jonathanknowles self-assigned this Oct 20, 2021
@jonathanknowles jonathanknowles force-pushed the jonathanknowles/adp-1037/test-coin-selection-7 branch from 0c56da8 to 03bb00b Compare October 20, 2021 05:40
@jonathanknowles jonathanknowles marked this pull request as ready for review October 20, 2021 05:50
@jonathanknowles
Copy link
Contributor Author

bors r+

iohk-bors bot added a commit that referenced this pull request Oct 20, 2021
2979: More test coverage of selection errors. r=jonathanknowles a=jonathanknowles

## Issue Number

ADP-1037

## Summary

This PR adds more test coverage of selection errors.

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

iohk-bors bot commented Oct 20, 2021

Build failed:

Failures:
 
  src/Test/Integration/Scenario/CLI/Shelley/Transactions.hs:243:59: 
  1) CLI Specifications, SHELLEY_CLI_TRANSACTIONS, TRANS_CREATE_06 - Invalid amount, string with wildcards
       uncaught exception: IOException of type ResourceVanished
       fd:131: hFlush: resource vanished (Broken pipe)
 
  To rerun use: --match "/CLI Specifications/SHELLEY_CLI_TRANSACTIONS/TRANS_CREATE_06 - Invalid amount/string with wildcards/"
 
Randomized with seed 1642389836
 
Finished in 881.4515 seconds, used 943.6028 seconds of CPU time
871 examples, 1 failure, 45 pending

Can't seem to find the log on hydra, but here's the failure on buildkite: https://buildkite.com/input-output-hk/cardano-wallet/builds/17337#162299be-70ce-4613-a1fb-9f9193c13795)

I'm going try rebuilding just this job before any new merge attempt (to avoid running into a "cached failure"). EDIT: The rebuild succeeded (https://hydra.iohk.io/build/8006913), so going to make a fresh merge attempt.

Failure ticket:
#2980

@jonathanknowles
Copy link
Contributor Author

bors r+

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Oct 20, 2021

Build succeeded:

@iohk-bors iohk-bors bot merged commit db038ac into master Oct 20, 2021
@iohk-bors iohk-bors bot deleted the jonathanknowles/adp-1037/test-coin-selection-7 branch October 20, 2021 13:04
Copy link

@FAVE01 FAVE01 left a comment

Choose a reason for hiding this comment

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

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.

3 participants