Address comments in #149 #197
Conversation
The concept of TestPodConfig and TestPod is defined, letting us create testing pods from a single interface, and automating the creation of UUID data to exchange. As late modifications has shown that the number of parameters keeps growing having a single fixture to create test pods will make easier to write and read E2E tests. Depends-On: submariner-io#148
- renamed TestPod to NetworkPod - renamed createListenerConnectorPair t0 runAndVerifyNetworkPod2ServicePair - renamed other function names where appropriate Signed-off-by: Tom Pantelis <tompantelis@gmail.com>
| @@ -30,51 +30,51 @@ var _ = Describe("[dataplane] Basic Pod to Service tests across clusters without | |||
| }) | |||
| }) | |||
|
|
|||
| func testPod2ServiceTCP(f *framework.Framework, leftScheduling framework.TestPodScheduling, rightScheduling framework.TestPodScheduling) { | |||
| func testPod2ServiceTCP(f *framework.Framework, leftScheduling framework.NetworkPodScheduling, rightScheduling framework.NetworkPodScheduling) { | |||
There was a problem hiding this comment.
I like "TestPod..." more as it is generic. NetworkPod... becomes more specific which at this point is not needed IMO
There was a problem hiding this comment.
Yeah, there was a discussion on the other patch around this.
TestPod is shorter, I agree with you, and we can assume that pods are going to be "network" always for us... but I'm ok with the current patch, we need to make progress. And thanks tom for proactively proposing the amendments.
There was a problem hiding this comment.
I'm OK either way - just trying to address Mike's concerns. While TestPod is generic, these pods really aren't generic, ie they run specific commands to test networking so NetworkPod seems more meaningful in this case.
| np.Pod = finalPod | ||
| } | ||
|
|
||
| func (np *NetworkPod) AwaitSuccessfulFinish() { |
There was a problem hiding this comment.
My version was more generic though. For example being able to wait for non-successful makes it possible to do negative tests in network policies (pods don't connect), but we can add another function on a later PR very easily. The resulting test code is cleaner your way.
There was a problem hiding this comment.
yeah that's what I was thinking, ie add AwaitFinish later if needed.
#149