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

TestStore expects shared state to be nil when state was set to nil in an independent test showing the same destination #3468

Open
2 of 3 tasks
dominikmayer opened this issue Oct 25, 2024 · 5 comments
Labels
bug Something isn't working due to a bug in the library.

Comments

@dominikmayer
Copy link

dominikmayer commented Oct 25, 2024

Description

I have the following struct, which is shared in memory:

struct Journal: Equatable {}

extension PersistenceReaderKey where Self == PersistenceKeyDefault<InMemoryKey<Journal?>> {
    static var journal: Self {
        return PersistenceKeyDefault(.inMemory("journal"), nil)
    }
}

I then have a feature that can show two other features, one of which contains the shared journal:

@Reducer
struct EntryFeature {
    
    @ObservableState
    struct State: Equatable {
        var url: URL
        @SharedReader(.journal) var journal
    }
}

@Reducer
struct DayFeature {
    
    @ObservableState
    struct State: Equatable {
        var entries: [EntryFeature.State]
    }
}

And the feature that shows them:

@Reducer
struct CalendarFeature {
    
    @Reducer(state: .equatable)
    enum Destination {
        case day(DayFeature)
        case entry(EntryFeature)
    }
    
    @ObservableState
    struct State: Equatable {

        @Presents
        var destination: Destination.State?
        @Shared(.journal) var journal
    }
    
    enum Action: Equatable {
        case destination(PresentationAction<Destination.Action>)
        case loadEntry(URL)
    }
    
    var body: some ReducerOf<Self> {
        Reduce { state, action in
            switch action {
                
            case .loadEntry(let url):
                let updatedEntryState = EntryFeature.State(url: url)
                state.destination = .entry(updatedEntryState)
                return .none
                
            case .destination:
                return .none
            }
        }
        .ifLet(\.$destination, action: \.destination)
    }
}

extension CalendarFeature.Destination.Action: Equatable {}

Now when trying to test this setup then the test passes when run on its own but fails when run together with another, independent test, that

a) shows the same destination as will be set in the brittle test and
b) sets the shared journal to nil.

This happens even when the test are run in .serialized mode.

@Suite(.serialized)
@MainActor
struct Test_CalendarFeature {

    let journal = Journal()
        
    @Test
    func settingSharedVariableNil() async {
        // We never use this store but if we remove it, the issue doesn't appear
        let _ = TestStore(
            initialState: .init(
                // This has to be the same entry that will be set by the other test. You can try to change it to .entryFeature2 and everything passes
                destination: .day(.init(entries: [.entryFeature1]))
            )
        ) {
            CalendarFeature()
        }

        @Shared(.journal) var sharedJournal
        $sharedJournal.withLock {
            $0 = nil
        }
        // We can but don't even need to send an action here
    }
    
    // This test passes when it's run alone but fails when it's run as part of the suite together with the other test, even when both tests are serialized
    @Test
    func brittleTest() async {
        @Shared(.journal) var sharedJournal
        $sharedJournal.withLock {
            $0 = journal
        }

        let store = TestStore(
            initialState: .init()
        ) {
            CalendarFeature()
        }
        
        await store.send(.loadEntry(.entry1)) {
            $0.destination = .entry(.entryFeature1)
        }
    }
}

// MARK: - Test Items
extension URL {
    static let entry1 = URL(fileURLWithPath: "/some/path/")
    static let entry2 = URL(fileURLWithPath: "/some/other/path/")
}

extension EntryFeature.State {
    static let entryFeature1 = EntryFeature.State(
        url: .entry1
    )
    static let entryFeature2 = EntryFeature.State(
        url: .entry2
    )
}

Checklist

  • I have determined whether this bug is also reproducible in a vanilla SwiftUI project.
  • If possible, I've reproduced the issue using the main branch of this package.
  • This issue hasn't been addressed in an existing GitHub issue or discussion.

Expected behavior

The test yields the same result whether run independently or as part of a suite.

Actual behavior

Test passes when run independently but not when run as part of a test suite.

Reproducing project

https://github.com/dominikmayer/TCA-Shared-Test-Issue/

The Composable Architecture version information

1.15.2

Destination operating system

macOS 15.0

Xcode version information

Version 16.0 (16A242d)

Swift Compiler version information

swift-driver version: 1.115 Apple Swift version 6.0 (swiftlang-6.0.0.9.10 clang-1600.0.26.2)
Target: arm64-apple-macosx15.0
@dominikmayer dominikmayer added the bug Something isn't working due to a bug in the library. label Oct 25, 2024
@OguzYuuksel
Copy link

OguzYuuksel commented Oct 30, 2024

Hello @dominikmayer,
My guess why it is working as standalone test because you reach entryFeature1 first time.

Why it is not working as test suit because you store mock in memory (static let) which has reference Shared property and then does not clear it out after first test completed, it causes some leak.

What I would suggest you not to store mocks. You will see problem will be solved if you change your mock as below

  static var entryFeature1: Self { 
      .init(url: .entry1)
   }

@dominikmayer
Copy link
Author

Hi @OguzYuuksel. Thanks for looking into that. Yeah, the first test initiating the shared journal with the default value is probably what's happening here.

I just think this is a bug. The TestStore should accommodate for different shared state in different tests. Otherwise it is very difficult to test. I had this issue come up in a much larger feature with a much larger test suite. It took me four hours to strip everything down to what I shared here to actually understand, where the problem is coming from.

Regarding your suggestion. Isn't it almost the same as my definition of entryFeature1? Except for storing it as a variable rather than a constant. This is what I have:

extension EntryFeature.State {
    static let entryFeature1 = EntryFeature.State(
        url: .entry1
    )
}

I changed to a var but the issue persists.

@OguzYuuksel
Copy link

OguzYuuksel commented Oct 30, 2024

@dominikmayer
It is computed property in my suggestion so you reinitialize state on each get and also the shared module so no reference leak.

@dominikmayer
Copy link
Author

Ah, right. Missed that. This does solve the issue. Thank you.

@dominikmayer
Copy link
Author

I've been thinking about this more. So I agree that it was probably not smart to use a static constant as it will not be reset during tests.

But still: The tests are running serially not in parallel. And the shared variable was set in a test that should be running independent of the other test. So even if the other test initiates the shared variable, shouldn't the explicit setting in the brittle test change that?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working due to a bug in the library.
Projects
None yet
Development

No branches or pull requests

2 participants