Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecation warning for have(n).errors_on(:attr) lies in RSpec 2.99 #1026

Closed
yujinakayama opened this issue May 18, 2014 · 14 comments
Closed
Assignees
Milestone

Comments

@yujinakayama
Copy link
Member

Here's an example using have(n).errors_on(:attr):

describe Book do
  it 'fails validation with no name' do
    expect(Book.new).to have(1).error_on(:name)
  end
end

then running rspec 2.99 (the current 2-99-maintenance head) outputs the following deprecation warning:

expect(collection_owner).to have(1).error_on is deprecated. Use the rspec-collection_matchers gem or replace your expectation with something like expect(collection_owner.error_on.size).to eq(1) instead. Called from /Users/me/Desktop/sample-rails/spec/models/book_spec.rb:5:in `block (2 levels) in <top (required)>'.

If you change the expectation to the following as suggested by the warning (currently Transpec also converts it in the same way):

describe Book do
  it 'fails validation with no name' do
    expect(Book.new.error_on(:name).size).to eq(1)
  end
end

and upgrade to RSpec 3.0 (the current master) and run rspec:

Failures:

  1) Book fails validation with no name
     Failure/Error: expect(Book.new.error_on(:name).size).to eq(1)
     NoMethodError:
       undefined method `error_on' for #<Book id: nil, name: nil, created_at: nil, updated_at: nil>
     # ./spec/models/book_spec.rb:5:in `block (2 levels) in <top (required)>'

This is caused by the removal of the monkey-patch (ActiveModel::Validations#errors_on) from rspec-rails (#1005).

@cupakromer cupakromer added this to the 3.0.0.rc1 milestone May 18, 2014
@cupakromer cupakromer self-assigned this May 18, 2014
@cupakromer
Copy link
Member

@yujinakayama it seems that if you use the rspec-collection_matchers gem, you can keep this syntax. Otherwise, the change will be a bit more manual as you need to call valid? first.

I can update the deprecation warning accordingly.

@kWhittington
Copy link

I'm getting a similar error as yujinakayama,
Failure/Error: expect(User.new.error_on(:username).size).to be 1
NoMethodError:
undefined method `error_on' for #User:0x007fa27c56a8a8

rspec-collection_matchers is installed and required in spec_helper.rb
Was error_on removed entirely?

Thanks.

@cupakromer
Copy link
Member

@kWhittington it is part of the rspec-collection_matchers gem. However, I see we need to cut a new release for the gem. Working on that now, sorry for the interruption in usage.

@myronmarston
Copy link
Member

@kWhittington -- what version of rspec-collection_matchers are you running? As @cupakromer said, that method got moved into the gem (as it's really only needed by the collection matchers). v0.0.4 (release 2014-04-24) has the method.

@kWhittington
Copy link

I'm using v0.0.4 and those particular tests seem to be passing.
However, there seems to be an issue with the controller tests now.
My ControllerHelper functions aren't being loaded correctly (they're filtered on :type => :controller metadata) and @request.env is throwing a NilClass error, "now method 'env' for NilClass."

@myronmarston
Copy link
Member

@kWhittington -- rspec-rails 2.x had some very implicit ("magical", to some) behavior where it would infer the spec type based on the file location. In RSpec 3 (starting with RC1) you need to opt-in if you want this, or set the spec type yourself:

describe MyController, :type => :controller do
end

# or
RSpec.configure do |config|
  config.infer_spec_type_from_file_location!
end

See #970 for more info.

@kWhittington
Copy link

Thank you so much for point that out to me!
As of right now everything's working a-OK save for a few deprecated warnings from Capybara.

Thanks again!

@cupakromer
Copy link
Member

How do people feel about changing the deprecation warning to:

expect(collection_owner).to have(1).error_on is deprecated. Use the rspec-collection_matchers gem or replace your expectation with something like:

collection_owner.valid?
expect(collection_owner.error_on.size).to eq(1)

error_on isn't a matcher. It's actually just a monkey patch method on ActiveModel which calls valid? before returning a flattened version of the errors hash messages.

IMO this is not a useful tool at all. It's prone to all sorts of false positives. I've typically in the pass rolled my own matcher or done something like:

expect{ collection_owner.valid? }.to change{ collection_owner.errors[:attr] }.
  to include("error message")

Post 3.0, I'd like to consider making this a more proper matcher:

expect(collection_owner).to have_validation_error("foo bar").on(:attr)

@fables-tales
Copy link
Member

@cupakromer that deprecation warning reads very clearly. I also like the idea of having a matcher for this case in the future.

@JonRowe
Copy link
Member

JonRowe commented May 20, 2014

Do we still need to update the deprecation message?

@cupakromer
Copy link
Member

😬 we do, however, the error currently comes from rspec-expectations not rspec-rails. We'd have to special case errors_on.

@JonRowe
Copy link
Member

JonRowe commented May 21, 2014

I'd be ok with that in 2.99, perhaps check that both rspec-rails is loaded and that it's errors_on

@myronmarston
Copy link
Member

That sounds OK to me, too...but only because it's 2.99 so we're not left with that kind of cruft :).

@cupakromer
Copy link
Member

This has been implemented.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants