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

Mock for Combine #175

Closed
keithcml opened this issue Jan 17, 2022 · 6 comments
Closed

Mock for Combine #175

keithcml opened this issue Jan 17, 2022 · 6 comments

Comments

@keithcml
Copy link

keithcml commented Jan 17, 2022

I have tried to annotate with the following for my AnyPublisher

/// @mockable(combine: publisher = CurrentValueSubject)

But the generator produces PassthroughSubject as

public private(set) var publisherSubject: PassthroughSubject...

I would expect there is a CurrentValueSubject instead.

@ffittschen
Copy link
Contributor

ffittschen commented Feb 22, 2022

We are currently facing the same issue. After investigating it a bit, it only fails to generate the mock as a CurrentValueSubject, if it can't find any default value. E.g. in the following example, mockolo will generate a fooPublisherSubject = PassthroughSubject<FooEnum, Never>() and a barSubject = CurrentValueSubject<Bool, Never>(false)

public enum FooEnum {
    case one, two, three
}

/// @mockable(combine: fooPublisher = CurrentValueSubject; barPublisher = CurrentValueSubject)
public protocol SomeStream: AnyObject {
    var fooPublisher: AnyPublisher<FooEnum, Never> { get }
    var barPublisher: AnyPublisher<Bool, Never> { get }
}

In these lines of code, mockolo will decide to fall back to a PassthroughSubject in case it can't find a default value.

As mockolo is already capable of adding parameters to the init method in the case of assigning an AnyPublisher to a property, we might be able to extend it to add parameters for types of the CurrentValueSubjects as well?

Alternatively, we could probably add a new case to the CombineType to allow overwriting the AnyPublisher as if it were a normal variable, i.e. making mockolo backwards compatible to versions before PR #155. Then we could simply do this:

var someStream = SomeStreamMock()
let someSubject = CurrentValueSubject<FooEnum, Never>(.one)
someStream.fooPublisher = someSubject.eraseToAnyPublisher()

@maxwellE
Copy link
Collaborator

I believe this was fixed in #186 ?

@uhooi
Copy link
Collaborator

uhooi commented Nov 23, 2022

@keithcml Can you confirm that it is fixed?

@kernandreas
Copy link

Just ran in the same situation. Thank you @ffittschen for the investigation. How did you work around in your code base?

Btw. #186 does not fix the root cause. It adds a new flag to disable Subject synthesis for all AnyPublisher properties.

@fummicc1
Copy link
Collaborator

fummicc1 commented May 3, 2023

I try to fix this issue by adding a new metadata to set a default value of CurrentValueSubject in #234.

If you have any advice, please let me know.

@fummicc1
Copy link
Collaborator

After reviewing my idea, I believe we can resolve this issue by using the --disable-combine-default-values option. Therefore, I'll be closing this PR. Please feel free to reopen it whenever you wish to continue discussing this topic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants