Ruby Forum RSpec > Example for attr_accessible?

Is anyone out there writing specs to check attr_accessible fields? I had
originally written my spec to check for allowing the desired fields, and
then none of the other regular db fields. Unfortunately this isn't
satisfactory, because attr_protected could have been used instead, which
of course wouldn't prevent mass assignment to any whatever=(val) method.

I'm thinking of maybe some internal Rails variable that holds an array 
of
accessible fields maybe? Does anyone know if such a thing exists? Just 
to
make it clear, I don't want to test Rails, I want to check that the
accessible fields have been set properly. Would it be possible to mock 
or
some such to check for a call to attr_accessible?

Thanks,
Steve

On 10/15/07, Steve <vertebrate@gmail.com> wrote:
> some such to check for a call to attr_accessible?
>
> Thanks,
> Steve
>
> _______________________________________________
> rspec-users mailing list
> rspec-users@rubyforge.org
> http://rubyforge.org/mailman/listinfo/rspec-users
>

You could expect a call to attr_accessible and then load your model 
file.

Or, loop through all the column names in your model, and make sure
that only the ones that are supposed to be accessible get set.

Pat

> 
> You could expect a call to attr_accessible and then load your model file.
> 
> Or, loop through all the column names in your model, and make sure
> that only the ones that are supposed to be accessible get set.
> 
> Pat

Expecting the call is what I had originally thought of, but couldn't
figure out how to write that test. Isn't attr_accessible called when you
create a new instance, so there wouldn't be time to setup an expectation
for the call?

On 10/16/07, Steve <vertebrate@gmail.com> wrote:
> create a new instance, so there wouldn't be time to setup an expectation
> for the call?

describe Chicken do
  it "should make only :name and :age attr_accessible" do
    Chicken.should_receive(:attr_accessible).with(:name, :age)
    load "#{RAILS_ROOT}/app/models/chicken.rb"
  end
end

I first saw this technique described by David Chelimsky at
http://rubyforge.org/pipermail/rspec-users/2007-September/002965.html

Pat

On 10/16/07, Pat Maddox <pergesu@gmail.com> wrote:
> describe Chicken do
>   it "should make only :name and :age attr_accessible" do
>     Chicken.should_receive(:attr_accessible).with(:name, :age)
>     load "#{RAILS_ROOT}/app/models/chicken.rb"
>   end
> end
>
> I first saw this technique described by David Chelimsky at
> http://rubyforge.org/pipermail/rspec-users/2007-September/002965.html

Which I first saw described by Jay Fields at
http://blog.jayfields.com/2006/12/rails-unit-testing-activerecord.html

On Tue, 16 Oct 2007 13:43:43 -0500, David Chelimsky wrote:

> 
> Which I first saw described by Jay Fields at
> http://blog.jayfields.com/2006/12/rails-unit-testing-activerecord.html

Sweet, thanks guys. That's exactly what I was looking for.

El 16/10/2007, a las 6:50, Steve <vertebrate@gmail.com> 
escribió:
> I'm thinking of maybe some internal Rails variable that holds an  
> array of
> accessible fields maybe? Does anyone know if such a thing exists?  
> Just to
> make it clear, I don't want to test Rails, I want to check that the
> accessible fields have been set properly. Would it be possible to  
> mock or
> some such to check for a call to attr_accessible?

Yes, I spec this kind of thing.

   describe User, 'accessible attributes' do
     it 'should allow mass-assignment to the login name' do
       lambda { new_user.update_attributes(:login_name =>
String.random) }.should_not raise_error
     end
     # many other examples here, one for each accessible attribute
that I care about
   end

   describe User, 'protected attributes' do
     it 'should deny mass-assignment to the passphrase hash' do
       lambda { new_user.update_attributes(:passphrase_hash =>
String.random) }.should raise_error
     end
     # many other examples here, one for each protected attribute
that I care about
   end

I do it this way because:

1. I am most interested in the *behaviour*; does this let me mass-
assign to this attribute (or not)?

2. I don't want my specs to depend on internal knowledge of the Rails
attribute protection/access implementation.

The style of spec shown above seems the best way to me of both
documenting the desired behaviour and confirming that it exists; I
don't view these as tests of Rails' internals, but as tests that I've
used attr_accessible and attr_protected correctly. I tried other ways
in the past, but this is the one I like best.

Another thing to note that helps the readability of the specs: the
new_user and String.random methods are provided by FixtureReplacement
(<http://replacefixtures.rubyforge.org/>). Note that by default
FixtureReplacement used mass-assignment under the covers, so it can't
be used with protected attributes, but I've sent a patch in to the
maintainer which changes that:

Index: lib/fixture_replacement/fixture_replacement.rb
===================================================================
--- lib/fixture_replacement/fixture_replacement.rb      (revision 31)
+++ lib/fixture_replacement/fixture_replacement.rb      (working copy)
@@ -70,7 +70,9 @@
            hash_given = args[0] || Hash.new
            merged_hash = self.send(attributes_method).merge(hash_given)
            evaluated_hash = Generator.merge_unevaluated_method
(self, :create, merged_hash)
-          obj = class_name.create!(evaluated_hash)
+          obj = class_name.new
+          evaluated_hash.each { |k, v| obj.send("#{k}=", v) }
+          obj.save!
            obj
          end
        end
@@ -86,7 +88,9 @@
            hash_given = args[0] || Hash.new
            merged_hash = self.send(attributes_method).merge(hash_given)
            evaluated_hash = Generator.merge_unevaluated_method
(self, :create, merged_hash)
-          class_name.new(evaluated_hash)
+          obj = class_name.new
+          evaluated_hash.each { |k, v| obj.send("#{k}=", v) }
+          obj
          end
        end
      end

Hopefully this will be included soon.

One more thing to note: the documentation for FixtureReplacement is
pretty thin at this stage, so I threw together this cheatsheet:

<http://wincent.com/knowledge-base/FixtureReplacement_cheatsheet>

This is a bit easier than consulting the (excellent) screencast
provided by the author:

<http://http://replacefixtures.rubyforge.org/>

Cheers,
Wincent

On Tue, 16 Oct 2007 13:43:43 -0500, David Chelimsky wrote:

> 
> Which I first saw described by Jay Fields at
> http://blog.jayfields.com/2006/12/rails-unit-testing-activerecord.html

I've been thinking about this throughout the day, and is this a pattern
that should maybe be implemented in a more widespread pattern? For
example, check that the model makes the requisite calls to the various
validation functions? It would seem that unless you have some very 
custom
validation methods, that you would be testing rails implementation of 
its
validators, by running through all of the various checks manually.

I guess maybe it depends on if you view the testing as mostly testing a
black box, or if you assume some knowledge of the code internals? 
Thoughts?

Steve

On 10/16/07, Steve <vertebrate@gmail.com> wrote:
> >> I first saw this technique described by David Chelimsky at
> validators, by running through all of the various checks manually.
>
> I guess maybe it depends on if you view the testing as mostly testing a
> black box, or if you assume some knowledge of the code internals? Thoughts?

Well, you'll find differing opinions on this.  Every time I've done
validations I've written a spec that actually exercises the behavior.
For example

class Person < ActiveRecord::Base
  validates_presence_of :name
end

describe Person do
  it "should not be valid without a name" do
    p = Person.new
    p.should_not be_valid
    p.errors.on(:name).should == "can't be blank"
  end
end

As far as I'm concerned, that's specifying the behavior.  A person
without a name is not valid, and it results in an error message on
name.  Doing it the way we've discussed in this thread is ugly and
stupid, imo.

However, I had never used attr_accessible up to this point, and in
this case I thought it made sense.  It's just a lot easier to expect
the call itself than to try to test every single column that's not
accessible.  You end up looping through the columns, writing helper
methods to provide default values...it all gets ugly.  So, I thought
this works.

An interesting thing is that since I decided it's okay for
attr_accessible, I'm starting to consider whether it may be okay for
other validations.  afaik David C doesn't have a firm opinion on this,
and I have to admit that mine is softening up a bit.

Personally, if I wanted to extract this pattern, I would use a custom
expectation matcher.

Person.should require_field(:name) =>
  p = Person.new
  p.should_not be_valid
  p.errors.on(:name).should == "can't be blank"

or to make it mirror AR a bit more
Person.should validate_presence_of(:name)

I would be happy with a nice little library of expectation matchers
that handle all the validations like that.  It may seem silly since
the name is essentially the same as the method call itself, but I
prefer it because you're using the object's actual behavior.
Generally I want to exercise the behavior as much as possible and mock
out as little as I can.  The exceptions are when I haven't implemented
an interaction yet, so I mock it out, or when I'm dealing with code
from a lower layer in which case I keep the mocks for speed/dependency
purposes.

Pat

On 10/16/07, Pat Maddox <pergesu@gmail.com> wrote:
>
> Well, you'll find differing opinions on this.

Yes, you will. This gets very gray. Message expectations (mocks)
implicitly know about internals, but they also allow you to spec more
accurately WHERE behaviour gets implemented.

The argument in favor of loading up the file and expecting an AR
declaration (like attr_accessible) is that the behaivour is actually
implemented in AR, and it's really not any different from a Message
Expectation except it's happening on file load. It also means you
don't have to hit the database, which means it runs faster.

The argument against is that the declaration is an implementation
detail and therefore brittle.

>     p = Person.new
>     p.should_not be_valid
>     p.errors.on(:name).should == "can't be blank"
>   end
> end

I've got one project where I do this - it comes from somebody's post
on this list (though I'm at a loss for who right now):

person.should reject(nil).for(:name).with("can't be blank")
person.should reject("").for(:name).with("can't be blank")
person.should accept("a name").for(:name)

<snip>

> An interesting thing is that since I decided it's okay for
> attr_accessible, I'm starting to consider whether it may be okay for
> other validations.  afaik David C doesn't have a firm opinion on this,
> and I have to admit that mine is softening up a bit.
>
> Personally, if I wanted to extract this pattern, I would use a custom
> expectation matcher.

... which would probably be implemented like this under the hood:

def matches?(target)
  target.should_receive(:validates_presence_of).with(@expected)
  load "#{RAILS_ROOT}/app/models/#{target.humanize}.rb"
end

No?

On 10/16/07, David Chelimsky <dchelimsky@gmail.com> wrote:
>
> No?

No.  I actually showed it in my post, though I suppose I did it in a
slightly confusing format.  I would implement it like so:

def matches?(target)
  instance = Target.new
  instance.should_not be_valid
  instance.error_messages.on(@expected).should == "can't be blank"
end

A couple key points:
1. AR defines validates_presence_of, but your subclass is responsible
for doing the actual validation
2. This particular code is lightweight enough that it's okay to use
the real implementation.  You don't get a big performance hit like you
do when you're creating/updating objects in the db and their
associations
3. When you call MyModel.new when spec'ing business logic, it hits the
db to get the column info.  You already have a coupling to the DB
simply by using AR, so I don't think that's a good reason to prefer
expecting the validates_* method call.  If perf does become an issue,
you can stub out the columns and remove the db dependency all
together.  Jay Fields mentions that in a blog, and I think he's insane
(at least the way he showed), but it should be easy enough to preload
all the column info from schema.rb.  In fact I think I heard of that
technique from someone on this list.

Pat

On Tue, 16 Oct 2007 10:39:32 +0200, Wincent Colaiuta wrote:

> 
> 
> in the past, but this is the one I like best.
> 
> Another thing to note that helps the readability of the specs: the  
> new_user and String.random methods are provided by FixtureReplacement  
> (<http://replacefixtures.rubyforge.org/>). Note that by default  
> FixtureReplacement used mass-assignment under the covers, so it can't  
> be used with protected attributes, but I've sent a patch in to the  
> maintainer which changes that:

I'm all for avoiding rails internal behavior, but attr_accessible deals 
on a share least model. If you can verify that only
attrs x, y, z are accessible, you know that any other field
that rails can come up with, or you add later is covered.

I just saw FixtureReplacement the other day, and plan on giving it a go.
Looks like it does a bunch of what I already do manually.

El 17/10/2007, a las 7:26, "Pat Maddox" <pergesu@gmail.com> 
escribió:
>     p = Person.new
>     p.should_not be_valid
>     p.errors.on(:name).should == "can't be blank"
>   end
> end
>
> As far as I'm concerned, that's specifying the behavior.  A person
> without a name is not valid, and it results in an error message on
> name.  Doing it the way we've discussed in this thread is ugly and
> stupid, imo.

I agree that you'll find many opinions on this. My personal take is
that you should be testing the behaviour, not testing that the
"validates..." method was called.

Why? Because while this is fine for a simple validation like
validates_presence_of, it can fall down for complex validations
with :on, :if, and other clauses. The problem is that it is fairly
easy to write a complex validation that doesn't actually do what you
think it does. Testing that you set it up gives you very little in
that case; you should be testing that it *does* what you think it
does. You are not testing ActiveRecord in this case (it's well
tested); you are testing that your use of ActiveRecord gives you the
desired behaviour. And as has already been pointed out, you can write
a custom matcher to make such oft-used specs easy to write. My own
take on this is here:

<http://wincent.com/a/about/wincent/weblog/archives/2007/10/
custom_validation_matcher.php>

Compare this with attr_accesible. That's much simpler; it takes a
list of symbols. It is much harder to make a mistake. In that case I
think it's fine to mock the call and just check that your code does
the attr_accessible call. But having said that, seeing as testing the
behaviour directly is so simple (just try mass assigning and see if
it worked or not) I just test the behaviour anyway and forget about
the mock+load trick.

Cheers,
Wincent