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

Few negative mnemonic test cases #106

Merged
merged 2 commits into from
Mar 25, 2019
Merged

Conversation

piotr-iohk
Copy link
Contributor

Issue Number

#94

Overview

  • I have added few negative mnemonic test cases

Comments

Well, I don't know, but I was expecting that code coverage will slightly improve but it stayed the same :(
I was expecting to get coverage on those two lefts below but hpc still shows ErrMnemonicWords and ErrDictionary not executed...

  163 mkMnemonic wordsm = do
  164     phrase <- left ErrMnemonicWords
  165         $ mnemonicPhrase @mw (toUtf8String <$> wordsm)
  166 
  167     sentence <- left ErrDictionary
  168         $ mnemonicPhraseToMnemonicSentence Dictionary.english phrase

@piotr-iohk piotr-iohk self-assigned this Mar 21, 2019
@KtorZ
Copy link
Member

KtorZ commented Mar 22, 2019

@piotr-iohk I believe the coverage didn't improve here because you aren't actually evaluating the errors but just, verifying that the constructor yields a Left. So GHC doesn't need to go much further in the evaluation steps and what errors is actually triggered isn't tested 😉

So, we do need a stronger assertion than isLeft, likely, a strong equality.

@KtorZ KtorZ mentioned this pull request Mar 22, 2019
@KtorZ KtorZ force-pushed the master branch 2 times, most recently from 049fddd to a8d7c7d Compare March 23, 2019 12:45
@piotr-iohk piotr-iohk force-pushed the piotr/94/mnemonic_error_cases branch from 7482f55 to 8149cbe Compare March 25, 2019 12:31
@piotr-iohk
Copy link
Contributor Author

@KtorZ, I have added expectations against specific values where I could. Code coverage now increased indeed 👍

The only error I was not able to produce was UnexpectedEntropyError in:

genEntropy =
       let
           size =
              fromIntegral $ ambiguousNatVal @n
           eitherToIO =
               either (throwM . UnexpectedEntropyError) return
       in
           (eitherToIO . mkEntropy) =<< Crypto.getEntropy (size `div` 8)

Copy link
Member

@KtorZ KtorZ left a comment

Choose a reason for hiding this comment

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

🎉

@KtorZ KtorZ merged commit cc958af into master Mar 25, 2019
@KtorZ KtorZ deleted the piotr/94/mnemonic_error_cases branch March 25, 2019 15:09
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.

None yet

2 participants