Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(cordova/apple/ios): remove reachability test #1663

Merged
merged 2 commits into from
Jul 11, 2023
Merged

Conversation

fortuna
Copy link
Collaborator

@fortuna fortuna commented Jul 11, 2023

The test adds too much complexity, without much value.
Most importantly, it requires the per-platform code to inspect the config, which we want to avoid.

This is a step closer to config-agnostic platform code.

I'm removing the reachability code from iOS. We can do the same cleanup on Android and outline-go-tun2socks, but I'll leave that for later, since it doesn't block the config work.

/cc @bi-kh @daniellacosse @sbruens

@fortuna fortuna requested a review from jyyi1 July 11, 2023 21:07
@fortuna fortuna requested review from a team as code owners July 11, 2023 21:07
@fortuna fortuna changed the title Remove reachability test cleanup: remove reachability test Jul 11, 2023
@codecov
Copy link

codecov bot commented Jul 11, 2023

Codecov Report

Patch coverage: 84% and no project coverage change.

Comparison is base (6e5bfef) 36% compared to head (d344d65) 36%.

Additional details and impacted files
@@          Coverage Diff           @@
##           master   #1663   +/-   ##
======================================
  Coverage      36%     36%           
======================================
  Files          46      45    -1     
  Lines        2815    2756   -59     
  Branches      314     312    -2     
======================================
- Hits         1018    1006   -12     
+ Misses       1797    1750   -47     
Flag Coverage Δ
apple 14% <ø> (+<1%) ⬆️
ios 15% <ø> (+<1%) ⬆️
macos 14% <ø> (+<1%) ⬆️
unittests 36% <84%> (+<1%) ⬆️
www 45% <84%> (+<1%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...eLib/Sources/OutlineTunnelSources/OutlineVpn.swift 0% <ø> (ø)
...PacketTunnelProviderSources/PacketTunnelProvider.m 0% <ø> (ø)
...ib/Tests/OutlineTunnelTest/OutlineTunnelTest.swift 100% <ø> (ø)
src/www/app/app.ts 8% <0%> (+<1%) ⬆️
src/www/app/outline_server_repository/index.ts 73% <ø> (-1%) ⬇️
src/www/app/outline_server_repository/server.ts 34% <ø> (+<1%) ⬆️
src/www/app/platform.ts 100% <ø> (ø)
src/www/model/server.ts 100% <ø> (ø)
...erver_repository/outline_server_repository.spec.ts 100% <100%> (ø)

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@fortuna fortuna changed the title cleanup: remove reachability test refactor: remove reachability test Jul 11, 2023
@daniellacosse daniellacosse changed the title refactor: remove reachability test refactor(cordova/apple): remove reachability test Jul 11, 2023
@daniellacosse daniellacosse changed the title refactor(cordova/apple): remove reachability test refactor(cordova/apple/ios): remove reachability test Jul 11, 2023
@fortuna fortuna merged commit 4938e04 into master Jul 11, 2023
27 of 29 checks passed
@fortuna fortuna deleted the fortuna-reach branch July 11, 2023 22:38
jyyi1 added a commit that referenced this pull request Jul 19, 2024
…2054)

The logic has already been removed in #1663 , this PR cleans up the dead code for Android.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants