Project

General

Profile

Actions

Task #3307

closed

UnicastUdpTransport: test suite improvement

Added by Junxiao Shi about 9 years ago. Updated almost 9 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
Faces
Target version:
Start date:
Due date:
% Done:

100%

Estimated time:
3.00 h

Description

Improve UnicastUdpTransport test suite with new test cases:

  • static properties for non-local IP
  • send
  • receive normal
  • receive malformed
  • close from transport
  • recover from remote close and re-open

Related issues 3 (0 open3 closed)

Related to NFD - Task #3346: MulticastUdpTransport: test suite improvementClosedEric Newberry

Actions
Blocked by NFD - Feature #3168: UdpTransportClosedYukai Tu

Actions
Blocks NFD - Task #3172: Refactor Face: completionClosedJunxiao Shi

Actions
Actions #1

Updated by Junxiao Shi about 9 years ago

Actions #2

Updated by Junxiao Shi about 9 years ago

  • Blocks Task #3172: Refactor Face: completion added
Actions #3

Updated by Junxiao Shi about 9 years ago

  • Description updated (diff)
Actions #4

Updated by Junxiao Shi about 9 years ago

  • Related to Task #3346: MulticastUdpTransport: test suite improvement added
Actions #5

Updated by Junxiao Shi almost 9 years ago

  • Assignee set to Eric Newberry
Actions #6

Updated by Eric Newberry almost 9 years ago

  • Status changed from New to In Progress
Actions #7

Updated by Eric Newberry almost 9 years ago

Asio UDP sockets do not support async_read. Should I change the StreamTransport::Send test to use a different command or exclude UnicastUdp from the StreamTransport tests and make customized versions for UDP?

Actions #8

Updated by Davide Pesavento almost 9 years ago

UDP sockets are not stream-based, and UnicastUdpTransport does not inherit from StreamTransport, so it would be conceptually wrong to make UDP tests part of the StreamTransport tests.

Actions #9

Updated by Eric Newberry almost 9 years ago

Davide Pesavento wrote:

UDP sockets are not stream-based, and UnicastUdpTransport does not inherit from StreamTransport, so it would be conceptually wrong to make UDP tests part of the StreamTransport tests.

Oh whoops, UDP is datagram and not stream.

Actions #10

Updated by Eric Newberry almost 9 years ago

DatagramTransport does not appear to be able to handle receiving packets in multiple segments or multiple blocks in one packet. Is this correct? If so, what, besides receiving a packet that is too large, counts as a malformed packet?

Actions #11

Updated by Davide Pesavento almost 9 years ago

Yes, that is expected behavior. There must be a 1:1 mapping between NDN packets and UDP datagrams.

Other kinds of malformed packets would be truncated ones I guess? And also datagrams that contain a full NDN packet + some trailing garbage I think.

Actions #12

Updated by Junxiao Shi almost 9 years ago

Answer to note-10:

For datagram-based transports, the only "receive normal" situation is: payload is one complete TLV structure.

Abnormal situations include:

  • There is junk after one complete TLV (even if the junk is another complete TLV). Example:

    01 03
       DD DD DD
    02 01 // junk: it's another TLV but this doesn't matter
       DD
    
  • TLV-TYPE is missing. Example:

    (empty payload)
    
  • There isn't enough octets for TLV-TYPE. Example:

    FF 01
    
  • TLV-LENGTH is missing. Example:

    01
    
  • There isn't enough octets for TLV-LENGTH. Example:

    01 FF 01
    
  • TLV-VALUE is missing when TLV-LENGTH is not zero. Example:

    01 08
    
  • There isn't enough octets for TLV-VALUE whose length is given in TLV-LENGTH. Example:

    01 08
       DD DD DD DD
    

These are not abnormal situations:

  • Top-level TLV-TYPE is not Interest/Data/LpPacket. Transport is not responsible for this.
  • TLV-VALUE is omitted when TLV-LENGTH equals zero. This is still a valid TLV.
Actions #13

Updated by Davide Pesavento almost 9 years ago

Let's not exaggerate... it's not the responsibility of DatagramTransport (or any other transport) to perform TLV parsing and sanity checking. It's wrong to test all possible combinations of invalid TLVs in the UdpTransport test suite.

The point is to test only the code paths and different branches in the class(es) that are being tested. So, for DatagramTransport receive path for example we have:

  • the "normal" path
  • the if on line 151
  • the if on line 159, caused by Block::fromBuffer failing
  • the if on line 164, caused I believe by trailing garbage in the UDP datagram

and nothing else. In particular, there is no need to test the various ways in which Block::fromBuffer can fail. That must be tested in Block test suite.

Actions #14

Updated by Eric Newberry almost 9 years ago

  • Status changed from In Progress to Code review
Actions #15

Updated by Eric Newberry almost 9 years ago

  • Status changed from Code review to Closed
  • % Done changed from 0 to 100
Actions

Also available in: Atom PDF