Task #3370
closedTcpChannel: test suite improvement
100%
Description
Improve TcpChannel
test suite with coverage for these functions and situations:
- accept multiple incoming connections, and check each face works
- after a face is closed, it's removed from the channel
Reference: commit:65caf200924b28748037750449e28bcb548dbc9c tcp.t.cpp
, MultipleAccepts FaceClosing
test cases.
Updated by Spencer Lee almost 9 years ago
- Status changed from New to In Progress
- Assignee set to Spencer Lee
Updated by Spencer Lee almost 9 years ago
A lot of files were affected in in change 2632.
I am wondering what files I should be looking at and studying in order to write the MultipleAccepts and FaceClosing tests in tcp-channel.t.cpp
,
besides daemon/face/tcp-channel.cpp
and daemon/face/tcp-factory.cpp
.
Updated by Junxiao Shi almost 9 years ago
Updated by Spencer Lee almost 9 years ago
Is there an advantage to using TcpFactory::createChannel versus the TcpChannel constructor?
tcp-factory.cpp/hpp
shared_ptr<TcpChannel>
createChannel(const std::string& localIp, const std::string& localPort);
versus
tcp-channel.cpp/hpp
TcpChannel(const tcp::Endpoint& localEndpoint);
Updated by Davide Pesavento almost 9 years ago
If you're working on the TcpChannel
test suite, you should NOT use any TcpFactory
methods. Use TcpChannel
constructor directly instead.
Updated by Spencer Lee almost 9 years ago
- Status changed from In Progress to Code review
Updated by Spencer Lee almost 9 years ago
Change 2632 removed Face::onFail,
and I am uncertain as to what equivalent on fail callback should be specified in the EndToEndFixture, and if the usage of the fixture is preferred in this case.
Updated by Junxiao Shi almost 9 years ago
Answer to note-7:
Face::onFail
is replaced with Face::afterStateChange
where newState is CLOSED.
LpFaceWrapper and its .cpp is a useful reference to determine the relation between face2014 and face2015 APIs.
Updated by Spencer Lee almost 9 years ago
Have you seen this error occur before? The faces were not closing before I changed onFail to afterStateChange, and now the failure is a SIGABRT being called:
unknown location(0): fatal error in "FaceClosing": signal: SIGABRT (application abort requested)
../tests/daemon/face/tcp-channel.t.cpp(254): last checkpoint
Is there anything wrong with these lines:
// Face::close must be invoked during io run to be counted as an op
scheduler::schedule(time::milliseconds(100), bind(&Face::close, face1));
BOOST_CHECK_MESSAGE(limitedIo.run(2, time::seconds(10)) == LimitedIo::EXCEED_OPS,
"FaceClosing error: cannot properly close faces");
// both faces should get closed
BOOST_CHECK(!static_cast<bool>(face1));
BOOST_CHECK(!static_cast<bool>(face2));
Updated by Spencer Lee almost 9 years ago
Another question - am I able to run specific unit-tests? Instead of run the full test suite within /build.
Also am I able to debug unit-test runs with gdb? I have tried gdb on a cpp.3.o file within /build and got a permission denied error.
Updated by Alex Afanasyev almost 9 years ago
Spencer Lee wrote:
Have you seen this error occur before? The faces were not closing before I changed onFail to afterStateChange, and now the failure is a SIGABRT being called:
unknown location(0): fatal error in "FaceClosing": signal: SIGABRT (application abort requested) ../tests/daemon/face/tcp-channel.t.cpp(254): last checkpoint
Is there anything wrong with these lines:
// Face::close must be invoked during io run to be counted as an op scheduler::schedule(time::milliseconds(100), bind(&Face::close, face1)); BOOST_CHECK_MESSAGE(limitedIo.run(2, time::seconds(10)) == LimitedIo::EXCEED_OPS, "FaceClosing error: cannot properly close faces"); // both faces should get closed BOOST_CHECK(!static_cast<bool>(face1)); BOOST_CHECK(!static_cast<bool>(face2));
Next time, please upload the code that has the error. Otherwise, there is only a guess work.
What you probably missed is the check that the new state of the face is CLOSED:
face1->afterStateChange.connect([this] (face::FaceState oldState, face::FaceState newState) {
if (newState == face::FaceState::CLOSED) {
this->face1_onFail();
}
});
and
face2->afterStateChange.connect([this] (face::FaceState oldState, face::FaceState newState) {
if (newState == face::FaceState::CLOSED) {
this->face2_onFail();
}
});
With these signals, I'm not getting any segfaults.
Updated by Alex Afanasyev almost 9 years ago
Spencer Lee wrote:
Another question - am I able to run specific unit-tests? Instead of run the full test suite within /build.
Yes. Boost.Test provides an extensive mechanism on how can you run specific Suite or unit test. We put some basic info in NFD Developer's Guide, but more extensive and definitive documentation is in Boost.Test itself
Just as an example. To run this specific test, you can
./build/unit-tests-daemon -t Face/TestTcpChannel/FaceClosing -l all
Also am I able to debug unit-test runs with gdb? I have tried gdb on a cpp.3.o file within /build and got a permission denied error.
I'm a little confused. How can you gdb/lldb an object file? You use gdb/lldb on the executable, like this
gdb --args ./build/unit-tests-daemon -t Face/TestTcpChannel/FaceClosing -l all
lldb -- ./build/unit-tests-daemon -t Face/TestTcpChannel/FaceClosing -l all
To allow gdb handle segfaults, you also need to add -s no
(--catch_system_errors=no
)
Updated by Spencer Lee almost 9 years ago
I see, sorry for the confusion, thank you very much.
I guess there were two different issues.
The error was listed as line 254 and the code I posted included that line number, which was for the failing test case, but the code for the SIGABRT was not included, leading to confusion.
Checking the closed faces resolved all issues, thank you.
Updated by Davide Pesavento almost 9 years ago
Tests for the following behaviors are also missing:
getUri()
: check that the channel URI is set correctlyisListening()
: check that the listening state is toggled correctly (although there are zero users of this functionality throughout NFD, so we could simply remove it)- connecting to the same endpoint twice doesn't create two different faces, but the first face is reused for the second
connect()
- connecting to an endpoint that isn't listening (or doesn't exist) times out and invokes the
FaceCreationFailedCallback
- faces on the listening side are created with on-demand persistency; faces on the connecting side are persistent
Updated by Spencer Lee almost 9 years ago
faces on the listening side are created with on-demand persistency; faces on the connecting side are persistent
Is this a valid method to test this?
faces on the listening side are created with on-demand persistency:
- Check if a face on the connecting side closes, then the corresponding listening side face should also close
faces on the connecting side are persistent:
- Check if a face on the listening side closes, then the corresponding listening side face should not be closed
Updated by Davide Pesavento almost 9 years ago
Spencer Lee wrote:
Is this a valid method to test this?
[...]
No, just use Face::getPersistency()
.
Updated by Davide Pesavento almost 9 years ago
Junxiao, what do you mean by "check each face works"?
Updated by Junxiao Shi almost 9 years ago
what do you mean by "check each face works"?
For example, it is not null, has correct properties, can send or receive packets.
It's unnecessary to test all of the above.
Updated by Davide Pesavento almost 9 years ago
How do we do that without duplicating the transport tests?
Updated by Junxiao Shi almost 9 years ago
How do we do that without duplicating the transport tests?
I think just testing non-null is fine.
Updated by Junxiao Shi almost 9 years ago
- Status changed from Code review to Closed
Updated by Junxiao Shi almost 9 years ago
- Blocks Task #3514: Merge Channel test cases as template added