twisted.web.http.Request.noLongerQueued has no test coverage

[Jean-Paul Calderone]

It should have complete test coverage (found while porting http.py to Python 3).

[moijes12]

Written a test for a scenario where it is called for an object that is not in queued results in a RuntimeError being raised.

[Tom Prince]

(In [36871]) Branching to Request.noLongerQueued-test-coverage-6118.

[Tom Prince]

Thanks for working on this. The test you added looks good, but could be improved by checking the message of the exception (there are a bunch of examples throughout the existing tests).

This test covers one case, but there are a bunch more need. Off hand:

1. That data is written to the channel transport if-and-only-if there is existing data).
2. If there is a producer, than it is properly registered with the transport (which may trigger writes)
3. If the request if finished, then cleanup is done.

Each of those probably deserves at least a couple of test cases.

[moijes12]

Made some additions.

[Tom Prince]

(In [37072]) Branching to defer-passthru-helper-6118.

[Tom Prince]

Thanks. There are a couple of more things that it would be worthwhile to test.

1. noLongerQueued switches the transport of the request from an in-memory transport to the one provided by the channel. There should be a test that verifies this.
   1. Relatedly, the tests that assert that a producer is hooked up to a transport could check that they are hooked up to the channel transport, rather than whatever transport request ends up pointing to.
2. I'm not sure what test_noLongerQueuedWriteIfData is trying to test, but I think it wants to test that the channel transport has the data that was originally written to the temporary request transport. (DummyChannel has a test transport which can be used to check this).
3. There should be a test that ensure that the request is no longer queued, afterwards.
4. test_noLongerQueuedCleanupIfFinished could be improved to use .notifyFinish() and 'TestCase.successResultOf to verify that _cleanup gets called. notifyFinish` is something that is definitely going to happen, whereas the other could change.
5. (minor) .finished should be set to True rather than 1. (I suspect this code predates the introduction of True)

Please resubmit for review, after addressing the above points.

[Tom Prince]

(fixing branch info)

[moijes12]

4. test_noLongerQueuedCleanupIfFinished could be improved to use .notifyFinish() and 'TestCase.successResultOf to verify that _cleanup gets called. notifyFinish` is something that is definitely going to happen, whereas the other could change.

[moijes12] est_noLongerQueuedCleanupIfFinished verifyies that _cleanup gets called by checking that self.content is deleted.

2. I'm not sure what test_noLongerQueuedWriteIfData is trying to test, but I think it wants to test that the channel transport has the data that was originally written to the temporary request transport. (DummyChannel has a test transport which can be used to check this).

[moijes12] The function documentation of test_noLongerQueuedWriteIfData clearly mentions its purpose. Also, I cannot find the test transport method in twisted.web.test.requesthelper.DummyChannel.