Relocate properties to DBSpec for further sharing #270
Conversation
| , DummyTarget | ||
| , DummyStateMVar (..) | ||
| , dbPropertyTests | ||
| , prop_readAfterPutBoundary |
There was a problem hiding this comment.
oh, this is for SqliteSpec. There was an error when put . read if status is Restoring Quantity 100 as this was written as Ready
There was a problem hiding this comment.
namely, it will be used for check that Restoring . Quantity 100 upon write results in Ready upon read test
There was a problem hiding this comment.
Why not include it in the dbPropertyTests, that should hold for the MVar too no ?
There was a problem hiding this comment.
because, it fails -
1) Cardano.Wallet.DB.MVar, put . read yields a result, check that Restoring . Quantity 100 upon write results in Ready upon read
Falsifiable (after 1 test):
(PrimaryKey (WalletId {getWalletId = 8adfe5c94630e8ae4b47d6e4c9363ef31f5fa36b}),WalletMetadata {name = WalletName {getWalletName = "bulbazaur"}, passphraseInfo = Just (WalletPassphraseInfo {lastUpdatedAt = 1864-05-09 09:25:31.730365313989 UTC}), status = Ready, delegation = NotDelegating})
expected: Just (WalletMetadata {name = WalletName {getWalletName = "bulbazaur"}, passphraseInfo = Just (WalletPassphraseInfo {lastUpdatedAt = 1864-05-09 09:25:31.730365313989 UTC}), status = Ready, delegation = NotDelegating})
but got: Just (WalletMetadata {name = WalletName {getWalletName = "bulbazaur"}, passphraseInfo = Just (WalletPassphraseInfo {lastUpdatedAt = 1864-05-09 09:25:31.730365313989 UTC}), status = Restoring (Quantity (Percentage {getPercentage = 100})), delegation = NotDelegating})
There was a problem hiding this comment.
Well, something's wrong then 😄 ! Probably in the generator itself actually. Not sure that Restoring (Quantity 100) should be an allowed value. I thought about that the other day and we do construct the wallet restoration state "by hand". We'd probably be better off having some clear API for manipulating this type, which enforces that the status can't be above 99.
There was a problem hiding this comment.
Restoring (Quantity (Percentage {getPercentage = 100}) is stored as it is, but in Sqlite (at least how we implemented it) it was represented as Ready. This is because of how status 100 is handled in the code (maybe we will change it, and then this property is not needed)
There was a problem hiding this comment.
ok, I will remove this property and reintroduce it later (if needed) !
then customedGen will be removed
There was a problem hiding this comment.
ah, customizedGen makes sense - will recover it
| module Cardano.Wallet.DBSpec | ||
| ( spec | ||
| , DummyTarget | ||
| , DummyStateMVar (..) |
There was a problem hiding this comment.
If that's MVar-specific, shouldn't it be with DB.MVarSpec ?
There was a problem hiding this comment.
I had problems with Arbitrary instances needed in DbSpec after relocation. To avoid them I put every instances in DBSpec...Maybe it was too quick decision
There was a problem hiding this comment.
That's quite minor, but yes, I think at least DummyStateMVar could just be with the MVar specs.
There was a problem hiding this comment.
fair argument - done
paweljakubas
force-pushed
the
paweljakubas/154/db-unit-tests-sharing
branch
2 times, most recently
from
1ef0525
to
34306da
Compare
Remove one property and relocate DummyMvarState
Issue Number
#154
Overview
Comments