NFD

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.

Answer to note-2:

Change 2632 is a big change that removed many test cases.
The missing test cases will be added in #3369 #3370 #3371 #3372 #3373 #3374 #3375 #3376.

Only tcp-channel.* is relevant to this task.

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);

If you're working on the TcpChannel test suite, you should NOT use any TcpFactory methods. Use TcpChannel constructor directly instead.

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.

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.

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));

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.

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.

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)

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.

Tests for the following behaviors are also missing:

• getUri(): check that the channel URI is set correctly
• isListening(): 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

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

Spencer Lee wrote:

Is this a valid method to test this?
[...]

No, just use Face::getPersistency().

Junxiao, what do you mean by "check each face works"?

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.

How do we do that without duplicating the transport tests?

How do we do that without duplicating the transport tests?

I think just testing non-null is fine.