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

Support have_http_status with Rack::MockResponse #2771

Merged

Conversation

cbliard
Copy link
Contributor

@cbliard cbliard commented Jun 25, 2024

When using rack-test, doing something like expect(last_response).to have_http_status(:ok) would fail with the error "expected a response object, but an instance of Rack::MockResponse was received".

This PR creates a ActionDispatch::TestResponse from the Rack::MockResponse to make have_http_status usable in specs relying on rack-test to perform requests.

It should fix rubocop/rubocop-rspec_rails#20.

Reproduction script:

# frozen_string_literal: true

require "bundler/inline"

gemfile(true) do
  source "https://rubygems.org"

  git_source(:github) { |repo| "https://github.com/#{repo}.git" }

  # Activate the gem you are reporting the issue against.
  gem "rails", "6.1.0"
  gem "rspec-rails"
end

require "rspec-rails"
require "action_controller/railtie"

class TestApp < Rails::Application
  config.root = __dir__
  config.hosts << "example.org"
  config.session_store :cookie_store, key: "cookie_store_key"
  secrets.secret_key_base = "secret_key_base"

  config.logger = Logger.new($stdout)
  Rails.logger  = config.logger

  routes.draw do
    get "/" => "test#index"
  end
end

class TestController < ActionController::Base
  include Rails.application.routes.url_helpers

  def index
    render plain: "Hello, World!"
  end
end

require "rspec/autorun"
require "rspec/rails/matchers"

RSpec.describe "additions", type: :request do
  include Rack::Test::Methods
  include RSpec::Rails::Matchers

  it do
    get "/"
    expect(last_response).to have_http_status(:ok)
  end

  private

  def app
    Rails.application
  end
end

Copy link
Member

@pirj pirj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please provide a reproduction script a-la #2502 (comment) ?

lib/rspec/rails/matchers/have_http_status.rb Outdated Show resolved Hide resolved
@cbliard
Copy link
Contributor Author

cbliard commented Jun 25, 2024

Can you please provide a reproduction script a-la #2502 (comment) ?

I added one in the description. Thanks for pointing to a nice example.

Copy link
Member

@JonRowe JonRowe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👋 Thanks for drawing this to our attention, I think the simpler solution would be to both construct a real Rack::MockResponse in the test, and just match on that earlier.

@cbliard cbliard force-pushed the support-rack-mock-response-with-have-http-status branch from 690fb49 to 56f26a1 Compare June 25, 2024 09:14
@cbliard
Copy link
Contributor Author

cbliard commented Jun 25, 2024

Thanks @pirj and @JonRowe for your reviews and suggestions. I just pushed a fix.

@cbliard cbliard requested a review from JonRowe June 25, 2024 12:05
@JonRowe JonRowe merged commit 0bb51ef into rspec:main Jun 25, 2024
17 checks passed
@JonRowe
Copy link
Member

JonRowe commented Jun 25, 2024

Thanks!

JonRowe added a commit that referenced this pull request Jun 26, 2024
JonRowe added a commit that referenced this pull request Jun 26, 2024
…-have-http-status

Support `have_http_status` with `Rack::MockResponse`
JonRowe added a commit that referenced this pull request Jun 26, 2024
@JonRowe
Copy link
Member

JonRowe commented Aug 20, 2024

Released in 6.1.4

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

Successfully merging this pull request may close these issues.

RSpec/Rails/HttpStatus works partially with last_response
3 participants