Handle when the script fails due there is not internet connection or due an unexpected error #21
Conversation
This commit adds two exceptions, one te report there is not internet connection and another to handle an unexpected error.
If the request failed first check if it is because there is not internet connection, if true, raise an NoInternetConnectionError, if it is not the error, it raise an UnexpectedError and report to the user the error, encouraging to report it in the official repository.
|
The check that fails in this PR is the coverage test by Coveralls, because it reports a decrease of 4.2 %. The reason of this decrease are the two new conditions applied to raise the errors. I am very new performing coverage tests so I need to learn how to cover this part of the code inside tests. In addition, I have to learn how to test the condition applied when there is not internet connection. Any idea? Either way, I will probably ask in Stack Overflow about this kind of tests before to go sleep. CC. @owenvoke P.S. I have think in the development of a mock, but I do not know how the mock could simulate a lack of internet connection. |
|
I don't know Elixr, but achieving complete code coverage with tests is all about inversion of control. If your code is sufficiently decoupled, you should be able to swap parts out for other "mock" parts. If you haven't already, the SOLID principles are great to read up on. |
Very interesting point. I am going to read more about it. I ask to you in Gitter because although someone does not know about one or another language, sometimes the procedural and logic, the theories, may help to address what someone wants in another language. This is the reason why I am going to read more about what you commented. Thank you very much @sbrl! I would like to test everything before merge it. This case is a bit different, but I still have this idea. However, it may delay the development of others task. Anyway, I already asked in Stack Overflow. |
It is a "possible test" because it can not be implemented as it is, because HTTPoison does not raise the defined exception. But it is interesting to keep the test for the future implementation of the necessary behaviour to adress this issue.
|
Well, after a very interesting answer to the question I asked, several readings about how mocks work and how it would be possible to create a test with a mock, I finally use Mox for this issue (check 75fede7 although the test is commented for future reference, because now it will fails). Although I think I know how it might be address, at this moment it does not viable to apply because the methodology should be:
With this it would be possible to create a mock for the behavior and then test the response of the behavior and not the response of HTTPotion. At least I have in my head how it could be done, although there may be something I am not considering. Now I can not define and apply the behavior, due I will not have enough time during this week, and I do not know if I could the next week. So I propose two options:
I prefer the first option: we change the What do you think about it? CC. @sbrl @owenvoke (I mentioned both of you because you already participated in ExTldr). |
| #test "should return lack of internet" do | ||
| # assert_raise NoInternetConnectionError, | ||
| # fn -> | ||
| # ClientMock | ||
| # |> expect(:get, fn _ -> {:error, %HTTPoison.Error{reason: :nxdomain}} end) | ||
| # end | ||
| #end |
There was a problem hiding this comment.
This is the test I referred in my last comment. I think that this one applied to a mock of a behavior defined for our wrapper, and not for HTTPoison.Base, will work (if am not wrong...).
|
I have finally decided to opt for the first option that I exposed in my last comment in this PR: merge this PR and open two new issues, one for the behavior and another one for the test. |
This PR consists of two commits. One in which is added two exceptions in
lib/ex_tldr/exceptions.exand the another one in which the exceptions are applied to each case:NoInternetConnectionError.UnexpectedErrorwith the error message and encourage the user to report it on this repository.