rspec users - Jan 2007 - Collection proxies need to be stubbed ?

Hi all !

I just started writing specs on a new project.  I would just like to
validate that this is the way you would write it.  I know about mocks,
stubs and expectations.  I don''t think this is a problem for me.

My question really boils down to:

def index
  @projects = current_user.projects.active
end

My spec needs to return the proxy, no ?  Here''s my code:

context "A logged in user visiting the index action" do
  controller_name :dashboard

  setup do
    @user = mock("user")
    controller.stub!(:current_user).and_return(@user)

    @user.should_receive(:projects).and_return(projects_proxy = Object.new)
    projects_proxy.should_receive(:active).and_return([:a, :b])
    get :index
  end

  specify "should have a list of projects available" do
    assigns[:projects].should == [:a, :b]
  end

  specify "should render the index view" do
    controller.should_render :template => "dashboard/index"
  end

  specify "should be a successful call" do
    response.should_be_success
  end
end

Thanks !
-- 
Fran?ois Beausoleil
http://blog.teksol.info/
http://piston.rubyforge.org/

Hi!

Francois Beausoleil wrote:
> My question really boils down to:
>
> def index
>   @projects = current_user.projects.active
> end
>
> My spec needs to return the proxy, no ?  Here''s my code:
>   
>From a RSpec-newbie''s point of view:
Looks like current_user is a method of the controller, that selects the
user from the request params, right? Mocking this means, current_user()
will not be covered by the specs. Normally I avoid mocking/stubbing
members of a class under test or private and protected members of any
class. You could instead mock the User class directly to return the User
mock.

But why not use fixtures and let it run with real data instead of
mocking the database away?

bye,

Tobias

On 1/20/07, Francois Beausoleil <francois.beausoleil at gmail.com>
wrote:
> Hi all !
>
> I just started writing specs on a new project.  I would just like to
> validate that this is the way you would write it.  I know about mocks,
> stubs and expectations.  I don''t think this is a problem for me.
>
> My question really boils down to:
>
> def index
>   @projects = current_user.projects.active
> end
>
> My spec needs to return the proxy, no ?  Here''s my code:
>
> context "A logged in user visiting the index action" do
>   controller_name :dashboard
>
>   setup do
>     @user = mock("user")
>     controller.stub!(:current_user).and_return(@user)
>
>     @user.should_receive(:projects).and_return(projects_proxy = Object.new)
>     projects_proxy.should_receive(:active).and_return([:a, :b])
>     get :index
>   end
>
>   specify "should have a list of projects available" do
>     assigns[:projects].should == [:a, :b]
>   end
>
>   specify "should render the index view" do
>     controller.should_render :template => "dashboard/index"
>   end
>
>   specify "should be a successful call" do
>     response.should_be_success
>   end
> end
I would approach this quite differently. The first difference being I
would have started with a spec, letting the spec help me to discover
the interface that I want on User.

context "A logged in user visiting the index action" do
  controller_name :dashboard

  setup do
    @user = mock("user")
    controller.stub!(:current_user).and_return(@user)
    @user.should_receive(:active_projects).and_return([:a, :b])

    get :index
  end

  specify "should have a list of projects available" do
    assigns[:projects].should == [:a, :b]
  end

  specify "should render the index view" do
    response.should_render :template => "dashboard/index"
  end

  specify "should be a successful call" do
    response.should_be_success
  end
end

This would lead me to this implementation ...

def index
  @projects = current_user.active_projects
end

... which would, in turn, lead me to spec out a method named
active_projects on User.

The other thing that I''ve been doing lately is making an effort to
separate out the noise (in setup) from the interesting bits in each
specification. I do that by using stub! in setup and should_receive in
specify, using should_receive to override stub! when appropriate. This
leads to a more verbose spec, but it makes each specify block complete
(so you don''t have to look back at setup).

context "A logged in user visiting the index action" do
  controller_name :dashboard

  setup do
    @user = mock("user")
    @user.stub!(:active_projects).and_return([])
    controller.stub!(:current_user).and_return(@user)
  end

  specify "should have a list of projects available" do
    #given
    @user.should_receive(:active_projects).and_return([:a, :b])

    #when
    get :index

    #then
    assigns[:projects].should == [:a, :b]
  end

  specify "should render the index view" do
    #given - nothing unusual

    #when
    get :index

    #then
    response.should_be_success
    response.should_render :template => "dashboard/index"
  end
end

Here I''ve identified in each spec the Given/When/Then with comments.
Note that I abhor comments like this in production code, but I''ve come
to view specs as different from code. The result is that each spec is
easy to read and understand in isolation. You don''t have to look up at
setup to understand a problem.

Note also that I''ve used two expectations in one spec. For me, having
a separate spec for should_be_success is not that meaningful when
we''re spec''ing the template. From a developer standpoint it
might be
helpful to have the statement there when trying to understand a
problem, but from the outside we know it was successful if the right
template shows up, no?

A little more than 2 cents. Hope this is all helpful.

Cheers,
David

>
> Thanks !
> --
> Fran?ois Beausoleil
> http://blog.teksol.info/
> http://piston.rubyforge.org/
> _______________________________________________
> rspec-users mailing list
> rspec-users at rubyforge.org
> http://rubyforge.org/mailman/listinfo/rspec-users

On 1/21/07, Tobias Grimm <listaccount at e-tobi.net>
wrote:
> Hi!
>
> Francois Beausoleil wrote:
> > My question really boils down to:
> >
> > def index
> >   @projects = current_user.projects.active
> > end
> >
> > My spec needs to return the proxy, no ?  Here''s my code:
> >
> >From a RSpec-newbie''s point of view:
>
> Looks like current_user is a method of the controller, that selects the
> user from the request params, right? Mocking this means, current_user()
> will not be covered by the specs. Normally I avoid mocking/stubbing
> members of a class under test or private and protected members of any
> class. You could instead mock the User class directly to return the User
> mock.
If the current_user is actually coming from request params I agree w/
this. If it''s a user stored in the session, I don''t agree that
it
should come from the User class. I would probably stick the mock user
in the session though, rather than stub the method.
> But why not use fixtures and let it run with real data instead of
> mocking the database away?
Because specs run faster when you don''t rely on the database.
You''re
going to be spec''ing all the same stuff in your model specs, so why
hit the database more than you need to?

Cheers,
David
>
> bye,
>
> Tobias
>
>
> _______________________________________________
> rspec-users mailing list
> rspec-users at rubyforge.org
> http://rubyforge.org/mailman/listinfo/rspec-users
>

David Chelimsky wrote:
> this. If it''s a user stored in the session, I don''t agree
that it
> should come from the User class. I would probably stick the mock user
> in the session though, rather than stub the method.
>   
Agreed - if it''s in the session, this should be the preferred way.
> Because specs run faster when you don''t rely on the database.
You''re
> going to be spec''ing all the same stuff in your model specs, so
why
> hit the database more than you need to?
>   
I don''t have any experience with RSpec/Rails using a database - my
current Rails project is DB-less :-) But on a recent C# project (with
NUnit) I''ve chosen to just use a database in my tests. It
wasn''t even an
in-memory database (embedded Firebird) but it was still fast enough with
several hundred tests hitting the database. Rails with SQLite in memory
should even be faster. So speed isn''t really much of an issue in my
experience.

But you''re right that stubbing database access should be preferred.
Sometimes it''s just easier to put complex data into a fixture and
retrieve it from the database. Maybe that''s a design smell -
I''ll keep
this in mind when I run into this situation the next time.


Tobias

On 1/21/07, Tobias Grimm <listaccount at e-tobi.net>
wrote:
> David Chelimsky wrote:
> > this. If it''s a user stored in the session, I don''t
agree that it
> > should come from the User class. I would probably stick the mock user
> > in the session though, rather than stub the method.
> >
>
> Agreed - if it''s in the session, this should be the preferred way.
>
> > Because specs run faster when you don''t rely on the database.
You''re
> > going to be spec''ing all the same stuff in your model specs,
so why
> > hit the database more than you need to?
> >
>
> I don''t have any experience with RSpec/Rails using a database - my
> current Rails project is DB-less :-) But on a recent C# project (with
> NUnit) I''ve chosen to just use a database in my tests. It
wasn''t even an
> in-memory database (embedded Firebird) but it was still fast enough with
> several hundred tests hitting the database.
What''s fast enough?

When doing BDD or TDD, the goal is to run the specs (tests) as often
as possible, after every little change. I run my specs dozens (if not
hundreds) of times a day, so a difference of a few seconds adds up to
a big difference.
>Rails with SQLite in memory
> should even be faster. So speed isn''t really much of an issue in
my
> experience.
>
> But you''re right that stubbing database access should be
preferred.
> Sometimes it''s just easier to put complex data into a fixture and
> retrieve it from the database. Maybe that''s a design smell -
I''ll keep
> this in mind when I run into this situation the next time.
>
>
> Tobias
>
> _______________________________________________
> rspec-users mailing list
> rspec-users at rubyforge.org
> http://rubyforge.org/mailman/listinfo/rspec-users
>

David Chelimsky wrote:
> What''s fast enough?
>   
It''s fast enough, if it doesn''t break my "flow" -
which most likely is
slower than your''s :-)

Tobias

Hello David, all,

2007/1/21, David Chelimsky <dchelimsky at
gmail.com>:
> On 1/20/07, Francois Beausoleil <francois.beausoleil at gmail.com>
wrote:
> > def index
> >   @projects = current_user.projects.active
> > end
>
> I would approach this quite differently. The first difference being I
> would have started with a spec, letting the spec help me to discover
> the interface that I want on User.
>
> def index
>   @projects = current_user.active_projects
> end
>
> ... which would, in turn, lead me to spec out a method named
> active_projects on User.
I agree with you, David, but Rails has this nice facility for allowing
developers to add methods on collection proxies.  The end goal is that
I get only active / inactive projects, right ?  Whatever
implementation floats the boat is good.

Adding #active_projects on User leads to a kind of namespace
pollution, no ?  Really, who knows what projects are active ?  The
Project class, or the User ?

has_many :projects do
  def active
    find(:all, :conditions => [''active = 1''])
  end
end

has_many :projects
def active_projects

end

We''re going off-topic here, but these are good design questions anyway.

Thank you for enlightning me !

Bye !
-- 
Fran?ois Beausoleil
http://blog.teksol.info/
http://piston.rubyforge.org/

Shoot!  Message went away too fast.

has_many :projects
def active_projects
  self.projects.find(:all, :conditions => [''active = 1''])
end

Or another alternative:
class Project
  def self.active
    with_scope(:find => [''active = 1'']) do
      yield
    end
  end
end

In the end, it boils down to preferences, no ?

Thanks !
-- 
Fran?ois Beausoleil
http://blog.teksol.info/
http://piston.rubyforge.org/

I''ve got a question similar to Francois''s.  I''m
writing a to-do-like
application, where each Task has a number of Events, consisting of 
event.date and event.status.  I want to see how long it''s been since
the
Task was last completed.  So:

context "A task completed once on 2001-01-02" do
   setup do
     @task = Task.new
     mock_event = Struct.new(:date, :status)
     @task.stub!(:events).and_return(
       [mock_event.new(Date.parse("2001-01-02"),
"completed")])
   end

   specify "was completed 0 days ago on 2001-01-02" do
     @task.completed_days_ago(Date.parse("2001-01-02")).should_eql 0
   end
end

task.rb looks like:

   def cache_completions
     @completions = {}
     events.each do |event|
       @completions[event.date] = event.status
     end
   end

   def completed_days_ago(todays_date)
     cache_completions unless @completions
     date = todays_date
     while date > todays_date - 7 do
       return (todays_date - date).to_i if @completions[date] ==
"completed"
       date = date - 1
     end

     "never"
   end


This smells a bit.  I could separate part of completed_days_ago into 
last_completed_on(date), but no matter how I slice it I still end up 
mocking Task.cells for some routine.  Suggestions?

Jay

Do partial mocks solve your problem?

http://rspec.rubyforge.org/documentation/mocks/partial_mocks.html

I run into a lot of these issues when spec''ing, and it seems writing  
mocks that cover as many cases as I expect AR to cough up can be  
difficult. Others might have different experiences. I found partial  
mocks intriguing, although I''ve not yet explored them.

Hope I''m not barking up the wrong tree.

--steve

On Jan 21, 2007, at 12:42 PM, Francois Beausoleil wrote:
> Hello David, all,
>
> 2007/1/21, David Chelimsky <dchelimsky at gmail.com>:
>> On 1/20/07, Francois Beausoleil <francois.beausoleil at
gmail.com>
>> wrote:
>>> def index
>>>   @projects = current_user.projects.active
>>> end
>>
>> I would approach this quite differently. The first difference being I
>> would have started with a spec, letting the spec help me to discover
>> the interface that I want on User.
>>
>> def index
>>   @projects = current_user.active_projects
>> end
>>
>> ... which would, in turn, lead me to spec out a method named
>> active_projects on User.
>
> I agree with you, David, but Rails has this nice facility for allowing
> developers to add methods on collection proxies.  The end goal is that
> I get only active / inactive projects, right ?  Whatever
> implementation floats the boat is good.
>
> Adding #active_projects on User leads to a kind of namespace
> pollution, no ?  Really, who knows what projects are active ?  The
> Project class, or the User ?
>
> has_many :projects do
>   def active
>     find(:all, :conditions => [''active = 1''])
>   end
> end
>
> has_many :projects
> def active_projects
>
> end
>
> We''re going off-topic here, but these are good design questions  
> anyway.
>
> Thank you for enlightning me !
>
> Bye !
> -- 
> Fran?ois Beausoleil
> http://blog.teksol.info/
> http://piston.rubyforge.org/
> _______________________________________________
> rspec-users mailing list
> rspec-users at rubyforge.org
> http://rubyforge.org/mailman/listinfo/rspec-users

Francois Beausoleil wrote:
> 
> Adding #active_projects on User leads to a kind of namespace
> pollution, no ?  Really, who knows what projects are active ?  The
> Project class, or the User ?
This seems to be a basic conflict between "Tell, don''t ask"
and
domain-driven design.  DDD, as I understand it, says the Project class 
should encapsulate all knowledge about projects; TDA says we shouldn''t 
ask for the project proxy.  Yes?

Jay

On 1/21/07, Francois Beausoleil <francois.beausoleil at gmail.com>
wrote:
> Hello David, all,
>
> 2007/1/21, David Chelimsky <dchelimsky at gmail.com>:
> > On 1/20/07, Francois Beausoleil <francois.beausoleil at
gmail.com> wrote:
> > > def index
> > >   @projects = current_user.projects.active
> > > end
> >
> > I would approach this quite differently. The first difference being I
> > would have started with a spec, letting the spec help me to discover
> > the interface that I want on User.
> >
> > def index
> >   @projects = current_user.active_projects
> > end
> >
> > ... which would, in turn, lead me to spec out a method named
> > active_projects on User.
>
> I agree with you, David, but Rails has this nice facility for allowing
> developers to add methods on collection proxies.  The end goal is that
> I get only active / inactive projects, right ?  Whatever
> implementation floats the boat is good.
>
> Adding #active_projects on User leads to a kind of namespace
> pollution, no ?  Really, who knows what projects are active ?  The
> Project class, or the User ?
>
> has_many :projects do
>   def active
>     find(:all, :conditions => [''active = 1''])
>   end
> end
>
> has_many :projects
> def active_projects
>
> end
>
> We''re going off-topic here, but these are good design questions
anyway.
RSpec is ALL ABOUT design, so these questions are certainly on topic
(in my view).
>
> Thank you for enlightning me !
>
> Bye !
> --
> Fran?ois Beausoleil
> http://blog.teksol.info/
> http://piston.rubyforge.org/
> _______________________________________________
> rspec-users mailing list
> rspec-users at rubyforge.org
> http://rubyforge.org/mailman/listinfo/rspec-users

Sorry for continuing the divergence, but the design thoughts are very  
interesting.

Couldn''t DDD and TDA be satisfied by:
Project.find_active_projects_for_user(:current_user)

---Brian Yamabe

On Jan 21, 2007, at 12:53 PM, Jay Levitt wrote:
> Francois Beausoleil wrote:
>
>>
>> Adding #active_projects on User leads to a kind of namespace
>> pollution, no ?  Really, who knows what projects are active ?  The
>> Project class, or the User ?
>
> This seems to be a basic conflict between "Tell, don''t
ask" and
> domain-driven design.  DDD, as I understand it, says the Project class
> should encapsulate all knowledge about projects; TDA says we
shouldn''t
> ask for the project proxy.  Yes?
>
> Jay
>
> _______________________________________________
> rspec-users mailing list
> rspec-users at rubyforge.org
> http://rubyforge.org/mailman/listinfo/rspec-users

On 1/21/07, Jay Levitt <lists-rspec at shopwatch.org>
wrote:
> Francois Beausoleil wrote:
>
> >
> > Adding #active_projects on User leads to a kind of namespace
> > pollution, no ?  Really, who knows what projects are active ?  The
> > Project class, or the User ?
>
> This seems to be a basic conflict between "Tell, don''t
ask" and
> domain-driven design.  DDD, as I understand it, says the Project class
> should encapsulate all knowledge about projects; TDA says we
shouldn''t
> ask for the project proxy.  Yes?
TDA says we shouldn''t even ask the User for its Projects. It''s
the Law
of Demeter that says we shouldn''t ask the User for it''s
Projects and
THEN ask the Projects whether they are active.

Just because rails makes it easy doesn''t make it a good design choice.
It really depends on context. If your app is never going to change
requirements, then the problems associated with violations of OO
design principles don''t really matter. They are all about writing
maintainable software.

I''m serious here - I don''t mean to patronize. There are times
when the
application you''re writing will have simple requirements and a short
potential life-span. In those cases, the costs of heeding to design
principles might outweigh the benefits of a flexible design.

That said, imagine that somewhere down the line the requirements
change such that User knows its Projects through some middle man, like
a ProjectManager. Now you''d have to change all the places in the code
that say "user.projects.active" would have to change to
"user.project_manager.projects.active".

Of course at that point you could change #projects to delegate to
project_manager.projects, so clients of User wouldn''t have to change,
but you see the point.

So I''d prefer to not advertise how the User keeps its Projects.

But that''s me. 2 cents worth.
>
> Jay
>
> _______________________________________________
> rspec-users mailing list
> rspec-users at rubyforge.org
> http://rubyforge.org/mailman/listinfo/rspec-users
>

On 1/21/07, Brian Yamabe <brian at yamabe.net>
wrote:
> Sorry for continuing the divergence, but the design thoughts are very
> interesting.
>
> Couldn''t DDD and TDA be satisfied by:
> Project.find_active_projects_for_user(:current_user)
How would that be implemented?

def self.find_active_projects_for_user(user)
  return user.projects.active_projects
end

This bi-directional dependency seems problematic to me. I realize that
rails makes it easy to do these, but again, just because its easy
doesn''t make it a good decision.

This is one of those questions that can be enlightened by client need.
Do clients of Projects ask for their users? Do clients of User ask for
its Projects? If both answers are yes, then maybe the bi-directional
deal makes sense. If only one is needed, then only one should be
implemented.

OK. I''ll shut up now. Go Bears!

David

> ---Brian Yamabe
>
> On Jan 21, 2007, at 12:53 PM, Jay Levitt wrote:
>
> > Francois Beausoleil wrote:
> >
> >>
> >> Adding #active_projects on User leads to a kind of namespace
> >> pollution, no ?  Really, who knows what projects are active ?  The
> >> Project class, or the User ?
> >
> > This seems to be a basic conflict between "Tell, don''t
ask" and
> > domain-driven design.  DDD, as I understand it, says the Project class
> > should encapsulate all knowledge about projects; TDA says we
shouldn''t
> > ask for the project proxy.  Yes?
> >
> > Jay
> >
> > _______________________________________________
> > rspec-users mailing list
> > rspec-users at rubyforge.org
> > http://rubyforge.org/mailman/listinfo/rspec-users
>
> _______________________________________________
> rspec-users mailing list
> rspec-users at rubyforge.org
> http://rubyforge.org/mailman/listinfo/rspec-users
>

On 1/21/07, Jay Levitt <lists-rspec at shopwatch.org>
wrote:
> I''ve got a question similar to Francois''s.  I''m
writing a to-do-like
> application, where each Task has a number of Events, consisting of
> event.date and event.status.  I want to see how long it''s been
since the
> Task was last completed.  So:
>
> context "A task completed once on 2001-01-02" do
>    setup do
>      @task = Task.new
>      mock_event = Struct.new(:date, :status)
>      @task.stub!(:events).and_return(
>        [mock_event.new(Date.parse("2001-01-02"),
"completed")])
>    end
>
>    specify "was completed 0 days ago on 2001-01-02" do
>     
@task.completed_days_ago(Date.parse("2001-01-02")).should_eql 0
>    end
> end
>
> task.rb looks like:
>
>    def cache_completions
>      @completions = {}
>      events.each do |event|
>        @completions[event.date] = event.status
>      end
>    end
>
>    def completed_days_ago(todays_date)
>      cache_completions unless @completions
>      date = todays_date
>      while date > todays_date - 7 do
>        return (todays_date - date).to_i if @completions[date] ==
"completed"
>        date = date - 1
>      end
>
>      "never"
>    end
>
>
> This smells a bit.  I could separate part of completed_days_ago into
> last_completed_on(date), but no matter how I slice it I still end up
> mocking Task.cells for some routine.  Suggestions?
Ah, how I love these questions. Thanks Jay!

So first of all, lets think about what we REALLY want the interface to
Task to be. What you have there is:

  @task.completed_days_ago(date)

In your description, you say "how long it''s been since the Task
was
last completed". That leads me to a method name like this:

  @task.days_since_completed(date)

The next question is, why are we passing in the date? We want to how
many days have passed since it was completed, not how many days
between completion and some arbitrary date. Now this gets tricky
because the method becomes more testable if you can supply the date,
but supplying the date doesn''t "feel" right in the context of
"days_since_completed".
>From here, I see a couple of ways to go. One would be to stub Time.new
(or local, or whatever method you wish to use to generate current
time) and leave the method with no argument:

context "A task completed once on 2001-02-03" do
  setup do
    event_stub = Struct.new(:date,
:status).new(Time.parse("2001-02-03"), "completed")
    Time.stub!(:new).and_return(Time.parse("2001-02-03"))

    @task = Task.new
    @task.stub!(:events).and_return([event_stub])
  end

  specify "was completed 0 days ago on 2001-02-03" do
    @task.days_since_completed.should_eql 0
  end
end

Another would be to change the meaning of the method to something like:

  @task.days_from_completion_date_to(date)

No perfect answer here. Just food for thought.

Cheers,
David

>
> Jay
>
>
> _______________________________________________
> rspec-users mailing list
> rspec-users at rubyforge.org
> http://rubyforge.org/mailman/listinfo/rspec-users
>

David Chelimsky wrote:
> On 1/21/07, Jay Levitt <lists-rspec at shopwatch.org> wrote:
>> I''ve got a question similar to Francois''s. 
I''m writing a to-do-like
>> application, where each Task has a number of Events, consisting of
>> event.date and event.status.  I want to see how long it''s been
since the
>> Task was last completed.  So:
[snip]
>> This smells a bit.  I could separate part of completed_days_ago into
>> last_completed_on(date), but no matter how I slice it I still end up
>> mocking Task.cells for some routine.  Suggestions?
> 
> The next question is, why are we passing in the date? We want to how
> many days have passed since it was completed, not how many days
> between completion and some arbitrary date. 
Actually, we want the latter - but of course you couldn''t tell that
from
my method name!  The tasks are repeating, and this app fills in a 
spreadsheet-like series of cells, one per day, with various colors, 
depending on how often you''re completing a task.  So we actually need
to
check the task''s status on any given day.

[..]
>     event_stub = Struct.new(:date,
> :status).new(Time.parse("2001-02-03"), "completed")
>     @task.stub!(:events).and_return([event_stub])
[..]

You don''t mind stubbing the association proxy as a struct?  That was my
real question; it seems to lead down a messy road of trying to stub 
every ActiveRecord method.  On the one hand, it seems like BDD says I 
should stub or mock any object other than the one under test.  On the 
other, since I''m still in the models realm, maybe it''s okay to
use
fixtures to represent the model relations.  Or maybe there''s a third
way.

Jay

David Chelimsky wrote:
> TDA says we shouldn''t even ask the User for its Projects.
It''s the Law
> of Demeter that says we shouldn''t ask the User for it''s
Projects and
> THEN ask the Projects whether they are active.
Oops, my bad, then!  So is it my misunderstanding of domain-driven 
design - especially the AGGREGATE pattern - or is TDA diametrically 
opposed to DDD here?  Can we get Alec Sharp and Eric Evans in a boxing ring?

Jay

David, thanks for making me think. If the bi-directional dependency  
is needed, would it be cleaned up with a "has_many :through" (An  
Assignment class as an example)?
> def self.find_active_projects_for_user(user)
>   return user.projects.active_projects
> end
>
> This bi-directional dependency seems problematic to me. I realize that
> rails makes it easy to do these, but again, just because its easy
> doesn''t make it a good decision.
>
> This is one of those questions that can be enlightened by client need.
> Do clients of Projects ask for their users? Do clients of User ask for
> its Projects? If both answers are yes, then maybe the bi-directional
> deal makes sense. If only one is needed, then only one should be
> implemented.
>
> OK. I''ll shut up now. Go Bears!
>
> David
>
>
>> ---Brian Yamabe
>>
>> On Jan 21, 2007, at 12:53 PM, Jay Levitt wrote:
>>
>>> Francois Beausoleil wrote:
>>>
>>>>
>>>> Adding #active_projects on User leads to a kind of namespace
>>>> pollution, no ?  Really, who knows what projects are active ? 
The
>>>> Project class, or the User ?
>>>
>>> This seems to be a basic conflict between "Tell,
don''t ask" and
>>> domain-driven design.  DDD, as I understand it, says the Project  
>>> class
>>> should encapsulate all knowledge about projects; TDA says we  
>>> shouldn''t
>>> ask for the project proxy.  Yes?
>>>
>>> Jay
>>>
>>> _______________________________________________
>>> rspec-users mailing list
>>> rspec-users at rubyforge.org
>>> http://rubyforge.org/mailman/listinfo/rspec-users
>>
>> _______________________________________________
>> rspec-users mailing list
>> rspec-users at rubyforge.org
>> http://rubyforge.org/mailman/listinfo/rspec-users
>>
> _______________________________________________
> rspec-users mailing list
> rspec-users at rubyforge.org
> http://rubyforge.org/mailman/listinfo/rspec-users

On 1/21/07, Jay Levitt <lists-rspec at shopwatch.org>
wrote:
> David Chelimsky wrote:
> > TDA says we shouldn''t even ask the User for its Projects.
It''s the Law
> > of Demeter that says we shouldn''t ask the User for
it''s Projects and
> > THEN ask the Projects whether they are active.
>
> Oops, my bad, then!  So is it my misunderstanding of domain-driven
> design - especially the AGGREGATE pattern - or is TDA diametrically
> opposed to DDD here?  Can we get Alec Sharp and Eric Evans in a boxing
ring?
No - I think that''s right. My read is that aggregate looks like the
antithesis of LoD, BUT, there is an important distinction to be made.
In DDD, the goal is ubiquitous language built around the business
domain. If the business domain says that there are Leagues that have
Conferences that have Teams that have Players, then those
relationships are very unlikely to change, and therefore minimize the
risks assoicated with LoD violations.

The real problem w/ LoD is when you see stuff like:

  player = team.jersey_number_to_player_map[17]

as opposed to

  player = team.find_player_by_jersey_number[17]

Even though it''s not obvious, the first is an LoD violation. Here are
the same statements in java:

  player = team.getJerseyNumberToPlayerMap().get("17")

as opposed to

  player = team.findPlayerByJerseyNumber("17")

The fact that team stores players in a map is none of anybody''s
business. The fact that it stores players is arguably a domain concept
that makes sense.

Make sense?

David

>
> Jay
>
> _______________________________________________
> rspec-users mailing list
> rspec-users at rubyforge.org
> http://rubyforge.org/mailman/listinfo/rspec-users
>

On 1/21/07, Brian Yamabe <brian at yamabe.net>
wrote:
> David, thanks for making me think.
Back atcha.
> If the bi-directional dependency
> is needed, would it be cleaned up with a "has_many :through" (An
> Assignment class as an example)?
That happens to work in a lot of cases because rails supports it. But
bear in mind that :through relationships weren''t supported in 1.0.
They came later because the problem of a particular type of
relationship came up enough to do something about it in the framework.

I''m sure that sooner or later you''re going to come up w/ a
model that
isn''t supported directly by rails, and then you need to think about
all this stuff.

<soapbox>
Also, while this may be far from most people''s minds, letting all of
this AR goodness seep out past your model classes binds your
application to AR. There are other persistence frameworks, like OG,
that are promising. If you decided to give on a try, you''d have a much
easier time if you encapsulate all of that goodness inside your models
and expose methods that are framework neutral.

Not saying that you shouldn''t exploit AR goodness. Just that
localizing the exploitation makes things more flexible in the long
run.
</soapbox>
>
> > def self.find_active_projects_for_user(user)
> >   return user.projects.active_projects
> > end
> >
> > This bi-directional dependency seems problematic to me. I realize that
> > rails makes it easy to do these, but again, just because its easy
> > doesn''t make it a good decision.
> >
> > This is one of those questions that can be enlightened by client need.
> > Do clients of Projects ask for their users? Do clients of User ask for
> > its Projects? If both answers are yes, then maybe the bi-directional
> > deal makes sense. If only one is needed, then only one should be
> > implemented.
> >
> > OK. I''ll shut up now. Go Bears!
> >
> > David
> >
> >
> >> ---Brian Yamabe
> >>
> >> On Jan 21, 2007, at 12:53 PM, Jay Levitt wrote:
> >>
> >>> Francois Beausoleil wrote:
> >>>
> >>>>
> >>>> Adding #active_projects on User leads to a kind of
namespace
> >>>> pollution, no ?  Really, who knows what projects are
active ?  The
> >>>> Project class, or the User ?
> >>>
> >>> This seems to be a basic conflict between "Tell,
don''t ask" and
> >>> domain-driven design.  DDD, as I understand it, says the
Project
> >>> class
> >>> should encapsulate all knowledge about projects; TDA says we
> >>> shouldn''t
> >>> ask for the project proxy.  Yes?
> >>>
> >>> Jay
> >>>
> >>> _______________________________________________
> >>> rspec-users mailing list
> >>> rspec-users at rubyforge.org
> >>> http://rubyforge.org/mailman/listinfo/rspec-users
> >>
> >> _______________________________________________
> >> rspec-users mailing list
> >> rspec-users at rubyforge.org
> >> http://rubyforge.org/mailman/listinfo/rspec-users
> >>
> > _______________________________________________
> > rspec-users mailing list
> > rspec-users at rubyforge.org
> > http://rubyforge.org/mailman/listinfo/rspec-users
>
> _______________________________________________
> rspec-users mailing list
> rspec-users at rubyforge.org
> http://rubyforge.org/mailman/listinfo/rspec-users
>

David Chelimsky wrote:
> The fact that team stores players in a map is none of anybody''s
> business. The fact that it stores players is arguably a domain concept
> that makes sense.
Great distinction... thanks.

Jay

Hi all,

2007/1/21, David Chelimsky <dchelimsky at
gmail.com>:
> <soapbox>
> Also, while this may be far from most people''s minds, letting all
of
> this AR goodness seep out past your model classes binds your
> application to AR. There are other persistence frameworks, like OG,
> that are promising. If you decided to give on a try, you''d have a
much
> easier time if you encapsulate all of that goodness inside your models
> and expose methods that are framework neutral.
>
> Not saying that you shouldn''t exploit AR goodness. Just that
> localizing the exploitation makes things more flexible in the long
> run.
> </soapbox>
So in essence, you are arguing for current_user.active_projects
instead of anything else, right :)

In my application, projects exist only in relationship to users.  And
users access projects through a Role.  That wasn''t relevant in the
initial discussions, so I didn''t speak about it.

Thanks all !
-- 
Fran?ois Beausoleil
http://blog.teksol.info/
http://piston.rubyforge.org/

> I''m sure that sooner or later you''re going to come up w/
a model that
> isn''t supported directly by rails, and then you need to think
about
> all this stuff.
>
> <soapbox>
> Also, while this may be far from most people''s minds, letting all
of
> this AR goodness seep out past your model classes binds your
> application to AR. There are other persistence frameworks, like OG,
> that are promising. If you decided to give on a try, you''d have a
much
> easier time if you encapsulate all of that goodness inside your models
> and expose methods that are framework neutral.
>
> Not saying that you shouldn''t exploit AR goodness. Just that
> localizing the exploitation makes things more flexible in the long
> run.
> </soapbox>
Even before I got your <soapbox/> I was thinking about what to do if
this weren''t using Rails. I still like the notion of an intermediary;  
a class
that manages the relationship between Users and Projects. This would
minimize any coupling between the User and Project classes and allow
for a richer set of relationships to be tracked (inactive project  
members,
inactive user projects, etc.). The intermediary class could either  
exploit the
"AR goodness" or not. I could hear someone crying, "YAGNI,"
but the
decoupling of User and Project is the main concern.  The flexibility  
comes
almost as a side effect.

---Brian Yamabe

On 1/21/07, Jay Levitt <lists-rspec at shopwatch.org>
wrote:
> David Chelimsky wrote:
> > On 1/21/07, Jay Levitt <lists-rspec at shopwatch.org> wrote:
> >> I''ve got a question similar to Francois''s. 
I''m writing a to-do-like
> >> application, where each Task has a number of Events, consisting of
> >> event.date and event.status.  I want to see how long it''s
been since the
> >> Task was last completed.  So:
>
> [snip]
>
> >> This smells a bit.  I could separate part of completed_days_ago
into
> >> last_completed_on(date), but no matter how I slice it I still end
up
> >> mocking Task.cells for some routine.  Suggestions?
> >
> > The next question is, why are we passing in the date? We want to how
> > many days have passed since it was completed, not how many days
> > between completion and some arbitrary date.
>
> Actually, we want the latter - but of course you couldn''t tell
that from
> my method name!  The tasks are repeating, and this app fills in a
> spreadsheet-like series of cells, one per day, with various colors,
> depending on how often you''re completing a task.  So we actually
need to
> check the task''s status on any given day.
>
> [..]
> >     event_stub = Struct.new(:date,
> > :status).new(Time.parse("2001-02-03"),
"completed")
> >     @task.stub!(:events).and_return([event_stub])
> [..]
>
> You don''t mind stubbing the association proxy as a struct?  That
was my
> real question; it seems to lead down a messy road of trying to stub
> every ActiveRecord method.  On the one hand, it seems like BDD says I
> should stub or mock any object other than the one under test.  On the
> other, since I''m still in the models realm, maybe it''s
okay to use
> fixtures to represent the model relations.  Or maybe there''s a
third way.
I''m kind of on the fence on this topic. I''ve been of the
mindset that
since models are so tightly bound to AR that it''s more trouble than
its worth to try and break that dependency. Because I mock models in
all of my controller and view specs, I''m only hitting the db for the
things that are interesting about models.

Recently, I read a post from Jay Fields where he talks about what
amounts to mocking out the loading of a file. So if you want to spec
that User should validate presence of email, you do this:

User.should_receive(:validates_presence_of).with(:email)
load "${RAILS_ROOT}/app/models/user.rb"

Odd, huh? It wouldn''t be if the API for AR was like this:

@model_registry.should_receive(:validate_presence_of).with(User, :email)
User.register(@model_registry)

Essentially, loading the file IS the entry point to the AR API.
Interesting point of view, no?

I like two things about this:
clear specs
fast specs

I can write all my validation specs in two lines each. No models to
create. No database to connect to. Pretty awesome.

So, no, I don''t mind mocking the proxy in model specs, though I would
always try to avoid that seeping out to the controllers and views.

That helpful?

Cheers,
David

>
> Jay
>
> _______________________________________________
> rspec-users mailing list
> rspec-users at rubyforge.org
> http://rubyforge.org/mailman/listinfo/rspec-users
>

David Chelimsky wrote:
> I''m kind of on the fence on this topic. I''ve been of the
mindset that
> since models are so tightly bound to AR that it''s more trouble
than
> its worth to try and break that dependency. Because I mock models in
> all of my controller and view specs, I''m only hitting the db for
the
> things that are interesting about models.
I think I''m of the same mind; I just wanted to make sure I
wasn''t taking
the easy way out.

I was thinking... a great addition to rSpec someday - though a major 
project in its own right - might be a pre-stubbed version of 
ActiveRecord with fixtures.  Then again, that might be just as complex 
as the real AR, and maybe the solution is to just use an in-memory 
database when testing.
> 
> Recently, I read a post from Jay Fields where he talks about what
> amounts to mocking out the loading of a file. So if you want to spec
> that User should validate presence of email, you do this:
> 
> User.should_receive(:validates_presence_of).with(:email)
> load "${RAILS_ROOT}/app/models/user.rb"
> 
> Odd, huh? It wouldn''t be if the API for AR was like this:
> 
> @model_registry.should_receive(:validate_presence_of).with(User, :email)
> User.register(@model_registry)
> 
> Essentially, loading the file IS the entry point to the AR API.
> Interesting point of view, no?
Wow - I never thought of it like that.  I think (as a new convert to 
mocks) that type of mock is a little too specific and un-DRY for my 
taste, though; it feels like specifying that "line 6 of method foo 
should be an if statement".  There''s double-entry bookkeeping, and
then
there''s keeping two sets of books.
> So, no, I don''t mind mocking the proxy in model specs, though I
would
> always try to avoid that seeping out to the controllers and views.
> 
> That helpful?
Very!

Jay Levitt wrote:
> David Chelimsky wrote:
>> User.should_receive(:validates_presence_of).with(:email)
> Wow - I never thought of it like that.  I think (as a new convert to 
> mocks) that type of mock is a little too specific and un-DRY for my 
> taste, though; it feels like specifying that "line 6 of method foo 
> should be an if statement".  There''s double-entry
bookkeeping, and then
> there''s keeping two sets of books.
While I''m out on this limb, I''m gonna say that, given the way
Rails
validations are structured, it''s better to use state-based testing than
interaction-based testing!

validates_presence_of is a bad example, because the two methods are 
practically interchangeable.  But consider a validation that uses a 
regex to verify a legal IP address.  Do you want your specs to repeat 
the regex, or do you want to test various legal and illegal IP-address 
strings and see what breaks?  To me, it''s the second one
that''s actually
testing the behavior of the application.


Jay

On 1/24/07, Jay Levitt <lists-rspec at shopwatch.org>
wrote:
> Jay Levitt wrote:
> > David Chelimsky wrote:
> >> User.should_receive(:validates_presence_of).with(:email)
> > Wow - I never thought of it like that.  I think (as a new convert to
> > mocks) that type of mock is a little too specific and un-DRY for my
> > taste, though; it feels like specifying that "line 6 of method
foo
> > should be an if statement".  There''s double-entry
bookkeeping, and then
> > there''s keeping two sets of books.
>
> While I''m out on this limb, I''m gonna say that, given the
way Rails
> validations are structured, it''s better to use state-based testing
than
> interaction-based testing!
>
> validates_presence_of is a bad example, because the two methods are
> practically interchangeable.  But consider a validation that uses a
> regex to verify a legal IP address.  Do you want your specs to repeat
> the regex, or do you want to test various legal and illegal IP-address
> strings and see what breaks?  To me, it''s the second one
that''s actually
> testing the behavior of the application.
Another view would be that the Regexp is a separate component that
you''d want to test separately from the use of that component. So the
test that your model validates_format_of using a Regexp uses the right
one and then have other tests just for that Regexp.

WDYT?
>
>
> Jay
>
> _______________________________________________
> rspec-users mailing list
> rspec-users at rubyforge.org
> http://rubyforge.org/mailman/listinfo/rspec-users
>

David Chelimsky wrote:
> On 1/24/07, Jay Levitt <lists-rspec at shopwatch.org> wrote:
>> validates_presence_of is a bad example, because the two methods are
>> practically interchangeable.  But consider a validation that uses a
>> regex to verify a legal IP address.  Do you want your specs to repeat
>> the regex, or do you want to test various legal and illegal IP-address
>> strings and see what breaks?  To me, it''s the second one
that''s actually
>> testing the behavior of the application.
> 
> Another view would be that the Regexp is a separate component that
> you''d want to test separately from the use of that component. So
the
> test that your model validates_format_of using a Regexp uses the right
> one and then have other tests just for that Regexp.
> 
> WDYT?
Hmm.. I can see your argument, but I''m not convinced.  Perhaps more 
experience will convince me :)

Jay

On 1/24/07, Jay Levitt <lists-rspec at shopwatch.org>
wrote:
> David Chelimsky wrote:
> > On 1/24/07, Jay Levitt <lists-rspec at shopwatch.org> wrote:
> >> validates_presence_of is a bad example, because the two methods
are
> >> practically interchangeable.  But consider a validation that uses
a
> >> regex to verify a legal IP address.  Do you want your specs to
repeat
> >> the regex, or do you want to test various legal and illegal
IP-address
> >> strings and see what breaks?  To me, it''s the second one
that''s actually
> >> testing the behavior of the application.
> >
> > Another view would be that the Regexp is a separate component that
> > you''d want to test separately from the use of that component.
So the
> > test that your model validates_format_of using a Regexp uses the right
> > one and then have other tests just for that Regexp.
> >
> > WDYT?
>
> Hmm.. I can see your argument, but I''m not convinced.  Perhaps
more
> experience will convince me :)
FWIW, I''m not convinced either. It was just an idea for the purpose of
discussion.

>
> Jay
>
> _______________________________________________
> rspec-users mailing list
> rspec-users at rubyforge.org
> http://rubyforge.org/mailman/listinfo/rspec-users
>

David Chelimsky wrote:
>> Jay Levitt wrote:
>>
>> validates_presence_of is a bad example, because the two methods are
>> practically interchangeable.  But consider a validation that uses a
>> regex to verify a legal IP address.  Do you want your specs to repeat
>> the regex, or do you want to test various legal and illegal IP-address
>> strings and see what breaks?  To me, it''s the second one
that''s actually
>> testing the behavior of the application.
> 
> Another view would be that the Regexp is a separate component that
> you''d want to test separately from the use of that component. So
the
> test that your model validates_format_of using a Regexp uses the right
> one and then have other tests just for that Regexp.
I''d counter that the Regexp is essentially a private method that 
shouldn''t be tested.  It''s never directly used by the outside
world;
only the validation is.

Jay

Why shouldn''t private methods be tested? I use Object#send all the
time to test private methods, otherwise I''ve got the idea the step to
implement the spec is too big, for example look at the specs of my
SMSer project (http://pastie.caboo.se/36044).

On 1/26/07, Jay Levitt <lists-rspec at shopwatch.org>
wrote:
> David Chelimsky wrote:
> >> Jay Levitt wrote:
> >>
> >> validates_presence_of is a bad example, because the two methods
are
> >> practically interchangeable.  But consider a validation that uses
a
> >> regex to verify a legal IP address.  Do you want your specs to
repeat
> >> the regex, or do you want to test various legal and illegal
IP-address
> >> strings and see what breaks?  To me, it''s the second one
that''s actually
> >> testing the behavior of the application.
> >
> > Another view would be that the Regexp is a separate component that
> > you''d want to test separately from the use of that component.
So the
> > test that your model validates_format_of using a Regexp uses the right
> > one and then have other tests just for that Regexp.
>
> I''d counter that the Regexp is essentially a private method that
> shouldn''t be tested.  It''s never directly used by the
outside world;
> only the validation is.
>
> Jay
>
> _______________________________________________
> rspec-users mailing list
> rspec-users at rubyforge.org
> http://rubyforge.org/mailman/listinfo/rspec-users
>

On 1/27/07, Matthijs Langenberg <mlangenberg at gmail.com>
wrote:
> Why shouldn''t private methods be tested? I use Object#send all the
> time to test private methods, otherwise I''ve got the idea the step
to
> implement the spec is too big, for example look at the specs of my
> SMSer project (http://pastie.caboo.se/36044).
There is a school of thought that suggests that if you need to test
privates that you''re probably doing too much in the one class and you
should consider breaking things out. Admittedly, this school of
thought comes from static languages in which you have to go through
crazy and error prone hoops in order to test privates. Since Ruby
makes it so simple, it seems less harmful and risky.

It should, however, be at least a red flag about your design. It
should make you ask "why am I testing privates?".

The questions I have are:

What is the impact of extracting the error code?
Does doing so change some other behaviour?
Isn''t it enough to spec "should raise an exception if the
webservice
returned an error"?

Getting back to my earlier statement about breaking things out, you
*could* (and I likely would) choose to make a new class SmsResponse
that gets initialized with the xml response text and exposes :code and
:message. Then you could eliminate the last two specs in your example
and have a separate spec just for this new class:
http://pastie.caboo.se/36071

The cost of this is an extra class. The benefit is good separation of
concerns (parsing XML vs processing a logical response).

Similarly, the product of :create_post_data must go somewhere, right?
What effect does that have on behavior? If it simply gets stored, then
you can mock out the storage to make sure it gets stored correctly. If
it changes the behaviour of the SMSer, then you can have examples that
show the different behaviours.

All of this said, these principles exist because people have
experienced pain from doing things. You may be absolutely fine w/ what
you have. The risk that you run is that business rules about SMSer and
the XML schema for the response change independently, and then you
have two reasons to change the same class (violation of the Single
Responsibility Principle), making it more prone to ripple effects.

An other thing to consider is whether there are other parts of your
system that have to parse the same XML. If that is the case, then its
definitely to your benefit to break out an SmsResponse class.

WDYT?

David



>
> On 1/26/07, Jay Levitt <lists-rspec at shopwatch.org> wrote:
> > David Chelimsky wrote:
> > >> Jay Levitt wrote:
> > >>
> > >> validates_presence_of is a bad example, because the two
methods are
> > >> practically interchangeable.  But consider a validation that
uses a
> > >> regex to verify a legal IP address.  Do you want your specs
to repeat
> > >> the regex, or do you want to test various legal and illegal
IP-address
> > >> strings and see what breaks?  To me, it''s the second
one that''s actually
> > >> testing the behavior of the application.
> > >
> > > Another view would be that the Regexp is a separate component
that
> > > you''d want to test separately from the use of that
component. So the
> > > test that your model validates_format_of using a Regexp uses the
right
> > > one and then have other tests just for that Regexp.
> >
> > I''d counter that the Regexp is essentially a private method
that
> > shouldn''t be tested.  It''s never directly used by
the outside world;
> > only the validation is.
> >
> > Jay
> >
> > _______________________________________________
> > rspec-users mailing list
> > rspec-users at rubyforge.org
> > http://rubyforge.org/mailman/listinfo/rspec-users
> >
> _______________________________________________
> rspec-users mailing list
> rspec-users at rubyforge.org
> http://rubyforge.org/mailman/listinfo/rspec-users
>

David, thanks for your reply. I''ve made some changes to my specs and
code, but I''m still not really sure where I should move the private
''create_post_data'' method, should there be a seperate class
for that
method to?
Same question for the ''send_http_post_request'' method,
although I
think it''s a good idea to put that method in a different class /
context, allowing me to write a sample webservice to assert the http
post.
This is currently my spec and code: http://pastie.caboo.se/36900

- Matthijs

On 1/27/07, David Chelimsky <dchelimsky at gmail.com>
wrote:
> On 1/27/07, Matthijs Langenberg <mlangenberg at gmail.com> wrote:
> > Why shouldn''t private methods be tested? I use Object#send
all the
> > time to test private methods, otherwise I''ve got the idea the
step to
> > implement the spec is too big, for example look at the specs of my
> > SMSer project (http://pastie.caboo.se/36044).
>
> There is a school of thought that suggests that if you need to test
> privates that you''re probably doing too much in the one class and
you
> should consider breaking things out. Admittedly, this school of
> thought comes from static languages in which you have to go through
> crazy and error prone hoops in order to test privates. Since Ruby
> makes it so simple, it seems less harmful and risky.
>
> It should, however, be at least a red flag about your design. It
> should make you ask "why am I testing privates?".
>
> The questions I have are:
>
> What is the impact of extracting the error code?
> Does doing so change some other behaviour?
> Isn''t it enough to spec "should raise an exception if the
webservice
> returned an error"?
>
> Getting back to my earlier statement about breaking things out, you
> *could* (and I likely would) choose to make a new class SmsResponse
> that gets initialized with the xml response text and exposes :code and
> :message. Then you could eliminate the last two specs in your example
> and have a separate spec just for this new class:
> http://pastie.caboo.se/36071
>
> The cost of this is an extra class. The benefit is good separation of
> concerns (parsing XML vs processing a logical response).
>
> Similarly, the product of :create_post_data must go somewhere, right?
> What effect does that have on behavior? If it simply gets stored, then
> you can mock out the storage to make sure it gets stored correctly. If
> it changes the behaviour of the SMSer, then you can have examples that
> show the different behaviours.
>
> All of this said, these principles exist because people have
> experienced pain from doing things. You may be absolutely fine w/ what
> you have. The risk that you run is that business rules about SMSer and
> the XML schema for the response change independently, and then you
> have two reasons to change the same class (violation of the Single
> Responsibility Principle), making it more prone to ripple effects.
>
> An other thing to consider is whether there are other parts of your
> system that have to parse the same XML. If that is the case, then its
> definitely to your benefit to break out an SmsResponse class.
>
> WDYT?
>
> David
>
>
>
>
> >
> > On 1/26/07, Jay Levitt <lists-rspec at shopwatch.org> wrote:
> > > David Chelimsky wrote:
> > > >> Jay Levitt wrote:
> > > >>
> > > >> validates_presence_of is a bad example, because the two
methods are
> > > >> practically interchangeable.  But consider a validation
that uses a
> > > >> regex to verify a legal IP address.  Do you want your
specs to repeat
> > > >> the regex, or do you want to test various legal and
illegal IP-address
> > > >> strings and see what breaks?  To me, it''s the
second one that''s actually
> > > >> testing the behavior of the application.
> > > >
> > > > Another view would be that the Regexp is a separate
component that
> > > > you''d want to test separately from the use of that
component. So the
> > > > test that your model validates_format_of using a Regexp uses
the right
> > > > one and then have other tests just for that Regexp.
> > >
> > > I''d counter that the Regexp is essentially a private
method that
> > > shouldn''t be tested.  It''s never directly used
by the outside world;
> > > only the validation is.
> > >
> > > Jay
> > >
> > > _______________________________________________
> > > rspec-users mailing list
> > > rspec-users at rubyforge.org
> > > http://rubyforge.org/mailman/listinfo/rspec-users
> > >
> > _______________________________________________
> > rspec-users mailing list
> > rspec-users at rubyforge.org
> > http://rubyforge.org/mailman/listinfo/rspec-users
> >
> _______________________________________________
> rspec-users mailing list
> rspec-users at rubyforge.org
> http://rubyforge.org/mailman/listinfo/rspec-users
>

On 1/31/07, Matthijs Langenberg <mlangenberg at gmail.com>
wrote:
> David, thanks for your reply. I''ve made some changes to my specs
and
> code, but I''m still not really sure where I should move the
private
> ''create_post_data'' method, should there be a seperate
class for that
> method to?
> Same question for the ''send_http_post_request'' method,
although I
> think it''s a good idea to put that method in a different class /
> context, allowing me to write a sample webservice to assert the http
> post.
> This is currently my spec and code: http://pastie.caboo.se/36900
This line is the perfect place to introduce a new class:

@sms_response = SMSResponse.new(send_http_post_request(SERVICE_URI,
create_post_data(dutch_mobile, msg)))

How about this:

@sms_response = SMSRequest.new(dutch_mobile, msg).post(SERVICE_URI)

and then moving the creation of the SMSResponse to the SMSRequest?
This applies TDA and separates things nicely.

WDYT?
>
> - Matthijs
>
> On 1/27/07, David Chelimsky <dchelimsky at gmail.com> wrote:
> > On 1/27/07, Matthijs Langenberg <mlangenberg at gmail.com>
wrote:
> > > Why shouldn''t private methods be tested? I use
Object#send all the
> > > time to test private methods, otherwise I''ve got the
idea the step to
> > > implement the spec is too big, for example look at the specs of
my
> > > SMSer project (http://pastie.caboo.se/36044).
> >
> > There is a school of thought that suggests that if you need to test
> > privates that you''re probably doing too much in the one class
and you
> > should consider breaking things out. Admittedly, this school of
> > thought comes from static languages in which you have to go through
> > crazy and error prone hoops in order to test privates. Since Ruby
> > makes it so simple, it seems less harmful and risky.
> >
> > It should, however, be at least a red flag about your design. It
> > should make you ask "why am I testing privates?".
> >
> > The questions I have are:
> >
> > What is the impact of extracting the error code?
> > Does doing so change some other behaviour?
> > Isn''t it enough to spec "should raise an exception if
the webservice
> > returned an error"?
> >
> > Getting back to my earlier statement about breaking things out, you
> > *could* (and I likely would) choose to make a new class SmsResponse
> > that gets initialized with the xml response text and exposes :code and
> > :message. Then you could eliminate the last two specs in your example
> > and have a separate spec just for this new class:
> > http://pastie.caboo.se/36071
> >
> > The cost of this is an extra class. The benefit is good separation of
> > concerns (parsing XML vs processing a logical response).
> >
> > Similarly, the product of :create_post_data must go somewhere, right?
> > What effect does that have on behavior? If it simply gets stored, then
> > you can mock out the storage to make sure it gets stored correctly. If
> > it changes the behaviour of the SMSer, then you can have examples that
> > show the different behaviours.
> >
> > All of this said, these principles exist because people have
> > experienced pain from doing things. You may be absolutely fine w/ what
> > you have. The risk that you run is that business rules about SMSer and
> > the XML schema for the response change independently, and then you
> > have two reasons to change the same class (violation of the Single
> > Responsibility Principle), making it more prone to ripple effects.
> >
> > An other thing to consider is whether there are other parts of your
> > system that have to parse the same XML. If that is the case, then its
> > definitely to your benefit to break out an SmsResponse class.
> >
> > WDYT?
> >
> > David
> >
> >
> >
> >
> > >
> > > On 1/26/07, Jay Levitt <lists-rspec at shopwatch.org>
wrote:
> > > > David Chelimsky wrote:
> > > > >> Jay Levitt wrote:
> > > > >>
> > > > >> validates_presence_of is a bad example, because the
two methods are
> > > > >> practically interchangeable.  But consider a
validation that uses a
> > > > >> regex to verify a legal IP address.  Do you want
your specs to repeat
> > > > >> the regex, or do you want to test various legal and
illegal IP-address
> > > > >> strings and see what breaks?  To me, it''s
the second one that''s actually
> > > > >> testing the behavior of the application.
> > > > >
> > > > > Another view would be that the Regexp is a separate
component that
> > > > > you''d want to test separately from the use of
that component. So the
> > > > > test that your model validates_format_of using a Regexp
uses the right
> > > > > one and then have other tests just for that Regexp.
> > > >
> > > > I''d counter that the Regexp is essentially a
private method that
> > > > shouldn''t be tested.  It''s never directly
used by the outside world;
> > > > only the validation is.
> > > >
> > > > Jay
> > > >
> > > > _______________________________________________
> > > > rspec-users mailing list
> > > > rspec-users at rubyforge.org
> > > > http://rubyforge.org/mailman/listinfo/rspec-users
> > > >
> > > _______________________________________________
> > > rspec-users mailing list
> > > rspec-users at rubyforge.org
> > > http://rubyforge.org/mailman/listinfo/rspec-users
> > >
> > _______________________________________________
> > rspec-users mailing list
> > rspec-users at rubyforge.org
> > http://rubyforge.org/mailman/listinfo/rspec-users
> >
> _______________________________________________
> rspec-users mailing list
> rspec-users at rubyforge.org
> http://rubyforge.org/mailman/listinfo/rspec-users
>