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

Using satisfyAllOf with toEventually causes expression to be evaluated multiple times #529

Closed
iby opened this issue Jun 2, 2018 · 6 comments · Fixed by #1045
Closed

Comments

@iby
Copy link

iby commented Jun 2, 2018

Doing something like below results in expression being evaluated multiple times with "foo" being printed many times. With satisfyAllOf removed everything works as expected.

let testNotification = Notification(name: .init("Foo"), object: nil)
expect(expression: {
    print("foo")
    return DispatchQueue.global().async {
        Thread.sleep(forTimeInterval: 0.5)
        NotificationCenter.default.post(testNotification)
    }
}).toEventually(satisfyAllOf(postNotifications(equal([testNotification]))))

This must be happening because postNotifications has explicit logic for evaluating expression only once, while satisfyAllOf doesn't.

@ikesyo
Copy link
Member

ikesyo commented Sep 17, 2019

Async expectations (toEventually in this case) uses uncached expression evaluation to get a fresh result each time. And then, satisfyAllOf will

  • pass the given expression to the given matchers
  • evaluate the given expression by itself to get the actual value and construct an assertion message

So this is an edge case and I'm not sure if this is an expected behavior or a bug.

@ikesyo ikesyo changed the title Using satisfyAllOf with postNotifications causes expression to be evaluated multiple times Using satisfyAllOf with toEventually causes expression to be evaluated multiple times Sep 17, 2019
@ikesyo
Copy link
Member

ikesyo commented Sep 17, 2019

More precisely, having a side effect within a expression closure for async expectations is not an intended usage.

@ikesyo
Copy link
Member

ikesyo commented Sep 17, 2019

Maybe having a caching expression within satisfyAllOf matcher will mitigate the situation.

@iby
Copy link
Author

iby commented Sep 18, 2019

If memory is not letting me down this happened while attempting to create custom ReactiveSwift assertions using the same approach used by postNotifications with some help from satisfyAllOf to avoid boilerplate. So, the above was the narrowed down use case. It seemed legit, hence the issue. Can I provide any details that would be of use?

@ikesyo
Copy link
Member

ikesyo commented Mar 30, 2020

This is similar to #360.

@younata younata added this to the v11.0.0 milestone Apr 19, 2022
@younata younata modified the milestones: v11.0.0, v12.0.0 Oct 31, 2022
@younata
Copy link
Member

younata commented Oct 31, 2022

Bumping this to the next (major?) release in favor of getting async/await support out the door sooner.

younata added a commit that referenced this issue Apr 4, 2023
younata added a commit that referenced this issue Apr 4, 2023
younata added a commit that referenced this issue Apr 4, 2023
…e. (#1045)

* satisfyAllOf with toEventually should only evaluate the expression once each poll

Fixes #529

* Now apply that same behavior to satisfyAnyOf
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants