Commit 7438eece authored by Rémy Coutable's avatar Rémy Coutable

Merge branch 'rs-rspec-hook-argument-cop' into 'master'

Enable the RSpec/HookArgument cop and auto-correct offenses

See merge request !13484
parents 810c44ae add765e6
......@@ -1099,6 +1099,11 @@ RSpec/FilePath:
RSpec/Focus:
Enabled: true
# Checks the arguments passed to `before`, `around`, and `after`.
RSpec/HookArgument:
Enabled: true
EnforcedStyle: implicit
# Configuration parameters: EnforcedStyle, SupportedStyles.
# SupportedStyles: is_expected, should
RSpec/ImplicitExpect:
......
......@@ -70,12 +70,6 @@ RSpec/EmptyLineAfterFinalLet:
RSpec/EmptyLineAfterSubject:
Enabled: false
# Offense count: 78
# Configuration parameters: EnforcedStyle, SupportedStyles.
# SupportedStyles: implicit, each, example
RSpec/HookArgument:
Enabled: false
# Offense count: 9
# Configuration parameters: EnforcedStyle, SupportedStyles.
# SupportedStyles: it_behaves_like, it_should_behave_like
......
......@@ -188,16 +188,10 @@ Please consult the [dedicated "Frontend testing" guide](./fe_guide/testing.md).
### General Guidelines
- Use a single, top-level `describe ClassName` block.
- Use `described_class` instead of repeating the class name being described
(_this is enforced by RuboCop_).
- Use `.method` to describe class methods and `#method` to describe instance
methods.
- Use `context` to test branching logic.
- Use multi-line `do...end` blocks for `before` and `after`, even when it would
fit on a single line.
- Don't assert against the absolute value of a sequence-generated attribute (see [Gotchas](gotchas.md#dont-assert-against-the-absolute-value-of-a-sequence-generated-attribute)).
- Don't supply the `:each` argument to hooks since it's the default.
- Prefer `not_to` to `to_not` (_this is enforced by RuboCop_).
- Try to match the ordering of tests to the ordering within the class.
- Try to follow the [Four-Phase Test][four-phase-test] pattern, using newlines
to separate phases.
......
......@@ -20,12 +20,12 @@ describe 'mail_room.yml' do
YAML.load(output)
end
before(:each) do
before do
stub_env('GITLAB_REDIS_QUEUES_CONFIG_FILE', absolute_path(queues_config_path))
clear_queues_raw_config
end
after(:each) do
after do
clear_queues_raw_config
end
......
......@@ -320,7 +320,7 @@ describe AutocompleteController do
end
context 'authorized projects without admin_issue ability' do
before(:each) do
before do
authorized_project.add_guest(user)
expect(user.can?(:admin_issue, authorized_project)).to eq(false)
......
......@@ -5,7 +5,7 @@ describe RegistrationsController do
let(:user_params) { { user: { name: 'new_user', username: 'new_username', email: 'new@user.com', password: 'Any_password' } } }
context 'email confirmation' do
around(:each) do |example|
around do |example|
perform_enqueued_jobs do
example.run
end
......
......@@ -57,7 +57,7 @@ feature 'Edit group settings' do
TestEnv.clean_test_path
end
after(:example) do
after do
TestEnv.clean_test_path
end
......
......@@ -5,7 +5,7 @@ feature 'Diffs URL', js: true do
let(:merge_request) { create(:merge_request, source_project: project) }
context 'when visit with */* as accept header' do
before(:each) do
before do
page.driver.add_header('Accept', '*/*')
end
......
......@@ -26,7 +26,7 @@ feature 'Merge Request Discussions' do
let(:outdated_diff_refs) { project.commit("874797c3a73b60d2187ed6e2fcabd289ff75171e").diff_refs }
before(:each) do
before do
visit project_merge_request_path(project, merge_request)
end
......@@ -71,7 +71,7 @@ feature 'Merge Request Discussions' do
end
end
before(:each) do
before do
visit project_merge_request_path(project, merge_request)
end
......
......@@ -71,7 +71,7 @@ feature 'Merge requests > User posts diff notes', :js do
end
context 'with an unfolded line' do
before(:each) do
before do
find('.js-unfold', match: :first).click
wait_for_requests
end
......@@ -120,7 +120,7 @@ feature 'Merge requests > User posts diff notes', :js do
end
context 'with an unfolded line' do
before(:each) do
before do
find('.js-unfold', match: :first).click
wait_for_requests
end
......
......@@ -35,7 +35,7 @@ feature 'Profile > Account' do
TestEnv.clean_test_path
end
after(:example) do
after do
TestEnv.clean_test_path
end
......
......@@ -10,7 +10,7 @@ feature 'Import/Export - project import integration test', js: true do
allow_any_instance_of(Gitlab::ImportExport).to receive(:storage_path).and_return(export_path)
end
after(:each) do
after do
FileUtils.rm_rf(export_path, secure: true)
end
......
......@@ -65,7 +65,7 @@ describe 'Edit Project Settings' do
TestEnv.clean_test_path
end
after(:example) do
after do
TestEnv.clean_test_path
end
......@@ -107,11 +107,11 @@ describe 'Edit Project Settings' do
TestEnv.clean_test_path
end
before(:example) do
before do
group.add_owner(user)
end
after(:example) do
after do
TestEnv.clean_test_path
end
......
......@@ -82,7 +82,7 @@ feature 'Triggers', js: true do
end
describe 'trigger "Take ownership" workflow' do
before(:each) do
before do
create(:ci_trigger, owner: user2, project: @project, description: trigger_title)
visit project_settings_ci_cd_path(@project)
end
......@@ -104,7 +104,7 @@ feature 'Triggers', js: true do
end
describe 'trigger "Revoke" workflow' do
before(:each) do
before do
create(:ci_trigger, owner: user2, project: @project, description: trigger_title)
visit project_settings_ci_cd_path(@project)
end
......
......@@ -73,7 +73,7 @@ feature 'Users', js: true do
let(:loading_icon) { '.fa.fa-spinner' }
let(:username_input) { 'new_user_username' }
before(:each) do
before do
visit new_user_session_path
click_link 'Register'
end
......
......@@ -14,7 +14,7 @@ describe Admin::AbuseReportsController, '(JavaScript fixtures)', type: :controll
clean_frontend_fixtures('abuse_reports/')
end
before(:each) do
before do
sign_in(admin)
end
......
......@@ -13,7 +13,7 @@ describe Projects::BlobController, '(JavaScript fixtures)', type: :controller do
clean_frontend_fixtures('blob/')
end
before(:each) do
before do
sign_in(admin)
end
......
......@@ -13,7 +13,7 @@ describe Projects::BoardsController, '(JavaScript fixtures)', type: :controller
clean_frontend_fixtures('boards/')
end
before(:each) do
before do
sign_in(admin)
end
......
......@@ -13,7 +13,7 @@ describe Projects::BranchesController, '(JavaScript fixtures)', type: :controlle
clean_frontend_fixtures('branches/')
end
before(:each) do
before do
sign_in(admin)
end
......
......@@ -13,7 +13,7 @@ describe Dashboard::ProjectsController, '(JavaScript fixtures)', type: :controll
clean_frontend_fixtures('dashboard/')
end
before(:each) do
before do
sign_in(admin)
end
......
......@@ -12,7 +12,7 @@ describe Projects::DeployKeysController, '(JavaScript fixtures)', type: :control
clean_frontend_fixtures('deploy_keys/')
end
before(:each) do
before do
sign_in(admin)
end
......
......@@ -14,7 +14,7 @@ describe Projects::EnvironmentsController, '(JavaScript fixtures)', type: :contr
clean_frontend_fixtures('environments/metrics')
end
before(:each) do
before do
sign_in(admin)
end
......
......@@ -13,7 +13,7 @@ describe Projects::IssuesController, '(JavaScript fixtures)', type: :controller
clean_frontend_fixtures('issues/')
end
before(:each) do
before do
sign_in(admin)
end
......
......@@ -17,7 +17,7 @@ describe Projects::JobsController, '(JavaScript fixtures)', type: :controller do
clean_frontend_fixtures('builds/')
end
before(:each) do
before do
sign_in(admin)
end
......
......@@ -22,7 +22,7 @@ describe 'Labels (JavaScript fixtures)' do
describe Groups::LabelsController, '(JavaScript fixtures)', type: :controller do
render_views
before(:each) do
before do
sign_in(admin)
end
......@@ -39,7 +39,7 @@ describe 'Labels (JavaScript fixtures)' do
describe Projects::LabelsController, '(JavaScript fixtures)', type: :controller do
render_views
before(:each) do
before do
sign_in(admin)
end
......
......@@ -33,7 +33,7 @@ describe Projects::MergeRequestsController, '(JavaScript fixtures)', type: :cont
clean_frontend_fixtures('merge_requests/')
end
before(:each) do
before do
sign_in(admin)
end
......
......@@ -25,7 +25,7 @@ describe Projects::MergeRequests::DiffsController, '(JavaScript fixtures)', type
clean_frontend_fixtures('merge_request_diffs/')
end
before(:each) do
before do
sign_in(admin)
end
......
......@@ -19,7 +19,7 @@ describe Projects::PipelinesController, '(JavaScript fixtures)', type: :controll
clean_frontend_fixtures('pipelines/')
end
before(:each) do
before do
sign_in(admin)
end
......
......@@ -13,7 +13,7 @@ describe ProjectsController, '(JavaScript fixtures)', type: :controller do
clean_frontend_fixtures('projects/')
end
before(:each) do
before do
sign_in(admin)
end
......
......@@ -14,7 +14,7 @@ describe Projects::ServicesController, '(JavaScript fixtures)', type: :controlle
clean_frontend_fixtures('services/prometheus')
end
before(:each) do
before do
sign_in(admin)
end
......
......@@ -15,7 +15,7 @@ describe Projects::ServicesController, '(JavaScript fixtures)', type: :controlle
clean_frontend_fixtures('services/')
end
before(:each) do
before do
sign_in(admin)
end
......
......@@ -14,7 +14,7 @@ describe SnippetsController, '(JavaScript fixtures)', type: :controller do
clean_frontend_fixtures('snippets/')
end
before(:each) do
before do
sign_in(admin)
end
......
......@@ -18,7 +18,7 @@ describe 'Todos (JavaScript fixtures)' do
describe Dashboard::TodosController, '(JavaScript fixtures)', type: :controller do
render_views
before(:each) do
before do
sign_in(admin)
end
......@@ -33,7 +33,7 @@ describe 'Todos (JavaScript fixtures)' do
describe Projects::TodosController, '(JavaScript fixtures)', type: :controller do
render_views
before(:each) do
before do
sign_in(admin)
end
......
......@@ -13,7 +13,7 @@ describe Gitlab::Daemon do
allow(Kernel).to receive(:at_exit)
end
after(:each) do
after do
described_class.instance_variable_set(:@instance, nil)
end
......
......@@ -6,7 +6,7 @@ describe Gitlab::DataBuilder::Note do
let(:data) { described_class.build(note, user) }
let(:fixed_time) { Time.at(1425600000) } # Avoid time precision errors
before(:each) do
before do
expect(data).to have_key(:object_attributes)
expect(data[:object_attributes]).to have_key(:url)
expect(data[:object_attributes][:url])
......
......@@ -384,7 +384,7 @@ describe Gitlab::Git::DiffCollection, seed_helper: true do
context 'when go over safe limits on files' do
let(:iterator) { [fake_diff(1, 1)] * 4 }
before(:each) do
before do
stub_const('Gitlab::Git::DiffCollection::DEFAULT_LIMITS', { max_files: 2, max_lines: max_lines })
end
......@@ -409,7 +409,7 @@ describe Gitlab::Git::DiffCollection, seed_helper: true do
]
end
before(:each) do
before do
stub_const('Gitlab::Git::DiffCollection::DEFAULT_LIMITS', { max_files: max_files, max_lines: 80 })
end
......@@ -434,7 +434,7 @@ describe Gitlab::Git::DiffCollection, seed_helper: true do
]
end
before(:each) do
before do
stub_const('Gitlab::Git::DiffCollection::DEFAULT_LIMITS', { max_files: max_files, max_lines: 80 })
end
......
......@@ -1023,7 +1023,7 @@ describe Gitlab::Git::Repository, seed_helper: true do
end
context "with no .gitattrbutes" do
before(:each) do
before do
repository.copy_gitattributes("master")
end
......@@ -1031,13 +1031,13 @@ describe Gitlab::Git::Repository, seed_helper: true do
expect(File.exist?(attributes_path)).to be_falsey
end
after(:each) do
after do
FileUtils.rm_rf(attributes_path)
end
end
context "with .gitattrbutes" do
before(:each) do
before do
repository.copy_gitattributes("gitattributes")
end
......@@ -1050,13 +1050,13 @@ describe Gitlab::Git::Repository, seed_helper: true do
expect(contents).to eq("*.md binary\n")
end
after(:each) do
after do
FileUtils.rm_rf(attributes_path)
end
end
context "with updated .gitattrbutes" do
before(:each) do
before do
repository.copy_gitattributes("gitattributes")
repository.copy_gitattributes("gitattributes-updated")
end
......@@ -1070,13 +1070,13 @@ describe Gitlab::Git::Repository, seed_helper: true do
expect(contents).to eq("*.txt binary\n")
end
after(:each) do
after do
FileUtils.rm_rf(attributes_path)
end
end
context "with no .gitattrbutes in HEAD but with previous info/attributes" do
before(:each) do
before do
repository.copy_gitattributes("gitattributes")
repository.copy_gitattributes("master")
end
......@@ -1085,7 +1085,7 @@ describe Gitlab::Git::Repository, seed_helper: true do
expect(File.exist?(attributes_path)).to be_falsey
end
after(:each) do
after do
FileUtils.rm_rf(attributes_path)
end
end
......
......@@ -174,7 +174,7 @@ describe Gitlab::Git::Storage::CircuitBreaker, clean_gitlab_redis_shared_state:
end
describe '#track_storage_inaccessible' do
around(:each) do |example|
around do |example|
Timecop.freeze
example.run
......
......@@ -107,7 +107,7 @@ describe Gitlab::HealthChecks::FsShardsCheck do
end
# Unsolved intermittent failure in CI https://gitlab.com/gitlab-org/gitlab-ce/issues/31128
around(:each) do |example| # rubocop:disable RSpec/AroundBlock
around do |example| # rubocop:disable RSpec/AroundBlock
times_to_try = ENV['CI'] ? 4 : 1
example.run_with_retry retry: times_to_try
end
......
......@@ -7,7 +7,7 @@ describe Gitlab::RequestContext do
let(:env) { Hash.new }
context 'when RequestStore::Middleware is used' do
around(:each) do |example|
around do |example|
RequestStore::Middleware.new(-> (env) { example.run }).call({})
end
......
......@@ -545,7 +545,7 @@ describe Notify do
let(:note_author) { create(:user, name: 'author_name') }
let(:note) { create(:note, project: project, author: note_author) }
before :each do
before do
allow(Note).to receive(:find).with(note.id).and_return(note)
end
......@@ -661,7 +661,7 @@ describe Notify do
let(:project) { create(:project, :repository) }
let(:note_author) { create(:user, name: 'author_name') }
before :each do
before do
allow(Note).to receive(:find).with(note.id).and_return(note)
end
......@@ -779,7 +779,7 @@ describe Notify do
context 'items that are noteable, the email for a diff discussion note' do
let(:note_author) { create(:user, name: 'author_name') }
before :each do
before do
allow(Note).to receive(:find).with(note.id).and_return(note)
end
......
......@@ -300,7 +300,7 @@ describe Issuable do
let(:bug) { create(:label, project: project, title: 'bug') }
let(:issue) { create(:issue, project: project) }
before(:each) do
before do
issue.labels << bug
end
......@@ -402,7 +402,7 @@ describe Issuable do
let(:issue2) { create(:issue, title: "Bugfix2", project: project) }
let(:issue3) { create(:issue, title: "Feature1", project: project) }
before(:each) do
before do
issue1.labels << bug
issue1.labels << feature
issue2.labels << bug
......
......@@ -31,7 +31,7 @@ describe ReactiveCaching, :use_clean_rails_memory_store_caching do
let(:now) { Time.now.utc }
around(:each) do |example|
around do |example|
Timecop.freeze(now) { example.run }
end
......
......@@ -714,7 +714,7 @@ describe MergeRequest do
end
describe 'caching' do
before(:example) do
before do
allow(Rails).to receive(:cache).and_return(ActiveSupport::Cache::MemoryStore.new)
end
......
......@@ -43,7 +43,7 @@ describe DroneCiService, :use_clean_rails_memory_store_caching do
let(:build_page) { "#{drone_url}/gitlab/#{path}/redirect/commits/#{sha}?branch=#{branch}" }
let(:commit_status_path) { "#{drone_url}/gitlab/#{path}/commits/#{sha}?branch=#{branch}&access_token=#{token}" }
before(:each) do
before do
allow(drone).to receive_messages(
project_id: project.id,
project: project,
......
......@@ -36,7 +36,7 @@ describe HipchatService do
Gitlab::DataBuilder::Push.build_sample(project, user)
end
before(:each) do
before do
allow(hipchat).to receive_messages(
project_id: project.id,
project: project,
......
......@@ -651,7 +651,7 @@ describe Project do
let(:ext_project) { create(:redmine_project) }
context 'on existing projects with no value for has_external_issue_tracker' do
before(:each) do
before do
project.update_column(:has_external_issue_tracker, nil)
ext_project.update_column(:has_external_issue_tracker, nil)
end
......
......@@ -512,7 +512,7 @@ describe API::Groups do
let(:project) { create(:project) }
let(:project_path) { CGI.escape(project.full_path) }
before(:each) do
before do
allow_any_instance_of(Projects::TransferService)
.to receive(:execute).and_return(true)
end
......
......@@ -580,7 +580,7 @@ describe API::MergeRequests do
let!(:fork_project) { create(:project, forked_from_project: project, namespace: user2.namespace, creator_id: user2.id) }
let!(:unrelated_project) { create(:project, namespace: create(:user).namespace, creator_id: user2.id) }
before :each do |each|
before do |each|
fork_project.team << [user2, :reporter]
end
......
......@@ -504,7 +504,7 @@ describe API::V3::Groups do
let(:project) { create(:project) }
let(:project_path) { CGI.escape(project.full_path) }
before(:each) do
before do
allow_any_instance_of(Projects::TransferService)
.to receive(:execute).and_return(true)
end
......
......@@ -315,7 +315,7 @@ describe API::MergeRequests do
let!(:fork_project) { create(:project, forked_from_project: project, namespace: user2.namespace, creator_id: user2.id) }
let!(:unrelated_project) { create(:project, namespace: create(:user).namespace, creator_id: user2.id) }
before :each do |each|
before do |each|
fork_project.team << [user2, :reporter]
end
......
......@@ -13,7 +13,7 @@ describe Labels::UpdateService do
let(:expected_saved_color) { hex_color }
before(:each) do
before do
@label = Labels::CreateService.new(title: 'Initial', color: '#000000').execute(project: project)
expect(@label).to be_persisted
end
......
......@@ -4,7 +4,7 @@ describe NotificationService, :mailer do
let(:notification) { described_class.new }
let(:assignee) { create(:user) }
around(:each) do |example|
around do |example|
perform_enqueued_jobs do
example.run
end
......@@ -1196,7 +1196,7 @@ describe NotificationService, :mailer do
let(:group) { create(:group) }
let(:member) { create(:user) }
before(:each) do
before do
group.add_owner(creator)
group.add_developer(member, creator)
end
......@@ -1216,7 +1216,7 @@ describe NotificationService, :mailer do
let(:project) { create(:project) }
let(:member) { create(:user) }
before(:each) do
before do
project.add_developer(member, current_user: project.owner)
end
......
......@@ -24,7 +24,7 @@ describe Projects::HousekeepingService do
end
context 'when no lease can be obtained' do
before(:each) do
before do
expect(subject).to receive(:try_obtain_lease).and_return(false)
end
......
......@@ -15,7 +15,7 @@ describe WebHookService do
let(:service_instance) { described_class.new(project_hook, data, 'push_hooks') }
describe '#execute' do
before(:each) do
before do
project.hooks << [project_hook]
WebMock.stub_request(:post, project_hook.url)
......
......@@ -3,12 +3,12 @@ RSpec.shared_examples "redis_shared_examples" do
let(:test_redis_url) { "redis://redishost:#{redis_port}"}
before(:each) do
before do
stub_env(environment_config_file_name, Rails.root.join(config_file_name))
clear_raw_config
end
after(:each) do
after do
clear_raw_config
end
......
shared_context 'unique ips sign in limit' do
include StubENV
before(:each) do
before do
Gitlab::Redis::Cache.with(&:flushall)
Gitlab::Redis::Queues.with(&:flushall)
Gitlab::Redis::SharedState.with(&:flushall)
......
......@@ -14,7 +14,7 @@ describe ConfigLint do
end
describe 'config_lint rake task' do
before(:each) do
before do
# Prevent `system` from actually being called
allow(Kernel).to receive(:system).and_return(true)
end
......
......@@ -119,7 +119,7 @@ describe 'gitlab:app namespace rake task' do
let(:project) { create(:project, :repository) }
let(:user_backup_path) { "repositories/#{project.disk_path}" }
before(:each) do
before do
@origin_cd = Dir.pwd
path = File.join(project.repository.path_to_repo, filename)
......@@ -130,7 +130,7 @@ describe 'gitlab:app namespace rake task' do
create_backup
end
after(:each) do
after do
ENV["SKIP"] = ""
FileUtils.rm(@backup_tar)
Dir.chdir(@origin_cd)
......
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