samuong / alpaca

A local HTTP proxy for command-line tools. Supports PAC scripts and NTLM authentication.
Apache License 2.0
196 stars 35 forks source link

No error assertions in NoMAD unit tests #43

Closed samuong closed 4 years ago

samuong commented 4 years ago

Prior to #41, some of the unit tests for NoMAD integration contained assertions on the message within an error. With these removed, the tests just check for the presence of an error, without checking what type.

This should be changed to check the error type, using the new errors.As function, introduced in Go 1.13.

Additional context can be found in the comments of #41.

camh- commented 4 years ago

I would only do this if the error result is part of a stable API. It is sufficient to check that the function under test returns an error under error conditions. If the function is not documented to return a particular type of error, testing for that just makes the tests more fragile.

samuong commented 4 years ago

Ok that's a fair point, this is definitely not a documented API. Julia didn't seem to have a super strong opinion on this either, so I'll close this issue and we can leave the error type assertions out.