lunarmodules / luassert

Assertion library for Lua
MIT License
202 stars 76 forks source link

Feature request: support for "eventually" (willing to PR) #158

Closed BooleanCat closed 5 years ago

BooleanCat commented 5 years ago

Hi there!

I was playing around with busted for a project I'm working on. I have a use-case for something like the following:

assert.eventually(f).returns(x)

That is, a way to provide a function that is repeatedly called at some cadence, until the provided function returns an expected result. Specifically in my case, I wanted to assert that after starting a subprocess I expect that process to eventually exist on my system. Then when I issue a kill I expect that process to die eventually.

Other similar test frameworks, such as ginkgo in the go ecosystem, have support for something like Eventually(f).Should(Equal(x)). This is useful for firing off async instructions and waiting for a thing to happen.

I would suggest, as with ginkgo, that the timeout and check interval of eventually be configurable and default to a timeout of 1 second and a interval of 50 milliseconds (where the platform supports it). This could be implemented as optional arguments to the eventually matcher or as additional words for a more natural read. For example:

assert.eventually(f, 10, 0.1).returns(x)
assert.eventually(f).within(10).every(0.01).returns(x)

I'm not sure how controversial a suggestion this is, so here's more controversy. Maybe we could provide a more human readable experience for the optional args:

assert.eventually(f).within("10s").every("100ms").returns(x)

Bonus (?)

If the idea above is well received, I'm happy to also provide a PR for something like:

assert.consistently(f).returns(x)

The usage here would be similar to the above.

I'm happy to PR all of the above - let me know what you think!

Thanks!

Tieske commented 5 years ago

I don't think this can be implemented actually without the async support. This would take some serious designing imo.

For example, the every setting (check interval) requires a timer to validate. This timer would be depending on the async implementation. Which Busted explicitly tries to stay away from.

BooleanCat commented 5 years ago

Are you talking about something like a platform independant nanosleep? I'm quite new to the lua ecosystem so I'm unfamilar with what exists and what doesn't.

Is there a set of systems that busted aims to support or is it just any system that Lua runs on?

Tieske commented 5 years ago

Lua is single threaded, so there is no "async". But... Lua is usually part of a bigger application that might add threading, or a scheduler and timers. Consider OpenResty, Luvit, or Copas as such environments.

Busted 1.x had code specifically for some of those. In Busted 2.x it hasn't been added yet, but the idea was to have no implementation specific code in Busted. See https://github.com/Olivine-Labs/busted/pull/439

BooleanCat commented 5 years ago

Thanks for the info, I've had a read through the async issue.

I don't think what I'm suggesting requires busted async support. I imagined eventually sleeping the main thread like other implementations such as ginkgo. Async activities I'm interested in testing are out of process.

A few examples:

  1. fork, exec in the child
  2. use eventually to wait for it to be up in the parent process

Or:

  1. hit some external async API
  2. wait for the resource to become ready using eventually

Neither of the above would require busted async support as far as I can tell.

Tieske commented 5 years ago

use eventually to wait for it to be up in the parent process

How do you intent to "wait" if Busted has no notion of async? So I really think this is quite hard. But in the end it's not my call, maybe @ajacksified or @DorianGray can comment.

The array is a good example of what you want to do probably (except for not having any async stuff)

It's also very well possible to create a separate Lua module, that would provide those assertions on top of Luassert. You can simply use Luassert's API (see example) to register the assertions. There is no need for those to live inside the Luassert repo. Here are some project specific assertions for example.

So if you go ahead and the change is not accepted, it can still be it's own module, and nothing is lost for your own use case.

BooleanCat commented 5 years ago

So if you go ahead and the change is not accepted, it can still be it's own module, and nothing is lost for your own use case.

I'll go ahead with your suggestion and do this for sure.

However, I don't think I need a "wait" to implement this - sleep would be good enough. Pseudocoding this out it could be implemented something like:

loop {
  if timeout() {
    fail()
  }

  if f() == x {
    pass()
  }

  sleep(interval)
}
DorianGray commented 5 years ago

fascinating that you're using busted to test rust, I've also found the testing facilities there to be lackluster.

I'm going to close this for now, if it's still relevant please ping/reopen. This seems like a pretty edge case feature...I don't think I've ever written code that would eventually return a value without finality.