-
Notifications
You must be signed in to change notification settings - Fork 211
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
More tests for update passphrase via CLI #535
Conversation
if (title == "40 chars hex") then | ||
T.unpack err `shouldContain` | ||
"I couldn't find a wallet with the given id:\ | ||
\ 1111111111111111111111111111111111111111\n" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
made small change so CLI checks if wallet exists before attempting to update pass (which is consistent with other endpoints behaviour)
this is tested here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
Some minor comments
(ApiT wPassphraseOld) | ||
(ApiT wPassphraseNew) | ||
Left _ -> | ||
handleResponse Aeson.encodePretty res |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 noice
] | ||
|
||
(cTx, outTx, errTx) <- postTransactionViaCLI @t ctx pass args | ||
expectations (cTx, outTx, errTx) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test seems to be lesser than its title. We check that we can make a transaction with the new passphrase, but we don't actually check that a transaction cannot be made using the old passphrase, right ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but we don't actually check that a transaction cannot be made using the old passphrase, right ?
We do!
let matrix =
[ ("Old passphrase -> fail", oldPass
, expect (ExitFailure 1, mempty, errMsg403WrongPass)
)
, ("New passphrase -> OK", newPass
, expectTxOK
)
]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aaaaaah. Okay.
, expect (ExitFailure 1, mempty, "passphrase is too short") | ||
) | ||
, ( "Incorrect old passphrase", "wrong secure passphrase" | ||
, expect (ExitFailure 1, mempty, errMsg403WrongPass) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I cringed when I saw the indentation on these two lines 😬
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
take it easy ;) [will fix]
…e (same as it is when posting tx or estimating fee or updating name)
ee90d3e
to
a7c6028
Compare
Issue Number
#472
Overview
Comments