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.

Sign up for GitHub

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

Jump to bottom

Add expect syntax to spec #3375

Closed
wants to merge 1 commit into from
Closed

Add expect syntax to spec #3375

wants to merge 1 commit into from
+86 −6

Conversation

dukex
Copy link
Contributor

@dukex dukex commented Oct 3, 2016

Adding support to the expect syntax in specs.

The expect syntax was introduced in the middle of 2012 into rspec and the old syntax(should, should_not) was marked as deprecated in RSpec 3 in an effort to avoid Monkey Patching(read more here).

Because that(make tests without monkey patching) and because a liked the expect syntax I made this implementation pretty near of rspec-expectations

@chaniks
Copy link

chaniks commented Oct 3, 2016 •
edited
Loading

@dukex I thought they introduced expect syntax because should method didn't work well with proxy objects.

Is it inevitable in Crystal too?

Added:

In Crystal, method_missing works in different way, and I guess monkey patching will be an issue. I am just guessing, so please let me know if I missed anything.

@asterite
Copy link
Member

asterite commented Oct 3, 2016

@dukex Thank you!

But duplicate of #609 and #334

@asterite asterite closed this Oct 3, 2016
@asterite
Copy link
Member

asterite commented Oct 3, 2016

And #564

@dukex
Copy link
Contributor Author

dukex commented Oct 4, 2016

Hi @asterite,

Thanks for address me this issues, So I don't want start again the discussion about expect vs should, like I said one of my main proposal with this PR is because I prefer the expect syntax, I think the main mistake in #334 and #564 was try to follow rspec and change all specs to expect syntax, I don't know in details how crystal handle with monkey patching and if it's good or not, but why not add the expect syntax for the people think it is better?

In the other words, my PR is not about expect vs should but it's about expect and should syntax.

Make sense?

@ysbaddaden
Copy link
Contributor

We try to avoid "there are 2 ways to do 1 thing" in the stdlib. expect would duplicate what should already does. What about creating an expect Shard?

@dukex
Copy link
Contributor Author

dukex commented Oct 4, 2016

Nice @ysbaddaden, thank you, I think now is very clear for me the reason to close this.

@dukex dukex deleted the add-expect-syntax branch October 4, 2016 14:35
@keplersj
Copy link
Contributor

keplersj commented Oct 4, 2016

For what it's worth, I'd love an expect shard.

@dukex
Copy link
Contributor Author

dukex commented Oct 5, 2016

@keplersj created https://github.com/dukex/expect.cr

@straight-shoota straight-shoota mentioned this pull request Apr 9, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@dukex @chaniks @asterite @ysbaddaden @keplersj