Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(12)

Issue 320780043: Test suite for TCP Westwood

Can't Edit
Can't Publish+Mail
Start Review
Created:
5 years, 1 month ago by chirag.jamadagni
Modified:
4 years, 2 months ago
CC:
ns-3-reviews_googlegroups.com
Visibility:
Public.

Description

Test suite for TCP Westwood

Patch Set 1 #

Total comments: 3
Unified diffs Side-by-side diffs Delta from patch set Stats (+158 lines, -0 lines) Patch
A src/internet/test/tcp-westwood-test.cc View 1 chunk +157 lines, -0 lines 3 comments Download
M src/internet/wscript View 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 2
Tom Henderson
I have some difficulty correlating this test case with the description for TCP Westwood (https://en.wikipedia.org/wiki/TCP_Westwood) ...
5 years ago (2017-05-09 21:54:43 UTC) #1
Mohit Tahiliani
4 years, 2 months ago (2018-02-27 13:35:06 UTC) #2
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
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b