Add iOS test making sure app functioning when API is down

[niklasberglund]

Implemented test verifying that app is still functioning and can connect to a relay when the API is down. The simulation of API being down is not optimal. We have discussed it and decided to go for this way of simulating it for now. Might be improved in the future.

To test the changes run the test testAppStillFunctioningWhenAPIDown under ConnectivityTests. To simulate the the API being down all access methods are disabled and then a dummy access method is created to make API unreachable.

Duo to the extra setup and teardown steps the test function violates function_body_length(function body shouldn't be over 50 lines of code) so it is disabled for this test function.

---

This change is 

[linear]

IOS-463 Write test that makes sure connecting works without the API

[pinkisemils]

Reviewed 8 of 26 files at r1, all commit messages.
Reviewable status: 8 of 26 files reviewed, 1 unresolved discussion (waiting on @niklasberglund)

---

ios/MullvadVPN/Containers/Navigation/CustomNavigationController.swift line 29 at r1 (raw file):

    override func viewDidLayoutSubviews() {
        super.viewDidLayoutSubviews()
        navigationItem.backBarButtonItem?.accessibilityIdentifier = "asdasd"

Is this a good accessibility identifier name?

[niklasberglund]

Reviewable status: 7 of 26 files reviewed, 1 unresolved discussion (waiting on @pinkisemils)

---

ios/MullvadVPN/Containers/Navigation/CustomNavigationController.swift line 29 at r1 (raw file):

Previously, pinkisemils (Emīls Piņķis) wrote…

Is this a good accessibility identifier name?

Was probably testing something out and forgot to remove 😄 Have removed the accessibility identifier, it was unused

[rablador]

Reviewed 23 of 26 files at r1, 3 of 3 files at r2, all commit messages.
Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @niklasberglund and @pinkisemils)

---

ios/MullvadVPN/Coordinators/Settings/APIAccess/Cells/SwitchCellContentView.swift line 80 at r2 (raw file):

        switchContainer.transform = CGAffineTransform(scaleX: 0.85, y: 0.85)

        if let accessibilityIdentifier = actualConfiguration.accessibilityIdentifier {

accessibilityIdentifier can be optional, so no need to check for existence.

---

ios/MullvadVPN.xcodeproj/project.pbxproj line 3638 at r2 (raw file):

				85C7A2E82B89024B00035D5A /* SettingsTests.swift */,
				85D039972BA4711800940E7F /* SettingsMigrationTests.swift */,
				8588F5C42BCD7FB000374D0B /* XCUIElement+Extensions.swift */,

Seems to be a duplicate.

---

ios/MullvadVPNUITests/ConnectivityTests.swift line 46 at r2 (raw file):

    // swiftlint:disable function_body_length
    /// Test that the app is functioning when API is down. To simulate API being down we create a dummy access

Nit: "... method"?

---

ios/MullvadVPNUITests/ConnectivityTests.swift line 127 at r2 (raw file):

    /// Toggle enabled switch for all existing access methods. It is a precondition that the app is currently showing API access view.
    private func toggleAllAccessMethodsEnabledSwitch() {

Nit: The name is a little ambiguous. Reading the code and comment it's obvious what it does, but the name could suggest that you want to toggle all switches to be enabled.

---

ios/MullvadVPNUITests/Test base classes/BaseUITestCase.swift line 56 at r2 (raw file):

    }

    func allowLocalNetworkAccess() {

This and the function below do almost the same thing. Are both necessary?

[niklasberglund]

Reviewable status: 21 of 26 files reviewed, 6 unresolved discussions (waiting on @pinkisemils and @rablador)

---

ios/MullvadVPN.xcodeproj/project.pbxproj line 3638 at r2 (raw file):

Previously, rablador (Jon Petersson) wrote…

Seems to be a duplicate.

Ah yes. Removed duplicate reference.

---

ios/MullvadVPNUITests/ConnectivityTests.swift line 46 at r2 (raw file):

Previously, rablador (Jon Petersson) wrote…

Nit: "... method"?

Yes 😄 Added " method"

---

ios/MullvadVPNUITests/ConnectivityTests.swift line 127 at r2 (raw file):

Previously, rablador (Jon Petersson) wrote…

Nit: The name is a little ambiguous. Reading the code and comment it's obvious what it does, but the name could suggest that you want to toggle all switches to be enabled.

Agree, "enabled" is very close to "enable". It is the switch for toggling the "enable method" though. Would adding "method" in the name and documentation make it more clear? toggleAllAccessmethodsEnableAccessMethodSwitch() and documentation saying Toggle enable method switch for all existing access methods. It is a precondition that the app is currently showing API access view.

---

ios/MullvadVPNUITests/Test base classes/BaseUITestCase.swift line 56 at r2 (raw file):

Previously, rablador (Jon Petersson) wrote…

This and the function below do almost the same thing. Are both necessary?

Hmmmm good point. Actually its not used, only the conditional allowLocalNetworkAccessIfAsked() is used by test cases. Same goes for allowAddVPNConfigurations and allowAddVPNConfigurationsIfAsked.

The idea was that it's good to strictly make sure it is shown when we know it should be. But we never know.

[rablador]

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

---

ios/MullvadVPNUITests/ConnectivityTests.swift line 127 at r2 (raw file):

Previously, niklasberglund (Niklas Berglund) wrote…

Agree, "enabled" is very close to "enable". It is the switch for toggling the "enable method" though. Would adding "method" in the name and documentation make it more clear? toggleAllAccessmethodsEnableAccessMethodSwitch() and documentation saying Toggle enable method switch for all existing access methods. It is a precondition that the app is currently showing API access view.

I think perhaps adding rewording to [...]Switches would make it clearer, since we're toggling more than one.

[niklasberglund]

Reviewable status: 24 of 26 files reviewed, 2 unresolved discussions (waiting on @pinkisemils and @rablador)

---

ios/MullvadVPN/Coordinators/Settings/APIAccess/Cells/SwitchCellContentView.swift line 80 at r2 (raw file):

Previously, rablador (Jon Petersson) wrote…

accessibilityIdentifier can be optional, so no need to check for existence.

Ah yes. Fixed

---

ios/MullvadVPNUITests/ConnectivityTests.swift line 127 at r2 (raw file):

Previously, rablador (Jon Petersson) wrote…

I think perhaps adding rewording to [...]Switches would make it clearer, since we're toggling more than one.

Changed to switches plural

[rablador]

Reviewed 1 of 2 files at r4, 4 of 4 files at r5, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @pinkisemils)

[rablador]

Reviewed 1 of 1 files at r6, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @pinkisemils)

[pinkisemils]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @niklasberglund)

---

ios/MullvadVPN.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved line 0 at r6 (raw file):
This file should be reverted.

[niklasberglund]

Reviewable status: 25 of 26 files reviewed, 1 unresolved discussion (waiting on @pinkisemils and @rablador)

---

ios/MullvadVPN.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved line at r6 (raw file):

Previously, pinkisemils (Emīls Piņķis) wrote…

This file should be reverted.

Reverted now

[pinkisemils]

Reviewed 1 of 1 files at r7, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved

[pinkisemils]

Reviewed 4 of 4 files at r8, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved

[rablador]

Reviewed 1 of 1 files at r7, 4 of 4 files at r8, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @niklasberglund)

---

ios/MullvadVPNUITests/ConnectivityTests.swift line 61 at r8 (raw file):

            .tapApplyButton()

        // Select the first country, its first city and its first relay

This is not reliable since ownership and relay list can change in the future.

[niklasberglund]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @rablador)

---

ios/MullvadVPNUITests/ConnectivityTests.swift line 61 at r8 (raw file):

Previously, rablador (Jon Petersson) wrote…

This is not reliable since ownership and relay list can change in the future.

What type of changes would break the test? The test connects to any(the first) Mullvad-owned relay the applies filter for rented only so that the connected relay is not part of the filter.

[rablador]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @niklasberglund)

---

ios/MullvadVPNUITests/ConnectivityTests.swift line 61 at r8 (raw file):

Previously, niklasberglund (Niklas Berglund) wrote…

What type of changes would break the test? The test connects to any(the first) Mullvad-owned relay the applies filter for rented only so that the connected relay is not part of the filter.

Ah, my bad, I missed that you filter owned relays first. Nevermind!

[rablador]

Reviewable status: complete! all files reviewed, all discussions resolved

[pinkisemils]

Reviewed 2 of 2 files at r9, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved

[github-actions]

🚨 End to end tests failed. Please check the failed workflow run.