Code review comment for lp:~jpds/launchpad/fix_565345-mirror-prober-agent

Revision history for this message
Jeroen T. Vermeulen (jtv) wrote :

A good idea, especially since we've traditionally asked bots accessing our site to provide meaningful user agent identification. Shouldn't ours include a URL as well, by the way?

It's a shame you have to mess with sendHeader in this way, but the only alternative I see is to make FakeTransport log its header and that's probably more effort than it's worth.

A few points remain though:
 * Don't return self.assertEquals(...); just invoke it. It'll raise an exception if it fails.
 * Don't test "headers[1]"—a hard-coded index is far too fragile. Why not make "headers" a dict and test headers['User-Agent']?

Jeroen

review: Approve (code)

« Back to merge proposal