Skip to content
This repository

New syntax #266

Merged
merged 22 commits into from 11 months ago

6 participants

Myron Marston Sam Phippen Bradley Schaefer Jon Rowe Jeff Felchner David Chelimsky
Myron Marston
Owner

This is my first pass stab at the new syntax discussed in #153.

It's not ready to merge yet but I wanted to start the code review conversation. Notes:

  • I decided to call the old syntax the :direct syntax because you call methods directly on the object...and the new syntax the :wrapped syntax because you wrap the object with a method like expect or allow. What do others think of these names?
  • When we introduced the new syntax to rspec-expectations, we decided to default to both syntaxes enabled. This was necessary because we had supported expect { }.to for a long time, so it would have been a backwards incompatibility to only enable should by default. This time around, I'm thinking of only enabling the existing :direct syntax by default. Generally, I think projects should pick one syntax or the other, and only use both syntaxes during a transition period. There's no need to enable both syntaxes for backwards compatibility like there was for rspec-expectations. On the other hand, it might encourage more folks to try out the new syntax if both are enabled. I'm kinda on the fence here.
  • For any_instance stubbing, we never figured out what to do for the new syntax. For now, I've gone with expect_any_instance_of(klass).to receive and allow_any_instance_of(klass).to receive but @alindeman put together a gist of some other possibilities to consider.
  • With the current version of this code, the any instance stuff doesn't actually work when you enable only the :wrapped syntax (it works fine when both syntaxes are enabled). The problem is that the way that any instance works, it records should_receive and stub and then plays it back on the instance later...but with the :direct syntax disabled, the methods aren't there to playback onto. I'm not sure of a fix yet.
  • I didn't realize this, but the old syntax allowed any object to be as_null_object. I personally think that only makes sense on a test double, not on a partial mock. Any objections to only supporting it on test doubles for the new syntax? (On a side note, I need to enable it when only the new syntax is enabled).
  • stub_chain: Do we need to bring this forward to the new syntax? (It's kind a code smell, IMO).
  • The old syntax supported obj.stub(:msg_1 => 1, :msg_2 => 2) but I can't think of a good way to support that with the new syntax. (We'll still support double(:msg_1 => 1, :msg_2 => 2), of course). Any objects to not bringing that forward to the new syntax?
  • unstub isn't (yet) supported with the new syntax, and I'm on the fence about whether to support it or not. (There's discussion in #153 about this).
  • When rspec-mocks is used but rspec-expectations is not, expect is not available. It's trivial to write one (it'll be < 10 lines of code both for the method and the class that it instantiates), but I need to figure out how to reliably define expect if and only if it is actually needed, because we don't want it overriding expect from rspec-expectations. (I've implemented this now in ee9d82e).
  • Lots of documentation updates are needed.

Please review and give your feedback!

Sam Phippen
Collaborator

This is just a comment on the first most obvious thing I saw: I like that you've prevented users from making nonsensical negations like allow(bees).not_to receive.

Sam Phippen
Collaborator

I'm not quite sure I follow the allow syntax, and I couldn't quite get this running. What does allow(x).to receive(:foo) do? Just based on the english reading it, I'd imagine that x is allowed to receive foo, does this mean that it's not allowed to receive anything else? Does x fail if foo is never called?

Myron Marston
Owner

@samphippen -- read the background in #153 for the story on allow. Basically, it's the new syntax for stubbing a method. I need to add docs for it, though.

added some commits April 04, 2013
Myron Marston Add syntax configuration options. 0b40e0b
Myron Marston Fix `any_instance` so it doesn't rely upon `should_receive`/`stub` be…
…ing on every object.
2264fd8
Myron Marston Don't rely upon #null_object? being on every object.
This is based upon the logic in rspec/rspec-expectations@5fbe94a .

This is the last change we need to be able to use the :wrapped syntax with the :direct syntax disabled.
054e87f
Myron Marston Normalize methods to symbols to get spec to pass on 1.8.7. 37cd6f4
Myron Marston Remove obselete cucumber scenario.
With the changes for the new syntax, and the available config options,
`stub`, `should_receive` and `should_not_receive` are added to every
object initially (before RSpec::Mocks.setup) is called, but the config
can be set to remove them.
af7fa11
Myron Marston Fully qualify all constants.
On 1.9.2 I was getting failures like:

uninitialized constant BasicObject::Hash
3d664b0
Myron Marston Ensure the backtrace line is reported consistently.
MRI is different from JRuby and RBX in this regard. This should
get the travis build to pass, I hope.
f78297f
Myron Marston Allow the new syntax to be used w/o rspec-expectations.
Here we conditionally define `expect` in a superclass module
so that it is available, but `RSpec::Matchers` can still be
included later to override it.

Note: with this in place, `expect` won't be undefined when the
new syntax is disabled.  I think that's OK, though.
ee9d82e
Sam Phippen
Collaborator

I decided to call the old syntax the :direct syntax because you call methods directly on the object...and the new syntax the :wrapped syntax because you wrap the object with a method like expect or allow. What do others think of these names?

This seems sensible to me, and I can't think of any better names right now.

Bradley Schaefer
Collaborator

I think wrapped is really solid, but I'm not as sure about direct. To go with the opposite of wrapped, maybe bare?

Myron Marston
Owner

To me, "bare" implies the methods should have no receiver, (e.g: calling the on self).

Myron Marston myronmarston referenced this pull request in rspec/rspec-core April 11, 2013
Closed

Aliasing API for #describe #870

Jon Rowe
Collaborator

Do we have a feature describing this new behaviour? Seems like the quickest win for documentation...

Myron Marston
Owner

@JonRowe -- I'm planning to add both a cucumber feature and the YARD docs. I want to solidify the APIs (and the choices about which features we do or do not bring over from the old syntax) first -- otherwise I'll waste effort writing up docs for APIs that may change.

Jon Rowe
Collaborator

That's cool, just feeding back is all ;)

Jon Rowe
Collaborator

Incidentally I too dislike stub_chain but it is useful in circumstances where an API forces you to use method chaining, cough Arel.

Deleted user
ghost commented April 11, 2013

The content you are editing has changed. Reload the page and try again.

What about expect(Object, :any_instance).to receive(:foo)

Sending Request…

Attach images by dragging & dropping or selecting them. Octocat-spinner-32 Uploading your images… Unfortunately, we don't support that file type. Try again with a PNG, GIF, or JPG. Yowza, that's a big file. Try again with an image file smaller than 10MB. This browser doesn't support image attachments. We recommend updating to the latest Internet Explorer, Google Chrome, or Firefox. Something went really wrong, and we can't process that image. Try again.

Myron Marston
Owner

@burns -- that reads OK, but creates a funny situation where expect accepts a special :any_instance argument for receive, and otherwise does not accept other arguments. Plus expect is defined by rspec-expectations and I don't want rspec-mocks to monkey patch it.

Myron Marston
Owner

If we do want to support stub_chain with the new syntax, I suppose we could do something like:

allow(dbl).to receive_message_chain("a.b.c").and_return(5)

As for the current dbl.stub(:a => 1, :b: => 2) syntax, we could support that with:

allow(dbl).to receive_messages(:a => 1, :b => 2)

...or even:

allow(dbl).to receive(:a => 1, :b => 2)

Note that in both of these cases, it would allow the setting of multiple expectations for expect(dbl) -- which is a feature we didn't have before.

@dchelimsky -- when you have time, can you review this PR? I included a list of open questions in the PR message above.

Myron Marston myronmarston commented on the diff April 12, 2013
features/outside_rspec/configuration.feature
@@ -16,10 +16,6 @@ Feature: configure any test framework to use rspec-mocks
16 16
       should_not_receive
17 17
       stub
18 18
 
19  
-  In order to give control to the consuming framework, none of these facilities
20  
-  are added until RSpec::Mocks::setup(self) is called. Simply requiring
21  
-  'rspec/mocks' is not sufficient.
22  
-
6
Myron Marston Owner

@dchelimsky -- this is one thing I was unsure about. Now that the syntax is configurable, it defaults to the :direct syntax when rspec-mocks is loaded but can be reconfigured to :wrapped, which removes the methods from all objects. Before it delayed when the methods get added...the reason given here is "to give control to the consuming framework", but I don't really understand that. Is it important to continue to delay when the methods get added, given that this gives users the ability to remove the methods by changing the config?

David Chelimsky Owner
dchelimsky added a note April 12, 2013

IIRC people ran into trouble with respect to load order when rspec/mocks was required. Does that help?

Myron Marston Owner

It helps a bit. If nothing else, it suggests that the RC release(s) I'm planning is even more important.

Jon Rowe Collaborator
JonRowe added a note May 06, 2013

I'm pretty certain bundler causes some require order problems too, e.g. you want the setup happening after everything else.

Myron Marston Owner
myronmarston added a note May 07, 2013

@JonRowe -- I'm not sure what you mean...can you explain more?

Jon Rowe Collaborator
JonRowe added a note May 07, 2013

If I'm recalling correctly, RSpec::Mocks::setup(self) monkey patches the expectation (should_receive,should) methods onto self, and this needs to happen after other frameworks have finished monkey patching.

Bundler doesn't guarantee a load order when using the automatic require methods, hence the need to call this manually rather than have it performed on a require.

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

@myronmarston I vote :+1: for only defining as_null_object on RSpec's objects. Currently I have an object which I want to define my own as_null_object on and it is causing issues because respond_to?(:as_null_object) is returning true no matter what object I throw at it.

No insurmountable but in the spirit of what you're trying to do, it's one more place to cut down on the surface area of RSpec's interface.

Jon Rowe
Collaborator
I decided to call the old syntax the :direct syntax because you call methods directly
on the object...and the new syntax the :wrapped syntax because you wrap the object
with a method like expect or allow. What do others think of these names?

Those names make sense to me... but we could also use :should syntax and :expect syntax?

This time around, I'm thinking of only enabling the existing :direct syntax by default.

We should enable the :wrapped (:expect) syntax by default, because that's inline with the rest of RSpec no? I can appreciate the lack of desire to have two syntaxes but we need to make those calls eventually.

I didn't realize this, but the old syntax allowed any object to be as_null_object. I
personally think that only makes sense on a test double, not on a partial mock. 

I think this makes total sense. Very much :+1:

Sam Phippen
Collaborator

I like :expect and :should for the syntax names quite a lot. @myronmarston what do you think?

Jeff Felchner

@myronmarston I think :expect and :should for the syntax names are more clear and are also in line with rspec-expectations configuration option correct? I'm don't have a strong feeling about the names but I feel like rspec-expectations and rspec-mocks should match.

Myron Marston
Owner

Regarding naming the syntax options :expect and :should....I seriously considered it, but here's why I didn't (at least initially) choose that (but I'm still open to it):

  • For the new syntax in rspec-expectations, expect and should made tons of sense because they correspond to the methods you use to begin an expectation expression. Those are basically the only two methods involved. (Technically, there's also should_not, but that's just part of should).
  • For rspec-mocks, the existing syntax has many methods:
    • should_receive
    • should_not_receive
    • stub
    • stub!
    • unstub
    • unstub!
    • stub_chain
    • any_instance
  • Likewise, the new syntax has multiple methods:
    • expect (actually, this is added by rspec-expectations)
    • allow
    • receive
    • allow_any_instance_of
    • expect_any_instance_of
  • ...so should and expect don't seem to be very good summaries of these two syntaxes :(.
  • On top of that, one could use should with the new wrapped syntax: object.should receive(:foo).with(5). (Not that I'd recommend it, but I believe that would "just work").

Those of you recommending we use :expect and :should: do you still think those are the right names given these factors? (In retrospect, I kinda wish I had called the syntaxes :direct and :wrapped in rspec-expectations...)

Sam Phippen
Collaborator

So I agree with you that :should and :expect may not be the best summations of the new syntax. However: I think that our users know the two flavours of syntax as the "should" and "expect" syntax. As such I think we should use these names. Is it the case that we already use these names for rspec-expectations as @jfelchner says?

David Chelimsky
Owner

In the long run, I think it's a better outcome if these names align across rspec-expectations and rspec-mocks. i.e. if we go w/ direct/wrapped, let's do that for rspec-expectations as well. Even though it's already out there w/ should/expect, that can change - just need a rollout strategy (deprecation, etc).

Myron Marston
Owner

Is it the case that we already use these names for rspec-expectations as @jfelchner says?

Yep.

Sam Phippen
Collaborator

@dchelimsky regarding the rollout strategy do you think this is a deprecate in 2.14 -> remove in 3.0.0 thing or is that a little soon? I agree that we should sync them up regardless. I weakly prefer :expect and :should because it's what our users know and love at the moment but I could be convinced either way.

Jon Rowe
Collaborator

I prefer expect and should, because it makes it clear which syntax we're referring to, I know it doesn't describe all the methods but in my head I know which ones are should and which ones are expect, so it makes sense.

Jeff Felchner

@myronmarston fair points. I actually hadn't thought that far into it, which is why you're in charge. :wink:

I still think that even though I agree your way is more logical, most users of RSpec who are actually going to change this setting will be fine with :expect and :should. Personally I'm fine with whatever but think that both rspec-expectations and rspec-mocks should sync up.

Myron Marston
Owner

Is anyone planning to code review this?

David Chelimsky
Owner

I might have some time later this week.

Sam Phippen
Collaborator

I'm having a look right now. I'll leave some comments

Sam Phippen samphippen commented on the diff May 06, 2013
lib/rspec/mocks/targets.rb
... ...
@@ -0,0 +1,67 @@
  1
+module RSpec
  2
+  module Mocks
  3
+    UnsupportedMatcherError  = Class.new(StandardError)
  4
+    NegationUnsupportedError = Class.new(StandardError)
  5
+
  6
+    class TargetBase
  7
+      def initialize(target)
  8
+        @target = target
  9
+      end
  10
+
  11
+      def self.delegate_to(matcher_method, options = {})
  12
+        method_name = options.fetch(:from) { :to }
  13
+        define_method method_name do |matcher, &block|
4
Sam Phippen Collaborator
samphippen added a note May 06, 2013

If this is going in 2.14 and needs to be backward compatible should this be an eval instead of a define method, or am I missing something?

Jon Rowe Collaborator
JonRowe added a note May 06, 2013

I believe eval is only needed over define_method when we have *args blobs as 1.8.6 doesn't support them.

Myron Marston Owner
myronmarston added a note May 07, 2013

No, I think @samphippen is right -- on 1.8.6 this'll yield a syntax error.

Jon Rowe Collaborator
JonRowe added a note May 07, 2013

My bad, @samphippen is correct, it's *args you can do on 1.8.6 and not &block got mixed up (knew you couldn't do one of them)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
spec/rspec/mocks/methods_spec.rb
@@ -2,29 +2,25 @@
2 2
 
3 3
 module RSpec
4 4
   module Mocks
5  
-    describe Methods, :if => (Method.method_defined?(:owner)) do
6  
-      def added_methods(klass, owner)
7  
-        some_object = klass.new
8  
-        (some_object.methods + some_object.private_methods).select do |method|
9  
-          some_object.method(method).owner == owner
10  
-        end.map(&:to_sym)
  5
+    describe "Methods added to every object" do
  6
+      include_context "with syntax", :wrapped
  7
+
  8
+      def added_methods
  9
+        host = Class.new
  10
+        orig_instance_methods = host.instance_methods
  11
+        Syntax.enable_direct(host)
  12
+        (host.instance_methods - orig_instance_methods).map(&:to_sym)
1
Sam Phippen Collaborator
samphippen added a note May 06, 2013

I like this more than how it used to work.

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

I like most of this. I've left some comments. It seems that lots of the methods don't have docstrings. Do they need them? I suspect much of this is going to end up being private.

Jon Rowe JonRowe commented on the diff May 06, 2013
lib/rspec/mocks/matchers/have_received.rb
((50 lines not shown))
  50
+        def apply_constraints_to(expectation)
  51
+          @constraints.each do |constraint|
  52
+            expectation.send(*constraint)
  53
+          end
  54
+        end
  55
+
  56
+        def ensure_count_unconstrained
  57
+          if count_constraint
  58
+            raise RSpec::Mocks::MockExpectationError,
  59
+              "can't use #{count_constraint} when negative"
  60
+          end
  61
+        end
  62
+
  63
+        def count_constraint
  64
+          @constraints.map(&:first).detect do |constraint|
  65
+            COUNT_CONSTRAINTS.include?(constraint)
3
Jon Rowe Collaborator
JonRowe added a note May 06, 2013

Doing a chained map then detect makes me nervous (performance wise) cpi;d we streamline this into one loop? Or instead of the detect use the intersection operator?

@constraints.map { |constraints| constraints.first & COUNT_CONSTRAINTS }
Myron Marston Owner
myronmarston added a note May 07, 2013

This is existing code from #241. I simply moved it into the new matchers directory since I was adding the receive matcher.

You're right that there may be a way to optimize this, but on the surface your code snippet isn't obviously faster to me. (Plus it returns an array, not a single constraint). If you want to take a stab at benchmarking it and changing it in another PR, have at it. With the array only having 5 elements, I'm not terribly concerned.

Jon Rowe Collaborator
JonRowe added a note May 07, 2013

It's a 5x multiplier on the outer array, that's my only concern. I'll take a look at it after it's merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Jon Rowe
Collaborator
JonRowe commented May 06, 2013

I've been over this a few times now @myronmarston and I'm pretty happy with it :) You've done an awesome job, I've added a bit more 2¢ on a couple of things but mostly I'm still only really noticing the lack of a .feature file ;)

added some commits May 07, 2013
Sam Phippen Change :direct and :wrapped for :should and :expect.
Signed-off-by: Sam Phippen <samphippen@googlemail.com>
bc04bd0
Sam Phippen Add a .feature for the new expect syntax.
Signed-off-by: Sam Phippen <samphippen@googlemail.com>
f7d7676
Sam Phippen
Collaborator

@myronmarston @soulcutter @alindeman @JonRowe I've just written a .feature for this. It's my first one so any comments would be appreciated :)

Sam Phippen
Collaborator

I knew I forgot someone: @dchelimsky if you'd also take a look that'd be great.

features/message_expectations/expect_syntax.feature
... ...
@@ -0,0 +1,148 @@
  1
+Feature: the expect syntax for message expectations
  2
+
  3
+  Use expect(receiver).to receive(:message) to set an expectation that
2
Jon Rowe Collaborator
JonRowe added a note May 07, 2013

shouldn't expect(receiver).to receive(:message) be in backticks? expect(receiver).to receive(:message)

Sam Phippen Collaborator
samphippen added a note May 07, 2013

yes it should :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
features/message_expectations/expect_syntax.feature
... ...
@@ -0,0 +1,148 @@
  1
+Feature: the expect syntax for message expectations
  2
+
  3
+  Use expect(receiver).to receive(:message) to set an expectation that
  4
+  `receiver` should recieve the message `:message` before the example is
2
Jon Rowe Collaborator
JonRowe added a note May 07, 2013

receive the message...

Sam Phippen Collaborator
samphippen added a note May 07, 2013

:D speeling is hard

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
features/message_expectations/expect_syntax.feature
((29 lines not shown))
  29
+      class Account
  30
+        attr_accessor :logger
  31
+
  32
+        def close
  33
+          logger.account_closed
  34
+        end
  35
+      end
  36
+      """
  37
+    And a file named "spec/spec_helper.rb" with:
  38
+    """ruby
  39
+    RSpec.configure do |config|
  40
+      config.mock_with :rspec do |mocks|
  41
+        mocks.syntax = :expect
  42
+      end
  43
+    end
  44
+
2
Jon Rowe Collaborator
JonRowe added a note May 07, 2013

cough white space

Sam Phippen Collaborator
samphippen added a note May 07, 2013

this is now fixed, but the comment still lives on GitHub.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Sam Phippen Clean up expect_syntax.feature.
Review from @JonRowe.

Signed-off-by: Sam Phippen <samphippen@googlemail.com>
7a7643e
Jon Rowe
Collaborator
JonRowe commented May 07, 2013

The .feature looks pretty good to me; One thing that... irks... me is:

 And a file named "spec/spec_helper.rb" with:
 """ruby
 RSpec.configure do |config|
   config.mock_with :rspec do |mocks|
     mocks.syntax = :expect
   end
 end

I'm in two minds about it, on the one hand it nicely documents how to setup this syntax, on the other hand it feel's like it should be compressed out the way, as it's not totally relevant to the feature at hand... I think I'd like to see a background step

Background:
  Given there is a spec helper setting syntax to `:expect`

but I'd also be open to simply moving that block from the scenarios into a background.

Sam Phippen
Collaborator

Inserting some clarity:
(from IRC)

[00:08:18] <Sou|cutter>  samphippen: oooh, your first cuke eh? :)
[00:08:24] <samphippen>  not my first
[00:08:26] <samphippen>  I mean first on rspec
[00:08:32] <Sou|cutter>  oh ok :)
and others added some commits May 08, 2013
Sam Phippen Move the spec_helper create in expect_syntax.feature to a background.
Signed-off-by: Sam Phippen <samphippen@googlemail.com>
4af8d70
Bradley Schaefer Rename `receiver` to `object` and tighten up feature slightly 1a5a5b1
Sam Phippen Merge branch 'master' into new_syntax
Signed-off-by: Sam Phippen <samphippen@googlemail.com>

Conflicts:
	lib/rspec/mocks/have_received.rb
	lib/rspec/mocks/method_double.rb
87efdd7
Sam Phippen
Collaborator

@myronmarston I've merged this up with master. One concern: I think we should enable both syntaxes by default. The reason for this is that we prefer the expect syntax everywhere else. It seems like it may introduce cognitive dissonance if we prefer one syntax for one thing, but disable it for the other thing by default. WDYT?

David Chelimsky
Owner

@samphippen late to the party, but ... there are now two features: expect_message.feature and expect_syntax.feature, which read "expect a message" and "the expect syntax for message expectations". Each one makes sense on its own but I'd like to align them better, or perhaps merge them, or perhaps split them into files by sub-feature (expect message, expect message w/ args, etc). I'm going to play with this a bit and push to a branch if I come up with something I like.

Myron Marston
Owner

One concern: I think we should enable both syntaxes by default. The reason for this is that we prefer the expect syntax everywhere else. It seems like it may introduce cognitive dissonance if we prefer one syntax for one thing, but disable it for the other thing by default. WDYT?

I'm on the fence about this one. For rspec-expectations we had to enable both syntaxes by default due to backwards compatibility -- since the expect block form had been around for a long time. Here we don't have to enable both (which is why I didn't initially -- the thought being that folks will want to use one or the other rather than both), but I'm open to doing so. I don't have a strong preference. I'm happy to defer to whatever the majority community preference is here.

David Chelimsky
Owner

@samphippen Take a look at d8369b7. WDYT?

Sam Phippen
Collaborator

@dchelimsky I like what you've done. Care to merge it into this branch?

Jon Rowe
Collaborator
JonRowe commented May 08, 2013

Took the liberty of picking across ;)

David Chelimsky
Owner

Thanks @JonRowe

Sam Phippen
Collaborator

I've put together this gist to ask people to express their preferences. I'd like feedback today before sending it out to the community at large via the twitter/mailing list.

edit: @JonRowe @myronmarston @soulcutter @alindeman @dchelimsky if you could take a look and tell me what you think that'd be awesome :cat:

Jon Rowe
Collaborator
JonRowe commented May 09, 2013

Nicely worded :)

Sam Phippen
Collaborator

I'm basically wondering whether we should add some of the reasoning for the new syntax being the default in the gist?

Jon Rowe
Collaborator
JonRowe commented May 09, 2013

Sure, but I'd keep it brief.

Sam Phippen
Collaborator

Ok, I've done that. Feel free to take a look.

David Chelimsky
Owner
Sam Phippen
Collaborator

@dchelimsky I've reworded that now. WDYT?

David Chelimsky
Owner

Looks good, thanks.

Myron Marston
Owner
Sam Phippen
Collaborator

Ok. Looks like people want both on by default. Should I do that, update the cuke, wait for travis and then merge?

Myron Marston
Owner

Ok. Looks like people want both on by default. Should I do that, update the cuke, wait for travis and then merge?

:+1:

added some commits May 10, 2013
Sam Phippen Enable the :expect syntax by default.
Signed-off-by: Sam Phippen <samphippen@googlemail.com>
563e2f5
Sam Phippen Add a changelog entry for #266.
Signed-off-by: Sam Phippen <samphippen@googlemail.com>
5639d34
Sam Phippen samphippen merged commit ae03e63 into from May 10, 2013
Sam Phippen samphippen closed this May 10, 2013
Myron Marston
Owner

Thanks, @samphippen!

Looking over the diff one more time I noticed there are still places that reference the :wrapped syntax. I think these need to be updated -- want to take a stab at it?

spec/rspec/mocks/methods_spec.rb
6:      include_context "with syntax", :wrapped

spec/rspec/mocks/null_object_mock_spec.rb
107:    describe "when using the :wrapped syntax" do
108:      include_context "with syntax", :wrapped
Sam Phippen
Collaborator
Ilya Zayats somebody32 referenced this pull request in somebody32/adequack May 31, 2013
Closed

Add new expect syntax #5

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

Showing 22 unique commits by 5 authors.

Apr 03, 2013
Myron Marston Move `HaveReceived` matcher into `Matchers` module namespace. 70a6da3
Myron Marston Add new expect/allow syntax. 08ec2e8
Apr 07, 2013
Myron Marston Add syntax configuration options. 0b40e0b
Myron Marston Fix `any_instance` so it doesn't rely upon `should_receive`/`stub` be…
…ing on every object.
2264fd8
Myron Marston Don't rely upon #null_object? being on every object.
This is based upon the logic in rspec/rspec-expectations@5fbe94a .

This is the last change we need to be able to use the :wrapped syntax with the :direct syntax disabled.
054e87f
Myron Marston Normalize methods to symbols to get spec to pass on 1.8.7. 37cd6f4
Myron Marston Remove obselete cucumber scenario.
With the changes for the new syntax, and the available config options,
`stub`, `should_receive` and `should_not_receive` are added to every
object initially (before RSpec::Mocks.setup) is called, but the config
can be set to remove them.
af7fa11
Myron Marston Fully qualify all constants.
On 1.9.2 I was getting failures like:

uninitialized constant BasicObject::Hash
3d664b0
Myron Marston Ensure the backtrace line is reported consistently.
MRI is different from JRuby and RBX in this regard. This should
get the travis build to pass, I hope.
f78297f
Apr 08, 2013
Myron Marston Allow the new syntax to be used w/o rspec-expectations.
Here we conditionally define `expect` in a superclass module
so that it is available, but `RSpec::Matchers` can still be
included later to override it.

Note: with this in place, `expect` won't be undefined when the
new syntax is disabled.  I think that's OK, though.
ee9d82e
Apr 17, 2013
Myron Marston Support null_object test doubles with new syntax. e4525a9
Myron Marston Add yard docs for new methods. b9680ab
Myron Marston Improve the way we make `expect` available when rspec-expectations is…
… not used.

- Put it in a named rather than anonymous module.
- Allow the syntax to be toggled.
e6639ad
May 07, 2013
Sam Phippen Change :direct and :wrapped for :should and :expect.
Signed-off-by: Sam Phippen <samphippen@googlemail.com>
bc04bd0
Sam Phippen Add a .feature for the new expect syntax.
Signed-off-by: Sam Phippen <samphippen@googlemail.com>
f7d7676
Bradley Schaefer Rename `receiver` to `object` and tighten up feature slightly 1a5a5b1
May 08, 2013
Sam Phippen Clean up expect_syntax.feature.
Review from @JonRowe.

Signed-off-by: Sam Phippen <samphippen@googlemail.com>
7a7643e
Sam Phippen Move the spec_helper create in expect_syntax.feature to a background.
Signed-off-by: Sam Phippen <samphippen@googlemail.com>
4af8d70
Sam Phippen Merge branch 'master' into new_syntax
Signed-off-by: Sam Phippen <samphippen@googlemail.com>

Conflicts:
	lib/rspec/mocks/have_received.rb
	lib/rspec/mocks/method_double.rb
87efdd7
May 09, 2013
David Chelimsky align expect_message* cukes a15b91e
May 10, 2013
Sam Phippen Enable the :expect syntax by default.
Signed-off-by: Sam Phippen <samphippen@googlemail.com>
563e2f5
Sam Phippen Add a changelog entry for #266.
Signed-off-by: Sam Phippen <samphippen@googlemail.com>
5639d34
Something went wrong with that request. Please try again.