-
Notifications
You must be signed in to change notification settings - Fork 34
[Data Plane Manager] UT Test Automation #394
[Data Plane Manager] UT Test Automation #394
Conversation
List<NeighborInfo> neighborINFO = new ArrayList<>(); | ||
if (hasNeighbor) { | ||
for (int i = 0; i < neighborNum; i ++) { | ||
NeighborInfo neighborInfo = new NeighborInfo("10.213.43.18" + i, "ephost_0", "f37810eb-7f83-45fa-a4d4-1b31e75399d" + i, "86:ea:77:ad:52:55", "192.168.2.2", "9192a4d4-ffff-4ece-b3f0-8d36e3d88038", "a87e0f87-a2d9-44ef-9194-9a62f1785940"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider to keep these test parameters in a file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NP
@@ -84,13 +84,13 @@ private void buildVpcState(NetworkConfiguration networkConfig, GoalState.Builder | |||
} | |||
|
|||
for (PortState portState: portStates) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@chenpiaoping, please help review this file change. Let me know if there is any concern.
...ta_plane_manager/src/main/java/com/futurewei/alcor/dataplane/utils/DataPlaneManagerUtil.java
Outdated
Show resolved
Hide resolved
...ta_plane_manager/src/main/java/com/futurewei/alcor/dataplane/utils/DataPlaneManagerUtil.java
Show resolved
Hide resolved
for (int j = 0; j < hostNum; j ++) { | ||
for (int i = 0; i < portNum; i ++) { | ||
List<PortEntity.FixedIp> fixedIps = new ArrayList<>(); | ||
PortEntity.FixedIp fixedIp = new PortEntity.FixedIp(DPMAutoUnitTestConstant.subnetId + i, DPMAutoUnitTestConstant.IpAddress + i); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will make each port fall into a different subnet. Don't think it is needed. Let us have a quick sync up. @kevin-zhonghao
...ta_plane_manager/src/main/java/com/futurewei/alcor/dataplane/utils/DataPlaneManagerUtil.java
Show resolved
Hide resolved
...ta_plane_manager/src/main/java/com/futurewei/alcor/dataplane/utils/DataPlaneManagerUtil.java
Outdated
Show resolved
Hide resolved
...ta_plane_manager/src/main/java/com/futurewei/alcor/dataplane/utils/DataPlaneManagerUtil.java
Outdated
Show resolved
Hide resolved
null, false, null, null, 0, null, null, | ||
false, false); | ||
|
||
List<RouteEntity> routeEntities = new ArrayList<>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to use latest contract in PR #396
List<NeighborInfo> neighborINFO = new ArrayList<>(); | ||
if (hasNeighbor) { | ||
for (int i = 0; i < neighborNum; i ++) { | ||
NeighborInfo neighborInfo = new NeighborInfo(DPMAutoUnitTestConstant.hostIp + i, DPMAutoUnitTestConstant.hostId, DPMAutoUnitTestConstant.portId + i, DPMAutoUnitTestConstant.portMac, DPMAutoUnitTestConstant.portIp, DPMAutoUnitTestConstant.vpcId, DPMAutoUnitTestConstant.subnetId + i); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we use DPMAutoUnitTestConstant.portMac and DPMAutoUnitTestConstant.portIp, do that mean that we are generating multiple neighbor ports with same IP and MAC?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, because we assume there is one neighbor if it has neighbor in most of cases. Do we have the case to test multiple neighbor with different IP and MAC?
List<NeighborInfo> neighborINFO = new ArrayList<>(); | ||
if (hasNeighbor) { | ||
for (int i = 0; i < neighborNum; i ++) { | ||
NeighborInfo neighborInfo = new NeighborInfo(DPMAutoUnitTestConstant.hostIp + i, DPMAutoUnitTestConstant.hostId, DPMAutoUnitTestConstant.portId + i, DPMAutoUnitTestConstant.portMac, DPMAutoUnitTestConstant.portIp, DPMAutoUnitTestConstant.vpcId, DPMAutoUnitTestConstant.subnetId + i); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Subnet generation shouldn't reply on DPMAutoUnitTestConstant.subnetId + i.
This means that if neighborNum=N, we will have N neighbor port, each will be in a different subnet. Subnet size is always one, and basically against the following code to generate neighborTable (using NumOfIPsInSubnet1 and NumOfIPsInSubnet2).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's right, I will fix value of the subnet id
Codecov Report
@@ Coverage Diff @@
## master #394 +/- ##
============================================
+ Coverage 36.78% 36.82% +0.03%
- Complexity 1149 1152 +3
============================================
Files 447 447
Lines 10703 10703
Branches 1365 1365
============================================
+ Hits 3937 3941 +4
+ Misses 6232 6230 -2
+ Partials 534 532 -2
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @kevin-zhonghao This PR is a very good improvement and one step towards a fully automated tests for DPM.
Completed DPM's UT test automation