thoughtbot / shoulda

Company.has_many :employees, :class_name => User

Currently unsupported by the should_have_many macro.

Reported by Andrew Young

should_validate_uniqueness_of scoped to a datetime column fails when the existing value of the datetime column is nil.

See the fix here.

git://github.com/ayoung/shoulda.git

On a side note, I have not had any success running the unit tests for shoulda but I included a test for this fix anyway.

Reported by Ken Collins

It is not always necessary to add errors to an ActiveRecord object using a key on an attribute or method. Sometime you do this for human readable messages or to abstract underlying schema. In this case this patch makes sure that the object can respond to said error key before adding it to the pretty error message string.

Patch: https://thoughtbot.lighthouseapp.com/attachments/121309/pretty_message_fix.diff

Reported by Adam Cigánek

Hello,

When i run all my tests using rake test i can see that i have some deferred tests (using should_eventually or should without block). The problem is that it doesn't tell me in which files are they defined, so i have to search through all of them to find it. I suggest that the DEFERRED message includes a name of the file or test case it is defined in.

Reported by Dmitry Ratnikov

Hi, I have been using this extension for about 2 months now and thought I'd throw it out for review to see if is of interest for the core shoulda team.

When doing testing, there are usually three steps:
1) Set up data 2) Run methods that change the state 3) Checks whether the state changes occurred correctly.

Within shoulda framework, the first two steps are done by setup and the latter by should statements. I am proposing to separate 1 and 2 into setup and evaluate steps.

I hit the use case mostly with functional controller testing.

Here's an old style of testing creating some resource Foo:

  class FooControllerTest < ActionController::TestCase
    
    context "POST :create" do
      setup do
        @foo_hash = { :required_field => 'foo' }
      
        post :create, { :foo => @foo_hash }
      end

      should_redirect_to 'foo_path(Foo.last)'
      should_change 'Foo.count', :by => +1
    end

    context "POST :create with 'required_field' missing" do
      setup do
        @foo_hash = { :optional_field => 'bar' }
        post :create, { :foo => @foo_hash }
      end

      should_render_template 'new'
      should_not_change 'Foo.count'
    end  
  end

With the extension:

  class FooControllerTest < ActionController::TestCase
    
    context "POST :create" do
      setup { @foo_hash = { :required_field => 'foo' } }
      evaluate { post :create, { :foo => @foo_hash } }

      should_redirect_to 'foo_path(Foo.last)'
      should_change 'Foo.count', :by => +1

      context "with :required_field missing" do
        setup { @foo_hash.delete :require_field }
        should_render_template 'new'
        should_not_change 'Foo.count'
      end
    end
  end

The approach allows to declare what you are testing and then play with different setups and specifying expected behavior for those specific setups.

I attached the patch against 2.10.1 that implements the functionality.

PS The implementation in the patch is backward-compatible with current shoulda. However, it makes sense to execute :before procs after all setups and before all evaluates. Hence, I added a Shoulda.run_before_procs_after_setup which controls that behavior. By default it is set to false.

Patch: https://thoughtbot.lighthouseapp.com/attachments/119244/should_evaluate.patch

Reported by Matías Flores

When scoped_to is used with should_validate_uniqueness_of, the matcher will create a new record and will set the specified attribute and all the specified scoped fields to the values of the first record found in the db for that model, and will confirm that the validations in place reject the new record.

If that validation works, the matcher will then try to change the values of each one of the scoped fields, checking after each change that the new record has become valid. If the record is rejected as a duplicate after changing any of these scoped fields, the matcher assumes that the scope option defined in the model's validation is not valid or does not match exactly the one specified on the should_validate_uniqueness_of macro.

The problem with this approach is that when the matcher starts changing the scoped field values, it never restores the original values before moving to the next scoped field. So, once that we have already changed the first scoped field, we can be sure that the record will not be considered duplicated by any of the other scoped fields, regardless if they were included in the scope option in the model or not.

In the practice, the matcher works if the scoped_to call is incomplete. E.g.

validates_uniqueness_of :attr, :scope => [:scope1, :scope2]
should_validate_uniqueness_of :attr, :scoped_to => :scope1

But fails if we have more scoped_to options in the should_validate_uniqueness_of macro than fields in the scope option in the model. E.g.

validates_uniqueness_of :attr, :scope => [:scope1, :scope2]
should_validate_uniqueness_of :attr, :scoped_to => [:scope1, :scope2, :other]

This branch restores the original values of the current scoped fields before moving to the next one, so that only one field changes at a time.

http://github.com/matflores/shoulda/tree/validate_uniqueness_evaluating_one_scope_at_a_time

HTH

Reported by Tom Lea

Common problem

(skip to the point below if you bore of my rant).

I have a controller, and an index action, I wish to assert all kinds of things against the outcome of the action.

So I do a context with a setup, start macroing it all up.... should_respond_with.... should_have_metadata_foo.... should_be_awsome.

Now it's all tested.

I win.

But the controller sucks, because I can't code, so it takes like .5s to render a response.... and I can't stub it, because I would actually like to test something for a change.

I've just written 50 tests against the single response, so the test takes around 25s to run.

This is unacceptable, I am an outcast from my dev team. "Make the tests fast and complete or we will throw food at you" they say.

I give up and put all the tests in a single test case, loosing my shoulda sugar, leaving me feeling dirty and a little used.

I loose.

now I get to the point

Life should not be like this, I should be able to make all the assertions running the tests only once (I make a mental contract not the change any pre-setup data).

So here is an example of what I am trying to achieve:

@@@ ruby context "with an action controller" do
setup do

get :index

end

static_context "with a static context" do #All 3 tests just hit the controller once

should_respond_with :success
should_not_knife_me_in_the_face
should_have_css_of_the_gods :with => :penguins

end

should_test_something_that_requires_changing_the_data # Fresh data each time for these two should_test_something_else_that_requires_changing_the_data end
@@@

And here is what I have so far: http://github.com/cwninja/shoulda/tree/master

It does not support teardowns, it is probably not freeing objects from test cases right now
(was planning on killing the stashed data when all the tests in the context are done, child contexts are fine to do the same, the GC will do the rest). Also :before => proc{} hooks seem
like a silly idea in this context, so I banned them, as they would be unpredictable/unexpected
at best.

Is this a direction shoulda is interested in heading?

Shoulda do more?

Reported by François Beausoleil

This is an ActiveRecord extension to validate that all fixtures of the test's class are valid.

class UserTest < T::U::TC should_have_valid_fixtures end

Implemented on should_have_valid_fixtures2 of francois' fork:

http://github.com/francois/shoulda/tree/should_have_valid_fixtures2

Reported by Maxim Chernyak

This looks kinda ugly:

context '' do
  ...
end

Why not make it like this?

context do
  ...
end

Sometimes you just want a context for the sake of setup, without any extra text prepends.

All it takes is to add that tiny = ''

def context(name = '', &blk)
...

Reported by Brandon Dimcheff

I made a small patch to assertions.rb that makes assert_contains/assert_does_not_contain work properly for anything that's an Enumerable (or defines #include?, really). New tests added, passes all existing tests.

The commit is here: http://github.com/bdimcheff/shoulda/commit/92ebbedd922e46ab2d47f37127d9bd184ae55767

• Brandon

Reported by Mike Breen

http://github.com/hardbap/shoulda/tree/should_render_partial_macro

assert_template in Rails 2.3 allows you to test if a partial is rendered and the number of times it was rendered.

Reported by Nathaniel Bibler

I've found it very useful in several instances to customize the currently packaged assert_did_not_send_email assertion to receive a matcher block. This functionality already exists in assert_sent_email and adding it would make the two tests behave in a more consistent manor.

Most recently, it was useful when testing that a delayed job sent emails to certain, specific individuals, but not others. In that case, the current assert_did_not_send_email wasn't enough, as it was correct that some emails were being sent (while the current assertion just checks for empty deliveries). In this case, extending it to support matching email addresses helped greatly.

So, here is a patch which is based off of HEAD as of 10 minutes ago, with tests and documentation. :)

Patch: https://thoughtbot.lighthouseapp.com/attachments/99780/0001-Enhanced-assert_did_not_send_email-to-optionally-rec.patch

Reported by Josh Nichols

You get an error like this:

/Users/nichoj/Projects/anyminutenow/vendor/plugins/shoulda/lib/shoulda/context.rb:204:in `merge_block': You have a nil object when you didn't expect it! (NoMethodError)
The error occurred while evaluating nil.bind    from /Users/nichoj/Projects/anyminutenow/vendor/plugins/shoulda/lib/shoulda/context.rb:199:in `initialize'
        from /Users/nichoj/Projects/anyminutenow/vendor/plugins/shoulda/lib/shoulda/context.rb:174:in `new'
        from /Users/nichoj/Projects/anyminutenow/vendor/plugins/shoulda/lib/shoulda/context.rb:174:in `context'
        from test/functional/events_controller_test.rb:88

So, either context's should be able to be defined without a context, or an appropriate error should be given.

I was seeing this failure:

  2) Error:
test: As a superuser on successful PUT to :update should redirect to "edicts_url". (EdictsControllerTest):
NoMethodError: You have a nil object when you didn't expect it!
The error occurred while evaluating nil.assigns
    /Users/nichoj/Projects/sniftag.com/vendor/plugins/shoulda/lib/shoulda/controller/helpers.rb:36:in `instantiate_variables_from_assigns'
    /Users/nichoj/Projects/sniftag.com/vendor/plugins/shoulda/lib/shoulda/controller/macros.rb:156:in `__bind_1220545692_620496'
    /Users/nichoj/Projects/sniftag.com/vendor/plugins/shoulda/lib/shoulda/context.rb:224:in `call'
    /Users/nichoj/Projects/sniftag.com/vendor/plugins/shoulda/lib/shoulda/context.rb:224:in `test: As a superuser on successful PUT to :update should redirect to "edicts_url". '
    /Library/Ruby/Gems/1.8/gems/activesupport-2.1.0/lib/active_support/testing/setup_and_teardown.rb:67:in `__send__'
    /Library/Ruby/Gems/1.8/gems/activesupport-2.1.0/lib/active_support/testing/setup_and_teardown.rb:67:in `run'

Shortly after, I realized it was because I didn't actually do a get. It would be sweet to get a better error message than this.

I could imagine something like:

raise "@response.template was nil. Did you forget to call get/post/put/destroy?" if @responsetemplate.nil?

I made a branch for this:

http://github.com/technicalpickles/shoulda/tree/ticket-66

It just does an assert with a nice message.

I wanted to add some test for this, but I'm not sure what the best way would be. This is because all of shoulda's tests are testing shoulda in action, rather than testing how shoulda works... if that makes any sense...

Reported by figwit

should_require_unique_attributes doesn't support case-insensitive uniqueness validation.

Patch: https://thoughtbot.lighthouseapp.com/attachments/49183/case_sensitive.diff

Reported by James Bebbington (fractious)

Hi,

Would the attached patch be something you'd consider adding to shoulda? Having only just got started using shoulda I thought it might be useful. Wasn't sure exactly where I should be testing the extensions so apologies for the lame tests... the patch is more of "good/bad idea?" request than a "please consider for committing".

Cheers.

Patch: https://thoughtbot.lighthouseapp.com/attachments/57836/should_assign_to_extensions.rb

Reported by Nathaniel Bibler

The current should_render_template fails to support testing for render :nothing => true in actions.

Because should_render_template converts the value is passed into it to a string prior to sending to assert_template, then testing for a nil template is not possible.

should_render_template nil

1) Failure:
test: Foo should render template nil. (FooControllerTest)
...
expecting <""> but rendering with 

I created a new controller macro, called should_render_nothing which simply tests for assert_template nil.

This was done rather than modifying should_render_template, checking for a nil parameter, which doesn't seem terribly intuitive to me anyway (should_render_template nil doesn't logically follow render :nothing => true).

Patch: https://thoughtbot.lighthouseapp.com/attachments/58343/0001-Added-should_render_nothing-macro.patch

Reported by David Lowenfels

Passing a hash should perform assertions on flash[key]

should_set_the_flash_to :error => "Some error occurred"
## assert_equal "Some error occurred", flash[:error]

should_set_the_flash_to :notice => /match/
## assert_match /match/, flash[:notice]

etc.

diff attached to show the general implementation idea... if I get some more time I'll write some tests for it.

It would also be nice to be able to assert that flash[:error] is nil. e.g: should_not_flash :error

Patch: https://thoughtbot.lighthouseapp.com/attachments/72359/patch.diff

Reported by Daniel Schierbeck

I often use a single message in a validates_inclusion_of validation, and therefore don't need separate :low_message and :high_message values. This patch introduces a :message option, which handles both.

Patch: https://thoughtbot.lighthouseapp.com/attachments/77131/0001-Added-message-option-to-should_ensure_value_in_rang.patch

Reported by Jonathan Lim

Many validations allow you to skip validation if the value is blank or nil. These new macros will allow one to specify explicitly which values can be blank or nil.

http://github.com/snowblink/shoulda/tree/133_should_allow_blank_and_should_allow_nil

Reported by Celestino Gomes

use :with_attributes parameter to load data at new model instance.

Patch: https://thoughtbot.lighthouseapp.com/attachments/86411/with_attributes.patch

Reported by Lawrence Pit

Is there a reason why the ensure_inclusion_of matcher:

a) is not called validate_inclusion_of instead?

b) only accepts a range for it's :in option, instead of any enumerable object like rails allows?

Reported by Lawrence Pit

I was wondering if it would be possible for "us Test::Unit users" to use the matcher classes like it's used in rspec. I was thinking this could be made possible in a Test::Unit class like e.g.:

should have_many(:users).dependent(:delete_all)

In context.rb the should method would be something like:

def should(name, options = {}, &blk)
  if Shoulda.current_context
    block_given? ? Shoulda.current_context.should(name, options, &blk) : Should.current_context.should_eventually(name)
  elsif name.class.to_s =~ /^Shoulda::ActiveRecord::Matchers::/
    matcher = name
    should matcher.description do
      assert_accepts(matcher, subject)
    end
  else
    ...
  end
end

def subject
  # implementation somewhat like rspec
end

if this were possible I think you could deprecate the macros.rb file entirely.

Reported by Tammer Saleh

Something like this:

require 'test/unit'

class StupidTest < Test::Unit::TestCase
  
  def self.should_whatever
    file, line = caller(0)[1].split(':')
    class_eval "def test_should_whatever; assert false; end", file, line.to_i
  end
  
  should_whatever
  
end

Reported by Celestino Gomes

Macro renamed to should_validate_confirmation_of and ActiveRecord's macro accepts :with_attributes option for load new model instances with that.

Now, get on master of my Fork: http://github.com/tinogomes/shoulda/tree/master

When should_require_unique_attributes is used with the :scoped_to option, the matcher will fail if the scoped field is changed to a value that's already taken on the db.

There is a TODO note in the source code predicting this situation:

# Now test that the object is valid when changing the scoped attribute
# TODO:  There is a chance that we could change the scoped field
# to a value that's already taken.  An alternative implementation
# could actually find all values for scope and create a unique
# one.

On this branch you can find my attempt on fixing this issue:

http://github.com/matflores/shoulda/tree/should_validate_uniqueness_of_with_scope

Instead of calling #next on the previous_value I just call #next on the last value found in the db for that scoped field.

I have also removed the TODO note and added a test that proves the specific case that was failing.

For more details see the complete discussion at: https://thoughtbot.lighthouseapp.com/projects/5807/tickets/110-should_require_unique_attributes-with-scoped_by-not-actually-working

should_fail can be used to test the negative case for a shoulda macro:

should_fail do
  should_respond_with :success
end

This method should be documented and exposed in the public API.

This used to be ticket #162 on the Lighthouse. I developed some ActionMailer matchers on my fork:

http://github.com/enricob/shoulda/tree/actionmailer_matchers

Am I on the right track here? What other matchers can I add? Any feedback on the code itself?

I seem to have issues with shoulda (thoughtbot-shoulda (2.10.1)) when i'm using translations in rails 2.3.2. One method not working is: should_require_attributes.

should_validate_uniqueness_of scoped to 2 or more attributes doesn't fail when
model has validates_uniqueness_of scoped only to the first attribute

I have fixed it in this commit:
http://github.com/scambra/shoulda/commit/6e3b31053ec7e63a84e5e8c265f276b23a3ad390

The rails I18n implementation allows you to include {{attribute}}, {{model}} and {{value}} in the ActiveRecord I18n validation messages. E.g.:

en:
  activerecord:
    errors:
      messages:
        blank: "{{attribute}} can't be blank"

If you do so, shoulda fails to generate the error message on some validations like should_validate_presence_of and raises an exception. I fixed the issue in this commit: http://github.com/iGEL/shoulda/commit/f99dd3ba3d50ca44335a1d3dd882567f722d3849

Hi!

I have such scope in my model:

named_scope :not_expired, :conditions => [ 'created_at > ?', (Time.now - 1.day).to_s(:db) ]

And I have a test for it:

should_have_named_scope "not_expired", { :conditions => [ "created_at > ?", (Time.now - 1.day).to_s(:db) ] }

But that test fails with message like that:

not_expired to return results scoped to {:conditions=>["created_at > ?", "2009-07-03 14:33:28"]} but was scoped to {:conditions=>["created_at > ?", "2009-07-03 14:33:27"]}

As you can see the difference between should be and It is is 1 second.

RouteMatcher#description uses Hash#inspect, which can cause the same test to generate variable descriptions. This patch alters RouteMatcher#description to generate predictable strings: http://github.com/devver/shoulda/commit/bd1b194dc7ee21ea5d9a9c750a7c67f9919b33ee

validates_numericality_of allows :only_integer option that's useful for validate foreign_keys, positions and other integer fields.
This patch adds "integer" option to should_validate_numericallity_of like this

should_validate_numericallity_of :position, :integer => true

the commit is here http://github.com/eparreno/shoulda/blob/44c450edb098cadf35497915f482a79978095ce7

As far as I understand how rspec matchers work, "failure_message" is displayed when a "should" expectation fails and "negative_failure_message" is displayed when a "should_not" one does.

The AllowMassAssignment sets failure messages the other way round. This patch adds tests for failure messages and fixes the matcher: http://gist.github.com/152636