Add network connectivity stress test for VPN resilience#162
Merged
Conversation
Instrumented test that simulates real-world network disruptions (WiFi/mobile switching, airplane mode, flapping, long outages) and verifies VPN recovery via ping. Auto-detects real device vs emulator and uses appropriate network control strategy for each.
Contributor
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 43 minutes and 22 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds a new Android instrumentation stress test that runs randomized network disruption/restore cycles to validate VPN recovery, plus supporting README docs and test dependency/version additions. Changes
Sequence Diagram(s)sequenceDiagram
participant Test as NetworkConnectivityStressTest
participant UI as MainActivity/UI
participant VPN as VPN Service
participant Network as Device/Emulator Network Control
participant Ping as Ping Utility
Test->>Network: Restore connectivity
Test->>UI: Launch app & request VPN connect
UI->>VPN: Initiate connection
VPN-->>UI: Connection state updates
Test->>Ping: Verify ping to PING_TARGET
Ping-->>Test: Success
loop NUM_CYCLES
Test->>Network: Apply random disruption (WiFi/Data/Airplane/Emulator)
Network-->>Test: Disruption applied
Test->>Test: Wait + randomized delay
Test->>Network: Restore connectivity
Network-->>Test: Connectivity restored
loop Recovery verification
Test->>Ping: Single ping attempt
Ping-->>Test: Response or timeout
end
Test->>Test: Record pass/fail
end
Test->>Network: Final restoration
Test->>Test: Log summary
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
app/src/androidTest/java/io/netbird/client/NetworkConnectivityStressTest.java (3)
108-111: Consider migrating to ActivityScenarioRule in the future.
ActivityTestRuleis deprecated in favor ofActivityScenarioRule. The current usage is acceptable given the suppression annotation, but consider migrating when updating the test infrastructure.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/androidTest/java/io/netbird/client/NetworkConnectivityStressTest.java` around lines 108 - 111, The test currently uses the deprecated ActivityTestRule (activityRule: ActivityTestRule<MainActivity>) with a suppression; replace it with ActivityScenarioRule<MainActivity> when you migrate the test: remove the `@SuppressWarnings`("deprecation") and ActivityTestRule declaration and instead instantiate an ActivityScenarioRule for MainActivity (keep the rule name activityRule or similar) so tests run with the newer ActivityScenario API; update imports and any usages of activityRule.getActivity() to the ActivityScenario-compatible patterns (e.g., ActivityScenarioRule and ActivityScenario methods).
72-73: Consider making PING_TARGET configurable via test arguments.The hardcoded IP requires code changes for different test environments. This could be made configurable via
testInstrumentationRunnerArgumentsfor easier CI/CD integration.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/androidTest/java/io/netbird/client/NetworkConnectivityStressTest.java` around lines 72 - 73, Replace the hardcoded PING_TARGET in NetworkConnectivityStressTest with a configurable value read from test instrumentation arguments: read the "ping_target" (or similar key) from InstrumentationRegistry.getArguments() (or InstrumentationRegistry.getInstrumentation().getArguments()) during class setup (e.g., `@BeforeClass`) and assign it to PING_TARGET (or a non-final static field) with a sensible default of "100.72.116.70" if the argument is missing; update references to use that field so CI/CD can override the target via testInstrumentationRunnerArguments.
491-498: Iptables fallback flushes entire chains.The fallback uses
iptables -Fwhich flushes all rules in the OUTPUT/INPUT chains, not just the DROP rules added by this test. This could remove other rules if present. For test code on a dedicated emulator this is acceptable, but be aware of this behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/androidTest/java/io/netbird/client/NetworkConnectivityStressTest.java` around lines 491 - 498, The current fallback in the network disconnect/connect branches of the command handling uses "iptables -A OUTPUT -j DROP" and later "iptables -F OUTPUT"/"iptables -F INPUT", which flushes entire chains; change this to add a distinguishable rule (for example add the DROP rule with a unique comment) when executing the disconnect path in the block handling command.equals("network disconnect"), and on the connect path (command.equals("network connect")) remove only that specific rule (use iptables -D with the same matching parameters or match the comment) instead of using iptables -F; update the executeShellCommand invocations accordingly so the test only removes the DROP rules it added rather than flushing INPUT/OUTPUT chains.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@app/src/androidTest/java/io/netbird/client/NetworkConnectivityStressTest.java`:
- Around line 463-500: The sendEmulatorConsoleCommand method leaks resources if
an exception occurs after creating the Socket/BufferedReader/PrintWriter; change
it to use try-with-resources so the Socket, BufferedReader and PrintWriter are
declared in the try(...) header and will be auto-closed (remove the explicit
socket.close()), keep the existing logic that reads the banner via readUntilOK,
authenticates with EMULATOR_AUTH_TOKEN, sends the command, logs the response,
and retains the catch block that logs the error and falls back to
executeShellCommand for "network disconnect"/"network connect"; ensure
EMULATOR_CONSOLE_PORT is still used and no behavior changes otherwise.
In `@app/src/androidTest/README.md`:
- Around line 114-116: Update the fenced code block containing "=== Results:
18/20 passed, 2 failed ===" to include a language specifier (e.g., replace the
opening ``` with ```text) so the block becomes ```text ... ``` to enable proper
syntax highlighting and satisfy markdown lint rules.
In `@gradle/libs.versions.toml`:
- Around line 7-8: Update the testRules version entry from "1.6.1" to "1.7.0" in
libs.versions.toml (leave uiautomator at "2.3.0"), and make the same change for
any other duplicate testRules entries in the file (e.g., the other occurrence
referenced in the review) so all testRules declarations are consistently 1.7.0.
---
Nitpick comments:
In
`@app/src/androidTest/java/io/netbird/client/NetworkConnectivityStressTest.java`:
- Around line 108-111: The test currently uses the deprecated ActivityTestRule
(activityRule: ActivityTestRule<MainActivity>) with a suppression; replace it
with ActivityScenarioRule<MainActivity> when you migrate the test: remove the
`@SuppressWarnings`("deprecation") and ActivityTestRule declaration and instead
instantiate an ActivityScenarioRule for MainActivity (keep the rule name
activityRule or similar) so tests run with the newer ActivityScenario API;
update imports and any usages of activityRule.getActivity() to the
ActivityScenario-compatible patterns (e.g., ActivityScenarioRule and
ActivityScenario methods).
- Around line 72-73: Replace the hardcoded PING_TARGET in
NetworkConnectivityStressTest with a configurable value read from test
instrumentation arguments: read the "ping_target" (or similar key) from
InstrumentationRegistry.getArguments() (or
InstrumentationRegistry.getInstrumentation().getArguments()) during class setup
(e.g., `@BeforeClass`) and assign it to PING_TARGET (or a non-final static field)
with a sensible default of "100.72.116.70" if the argument is missing; update
references to use that field so CI/CD can override the target via
testInstrumentationRunnerArguments.
- Around line 491-498: The current fallback in the network disconnect/connect
branches of the command handling uses "iptables -A OUTPUT -j DROP" and later
"iptables -F OUTPUT"/"iptables -F INPUT", which flushes entire chains; change
this to add a distinguishable rule (for example add the DROP rule with a unique
comment) when executing the disconnect path in the block handling
command.equals("network disconnect"), and on the connect path
(command.equals("network connect")) remove only that specific rule (use iptables
-D with the same matching parameters or match the comment) instead of using
iptables -F; update the executeShellCommand invocations accordingly so the test
only removes the DROP rules it added rather than flushing INPUT/OUTPUT chains.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5eacabf4-4fa2-456a-a59f-dd50e323fcd9
📒 Files selected for processing (4)
app/build.gradle.ktsapp/src/androidTest/README.mdapp/src/androidTest/java/io/netbird/client/NetworkConnectivityStressTest.javagradle/libs.versions.toml
- Use try-with-resources in sendEmulatorConsoleCommand to prevent socket leak - Bump testRules version from 1.6.1 to 1.7.0 - Add language specifier to fenced code block in README
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@app/src/androidTest/java/io/netbird/client/NetworkConnectivityStressTest.java`:
- Around line 318-320: The DisruptionType entry creates a mismatch: the label
"EMU: Extreme latency (5s delay)" but the applied delay uses
emuNetworkDelay("10000") (10s). Fix by making the label and value consistent —
either change the label to "(10s delay)" or change the emuNetworkDelay argument
to "5000"; update the DisruptionType line that references DisruptionType(...)
and the emuNetworkDelay(...) calls accordingly so the human-readable label
matches the configured delay.
- Around line 501-514: The readUntilOK method currently swallows exceptions and
returns partial output; change it to fail fast: in readUntilOK(BufferedReader
reader) stop swallowing exceptions—propagate IOExceptions (or wrap and throw a
RuntimeException) instead of returning partial text, and after the loop if EOF
is reached without a line containing "OK" (or if a line contains "KO") throw an
exception indicating the command/auth failed and include the collected output in
the exception message so callers can detect failure; keep the same loop and
StringBuilder logic but remove the empty catch and add explicit failure throws
when "KO" or no "OK" is observed.
- Around line 164-196: The current cycle applies a network disruption with
disruption.apply.run() but only calls disruption.restore.run() in the normal
path, so if an exception occurs the network isn't restored; change the per-cycle
control flow so that after calling disruption.apply.run() you always restore in
a finally block (e.g., use try { apply; ... verify; } finally { try {
disruption.restore.run(); } catch(Exception restoreEx) { log/handle restore
failure } }), keep existing logging and passCount/failCount updates (from
waitForPingSuccess and exception handling) but ensure restore is executed even
when apply or the verification throws; reference disruption.apply,
disruption.restore, waitForPingSuccess and the per-cycle exception handling to
locate where to wrap the restore.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 41421730-1e36-497f-ae3a-8624c11b9148
📒 Files selected for processing (3)
app/src/androidTest/README.mdapp/src/androidTest/java/io/netbird/client/NetworkConnectivityStressTest.javagradle/libs.versions.toml
✅ Files skipped from review due to trivial changes (1)
- app/src/androidTest/README.md
🚧 Files skipped from review as they are similar to previous changes (1)
- gradle/libs.versions.toml
…error handling - Always call disruption.restore in finally block to prevent stale network state - Fix EMU latency label from 5s to 10s to match actual delay value - Make readUntilOK propagate IOException and detect KO/EOF failures
This test requires a configured VPN and real network interfaces, so it cannot run on the CI emulator. Run it manually on a real device.
lixmal
approved these changes
Apr 14, 2026
lixmal
approved these changes
Apr 14, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds an instrumented stress test that simulates real-world network disruptions and verifies that the VPN recovers automatically
Covers 10 real-device scenarios (WiFi/mobile switching, airplane mode, flapping, long outages) and 10 emulator scenarios (network disconnect, latency, speed throttling)
Auto-detects real device vs emulator and applies the appropriate network control strategy
Verifies VPN recovery by pinging a peer through the tunnel with exponential backoff
Background
Users reported that the VPN can get stuck in a disconnected state after network changes—for example, when leaving the office (WiFi → mobile) or arriving home (mobile → WiFi).
This test reproduces those conditions by randomly applying network disruptions across 20 cycles.
How to run
./gradlew assembleDebug assembleDebugAndroidTestSee app/src/androidTest/README.md for full details, configuration options, and helper commands.
Summary by CodeRabbit
Tests
Documentation
Chores