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

Add default value to Spy constructor argument #156

Open
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@JeroenDeDauw

JeroenDeDauw commented Oct 16, 2018

This allows replacing

mySpy = spy.new(function() end)

with

mySpy = spy.new()
Add default value to Spy constructor argument
This allows replacing

```lua
mySpy = spy.new(function() end)
```

with

```lua
mySpy = spy.new()
```
@JeroenDeDauw

This comment has been minimized.

JeroenDeDauw commented Oct 16, 2018

Submitting without tests as I do not know if this change is welcome. If is it, I will happily add some tests to this PR.

@ajacksified

This comment has been minimized.

Member

ajacksified commented Oct 25, 2018

Seems reasonable. Add some tests and 👍

@JeroenDeDauw

This comment has been minimized.

JeroenDeDauw commented Oct 27, 2018

Note that I did not run this test as I do not have lua installed

@JeroenDeDauw

This comment has been minimized.

JeroenDeDauw commented Oct 27, 2018

Would be nice to have some instructions on how to run the tests locally, especially without installing lua. In my project I added some docker stuff that allows me to run

docker-compose run --rm app busted

That won't work here since it is busted itself, and it is not really clear to me how to make it work. Seems like it gets rather involved with all the stuff in .travis/.

@Tieske

This comment has been minimized.

Contributor

Tieske commented Oct 28, 2018

@ajacksified just tested it locally. lgtm.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment