Opened 13 years ago

Last modified 12 years ago

#3353 defect new

lineLengthExceeded behaviour varies between LineReceiver and LineOnlyReceiver

Reported by: spiv Owned by:
Priority: normal Milestone:
Component: core Keywords:
Cc: ivank Branch:
Author:

Description

I'm experimenting with a patch to improve the tests for LineReceiver/LineOnlyReceiver, based on how bzrlib tests some things. Specifically, I'm applying the same test case to two different implementations of the one interface: LineOnlyReceiver and LineReceiver.

This has revealed some inconsistencies with how LineReceiver and LineOnlyReceiver handle lineLengthExceeded:

  • LineOnlyReceiver.lineLengthExceeded doesn't actually call loseConnection, it just returns a ConnectionError as if it had.
  • lineLengthExceeded is invoked with a different string in some cases when MAX_LENGTH is exceeded.

The attached patch to test_protocols shows these test failures. (This patch is relative to my patch on #3277, but it's also a bzr merge directive with full history vs. trunk).

As mentioned in #3277, I think lineLengthExceeded isn't a fully baked API, and these inconsistencies are symptoms of that.

Attachments (1)

lineReceiver-implementation-tests.patch (11.6 KB) - added by spiv 13 years ago.
Work-in-progress experiment that shows the inconsistent behaviour via test failures.

Download all attachments as: .zip

Change History (5)

Changed 13 years ago by spiv

Work-in-progress experiment that shows the inconsistent behaviour via test failures.

comment:1 Changed 13 years ago by amaury

LineOnlyReceiver.lineLengthExceeded doesn't actually call loseConnection

I think it should; is there any reason why LineOnlyReceiver and LineReceiver differ on this point?

The corresponding test has to be changed as well: the last line of testLineTooLong() could be:

self.assertTrue(a.transport.closed)

comment:2 Changed 13 years ago by ivank

Cc: ivank added

comment:3 Changed 12 years ago by Glyph

Owner: changed from Glyph to spiv

comment:4 Changed 11 years ago by <automation>

Owner: spiv deleted
Note: See TracTickets for help on using tickets.