Re: Minor modification to python client

[Alan Conway <ac...@redhat.com>]

Yup I knew the cleanup stuff was a bit dodgy. How does this sound:

TestBase: add these functions:

assertChannelOpen(chan): fail if chan is closed or has an exception.
assertChannelClean(chan): fail if channel has exceptions 
openChannel(id): return channel, add it to self.channelsUsed

In unit tests:
 1. Call openChannel rather than using client directly.
 2. After nowait ops call assertChannelOpen (*)

In tearDown:
 1. for ch in self.channelsUsed: close, note any exceptions
 2. Open new channel to clean up qs and exs. note any exceptions
 3. Close that channel
 4. If any exceptions noted above, fail the test.

I'll implement that today unless anyone has better ideas.

Cheers,
Alan.


On Mon, 2006-09-04 at 11:04 +0100, Gordon Sim wrote:
> I made a minor modification to the python client module in order to 
> write tests that check channel exceptions as sent by the broker.
> 
> Specifically I added a channel_close handler method to the clients 
> delegate which just adds the response to the channels response queue 
> (unblocking any invocations waiting for responses) and then closes the 
> channel.
> 
> As a channel_close isn't recognised as an expected response, this 
> results in a ValueError being raised, with the message frame as the 
> argument, on the invocation thread.
> 
> I'm not sure whether this is the desired way of handling this problem. 
> Its just a three line change though so should be easy to back out.
> 
> One outstanding issue is that if nowait is set then the exception will 
> not be thrown, but the channel will be closed; perhaps we want to store 
> the details of the close so that these can be included in the exception 
> raised when a closed channel has any invocation attempted on it.
> 
> Another issue, with regard to the test framework, is that the automatic 
> queue cleanup will fail if the main channel encounters a channel 
> exception (due to the channel being closed before the delete operation 
> is attempted).

Re: Minor modification to python client

[Rafael Schloming <ra...@redhat.com>]

I modified Gordon's close handler to not futz with the response queue 
and instead call ch.close(msg). I also modified the channel to store the 
closing message and check before any operation that the channel is 
closed and if not raise a new exception with the previously stored 
message. This should obviate the need for any sort of 
assertChannelOpen() and assertChannelClean() since those will 
effectively happen whenever a channel is used.

This also has the benefit of reporting the closing exception for *any* 
subsequent attempt to use the channel instead of just for the first one 
to read from the response queue after the channel is closed.

Gordon Sim wrote:
> Alan Conway wrote:
>> Yup I knew the cleanup stuff was a bit dodgy. How does this sound:
>>
>> TestBase: add these functions:
>>
>> assertChannelOpen(chan): fail if chan is closed or has an exception.
>> assertChannelClean(chan): fail if channel has exceptions 
> 
> I'm not sure whether this second one is needed. From a protocol 
> perspective, a channel exception *is* a channel.close i.e. if there is 
> an exception then it closed the channel.
> 
> It would be useful to have a means of checking this on the Channel class 
> itself, which may be exactly what you are proposing, with any assertion 
> utility method using that facility.
> 
>> openChannel(id): return channel, add it to self.channelsUsed
> 
> This is to allow the channels to be tracked? Do we need this or could we 
> not just check the list of known channels for the client?
> 
>> In unit tests:
>>  1. Call openChannel rather than using client directly.
>>  2. After nowait ops call assertChannelOpen (*)
>>
>> In tearDown:
>>  1. for ch in self.channelsUsed: close, note any exceptions
>>  2. Open new channel to clean up qs and exs. note any exceptions
>>  3. Close that channel
>>  4. If any exceptions noted above, fail the test.
>>
>> I'll implement that today unless anyone has better ideas.
>>
>> Cheers,
>> Alan.

Re: Minor modification to python client

[Alan Conway <ac...@redhat.com>]

On Tue, 2006-09-05 at 15:17 +0100, Gordon Sim wrote:
> > openChannel(id): return channel, add it to self.channelsUsed
> 
> This is to allow the channels to be tracked? Do we need this or could we 
> not just check the list of known channels for the client?
Probably, I'll look at the client code. That would be a better way to
implement it.

Re: Minor modification to python client

[Gordon Sim <gs...@redhat.com>]

Alan Conway wrote:
> Yup I knew the cleanup stuff was a bit dodgy. How does this sound:
> 
> TestBase: add these functions:
> 
> assertChannelOpen(chan): fail if chan is closed or has an exception.
> assertChannelClean(chan): fail if channel has exceptions 

I'm not sure whether this second one is needed. From a protocol 
perspective, a channel exception *is* a channel.close i.e. if there is 
an exception then it closed the channel.

It would be useful to have a means of checking this on the Channel class 
itself, which may be exactly what you are proposing, with any assertion 
utility method using that facility.

> openChannel(id): return channel, add it to self.channelsUsed

This is to allow the channels to be tracked? Do we need this or could we 
not just check the list of known channels for the client?

> In unit tests:
>  1. Call openChannel rather than using client directly.
>  2. After nowait ops call assertChannelOpen (*)
> 
> In tearDown:
>  1. for ch in self.channelsUsed: close, note any exceptions
>  2. Open new channel to clean up qs and exs. note any exceptions
>  3. Close that channel
>  4. If any exceptions noted above, fail the test.
> 
> I'll implement that today unless anyone has better ideas.
> 
> Cheers,
> Alan.