Commit 53b58da9 authored by Peter Leitzen's avatar Peter Leitzen Committed by Dmytro Zaporozhets

Add RuboCop for `have_gitlab_http_status(:symbol)`

    # bad
    expect(response).to have_http_status(200)
    expect(response).to have_http_status(:ok)
    expect(response).to have_gitlab_http_status(200)

    # good
    expect(response).to have_gitlab_http_status(:ok)

Our "Testing best practices" already suggests to prefer
`have_gitlab_http_status` over `have_http_status`.
parent 9218c22a
...@@ -361,6 +361,9 @@ RSpec/MissingExampleGroupArgument: ...@@ -361,6 +361,9 @@ RSpec/MissingExampleGroupArgument:
RSpec/UnspecifiedException: RSpec/UnspecifiedException:
Enabled: false Enabled: false
RSpec/HaveGitlabHttpStatus:
Enabled: false
Style/MultilineWhenThen: Style/MultilineWhenThen:
Enabled: false Enabled: false
......
...@@ -26,7 +26,7 @@ describe API::Labels do ...@@ -26,7 +26,7 @@ describe API::Labels do
get api("/projects/#{project.id}/labels", user) get api("/projects/#{project.id}/labels", user)
expect(response).to have_http_status(200) expect(response).to have_gitlab_http_status(:ok)
expect(json_response.first['name']).to eq('label1') expect(json_response.first['name']).to eq('label1')
end end
...@@ -35,7 +35,7 @@ describe API::Labels do ...@@ -35,7 +35,7 @@ describe API::Labels do
get api("/projects/#{project.id}/labels", user) get api("/projects/#{project.id}/labels", user)
expect(response).to have_http_status(200) expect(response).to have_gitlab_http_status(:ok)
expect(json_response.first['name']).to eq('label1') expect(json_response.first['name']).to eq('label1')
end end
end end
...@@ -77,7 +77,7 @@ describe API::Labels do ...@@ -77,7 +77,7 @@ describe API::Labels do
get api("/projects/#{project.id}/labels", user) get api("/projects/#{project.id}/labels", user)
expect(response).to have_http_status(200) expect(response).to have_gitlab_http_status(:ok)
expect(json_response.first['name']).to eq('foo') expect(json_response.first['name']).to eq('foo')
end end
...@@ -86,7 +86,7 @@ describe API::Labels do ...@@ -86,7 +86,7 @@ describe API::Labels do
get api("/projects/#{project.id}/labels", user) get api("/projects/#{project.id}/labels", user)
expect(response).to have_http_status(200) expect(response).to have_gitlab_http_status(:ok)
expect(json_response.first['name']).to eq('bar') expect(json_response.first['name']).to eq('bar')
end end
end end
......
# frozen_string_literal: true
require 'rack/utils'
module RuboCop
module Cop
module RSpec
# This cops checks for `have_http_status` usages in specs.
# It also discourages the usage of numeric HTTP status codes in
# `have_gitlab_http_status`.
#
# @example
#
# # bad
# expect(response).to have_http_status(200)
# expect(response).to have_http_status(:ok)
# expect(response).to have_gitlab_http_status(200)
#
# # good
# expect(response).to have_gitlab_http_status(:ok)
#
class HaveGitlabHttpStatus < RuboCop::Cop::Cop
CODE_TO_SYMBOL = Rack::Utils::SYMBOL_TO_STATUS_CODE.invert
MSG_MATCHER_NAME =
'Use `have_gitlab_http_status` instead of `have_http_status`.'
MSG_STATUS =
'Prefer named HTTP status `%{name}` over ' \
'its numeric representation `%{code}`.'
MSG_UNKNOWN = 'HTTP status `%{code}` is unknown. ' \
'Please provide a valid one or disable this cop.'
MSG_DOCS_LINK = 'https://docs.gitlab.com/ee/development/testing_guide/best_practices.html#have_gitlab_http_status'
REPLACEMENT = 'have_gitlab_http_status(%{arg})'
def_node_matcher :have_http_status?, <<~PATTERN
(
send nil?
{
:have_http_status
:have_gitlab_http_status
}
_
)
PATTERN
def on_send(node)
return unless have_http_status?(node)
offenses = [
offense_for_name(node),
offense_for_status(node)
].compact
return if offenses.empty?
add_offense(node, message: message_for(offenses))
end
def autocorrect(node)
lambda do |corrector|
corrector.replace(node.source_range, replacement(node))
end
end
private
def offense_for_name(node)
return if method_name(node) == :have_gitlab_http_status
MSG_MATCHER_NAME
end
def offense_for_status(node)
code = extract_numeric_code(node)
return unless code
symbol = code_to_symbol(code)
return format(MSG_UNKNOWN, code: code) unless symbol
format(MSG_STATUS, name: symbol, code: code)
end
def message_for(offenses)
(offenses + [MSG_DOCS_LINK]).join(' ')
end
def replacement(node)
code = extract_numeric_code(node)
arg = code_to_symbol(code) || argument(node).source
format(REPLACEMENT, arg: arg)
end
def code_to_symbol(code)
CODE_TO_SYMBOL[code]&.inspect
end
def extract_numeric_code(node)
arg_node = argument(node)
return unless arg_node&.type == :int
arg_node.children[0]
end
def method_name(node)
node.children[1]
end
def argument(node)
node.children[2]
end
end
end
end
end
...@@ -40,6 +40,7 @@ require_relative 'cop/rspec/be_success_matcher' ...@@ -40,6 +40,7 @@ require_relative 'cop/rspec/be_success_matcher'
require_relative 'cop/rspec/env_assignment' require_relative 'cop/rspec/env_assignment'
require_relative 'cop/rspec/factories_in_migration_specs' require_relative 'cop/rspec/factories_in_migration_specs'
require_relative 'cop/rspec/top_level_describe_path' require_relative 'cop/rspec/top_level_describe_path'
require_relative 'cop/rspec/have_gitlab_http_status'
require_relative 'cop/qa/element_with_pattern' require_relative 'cop/qa/element_with_pattern'
require_relative 'cop/qa/ambiguous_page_object_name' require_relative 'cop/qa/ambiguous_page_object_name'
require_relative 'cop/sidekiq_options_queue' require_relative 'cop/sidekiq_options_queue'
......
# frozen_string_literal: true
require 'fast_spec_helper'
require 'rspec-parameterized'
require 'rubocop'
require 'rubocop/rspec/support'
require_relative '../../../../rubocop/cop/rspec/have_gitlab_http_status'
describe RuboCop::Cop::RSpec::HaveGitlabHttpStatus do
include CopHelper
using RSpec::Parameterized::TableSyntax
let(:source_file) { 'spec/foo_spec.rb' }
subject(:cop) { described_class.new }
shared_examples 'offense' do |code|
it 'registers an offense' do
inspect_source(code, source_file)
expect(cop.offenses.size).to eq(1)
expect(cop.offenses.map(&:line)).to eq([1])
expect(cop.highlights).to eq([code])
end
end
shared_examples 'no offense' do |code|
it 'does not register an offense' do
inspect_source(code)
expect(cop.offenses).to be_empty
end
end
shared_examples 'autocorrect' do |bad, good|
it 'autocorrects' do
autocorrected = autocorrect_source(bad, source_file)
expect(autocorrected).to eql(good)
end
end
shared_examples 'no autocorrect' do |code|
it 'does not autocorrect' do
autocorrected = autocorrect_source(code, source_file)
expect(autocorrected).to eql(code)
end
end
describe 'offenses and autocorrections' do
where(:bad, :good) do
'have_http_status(:ok)' | 'have_gitlab_http_status(:ok)'
'have_http_status(204)' | 'have_gitlab_http_status(:no_content)'
'have_gitlab_http_status(201)' | 'have_gitlab_http_status(:created)'
'have_http_status(var)' | 'have_gitlab_http_status(var)'
'have_http_status(:success)' | 'have_gitlab_http_status(:success)'
'have_http_status(:invalid)' | 'have_gitlab_http_status(:invalid)'
end
with_them do
include_examples 'offense', params[:bad]
include_examples 'no offense', params[:good]
include_examples 'autocorrect', params[:bad], params[:good]
include_examples 'no autocorrect', params[:good]
end
end
describe 'partially autocorrects invalid numeric status' do
where(:bad, :good) do
'have_http_status(-1)' | 'have_gitlab_http_status(-1)'
end
with_them do
include_examples 'offense', params[:bad]
include_examples 'offense', params[:good]
include_examples 'autocorrect', params[:bad], params[:good]
include_examples 'no autocorrect', params[:good]
end
end
describe 'ignore' do
where(:code) do
[
'have_http_status',
'have_http_status { }',
'have_http_status(200, arg)',
'have_gitlab_http_status',
'have_gitlab_http_status { }',
'have_gitlab_http_status(200, arg)'
]
end
with_them do
include_examples 'no offense', params[:code]
include_examples 'no autocorrect', params[:code]
end
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment