thoughtbot / shoulda Goto Github PK

Reported by Justin

In my tests I use should_not_allow_values_for. After I updated to Rails 2.3.2 I started getting the following error:

test: User should not allow login to be set to "testguy!". (UserTest):
ArgumentError: interning empty string
/Library/Ruby/Gems/1.8/gems/thoughtbot-shoulda-2.10.1/lib/shoulda/active_record/helpers.rb:7:in `to_sym'
/Library/Ruby/Gems/1.8/gems/thoughtbot-shoulda-2.10.1/lib/shoulda/active_record/helpers.rb:7:in `pretty_error_messages'
/Library/Ruby/Gems/1.8/gems/thoughtbot-shoulda-2.10.1/lib/shoulda/active_record/helpers.rb:5:in `map'
/Library/Ruby/Gems/1.8/gems/thoughtbot-shoulda-2.10.1/lib/shoulda/active_record/helpers.rb:5:in `pretty_error_messages'
/Library/Ruby/Gems/1.8/gems/thoughtbot-shoulda-2.10.1/lib/shoulda/active_record/matchers/allow_value_matcher.rb:95:in `error_description'
/Library/Ruby/Gems/1.8/gems/thoughtbot-shoulda-2.10.1/lib/shoulda/active_record/matchers/allow_value_matcher.rb:52:in `negative_failure_message'
/Library/Ruby/Gems/1.8/gems/thoughtbot-shoulda-2.10.1/lib/shoulda/assertions.rb:56:in `assert_rejects'
/Library/Ruby/Gems/1.8/gems/thoughtbot-shoulda-2.10.1/lib/shoulda/active_record/macros.rb:174:in `__bind_1237419160_416826'
/Library/Ruby/Gems/1.8/gems/thoughtbot-shoulda-2.10.1/lib/shoulda/context.rb:253:in `call'
/Library/Ruby/Gems/1.8/gems/thoughtbot-shoulda-2.10.1/lib/shoulda/context.rb:253:in `test: User should not allow login to be set to "testguy!". '

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.

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 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

I have updated both should_change and should_not_change to accept a string description and a block.

The "old" behavior is still maintained but will display a deprecation warning.

You can find the updated code in my fork: http://github.com/matflores/shoulda/tree/should_change_with_block

For more details please see the complete discussion at Lighthouse:

https://thoughtbot.lighthouseapp.com/projects/5807/tickets/154-change-should_change-to-use-a-block

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

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 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?

The problem and my potential solution is outlined here:

http://groups.google.com/group/shoulda/browse_thread/thread/8c0a66c80ff4fd76

Patch:

http://pastie.org/418093

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 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

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

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

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

Many of my active record macros are failing because #get_options! tries to return an unsalted array. This is not going to work in 1.9.

def get_options!
 ret = [nil]
 return *ret
end

get_options! # => nil     Ruby 1.8
get_options! # => [nil]   Ruby 1.9

Patch: https://thoughtbot.lighthouseapp.com/attachments/121224/get_options_fix.diff

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

Should_allow_values_for will only fail if you use a value that doesn't pass an existing validates_format_of validation (well, actually any validation that uses the :invalid error).

I would expect should_allow_values_for to fail if there is ANY error, and should_not_allow_values_for to pass if it detects ANY error (unless the the :message option is used).

I have created a patch for this at:
http://github.com/matflores/shoulda/tree/should_allow_values_for_fix

Instead of allowing the :message option on should_allow_values_for macro, what I did was to make the test fail if ANY error is detected on the specified attribute. And should_not_allow_values_for still handles the :message option if present, but if it's omitted it will pass as long as it detects ANY error on the specified attribute.

Personally I expect that if I say "attr should accept 'xyz'" then the value 'xyz' should pass all validations for attr, without having to worry about the messages I would expect to get if I had passed an invalid (and different) value. But that's just my opinion, of course.

See the full discussion about this issue at: https://thoughtbot.lighthouseapp.com/projects/5807/tickets/194-should_allow_values_for-macro-always-passes

Company.has_many :employees, :class_name => User

Currently unsupported by the should_have_many macro.

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 gaffo

When using should assign to, if you accidentally put in :equal, it silently works and does not check the :equals condition. It should either raise invalid options for anything it's not expecting, or it should handle :equal and :equals.

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.

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

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

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 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?

Proposed shoulda macro: should_accept_nested_attributes_for

  should "accept nested attributes for brand_managers" do
    assert_respond_to Sponsor.new, :brand_managers_attributes=
  end

Passes when we have:

class Sponsor < ActiveRecord::Base
  accepts_nested_attributes_for :brand_managers
end

Thoughts?

We could also just port Remarkable's matcher.

Added a helpful clarification to README. Absence of this code caused me some unnecessary confusion.

http://github.com/railsdog/shoulda/commit/632b0452d813957a92bc08873158e020d3db3af8

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.

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

When running "rake" within ruby19 the current status of tests (which all pass in ruby1.8.6) is:

463 tests, 596 assertions, 10 failures, 20 errors, 0 skips

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 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.

Reported by figwit

should_require_unique_attributes doesn't support case-insensitive uniqueness validation.

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

Hi, plz update documentation with latest macros (for example should_require_attributes -> should_validate_presence_of etc). Thanks!

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

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

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

Hello -

I've pushed a branch to my fork that implements a set_cookie matcher and associated macros. It's based heavily on the set_the_flash and set_session matchers. My current app utilizes the cacheable-flash plugin which moves the flash messages into a JSON encoded cookie and then clears the flash. This matcher should allow you to match the cookie value on either string or regexp value.

http://github.com/sob/shoulda/tree/add_set_cookie_matcher

Thanks,

• sob

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 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 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 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 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

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?

Hi!

I've noticed, that Shoulda doesn't empty ActionMailer::Base.deliveries. Unless the user empty this by himself (run ActionMailer::Base.deliveries = [] in a setup), calling several emailing actions in a functional test will fill the deliveries Array, which might let assert_sent_email and assert_does_not_send_email to report wrong results.

I'm using ruby 1.8.6 on Windows Vista with Rails 2.3.2 and Shoulda 2.10.2

should_assign_to(:var) { false } always passes, if that value is assigned at all, even if it has another value.

Patch & test here: http://github.com/iGEL/shoulda/commit/7972a1d437a4e2608be4459a596e712fbe4f76ff

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 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