Mock data for screenshot test

[mojganii]

This PR introduces mocking data for snapshot testing with the following key assumptions:

• Login button must just proceed to the next screen
• The account view must refresh instantly and display 37 days of remaining time.
• The device name is set to Secure Mole.
• The app is always connected to se-got-wg-101.

---

This change is 

[linear]

IOS-653 Use mock data for screenshots

[buggmagnet]

This doesn't compile for me, how did you manage to run the Screenshot tests ?

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

---

ios/MullvadVPN/AppDelegate.swift line 50 at r1 (raw file):

    private(set) var configuredTransportProvider: ProxyConfigurationTransportProvider!
    private(set) var ipOverrideRepository = IPOverrideRepository()
    private var launchArguments: LaunchArguments?

This doesn't need to be optional, all the values in that struct are already defined at init time.

---

ios/MullvadVPN/AppDelegate.swift line 61 at r1 (raw file):

        launchArguments = ProcessInfo.processInfo.decode(LaunchArguments.self)

        if launchArguments?.isDisabledAnimations ?? false {

With the other suggestions I made, we could change this to

        if let overridenLaunchArguments = try? ProcessInfo.processInfo.decode(LaunchArguments.self) {
            launchArguments = overridenLaunchArguments
        }

        if launchArguments.isDisabledAnimations {
            UIView.setAnimationsEnabled(false)
        }

The first line is a bit longer, but we don't need to unwrap the optional which is slightly nicer IMHO

---

ios/Shared/LaunchArguments.swift line 31 at r1 (raw file):

    // Disable animations to speed up tests.
    public var isDisabledAnimations = false

We can rename this disableAnimations
it makes it easier to read
if launchArguments.disableAnimations == false { ... }

---

ios/Shared/LaunchArguments.swift line 55 at r1 (raw file):

private extension Decodable {
    static func decode(from json: String) -> Self? {

It doesn't make sense to have an encoding API that throws, but a decoding that doesn't from an API design perspective.
Since JSONDecoder APIs are already throws, we should take advantage of that.

Suggestion

public extension ProcessInfo {
    func decode<T: Taggable & Decodable>(_: T.Type) throws -> T {
        guard let environment = environment[T.tag] else {
            throw DecodingError.valueNotFound(
                T.self,
                DecodingError.Context(codingPath: [], debugDescription: "\(T.self) not found in environment")
            )
        }
        return try T.decode(from: environment)
    }
}

extension Encodable {
    public func toJSON(_ encoder: JSONEncoder = JSONEncoder()) throws -> String {
        let data = try encoder.encode(self)
        let result = String(decoding: data, as: UTF8.self)
        return result
    }
}

private extension Decodable {
    static func decode(from json: String) throws -> Self {
        guard let data = json.data(using: .utf8) else {
            throw DecodingError.valueNotFound(
                Self.self,
                DecodingError.Context(codingPath: [], debugDescription: "Could not convert \(json) into UTF-8 Data")
            )
        }

        return try JSONDecoder().decode(self, from: data)
    }
}

The bonus is that it makes the screenshot tests easier to read by removing the optional bindings, and taking advantage of throwing APIs.

    override func setUpWithError() throws {
        // Put setup code here. This method is called before the invocation of
        // each test method in the class.

        // In UI tests it is usually best to stop immediately when a failure occurs.
        continueAfterFailure = false

        let argumentsJsonString = try LaunchArguments(
            target: .screenshots,
            isDisabledAnimations: true
        ).toJSON()
        app.launchEnvironment[LaunchArguments.tag] = argumentsJsonString

    }

[rablador]

Compiles for me, both main app and various test targets.

Reviewed 21 of 21 files at r1, all commit messages.
Reviewable status: all files reviewed, 8 unresolved discussions (waiting on @mojganii)

---

ios/MullvadMockData/MullvadMockData.docc/MullvadMockData.md line 1 at r1 (raw file):

# ``MullvadMockData``

Can we remove this file or will it be re-generated automatically?

---

ios/MullvadVPNScreenshots/MullvadVPNScreenshots.swift line 96 at r1 (raw file):

        if case .phone = UIDevice.current.userInterfaceIdiom {
            app.buttons[AccessibilityIdentifier.selectLocationButton.rawValue].tap()
            // cityCell.buttons[AccessibilityIdentifier.expandButton.rawValue].tap()

Remove if unused.

---

ios/MullvadMockData/MullvadREST/AccountsProxy+Stubs.swift line 27 at r1 (raw file):

    func getAccountData(accountNumber: String) -> any RESTRequestExecutor<Account> {
        RESTRequestExecutorStub<Account>(success: {
            return Account(

Nit: Return is not necessary.

---

ios/MullvadMockData/MullvadREST/DevicesProxy+Stubs.swift line 15 at r1 (raw file):

struct DevicesProxyStub: DeviceHandling {
    var deviceResult: Result<Device, Error>?

todo

[mojganii]

didn't removing build cash work? for me, it's built.

Reviewable status: all files reviewed, 7 unresolved discussions (waiting on @buggmagnet and @rablador)

---

ios/MullvadMockData/MullvadMockData.docc/MullvadMockData.md line 1 at r1 (raw file):

Previously, rablador (Jon Petersson) wrote…

Can we remove this file or will it be re-generated automatically?

Done.

---

ios/MullvadVPN/AppDelegate.swift line 50 at r1 (raw file):

Previously, buggmagnet wrote…

This doesn't need to be optional, all the values in that struct are already defined at init time.

Done

---

ios/MullvadVPN/AppDelegate.swift line 61 at r1 (raw file):

Previously, buggmagnet wrote…

With the other suggestions I made, we could change this to

        if let overridenLaunchArguments = try? ProcessInfo.processInfo.decode(LaunchArguments.self) {
            launchArguments = overridenLaunchArguments
        }

        if launchArguments.isDisabledAnimations {
            UIView.setAnimationsEnabled(false)
        }

The first line is a bit longer, but we don't need to unwrap the optional which is slightly nicer IMHO

Done.

---

ios/MullvadVPNScreenshots/MullvadVPNScreenshots.swift line 96 at r1 (raw file):

Previously, rablador (Jon Petersson) wrote…

Remove if unused.

Done.

---

ios/Shared/LaunchArguments.swift line 31 at r1 (raw file):

Previously, buggmagnet wrote…

We can rename this disableAnimations
it makes it easier to read
if launchArguments.disableAnimations == false { ... }

the new name implies the data type of property.let's keep it isDisabledAnimations

---

ios/Shared/LaunchArguments.swift line 55 at r1 (raw file):

Previously, buggmagnet wrote…

It doesn't make sense to have an encoding API that throws, but a decoding that doesn't from an API design perspective.
Since JSONDecoder APIs are already throws, we should take advantage of that.

Suggestion

public extension ProcessInfo {
    func decode<T: Taggable & Decodable>(_: T.Type) throws -> T {
        guard let environment = environment[T.tag] else {
            throw DecodingError.valueNotFound(
                T.self,
                DecodingError.Context(codingPath: [], debugDescription: "\(T.self) not found in environment")
            )
        }
        return try T.decode(from: environment)
    }
}

extension Encodable {
    public func toJSON(_ encoder: JSONEncoder = JSONEncoder()) throws -> String {
        let data = try encoder.encode(self)
        let result = String(decoding: data, as: UTF8.self)
        return result
    }
}

private extension Decodable {
    static func decode(from json: String) throws -> Self {
        guard let data = json.data(using: .utf8) else {
            throw DecodingError.valueNotFound(
                Self.self,
                DecodingError.Context(codingPath: [], debugDescription: "Could not convert \(json) into UTF-8 Data")
            )
        }

        return try JSONDecoder().decode(self, from: data)
    }
}

The bonus is that it makes the screenshot tests easier to read by removing the optional bindings, and taking advantage of throwing APIs.

    override func setUpWithError() throws {
        // Put setup code here. This method is called before the invocation of
        // each test method in the class.

        // In UI tests it is usually best to stop immediately when a failure occurs.
        continueAfterFailure = false

        let argumentsJsonString = try LaunchArguments(
            target: .screenshots,
            isDisabledAnimations: true
        ).toJSON()
        app.launchEnvironment[LaunchArguments.tag] = argumentsJsonString

    }

Good suggestion!

---

ios/MullvadMockData/MullvadREST/AccountsProxy+Stubs.swift line 27 at r1 (raw file):

Previously, rablador (Jon Petersson) wrote…

Nit: Return is not necessary.

Done

---

ios/MullvadMockData/MullvadREST/DevicesProxy+Stubs.swift line 15 at r1 (raw file):

Previously, rablador (Jon Petersson) wrote…

todo

I couldn't follow what you meant.

[buggmagnet]

Reviewable status: all files reviewed, 8 unresolved discussions (waiting on @mojganii and @rablador)

---

ios/MullvadVPN.xcodeproj/project.pbxproj line 8588 at r1 (raw file):

				MODULE_VERIFIER_SUPPORTED_LANGUAGES = "objective-c objective-c++";
				MODULE_VERIFIER_SUPPORTED_LANGUAGE_STANDARDS = "gnu17 gnu++20";
				PRODUCT_BUNDLE_IDENTIFIER = Mojgan.MullvadMockData;

This should be $(APPLICATION_IDENTIFIER).MullvadMockData
This applies for all the configurations

[buggmagnet]

Reviewable status: all files reviewed, 9 unresolved discussions (waiting on @mojganii and @rablador)

---

ios/MullvadVPN.xcodeproj/project.pbxproj line 8566 at r1 (raw file):

				CLANG_CXX_LANGUAGE_STANDARD = "gnu++20";
				CLANG_WARN_UNGUARDED_AVAILABILITY = YES_AGGRESSIVE;
				CODE_SIGN_STYLE = Automatic;

This should be CODE_SIGN_IDENTITY = "Apple Development";

[rablador]

Reviewed 12 of 12 files at r2, all commit messages.
Reviewable status: all files reviewed, 7 unresolved discussions (waiting on @buggmagnet and @mojganii)

---

ios/MullvadMockData/MullvadREST/DevicesProxy+Stubs.swift line 15 at r1 (raw file):

Previously, mojganii wrote…

I couldn't follow what you meant.

Oops, my bad, I forgot to remove it!

[mojganii]

Reviewable status: 20 of 21 files reviewed, 7 unresolved discussions (waiting on @buggmagnet and @rablador)

---

ios/MullvadVPN.xcodeproj/project.pbxproj line 8566 at r1 (raw file):

Previously, buggmagnet wrote…

This should be CODE_SIGN_IDENTITY = "Apple Development";

Done.

---

ios/MullvadVPN.xcodeproj/project.pbxproj line 8588 at r1 (raw file):

Previously, buggmagnet wrote…

This should be $(APPLICATION_IDENTIFIER).MullvadMockData
This applies for all the configurations

Done.

[rablador]

Reviewed 1 of 1 files at r3, all commit messages.
Reviewable status: all files reviewed, 7 unresolved discussions (waiting on @buggmagnet)

[acb-mv]

Reviewed all commit messages.
Reviewable status: all files reviewed, 7 unresolved discussions (waiting on @buggmagnet)

[buggmagnet]

Reviewed 9 of 21 files at r1, 11 of 12 files at r2, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @mojganii)

---

ios/MullvadVPN.xcodeproj/project.pbxproj line 884 at r3 (raw file):

		F0ACE3262BE4E6C7006D5333 /* MullvadTypes.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 58D223D5294C8E5E0029F5F8 /* MullvadTypes.framework */; platformFilter = ios; };
		F0ACE3282BE4E712006D5333 /* WireGuardKitTypes in Frameworks */ = {isa = PBXBuildFile; productRef = F0ACE3272BE4E712006D5333 /* WireGuardKitTypes */; };
		F0ACE32A2BE4E712006D5333 /* WireGuardKitTypes in Embed Frameworks */ = {isa = PBXBuildFile; productRef = F0ACE3272BE4E712006D5333 /* WireGuardKitTypes */; settings = {ATTRIBUTES = (CodeSignOnCopy, ); }; };

This ATTRIBUTES shouldn't be here, we don't need to embed WireGuardKitTypes into MullvadMockData.

Please go to the Build Phases section of theMullvadMockData target, and disable the check box named CodeSignOnCopy

---

ios/Shared/LaunchArguments.swift line 31 at r1 (raw file):

Previously, mojganii wrote…

the new name implies the data type of property.let's keep it isDisabledAnimations

Could you clarify what you mean ?
Also launchArguments.isDisabledAnimations doesn't read like an english sentence.
If you want to keep theis/are prefix, then we can rename it areAnimationsDisabled so it reads naturally.

---

ios/MullvadMockData/MullvadREST/DevicesProxy+Stubs.swift line 15 at r3 (raw file):

struct DevicesProxyStub: DeviceHandling {
    var deviceResult: Result<Device, Error>?

It doesn't really make sense from an API point of view to have an Optional Result type, since it already has a failure case.
It would look cleaner if we default it to fail with a dummy error, and this would skip all the optionals unwraps down the line.

struct DevicesProxyStubError: Error {}

struct DevicesProxyStub: DeviceHandling {
    var deviceResult: Result<Device, Error> = .failure(DevicesProxyStubError())

before

if let result = deviceResult {
    completion(result)
}```

after
```swift
completion(deviceResult)

[mojganii]

Reviewable status: 15 of 21 files reviewed, 4 unresolved discussions (waiting on @buggmagnet and @rablador)

---

ios/MullvadVPN.xcodeproj/project.pbxproj line 884 at r3 (raw file):

Previously, buggmagnet wrote…

This ATTRIBUTES shouldn't be here, we don't need to embed WireGuardKitTypes into MullvadMockData.

Please go to the Build Phases section of theMullvadMockData target, and disable the check box named CodeSignOnCopy

Done.

---

ios/Shared/LaunchArguments.swift line 31 at r1 (raw file):

Previously, buggmagnet wrote…

Could you clarify what you mean ?
Also launchArguments.isDisabledAnimations doesn't read like an english sentence.
If you want to keep theis/are prefix, then we can rename it areAnimationsDisabled so it reads naturally.

you're right, I've forgotten the name is pullar.I'll go for areAnimationsDisabled.

---

ios/MullvadMockData/MullvadREST/DevicesProxy+Stubs.swift line 15 at r3 (raw file):

Previously, buggmagnet wrote…

It doesn't really make sense from an API point of view to have an Optional Result type, since it already has a failure case.
It would look cleaner if we default it to fail with a dummy error, and this would skip all the optionals unwraps down the line.

struct DevicesProxyStubError: Error {}

struct DevicesProxyStub: DeviceHandling {
    var deviceResult: Result<Device, Error> = .failure(DevicesProxyStubError())

before

if let result = deviceResult {
    completion(result)
}```

after
```swift
completion(deviceResult)

Done.

[buggmagnet]

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

[github-actions]

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