[Rock-dev] Failing unit tests for RTT rock-master branch

[Peter Soetens]

On Thursday 31 March 2011 12:12:30 Sylvain Joyeux wrote:
> On 03/31/2011 11:52 AM, Peter Soetens wrote:
> > FYI, I merged rock-master to master and these unit tests are failing + my 
comments:
> Damn. I thought I ran these tests. I'll have a look.
> 
> > 7:
> > /home/kaltan/src/git/orocos-toolchain/rtt/tests/corba_ipc_test.cpp(685):
> > error in "testBufferHalfs": check mi->read( result ) == NewData failed
> > [OldData != NewData]
> > 7:
> > /home/kaltan/src/git/orocos-toolchain/rtt/tests/corba_ipc_test.cpp(686):
> > error in "testBufferHalfs": check result == 4.44 failed
> > [6.4400000000000004 != 4.4400000000000004]
> > 7:
> > /home/kaltan/src/git/orocos-toolchain/rtt/tests/corba_ipc_test.cpp(691):
> > error in "testBufferHalfs": check result == 4.44 failed
> > [6.4400000000000004 != 4.4400000000000004]
> > 
> > Not sure what happened here, especially corba-test contains the same code
> > (I assume) and does not fail.
> 
> My experience with fixing these "half" tests is that they are really
> fragile, as they try to reproduce, in the test, the connection protocol.
> I.e., if the protocol changes then we need to change the tests
> accordingly. This is really annoying.

Well, they test the CORBA API. We need to offer some stability there too, not 
just C++ <-> C++, since some apps run only against the CORBA IDL, without RTT 
sources.

> 
> > 9: Test command:
> > /home/kaltan/src/git/orocos-toolchain/rtt/build/tests/mqueue-test 9:
> > Test timeout computed to be: 1500
> > 9: Running 5 test cases...
> > 9: /home/kaltan/src/git/orocos-toolchain/rtt/tests/mqueue_test.cpp(361):
> > fatal error in "testVectorTransport": critical check vout.createStream(
> > policy ) failed
> > 
> > Only mqueue test failing. Not sure why either.
> 
> Why do you call createStream on a mqueue ? Isn't it meant to be used
> with point-to-point connection ?

No. We can create an output/input stream only too. This works as such for 
mqueues and for the ROS transport (which is only streams).

> 
> > 18: Test command:
> > /home/kaltan/src/git/orocos-toolchain/rtt/build/tests/service_port_test
> > 18: Test timeout computed to be: 1500
> > 18: Running 6 test cases...
> > 18:
> > /home/kaltan/src/git/orocos-toolchain/rtt/tests/service_port_test.cpp(12
> > 1): error in "testUsePort": check read.ready() failed 18:
> > /home/kaltan/src/git/orocos-toolchain/rtt/tests/service_port_test.cpp(12
> > 3): error in "testUsePort": check result == 3 failed [537 != 3] 18:
> > /home/kaltan/src/git/orocos-toolchain/rtt/tests/service_port_test.cpp(12
> > 4): error in "testUsePort": check fs == NewData failed [NoData !=
> > NewData]
> > 18:
> > /home/kaltan/src/git/orocos-toolchain/rtt/tests/service_port_test.cpp(14
> > 4): error in "testUsePortWithOwner": check read.ready() failed 18:
> > /home/kaltan/src/git/orocos-toolchain/rtt/tests/service_port_test.cpp(14
> > 6): error in "testUsePortWithOwner": check result == 3 failed [537 != 3]
> > 18:
> > /home/kaltan/src/git/orocos-toolchain/rtt/tests/service_port_test.cpp(14
> > 7): error in "testUsePortWithOwner": check fs == NewData failed [NoData
> > != NewData]
> > 
> > Only error here is that read.ready() failed.
> 
> What is read.ready() doing ?

It's an OperationCaller<FlowStatus(T)> that calls the InputPort::read 
function. Since the signature _changed_ to FlowStatus(T, bool), this fails to 
initialize. Strong-typed C++ thingy. You'd have a similar problem if you had a 
pointer to the read function in some code.

> 
> > 22: Test command:
> > /home/kaltan/src/git/orocos-toolchain/rtt/build/tests/ports_test 22:
> > Test timeout computed to be: 1500
> > 22: Running 9 test cases...
> > 22: /home/kaltan/src/git/orocos-toolchain/rtt/tests/ports_test.cpp(468):
> > error in "testPortObjects": check mget.ready() failed 22:
> > /home/kaltan/src/git/orocos-toolchain/rtt/tests/ports_test.cpp(473):
> > error in "testPortObjects": check mget(get_value) failed 22:
> > /home/kaltan/src/git/orocos-toolchain/rtt/tests/ports_test.cpp(474):
> > error in "testPortObjects": difference{inf%} between
> > 3.991{3.9910000000000001} and get_value{0} exceeds 0.001%
> > 
> > Error here is that the read function's signature changed. Since read()
> > now takes two arguments, this breaks scripting and other code that
> 
> > used the signature of read(). See also next case:
> Why does that change something for these tests ? copy_old_data=true is
> the default, so read(sample) should have exactly the same behaviour than
> before.

Same as above. When the signature changes, the OperationCaller needs to be 
changed too. So even in C++ changing a function exported to scripting or CORBA 
(!) breaks existing code.

> 
> > 28: Test command:
> > /home/kaltan/src/git/orocos-toolchain/rtt/build/tests/state_test 28:
> > Test timeout computed to be: 1500
> > 28: Running 14 test cases...
> > 28: /home/kaltan/src/git/orocos-toolchain/rtt/tests/state_test.cpp(952):
> > fatal error in "testStateEvents": Parse error at line 6: Wrong number of
> > arguments in call of function "root.t_event.read": expected 2, received
> > 1.
> > 
> > A read() on a port in scripting now requires the 'bool' argument which
> > was added.
> > 
> > Summary: I would at least add a new read() function which implements the
> > copy_old_data=false behavior. It should preferably have a different name
> > as well, like readNew() or something similar.
> 
> I don't understand that proposal. Do you mean at the scripting level, or
> at the C++ level ?

C++. 

> 
> The problem with read/readNew is that we would need
> readNewest/readNewestNew ... :P

Oh yeah, I thought readNew already sounded familiar :-)

Another solution is to provide both methods:

FlowStatus read(Data&);
and
FlowStatus read(Data&, bool);

scripting does not provide overloading, but we could just expose the first 
method and leave the latter 'hidden' for now. I prefer that solution then to 
readNew()...

> 
> If possible, I would prefer keeping the current C++ API. Obviously, not
> breaking the scripting API takes precedence, though.
> 
> > The others need more investigating, but clearly some behavior changed by
> > improving the data port code.
> 
> Agreed ... The scripting behaviour is especially worrisome.

No harm done yet...

Peter