From c8b802471bcb6ea9ea7af3c7a139598a1f945230 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Thu, 10 Aug 2017 18:31:42 -0400 Subject: [PATCH 1/5] Enable the RSpec/HookArgument cop and auto-correct offenses --- .rubocop.yml | 5 +++++ .rubocop_todo.yml | 6 ------ spec/config/mail_room_spec.rb | 4 ++-- spec/controllers/autocomplete_controller_spec.rb | 2 +- .../controllers/registrations_controller_spec.rb | 2 +- spec/features/groups/group_settings_spec.rb | 2 +- spec/features/merge_requests/diffs_spec.rb | 2 +- spec/features/merge_requests/discussion_spec.rb | 4 ++-- .../merge_requests/user_posts_diff_notes_spec.rb | 4 ++-- spec/features/profiles/account_spec.rb | 2 +- .../projects/import_export/import_file_spec.rb | 2 +- spec/features/projects/project_settings_spec.rb | 6 +++--- spec/features/triggers_spec.rb | 4 ++-- spec/features/users_spec.rb | 2 +- spec/javascripts/fixtures/abuse_reports.rb | 2 +- spec/javascripts/fixtures/blob.rb | 2 +- spec/javascripts/fixtures/boards.rb | 2 +- spec/javascripts/fixtures/branches.rb | 2 +- spec/javascripts/fixtures/dashboard.rb | 2 +- spec/javascripts/fixtures/deploy_keys.rb | 2 +- spec/javascripts/fixtures/environments.rb | 2 +- spec/javascripts/fixtures/issues.rb | 2 +- spec/javascripts/fixtures/jobs.rb | 2 +- spec/javascripts/fixtures/labels.rb | 4 ++-- spec/javascripts/fixtures/merge_requests.rb | 2 +- .../javascripts/fixtures/merge_requests_diffs.rb | 2 +- spec/javascripts/fixtures/pipelines.rb | 2 +- spec/javascripts/fixtures/projects.rb | 2 +- spec/javascripts/fixtures/prometheus_service.rb | 2 +- spec/javascripts/fixtures/services.rb | 2 +- spec/javascripts/fixtures/snippet.rb | 2 +- spec/javascripts/fixtures/todos.rb | 4 ++-- spec/lib/gitlab/daemon_spec.rb | 2 +- spec/lib/gitlab/data_builder/note_spec.rb | 2 +- spec/lib/gitlab/git/diff_collection_spec.rb | 6 +++--- spec/lib/gitlab/git/repository_spec.rb | 16 ++++++++-------- .../gitlab/git/storage/circuit_breaker_spec.rb | 2 +- .../gitlab/health_checks/fs_shards_check_spec.rb | 2 +- spec/lib/gitlab/request_context_spec.rb | 2 +- spec/mailers/notify_spec.rb | 6 +++--- spec/models/concerns/issuable_spec.rb | 4 ++-- spec/models/concerns/reactive_caching_spec.rb | 2 +- spec/models/merge_request_spec.rb | 2 +- .../project_services/drone_ci_service_spec.rb | 2 +- .../project_services/hipchat_service_spec.rb | 2 +- spec/models/project_spec.rb | 2 +- spec/requests/api/groups_spec.rb | 2 +- spec/requests/api/merge_requests_spec.rb | 2 +- spec/requests/api/v3/groups_spec.rb | 2 +- spec/requests/api/v3/merge_requests_spec.rb | 2 +- spec/services/labels/update_service_spec.rb | 2 +- spec/services/notification_service_spec.rb | 6 +++--- .../projects/housekeeping_service_spec.rb | 2 +- spec/services/web_hook_service_spec.rb | 2 +- spec/support/redis/redis_shared_examples.rb | 4 ++-- spec/support/unique_ip_check_shared_examples.rb | 2 +- spec/tasks/config_lint_spec.rb | 2 +- spec/tasks/gitlab/backup_rake_spec.rb | 4 ++-- 58 files changed, 85 insertions(+), 86 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 84e4a3c2e49..18d8e009da6 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -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: diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 78ab7f7204f..cf14285ec2a 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -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 diff --git a/spec/config/mail_room_spec.rb b/spec/config/mail_room_spec.rb index a31e44fa928..74634dac713 100644 --- a/spec/config/mail_room_spec.rb +++ b/spec/config/mail_room_spec.rb @@ -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 diff --git a/spec/controllers/autocomplete_controller_spec.rb b/spec/controllers/autocomplete_controller_spec.rb index 379e3ce690f..2fbab1e4040 100644 --- a/spec/controllers/autocomplete_controller_spec.rb +++ b/spec/controllers/autocomplete_controller_spec.rb @@ -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) diff --git a/spec/controllers/registrations_controller_spec.rb b/spec/controllers/registrations_controller_spec.rb index 275181a3d64..5a4ab39ab86 100644 --- a/spec/controllers/registrations_controller_spec.rb +++ b/spec/controllers/registrations_controller_spec.rb @@ -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 diff --git a/spec/features/groups/group_settings_spec.rb b/spec/features/groups/group_settings_spec.rb index acb21eab03f..d0316cfb18d 100644 --- a/spec/features/groups/group_settings_spec.rb +++ b/spec/features/groups/group_settings_spec.rb @@ -57,7 +57,7 @@ feature 'Edit group settings' do TestEnv.clean_test_path end - after(:example) do + after do TestEnv.clean_test_path end diff --git a/spec/features/merge_requests/diffs_spec.rb b/spec/features/merge_requests/diffs_spec.rb index 201be4b9e40..a8f5dc275e4 100644 --- a/spec/features/merge_requests/diffs_spec.rb +++ b/spec/features/merge_requests/diffs_spec.rb @@ -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 diff --git a/spec/features/merge_requests/discussion_spec.rb b/spec/features/merge_requests/discussion_spec.rb index d1cc43e0690..05789bbd31d 100644 --- a/spec/features/merge_requests/discussion_spec.rb +++ b/spec/features/merge_requests/discussion_spec.rb @@ -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 diff --git a/spec/features/merge_requests/user_posts_diff_notes_spec.rb b/spec/features/merge_requests/user_posts_diff_notes_spec.rb index 1cfd78663e5..f89dd38e5cd 100644 --- a/spec/features/merge_requests/user_posts_diff_notes_spec.rb +++ b/spec/features/merge_requests/user_posts_diff_notes_spec.rb @@ -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 diff --git a/spec/features/profiles/account_spec.rb b/spec/features/profiles/account_spec.rb index 9944a6e1ff1..dcd0449dbcb 100644 --- a/spec/features/profiles/account_spec.rb +++ b/spec/features/profiles/account_spec.rb @@ -35,7 +35,7 @@ feature 'Profile > Account' do TestEnv.clean_test_path end - after(:example) do + after do TestEnv.clean_test_path end diff --git a/spec/features/projects/import_export/import_file_spec.rb b/spec/features/projects/import_export/import_file_spec.rb index 24e7843db63..6a324d32ca7 100644 --- a/spec/features/projects/import_export/import_file_spec.rb +++ b/spec/features/projects/import_export/import_file_spec.rb @@ -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 diff --git a/spec/features/projects/project_settings_spec.rb b/spec/features/projects/project_settings_spec.rb index 7d4ec2b4e68..80d91e5915f 100644 --- a/spec/features/projects/project_settings_spec.rb +++ b/spec/features/projects/project_settings_spec.rb @@ -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 diff --git a/spec/features/triggers_spec.rb b/spec/features/triggers_spec.rb index 8d12a492feb..47664de469a 100644 --- a/spec/features/triggers_spec.rb +++ b/spec/features/triggers_spec.rb @@ -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 diff --git a/spec/features/users_spec.rb b/spec/features/users_spec.rb index 1124b42721f..15b89dac572 100644 --- a/spec/features/users_spec.rb +++ b/spec/features/users_spec.rb @@ -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 diff --git a/spec/javascripts/fixtures/abuse_reports.rb b/spec/javascripts/fixtures/abuse_reports.rb index de673f94d72..387858cba77 100644 --- a/spec/javascripts/fixtures/abuse_reports.rb +++ b/spec/javascripts/fixtures/abuse_reports.rb @@ -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 diff --git a/spec/javascripts/fixtures/blob.rb b/spec/javascripts/fixtures/blob.rb index 16490ad5039..2dffc42b0ef 100644 --- a/spec/javascripts/fixtures/blob.rb +++ b/spec/javascripts/fixtures/blob.rb @@ -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 diff --git a/spec/javascripts/fixtures/boards.rb b/spec/javascripts/fixtures/boards.rb index d7c3dc0a235..494c9cabdcc 100644 --- a/spec/javascripts/fixtures/boards.rb +++ b/spec/javascripts/fixtures/boards.rb @@ -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 diff --git a/spec/javascripts/fixtures/branches.rb b/spec/javascripts/fixtures/branches.rb index a059818145b..bb3bdf7c215 100644 --- a/spec/javascripts/fixtures/branches.rb +++ b/spec/javascripts/fixtures/branches.rb @@ -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 diff --git a/spec/javascripts/fixtures/dashboard.rb b/spec/javascripts/fixtures/dashboard.rb index e83db8daaf2..793ffa7c220 100644 --- a/spec/javascripts/fixtures/dashboard.rb +++ b/spec/javascripts/fixtures/dashboard.rb @@ -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 diff --git a/spec/javascripts/fixtures/deploy_keys.rb b/spec/javascripts/fixtures/deploy_keys.rb index fca3f5b1bfe..bea161c514f 100644 --- a/spec/javascripts/fixtures/deploy_keys.rb +++ b/spec/javascripts/fixtures/deploy_keys.rb @@ -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 diff --git a/spec/javascripts/fixtures/environments.rb b/spec/javascripts/fixtures/environments.rb index 3474f4696ef..d2457d75419 100644 --- a/spec/javascripts/fixtures/environments.rb +++ b/spec/javascripts/fixtures/environments.rb @@ -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 diff --git a/spec/javascripts/fixtures/issues.rb b/spec/javascripts/fixtures/issues.rb index 1a30909977e..d3ad50af1b9 100644 --- a/spec/javascripts/fixtures/issues.rb +++ b/spec/javascripts/fixtures/issues.rb @@ -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 diff --git a/spec/javascripts/fixtures/jobs.rb b/spec/javascripts/fixtures/jobs.rb index dc7dde1138c..83a96797506 100644 --- a/spec/javascripts/fixtures/jobs.rb +++ b/spec/javascripts/fixtures/jobs.rb @@ -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 diff --git a/spec/javascripts/fixtures/labels.rb b/spec/javascripts/fixtures/labels.rb index 2e4811b64a4..814f065f3a4 100644 --- a/spec/javascripts/fixtures/labels.rb +++ b/spec/javascripts/fixtures/labels.rb @@ -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 diff --git a/spec/javascripts/fixtures/merge_requests.rb b/spec/javascripts/fixtures/merge_requests.rb index f9d8b5c569c..f97a5d2b5de 100644 --- a/spec/javascripts/fixtures/merge_requests.rb +++ b/spec/javascripts/fixtures/merge_requests.rb @@ -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 diff --git a/spec/javascripts/fixtures/merge_requests_diffs.rb b/spec/javascripts/fixtures/merge_requests_diffs.rb index 4481a187f63..6e0a97d2e3f 100644 --- a/spec/javascripts/fixtures/merge_requests_diffs.rb +++ b/spec/javascripts/fixtures/merge_requests_diffs.rb @@ -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 diff --git a/spec/javascripts/fixtures/pipelines.rb b/spec/javascripts/fixtures/pipelines.rb index daafbac86db..bb85da50f0f 100644 --- a/spec/javascripts/fixtures/pipelines.rb +++ b/spec/javascripts/fixtures/pipelines.rb @@ -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 diff --git a/spec/javascripts/fixtures/projects.rb b/spec/javascripts/fixtures/projects.rb index 6c33b240e5c..f09d44a49d1 100644 --- a/spec/javascripts/fixtures/projects.rb +++ b/spec/javascripts/fixtures/projects.rb @@ -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 diff --git a/spec/javascripts/fixtures/prometheus_service.rb b/spec/javascripts/fixtures/prometheus_service.rb index 3200577b326..7a46e47bb15 100644 --- a/spec/javascripts/fixtures/prometheus_service.rb +++ b/spec/javascripts/fixtures/prometheus_service.rb @@ -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 diff --git a/spec/javascripts/fixtures/services.rb b/spec/javascripts/fixtures/services.rb index 554451d1bbf..0a3c64d5d31 100644 --- a/spec/javascripts/fixtures/services.rb +++ b/spec/javascripts/fixtures/services.rb @@ -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 diff --git a/spec/javascripts/fixtures/snippet.rb b/spec/javascripts/fixtures/snippet.rb index cc825c82190..01bfb87b0c1 100644 --- a/spec/javascripts/fixtures/snippet.rb +++ b/spec/javascripts/fixtures/snippet.rb @@ -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 diff --git a/spec/javascripts/fixtures/todos.rb b/spec/javascripts/fixtures/todos.rb index a81ef8c5492..ba630365c18 100644 --- a/spec/javascripts/fixtures/todos.rb +++ b/spec/javascripts/fixtures/todos.rb @@ -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 diff --git a/spec/lib/gitlab/daemon_spec.rb b/spec/lib/gitlab/daemon_spec.rb index c519984a267..2bdb0dfc736 100644 --- a/spec/lib/gitlab/daemon_spec.rb +++ b/spec/lib/gitlab/daemon_spec.rb @@ -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 diff --git a/spec/lib/gitlab/data_builder/note_spec.rb b/spec/lib/gitlab/data_builder/note_spec.rb index 6415e4083d6..aaa42566a4d 100644 --- a/spec/lib/gitlab/data_builder/note_spec.rb +++ b/spec/lib/gitlab/data_builder/note_spec.rb @@ -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]) diff --git a/spec/lib/gitlab/git/diff_collection_spec.rb b/spec/lib/gitlab/git/diff_collection_spec.rb index 0cfb210e390..3494f0cc98d 100644 --- a/spec/lib/gitlab/git/diff_collection_spec.rb +++ b/spec/lib/gitlab/git/diff_collection_spec.rb @@ -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 diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index a90c848432e..04c86a2c1a7 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -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 diff --git a/spec/lib/gitlab/git/storage/circuit_breaker_spec.rb b/spec/lib/gitlab/git/storage/circuit_breaker_spec.rb index b2886628601..7256402b010 100644 --- a/spec/lib/gitlab/git/storage/circuit_breaker_spec.rb +++ b/spec/lib/gitlab/git/storage/circuit_breaker_spec.rb @@ -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 diff --git a/spec/lib/gitlab/health_checks/fs_shards_check_spec.rb b/spec/lib/gitlab/health_checks/fs_shards_check_spec.rb index 26574df8bb5..a0e5e401359 100644 --- a/spec/lib/gitlab/health_checks/fs_shards_check_spec.rb +++ b/spec/lib/gitlab/health_checks/fs_shards_check_spec.rb @@ -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 diff --git a/spec/lib/gitlab/request_context_spec.rb b/spec/lib/gitlab/request_context_spec.rb index e272bdb9284..8a28ad0e597 100644 --- a/spec/lib/gitlab/request_context_spec.rb +++ b/spec/lib/gitlab/request_context_spec.rb @@ -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 diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb index e36d7a1800c..1fa59ebd22b 100644 --- a/spec/mailers/notify_spec.rb +++ b/spec/mailers/notify_spec.rb @@ -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 diff --git a/spec/models/concerns/issuable_spec.rb b/spec/models/concerns/issuable_spec.rb index 0137f71be8f..dfbe1a7c192 100644 --- a/spec/models/concerns/issuable_spec.rb +++ b/spec/models/concerns/issuable_spec.rb @@ -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 diff --git a/spec/models/concerns/reactive_caching_spec.rb b/spec/models/concerns/reactive_caching_spec.rb index 5f9b7e0a367..a5d505af001 100644 --- a/spec/models/concerns/reactive_caching_spec.rb +++ b/spec/models/concerns/reactive_caching_spec.rb @@ -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 diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 4aada17c8c0..026bdbd26d1 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -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 diff --git a/spec/models/project_services/drone_ci_service_spec.rb b/spec/models/project_services/drone_ci_service_spec.rb index 5b0f24ce306..d8972aff407 100644 --- a/spec/models/project_services/drone_ci_service_spec.rb +++ b/spec/models/project_services/drone_ci_service_spec.rb @@ -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, diff --git a/spec/models/project_services/hipchat_service_spec.rb b/spec/models/project_services/hipchat_service_spec.rb index 7614bb897e8..23db29cb541 100644 --- a/spec/models/project_services/hipchat_service_spec.rb +++ b/spec/models/project_services/hipchat_service_spec.rb @@ -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, diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 4a80f41cdc9..a28e92446ea 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -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 diff --git a/spec/requests/api/groups_spec.rb b/spec/requests/api/groups_spec.rb index eba1db15da6..313c38cd29c 100644 --- a/spec/requests/api/groups_spec.rb +++ b/spec/requests/api/groups_spec.rb @@ -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 diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index 1e8eccb9b1c..9a6072e7eb7 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -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 diff --git a/spec/requests/api/v3/groups_spec.rb b/spec/requests/api/v3/groups_spec.rb index 10756e494c3..778fcc73c30 100644 --- a/spec/requests/api/v3/groups_spec.rb +++ b/spec/requests/api/v3/groups_spec.rb @@ -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 diff --git a/spec/requests/api/v3/merge_requests_spec.rb b/spec/requests/api/v3/merge_requests_spec.rb index 18d0a804137..ec684e7b9cd 100644 --- a/spec/requests/api/v3/merge_requests_spec.rb +++ b/spec/requests/api/v3/merge_requests_spec.rb @@ -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 diff --git a/spec/services/labels/update_service_spec.rb b/spec/services/labels/update_service_spec.rb index bb95fe20fbf..c3fe33045fa 100644 --- a/spec/services/labels/update_service_spec.rb +++ b/spec/services/labels/update_service_spec.rb @@ -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 diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index bd8ff5aaaa7..64981c199e4 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -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 diff --git a/spec/services/projects/housekeeping_service_spec.rb b/spec/services/projects/housekeeping_service_spec.rb index ebed802708d..385f56e447f 100644 --- a/spec/services/projects/housekeeping_service_spec.rb +++ b/spec/services/projects/housekeeping_service_spec.rb @@ -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 diff --git a/spec/services/web_hook_service_spec.rb b/spec/services/web_hook_service_spec.rb index 79d90defd78..365cb6b8f09 100644 --- a/spec/services/web_hook_service_spec.rb +++ b/spec/services/web_hook_service_spec.rb @@ -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) diff --git a/spec/support/redis/redis_shared_examples.rb b/spec/support/redis/redis_shared_examples.rb index f9552e41894..8676f895a83 100644 --- a/spec/support/redis/redis_shared_examples.rb +++ b/spec/support/redis/redis_shared_examples.rb @@ -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 diff --git a/spec/support/unique_ip_check_shared_examples.rb b/spec/support/unique_ip_check_shared_examples.rb index ff0b47899f5..2dfa5fbecea 100644 --- a/spec/support/unique_ip_check_shared_examples.rb +++ b/spec/support/unique_ip_check_shared_examples.rb @@ -1,6 +1,6 @@ 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) diff --git a/spec/tasks/config_lint_spec.rb b/spec/tasks/config_lint_spec.rb index 5b01665019a..83d54259dfa 100644 --- a/spec/tasks/config_lint_spec.rb +++ b/spec/tasks/config_lint_spec.rb @@ -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 diff --git a/spec/tasks/gitlab/backup_rake_spec.rb b/spec/tasks/gitlab/backup_rake_spec.rb index fae92451b46..0c8c8a2ab05 100644 --- a/spec/tasks/gitlab/backup_rake_spec.rb +++ b/spec/tasks/gitlab/backup_rake_spec.rb @@ -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) From add765e62313e15f0fbc9d0a18993e4096db5ab2 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Thu, 10 Aug 2017 18:33:44 -0400 Subject: [PATCH 2/5] Remove documentation about rules that are now enforced by RuboCop RuboCop is _living_ documentation! :sparkles: [ci skip] --- doc/development/testing.md | 6 ------ 1 file changed, 6 deletions(-) diff --git a/doc/development/testing.md b/doc/development/testing.md index 3d5aa3d45e9..ea94c87d8c6 100644 --- a/doc/development/testing.md +++ b/doc/development/testing.md @@ -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. From e66dcf49447d83ddfdaee52d1659ea1b2e0f442c Mon Sep 17 00:00:00 2001 From: Mehdi Lahmam Date: Fri, 11 Aug 2017 08:12:45 +0200 Subject: [PATCH 3/5] Remove an unnecessary `let` in spec/features/projects/user_edits_files_spec.rb It closes #36308 --- spec/features/projects/user_edits_files_spec.rb | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/spec/features/projects/user_edits_files_spec.rb b/spec/features/projects/user_edits_files_spec.rb index 8ae89c980b9..8c9fc8821e6 100644 --- a/spec/features/projects/user_edits_files_spec.rb +++ b/spec/features/projects/user_edits_files_spec.rb @@ -1,10 +1,6 @@ require 'spec_helper' describe 'User edits files' do - let(:fork_message) do - "You're not allowed to make changes to this project directly. "\ - "A fork of this project has been created that you can make changes in, so you can submit a merge request." - end let(:project) { create(:project, :repository, name: 'Shop') } let(:project2) { create(:project, :repository, name: 'Another Project', path: 'another-project') } let(:project_tree_path_root_ref) { project_tree_path(project, project.repository.root_ref) } @@ -91,7 +87,10 @@ describe 'User edits files' do click_link('Fork') - expect(page).to have_content(fork_message) + expect(page).to have_content( + "You're not allowed to make changes to this project directly. "\ + "A fork of this project has been created that you can make changes in, so you can submit a merge request." + ) execute_script("ace.edit('editor').setValue('*.rbca')") From 6debb556900ea8eed996e5ae5b580d30652875aa Mon Sep 17 00:00:00 2001 From: Tiago Botelho Date: Thu, 10 Aug 2017 16:50:14 +0100 Subject: [PATCH 4/5] Bumps omniauth-ldap gem version to 2.0.4 --- Gemfile | 2 +- Gemfile.lock | 4 ++-- .../unreleased/bump-omniauth-ldap-gem-version-2-0-4.yml | 4 ++++ 3 files changed, 7 insertions(+), 3 deletions(-) create mode 100644 changelogs/unreleased/bump-omniauth-ldap-gem-version-2-0-4.yml diff --git a/Gemfile b/Gemfile index 2a4b4717153..83f7199cbc4 100644 --- a/Gemfile +++ b/Gemfile @@ -64,7 +64,7 @@ gem 'gpgme' # LDAP Auth # GitLab fork with several improvements to original library. For full list of changes # see https://github.com/intridea/omniauth-ldap/compare/master...gitlabhq:master -gem 'gitlab_omniauth-ldap', '~> 2.0.3', require: 'omniauth-ldap' +gem 'gitlab_omniauth-ldap', '~> 2.0.4', require: 'omniauth-ldap' gem 'net-ldap' # Git Wiki diff --git a/Gemfile.lock b/Gemfile.lock index 79d1bc51358..3d435b6f901 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -289,7 +289,7 @@ GEM mime-types (>= 1.16, < 3) posix-spawn (~> 0.3) gitlab-markup (1.5.1) - gitlab_omniauth-ldap (2.0.3) + gitlab_omniauth-ldap (2.0.4) net-ldap (~> 0.16) omniauth (~> 1.3) pyu-ruby-sasl (>= 0.0.3.3, < 0.1) @@ -988,7 +988,7 @@ DEPENDENCIES github-linguist (~> 4.7.0) gitlab-flowdock-git-hook (~> 1.0.1) gitlab-markup (~> 1.5.1) - gitlab_omniauth-ldap (~> 2.0.3) + gitlab_omniauth-ldap (~> 2.0.4) gollum-lib (~> 4.2) gollum-rugged_adapter (~> 0.4.4) gon (~> 6.1.0) diff --git a/changelogs/unreleased/bump-omniauth-ldap-gem-version-2-0-4.yml b/changelogs/unreleased/bump-omniauth-ldap-gem-version-2-0-4.yml new file mode 100644 index 00000000000..7571999fa75 --- /dev/null +++ b/changelogs/unreleased/bump-omniauth-ldap-gem-version-2-0-4.yml @@ -0,0 +1,4 @@ +--- +title: Bumps omniauth-ldap gem version to 2.0.4 +merge_request: 13465 +author: From d2315054e6e8398fade9795f72830550b03fda0f Mon Sep 17 00:00:00 2001 From: Sean McGivern Date: Fri, 11 Aug 2017 11:58:21 +0100 Subject: [PATCH 5/5] Fix merge request diff deserialisation when too_large was absent Not all diffs in st_diffs had the too_large attribute set at all, but the column is non-nullable. Explicitly cast to boolean to avoid that problem. --- .../deserialize_merge_request_diffs_and_commits.rb | 2 ++ ...eserialize_merge_request_diffs_and_commits_spec.rb | 11 +++++++++++ 2 files changed, 13 insertions(+) diff --git a/lib/gitlab/background_migration/deserialize_merge_request_diffs_and_commits.rb b/lib/gitlab/background_migration/deserialize_merge_request_diffs_and_commits.rb index 0fbc6b70989..310a69a4bd4 100644 --- a/lib/gitlab/background_migration/deserialize_merge_request_diffs_and_commits.rb +++ b/lib/gitlab/background_migration/deserialize_merge_request_diffs_and_commits.rb @@ -85,6 +85,8 @@ module Gitlab diff_hash.tap do |hash| diff_text = hash[:diff] + hash[:too_large] = !!hash[:too_large] + if diff_text.encoding == Encoding::BINARY && !diff_text.ascii_only? hash[:binary] = true hash[:diff] = [diff_text].pack('m0') diff --git a/spec/lib/gitlab/background_migration/deserialize_merge_request_diffs_and_commits_spec.rb b/spec/lib/gitlab/background_migration/deserialize_merge_request_diffs_and_commits_spec.rb index f4dfa53f050..7cd2ce82eda 100644 --- a/spec/lib/gitlab/background_migration/deserialize_merge_request_diffs_and_commits_spec.rb +++ b/spec/lib/gitlab/background_migration/deserialize_merge_request_diffs_and_commits_spec.rb @@ -123,6 +123,17 @@ describe Gitlab::BackgroundMigration::DeserializeMergeRequestDiffsAndCommits do include_examples 'updated MR diff' end + context 'when the merge request diffs do not have too_large set' do + let(:commits) { merge_request_diff.commits.map(&:to_hash) } + let(:expected_diffs) { diffs_to_hashes(merge_request_diff.merge_request_diff_files) } + + let(:diffs) do + expected_diffs.map { |diff| diff.except(:too_large) } + end + + include_examples 'updated MR diff' + end + context 'when the merge request diffs have binary content' do let(:commits) { merge_request_diff.commits.map(&:to_hash) } let(:expected_diffs) { diffs }