Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

Closed
2 of 3 tasks
dominikmayer opened this issue Oct 25, 2024 · 6 comments

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?

@mbrandonw
Copy link
Member

Hi @dominikmayer, the failure you are experiencing is expected as @OguzYuuksel described. And it doesn't matter if the tests are being run serially/in parallel because there is only one single process that runs the full test suite. So the first test runs, touches entryFeature1, which initializes the shared state, and that data sticks around for the next test.

This kind of failure would happen without TCA or the sharing library since Swift Testing runs all tests in the a single process. This is in contrast to XCTest, which spins up a new process for each test. In that kind of set up you would not have experienced this failure.

Since this isn't an issue with the library I am going to convert it to a discussion. Please feel free to continue the conversation over there!

@mbrandonw mbrandonw removed the bug Something isn't working due to a bug in the library. label Jan 11, 2025
@pointfreeco pointfreeco locked and limited conversation to collaborators Jan 11, 2025
@mbrandonw mbrandonw converted this issue into discussion #3554 Jan 11, 2025

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants