Merge pull request #5738 from telstra/test/improvement-entities-compa…

…rison-ping

[TEST]: Refactoring: Verification of the ping operation

src-java/testing/functional-tests/src/main/groovy/org/openkilda/functionaltests/helpers/model/ComplexFlowPingResponse.groovy

@@ -0,0 +1,49 @@
+package org.openkilda.functionaltests.helpers.model
+
+import static org.openkilda.functionaltests.helpers.model.FlowDirection.FORWARD
+import static org.openkilda.functionaltests.helpers.model.FlowDirection.REVERSE
+
+import org.openkilda.northbound.dto.v2.haflows.HaFlowPingResult
+import org.openkilda.northbound.dto.v2.yflows.SubFlowPingPayload
+import org.openkilda.northbound.dto.v2.yflows.YFlowPingResult
+
+import groovy.transform.ToString
+
+@ToString
+class ComplexFlowPingResponse {
+    boolean pingSuccess
+    String error
+    List<SubFlowPingDiscrepancies> subFlowsDiscrepancies
+
+    ComplexFlowPingResponse(YFlowPingResult pingResponse) {
+        this.pingSuccess = pingResponse.pingSuccess
+        this.error = pingResponse.error
+        this.subFlowsDiscrepancies = collectDiscrepanciesPerSubFlow(pingResponse.subFlows)
+    }
+
+    ComplexFlowPingResponse(HaFlowPingResult pingResponse) {
+        this.pingSuccess = pingResponse.pingSuccess
+        this.error = pingResponse.error
+        this.subFlowsDiscrepancies = collectDiscrepanciesPerSubFlow(pingResponse.subFlows)
+    }
+
+    static private def collectDiscrepanciesPerSubFlow(List<SubFlowPingPayload> subFlowsPingDetails) {
+        List<SubFlowPingDiscrepancies> discrepanciesPerSubFlow = subFlowsPingDetails.collect { subFlowPingDetails ->
+            verifyPingLogic(subFlowPingDetails, FORWARD)
+            verifyPingLogic(subFlowPingDetails, REVERSE)
+
+            HashMap<FlowDirection, String> discrepancies = [:]
+            subFlowPingDetails.forward.pingSuccess ?: discrepancies.put(FORWARD, subFlowPingDetails.forward.error)
+            subFlowPingDetails.reverse.pingSuccess ?: discrepancies.put(REVERSE, subFlowPingDetails.reverse.error)
+
+            return discrepancies.isEmpty() ? null : new SubFlowPingDiscrepancies(subFlowPingDetails.flowId, discrepancies)
+        }.findAll()
+        return discrepanciesPerSubFlow
+    }
+
+    static private void verifyPingLogic(SubFlowPingPayload pingPayload, FlowDirection direction) {
+        def pingResult = direction == FORWARD ? pingPayload.forward : pingPayload.reverse
+        assert (pingResult.pingSuccess && !pingResult.error) || (!pingResult.pingSuccess && pingResult.error),
+                "There is an error in the ping logic for sub-flow ${pingPayload.flowId} $direction: $pingResult"
+    }
+}

src-java/testing/functional-tests/src/main/groovy/org/openkilda/functionaltests/helpers/model/FlowExtended.groovy

@@ -4,6 +4,7 @@ import static groovyx.gpars.GParsPool.withPool
 import static org.openkilda.functionaltests.helpers.FlowNameGenerator.FLOW
 import static org.openkilda.functionaltests.helpers.SwitchHelper.randomVlan
 import static org.openkilda.functionaltests.helpers.Wrappers.wait
+import static org.openkilda.functionaltests.helpers.model.FlowDirection.*
 import static org.openkilda.functionaltests.model.cleanup.CleanupActionType.DELETE_FLOW
 import static org.openkilda.functionaltests.model.cleanup.CleanupActionType.OTHER
 import static org.openkilda.functionaltests.model.cleanup.CleanupAfter.TEST
@@ -416,7 +417,25 @@ class FlowExtended {
         assert validationResponse.findAll { !it.asExpected } == validationResponse.findAll { !it.discrepancies.isEmpty() },
                 "There is an error in the logic of flow validation"
         validationResponse.findAll { !it.discrepancies.isEmpty() }
-                .collectEntries { [(FlowDirection.getByDirection(it.direction)): it.discrepancies] }
+                .collectEntries { [(getByDirection(it.direction)): it.discrepancies] }
+    }
+
+    Map<FlowDirection, String> pingAndCollectDiscrepancies(PingInput pingInput = new PingInput()) {
+        def pingResponse = ping(pingInput)
+        assert pingResponse.flowId == flowId, "Ping response for an incorrect flow"
+        verifyPingLogic(pingResponse, FORWARD)
+        verifyPingLogic(pingResponse, REVERSE)
+
+        Map<FlowDirection, String> discrepancies = [:]
+        pingResponse.forward.pingSuccess ?: discrepancies.put(FORWARD, pingResponse.forward.error)
+        pingResponse.reverse.pingSuccess ?: discrepancies.put(REVERSE, pingResponse.reverse.error)
+        return discrepancies
+    }
+
+    static private void verifyPingLogic(PingOutput pingPayload, FlowDirection direction) {
+        def pingResult = direction == FORWARD ? pingPayload.forward : pingPayload.reverse
+        assert (pingResult.pingSuccess && !pingResult.error) || (!pingResult.pingSuccess && pingResult.error),
+                "There is an error in the ping logic for $pingResult"
     }
 
     FlowReroutePayload sync() {
@@ -490,7 +509,7 @@ class FlowExtended {
     }
 
     def getFlowRulesCountBySwitch(FlowDirection direction, int involvedSwitchesCount, boolean isSwitchServer42) {
-        def flowEndpoint = direction == FlowDirection.FORWARD ? source : destination
+        def flowEndpoint = direction == FORWARD ? source : destination
         def swProps = northbound.getSwitchProperties(flowEndpoint.switchId)
         int count = involvedSwitchesCount - 1;
 

src-java/testing/functional-tests/src/main/groovy/org/openkilda/functionaltests/helpers/model/HaFlowExtended.groovy

@@ -271,9 +271,15 @@ class HaFlowExtended {
         return new HaFlowExtended(haFlow, northboundV2, topologyDefinition, cleanupManager)
     }
 
-    HaFlowPingResult ping(int timeoutMillis) {
+    HaFlowPingResult ping(HaFlowPingPayload haFlowPingPayload = new HaFlowPingPayload(2000)) {
         log.debug("Ping ha-flow '${haFlowId}'")
-        northboundV2.pingHaFlow(haFlowId, new HaFlowPingPayload(timeoutMillis))
+        northboundV2.pingHaFlow(haFlowId, haFlowPingPayload)
+    }
+
+    ComplexFlowPingResponse pingAndCollectDiscrepancies(HaFlowPingPayload haFlowPingPayload = new HaFlowPingPayload(2000)) {
+        def response = ping(haFlowPingPayload)
+        assert response.haFlowId == haFlowId, "Ping response for an incorrect ha-flow"
+        new ComplexFlowPingResponse(response)
     }
 
     List<SwitchPortVlan> occupiedEndpoints() {

src-java/testing/functional-tests/src/main/groovy/org/openkilda/functionaltests/helpers/model/SubFlowPingDiscrepancies.groovy

@@ -0,0 +1,11 @@
+package org.openkilda.functionaltests.helpers.model
+
+import groovy.transform.ToString
+import groovy.transform.TupleConstructor
+
+@TupleConstructor
+@ToString
+class SubFlowPingDiscrepancies {
+    String subFlowId
+    Map<FlowDirection, String> flowDiscrepancies
+}

src-java/testing/functional-tests/src/main/groovy/org/openkilda/functionaltests/helpers/model/YFlowExtended.groovy

@@ -275,6 +275,12 @@ class YFlowExtended {
         northboundV2.pingYFlow(yFlowId, payload)
     }
 
+    ComplexFlowPingResponse pingAndCollectDiscrepancies(YFlowPingPayload payload = new YFlowPingPayload(2000)) {
+        def response = ping(payload)
+        assert response.getYFlowId() == yFlowId, "Ping response for an incorrect Y-Flow"
+        new ComplexFlowPingResponse(response)
+    }
+
     YFlowRerouteResult reroute() {
         return northboundV2.rerouteYFlow(yFlowId)
     }

src-java/testing/functional-tests/src/test/groovy/org/openkilda/functionaltests/spec/flows/AutoRerouteSpec.groovy

@@ -529,11 +529,7 @@ triggering one more reroute of the current path"
 
         and: "Flow is pingable"
         retry(3, 0) { //Was unstable on Jenkins builds. Fresh env problem?
-            with(flow.ping()) {
-                it.forward.pingSuccess
-                it.reverse.pingSuccess
-            }
-            true
+            flow.pingAndCollectDiscrepancies().isEmpty()
         }
     }
 

src-java/testing/functional-tests/src/test/groovy/org/openkilda/functionaltests/spec/flows/ConnectedDevicesSpec.groovy

@@ -509,10 +509,7 @@ srcDevices=#newSrcEnabled, dstDevices=#newDstEnabled"() {
 
         and: "Flow is valid and pingable"
         flow.validateAndCollectDiscrepancies().isEmpty()
-        verifyAll(flow.ping()) {
-            assert it.forward.pingSuccess
-            assert it.reverse.pingSuccess
-        }
+        flow.pingAndCollectDiscrepancies().isEmpty()
 
         and: "Device sends an lldp+arp packet into a flow port on that switch (with a correct flow vlan)"
         device.sendLldp(lldpData)

src-java/testing/functional-tests/src/test/groovy/org/openkilda/functionaltests/spec/flows/FlowCrudSpec.groovy

@@ -899,10 +899,7 @@ types .* or update switch properties and add needed encapsulation type./).matche
 
         and: "Flow is valid and pingable"
         updatedFlow.validateAndCollectDiscrepancies().isEmpty()
-        with(updatedFlow.ping()) {
-            it.forward.pingSuccess
-            it.reverse.pingSuccess
-        }
+        updatedFlow.pingAndCollectDiscrepancies().isEmpty()
 
         and: "The src switch passes switch validation"
         !switchHelper.synchronizeAndCollectFixedDiscrepancies(srcSwitch.getDpId()).isPresent()
@@ -942,10 +939,7 @@ types .* or update switch properties and add needed encapsulation type./).matche
 
         and: "Flow is valid and pingable"
         updatedFlow.validateAndCollectDiscrepancies().isEmpty()
-        with(updatedFlow.ping()) {
-            it.forward.pingSuccess
-            it.reverse.pingSuccess
-        }
+        updatedFlow.pingAndCollectDiscrepancies().isEmpty()
 
         and: "The new and old dst switches pass switch validation"
         wait(RULES_DELETION_TIME) {
@@ -1090,10 +1084,7 @@ types .* or update switch properties and add needed encapsulation type./).matche
 
         and: "Flow is valid and pingable"
         updatedFlow.validateAndCollectDiscrepancies().isEmpty()
-        with(updatedFlow.ping()) {
-            it.forward.pingSuccess
-            it.reverse.pingSuccess
-        }
+        updatedFlow.pingAndCollectDiscrepancies().isEmpty()
 
         and: "Involved switches pass switch validation"
         def involvedSwitches = updatedFlow.retrieveAllEntityPaths().getInvolvedSwitches()

src-java/testing/functional-tests/src/test/groovy/org/openkilda/functionaltests/spec/flows/FlowPingSpec.groovy

@@ -1,6 +1,6 @@
 package org.openkilda.functionaltests.spec.flows
 
-import static com.shazam.shazamcrest.matcher.Matchers.sameBeanAs
+
 import static org.junit.jupiter.api.Assumptions.assumeTrue
 import static org.openkilda.functionaltests.extension.tags.Tag.LOW_PRIORITY
 import static org.openkilda.functionaltests.extension.tags.Tag.SMOKE
@@ -9,14 +9,14 @@ import static org.openkilda.testing.Constants.DefaultRule.VERIFICATION_UNICAST_R
 import static org.openkilda.testing.Constants.DefaultRule.VERIFICATION_UNICAST_VXLAN_RULE_COOKIE
 import static org.openkilda.testing.Constants.STATS_LOGGING_TIMEOUT
 import static org.openkilda.testing.Constants.WAIT_OFFSET
-import static spock.util.matcher.HamcrestSupport.expect
 
 import org.openkilda.functionaltests.HealthCheckSpecification
 import org.openkilda.functionaltests.error.flow.FlowNotCreatedExpectedError
 import org.openkilda.functionaltests.extension.tags.IterationTag
 import org.openkilda.functionaltests.extension.tags.Tags
 import org.openkilda.functionaltests.helpers.Wrappers
 import org.openkilda.functionaltests.helpers.factory.FlowFactory
+import org.openkilda.functionaltests.helpers.model.FlowDirection
 import org.openkilda.functionaltests.helpers.model.FlowEncapsulationType
 import org.openkilda.functionaltests.helpers.model.Path
 import org.openkilda.functionaltests.helpers.model.SwitchRulesFactory
@@ -174,8 +174,6 @@ class FlowPingSpec extends HealthCheckSpecification {
 
         //build a flow
         def flow = flowFactory.getRandom(swPair)
-
-        expectedPingResult.flowId = flow.flowId
         assert aswitchPathIsls == flow.retrieveAllEntityPaths().getInvolvedIsls()
 
         when: "Break the flow by removing rules from a-switch"
@@ -186,57 +184,54 @@ class FlowPingSpec extends HealthCheckSpecification {
         aSwitchFlows.removeFlows(rulesToRemove)
 
         and: "Ping the flow"
-        def response = flow.ping(data.pingInput)
+        def discrepancies = flow.pingAndCollectDiscrepancies(data.pingInput)
 
         then: "Ping response properly shows that certain direction is unpingable"
-        expect response, sameBeanAs(expectedPingResult)
-                .ignoring("forward.latency").ignoring("reverse.latency")
+        discrepancies == data.expectedDiscrepancy
 
         where:
         data << [
                 [
                         breakForward: true,
                         breakReverse: false,
-                        pingInput   : new PingInput()
+                        pingInput   : new PingInput(),
+                        expectedDiscrepancy: [(FlowDirection.FORWARD): "No ping for reasonable time"]
                 ],
                 [
                         breakForward: false,
                         breakReverse: true,
-                        pingInput   : new PingInput()
+                        pingInput   : new PingInput(),
+                        expectedDiscrepancy: [(FlowDirection.REVERSE): "No ping for reasonable time"]
                 ],
                 [
                         breakForward: true,
                         breakReverse: true,
-                        pingInput   : new PingInput()
+                        pingInput   : new PingInput(),
+                        expectedDiscrepancy: [(FlowDirection.FORWARD): "No ping for reasonable time",
+                                              (FlowDirection.REVERSE): "No ping for reasonable time"]
                 ],
                 [
                         breakForward: true,
                         breakReverse: false,
-                        pingInput   : new PingInput((getDiscoveryInterval() + 1) * 1000)
+                        pingInput   : new PingInput((getDiscoveryInterval() + 1) * 1000),
+                        expectedDiscrepancy: [(FlowDirection.FORWARD): "No ping for reasonable time"]
                 ],
                 [
                         breakForward: false,
                         breakReverse: true,
-                        pingInput   : new PingInput((getDiscoveryInterval() + 1) * 1000)
+                        pingInput   : new PingInput((getDiscoveryInterval() + 1) * 1000),
+                        expectedDiscrepancy: [(FlowDirection.REVERSE): "No ping for reasonable time"]
                 ],
                 [
                         breakForward: true,
                         breakReverse: true,
-                        pingInput   : new PingInput((getDiscoveryInterval() + 1) * 1000)
+                        pingInput   : new PingInput((getDiscoveryInterval() + 1) * 1000),
+                        expectedDiscrepancy: [(FlowDirection.FORWARD): "No ping for reasonable time",
+                                              (FlowDirection.REVERSE): "No ping for reasonable time"]
                 ]
         ]
         description = "${data.breakForward ? "forward" : ""}${data.breakForward && data.breakReverse ? " and " : ""}" +
                 "${data.breakReverse ? "reverse" : ""} path with ${data.pingInput.timeoutMillis}ms timeout"
-
-        expectedPingResult = new PingOutputBuilder().forward(
-                new UniFlowPingOutput(
-                        pingSuccess: !data.breakForward,
-                        error: data.breakForward ? "No ping for reasonable time" : null)).reverse(
-                new UniFlowPingOutput(
-                        pingSuccess: !data.breakReverse,
-                        error: data.breakReverse ? "No ping for reasonable time" : null))
-                .error(null)
-                .build()
     }
 
     def "Unable to ping a single-switch flow"() {
@@ -276,11 +271,7 @@ class FlowPingSpec extends HealthCheckSpecification {
         def flow = flowFactory.getBuilder(switchPair)
                 .withEncapsulationType(FlowEncapsulationType.VXLAN).build()
                 .create()
-        //make sure that flow is pingable
-        with(flow.ping()) {
-            it.forward.pingSuccess
-            it.reverse.pingSuccess
-        }
+       assert flow.pingAndCollectDiscrepancies().isEmpty()
 
         when: "Break the flow by removing flow rules from the intermediate switch"
         def intermediateSwId = flow.retrieveAllEntityPaths().getInvolvedSwitches()[1]
@@ -291,18 +282,14 @@ class FlowPingSpec extends HealthCheckSpecification {
             switchHelper.deleteSwitchRules(intermediateSwId, cookie)
         }
 
-        and: "Ping the flow"
-        def response = flow.ping()
-
         then: "Ping shows that path is broken"
-        !response.forward.pingSuccess
-        !response.reverse.pingSuccess
+        assert flow.pingAndCollectDiscrepancies().keySet() == [FlowDirection.FORWARD, FlowDirection.REVERSE].toSet()
     }
 
     def "Able to turn on periodic pings on a flow"() {
         when: "Create a flow with periodic pings turned on"
         def endpointSwitches = switchPairs.all().nonNeighbouring().random()
-        def flow = flowFactory.getBuilder(endpointSwitches).withPeriodicPing(true).build()
+        flowFactory.getBuilder(endpointSwitches).withPeriodicPing(true).build()
                 .create()
 
         then: "Packet counter on catch ping rules grows due to pings happening"
@@ -321,7 +308,7 @@ class FlowPingSpec extends HealthCheckSpecification {
     def "Unable to create a single-switch flow with periodic pings"() {
         when: "Try to create a single-switch flow with periodic pings"
         def singleSwitch = topology.activeSwitches.first()
-        def flow = flowFactory.getBuilder(singleSwitch, singleSwitch)
+        flowFactory.getBuilder(singleSwitch, singleSwitch)
                 .withPeriodicPing(true).build()
                 .create()
 

src-java/testing/functional-tests/src/test/groovy/org/openkilda/functionaltests/spec/flows/PartialUpdateSpec.groovy

@@ -282,10 +282,7 @@ class PartialUpdateSpec extends HealthCheckSpecification {
 
         and: "Flow is valid and pingable"
         flow.validateAndCollectDiscrepancies().isEmpty()
-        with(flow.ping()) {
-            it.forward.pingSuccess
-            it.reverse.pingSuccess
-        }
+        flow.pingAndCollectDiscrepancies().isEmpty()
 
         and: "The src switch passes switch validation"
         !switchHelper.synchronizeAndCollectFixedDiscrepancies(srcSwitch.dpId).isPresent()
@@ -329,10 +326,7 @@ class PartialUpdateSpec extends HealthCheckSpecification {
 
         and: "Flow is valid and pingable"
         flow.validateAndCollectDiscrepancies().isEmpty()
-        with(flow.ping()) {
-            it.forward.pingSuccess
-            it.reverse.pingSuccess
-        }
+        flow.pingAndCollectDiscrepancies().isEmpty()
 
         and: "The new and old dst switches pass switch validation"
         Wrappers.wait(RULES_DELETION_TIME) {