On Wed, Feb 14, 2018 at 10:30 AM, Axel Wagner <axel.wagner...@googlemail.com
> wrote:
>
> On Wed, Feb 14, 2018 at 10:01 AM mrx <patrik....@gmail.com> wrote:
>
>>
>> On Tue, Feb 13, 2018 at 5:26 PM, Axel Wagner <
>> axel.wagner...@googlemail.com> wrote:
>>
>>> Not very, but it does depend on the details. For example, you might
>>> provide your own http.Transport for the client to use, or even your own
>>> net.Conn.
>>>
>>
>> Using ioutils.ReadAll() on a HTTP request means to me to read out the
>> response's body. I cannot see how http.Transport and net.Conn would have
>> anything to do with this.
>>
>
> Presumably, to read out a response body means, that you made a request. By
> passing in a custom http.Transport, you can have that transport return a
> non-EOF error at will in tests.
>
> It would be far more helpful, if there would be actual code here.
>
Sure thing, taken from the http docs:
resp, err := http.Get("http://example.com/";)
if err != nil {
// handle error
}
defer resp.Body.Close()
body, err := ioutil.ReadAll(resp.Body)
That err returned from ReadAll. I cannot see how that can possibly fail.
> You might have the server stop sending any data, so eventually the
>>> connection will timeout.
>>>
>>
>> So what you're saying is that unless the response contain chunked data,
>> ioutil.ReadAll() will never fail?
>>
>
> I say that ioutil.ReadAll will fail if and only if the io.Reader it's
> called with returns a non-EOF, non-nil error on some read. That, at least,
> seems the most obvious semantics of that function to me and it's how I read
> its documentation:
>
> > ReadAll reads from r until an error or EOF and returns the data it
> read. A successful call returns err == nil, not err == EOF.
>
Sure, that make sense. When testing for err i try to imagine what could go
wrong, to get the error handling correct. But in this case (code from http
docs above) i cannot see it...
>
> The question is, though, why would you want that?
>>>
>>
>> As ioutil.ReadAll does return an error in its signature, i think it's
>> good form to test it. Don't you?
>>
>
> No, I don't think I do, actually :) Like, what failure modes is this
> testing, how often will they occur, how likely is it, that a future change
> would break the behavior? I'm just having trouble coming up with a bug that
> might be introduced in the future, that could be caught by testing this.
>
> Checking every branch seems to be an incarnation of the "we need 100% line
> coverage" testing philosophy, but honestly, I don't believe that's
> particularly helpful, because a) path coverage is much more important than
> line coverage
>
I'm not after a 100% line coverage. =)
Is there a way to get branch/path coverage in Go? I cannot find anything
like that in the help from go test.
> and b) 100% is unattainable in practice. Adhering to some kind of
> mechanical rule about what code must have test coverage will just lead to
> wasted time - if there are zero future bugs with or without that test, it
> definitionally was useless. And I'd say that the likelihood that this is
> going to catch a future bug is very low. So if, with very high probability,
> writing that test is a waste of time then that seems a good enough reason
> not to write it.
>
I agree, i'm just trying to get the error handling correct. That's really
difficult if i cannot even imagine what could go wrong.
>
> (unless, of course, you are trying to write tests *for ReadAll itself*,
> in which case checking the whole API contract makes of course sense. But I
> assume you're not :) )
>
You're correct, i'm not writing tests for ioutil.ReadAll()
>
> Is that actually a path that is worth testing? Personally, I kind of doubt
>>> it.
>>>
>>
>> That's kind of it really, i am having a hard time making up my mind here.
>> That's why i come for the golang nuts wisdom.
>>
>>
>>> You'll probably get more bang for your buck, if you instead send back
>>> broken/incomplete data from the server and see if the client handles that
>>> correctly.
>>>
>>
>> I already test for this kind of problems in my unittests. It's more a
>> matter of what to do with the error return value from ioutil.ReadAll() when
>> i cannot see how i could ever get anything but err == nil. It might just be
>> me, that doesn't know enough about the ioutil.ReadAll() internals.
>>
>
> After digging some into the code, it does inform a sensible additional
> test you might be doing, as it also returns an error if the reader returns
> *too
> much* data - so it would make sense to check what happens if the server
> would send back, say, an infinite bytestream. But that still isn't really a
> test for the ReadAll error return, as a check for "does this function
> behave well if fed unreasonable data".
>
> And to answer the question of what to do about err != nil: You should
> handle it :) It means the data the server sent was broken in some way, you
> should handle that in some way that is appropriate for the application you
> are writing - which might include logging it, ignoring it, aborting,
> retrying or anything else, really :) Presumably, you are also handling
> invalid data somehow - this shouldn't be any different.
>
Handling those errors still mean that i need to have at least an idea or
two on what could happen, no?
// Patrik
--
You received this message because you are subscribed to the Google Groups
"golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to golang-nuts+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
