Merge remote-tracking branch 'upstream/master' into 36089-handle-ref-failure-better

* upstream/master:
  Fix merge request diff deserialisation when too_large was absent
  Bumps omniauth-ldap gem version to 2.0.4
  Remove an unnecessary `let` in spec/features/projects/user_edits_files_spec.rb
  Remove documentation about rules that are now enforced by RuboCop
  Enable the RSpec/HookArgument cop and auto-correct offenses
This commit is contained in:
Lin Jen-Shin 2017-08-11 20:30:14 +08:00
commit 3b0dcbf235
63 changed files with 107 additions and 98 deletions

View file

@ -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:

View file

@ -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

View file

@ -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

View file

@ -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)

View file

@ -0,0 +1,4 @@
---
title: Bumps omniauth-ldap gem version to 2.0.4
merge_request: 13465
author:

View file

@ -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.

View file

@ -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')

View file

@ -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

View file

@ -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)

View file

@ -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

View file

@ -57,7 +57,7 @@ feature 'Edit group settings' do
TestEnv.clean_test_path
end
after(:example) do
after do
TestEnv.clean_test_path
end

View file

@ -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

View file

@ -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

View file

@ -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

View file

@ -35,7 +35,7 @@ feature 'Profile > Account' do
TestEnv.clean_test_path
end
after(:example) do
after do
TestEnv.clean_test_path
end

View file

@ -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

View file

@ -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

View file

@ -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')")

View file

@ -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

View file

@ -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

View file

@ -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

View file

@ -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

View file

@ -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

View file

@ -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

View file

@ -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

View file

@ -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

View file

@ -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

View file

@ -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

View file

@ -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

View file

@ -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

View file

@ -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

View file

@ -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

View file

@ -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

View file

@ -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

View file

@ -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

View file

@ -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

View file

@ -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

View file

@ -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

View file

@ -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 }

View file

@ -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

View file

@ -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])

View file

@ -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

View file

@ -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

View file

@ -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

View file

@ -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

View file

@ -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

View file

@ -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

View file

@ -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

View file

@ -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

View file

@ -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

View file

@ -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,

View file

@ -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,

View file

@ -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

View file

@ -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

View file

@ -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

View file

@ -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

View file

@ -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

View file

@ -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

View file

@ -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)

View file

@ -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

View file

@ -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)

View file

@ -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

View file

@ -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)