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

Strengthen tests for applyTx and filterByAddress. #2867

Conversation

jonathanknowles
Copy link
Contributor

Comments

This PR strengthens test coverage for PR 2848.

Issue Number

#2848

Changes

  • Introduces CoArbitrary instance for Address, which enables us to generate arbitrary functions of type Address -> Bool.
  • Use arbitrary Address -> Bool functions to strengthen property tests that formerly relied on constant functions.
  • Adds relevant coverage checks to individual properties.

This allows the definition of a `CoArbitrary` instance for `Address`,
which makes it possible for QuickCheck to generate arbitrary functions
of type `Address -> t`, provided that type `t` has an `Arbitrary`
instance.
Instead of using constant functions, we use arbitrary functions of type
`Address -> Bool`.
…sts.

Instead of using constant functions, we use arbitrary functions of type
`Address -> Bool`.
It's perfectly valid for a transaction to have no outputs if all
incoming value is consumed in some other way (either through the fee, or
by burning tokens).
By conservatively reducing coverage expectations, this change reduces
the overall run time from around 20 seconds to around 1 second.
@jonathanknowles jonathanknowles self-assigned this Sep 3, 2021
@jonathanknowles jonathanknowles changed the title Strengthen tests for PR 2848 Strengthen tests for applyTx and filterByAddress. Sep 3, 2021
Comment on lines 1486 to 1488
cover 10
(lhs /= mempty && lhs `leq` rhs)
"lhs /= mempty && lhs `leq` rhs" $
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suggestion:

Suggested change
cover 10
(lhs /= mempty && lhs `leq` rhs)
"lhs /= mempty && lhs `leq` rhs" $
cover 10
(lhs /= mempty && lhs `leq` rhs && lhs /= rhs)
"lhs /= mempty && lhs `leq` rhs && lhs /= rhs" $

Copy link
Contributor Author

@jonathanknowles jonathanknowles Sep 3, 2021

Choose a reason for hiding this comment

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

Fixed in 7cfc013.

What we really want here is to capture the notion of "strictly less
than", but `leq` gives us "less than OR equal". We can't use `<`, as
token maps are only partially ordered. So we have to use a combination
of `leq` and `/=`.

In response to review comment:

https://github.com/input-output-hk/cardano-wallet/pull/2867/files#r701595441
@jonathanknowles jonathanknowles force-pushed the jonathanknowles/apply-tx-testable branch from ff9ef8a to 7cfc013 Compare September 3, 2021 05:42
@jonathanknowles jonathanknowles merged commit 672c0b2 into sevanspowell/adp-1092/apply-tx-testable Sep 3, 2021
@jonathanknowles jonathanknowles deleted the jonathanknowles/apply-tx-testable branch September 3, 2021 06:25
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