Actually return the error #366
Conversation
|
Lol!! Could you add a unit test? |
We have what appears to be an error return but do not actually return the error. This line needs an explicit `return` statement otherwise it is a noop.
The `SortedMultiVec` constructor appears to have a bug in it, add a unit test to trigger the bug.
|
Damn this merged, I was thinking last night that this unit test shouldn't be merged. Its informational but not useful to maintain. We do not explicitly unit test every error path in the code so it seems odd to have this one. Not to worry. |
|
I would prefer if we did unit test every error path :) |
| @@ -46,7 +46,7 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> SortedMultiVec<Pk, Ctx> { | |||
| pub fn new(k: usize, pks: Vec<Pk>) -> Result<Self, Error> { | |||
| // A sortedmulti() is only defined for <= 20 keys (it maps to CHECKMULTISIG) | |||
| if pks.len() > MAX_PUBKEYS_PER_MULTISIG { | |||
| Error::BadDescriptor("Too many public keys".to_string()); | |||
There was a problem hiding this comment.
post-merge comment: Does Clippy warn about this?
There was a problem hiding this comment.
I think you just need to upgrade to 2018 edition. It's a standard warning now: #[must_use]
There was a problem hiding this comment.
Does Clippy warn about this?
ooo you got me, usually I always mention 'found by clippy' in the commit log but I forgot this time.
I think you just need to upgrade to 2018 edition
The PR is open!
We have what appears to be an error return but do not actually return
the error. This line needs an explicit
returnstatement otherwise itis a noop.