Refactor e2e leak tests

The main purpose of this refactoring is to reduce
the multiple layers of negation and to generally reduce
the risk of misunderstanding the test code.

3 files changed, 76 insertions, 46 deletions

diff --git a/android/test/e2e/src/main/kotlin/net/mullvad/mullvadvpn/test/e2e/LeakTest.kt b/android/test/e2e/src/main/kotlin/net/mullvad/mullvadvpn/test/e2e/LeakTest.kt
index 009a9aba2f..63b3bd1ae1 100644
--- a/android/test/e2e/src/main/kotlin/net/mullvad/mullvadvpn/test/e2e/LeakTest.kt
+++ b/android/test/e2e/src/main/kotlin/net/mullvad/mullvadvpn/test/e2e/LeakTest.kt
@@ -15,8 +15,10 @@ import net.mullvad.mullvadvpn.test.common.page.on
 import net.mullvad.mullvadvpn.test.common.rule.ForgetAllVpnAppsInSettingsTestRule
 import net.mullvad.mullvadvpn.test.e2e.annotations.HasDependencyOnLocalAPI
 import net.mullvad.mullvadvpn.test.e2e.misc.AccountTestRule
-import net.mullvad.mullvadvpn.test.e2e.misc.LeakCheck
+import net.mullvad.mullvadvpn.test.e2e.misc.NetworkTrafficChecker
 import net.mullvad.mullvadvpn.test.e2e.misc.NoTrafficToHostRule
+import net.mullvad.mullvadvpn.test.e2e.misc.SomeTrafficToHostRule
+import net.mullvad.mullvadvpn.test.e2e.misc.SomeTrafficToOtherHostsRule
 import net.mullvad.mullvadvpn.test.e2e.misc.TrafficGenerator
 import net.mullvad.mullvadvpn.test.e2e.router.packetCapture.PacketCapture
 import net.mullvad.mullvadvpn.test.e2e.router.packetCapture.PacketCaptureResult
@@ -58,7 +60,7 @@ class LeakTest : EndToEndTest(BuildConfig.FLAVOR_infrastructure) {
 
     @Test
     @HasDependencyOnLocalAPI
-    fun testNegativeLeak() =
+    fun testEnsureNoLeaksToSpecificHost() =
         runBlocking<Unit> {
             app.launch()
 
@@ -94,15 +96,20 @@ class LeakTest : EndToEndTest(BuildConfig.FLAVOR_infrastructure) {
             val capturedStreams = captureResult.streams
             val capturedPcap = captureResult.pcap
             val timestamp = System.currentTimeMillis()
-            Attachment.saveAttachment("capture-testNegativeLeak-$timestamp.pcap", capturedPcap)
+            Attachment.saveAttachment(
+                "capture-${javaClass.enclosingMethod}-$timestamp.pcap",
+                capturedPcap,
+            )
 
-            val leakRules = listOf(NoTrafficToHostRule(targetIpAddress))
-            LeakCheck.assertNoLeaks(capturedStreams, leakRules)
+            NetworkTrafficChecker.checkTrafficStreamsAgainstRules(
+                capturedStreams,
+                NoTrafficToHostRule(targetIpAddress),
+            )
         }
 
     @Test
     @HasDependencyOnLocalAPI
-    fun testShouldHaveNegativeLeak() =
+    fun testEnsureLeaksToSpecificHost() =
         runBlocking<Unit> {
             app.launch()
 
@@ -154,15 +161,21 @@ class LeakTest : EndToEndTest(BuildConfig.FLAVOR_infrastructure) {
             val capturedStreams = captureResult.streams
             val capturedPcap = captureResult.pcap
             val timestamp = System.currentTimeMillis()
-            Attachment.saveAttachment("capture-testShouldHaveLeak-$timestamp.pcap", capturedPcap)
+            Attachment.saveAttachment(
+                "capture-${javaClass.enclosingMethod}-$timestamp.pcap",
+                capturedPcap,
+            )
 
-            val leakRules = listOf(NoTrafficToHostRule(targetIpAddress))
-            LeakCheck.assertLeaks(capturedStreams, leakRules)
+            NetworkTrafficChecker.checkTrafficStreamsAgainstRules(
+                capturedStreams,
+                SomeTrafficToHostRule(targetIpAddress),
+                SomeTrafficToOtherHostsRule(targetIpAddress),
+            )
         }
 
     @Test
     @HasDependencyOnLocalAPI
-    fun testLeakWhenVpnSettingsChange() =
+    fun testEnsureNoLeaksToSpecificHostWhenSwitchingBetweenVariousVpnSettings() =
         runBlocking<Unit> {
             app.launch()
             // Obfuscation and Post-Quantum are by default set to automatic. Explicitly set to off.
@@ -208,12 +221,14 @@ class LeakTest : EndToEndTest(BuildConfig.FLAVOR_infrastructure) {
             val capturedPcap = captureResult.pcap
             val timestamp = System.currentTimeMillis()
             Attachment.saveAttachment(
-                "capture-testLeakWhenVpnSettingsChange-$timestamp.pcap",
+                "capture-${javaClass.enclosingMethod}-$timestamp.pcap",
                 capturedPcap,
             )
 
-            val leakRules = listOf(NoTrafficToHostRule(targetIpAddress))
-            LeakCheck.assertNoLeaks(capturedStreams, leakRules)
+            NetworkTrafficChecker.checkTrafficStreamsAgainstRules(
+                capturedStreams,
+                NoTrafficToHostRule(targetIpAddress),
+            )
         }
 
     private fun disableObfuscation() {
diff --git a/android/test/e2e/src/main/kotlin/net/mullvad/mullvadvpn/test/e2e/misc/LeakCheck.kt b/android/test/e2e/src/main/kotlin/net/mullvad/mullvadvpn/test/e2e/misc/LeakCheck.kt
deleted file mode 100644
index 6770551f65..0000000000
--- a/android/test/e2e/src/main/kotlin/net/mullvad/mullvadvpn/test/e2e/misc/LeakCheck.kt
+++ /dev/null
@@ -1,33 +0,0 @@
-package net.mullvad.mullvadvpn.test.e2e.misc
-
-import net.mullvad.mullvadvpn.test.e2e.router.packetCapture.Stream
-import org.junit.jupiter.api.Assertions.assertFalse
-import org.junit.jupiter.api.Assertions.assertTrue
-
-object LeakCheck {
-    fun assertNoLeaks(streams: List<Stream>, rules: List<LeakRule>) {
-        // Assert that there are streams to be analyzed. Stream objects are guaranteed to contain
-        // packets when initialized.
-        assertTrue(streams.isNotEmpty())
-
-        for (rule in rules) {
-            assertFalse(rule.isViolated(streams))
-        }
-    }
-
-    fun assertLeaks(streams: List<Stream>, rules: List<LeakRule>) {
-        for (rule in rules) {
-            assertTrue(rule.isViolated(streams))
-        }
-    }
-}
-
-interface LeakRule {
-    fun isViolated(streams: List<Stream>): Boolean
-}
-
-class NoTrafficToHostRule(private val host: String) : LeakRule {
-    override fun isViolated(streams: List<Stream>): Boolean {
-        return streams.any { it.destinationHost.ipAddress == host }
-    }
-}
diff --git a/android/test/e2e/src/main/kotlin/net/mullvad/mullvadvpn/test/e2e/misc/NetworkTrafficChecker.kt b/android/test/e2e/src/main/kotlin/net/mullvad/mullvadvpn/test/e2e/misc/NetworkTrafficChecker.kt
new file mode 100644
index 0000000000..373cd47026
--- /dev/null
+++ b/android/test/e2e/src/main/kotlin/net/mullvad/mullvadvpn/test/e2e/misc/NetworkTrafficChecker.kt
@@ -0,0 +1,48 @@
+package net.mullvad.mullvadvpn.test.e2e.misc
+
+import net.mullvad.mullvadvpn.test.e2e.router.packetCapture.Stream
+import org.junit.jupiter.api.Assertions.assertNotEquals
+import org.junit.jupiter.api.Assertions.assertTrue
+
+object NetworkTrafficChecker {
+    fun checkTrafficStreamsAgainstRules(streams: List<Stream>, vararg rules: TrafficRule) {
+        // Assert that there are streams to be analyzed. Stream objects are guaranteed to contain
+        // packets when initialized.
+        assertTrue(streams.isNotEmpty(), "List of streams is empty.")
+
+        for (rule in rules) {
+            rule.assertTraffic(streams)
+        }
+    }
+}
+
+interface TrafficRule {
+    fun assertTraffic(streams: List<Stream>)
+}
+
+class NoTrafficToHostRule(private val host: String) : TrafficRule {
+    override fun assertTraffic(streams: List<Stream>) {
+        streams.forEach { assertNotEquals(host, it.destinationHost.ipAddress) }
+    }
+}
+
+class SomeTrafficToHostRule(private val host: String) : TrafficRule {
+    override fun assertTraffic(streams: List<Stream>) {
+        val hasAnyTrafficToSpecifiedHost = streams.any { it.destinationHost.ipAddress == host }
+        assertTrue(
+            hasAnyTrafficToSpecifiedHost,
+            "Expected some traffic to the specified host ($host)," +
+                "but all traffic had other destinations addresses.",
+        )
+    }
+}
+
+class SomeTrafficToOtherHostsRule(private val hostToExclude: String) : TrafficRule {
+    override fun assertTraffic(streams: List<Stream>) {
+        val hasAnyTrafficToOtherHost = streams.any { it.destinationHost.ipAddress != hostToExclude }
+        assertTrue(
+            hasAnyTrafficToOtherHost,
+            "Expected some traffic to leak, but all traffic had destination address: $hostToExclude",
+        )
+    }
+}