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

Rails 6 will remove success? predicate in favor of successful? #1857

Closed
ybart opened this issue Aug 9, 2017 · 8 comments
Closed

Rails 6 will remove success? predicate in favor of successful? #1857

ybart opened this issue Aug 9, 2017 · 8 comments

Comments

@ybart
Copy link

ybart commented Aug 9, 2017

While running against Rails 5.2.0.alpha, I encountered the following message when running my test suite:

.DEPRECATION WARNING: The success? predicate is deprecated and will be removed in Rails 6.0. Please use successful? as provided by Rack::Response::Helpers. (called from matches? at rspec/rails/matchers/have_http_status.rb:263)

The message is coming from rails/rails#30104 (rails/rails@af3500b)

@ybart
Copy link
Author

ybart commented Aug 17, 2017

I'm not sure how to deal with this. Maybe we may edit

to allow statuses referenced here http://www.rubydoc.info/gems/rack/Rack/Response/Helpers as well.

@trev
Copy link

trev commented Aug 17, 2017

Basically the whole GenericStatus class functionality needs to be deprecated and removed in lockstep with Rails 6.0.

@trev
Copy link

trev commented Aug 17, 2017

Probably something like:

        def self.matcher_for_status(target)
          if GenericStatus.valid_statuses.include?(target)
            RSpec.deprecate(
              GenericStatus.valid_statuses.map { |c| "`:#{c}`" }.join(', '),
              replacement: "the Rack::Response::Helpers predicates as symbols",
            )
            GenericStatus.new(target)
          elsif Symbol === target
            SymbolicStatus.new(target)
          else
            NumericCode.new(target)
          end
        end

alternatively we could just leave as is since the Rails deprecation takes care of it.

In your case you're probably using something like expect(response).to have_http_status(:success) in your tests. To get rid of the deprecation just use expect(response).to have_http_status(:successful)

@ybart
Copy link
Author

ybart commented Aug 17, 2017

Thanks for the answer, just tried that and got the following error :

1) HomeController GET #show returns http success
     Failure/Error: expect(response).to have_http_status(:successful)

     ArgumentError:
       Invalid HTTP status: :successful
     # ./spec/controllers/home_controller_spec.rb:11:in `block (3 levels) in <top (required)>'

I thinks this is because SymbolicStatus matches against Rack::Utils::SYMBOL_TO_STATUS_CODE, which does not contain :successful.

@trev
Copy link

trev commented Aug 17, 2017

Good point. In the interim you could do expect(response).to have_http_status(200) or whatever successful response code you'd expect.

Perhaps a more adequate solution would be to make these Rack Helpers the new GenericStatus.valid_statuses whilst deprecating the old ones.

@ybart
Copy link
Author

ybart commented Aug 23, 2017

I'm writing a PR for the more adequate solution.

However, I am not sure about redirect: This commit removed it in favor of Rack redirect one, but semantics are not exactly the same, redirection means [300..399] and redirect means [301, 302, 307, 308].

This is a breaking change already in Rails 5, so I think we should support both here, but without any deprecation warning.

@jonekdahl
Copy link

Is this fixed by #1951?

@JonRowe
Copy link
Member

JonRowe commented Apr 19, 2018

Closed by #1951

@JonRowe JonRowe closed this as completed Apr 19, 2018
jfly added a commit to jfly/worldcubeassociation.org that referenced this issue Sep 12, 2018
jfly added a commit to thewca/worldcubeassociation.org that referenced this issue Sep 12, 2018
databus23 added a commit to sapcc/lyra that referenced this issue Aug 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants