Rewrite screenshot tests to use new syntax

[rablador]

We should update our screenshot tests to use the new syntax we use for UI tests.

This PR also adds replaces the location selection screenshot with a custom list selection screenshot, and includes https://linear.app/mullvad/issue/IOS-684/update-screenshots-to-use-pq-tunnels.

---

This change is 

[linear]

IOS-645 Update location view screenshot automation

[niklasberglund]

Reviewable status: 0 of 17 files reviewed, 4 unresolved discussions (waiting on @rablador)

---

ios/MullvadVPNUITests/Pages/EditCustomListLocationsPage.swift line 61 at r1 (raw file):

        }

        app.navigationBars[navigationBarName].buttons.firstMatch.tap()

This class EditCustomListLocationsPage is now for both add and edit, so the action context is required. Maybe it should be broken out into two page classes? There are a few differences between the edit and add views even though they also have much in common.

---

ios/MullvadVPNUITests/Screenshots/ScreenshotTests.swift line 37 at r1 (raw file):

    }

    func testTakeScreenshotOfSecuredConnection() throws {

Should this screenshot still be captured? My interpretation of https://linear.app/mullvad/issue/IOS-684/update-screenshots-to-use-pq-tunnels is that this screenshot should be replaced by the screenshot with a quantum secured connection. It's nice to still have this around, but the drawback is that it opens up for possible mistakes if all screenshots should be used except this one.

---

ios/MullvadVPNUITests/Screenshots/ScreenshotTests.swift line 112 at r1 (raw file):

        CustomListPage(app)
            .renameCustomList(name: "Low latency locations")

nit: would be nice as a constant since it is reused

---

ios/TestPlans/MullvadVPNScreenshots.xctestplan line 31 at r1 (raw file):

        "SettingsMigrationTests",
        "SettingsTests"
      ],

Make sure to also uncheck "Automatically include new tests" for the ScreenshotTests test plan, otherwise new tests will be included in the plan.

[rablador]

Reviewable status: 0 of 18 files reviewed, 3 unresolved discussions (waiting on @niklasberglund)

---

ios/MullvadVPNUITests/Pages/EditCustomListLocationsPage.swift line 61 at r1 (raw file):

Previously, niklasberglund (Niklas Berglund) wrote…

This class EditCustomListLocationsPage is now for both add and edit, so the action context is required. Maybe it should be broken out into two page classes? There are a few differences between the edit and add views even though they also have much in common.

Right now the difference is basically the navigation title. We don't use inheritance much in the app, but I think a new AddCustomListLocationsPage could simply inherit EditCustomListLocationsPage and override the back tap function.

---

ios/MullvadVPNUITests/Screenshots/ScreenshotTests.swift line 37 at r1 (raw file):

Previously, niklasberglund (Niklas Berglund) wrote…

Should this screenshot still be captured? My interpretation of https://linear.app/mullvad/issue/IOS-684/update-screenshots-to-use-pq-tunnels is that this screenshot should be replaced by the screenshot with a quantum secured connection. It's nice to still have this around, but the drawback is that it opens up for possible mistakes if all screenshots should be used except this one.

Yeah, you're probably right. I'll remove this test.

---

ios/MullvadVPNUITests/Screenshots/ScreenshotTests.swift line 112 at r1 (raw file):

Previously, niklasberglund (Niklas Berglund) wrote…

nit: would be nice as a constant since it is reused

Yep

---

ios/TestPlans/MullvadVPNScreenshots.xctestplan line 31 at r1 (raw file):

Previously, niklasberglund (Niklas Berglund) wrote…

Make sure to also uncheck "Automatically include new tests" for the ScreenshotTests test plan, otherwise new tests will be included in the plan.

Done.

[niklasberglund]

Reviewable status: 0 of 18 files reviewed, 3 unresolved discussions

[niklasberglund]

Reviewable status: 0 of 18 files reviewed, 4 unresolved discussions (waiting on @rablador)

---

ios/MullvadVPNUITests/Screenshots/ScreenshotTests.swift line 33 at r2 (raw file):

        if TunnelControlPage(app).connectionIsSecured {
            TunnelControlPage(app)
                .tapDisconnectButton()

I think it would be easier to follow the code if this reset is done in testTakeScreenshotOfQuantumSecuredConnection teardown.

[rablador]

Reviewable status: 0 of 18 files reviewed, 4 unresolved discussions (waiting on @niklasberglund)

---

ios/MullvadVPNUITests/Screenshots/ScreenshotTests.swift line 33 at r2 (raw file):

Previously, niklasberglund (Niklas Berglund) wrote…

I think it would be easier to follow the code if this reset is done in testTakeScreenshotOfQuantumSecuredConnection teardown.

I agree, but that also requires each test to go back to the home screen when they're done. On startup that's where we begin each time. The real "problem" here is that the tests aren't completely stand-alone, but it felt unnecessarily thorough/slow to uninstall the app between each of them.

[niklasberglund]

Reviewable status: 0 of 18 files reviewed, 4 unresolved discussions (waiting on @rablador)

---

ios/MullvadVPNUITests/Screenshots/ScreenshotTests.swift line 33 at r2 (raw file):

Previously, rablador (Jon Petersson) wrote…

I agree, but that also requires each test to go back to the home screen when they're done. On startup that's where we begin each time. The real "problem" here is that the tests aren't completely stand-alone, but it felt unnecessarily thorough/slow to uninstall the app between each of them.

It looks like testTakeScreenshotOfQuantumSecuredConnection is the only test case connecting to a relay, so this code will only run after testTakeScreenshotOfQuantumSecuredConnection, so it's kind of its teardown but instead it is in the next test cases' setup.

[rablador]

Reviewable status: 0 of 18 files reviewed, 4 unresolved discussions (waiting on @niklasberglund)

---

ios/MullvadVPNUITests/Screenshots/ScreenshotTests.swift line 33 at r2 (raw file):

Previously, niklasberglund (Niklas Berglund) wrote…

It looks like testTakeScreenshotOfQuantumSecuredConnection is the only test case connecting to a relay, so this code will only run after testTakeScreenshotOfQuantumSecuredConnection, so it's kind of its teardown but instead it is in the next test cases' setup.

Discussed offline. Will inherit from LoggedInWithTimeUITestCase to make tests independent of each other (results in a logout if logged in before each test).

[niklasberglund]

Reviewable status: 0 of 18 files reviewed, 4 unresolved discussions

[buggmagnet]

Reviewed 12 of 17 files at r1, 5 of 6 files at r2, all commit messages.
Reviewable status: 17 of 18 files reviewed, 5 unresolved discussions (waiting on @niklasberglund and @rablador)

---

ios/MullvadVPNUITests/Screenshots/ScreenshotTests.swift line 25 at r3 (raw file):

    func testTakeScreenshotOfQuantumSecuredConnection() throws {
        // We can't close banners in the screenshot tests due to how the NotificationController view

Is it because the notification is not accessible ?
Can we make it accessible in that case ? That seems like a silly limitation to have.

[rablador]

Reviewable status: 17 of 18 files reviewed, 5 unresolved discussions (waiting on @buggmagnet and @niklasberglund)

---

ios/MullvadVPNUITests/Screenshots/ScreenshotTests.swift line 25 at r3 (raw file):

Previously, buggmagnet wrote…

Is it because the notification is not accessible ?
Can we make it accessible in that case ? That seems like a silly limitation to have.

It's indeed silly. There's a PR for it here: https://linear.app/mullvad/issue/IOS-704/make-notifications-accessible

[buggmagnet]

Reviewed 1 of 1 files at r3.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @niklasberglund)

[github-actions]

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