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

Make DB QSM tests fail if one or more tags are not covered. #367

Merged
merged 1 commit into from
Jun 5, 2019
+22 −4
Merged

Make DB QSM tests fail if one or more tags are not covered. #367

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions lib/core/cardano-wallet-core.cabal
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ test-suite unit
, containers
, cryptonite
, deepseq
, extra >= 1.6.17
Copy link
Contributor

Choose a reason for hiding this comment

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

To get enumerate = [minBound..maxBound]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed, it's a very small function!

But [minBound .. maxBound] seems to be surprisingly common.

, file-embed
, fmt
, foldl
Expand Down
23 changes: 20 additions & 3 deletions lib/core/test/unit/Cardano/Wallet/DB/StateMachine.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,11 @@ import Crypto.Hash
import Data.Bifunctor
( bimap, first )
import Data.Foldable
( toList )
( foldl', toList )
import Data.Functor.Classes
( Eq1, Show1 )
import Data.List.Extra
( enumerate )
import Data.Map
( Map )
import Data.Maybe
Expand Down Expand Up @@ -640,7 +642,11 @@ data Tag
-- ^ Private key was written then read.
| ReadTxHistoryAfterDelete
-- ^ wallet deleted, then tx history read.
deriving (Show)
deriving (Bounded, Enum, Eq, Ord, Show)

-- | The list of all possible 'Tag' values.
allTags :: [Tag]
allTags = enumerate

tag :: [Event Symbolic] -> [Tag]
tag = Foldl.fold $ catMaybes <$> sequenceA
Expand Down Expand Up @@ -854,13 +860,24 @@ repeatedly = flip . L.foldl' . flip

prop_sequential :: DBLayerTest -> Property
prop_sequential db =
QC.checkCoverage $
forAllCommands (sm dbLayerUnused) Nothing $ \cmds ->
monadicIO $ do
liftIO $ cleanDB db
let sm' = sm db
(hist, _model, res) <- runCommands sm' cmds
prettyCommands sm' hist $ res === Ok
prettyCommands sm' hist
$ measureTagCoverage cmds
$ res === Ok
where
measureTagCoverage :: Commands (At Cmd) (At Resp) -> Property -> Property
measureTagCoverage cmds prop = foldl' measureTag prop allTags
where
measureTag :: Property -> Tag -> Property
measureTag p t = QC.cover 5 (t `Set.member` matchedTags) (show t) p

matchedTags :: Set Tag
matchedTags = Set.fromList $ tag $ execCmds cmds

prop_parallel :: DBLayerTest -> Property
prop_parallel db =
Expand Down
2 changes: 1 addition & 1 deletion stack.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
resolver: lts-13.8
resolver: lts-13.24
Copy link
Member

Choose a reason for hiding this comment

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

Wow! Careful with this 😄 ... Now we need to reset all the caches in CI because they'll likely have doubled in size.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wow! Careful with this smile ... Now we need to reset all the caches in CI because they'll likely have doubled in size.

Good point. The first time Travis built this, the build time nearly exceeded the 50 minutes limit. Do we have a defined process for changing the resolver?

Copy link
Member

Choose a reason for hiding this comment

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

Not necessarily, in the sense that, this should work just fine. But we have to make sure to remove the cache from master before merging. Otherwise, we end up with a cache twice as big on master because it'll carry two LTS. Although, this will be needed for a short while until everyone gets existing PRs up-to-date.

It's just that, when I see "Make DB QSM tests fail ...", I don't expect an LTS bump 😅 ... It's good to advertise this kind of change on our Slack channel because it impacts everyone.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But we have to make sure to remove the cache from master before merging.

Just to confirm: Can this be done by navigating to https://travis-ci.org/input-output-hk/cardano-wallet/caches and then hitting the garbage can icon?

It's just that, when I see "Make DB QSM tests fail ...", I don't expect an LTS bump sweat_smile ... It's good to advertise this kind of change on our Slack channel because it impacts everyone.

Good point.

Perhaps this is something we can add to our list of things to take care of when writing a PR / doing a review.

Copy link
Member

Choose a reason for hiding this comment

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

Just to confirm: Can this be done by navigating to https://travis-ci.org/input-output-hk/cardano-wallet/caches and then hitting the garbage can icon?

Indeed.

Perhaps this is something we can add to our list of things to take care of when writing a PR / doing a review.

👍

packages:
- .
- lib/bech32
Expand Down