> On Jan. 3, 2014, 12:22 a.m., Andrew Stitcher wrote: > > /trunk/qpid/cpp/src/tests/ha_tests.py, line 237 > > <https://reviews.apache.org/r/16577/diff/1/?file=413695#file413695line237> > > > > This doesn't appear to be an HA test why is it in these tests? > > Alan Conway wrote: > Agreed, should be in the qpid/python tests somewhere. I'll look for an > appropriate place. > > Alan Conway wrote: > The trick here is that the failover part of the test requires starting > and killing a second broker, which is beyond the capabilities of the > qpid/python tests. > Options I can see: > - leave it where it is since it tests failover which is HA-related > - create a new cpp/src/tests/python_client_test.py test suite (using > brokertest.py) with this test to start, can collect further python client > tests there that need to start/stop brokers but don't require the use of the > HA plugin. > I will do the latter if you think it is worthwhile.
Hmm, I think the broker starting functionality in the python test framework is much more recent than the original python tests, but perhaps the original python tests should actually start the broker itself. On the other hand I think the python test can actually be used against both the c++ and java brokers, so perhaps it should only start the broker if how to do that can be specified. If this test is specific to the C++ broker then I think it probably should actually start a new set of tests and we should migrate tests there that are specific to the C++ broker. I'll stop rambling now - hope that made sense. > On Jan. 3, 2014, 12:22 a.m., Andrew Stitcher wrote: > > /trunk/qpid/cpp/src/tests/ha_tests.py, line 243 > > <https://reviews.apache.org/r/16577/diff/1/?file=413695#file413695line243> > > > > I don't see anywhere that you send SIGCONT to the broker to get it to > > carry on - how do subsequent tests work? > > Alan Conway wrote: > The brokertest framework automatically tears down processes (including > brokers) started by each test, each test starts new brokers as needed. That sounds pretty inefficient - do you really mean that the broker gets torn down for every test rather than say every test suite? If so that would make these tests much slower than perhaps necessary. - Andrew ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16577/#review31093 ----------------------------------------------------------- On Jan. 6, 2014, 8:46 p.m., Alan Conway wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/16577/ > ----------------------------------------------------------- > > (Updated Jan. 6, 2014, 8:46 p.m.) > > > Review request for qpid, Gordon Sim and Rafael Schloming. > > > Bugs: qpid-5428 > https://issues.apache.org/jira/browse/qpid-5428 > > > Repository: qpid > > > Description > ------- > > QPID-5428: Heartbeats not in use when attempting to connect with python > client. > > Heartbeats ignored when opening a connection, could hang indefinitely > Need to cover 3 cases (test included): > - Connect sucessful but then broker stalls. > - Connect to a stalled broker that never responds. > - Fail-over to a stalled broker that never responds > > All cases are handled by the following fixes to driver.py: > - Check for heartbeats even before engine._connected since we may time out > before receiving open-ok if the peer is stalled and never sends data. > - Set _last_in and _last_out so that we time heartbeats from the start of the > connection if no data is ever sent or received. > - Call self.update_status in Driver.timeout to detect connection closed due to > heartbeat timeout (rather than a readable or writeable event.) > Make update_status a no-op if engine or transport are not yet set up. > - Don't consider reconnect complete in connect(), wait till we get the > open-ok. > See the comment on Driver._check_retry_ok() > > > Diffs > ----- > > /trunk/qpid/cpp/src/tests/ha_tests.py 1555989 > /trunk/qpid/python/qpid/messaging/driver.py 1555989 > > Diff: https://reviews.apache.org/r/16577/diff/ > > > Testing > ------- > > > Thanks, > > Alan Conway > >
