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

[WIP] Variable definitions #290

Closed
wants to merge 13 commits into from
Closed

[WIP] Variable definitions #290

wants to merge 13 commits into from

Conversation

assafgelber
Copy link
Contributor

Hi there!

Background:
I really like the way lets work in rspec, and also saw issue #231, so I tried implementing something similar for quick.

Implementation:
I added a Definitions class which holds a dictionary which maps "variable names" as strings to closures and a dictionary which holds memoized values of those closures.
I added a definitions property to Configuration which can also be used through World, in a similar fashion to the way hooks are used.
So now from anywhere in your tests, you can use define (let was taken 😸) which takes a name and a closure to define a variable and fetch which takes a variable name and returns the value for that variable. The values are memoized for example and reset between examples (just like rspec).

My Problem:
In swift, this works just as expected, the tests I wrote are passing and everything is happy.
In Objective-C, however, my last test (which is identical to the swift one) is failing. Instead of number being 4, I get 2 again. My assumption is that the value of mutatingNumber is being captured in the block even though I marked it as __block. I am not completely sure why and I couldn't get this to work, so I though one of you guys might have a solution to this.

I really appreciate the work you've all been putting into Quick and would really love to see this feature making it's way in if we can figure this out.
Let me know if there's any more information I could give or any other thing I can do!

Thanks! 🍻


class FunctionalTests_DefinitionsSpec: QuickSpec {
override func spec() {
define("one") { return 1 }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting. You could probably make this even shorter:

define("one") { 1 }

I bet you could also have define take an @autoclosure(escaping) parameter, which would allow you to write:

define("one", 1)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@modocache
Copy link
Member

OK, so this pull request blew me away. Thanks a ton for the test coverage and the detailed message explaining what you did. 👍

And while overall I think this is an interesting approach, ultimately I think it's missing type safety. The casting between AnyObject and the desired type allows for too much room for user error. 🚨

I do think, however, Quick should allow users to customize it's behavior as much as possible. We could allow for define to be vended as a third-party Quick extension with a little work. If that sounds interesting, @agelber, I'll create an issue on the Quick repository to track that change.

I think we should look to RSpec as much as possible for guidance here. Some key questions I'd want to research before proceeding here are:

  1. How does RSpec implement let? Yes, it's Ruby, so we probably won't be able to use its approach directly, but their implementation would be a great frame of reference.
  2. RSpec 3.0 extracted its into a separate RubyGem. I assume rspec/rspec-its also needs to integrate with RSpec. How does RSpec 3.0 allow external gems to be integrated with how it runs specs?

@assafgelber
Copy link
Contributor Author

Yeah, I see what you mean about the type safety issue, I didn't quite think about that, coming from ruby.
Another approach I had was to have define methods for types, so for example, defineInt, defineString and so on, but that would still leave you with the issue for most use-cases.
The other option would be to use generics. Is it possible to have method-level generics as opposed to class-level? i.e. define<T>(name: String, closure: () -> T and fetch<T>() -> T?

Either way, a plugin system sounds like an awesome idea. I'd be happy to help with anything you need. Feel free to contact me via email as well.

@jeffh
Copy link
Member

jeffh commented May 6, 2015

You can have generic methods and functions, but you'll need to link the generic type from its definition to its usage in a way the Swift compiler can track and infer its type.


Sent from my iPhone

On Wed, May 6, 2015 at 12:44 AM, Assaf Gelber notifications@github.com
wrote:

Yeah, I see what you mean about the type safety issue, I didn't quite think about that, coming from ruby.
Another approach I had was to have define methods for types, so for example, defineInt, defineString and so on, but that would still leave you with the issue for most use-cases.
The other option would be to use generics. Is it possible to have method-level generics as opposed to class-level? i.e. define<T>(name: String, closure: () -> T and fetch<T>() -> T?

Either way, a plugin system sounds like an awesome idea. I'd be happy to help with anything you need. Feel free to contact me via email as well.

Reply to this email directly or view it on GitHub:
#290 (comment)

@assafgelber
Copy link
Contributor Author

I see.. I'll try to think of other ways, I guess. Would be glad to get some pointers or suggestions 🍻

@modocache
Copy link
Member

Thanks again for the great code and discussion, @agelber! 💐

To keep the pull request tab clean--I'd like only requests in active review to stay there--I'm going to close this pull request in favor of two issues:

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 this pull request may close these issues.

None yet

3 participants