Refactor URLSession out of outgoing connection proxy tests

[rablador]

---

This change is 

[linear]

IOS-383 Refactor the URLSession out of `OutgoingConnectionProxy`

• It will help us remove a lot of custom written code for tests

See this comment for the reason why.

It should be a janitor task

[rablador]

Reviewable status: 0 of 6 files reviewed, 1 unresolved discussion

---

ios/MullvadVPNTests/OutgoingConnectionProxyTests.swift line 30 at r1 (raw file):

    }

    func testNoInternetConnection() async throws {

Ok to remove this test? It relies on the previoud MockURLProtocol to work and isn't really compatible with the refactor.

[buggmagnet]

Reviewed 4 of 6 files at r1, 1 of 2 files at r2.
Reviewable status: 5 of 6 files reviewed, 4 unresolved discussions (waiting on @rablador)

---

ios/MullvadVPN/GeneralAPIs/OutgoingConnectionProxy.swift line 72 at r2 (raw file):

            throw REST.Error.network(URLError(.badURL))
        }
        let request = URLRequest(

This is not used anymore since we are using func data(from url: URL) async throws -> (Data, URLResponse) instead.

If that was intentional, please delete this variable.
If it wasn't, please adjust the protocol accordingly.

---

ios/MullvadVPNTests/MockURLSession.swift line 11 at r2 (raw file):

import Foundation

class MockURLSession: URLSessionProtocol {

Let's rename this URLSessionStub
(I never get tired of posting this article 🤠)

---

ios/MullvadVPNTests/OutgoingConnectionProxyTests.swift line 30 at r1 (raw file):

Previously, rablador (Jon Petersson) wrote…

Ok to remove this test? It relies on the previoud MockURLProtocol to work and isn't really compatible with the refactor.

I think that's okay.
On the other hand, it would be nice if we could have a test that validates the retry logic of OutgoingConnectionProxy

---

ios/MullvadVPNTests/OutgoingConnectionProxyTests.swift line 85 at r2 (raw file):

extension OutgoingConnectionProxyTests {
    private enum IPVersion: String {

If ExitIPVersion wasn't declared as a private enum to OutgoingConnectionProxy
we could reuse it here instead of declaring a similar type

I think it's worth making ExitIPVersion internal, for that purpose, and declare it outside of OutgoingConnectionProxy

[rablador]

Reviewable status: 5 of 6 files reviewed, 4 unresolved discussions (waiting on @buggmagnet)

---

ios/MullvadVPN/GeneralAPIs/OutgoingConnectionProxy.swift line 72 at r2 (raw file):

Previously, buggmagnet wrote…

This is not used anymore since we are using func data(from url: URL) async throws -> (Data, URLResponse) instead.

If that was intentional, please delete this variable.
If it wasn't, please adjust the protocol accordingly.

Forgot to remove, but on second thought I think should just keep it and adjust the protocol. I don't believe the custom timeout matters much here, but is is more consistent.

---

ios/MullvadVPNTests/MockURLSession.swift line 11 at r2 (raw file):

Previously, buggmagnet wrote…

Let's rename this URLSessionStub
(I never get tired of posting this article 🤠)

Done.

---

ios/MullvadVPNTests/OutgoingConnectionProxyTests.swift line 85 at r2 (raw file):

Previously, buggmagnet wrote…

If ExitIPVersion wasn't declared as a private enum to OutgoingConnectionProxy
we could reuse it here instead of declaring a similar type

I think it's worth making ExitIPVersion internal, for that purpose, and declare it outside of OutgoingConnectionProxy

Done.

[buggmagnet]

Nice job !

Reviewed 5 of 5 files at r3, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @rablador)

---

ios/MullvadVPN/GeneralAPIs/OutgoingConnectionProxy.swift line 72 at r2 (raw file):

Previously, rablador (Jon Petersson) wrote…

Forgot to remove, but on second thought I think should just keep it and adjust the protocol. I don't believe the custom timeout matters much here, but is is more consistent.

IIRC the default timeout for URLRequest is 60 seconds, so slightly different.
But I agree with you on the consistent values 👍