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

[jonathanknowles]

Issue Number

#353

Overview

Module Cardano.Wallet.DB.StateMachine defines set of Tag values to represent various test scenarios that are considered important or interesting. It also supplies a tag function to match Tag values to arbitrary sequences of commands.

However, the test suite doesn't actually check that all possible Tag values are covered during execution. This creates a risk that we might actually not be covering all the cases we'd like.

• I have made the prop_sequential property fail if one or more tags are not covered.

Comments

With this change applied, we get the following output from the test suite:

Cardano.Wallet.DB.Sqlite
  Sqlite State machine tests
    Sequential
      +++ OK, passed 400 tests:
      64.0% UnsuccessfulReadTxHistory
      61.8% CreateThenList
      60.0% SuccessfulReadCheckpoint
      54.2% CreateWalletTwice
      54.2% TxUnsortedOutputs
      54.0% TxUnsortedInputs
      36.2% ReadTxHistoryAfterDelete
      35.2% UnsuccessfulReadCheckpoint
      30.5% SuccessfulReadTxHistory
      26.0% CreateThreeWallets
      15.5% SuccessfulReadPrivateKey
      14.8% RemoveWalletTwice

Finished in 38.8470 seconds
1 example, 0 failures

Suppose we add a new constructor Wibble to the Tag data type, without modifying the tags function.

Now, if we re-run the test suite, we get a failure:

Cardano.Wallet.DB.Sqlite
  Sqlite State machine tests
    Sequential FAILED [1]

Failures:

  test/unit/Cardano/Wallet/DB/SqliteSpec.hs:80:9: 
  1) Cardano.Wallet.DB.Sqlite, Sqlite State machine tests, Sequential
       Insufficient coverage (after 800 tests):
         62.7% CreateThenList
         62.1% SuccessfulReadCheckpoint
         61.1% UnsuccessfulReadTxHistory
         59.4% TxUnsortedInputs
         59.3% TxUnsortedOutputs
         54.6% CreateWalletTwice
         37.9% UnsuccessfulReadCheckpoint
         36.0% ReadTxHistoryAfterDelete
         33.9% SuccessfulReadTxHistory
         28.4% CreateThreeWallets
         20.0% SuccessfulReadPrivateKey
         17.0% RemoveWalletTwice
         
         Only 0.0% Wibble, but expected 5.0%

[rvl]

Great, I was wondering how to do this.

[rvl]

To get enumerate = [minBound..maxBound]

[jonathanknowles]

Agreed, it's a very small function!

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

[KtorZ]

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

[jonathanknowles]

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?

[KtorZ]

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.

[jonathanknowles]

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.

[KtorZ]

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.

👍

[jonathanknowles]

Great, I was wondering how to do this.

If we add more tags in future, we might have to adjust the coverage threshold. (Currently, it's 5% for every tag.)