[MERGE][1.6][#254797] Fix NotImplementedError when probing for smart protocol via HTTP.
John Arbash Meinel
john at arbash-meinel.com
Thu Aug 14 04:22:03 BST 2008
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Andrew Bennetts wrote:
> This fixes a regression introduced in 1.6. Probing for a smart server over HTTP was
> triggering a NotImplementedError, because read_line was not implemented on the
> HTTP medium request class.
>
> This fix implements that method, and does some very simple refactoring to do so
> without duplicating code.
>
> It also adds a simple test to test_http that fails without this fix.
> Implementing that test required improving the HTTPServerWithSmarts test helper
> to behave a little more realistically.
>
> Despite the size of the diff, this is a simple and I believe safe change to
> incorporate in 1.6 final. The bulk of the diff affects code that was not
> working without this fix, or tests for that fix. The rest is a straightforward
> refactoring of code that has extensive test coverage.
>
> -Andrew.
>
>
The only thing I don't really like is:
+ def _read_line(self):
+ line, excess = medium._get_line(self._response_body.read)
+ if excess != '':
+ raise AssertionError(
+ '_get_line returned excess bytes, but this mediumrequest '
+ 'cannot handle excess. (%r)' % (excess,))
+ return line
+
^- you are using AssertionError to validate data from the wire, and the code
path is completely untested.
I'll let it pass, but I personally like to see failure code *exercised* by the
test suite.
The test also seems a bit indirect. I realize it probably would fail to
BzrDir.open('smart+http://') but it doesn't really feel like you are testing
that Medium's are implementing the appropriate interface.
Anyway, this seems like a minimal fix for 1.6, and there isn't anything I
would strictly block on. And it mostly comes back to wanting
"smart_medium_implementations/" testing.
So...
BB:approve
And I'll get it queued up for 1.6.
John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
iD8DBQFIo6TbJdeBCYSNAAMRAqYGAKDZ+AJbM6xK14afVY+OHGk/vHrtXQCgsjIf
zFrsZ7EMPE2Vlpki0dcppsQ=
=eF+2
-----END PGP SIGNATURE-----
More information about the bazaar
mailing list