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. 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. 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 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. (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 :) ) 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. -- 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.
