Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Potential Breaking Change: Removing TestStore.scope #1148

stephencelis started this conversation in General
Discussion options

While TestStore.scope was an interesting experiment, and we utilized it in our Tic-Tac-Toe demo to test view state, there are a couple awkward things it introduces that make us think it should be on the chopping block:

  1. The feature makes TestStore a pretty intense type: it has five generics 😱

    public final class TestStore<State, LocalState, Action, LocalAction, Environment> {

    In general it's a lot to take in, and simplifying things would make the type more approachable.

  2. Scoped test stores have an awkward matrix of what level you're at during an assertion:

    Assertion Action Type State Type
    send Local Local
    receive Global Local

    Received actions come from effects and not the view, so it doesn't make sense to receive view actions...but this mismatch is pretty trippy.

A cursory search on GitHub doesn't find much public use, but we were wondering if anyone following the repo takes advantage of this feature privately, and how heavily invested in it they may be.

There are a few alternate ways we find that folks test view state and actions:

  • Unit test them manually as data:

    XCTAssertEqual(
      ViewState(State(...)), // Test view state init'd with `State`
      ViewState(...)         // against view state init'd manually
    )
    
    XCTAssertEqual(
      ViewAction(.action),   // Test view action init'd with `Action`
      ViewAction.action      // against view action init'd manually
    )

    This is simple, but completely separated from reducer logic that produces the state in the first place, for better and/or worse.

  • Unit test them manually in an otherwise unscoped TestStore:

    // Test view action before sending it to the store:
    store.send(ViewAction(.action)) { $0.state = updatedState }
    
    // Test view state after store state has been updated:
    XCTAssertEqual(ViewState(store.state), ViewState(...))

    This gives users the ability to get view state/action coverage in a normal TestStore test.

  • Test using a view store, instead:

    let viewStore = ViewStore(
      store.scope(state: ViewState.init, action: ViewAction.init)
    )
    viewStore.send(.action)
    XCTAssertEqual(viewStore.state, ViewState(...))

    This focuses on the "view" aspect of this data, which means you lose test store affordances like testing exhaustivity and effects, but that might be how you want to focus on this aspect of testing.

Given the awkwardness of TestStore.scope and the availability of these less awkward alternatives, we think it's OK to deprecate, but would like to hear from you! We'll be rolling out an update soon with deprecation warnings in order to solicit more feedback (linking here).

You must be logged in to vote

Replies: 4 comments · 4 replies

Comment options

Any scoped TestStore can be made explicitly as TestStore and could be preferable as such. I have not made use of TestStore.scoped. So +1 from me.

You must be logged in to vote
0 replies
Comment options

I wonder what is the latest status on this, now that we seem to have new code for TestStore.scope, and have one less generic type parameters?

You must be logged in to vote
1 reply
@stephencelis
Comment options

stephencelis Sep 1, 2022
Maintainer Author

@notcome We were able to retain scoping in the protocol branch, so we decided to leave it for now. We'd still love to hear from you if you do use scoping, and if you have found it to be valuable or not.

Comment options

Late to the party here. We have >550 tests using using observe:/send: 😭

I will attempt to migrate these using a ViewStore instead 🤞. If anyone has any tips for doing this, I would love to hear them!

You must be logged in to vote
3 replies
@stephencelis
Comment options

stephencelis Jul 20, 2024
Maintainer Author

Sorry to hear that! If you'd like to share your case study we'd be interested to know about it, though I hope you understand why we came to the above conclusion. If you share how you used test store scoping maybe it'd spark some alternate ideas 😄

@davidcairns
Comment options

@stephencelis I’ve been slowly updating our (quite extensive!) app to use newer and newer versions of TCA. We’re currently up to 0.55.1, and I found the deprecation in 0.56.0.

When writing tests, our best practices have always been to scope to the \.viewState, which we like because it’s like an end to end test but without any of the downsides — clearly communicates how “doing stuff” produces visible effects to the user.

I’ve watched a lot (though not all!) of your videos in the past two years. I don’t recall seeing you guys write tests for the state —> viewState transform. Do you have any useful examples there? Thanks.

@lukeredpath
Comment options

@davidcairns the good news is by the time you catch up to the latest version of TCA, you probably won't need your ViewState structs for the most part, as modern observation makes it redundant.

Comment options

I’ve come up with a good pattern for this that will allow us to update a lot of our tests without TOO MUCH thrash.

Existing test:

        let initialState = …
        let store = TestStore(
            initialState: initialState,
            reducer: MyReducer(),
            observe: \.viewState
        )

        store.send(.displayAllTapped) { state in
            state.headerTitle = "All"
            state.nextButtonAlpha = 0
            state.previousButtonAlpha = 0
        }
        …

Rewrite:

        let initialState = …
        let store = TestStore(
            initialState: initialState
        ) { MyReducer() }

        var viewState = initialState.viewState

        store.send(.displayAllTapped) { state in
            state.viewingMode = .all

            viewState.headerTitle = "All views"
            viewState.nextButtonAlpha = 0
            viewState.previousButtonAlpha = 0
            XCTAssertEqual(state.viewState, viewState)
        }
        …

I found it WAY too heavy to add a XCTAssertEqual(state.viewState, ViewState(…)) in every one of these. Some of our view states are quite large, for complex screens.

You must be logged in to vote
0 replies
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
5 participants
Morty Proxy This is a proxified and sanitized view of the page, visit original site.