Test suite for TCP Westwood

Test suite for TCP Westwood

[Tom Henderson, 2017-05-09 21:54:43]

I have some difficulty correlating this test case with the description for TCP
Westwood (https://en.wikipedia.org/wiki/TCP_Westwood) since the test is not
actually running a real connection but just unit testing the object for a
specific input sequence.

TCP Westwood talks about Eligible Rate, Agile Probing, and PNCD.  Can you please
clarify for the future reader of this code (by adding some class doxygen) what
elements of Westwood that this test case is testing, and what elements are not
being checked?

Do you have plans to add to this test suite in the future?

https://codereview.appspot.com/320780043/diff/1/src/internet/test/tcp-westwoo...
File src/internet/test/tcp-westwood-test.cc (right):

https://codereview.appspot.com/320780043/diff/1/src/internet/test/tcp-westwoo...
src/internet/test/tcp-westwood-test.cc:37: * \brief Test case for TCP Westwood
Please clarify what this test case is explicitly testing.

If the user were to substitute some other TCP variant instead of TcpWestwood,
can you explain here why such variant would fail the test?

https://codereview.appspot.com/320780043/diff/1/src/internet/test/tcp-westwoo...
src/internet/test/tcp-westwood-test.cc:139: "ssThresh has not updated
correctly");
Because this test macro is hardcoded to check 3994U, what will happen if the
user creates a new instance of a test case below by passing different arguments
to the constructor?

https://codereview.appspot.com/320780043/diff/1/src/internet/test/tcp-westwoo...
src/internet/test/tcp-westwood-test.cc:140: }
Because this is hardcoded to the value 3994, what will happen if the future
user/maintainer adds a second instance of this test case by passing different
arguments to the test case constructor?

[Mohit Tahiliani, 2018-02-27 13:35:06]

On 2017/05/09 21:54:43, Tom Henderson wrote:
> I have some difficulty correlating this test case with the description for TCP
> Westwood (https://en.wikipedia.org/wiki/TCP_Westwood) since the test is not
> actually running a real connection but just unit testing the object for a
> specific input sequence.
> 
> TCP Westwood talks about Eligible Rate, Agile Probing, and PNCD.  Can you
please
> clarify for the future reader of this code (by adding some class doxygen) what
> elements of Westwood that this test case is testing, and what elements are not
> being checked?
> 
> Do you have plans to add to this test suite in the future?
> 
>
https://codereview.appspot.com/320780043/diff/1/src/internet/test/tcp-westwoo...
> File src/internet/test/tcp-westwood-test.cc (right):
> 
>
https://codereview.appspot.com/320780043/diff/1/src/internet/test/tcp-westwoo...
> src/internet/test/tcp-westwood-test.cc:37: * \brief Test case for TCP Westwood
> Please clarify what this test case is explicitly testing.
> 
> If the user were to substitute some other TCP variant instead of TcpWestwood,
> can you explain here why such variant would fail the test?
> 
>
https://codereview.appspot.com/320780043/diff/1/src/internet/test/tcp-westwoo...
> src/internet/test/tcp-westwood-test.cc:139: "ssThresh has not updated
> correctly");
> Because this test macro is hardcoded to check 3994U, what will happen if the
> user creates a new instance of a test case below by passing different
arguments
> to the constructor?
> 
>
https://codereview.appspot.com/320780043/diff/1/src/internet/test/tcp-westwoo...
> src/internet/test/tcp-westwood-test.cc:140: }
> Because this is hardcoded to the value 3994, what will happen if the future
> user/maintainer adds a second instance of this test case by passing different
> arguments to the test case constructor?

Hello Tom,

Thanks for the reviews and I apologise for the delay in preparing a new patch.

Vivek Jain has prepared a new patch addressing the above comments and is
uploaded on the following link:

https://codereview.appspot.com/338510043/

Regards,
Mohit P. Tahiliani