ARTEMIS-1924 small tweaks on HeartBeat test#2152
Conversation
|
|
||
| public static void main(String[] arg) { | ||
| if (arg.length > 0 && arg[0].equals("testConnectionReset")) { | ||
| if (arg.length > 0 && arg[0].startsWith("testCloseConsumerOnConnectionReset")) { |
There was a problem hiding this comment.
How about using the argument as the queue name? That way you dont need a constant, and there are side benefits such as test isolation and if things to wonky you can read the logs and immediately know what test the output came from etc?
There was a problem hiding this comment.
I was going to do that, but then the argument was: testCloseConsumerOnConnectionResetuseOverride=true
The test is using parameters.. which I didn't like as the name of the queue...
I would prefer leaving it as is...
d7f6535 to
bfb20ac
Compare
bfb20ac to
6f72e0b
Compare
|
Looks good |
|
@gemmellr thanks.. I will merge it then |
|
(I still don't think it should be in that class though per #2149 comments...but the test itself looks good now) |
|
@gemmellr that I disagree with.. this is about no heart beat.. and disconnecting the client. |
|
To me it seemed more about cleaning up the client regardless of idle-timeout, and that it should do exactly the same if idle-timeout handling is enabled or if it wasn't using AMQP at all. |
|
@gemmellr the issue i was dealing with was a .NET client in windows not cleaning up after a disconnect, after I created the feature. So.. this is definitely related. We can create more tests outside of this context.. but this test is definitely needed IMO. |
|
Yes, after a connection-reset-by-peer error which meant the broker knew the connection had gone, which it knew regardless of whether idle-timeout handling was enabled or not, but failed to clear it up at the time. I think we may need to agree to disagree here :) |
|
with heart beats.. you won't need this test.. as ping/pongs will disconnect the client. |
|
It's pointless to argue anyway.. it's just a test and it's working as expected... I need to move to a different task ;) |
The idle-timeout stuff defaults to killing after 60 seconds, which the test doesnt even allow for running long enough to occur, so disabling heartbeats should make no difference to the test, which is why I dont think it should be in this class. But I agree, lets move on. |
3 participants
No description provided.