Fix missed error return in loadCipher

[qmuntal]

We were missing to return an error when a cipher algorithm could not be fetched. That error can only occur if there is something wrong in CNG itself or we call loadCipher with the wrong arguments, which we are not.

I haven't found a way to trigger this bug from the public API, so I'm not adding a test for it in this PR. I´ll submit a follow-up PR that simplifies all calls to loadOrStoreAlg so the caller doesn't have to do the error handling on the calling site.

[karianna]

In hindsight, should this have been caught in a test?

[qmuntal]

In hindsight, should this have been caught in a test?

It is not possible to trigger an error in that line of code using the public API (at least without mocking CNG itself), and we normally don't test implementation details (aka black box testing). In #63 I've removed the offending lines, so this error shouldn't happen again.

Having said this, it is fine for me to add tests exercising code that is not reachable using the public API to aim for 100% of code coverage (the current coverage is 80%). We can discuss the priority of this in our weekly sync. The drawback would be less velocity, as existing code would be more difficult to refactor without breaking tests and new code would be slower to add because it would require more testing.

[karianna]

In hindsight, should this have been caught in a test?

It is not possible to trigger an error in that line of code using the public API (at least without mocking CNG itself), and we normally don't test implementation details (aka black box testing). In #63 I've removed the offending lines, so this error shouldn't happen again.

Having said this, it is fine for me to add tests exercising code that is not reachable using the public API to aim for 100% of code coverage (the current coverage is 80%). We can discuss the priority of this in our weekly sync. The drawback would be less velocity, as existing code would be more difficult to refactor without breaking tests and new code would be slower to add because it would require more testing.

I'd say its low priority in that case, thanks for the context!