Hacker News Reader

Comment by kstrauser | Hacker News Reader

If this were my team, I would be unsettled by the fact that we never caught it in testing. Did no one write tests to exercise this part of the app - the one where we're handcrafting HTTP requests?

Objectively, you need to write more tests. At the minimum, this bug should have a regression test so that it can never accidentally happen again (say when a dev merges an old branch in for whatever reason).

View on HN

What test would you have written to catch this? One that checks the exact contents of headers passed along? It's possibly they even had tests around this, but were expecting the same output that they were inputting (copy+pasta). Perhaps they had a more "integration"-ee test that actually hit the web with that bad header. At the point they wrote it, that test would have been passing. It wasn't until the parsing server changed (to Coyote, it seems) that the test would have started to fail.

Yes, I would have written a test to confirm that input_a generates output_b. The first half of that function is nothing but a string builder and easily testable. If they were copy-and-pasting the actual output to get the expected output, then yes: they screwed that part up.

I'm far from a TDD purist, but it's clearly true that they're not sufficiently validating their code. If they had been, this would not have happened. I'm not saying this as an attack on their skills as programmers, but as caution to others reading the story: you have to - have to - test your stuff.

It's one thing to lean on third-party libraries and expect them to mostly Do The Right Thing, especially if they're popular and come from a culture of valuing test coverage. If you're writing a Rails app, for instance, you might be forgiven for not writing your own independent validations of the Ruby methods you call. But writing string-building code to implement RFC-defined network protocols? You should have some confidence that your program is generating the output that the other party will be expected. Especially with something as commonly proxied as unencrypted HTTP; you just have to assume that your data will be traversing and analyzed by systems 100% outside of your control.

At first I was thinking that suggesting that you exactly check the output of a request might be a bit much, especially since it could be entirely variable and cause your tests to break at any point during refactoring. If that was done by a third party framework, as you point out, you might not get a whole lot of value from testing its output. However, if you're constructing your own HTTP requests, as seem to be, then yeah, you probably need to explicitly check that it is being built up correctly. Or, since this appears to be a single build up, and not common/shared functionality, it could probably be abstracted into a common function/utility that does it for you. That should be easily unit-testable. Fair enough.

>Objectively, you need to write more tests

That is precisely the opposite of objective. Personal thoughts, feelings and opinions are subjective by definition. 2+2=4 is objective. "You need to put more cheese on that pizza" is subjective.

"We don't have enough test coverage to ensure that we're generating RFC-compliant output" is objective. That's not a personal though, feeling, or opinion. If you wish, you may generalize that to "we don't have enough tests to catch an error that made it into the shipped product".

That would only be objective if it were possible to have enough test coverage to ensure that. But that is not possible. So it is purely a subjective question of how much test coverage person A thinks is "good enough".

Reply on news.ycombinator.com