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

Refactored SwiftUI tests to use async/await APIs #171

Merged
merged 40 commits into from
Feb 23, 2022

Conversation

Tyler-Keith-Thompson
Copy link
Collaborator

@Tyler-Keith-Thompson Tyler-Keith-Thompson commented Feb 4, 2022

Linked Issue:

Checklist:


If Applicable:

  • Did you test when the first item is skipped?
  • Did you test when the last item is skipped?
  • Did you test when middle items are skipped?
  • Did you test when incorrect data is passed forward?
  • Did you test proceeding backwards?

…ing, but async await looks like it means we can get rid of that stupid removeQueuedExpectations thing - TT
@Tyler-Keith-Thompson Tyler-Keith-Thompson self-assigned this Feb 4, 2022
@Tyler-Keith-Thompson
Copy link
Collaborator Author

Tyler-Keith-Thompson commented Feb 4, 2022

I recognize this refactor can be hard to grok. Here's your key takeaway. I am turning this horrible pyramid of doom:

func testRemovedAfterProceeding_OnMiddleItemInAWorkflow() throws {
    struct FR1: View, FlowRepresentable, Inspectable {
        var _workflowPointer: AnyFlowRepresentable?
        var body: some View { Text("FR1 type") }
    }
    struct FR2: View, FlowRepresentable, Inspectable {
        var _workflowPointer: AnyFlowRepresentable?
        var body: some View { Text("FR2 type") }
    }
    struct FR3: View, FlowRepresentable, Inspectable {
        var _workflowPointer: AnyFlowRepresentable?
        var body: some View { Text("FR3 type") }
    }
    struct FR4: View, FlowRepresentable, Inspectable {
        var _workflowPointer: AnyFlowRepresentable?
        var body: some View { Text("FR4 type") }
    }
    let expectViewLoaded = ViewHosting.loadView(
        WorkflowLauncher(isLaunched: .constant(true)) {
            thenProceed(with: FR1.self) {
                thenProceed(with: FR2.self) {
                    thenProceed(with: FR3.self) {
                        thenProceed(with: FR4.self)
                    }
                }
                .persistence(.removedAfterProceeding)
            }
        }
    ).inspection.inspect { fr1 in
        XCTAssertNoThrow(try fr1.find(FR1.self).actualView().proceedInWorkflow())
        try fr1.actualView().inspectWrapped { fr2 in
            XCTAssertNoThrow(try fr2.find(FR2.self).actualView().proceedInWorkflow())
            try fr2.actualView().inspectWrapped { fr3 in
                XCTAssertNoThrow(try fr3.find(FR3.self).actualView().backUpInWorkflow())
                try fr1.actualView().inspect { fr1 in
                    XCTAssertNoThrow(try fr1.find(FR1.self).actualView().proceedInWorkflow())
                    try fr1.actualView().inspectWrapped { fr2 in
                        XCTAssertNoThrow(try fr2.find(FR2.self).actualView().proceedInWorkflow())
                        try fr2.actualView().inspectWrapped { fr3 in
                            XCTAssertNoThrow(try fr3.find(FR3.self).actualView().proceedInWorkflow())
                            try fr3.actualView().inspectWrapped { fr4 in
                                XCTAssertNoThrow(try fr4.find(FR4.self).actualView().proceedInWorkflow())
                            }
                        }
                    }
                }
            }
        }
    }

    wait(for: [expectViewLoaded], timeout: TestConstant.timeout)
}

into this much less horrible serial set of tests

func testRemovedAfterProceeding_OnMiddleItemInAWorkflow() async throws {
    struct FR1: View, FlowRepresentable, Inspectable {
        var _workflowPointer: AnyFlowRepresentable?
        var body: some View { Text("FR1 type") }
    }
    struct FR2: View, FlowRepresentable, Inspectable {
        var _workflowPointer: AnyFlowRepresentable?
        var body: some View { Text("FR2 type") }
    }
    struct FR3: View, FlowRepresentable, Inspectable {
        var _workflowPointer: AnyFlowRepresentable?
        var body: some View { Text("FR3 type") }
    }
    struct FR4: View, FlowRepresentable, Inspectable {
        var _workflowPointer: AnyFlowRepresentable?
        var body: some View { Text("FR4 type") }
    }
    let launcher = try await MainActor.run {
        WorkflowLauncher(isLaunched: .constant(true)) {
            thenProceed(with: FR1.self) {
                thenProceed(with: FR2.self) {
                    thenProceed(with: FR3.self) {
                        thenProceed(with: FR4.self)
                    }
                }
                .persistence(.removedAfterProceeding)
            }
        }
    }.hostAndInspect(with: \.inspection)

    try await launcher.find(FR1.self).proceedInWorkflow()
    try await launcher.find(FR2.self).proceedInWorkflow()
    try await launcher.find(FR3.self).backUpInWorkflow()

    try await launcher.find(FR1.self).proceedInWorkflow()
    try await launcher.find(FR2.self).proceedInWorkflow()
    try await launcher.find(FR3.self).proceedInWorkflow()
    try await launcher.find(FR4.self).proceedInWorkflow()
}

…ways to go and some issues with navs and modals - TT
…ng one that never could have worked correctly - TT
… still some nav links to go but the races are solved - TT
@Tyler-Keith-Thompson
Copy link
Collaborator Author

WARNING DANGER WILL ROBINSON: So this works, and the refactor is definitely nicer to read and work with. However, our pipeline is failing on timeouts. I've run this on really poor machines and on really performant machines. I do not believe it's timing out because there's a case. I believe it's timing out because the pipeline is not on Monterrey. So despite the new test niceties, we cannot merge this without our pipeline failing :(

@Tyler-Keith-Thompson Tyler-Keith-Thompson marked this pull request as ready for review February 8, 2022 18:53
@Tyler-Keith-Thompson Tyler-Keith-Thompson requested a review from a team as a code owner February 8, 2022 18:53
Copy link
Contributor

@nickkaczmarek nickkaczmarek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Start of the review.

morganzellers
morganzellers previously approved these changes Feb 9, 2022
Co-authored-by: Matt Freiburg <Matt.Freiburg@wwt.com>
@codecov-commenter
Copy link

Codecov Report

Merging #171 (c39a009) into main (edaedd0) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #171   +/-   ##
=======================================
  Coverage   91.28%   91.28%           
=======================================
  Files          92       92           
  Lines        2375     2375           
=======================================
  Hits         2168     2168           
  Misses        207      207           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 24dfab7...c39a009. Read the comment docs.

@Tyler-Keith-Thompson Tyler-Keith-Thompson added the enhancement New feature or request label Feb 23, 2022
@MattFreiburgAsynchrony MattFreiburgAsynchrony merged commit ee81bd4 into main Feb 23, 2022
@MattFreiburgAsynchrony MattFreiburgAsynchrony deleted the async-swiftui-tests branch February 23, 2022 17:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants