Nimble icon indicating copy to clipboard operation
Nimble copied to clipboard

Using satisfyAllOf with toEventually causes expression to be evaluated multiple times

Open iby opened this issue 7 years ago • 5 comments

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.

iby avatar Jun 02 '18 22:06 iby

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 avatar Sep 17 '19 17:09 ikesyo

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

ikesyo avatar Sep 17 '19 18:09 ikesyo

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

ikesyo avatar Sep 17 '19 19:09 ikesyo

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?

iby avatar Sep 18 '19 06:09 iby

This is similar to #360.

ikesyo avatar Mar 30 '20 19:03 ikesyo

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

younata avatar Oct 31 '22 21:10 younata