Em 24-04-2011 23:39, David C. escreveu:
example “stub should work with find(id)” do
Company.stub!(:find).with(company.id, conditions: nil).and_return company
Company.stub(:find).with_args_including(company.id).and_return company
Company.find(1, 2)
There is a similar thread, right now, btw, in an issue raised on rspec-rails:
Rails 3, RSpec 2.5: Using should_receive or stub_chain with named scopes · Issue #358 · rspec/rspec-rails · GitHub. Feel free to review my comments
there and add yours. I’m not closed to any of these ideas, but I am opposed to
introducing brittle dependencies, or new APIs that open the door for a battery of
new expectations/requirements.
I agree with you. I do think we should keep it as simple as possible
relying as little much as possible on Rails itself.
Using your examples:
company = mock_active_model(Company, :id => 1)
Company.find(1, 2)
I would expect this to return [company, nil] unless Company.find(2)
actually returns something. Behind the scenes, Company.find(1) would use
the stub, while Company.find(2) would not.
company = mock_active_model(Company, :id => 1, :published_on =>
8.days.ago)
Company.find(1, :conditions => [ “published_on> ?”, 7.days.ago ]
I would expect this to return company, regardless on the conditions.
This should be explicitated on documentation of course. The idea is not
to replicate Rails here, but just to make it easy for constructing basic
object creation and simple relationships like belongs_to and has_many.
This should not work on scopes as it would be too complicated and
dependent on Rails internals.
I recognize that this last example is not that easy to decide on, since
the above approach could lead to some unexpected behaviors regarding
scopes for instance.
Maybe another approach would be defining something like that, although
this is a bit specific to Rails internals:
def mock_active_model(model, options={})
mock_model(model, options).tap do |m| # I know, I know, ignore tap
and the new hash syntax and you get the idea
model.stub!(:find).with {|id, *options|
id == m.id && (options.size == 0 || (options.size == 1 &&
options[0] == {conditions: nil}) )
}.and_return(m)
end
end
That way, the following call won’t be stubbed:
Company.find(1, :conditions => [ “published_on> ?”, 7.days.ago ]
I understand that this is not much desired because it creates a little
dependency on Rails internals, but I do see more benefits… On the
other side, supporting scopes doesn’t pay off in my opinion.
Here is why I think that this is not a big issue. Rails find API hasn’t
change as far as I remember. It just happens that Rails used to call
Company.find(1) instead of Company.find(1, conditions: nil). But this
was already possible before Rails 3. And the results should remain the
same.
On the other side, if Rails decides to change this in some next release
to use another useless option, the changes in RSpec would be trivial, I
guess, although it might clutter the code with lots of “if
Rails.version==…”. But the good news is that I don’t think that things
like that changes very often in small releases, and bigger releases use
to happen in a long time. Also, the Rails core team decided to publish
some release candidate before each final release. This provides us the
chance to request them to rollback some change that would break RSpec if
it is simple for them to fix the issue…
I’m only asking for supporting the commonest case, because it is too
common and too hard to understand what the mock is doing as it is
implemented now…
class A; end
class B
belongs_to :a
end
When someone needs some scope or is avoiding the conventions, then I
don’t think he or she would mind to write a bit more in mocks too…
I wish I could have some definite answer to your questions, but
unfortunately I don’t and I understand your concerns and I know you also
did understand mine. Maybe someone else could enlight us with another
approach
Best regards,
Rodrigo.