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

Combine 'any instance' cukes #398

Merged
merged 2 commits into from
Aug 22, 2013
Merged

Conversation

soulcutter
Copy link
Member

expect_any_instance_of.feature and any_instance.feature both have the same Feature description, which makes navigating relish app confusing:

any_instance_relishapp

To resolve this, I combined those files.

Additionally I would like to pose the question of whether it makes sense to combine message_expectations/allow_any_instance_of.feature with method_stubs/any_instance.feature ?

@coveralls
Copy link

Coverage Status

Coverage decreased (-0%) when pulling 336b691 on any_instance-combine-features into 55f06e7 on master.

@myronmarston
Copy link
Member

Thanks for taking initiative on this, @soulcutter. Long term, I'm thinking I'd rather the cukes use the new syntax consistently everywhere, and then we can have on cuke file that demonstrates usage of the old syntax (but doesn't show every last feature of it).

Do you think it makes sense to move in that direction now?

@soulcutter
Copy link
Member Author

Yeah, now is probably the time to start that conversion. What are we calling the old syntax anyway? monkey-patched? extended Object? old?..

Argh, looks like my merge was not quite right either. 😕

@myronmarston
Copy link
Member

What are we calling the old syntax anyway? monkey-patched? extended Object? old?..

We discussed several options in #266. Originally I was going to call the syntaxes :wrapped and :direct but we decided to go with :should and :expect for parity with rspec-expectations.


it "verifies that one instance of the class receives the message" do
o = Object.new
o.foo.should eq(:return_value)
Copy link
Member

Choose a reason for hiding this comment

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

Why not use expect here?

Copy link
Member

Choose a reason for hiding this comment

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

👍 especially give you're setting the expectation with the new syntax

@coveralls
Copy link

Coverage Status

Coverage decreased (-0%) when pulling 3751953 on any_instance-combine-features into 55f06e7 on master.

myronmarston added a commit that referenced this pull request Aug 22, 2013
@myronmarston myronmarston merged commit 986e20c into master Aug 22, 2013
@myronmarston myronmarston deleted the any_instance-combine-features branch August 22, 2013 05:48
@myronmarston
Copy link
Member

Thanks, @soulcutter!

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.

4 participants