diff options
Diffstat (limited to 'spec')
152 files changed, 2875 insertions, 947 deletions
diff --git a/spec/bin/changelog_spec.rb b/spec/bin/changelog_spec.rb index 91aff0db7cc..6d8b9865dcb 100644 --- a/spec/bin/changelog_spec.rb +++ b/spec/bin/changelog_spec.rb @@ -4,56 +4,90 @@ load File.expand_path('../../bin/changelog', __dir__) describe 'bin/changelog' do describe ChangelogOptionParser do - it 'parses --ammend' do - options = described_class.parse(%w[foo bar --amend]) + describe '.parse' do + it 'parses --amend' do + options = described_class.parse(%w[foo bar --amend]) - expect(options.amend).to eq true - end + expect(options.amend).to eq true + end - it 'parses --force and -f' do - %w[--force -f].each do |flag| - options = described_class.parse(%W[foo #{flag} bar]) + it 'parses --force and -f' do + %w[--force -f].each do |flag| + options = described_class.parse(%W[foo #{flag} bar]) - expect(options.force).to eq true + expect(options.force).to eq true + end end - end - it 'parses --merge-request and -m' do - %w[--merge-request -m].each do |flag| - options = described_class.parse(%W[foo #{flag} 1234 bar]) + it 'parses --merge-request and -m' do + %w[--merge-request -m].each do |flag| + options = described_class.parse(%W[foo #{flag} 1234 bar]) - expect(options.merge_request).to eq 1234 + expect(options.merge_request).to eq 1234 + end end - end - it 'parses --dry-run and -n' do - %w[--dry-run -n].each do |flag| - options = described_class.parse(%W[foo #{flag} bar]) + it 'parses --dry-run and -n' do + %w[--dry-run -n].each do |flag| + options = described_class.parse(%W[foo #{flag} bar]) - expect(options.dry_run).to eq true + expect(options.dry_run).to eq true + end end - end - it 'parses --git-username and -u' do - allow(described_class).to receive(:git_user_name).and_return('Jane Doe') + it 'parses --git-username and -u' do + allow(described_class).to receive(:git_user_name).and_return('Jane Doe') - %w[--git-username -u].each do |flag| - options = described_class.parse(%W[foo #{flag} bar]) + %w[--git-username -u].each do |flag| + options = described_class.parse(%W[foo #{flag} bar]) - expect(options.author).to eq 'Jane Doe' + expect(options.author).to eq 'Jane Doe' + end + end + + it 'parses --type and -t' do + %w[--type -t].each do |flag| + options = described_class.parse(%W[foo #{flag} security]) + + expect(options.type).to eq 'security' + end end - end - it 'parses -h' do - expect do - expect { described_class.parse(%w[foo -h bar]) }.to output.to_stdout - end.to raise_error(SystemExit) + it 'parses -h' do + expect do + expect { described_class.parse(%w[foo -h bar]) }.to output.to_stdout + end.to raise_error(SystemExit) + end + + it 'assigns title' do + options = described_class.parse(%W[foo -m 1 bar\n -u baz\r\n --amend]) + + expect(options.title).to eq 'foo bar baz' + end end - it 'assigns title' do - options = described_class.parse(%W[foo -m 1 bar\n -u baz\r\n --amend]) + describe '.read_type' do + let(:type) { '1' } - expect(options.title).to eq 'foo bar baz' + it 'reads type from $stdin' do + expect($stdin).to receive(:getc).and_return(type) + expect do + expect(described_class.read_type).to eq('added') + end.to output.to_stdout + end + + context 'invalid type given' do + let(:type) { '99' } + + it 'shows error message and exits the program' do + allow($stdin).to receive(:getc).and_return(type) + expect do + expect do + expect { described_class.read_type }.to raise_error(SystemExit) + end.to output("Invalid category index, please select an index between 1 and 7\n").to_stderr + end.to output.to_stdout + end + end end end end 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/admin/projects_controller_spec.rb b/spec/controllers/admin/projects_controller_spec.rb index 65587064eb1..373260b3978 100644 --- a/spec/controllers/admin/projects_controller_spec.rb +++ b/spec/controllers/admin/projects_controller_spec.rb @@ -12,12 +12,24 @@ describe Admin::ProjectsController do it 'retrieves the project for the given visibility level' do get :index, visibility_level: [Gitlab::VisibilityLevel::PUBLIC] + expect(response.body).to match(project.name) end it 'does not retrieve the project' do get :index, visibility_level: [Gitlab::VisibilityLevel::INTERNAL] + expect(response.body).not_to match(project.name) end + + it 'does not respond with projects pending deletion' do + pending_delete_project = create(:project, pending_delete: true) + + get :index + + expect(response).to have_http_status(200) + expect(response.body).not_to match(pending_delete_project.name) + expect(response.body).to match(project.name) + end end end diff --git a/spec/controllers/autocomplete_controller_spec.rb b/spec/controllers/autocomplete_controller_spec.rb index 3c396e36b24..2fbab1e4040 100644 --- a/spec/controllers/autocomplete_controller_spec.rb +++ b/spec/controllers/autocomplete_controller_spec.rb @@ -1,8 +1,8 @@ require 'spec_helper' describe AutocompleteController do - let!(:project) { create(:project) } - let!(:user) { create(:user) } + let(:project) { create(:project) } + let(:user) { project.owner } context 'GET users' do let!(:user2) { create(:user) } @@ -11,7 +11,6 @@ describe AutocompleteController do context 'project members' do before do sign_in(user) - project.add_master(user) end describe 'GET #users with project ID' do @@ -19,11 +18,11 @@ describe AutocompleteController do get(:users, project_id: project.id) end - let(:body) { JSON.parse(response.body) } - - it { expect(body).to be_kind_of(Array) } - it { expect(body.size).to eq 2 } - it { expect(body.map { |u| u["username"] }).to include(user.username) } + it 'returns the project members' do + expect(json_response).to be_kind_of(Array) + expect(json_response.size).to eq(1) + expect(json_response.map { |u| u["username"] }).to include(user.username) + end end describe 'GET #users with unknown project' do @@ -39,20 +38,20 @@ describe AutocompleteController do let(:group) { create(:group) } before do - sign_in(user) group.add_owner(user) + sign_in(user) end - let(:body) { JSON.parse(response.body) } - describe 'GET #users with group ID' do before do get(:users, group_id: group.id) end - it { expect(body).to be_kind_of(Array) } - it { expect(body.size).to eq 1 } - it { expect(body.first["username"]).to eq user.username } + it 'returns the group members' do + expect(json_response).to be_kind_of(Array) + expect(json_response.size).to eq(1) + expect(json_response.first["username"]).to eq user.username + end end describe 'GET #users with unknown group ID' do @@ -65,23 +64,22 @@ describe AutocompleteController do end context 'non-member login for public project' do - let!(:project) { create(:project, :public) } + let(:project) { create(:project, :public) } before do sign_in(non_member) - project.add_master(user) end - let(:body) { JSON.parse(response.body) } - describe 'GET #users with project ID' do before do get(:users, project_id: project.id, current_user: true) end - it { expect(body).to be_kind_of(Array) } - it { expect(body.size).to eq 3 } - it { expect(body.map { |u| u['username'] }).to include(user.username, non_member.username) } + it 'returns the project members and non-members' do + expect(json_response).to be_kind_of(Array) + expect(json_response.size).to eq(2) + expect(json_response.map { |u| u['username'] }).to include(user.username, non_member.username) + end end end @@ -91,10 +89,8 @@ describe AutocompleteController do get(:users) end - let(:body) { JSON.parse(response.body) } - - it { expect(body).to be_kind_of(Array) } - it { expect(body.size).to eq User.count } + it { expect(json_response).to be_kind_of(Array) } + it { expect(json_response.size).to eq User.count } end context 'user order' do @@ -106,7 +102,7 @@ describe AutocompleteController do sign_in(user) get(:users, search: 'user') - response_usernames = JSON.parse(response.body).map { |user| user['username'] } + response_usernames = json_response.map { |user| user['username'] } expect(response_usernames.take(3)).to match_array([user.username, reported_user.username, user1.username]) end @@ -120,15 +116,12 @@ describe AutocompleteController do get(:users, per_page: per_page) end - let(:body) { JSON.parse(response.body) } - - it { expect(body).to be_kind_of(Array) } - it { expect(body.size).to eq per_page } + it { expect(json_response).to be_kind_of(Array) } + it { expect(json_response.size).to eq(per_page) } end context 'unauthenticated user' do let(:public_project) { create(:project, :public) } - let(:body) { JSON.parse(response.body) } describe 'GET #users with public project' do before do @@ -136,8 +129,8 @@ describe AutocompleteController do get(:users, project_id: public_project.id) end - it { expect(body).to be_kind_of(Array) } - it { expect(body.size).to eq 2 } + it { expect(json_response).to be_kind_of(Array) } + it { expect(json_response.size).to eq 2 } end describe 'GET #users with project' do @@ -170,8 +163,8 @@ describe AutocompleteController do get(:users) end - it { expect(body).to be_kind_of(Array) } - it { expect(body.size).to eq 0 } + it { expect(json_response).to be_kind_of(Array) } + it { expect(json_response).to be_empty } end describe 'GET #users with todo filter' do @@ -179,14 +172,12 @@ describe AutocompleteController do get :users, todo_filter: true expect(response.status).to eq 200 - expect(body).to be_kind_of(Array) + expect(json_response).to be_kind_of(Array) end end end context 'author of issuable included' do - let(:body) { JSON.parse(response.body) } - context 'authenticated' do before do sign_in(user) @@ -195,13 +186,13 @@ describe AutocompleteController do it 'includes the author' do get(:users, author_id: non_member.id) - expect(body.first["username"]).to eq non_member.username + expect(json_response.first["username"]).to eq non_member.username end it 'rejects non existent user ids' do get(:users, author_id: 99999) - expect(body.collect { |u| u['id'] }).not_to include(99999) + expect(json_response.collect { |u| u['id'] }).not_to include(99999) end end @@ -209,7 +200,7 @@ describe AutocompleteController do it 'returns empty result' do get(:users, author_id: non_member.id) - expect(body).to be_empty + expect(json_response).to be_empty end end end @@ -222,10 +213,9 @@ describe AutocompleteController do it 'skips the user IDs passed' do get(:users, skip_users: [user, user2].map(&:id)) - other_user_ids = [non_member, project.owner, project.creator].map(&:id) - response_user_ids = JSON.parse(response.body).map { |user| user['id'] } + response_user_ids = json_response.map { |user| user['id'] } - expect(response_user_ids).to contain_exactly(*other_user_ids) + expect(response_user_ids).to contain_exactly(non_member.id) end end end @@ -249,17 +239,15 @@ describe AutocompleteController do get(:projects, project_id: project.id) end - let(:body) { JSON.parse(response.body) } - - it do - expect(body).to be_kind_of(Array) - expect(body.size).to eq 2 + it 'returns projects' do + expect(json_response).to be_kind_of(Array) + expect(json_response.size).to eq(2) - expect(body.first['id']).to eq 0 - expect(body.first['name_with_namespace']).to eq 'No project' + expect(json_response.first['id']).to eq(0) + expect(json_response.first['name_with_namespace']).to eq 'No project' - expect(body.last['id']).to eq authorized_project.id - expect(body.last['name_with_namespace']).to eq authorized_project.name_with_namespace + expect(json_response.last['id']).to eq authorized_project.id + expect(json_response.last['name_with_namespace']).to eq authorized_project.name_with_namespace end end end @@ -275,14 +263,12 @@ describe AutocompleteController do get(:projects, project_id: project.id, search: 'rugged') end - let(:body) { JSON.parse(response.body) } + it 'returns projects' do + expect(json_response).to be_kind_of(Array) + expect(json_response.size).to eq(2) - it do - expect(body).to be_kind_of(Array) - expect(body.size).to eq 2 - - expect(body.last['id']).to eq authorized_search_project.id - expect(body.last['name_with_namespace']).to eq authorized_search_project.name_with_namespace + expect(json_response.last['id']).to eq authorized_search_project.id + expect(json_response.last['name_with_namespace']).to eq authorized_search_project.name_with_namespace end end end @@ -304,11 +290,9 @@ describe AutocompleteController do get(:projects, project_id: project.id) end - let(:body) { JSON.parse(response.body) } - - it do - expect(body).to be_kind_of(Array) - expect(body.size).to eq 3 # Of a total of 4 + it 'returns projects' do + expect(json_response).to be_kind_of(Array) + expect(json_response.size).to eq 3 # Of a total of 4 end end end @@ -328,17 +312,15 @@ describe AutocompleteController do get(:projects, project_id: project.id, offset_id: authorized_project.id) end - let(:body) { JSON.parse(response.body) } - - it do - expect(body.detect { |item| item['id'] == 0 }).to be_nil # 'No project' is not there - expect(body.detect { |item| item['id'] == authorized_project.id }).to be_nil # Offset project is not there either + it 'returns "No project"' do + expect(json_response.detect { |item| item['id'] == 0 }).to be_nil # 'No project' is not there + expect(json_response.detect { |item| item['id'] == authorized_project.id }).to be_nil # Offset project is not there either end end 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) @@ -349,13 +331,10 @@ describe AutocompleteController do get(:projects, project_id: project.id) end - let(:body) { JSON.parse(response.body) } - - it do - expect(body).to be_kind_of(Array) - expect(body.size).to eq 1 # 'No project' - - expect(body.first['id']).to eq 0 + it 'returns a single "No project"' do + expect(json_response).to be_kind_of(Array) + expect(json_response.size).to eq(1) # 'No project' + expect(json_response.first['id']).to eq 0 end end end diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb index 23601c457b0..b571b11dcac 100644 --- a/spec/controllers/projects/issues_controller_spec.rb +++ b/spec/controllers/projects/issues_controller_spec.rb @@ -1,7 +1,7 @@ require('spec_helper') describe Projects::IssuesController do - let(:project) { create(:project_empty_repo) } + let(:project) { create(:project) } let(:user) { create(:user) } let(:issue) { create(:issue, project: project) } @@ -841,7 +841,7 @@ describe Projects::IssuesController do describe 'POST #toggle_award_emoji' do before do sign_in(user) - project.team << [user, :developer] + project.add_developer(user) end it "toggles the award emoji" do @@ -855,6 +855,8 @@ describe Projects::IssuesController do end describe 'POST create_merge_request' do + let(:project) { create(:project, :repository) } + before do project.add_developer(user) sign_in(user) diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb index 8ecd8b6ca71..c0e48046937 100644 --- a/spec/controllers/projects_controller_spec.rb +++ b/spec/controllers/projects_controller_spec.rb @@ -578,6 +578,118 @@ describe ProjectsController do end end + describe '#export' do + before do + sign_in(user) + + project.add_master(user) + end + + context 'when project export is enabled' do + it 'returns 302' do + get :export, namespace_id: project.namespace, id: project + + expect(response).to have_http_status(302) + end + end + + context 'when project export is disabled' do + before do + stub_application_setting(project_export_enabled?: false) + end + + it 'returns 404' do + get :export, namespace_id: project.namespace, id: project + + expect(response).to have_http_status(404) + end + end + end + + describe '#download_export' do + before do + sign_in(user) + + project.add_master(user) + end + + context 'when project export is enabled' do + it 'returns 302' do + get :download_export, namespace_id: project.namespace, id: project + + expect(response).to have_http_status(302) + end + end + + context 'when project export is disabled' do + before do + stub_application_setting(project_export_enabled?: false) + end + + it 'returns 404' do + get :download_export, namespace_id: project.namespace, id: project + + expect(response).to have_http_status(404) + end + end + end + + describe '#remove_export' do + before do + sign_in(user) + + project.add_master(user) + end + + context 'when project export is enabled' do + it 'returns 302' do + post :remove_export, namespace_id: project.namespace, id: project + + expect(response).to have_http_status(302) + end + end + + context 'when project export is disabled' do + before do + stub_application_setting(project_export_enabled?: false) + end + + it 'returns 404' do + post :remove_export, namespace_id: project.namespace, id: project + + expect(response).to have_http_status(404) + end + end + end + + describe '#generate_new_export' do + before do + sign_in(user) + + project.add_master(user) + end + + context 'when project export is enabled' do + it 'returns 302' do + post :generate_new_export, namespace_id: project.namespace, id: project + + expect(response).to have_http_status(302) + end + end + + context 'when project export is disabled' do + before do + stub_application_setting(project_export_enabled?: false) + end + + it 'returns 404' do + post :generate_new_export, namespace_id: project.namespace, id: project + + expect(response).to have_http_status(404) + end + end + end + def project_moved_message(redirect_route, project) "Project '#{redirect_route.path}' was moved to '#{project.full_path}'. Please update any links and bookmarks that may still have the old path." end 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/controllers/snippets_controller_spec.rb b/spec/controllers/snippets_controller_spec.rb index 475ceda11fe..7c5d059760f 100644 --- a/spec/controllers/snippets_controller_spec.rb +++ b/spec/controllers/snippets_controller_spec.rb @@ -186,8 +186,8 @@ describe SnippetsController do end context 'when the snippet description contains a file' do - let(:picture_file) { '/system/temp/secret56/picture.jpg' } - let(:text_file) { '/system/temp/secret78/text.txt' } + let(:picture_file) { '/-/system/temp/secret56/picture.jpg' } + let(:text_file) { '/-/system/temp/secret78/text.txt' } let(:description) do "Description with picture: ![picture](/uploads#{picture_file}) and "\ "text: [text.txt](/uploads#{text_file})" @@ -208,8 +208,8 @@ describe SnippetsController do snippet = subject expected_description = "Description with picture: "\ - "![picture](/uploads/system/personal_snippet/#{snippet.id}/secret56/picture.jpg) and "\ - "text: [text.txt](/uploads/system/personal_snippet/#{snippet.id}/secret78/text.txt)" + "![picture](/uploads/-/system/personal_snippet/#{snippet.id}/secret56/picture.jpg) and "\ + "text: [text.txt](/uploads/-/system/personal_snippet/#{snippet.id}/secret78/text.txt)" expect(snippet.description).to eq(expected_description) end diff --git a/spec/controllers/uploads_controller_spec.rb b/spec/controllers/uploads_controller_spec.rb index b3a40f5d15c..b29f3d861be 100644 --- a/spec/controllers/uploads_controller_spec.rb +++ b/spec/controllers/uploads_controller_spec.rb @@ -102,7 +102,7 @@ describe UploadsController do subject expect(response.body).to match '\"alt\":\"rails_sample\"' - expect(response.body).to match "\"url\":\"/uploads/system/temp" + expect(response.body).to match "\"url\":\"/uploads/-/system/temp" end it 'does not create an Upload record' do @@ -119,7 +119,7 @@ describe UploadsController do subject expect(response.body).to match '\"alt\":\"doc_sample.txt\"' - expect(response.body).to match "\"url\":\"/uploads/system/temp" + expect(response.body).to match "\"url\":\"/uploads/-/system/temp" end it 'does not create an Upload record' do diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index a64ad73cba8..2cecd2646fc 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -92,8 +92,14 @@ describe UsersController do before do sign_in(user) project.team << [user, :developer] - EventCreateService.new.push(project, user, []) - EventCreateService.new.push(forked_project, user, []) + + push_data = Gitlab::DataBuilder::Push.build_sample(project, user) + + fork_push_data = Gitlab::DataBuilder::Push + .build_sample(forked_project, user) + + EventCreateService.new.push(project, user, push_data) + EventCreateService.new.push(forked_project, user, fork_push_data) end it 'includes forked projects' do diff --git a/spec/factories/deployments.rb b/spec/factories/deployments.rb index 29ad1af9fd9..e5abfd67d60 100644 --- a/spec/factories/deployments.rb +++ b/spec/factories/deployments.rb @@ -10,6 +10,10 @@ FactoryGirl.define do after(:build) do |deployment, evaluator| deployment.project ||= deployment.environment.project + + unless deployment.project.repository_exists? + allow(deployment.project.repository).to receive(:fetch_ref) + end end end end diff --git a/spec/factories/events.rb b/spec/factories/events.rb index 11d2016955c..ad9f7e2caef 100644 --- a/spec/factories/events.rb +++ b/spec/factories/events.rb @@ -2,6 +2,7 @@ FactoryGirl.define do factory :event do project author factory: :user + action Event::JOINED trait(:created) { action Event::CREATED } trait(:updated) { action Event::UPDATED } @@ -20,4 +21,19 @@ FactoryGirl.define do target factory: :closed_issue end end + + factory :push_event, class: PushEvent do + project factory: :project_empty_repo + author factory: :user + action Event::PUSHED + end + + factory :push_event_payload do + event + commit_count 1 + action :pushed + ref_type :branch + ref 'master' + commit_to '3cdce97ed87c91368561584e7358f4d46e3e173c' + end end diff --git a/spec/factories/merge_requests.rb b/spec/factories/merge_requests.rb index 1bc530d06db..cbec716d6ea 100644 --- a/spec/factories/merge_requests.rb +++ b/spec/factories/merge_requests.rb @@ -68,6 +68,17 @@ FactoryGirl.define do merge_user author end + after(:build) do |merge_request| + target_project = merge_request.target_project + source_project = merge_request.source_project + + # Fake `write_ref` if we don't have repository + # We have too many existing tests replying on this behaviour + unless [target_project, source_project].all?(&:repository_exists?) + allow(merge_request).to receive(:write_ref) + end + end + factory :merged_merge_request, traits: [:merged] factory :closed_merge_request, traits: [:closed] factory :reopened_merge_request, traits: [:opened] diff --git a/spec/factories/projects.rb b/spec/factories/projects.rb index 3f8e7030b1c..4a2034b31b3 100644 --- a/spec/factories/projects.rb +++ b/spec/factories/projects.rb @@ -8,12 +8,47 @@ FactoryGirl.define do factory :project, class: 'Project' do sequence(:name) { |n| "project#{n}" } path { name.downcase.gsub(/\s/, '_') } - namespace - creator - # Behaves differently to nil due to cache_has_external_issue_tracker has_external_issue_tracker false + # Associations + namespace + creator { group ? create(:user) : namespace&.owner } + + # Nest Project Feature attributes + transient do + wiki_access_level ProjectFeature::ENABLED + builds_access_level ProjectFeature::ENABLED + snippets_access_level ProjectFeature::ENABLED + issues_access_level ProjectFeature::ENABLED + merge_requests_access_level ProjectFeature::ENABLED + repository_access_level ProjectFeature::ENABLED + end + + after(:create) do |project, evaluator| + # Builds and MRs can't have higher visibility level than repository access level. + builds_access_level = [evaluator.builds_access_level, evaluator.repository_access_level].min + merge_requests_access_level = [evaluator.merge_requests_access_level, evaluator.repository_access_level].min + + project.project_feature.update_columns( + wiki_access_level: evaluator.wiki_access_level, + builds_access_level: builds_access_level, + snippets_access_level: evaluator.snippets_access_level, + issues_access_level: evaluator.issues_access_level, + merge_requests_access_level: merge_requests_access_level, + repository_access_level: evaluator.repository_access_level) + + # Normally the class Projects::CreateService is used for creating + # projects, and this class takes care of making sure the owner and current + # user have access to the project. Our specs don't use said service class, + # thus we must manually refresh things here. + unless project.group || project.pending_delete + project.add_master(project.owner) + end + + project.group&.refresh_members_authorized_projects + end + trait :public do visibility_level Gitlab::VisibilityLevel::PUBLIC end @@ -67,30 +102,28 @@ FactoryGirl.define do test_repo transient do - create_template nil + create_templates nil end after :create do |project, evaluator| - if evaluator.create_template - args = evaluator.create_template - - project.add_user(args[:user], args[:access]) + if evaluator.create_templates + templates_path = "#{evaluator.create_templates}_templates" project.repository.create_file( - args[:user], - ".gitlab/#{args[:path]}/bug.md", + project.creator, + ".gitlab/#{templates_path}/bug.md", 'something valid', message: 'test 3', branch_name: 'master') project.repository.create_file( - args[:user], - ".gitlab/#{args[:path]}/template_test.md", + project.creator, + ".gitlab/#{templates_path}/template_test.md", 'template_test', message: 'test 1', branch_name: 'master') project.repository.create_file( - args[:user], - ".gitlab/#{args[:path]}/feature_proposal.md", + project.creator, + ".gitlab/#{templates_path}/feature_proposal.md", 'feature_proposal', message: 'test 2', branch_name: 'master') @@ -142,44 +175,6 @@ FactoryGirl.define do trait(:repository_enabled) { repository_access_level ProjectFeature::ENABLED } trait(:repository_disabled) { repository_access_level ProjectFeature::DISABLED } trait(:repository_private) { repository_access_level ProjectFeature::PRIVATE } - - # Nest Project Feature attributes - transient do - wiki_access_level ProjectFeature::ENABLED - builds_access_level ProjectFeature::ENABLED - snippets_access_level ProjectFeature::ENABLED - issues_access_level ProjectFeature::ENABLED - merge_requests_access_level ProjectFeature::ENABLED - repository_access_level ProjectFeature::ENABLED - end - - after(:create) do |project, evaluator| - # Builds and MRs can't have higher visibility level than repository access level. - builds_access_level = [evaluator.builds_access_level, evaluator.repository_access_level].min - merge_requests_access_level = [evaluator.merge_requests_access_level, evaluator.repository_access_level].min - - project.project_feature - .update_attributes!( - wiki_access_level: evaluator.wiki_access_level, - builds_access_level: builds_access_level, - snippets_access_level: evaluator.snippets_access_level, - issues_access_level: evaluator.issues_access_level, - merge_requests_access_level: merge_requests_access_level, - repository_access_level: evaluator.repository_access_level - ) - - # Normally the class Projects::CreateService is used for creating - # projects, and this class takes care of making sure the owner and current - # user have access to the project. Our specs don't use said service class, - # thus we must manually refresh things here. - owner = project.owner - - if owner && owner.is_a?(User) && !project.pending_delete - project.members.create!(user: owner, access_level: Gitlab::Access::MASTER) - end - - project.group&.refresh_members_authorized_projects - end end # Project with empty repository diff --git a/spec/factories/users.rb b/spec/factories/users.rb index e60fe713bc3..4000cd085b7 100644 --- a/spec/factories/users.rb +++ b/spec/factories/users.rb @@ -1,5 +1,5 @@ FactoryGirl.define do - factory :user, aliases: [:author, :assignee, :recipient, :owner, :creator, :resource_owner] do + factory :user, aliases: [:author, :assignee, :recipient, :owner, :resource_owner] do email { generate(:email) } name { generate(:name) } username { generate(:username) } @@ -8,6 +8,10 @@ FactoryGirl.define do confirmation_token { nil } can_create_group true + after(:stub) do |user| + user.notification_email = user.email + end + before(:create) do |user| user.ensure_rss_token end diff --git a/spec/features/admin/admin_settings_spec.rb b/spec/features/admin/admin_settings_spec.rb index c9591a7d854..5db42175c15 100644 --- a/spec/features/admin/admin_settings_spec.rb +++ b/spec/features/admin/admin_settings_spec.rb @@ -35,6 +35,7 @@ feature 'Admin updates settings' do fill_in 'Help page text', with: 'Example text' check 'Hide marketing-related entries from help' fill_in 'Support page URL', with: 'http://example.com/help' + uncheck 'Project export enabled' click_button 'Save' expect(current_application_settings.gravatar_enabled).to be_falsey @@ -42,6 +43,7 @@ feature 'Admin updates settings' do expect(current_application_settings.help_page_text).to eq "Example text" expect(current_application_settings.help_page_hide_commercial_content).to be_truthy expect(current_application_settings.help_page_support_url).to eq "http://example.com/help" + expect(current_application_settings.project_export_enabled).to be_falsey expect(page).to have_content "Application settings saved successfully" end diff --git a/spec/features/boards/sidebar_spec.rb b/spec/features/boards/sidebar_spec.rb index 8d3d4ff8773..c3bf50ef9d1 100644 --- a/spec/features/boards/sidebar_spec.rb +++ b/spec/features/boards/sidebar_spec.rb @@ -15,10 +15,12 @@ describe 'Issue Boards', js: true do let!(:list) { create(:list, board: board, label: development, position: 0) } let(:card) { find('.board:nth-child(2)').first('.card') } - before do - Timecop.freeze + around do |example| + Timecop.freeze { example.run } + end - project.team << [user, :master] + before do + project.add_master(user) sign_in(user) @@ -26,10 +28,6 @@ describe 'Issue Boards', js: true do wait_for_requests end - after do - Timecop.return - end - it 'shows sidebar when clicking issue' do click_card(card) diff --git a/spec/features/calendar_spec.rb b/spec/features/calendar_spec.rb index 64fbc80cb81..9a597a2d690 100644 --- a/spec/features/calendar_spec.rb +++ b/spec/features/calendar_spec.rb @@ -42,14 +42,14 @@ feature 'Contributions Calendar', :js do end def push_code_contribution - push_params = { - project: contributed_project, - action: Event::PUSHED, - author_id: user.id, - data: { commit_count: 3 } - } - - Event.create(push_params) + event = create(:push_event, project: contributed_project, author: user) + + create(:push_event_payload, + event: event, + commit_from: '11f9ac0a48b62cef25eedede4c1819964f08d5ce', + commit_to: '1cf19a015df3523caf0a1f9d40c98a267d6a2fc2', + commit_count: 3, + ref: 'master') end def note_comment_contribution diff --git a/spec/features/cycle_analytics_spec.rb b/spec/features/cycle_analytics_spec.rb index 5c60cca10b9..bfe9dac3bd4 100644 --- a/spec/features/cycle_analytics_spec.rb +++ b/spec/features/cycle_analytics_spec.rb @@ -24,6 +24,12 @@ feature 'Cycle Analytics', js: true do expect(page).to have_content('Introducing Cycle Analytics') end + it 'shows pipeline summary' do + expect(new_issues_counter).to have_content('-') + expect(commits_counter).to have_content('-') + expect(deploys_counter).to have_content('-') + end + it 'shows active stage with empty message' do expect(page).to have_selector('.stage-nav-item.active', text: 'Issue') expect(page).to have_content("We don't have enough data to show this stage.") @@ -42,6 +48,12 @@ feature 'Cycle Analytics', js: true do visit project_cycle_analytics_path(project) end + it 'shows pipeline summary' do + expect(new_issues_counter).to have_content('1') + expect(commits_counter).to have_content('2') + expect(deploys_counter).to have_content('1') + end + it 'shows data on each stage' do expect_issue_to_be_present @@ -63,6 +75,20 @@ feature 'Cycle Analytics', js: true do click_stage('Production') expect_issue_to_be_present end + + context "when I change the time period observed" do + before do + _two_weeks_old_issue = create(:issue, project: project, created_at: 2.weeks.ago) + + click_button('Last 30 days') + click_link('Last 7 days') + wait_for_requests + end + + it 'shows only relevant data' do + expect(new_issues_counter).to have_content('1') + end + end end context "when my preferred language is Spanish" do @@ -109,6 +135,18 @@ feature 'Cycle Analytics', js: true do end end + def new_issues_counter + find(:xpath, "//p[contains(text(),'New Issue')]/preceding-sibling::h3") + end + + def commits_counter + find(:xpath, "//p[contains(text(),'Commits')]/preceding-sibling::h3") + end + + def deploys_counter + find(:xpath, "//p[contains(text(),'Deploy')]/preceding-sibling::h3") + end + def expect_issue_to_be_present expect(find('.stage-events')).to have_content(issue.title) expect(find('.stage-events')).to have_content(issue.author.name) diff --git a/spec/features/dashboard/activity_spec.rb b/spec/features/dashboard/activity_spec.rb index 4917dfcf1d1..582868bac1e 100644 --- a/spec/features/dashboard/activity_spec.rb +++ b/spec/features/dashboard/activity_spec.rb @@ -23,27 +23,19 @@ feature 'Dashboard > Activity' do create(:merge_request, author: user, source_project: project, target_project: project) end - let(:push_event_data) do - { - before: Gitlab::Git::BLANK_SHA, - after: '0220c11b9a3e6c69dc8fd35321254ca9a7b98f7e', - ref: 'refs/heads/new_design', - user_id: user.id, - user_name: user.name, - repository: { - name: project.name, - url: 'localhost/rubinius', - description: '', - homepage: 'localhost/rubinius', - private: true - } - } - end - let(:note) { create(:note, project: project, noteable: merge_request) } let!(:push_event) do - create(:event, :pushed, data: push_event_data, project: project, author: user) + event = create(:push_event, project: project, author: user) + + create(:push_event_payload, + event: event, + action: :created, + commit_to: '0220c11b9a3e6c69dc8fd35321254ca9a7b98f7e', + ref: 'new_design', + commit_count: 1) + + event end let!(:merged_event) do 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/groups/milestone_spec.rb b/spec/features/groups/milestone_spec.rb index 574bbe0e0e1..32b3e13c624 100644 --- a/spec/features/groups/milestone_spec.rb +++ b/spec/features/groups/milestone_spec.rb @@ -5,14 +5,12 @@ feature 'Group milestones', :js do let!(:project) { create(:project_empty_repo, group: group) } let(:user) { create(:group_member, :master, user: create(:user), group: group ).user } - before do - Timecop.freeze - - sign_in(user) + around do |example| + Timecop.freeze { example.run } end - after do - Timecop.return + before do + sign_in(user) end context 'create a milestone' do diff --git a/spec/features/issues/note_polling_spec.rb b/spec/features/issues/note_polling_spec.rb index 9f08ecc214b..62dbc3efb01 100644 --- a/spec/features/issues/note_polling_spec.rb +++ b/spec/features/issues/note_polling_spec.rb @@ -133,8 +133,6 @@ feature 'Issue notes polling', :js do def click_edit_action(note) note_element = find("#note_#{note.id}") - open_more_actions_dropdown(note) - note_element.find('.js-note-edit').click end end diff --git a/spec/features/issues_spec.rb b/spec/features/issues_spec.rb index a5bb642221c..3c8e37ff920 100644 --- a/spec/features/issues_spec.rb +++ b/spec/features/issues_spec.rb @@ -729,7 +729,6 @@ describe 'Issues' do visit project_issue_path(project, issue) expect(page).not_to have_css('.is-confidential') - expect(page).to have_css('.is-not-confidential') end end end diff --git a/spec/features/merge_requests/diff_notes_avatars_spec.rb b/spec/features/merge_requests/diff_notes_avatars_spec.rb index 2d9419d6124..c4f02311f13 100644 --- a/spec/features/merge_requests/diff_notes_avatars_spec.rb +++ b/spec/features/merge_requests/diff_notes_avatars_spec.rb @@ -157,7 +157,7 @@ feature 'Diff note avatars', js: true do end page.within find("[id='#{position.line_code(project.repository)}']") do - find('.diff-notes-collapse').click + find('.diff-notes-collapse').trigger('click') expect(page).to have_selector('img.js-diff-comment-avatar', count: 3) expect(find('.diff-comments-more-count')).to have_content '+1' 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/filter_by_labels_spec.rb b/spec/features/merge_requests/filter_by_labels_spec.rb index 1d52a4659ad..9912e8165e6 100644 --- a/spec/features/merge_requests/filter_by_labels_spec.rb +++ b/spec/features/merge_requests/filter_by_labels_spec.rb @@ -1,6 +1,6 @@ require 'rails_helper' -feature 'Merge Request filtering by Labels', js: true do +feature 'Merge Request filtering by Labels', :js do include FilteredSearchHelpers include MergeRequestHelpers @@ -12,9 +12,9 @@ feature 'Merge Request filtering by Labels', js: true do let!(:feature) { create(:label, project: project, title: 'feature') } let!(:enhancement) { create(:label, project: project, title: 'enhancement') } - let!(:mr1) { create(:merge_request, title: "Bugfix1", source_project: project, target_project: project, source_branch: "bugfix1") } - let!(:mr2) { create(:merge_request, title: "Bugfix2", source_project: project, target_project: project, source_branch: "bugfix2") } - let!(:mr3) { create(:merge_request, title: "Feature1", source_project: project, target_project: project, source_branch: "feature1") } + let!(:mr1) { create(:merge_request, title: "Bugfix1", source_project: project, target_project: project, source_branch: "fix") } + let!(:mr2) { create(:merge_request, title: "Bugfix2", source_project: project, target_project: project, source_branch: "wip") } + let!(:mr3) { create(:merge_request, title: "Feature1", source_project: project, target_project: project, source_branch: "improve/awesome") } before do mr1.labels << bug @@ -25,7 +25,7 @@ feature 'Merge Request filtering by Labels', js: true do mr3.title = "Feature1" mr3.labels << feature - project.team << [user, :master] + project.add_master(user) sign_in(user) visit project_merge_requests_path(project) diff --git a/spec/features/merge_requests/filter_merge_requests_spec.rb b/spec/features/merge_requests/filter_merge_requests_spec.rb index f0019be86ad..3686131fee4 100644 --- a/spec/features/merge_requests/filter_merge_requests_spec.rb +++ b/spec/features/merge_requests/filter_merge_requests_spec.rb @@ -12,7 +12,7 @@ describe 'Filter merge requests' do let!(:wontfix) { create(:label, project: project, title: "Won't fix") } before do - project.team << [user, :master] + project.add_master(user) group.add_developer(user) sign_in(user) create(:merge_request, source_project: project, target_project: project) @@ -170,7 +170,7 @@ describe 'Filter merge requests' do describe 'filter merge requests by text' do before do - create(:merge_request, title: "Bug", source_project: project, target_project: project, source_branch: "bug") + create(:merge_request, title: "Bug", source_project: project, target_project: project, source_branch: "wip") bug_label = create(:label, project: project, title: 'bug') milestone = create(:milestone, title: "8", project: project) @@ -179,7 +179,7 @@ describe 'Filter merge requests' do title: "Bug 2", source_project: project, target_project: project, - source_branch: "bug2", + source_branch: "fix", milestone: milestone, author: user, assignee: user) @@ -259,12 +259,12 @@ describe 'Filter merge requests' do end end - describe 'filter merge requests and sort', js: true do + describe 'filter merge requests and sort', :js do before do bug_label = create(:label, project: project, title: 'bug') - mr1 = create(:merge_request, title: "Frontend", source_project: project, target_project: project, source_branch: "Frontend") - mr2 = create(:merge_request, title: "Bug 2", source_project: project, target_project: project, source_branch: "bug2") + mr1 = create(:merge_request, title: "Frontend", source_project: project, target_project: project, source_branch: "wip") + mr2 = create(:merge_request, title: "Bug 2", source_project: project, target_project: project, source_branch: "fix") mr1.labels << bug_label mr2.labels << bug_label diff --git a/spec/features/merge_requests/reset_filters_spec.rb b/spec/features/merge_requests/reset_filters_spec.rb index c1b90e5f875..eed95816bdf 100644 --- a/spec/features/merge_requests/reset_filters_spec.rb +++ b/spec/features/merge_requests/reset_filters_spec.rb @@ -1,6 +1,6 @@ require 'rails_helper' -feature 'Merge requests filter clear button', js: true do +feature 'Merge requests filter clear button', :js do include FilteredSearchHelpers include MergeRequestHelpers include IssueHelpers @@ -9,8 +9,8 @@ feature 'Merge requests filter clear button', js: true do let!(:user) { create(:user) } let!(:milestone) { create(:milestone, project: project) } let!(:bug) { create(:label, project: project, name: 'bug')} - let!(:mr1) { create(:merge_request, title: "Feature", source_project: project, target_project: project, source_branch: "Feature", milestone: milestone, author: user, assignee: user) } - let!(:mr2) { create(:merge_request, title: "Bugfix1", source_project: project, target_project: project, source_branch: "Bugfix1") } + let!(:mr1) { create(:merge_request, title: "Feature", source_project: project, target_project: project, source_branch: "improve/awesome", milestone: milestone, author: user, assignee: user) } + let!(:mr2) { create(:merge_request, title: "Bugfix1", source_project: project, target_project: project, source_branch: "fix") } let(:merge_request_css) { '.merge-request' } let(:clear_search_css) { '.filtered-search-box .clear-search' } diff --git a/spec/features/merge_requests/user_lists_merge_requests_spec.rb b/spec/features/merge_requests/user_lists_merge_requests_spec.rb index d62b035b40b..20008b4e7f9 100644 --- a/spec/features/merge_requests/user_lists_merge_requests_spec.rb +++ b/spec/features/merge_requests/user_lists_merge_requests_spec.rb @@ -24,12 +24,10 @@ describe 'Projects > Merge requests > User lists merge requests' do milestone: create(:milestone, due_date: '2013-12-12'), created_at: 2.minutes.ago, updated_at: 2.minutes.ago) - # lfs in itself is not a great choice for the title if one wants to match the whole body content later on - # just think about the scenario when faker generates 'Chester Runolfsson' as the user's name create(:merge_request, - title: 'merge_lfs', + title: 'merge-test', source_project: project, - source_branch: 'merge_lfs', + source_branch: 'merge-test', created_at: 3.minutes.ago, updated_at: 10.seconds.ago) end @@ -38,7 +36,7 @@ describe 'Projects > Merge requests > User lists merge requests' do visit_merge_requests(project, assignee_id: IssuableFinder::NONE) expect(current_path).to eq(project_merge_requests_path(project)) - expect(page).to have_content 'merge_lfs' + expect(page).to have_content 'merge-test' expect(page).not_to have_content 'fix' expect(page).not_to have_content 'markdown' expect(count_merge_requests).to eq(1) @@ -47,7 +45,7 @@ describe 'Projects > Merge requests > User lists merge requests' do it 'filters on a specific assignee' do visit_merge_requests(project, assignee_id: user.id) - expect(page).not_to have_content 'merge_lfs' + expect(page).not_to have_content 'merge-test' expect(page).to have_content 'fix' expect(page).to have_content 'markdown' expect(count_merge_requests).to eq(2) @@ -57,14 +55,14 @@ describe 'Projects > Merge requests > User lists merge requests' do visit_merge_requests(project, sort: sort_value_recently_created) expect(first_merge_request).to include('fix') - expect(last_merge_request).to include('merge_lfs') + expect(last_merge_request).to include('merge-test') expect(count_merge_requests).to eq(3) end it 'sorts by oldest' do visit_merge_requests(project, sort: sort_value_oldest_created) - expect(first_merge_request).to include('merge_lfs') + expect(first_merge_request).to include('merge-test') expect(last_merge_request).to include('fix') expect(count_merge_requests).to eq(3) end @@ -72,7 +70,7 @@ describe 'Projects > Merge requests > User lists merge requests' do it 'sorts by last updated' do visit_merge_requests(project, sort: sort_value_recently_updated) - expect(first_merge_request).to include('merge_lfs') + expect(first_merge_request).to include('merge-test') expect(count_merge_requests).to eq(3) 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/merge_requests/user_posts_notes_spec.rb b/spec/features/merge_requests/user_posts_notes_spec.rb index 74d21822a59..d7cda73ab40 100644 --- a/spec/features/merge_requests/user_posts_notes_spec.rb +++ b/spec/features/merge_requests/user_posts_notes_spec.rb @@ -75,7 +75,6 @@ describe 'Merge requests > User posts notes', :js do describe 'editing the note' do before do find('.note').hover - open_more_actions_dropdown(note) find('.js-note-edit').click end @@ -104,7 +103,6 @@ describe 'Merge requests > User posts notes', :js do wait_for_requests find('.note').hover - open_more_actions_dropdown(note) find('.js-note-edit').click @@ -132,7 +130,6 @@ describe 'Merge requests > User posts notes', :js do describe 'deleting an attachment' do before do find('.note').hover - open_more_actions_dropdown(note) find('.js-note-edit').click 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/projects/user_edits_files_spec.rb b/spec/features/projects/user_edits_files_spec.rb index 8ae89c980b9..3129aad8473 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) } @@ -24,6 +20,9 @@ describe 'User edits files' do it 'inserts a content of a file', js: true do click_link('.gitignore') find('.js-edit-blob').click + + wait_for_requests + execute_script("ace.edit('editor').setValue('*.rbca')") expect(evaluate_script('ace.edit("editor").getValue()')).to eq('*.rbca') @@ -39,6 +38,9 @@ describe 'User edits files' do it 'commits an edited file', js: true do click_link('.gitignore') find('.js-edit-blob').click + + wait_for_requests + execute_script("ace.edit('editor').setValue('*.rbca')") fill_in(:commit_message, with: 'New commit message', visible: true) click_button('Commit changes') @@ -53,6 +55,9 @@ describe 'User edits files' do it 'commits an edited file to a new branch', js: true do click_link('.gitignore') find('.js-edit-blob').click + + wait_for_requests + execute_script("ace.edit('editor').setValue('*.rbca')") fill_in(:commit_message, with: 'New commit message', visible: true) fill_in(:branch_name, with: 'new_branch_name', visible: true) @@ -69,6 +74,9 @@ describe 'User edits files' do it 'shows the diff of an edited file', js: true do click_link('.gitignore') find('.js-edit-blob').click + + wait_for_requests + execute_script("ace.edit('editor').setValue('*.rbca')") click_link('Preview changes') @@ -91,7 +99,12 @@ 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." + ) + + wait_for_requests execute_script("ace.edit('editor').setValue('*.rbca')") @@ -106,6 +119,9 @@ describe 'User edits files' do expect(page).to have_button('Cancel') click_link('Fork') + + wait_for_requests + execute_script("ace.edit('editor').setValue('*.rbca')") fill_in(:commit_message, with: 'New commit message', visible: true) click_button('Commit changes') diff --git a/spec/features/snippets/notes_on_personal_snippets_spec.rb b/spec/features/snippets/notes_on_personal_snippets_spec.rb index f1d0905738b..c0c293dee78 100644 --- a/spec/features/snippets/notes_on_personal_snippets_spec.rb +++ b/spec/features/snippets/notes_on_personal_snippets_spec.rb @@ -91,11 +91,7 @@ describe 'Comments on personal snippets', :js do context 'when editing a note' do it 'changes the text' do - open_more_actions_dropdown(snippet_notes[0]) - - page.within("#notes-list li#note_#{snippet_notes[0].id}") do - click_on 'Edit comment' - end + find('.js-note-edit').click page.within('.current-note-edit-form') do fill_in 'note[note]', with: 'new content' diff --git a/spec/features/snippets/user_creates_snippet_spec.rb b/spec/features/snippets/user_creates_snippet_spec.rb index a919f5fa20b..d732383a1e1 100644 --- a/spec/features/snippets/user_creates_snippet_spec.rb +++ b/spec/features/snippets/user_creates_snippet_spec.rb @@ -41,7 +41,7 @@ feature 'User creates snippet', :js do expect(page).to have_content('My Snippet') link = find('a.no-attachment-icon img[alt="banana_sample"]')['src'] - expect(link).to match(%r{/uploads/system/temp/\h{32}/banana_sample\.gif\z}) + expect(link).to match(%r{/uploads/-/system/temp/\h{32}/banana_sample\.gif\z}) visit(link) expect(page.status_code).to eq(200) @@ -59,7 +59,7 @@ feature 'User creates snippet', :js do wait_for_requests link = find('a.no-attachment-icon img[alt="banana_sample"]')['src'] - expect(link).to match(%r{/uploads/system/personal_snippet/#{Snippet.last.id}/\h{32}/banana_sample\.gif\z}) + expect(link).to match(%r{/uploads/-/system/personal_snippet/#{Snippet.last.id}/\h{32}/banana_sample\.gif\z}) visit(link) expect(page.status_code).to eq(200) @@ -84,7 +84,7 @@ feature 'User creates snippet', :js do end expect(page).to have_content('Hello World!') link = find('a.no-attachment-icon img[alt="banana_sample"]')['src'] - expect(link).to match(%r{/uploads/system/personal_snippet/#{Snippet.last.id}/\h{32}/banana_sample\.gif\z}) + expect(link).to match(%r{/uploads/-/system/personal_snippet/#{Snippet.last.id}/\h{32}/banana_sample\.gif\z}) visit(link) expect(page.status_code).to eq(200) diff --git a/spec/features/snippets/user_edits_snippet_spec.rb b/spec/features/snippets/user_edits_snippet_spec.rb index 26070e508e2..71de6b6bd1c 100644 --- a/spec/features/snippets/user_edits_snippet_spec.rb +++ b/spec/features/snippets/user_edits_snippet_spec.rb @@ -33,7 +33,7 @@ feature 'User edits snippet', :js do wait_for_requests link = find('a.no-attachment-icon img[alt="banana_sample"]')['src'] - expect(link).to match(%r{/uploads/system/personal_snippet/#{snippet.id}/\h{32}/banana_sample\.gif\z}) + expect(link).to match(%r{/uploads/-/system/personal_snippet/#{snippet.id}/\h{32}/banana_sample\.gif\z}) end it 'updates the snippet to make it internal' do diff --git a/spec/features/task_lists_spec.rb b/spec/features/task_lists_spec.rb index c14826df55a..580258f77eb 100644 --- a/spec/features/task_lists_spec.rb +++ b/spec/features/task_lists_spec.rb @@ -52,8 +52,8 @@ feature 'Task Lists' do before do Warden.test_mode! - project.team << [user, :master] - project.team << [user2, :guest] + project.add_master(user) + project.add_guest(user2) login_as(user) 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/finders/admin/projects_finder_spec.rb b/spec/finders/admin/projects_finder_spec.rb index 4e367d39cf3..28e36330029 100644 --- a/spec/finders/admin/projects_finder_spec.rb +++ b/spec/finders/admin/projects_finder_spec.rb @@ -38,6 +38,12 @@ describe Admin::ProjectsFinder do it { is_expected.to match_array([shared_project, public_project, internal_project, private_project]) } end + context 'with pending delete project' do + let!(:pending_delete_project) { create(:project, pending_delete: true) } + + it { is_expected.not_to include(pending_delete_project) } + end + context 'filter by namespace_id' do let(:namespace) { create(:namespace) } let!(:project_in_namespace) { create(:project, namespace: namespace) } diff --git a/spec/finders/contributed_projects_finder_spec.rb b/spec/finders/contributed_projects_finder_spec.rb index 2d079ea83b4..60ea98e61c7 100644 --- a/spec/finders/contributed_projects_finder_spec.rb +++ b/spec/finders/contributed_projects_finder_spec.rb @@ -14,8 +14,8 @@ describe ContributedProjectsFinder do private_project.add_developer(current_user) public_project.add_master(source_user) - create(:event, :pushed, project: public_project, target: public_project, author: source_user) - create(:event, :pushed, project: private_project, target: private_project, author: source_user) + create(:push_event, project: public_project, author: source_user) + create(:push_event, project: private_project, author: source_user) end describe 'without a current user' do diff --git a/spec/finders/environments_finder_spec.rb b/spec/finders/environments_finder_spec.rb index 0c063f6d5ee..3a8a1e7de74 100644 --- a/spec/finders/environments_finder_spec.rb +++ b/spec/finders/environments_finder_spec.rb @@ -12,7 +12,7 @@ describe EnvironmentsFinder do context 'tagged deployment' do before do - create(:deployment, environment: environment, ref: '1.0', tag: true, sha: project.commit.id) + create(:deployment, environment: environment, ref: 'v1.1.0', tag: true, sha: project.commit.id) end it 'returns environment when with_tags is set' do diff --git a/spec/helpers/pagination_helper_spec.rb b/spec/helpers/pagination_helper_spec.rb new file mode 100644 index 00000000000..e235475fb47 --- /dev/null +++ b/spec/helpers/pagination_helper_spec.rb @@ -0,0 +1,23 @@ +require 'spec_helper' + +describe PaginationHelper do + describe '#paginate_collection' do + let(:collection) { User.all.page(1) } + + it 'paginates a collection without using a COUNT' do + without_count = collection.without_count + + expect(helper).to receive(:paginate_without_count) + .with(without_count) + .and_call_original + + helper.paginate_collection(without_count) + end + + it 'paginates a collection using a COUNT' do + expect(helper).to receive(:paginate_with_count).and_call_original + + helper.paginate_collection(collection) + end + end +end diff --git a/spec/javascripts/breakpoints_spec.js b/spec/javascripts/breakpoints_spec.js new file mode 100644 index 00000000000..b1b5d36c1fb --- /dev/null +++ b/spec/javascripts/breakpoints_spec.js @@ -0,0 +1,15 @@ +import bp, { + breakpoints, +} from '~/breakpoints'; + +describe('breakpoints', () => { + Object.keys(breakpoints).forEach((key) => { + const size = breakpoints[key]; + + it(`returns ${key} when larger than ${size}`, () => { + spyOn(bp, 'windowWidth').and.returnValue(size + 10); + + expect(bp.getBreakpointSize()).toBe(key); + }); + }); +}); 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/javascripts/fly_out_nav_spec.js b/spec/javascripts/fly_out_nav_spec.js index d7b7acaa3f4..65a7459c5ed 100644 --- a/spec/javascripts/fly_out_nav_spec.js +++ b/spec/javascripts/fly_out_nav_spec.js @@ -1,12 +1,17 @@ -/* global bp */ import Cookies from 'js-cookie'; import { calculateTop, - hideSubLevelItems, showSubLevelItems, canShowSubItems, canShowActiveSubItems, + mouseEnterTopItems, + mouseLeaveTopItem, + setOpenMenu, + mousePos, + getHideSubItemsInterval, + documentMouseMove, } from '~/fly_out_nav'; +import bp from '~/breakpoints'; describe('Fly out sidebar navigation', () => { let el; @@ -18,11 +23,14 @@ describe('Fly out sidebar navigation', () => { document.body.appendChild(el); spyOn(bp, 'getBreakpointSize').and.callFake(() => breakpointSize); + + setOpenMenu(null); }); afterEach(() => { - el.remove(); + document.body.innerHTML = ''; breakpointSize = 'lg'; + mousePos.length = 0; }); describe('calculateTop', () => { @@ -49,61 +57,152 @@ describe('Fly out sidebar navigation', () => { }); }); - describe('hideSubLevelItems', () => { + describe('getHideSubItemsInterval', () => { beforeEach(() => { - el.innerHTML = '<div class="sidebar-sub-level-items"></div>'; + el.innerHTML = '<div class="sidebar-sub-level-items" style="position: fixed; top: 0; left: 100px; height: 50px;"></div>'; }); - it('hides subitems', () => { - hideSubLevelItems(el); + it('returns 0 if currentOpenMenu is nil', () => { + expect( + getHideSubItemsInterval(), + ).toBe(0); + }); + + it('returns 0 when mouse above sub-items', () => { + showSubLevelItems(el); + documentMouseMove({ + clientX: el.getBoundingClientRect().left, + clientY: el.getBoundingClientRect().top, + }); + documentMouseMove({ + clientX: el.getBoundingClientRect().left, + clientY: el.getBoundingClientRect().top - 50, + }); expect( - el.querySelector('.sidebar-sub-level-items').style.display, - ).toBe(''); + getHideSubItemsInterval(), + ).toBe(0); }); - it('does not hude subitems on mobile', () => { - breakpointSize = 'xs'; + it('returns 0 when mouse is below sub-items', () => { + const subItems = el.querySelector('.sidebar-sub-level-items'); - hideSubLevelItems(el); + showSubLevelItems(el); + documentMouseMove({ + clientX: el.getBoundingClientRect().left, + clientY: el.getBoundingClientRect().top, + }); + documentMouseMove({ + clientX: el.getBoundingClientRect().left, + clientY: (el.getBoundingClientRect().top - subItems.getBoundingClientRect().height) + 50, + }); expect( - el.querySelector('.sidebar-sub-level-items').style.display, - ).not.toBe('none'); + getHideSubItemsInterval(), + ).toBe(0); }); - it('removes is-over class', () => { + it('returns 300 when mouse is moved towards sub-items', () => { + documentMouseMove({ + clientX: el.getBoundingClientRect().left, + clientY: el.getBoundingClientRect().top, + }); + showSubLevelItems(el); + documentMouseMove({ + clientX: el.getBoundingClientRect().left + 20, + clientY: el.getBoundingClientRect().top + 10, + }); + + expect( + getHideSubItemsInterval(), + ).toBe(300); + }); + }); + + describe('mouseLeaveTopItem', () => { + beforeEach(() => { spyOn(el.classList, 'remove'); + }); - hideSubLevelItems(el); + it('removes is-over class if currentOpenMenu is null', () => { + mouseLeaveTopItem(el); expect( el.classList.remove, ).toHaveBeenCalledWith('is-over'); }); - it('removes is-above class from sub-items', () => { - const subItems = el.querySelector('.sidebar-sub-level-items'); + it('removes is-over class if currentOpenMenu is null & there are sub-items', () => { + el.innerHTML = '<div class="sidebar-sub-level-items" style="position: absolute;"></div>'; + + mouseLeaveTopItem(el); + + expect( + el.classList.remove, + ).toHaveBeenCalledWith('is-over'); + }); + + it('does not remove is-over class if currentOpenMenu is the passed in sub-items', () => { + el.innerHTML = '<div class="sidebar-sub-level-items" style="position: absolute;"></div>'; + + setOpenMenu(el.querySelector('.sidebar-sub-level-items')); + mouseLeaveTopItem(el); + + expect( + el.classList.remove, + ).not.toHaveBeenCalled(); + }); + }); - spyOn(subItems.classList, 'remove'); + describe('mouseEnterTopItems', () => { + beforeEach(() => { + jasmine.clock().install(); - hideSubLevelItems(el); + el.innerHTML = '<div class="sidebar-sub-level-items" style="position: absolute; top: 0; left: 100px; height: 200px;"></div>'; + }); + + afterEach(() => { + jasmine.clock().uninstall(); + }); + + it('shows sub-items after 0ms if no menu is open', () => { + mouseEnterTopItems(el); expect( - subItems.classList.remove, - ).toHaveBeenCalledWith('is-above'); + getHideSubItemsInterval(), + ).toBe(0); + + jasmine.clock().tick(0); + + expect( + el.querySelector('.sidebar-sub-level-items').style.display, + ).toBe('block'); }); - it('does nothing if el has no sub-items', () => { - el.innerHTML = ''; + it('shows sub-items after 300ms if a menu is currently open', () => { + documentMouseMove({ + clientX: el.getBoundingClientRect().left, + clientY: el.getBoundingClientRect().top, + }); - spyOn(el.classList, 'remove'); + setOpenMenu(el.querySelector('.sidebar-sub-level-items')); + + documentMouseMove({ + clientX: el.getBoundingClientRect().left + 20, + clientY: el.getBoundingClientRect().top + 10, + }); - hideSubLevelItems(el); + mouseEnterTopItems(el); expect( - el.classList.remove, - ).not.toHaveBeenCalledWith(); + getHideSubItemsInterval(), + ).toBe(300); + + jasmine.clock().tick(300); + + expect( + el.querySelector('.sidebar-sub-level-items').style.display, + ).toBe('block'); }); }); @@ -132,7 +231,7 @@ describe('Fly out sidebar navigation', () => { ).not.toBe('block'); }); - it('does not shows sub-items', () => { + it('shows sub-items', () => { showSubLevelItems(el); expect( diff --git a/spec/lib/event_filter_spec.rb b/spec/lib/event_filter_spec.rb index b0efcab47fb..87ae6b6cf01 100644 --- a/spec/lib/event_filter_spec.rb +++ b/spec/lib/event_filter_spec.rb @@ -5,7 +5,7 @@ describe EventFilter do let(:source_user) { create(:user) } let!(:public_project) { create(:project, :public) } - let!(:push_event) { create(:event, :pushed, project: public_project, target: public_project, author: source_user) } + let!(:push_event) { create(:push_event, project: public_project, author: source_user) } let!(:merged_event) { create(:event, :merged, project: public_project, target: public_project, author: source_user) } let!(:created_event) { create(:event, :created, project: public_project, target: public_project, author: source_user) } let!(:updated_event) { create(:event, :updated, project: public_project, target: public_project, author: source_user) } 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 } diff --git a/spec/lib/gitlab/background_migration/migrate_events_to_push_event_payloads_spec.rb b/spec/lib/gitlab/background_migration/migrate_events_to_push_event_payloads_spec.rb new file mode 100644 index 00000000000..87f45619e7a --- /dev/null +++ b/spec/lib/gitlab/background_migration/migrate_events_to_push_event_payloads_spec.rb @@ -0,0 +1,423 @@ +require 'spec_helper' + +describe Gitlab::BackgroundMigration::MigrateEventsToPushEventPayloads::Event do + describe '#commit_title' do + it 'returns nil when there are no commits' do + expect(described_class.new.commit_title).to be_nil + end + + it 'returns nil when there are commits without commit messages' do + event = described_class.new + + allow(event).to receive(:commits).and_return([{ id: '123' }]) + + expect(event.commit_title).to be_nil + end + + it 'returns the commit message when it is less than 70 characters long' do + event = described_class.new + + allow(event).to receive(:commits).and_return([{ message: 'Hello world' }]) + + expect(event.commit_title).to eq('Hello world') + end + + it 'returns the first line of a commit message if multiple lines are present' do + event = described_class.new + + allow(event).to receive(:commits).and_return([{ message: "Hello\n\nworld" }]) + + expect(event.commit_title).to eq('Hello') + end + + it 'truncates the commit to 70 characters when it is too long' do + event = described_class.new + + allow(event).to receive(:commits).and_return([{ message: 'a' * 100 }]) + + expect(event.commit_title).to eq(('a' * 67) + '...') + end + end + + describe '#commit_from_sha' do + it 'returns nil when pushing to a new ref' do + event = described_class.new + + allow(event).to receive(:create?).and_return(true) + + expect(event.commit_from_sha).to be_nil + end + + it 'returns the ID of the first commit when pushing to an existing ref' do + event = described_class.new + + allow(event).to receive(:create?).and_return(false) + allow(event).to receive(:data).and_return(before: '123') + + expect(event.commit_from_sha).to eq('123') + end + end + + describe '#commit_to_sha' do + it 'returns nil when removing an existing ref' do + event = described_class.new + + allow(event).to receive(:remove?).and_return(true) + + expect(event.commit_to_sha).to be_nil + end + + it 'returns the ID of the last commit when pushing to an existing ref' do + event = described_class.new + + allow(event).to receive(:remove?).and_return(false) + allow(event).to receive(:data).and_return(after: '123') + + expect(event.commit_to_sha).to eq('123') + end + end + + describe '#data' do + it 'returns the deserialized data' do + event = described_class.new(data: { before: '123' }) + + expect(event.data).to eq(before: '123') + end + + it 'returns an empty hash when no data is present' do + event = described_class.new + + expect(event.data).to eq({}) + end + end + + describe '#commits' do + it 'returns an Array of commits' do + event = described_class.new(data: { commits: [{ id: '123' }] }) + + expect(event.commits).to eq([{ id: '123' }]) + end + + it 'returns an empty array when no data is present' do + event = described_class.new + + expect(event.commits).to eq([]) + end + end + + describe '#commit_count' do + it 'returns the number of commits' do + event = described_class.new(data: { total_commits_count: 2 }) + + expect(event.commit_count).to eq(2) + end + + it 'returns 0 when no data is present' do + event = described_class.new + + expect(event.commit_count).to eq(0) + end + end + + describe '#ref' do + it 'returns the name of the ref' do + event = described_class.new(data: { ref: 'refs/heads/master' }) + + expect(event.ref).to eq('refs/heads/master') + end + end + + describe '#trimmed_ref_name' do + it 'returns the trimmed ref name for a branch' do + event = described_class.new(data: { ref: 'refs/heads/master' }) + + expect(event.trimmed_ref_name).to eq('master') + end + + it 'returns the trimmed ref name for a tag' do + event = described_class.new(data: { ref: 'refs/tags/v1.2' }) + + expect(event.trimmed_ref_name).to eq('v1.2') + end + end + + describe '#create?' do + it 'returns true when creating a new ref' do + event = described_class.new(data: { before: described_class::BLANK_REF }) + + expect(event.create?).to eq(true) + end + + it 'returns false when pushing to an existing ref' do + event = described_class.new(data: { before: '123' }) + + expect(event.create?).to eq(false) + end + end + + describe '#remove?' do + it 'returns true when removing an existing ref' do + event = described_class.new(data: { after: described_class::BLANK_REF }) + + expect(event.remove?).to eq(true) + end + + it 'returns false when pushing to an existing ref' do + event = described_class.new(data: { after: '123' }) + + expect(event.remove?).to eq(false) + end + end + + describe '#push_action' do + let(:event) { described_class.new } + + it 'returns :created when creating a new ref' do + allow(event).to receive(:create?).and_return(true) + + expect(event.push_action).to eq(:created) + end + + it 'returns :removed when removing an existing ref' do + allow(event).to receive(:create?).and_return(false) + allow(event).to receive(:remove?).and_return(true) + + expect(event.push_action).to eq(:removed) + end + + it 'returns :pushed when pushing to an existing ref' do + allow(event).to receive(:create?).and_return(false) + allow(event).to receive(:remove?).and_return(false) + + expect(event.push_action).to eq(:pushed) + end + end + + describe '#ref_type' do + let(:event) { described_class.new } + + it 'returns :tag for a tag' do + allow(event).to receive(:ref).and_return('refs/tags/1.2') + + expect(event.ref_type).to eq(:tag) + end + + it 'returns :branch for a branch' do + allow(event).to receive(:ref).and_return('refs/heads/1.2') + + expect(event.ref_type).to eq(:branch) + end + end +end + +describe Gitlab::BackgroundMigration::MigrateEventsToPushEventPayloads do + let(:migration) { described_class.new } + let(:project) { create(:project_empty_repo) } + let(:author) { create(:user) } + + # We can not rely on FactoryGirl as the state of Event may change in ways that + # the background migration does not expect, hence we use the Event class of + # the migration itself. + def create_push_event(project, author, data = nil) + klass = Gitlab::BackgroundMigration::MigrateEventsToPushEventPayloads::Event + + klass.create!( + action: klass::PUSHED, + project_id: project.id, + author_id: author.id, + data: data + ) + end + + # The background migration relies on a temporary table, hence we're migrating + # to a specific version of the database where said table is still present. + before :all do + ActiveRecord::Migration.verbose = false + + ActiveRecord::Migrator + .migrate(ActiveRecord::Migrator.migrations_paths, 20170608152748) + end + + after :all do + ActiveRecord::Migrator.migrate(ActiveRecord::Migrator.migrations_paths) + + ActiveRecord::Migration.verbose = true + end + + describe '#perform' do + it 'returns if data should not be migrated' do + allow(migration).to receive(:migrate?).and_return(false) + + expect(migration).not_to receive(:find_events) + + migration.perform(1, 10) + end + + it 'migrates the range of events if data is to be migrated' do + event1 = create_push_event(project, author, { commits: [] }) + event2 = create_push_event(project, author, { commits: [] }) + + allow(migration).to receive(:migrate?).and_return(true) + + expect(migration).to receive(:process_event).twice + + migration.perform(event1.id, event2.id) + end + end + + describe '#process_event' do + it 'processes a regular event' do + event = double(:event, push_event?: false) + + expect(migration).to receive(:replicate_event) + expect(migration).not_to receive(:create_push_event_payload) + + migration.process_event(event) + end + + it 'processes a push event' do + event = double(:event, push_event?: true) + + expect(migration).to receive(:replicate_event) + expect(migration).to receive(:create_push_event_payload) + + migration.process_event(event) + end + end + + describe '#replicate_event' do + it 'replicates the event to the "events_for_migration" table' do + event = create_push_event( + project, + author, + data: { commits: [] }, + title: 'bla' + ) + + attributes = event + .attributes.with_indifferent_access.except(:title, :data) + + expect(described_class::EventForMigration) + .to receive(:create!) + .with(attributes) + + migration.replicate_event(event) + end + end + + describe '#create_push_event_payload' do + let(:push_data) do + { + commits: [], + ref: 'refs/heads/master', + before: '156e0e9adc587a383a7eeb5b21ddecb9044768a8', + after: '0' * 40, + total_commits_count: 1 + } + end + + let(:event) do + create_push_event(project, author, push_data) + end + + before do + # The foreign key in push_event_payloads at this point points to the + # "events_for_migration" table so we need to make sure a row exists in + # said table. + migration.replicate_event(event) + end + + it 'creates a push event payload for an event' do + payload = migration.create_push_event_payload(event) + + expect(PushEventPayload.count).to eq(1) + expect(payload.valid?).to eq(true) + end + + it 'does not create push event payloads for removed events' do + allow(event).to receive(:id).and_return(-1) + + payload = migration.create_push_event_payload(event) + + expect(payload).to be_nil + expect(PushEventPayload.count).to eq(0) + end + + it 'encodes and decodes the commit IDs from and to binary data' do + payload = migration.create_push_event_payload(event) + packed = migration.pack(push_data[:before]) + + expect(payload.commit_from).to eq(packed) + expect(payload.commit_to).to be_nil + end + end + + describe '#find_events' do + it 'returns the events for the given ID range' do + event1 = create_push_event(project, author, { commits: [] }) + event2 = create_push_event(project, author, { commits: [] }) + event3 = create_push_event(project, author, { commits: [] }) + events = migration.find_events(event1.id, event2.id) + + expect(events.length).to eq(2) + expect(events.pluck(:id)).not_to include(event3.id) + end + end + + describe '#migrate?' do + it 'returns true when data should be migrated' do + allow(described_class::Event) + .to receive(:table_exists?).and_return(true) + + allow(described_class::PushEventPayload) + .to receive(:table_exists?).and_return(true) + + allow(described_class::EventForMigration) + .to receive(:table_exists?).and_return(true) + + expect(migration.migrate?).to eq(true) + end + + it 'returns false if the "events" table does not exist' do + allow(described_class::Event) + .to receive(:table_exists?).and_return(false) + + expect(migration.migrate?).to eq(false) + end + + it 'returns false if the "push_event_payloads" table does not exist' do + allow(described_class::Event) + .to receive(:table_exists?).and_return(true) + + allow(described_class::PushEventPayload) + .to receive(:table_exists?).and_return(false) + + expect(migration.migrate?).to eq(false) + end + + it 'returns false when the "events_for_migration" table does not exist' do + allow(described_class::Event) + .to receive(:table_exists?).and_return(true) + + allow(described_class::PushEventPayload) + .to receive(:table_exists?).and_return(true) + + allow(described_class::EventForMigration) + .to receive(:table_exists?).and_return(false) + + expect(migration.migrate?).to eq(false) + end + end + + describe '#pack' do + it 'packs a SHA1 into a 20 byte binary string' do + packed = migration.pack('156e0e9adc587a383a7eeb5b21ddecb9044768a8') + + expect(packed.bytesize).to eq(20) + end + + it 'returns nil if the input value is nil' do + expect(migration.pack(nil)).to be_nil + end + end +end diff --git a/spec/lib/gitlab/background_migration/move_personal_snippet_files_spec.rb b/spec/lib/gitlab/background_migration/move_personal_snippet_files_spec.rb new file mode 100644 index 00000000000..ee60e498b59 --- /dev/null +++ b/spec/lib/gitlab/background_migration/move_personal_snippet_files_spec.rb @@ -0,0 +1,72 @@ +require 'spec_helper' + +describe Gitlab::BackgroundMigration::MovePersonalSnippetFiles do + let(:test_dir) { File.join(Rails.root, 'tmp', 'tests', 'move_snippet_files_test') } + let(:old_uploads_dir) { File.join('uploads', 'system', 'personal_snippet') } + let(:new_uploads_dir) { File.join('uploads', '-', 'system', 'personal_snippet') } + let(:snippet) do + snippet = create(:personal_snippet) + create_upload_for_snippet(snippet) + snippet.update_attributes!(description: markdown_linking_file(snippet)) + snippet + end + + let(:migration) { described_class.new } + + before do + allow(migration).to receive(:base_directory) { test_dir } + end + + describe '#perform' do + it 'moves the file on the disk' do + expected_path = File.join(test_dir, new_uploads_dir, snippet.id.to_s, "secret#{snippet.id}", 'upload.txt') + + migration.perform(old_uploads_dir, new_uploads_dir) + + expect(File.exist?(expected_path)).to be_truthy + end + + it 'updates the markdown of the snippet' do + expected_path = File.join(new_uploads_dir, snippet.id.to_s, "secret#{snippet.id}", 'upload.txt') + expected_markdown = "[an upload](#{expected_path})" + + migration.perform(old_uploads_dir, new_uploads_dir) + + expect(snippet.reload.description).to eq(expected_markdown) + end + + it 'updates the markdown of notes' do + expected_path = File.join(new_uploads_dir, snippet.id.to_s, "secret#{snippet.id}", 'upload.txt') + expected_markdown = "with [an upload](#{expected_path})" + + note = create(:note_on_personal_snippet, noteable: snippet, note: "with #{markdown_linking_file(snippet)}") + + migration.perform(old_uploads_dir, new_uploads_dir) + + expect(note.reload.note).to eq(expected_markdown) + end + end + + def create_upload_for_snippet(snippet) + snippet_path = path_for_file_in_snippet(snippet) + path = File.join(old_uploads_dir, snippet.id.to_s, snippet_path) + absolute_path = File.join(test_dir, path) + + FileUtils.mkdir_p(File.dirname(absolute_path)) + FileUtils.touch(absolute_path) + + create(:upload, model: snippet, path: snippet_path, uploader: PersonalFileUploader) + end + + def path_for_file_in_snippet(snippet) + secret = "secret#{snippet.id}" + filename = 'upload.txt' + + File.join(secret, filename) + end + + def markdown_linking_file(snippet) + path = File.join(old_uploads_dir, snippet.id.to_s, path_for_file_in_snippet(snippet)) + "[an upload](#{path})" + end +end diff --git a/spec/lib/gitlab/checks/force_push_spec.rb b/spec/lib/gitlab/checks/force_push_spec.rb index 6c4cfa1203e..f8c8b83a3ac 100644 --- a/spec/lib/gitlab/checks/force_push_spec.rb +++ b/spec/lib/gitlab/checks/force_push_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe Gitlab::Checks::ForcePush do let(:project) { create(:project, :repository) } - context "exit code checking" do + context "exit code checking", skip_gitaly_mock: true do it "does not raise a runtime error if the `popen` call to git returns a zero exit code" do allow(Gitlab::Popen).to receive(:popen).and_return(['normal output', 0]) 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/database_spec.rb b/spec/lib/gitlab/database_spec.rb index c5f9aecd867..5fa94999d25 100644 --- a/spec/lib/gitlab/database_spec.rb +++ b/spec/lib/gitlab/database_spec.rb @@ -51,6 +51,28 @@ describe Gitlab::Database do end end + describe '.join_lateral_supported?' do + it 'returns false when using MySQL' do + allow(described_class).to receive(:postgresql?).and_return(false) + + expect(described_class.join_lateral_supported?).to eq(false) + end + + it 'returns false when using PostgreSQL 9.2' do + allow(described_class).to receive(:postgresql?).and_return(true) + allow(described_class).to receive(:version).and_return('9.2.1') + + expect(described_class.join_lateral_supported?).to eq(false) + end + + it 'returns true when using PostgreSQL 9.3.0 or newer' do + allow(described_class).to receive(:postgresql?).and_return(true) + allow(described_class).to receive(:version).and_return('9.3.0') + + expect(described_class.join_lateral_supported?).to eq(true) + end + end + describe '.nulls_last_order' do context 'when using PostgreSQL' do before do 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..8d24d6f55b8 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -1002,7 +1002,7 @@ describe Gitlab::Git::Repository, seed_helper: true do expect(master_file_paths).to include("files/html/500.html") end - it "dose not read submodule directory and empty directory of master branch" do + it "does not read submodule directory and empty directory of master branch" do expect(master_file_paths).not_to include("six") 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 diff --git a/spec/lib/gitlab/git/storage/circuit_breaker_spec.rb b/spec/lib/gitlab/git/storage/circuit_breaker_spec.rb index b2886628601..9d1763b96ad 100644 --- a/spec/lib/gitlab/git/storage/circuit_breaker_spec.rb +++ b/spec/lib/gitlab/git/storage/circuit_breaker_spec.rb @@ -174,12 +174,8 @@ describe Gitlab::Git::Storage::CircuitBreaker, clean_gitlab_redis_shared_state: end describe '#track_storage_inaccessible' do - around(:each) do |example| - Timecop.freeze - - example.run - - Timecop.return + around do |example| + Timecop.freeze { example.run } end it 'records the failure time in redis' do diff --git a/spec/lib/gitlab/gpg_spec.rb b/spec/lib/gitlab/gpg_spec.rb index 8041518117d..30ad033b204 100644 --- a/spec/lib/gitlab/gpg_spec.rb +++ b/spec/lib/gitlab/gpg_spec.rb @@ -43,6 +43,58 @@ describe Gitlab::Gpg do ).to eq [] end end + + describe '.current_home_dir' do + let(:default_home_dir) { GPGME::Engine.dirinfo('homedir') } + + it 'returns the default value when no explicit home dir has been set' do + expect(described_class.current_home_dir).to eq default_home_dir + end + + it 'returns the explicitely set home dir' do + GPGME::Engine.home_dir = '/tmp/gpg' + + expect(described_class.current_home_dir).to eq '/tmp/gpg' + + GPGME::Engine.home_dir = GPGME::Engine.dirinfo('homedir') + end + + it 'returns the default value when explicitely setting the home dir to nil' do + GPGME::Engine.home_dir = nil + + expect(described_class.current_home_dir).to eq default_home_dir + end + end + + describe '.using_tmp_keychain' do + it "the second thread does not change the first thread's directory" do + thread1 = Thread.new do + described_class.using_tmp_keychain do + dir = described_class.current_home_dir + sleep 0.1 + expect(described_class.current_home_dir).to eq dir + end + end + + thread2 = Thread.new do + described_class.using_tmp_keychain do + sleep 0.2 + end + end + + thread1.join + thread2.join + end + + it 'allows recursive execution in the same thread' do + expect do + described_class.using_tmp_keychain do + described_class.using_tmp_keychain do + end + end + end.not_to raise_error(ThreadError) + end + end end describe Gitlab::Gpg::CurrentKeyChain do 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..f5c9680bf59 100644 --- a/spec/lib/gitlab/health_checks/fs_shards_check_spec.rb +++ b/spec/lib/gitlab/health_checks/fs_shards_check_spec.rb @@ -106,12 +106,6 @@ describe Gitlab::HealthChecks::FsShardsCheck do }.with_indifferent_access end - # Unsolved intermittent failure in CI https://gitlab.com/gitlab-org/gitlab-ce/issues/31128 - around(:each) do |example| # rubocop:disable RSpec/AroundBlock - times_to_try = ENV['CI'] ? 4 : 1 - example.run_with_retry retry: times_to_try - end - it 'provides metrics' do metrics = described_class.metrics diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index 6a41afe0c25..8da02b0cf00 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -22,6 +22,7 @@ events: - author - project - target +- push_event_payload notes: - award_emoji - project @@ -272,3 +273,5 @@ timelogs: - issue - merge_request - user +push_event_payload: +- event diff --git a/spec/lib/gitlab/import_export/file_importer_spec.rb b/spec/lib/gitlab/import_export/file_importer_spec.rb index 690c7625c52..162b776e107 100644 --- a/spec/lib/gitlab/import_export/file_importer_spec.rb +++ b/spec/lib/gitlab/import_export/file_importer_spec.rb @@ -5,6 +5,7 @@ describe Gitlab::ImportExport::FileImporter do let(:export_path) { "#{Dir.tmpdir}/file_importer_spec" } let(:valid_file) { "#{shared.export_path}/valid.json" } let(:symlink_file) { "#{shared.export_path}/invalid.json" } + let(:hidden_symlink_file) { "#{shared.export_path}/.hidden" } let(:subfolder_symlink_file) { "#{shared.export_path}/subfolder/invalid.json" } before do @@ -25,6 +26,10 @@ describe Gitlab::ImportExport::FileImporter do expect(File.exist?(symlink_file)).to be false end + it 'removes hidden symlinks in root folder' do + expect(File.exist?(hidden_symlink_file)).to be false + end + it 'removes symlinks in subfolders' do expect(File.exist?(subfolder_symlink_file)).to be false end diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml index 4dce48f8079..ae3b0173160 100644 --- a/spec/lib/gitlab/import_export/safe_model_attributes.yml +++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml @@ -36,6 +36,14 @@ Event: - updated_at - action - author_id +PushEventPayload: +- commit_count +- action +- ref_type +- commit_from +- commit_to +- ref +- commit_title Note: - id - note diff --git a/spec/lib/gitlab/metrics/requests_rack_middleware_spec.rb b/spec/lib/gitlab/metrics/requests_rack_middleware_spec.rb index 461b1e4182a..ebe66948a91 100644 --- a/spec/lib/gitlab/metrics/requests_rack_middleware_spec.rb +++ b/spec/lib/gitlab/metrics/requests_rack_middleware_spec.rb @@ -4,10 +4,6 @@ describe Gitlab::Metrics::RequestsRackMiddleware do let(:app) { double('app') } subject { described_class.new(app) } - around do |example| - Timecop.freeze { example.run } - end - describe '#call' do let(:status) { 100 } let(:env) { { 'REQUEST_METHOD' => 'GET' } } @@ -28,16 +24,14 @@ describe Gitlab::Metrics::RequestsRackMiddleware do subject.call(env) end - it 'measures execution time' do - execution_time = 10 - allow(app).to receive(:call) do |*args| - Timecop.freeze(execution_time.seconds) - [200, nil, nil] - end + RSpec::Matchers.define :a_positive_execution_time do + match { |actual| actual > 0 } + end - expect(described_class).to receive_message_chain(:http_request_duration_seconds, :observe).with({ status: 200, method: 'get' }, execution_time) + it 'measures execution time' do + expect(described_class).to receive_message_chain(:http_request_duration_seconds, :observe).with({ status: 200, method: 'get' }, a_positive_execution_time) - subject.call(env) + Timecop.scale(3600) { subject.call(env) } end 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/lib/gitlab/template/issue_template_spec.rb b/spec/lib/gitlab/template/issue_template_spec.rb index 6e0b1075a89..7098499f996 100644 --- a/spec/lib/gitlab/template/issue_template_spec.rb +++ b/spec/lib/gitlab/template/issue_template_spec.rb @@ -1,41 +1,28 @@ require 'spec_helper' describe Gitlab::Template::IssueTemplate do - subject { described_class } - - let(:user) { create(:user) } - - let(:project) do - create(:project, - :repository, - create_template: { - user: user, - access: Gitlab::Access::MASTER, - path: 'issue_templates' - }) - end + let(:project) { create(:project, :repository, create_templates: :issue) } describe '.all' do it 'strips the md suffix' do - expect(subject.all(project).first.name).not_to end_with('.issue_template') + expect(described_class.all(project).first.name).not_to end_with('.issue_template') end it 'combines the globals and rest' do - all = subject.all(project).map(&:name) + all = described_class.all(project).map(&:name) expect(all).to include('bug') expect(all).to include('feature_proposal') - expect(all).to include('template_test') end end describe '.find' do it 'returns nil if the file does not exist' do - expect { subject.find('mepmep-yadida', project) }.to raise_error(Gitlab::Template::Finders::RepoTemplateFinder::FileNotFoundError) + expect { described_class.find('mepmep-yadida', project) }.to raise_error(Gitlab::Template::Finders::RepoTemplateFinder::FileNotFoundError) end it 'returns the issue object of a valid file' do - ruby = subject.find('bug', project) + ruby = described_class.find('bug', project) expect(ruby).to be_a described_class expect(ruby.name).to eq('bug') @@ -44,21 +31,17 @@ describe Gitlab::Template::IssueTemplate do describe '.by_category' do it 'return array of templates' do - all = subject.by_category('', project).map(&:name) + all = described_class.by_category('', project).map(&:name) expect(all).to include('bug') expect(all).to include('feature_proposal') - expect(all).to include('template_test') end context 'when repo is bare or empty' do let(:empty_project) { create(:project) } - before do - empty_project.add_user(user, Gitlab::Access::MASTER) - end - it "returns empty array" do - templates = subject.by_category('', empty_project) + templates = described_class.by_category('', empty_project) + expect(templates).to be_empty end end @@ -66,26 +49,23 @@ describe Gitlab::Template::IssueTemplate do describe '#content' do it 'loads the full file' do - issue_template = subject.new('.gitlab/issue_templates/bug.md', project) + issue_template = described_class.new('.gitlab/issue_templates/bug.md', project) expect(issue_template.name).to eq 'bug' expect(issue_template.content).to eq('something valid') end it 'raises error when file is not found' do - issue_template = subject.new('.gitlab/issue_templates/bugnot.md', project) + issue_template = described_class.new('.gitlab/issue_templates/bugnot.md', project) expect { issue_template.content }.to raise_error(Gitlab::Template::Finders::RepoTemplateFinder::FileNotFoundError) end context "when repo is empty" do let(:empty_project) { create(:project) } - before do - empty_project.add_user(user, Gitlab::Access::MASTER) - end - it "raises file not found" do - issue_template = subject.new('.gitlab/issue_templates/not_existent.md', empty_project) + issue_template = described_class.new('.gitlab/issue_templates/not_existent.md', empty_project) + expect { issue_template.content }.to raise_error(Gitlab::Template::Finders::RepoTemplateFinder::FileNotFoundError) end end diff --git a/spec/lib/gitlab/template/merge_request_template_spec.rb b/spec/lib/gitlab/template/merge_request_template_spec.rb index b952274cd24..bd7ff64aa8a 100644 --- a/spec/lib/gitlab/template/merge_request_template_spec.rb +++ b/spec/lib/gitlab/template/merge_request_template_spec.rb @@ -1,41 +1,28 @@ require 'spec_helper' describe Gitlab::Template::MergeRequestTemplate do - subject { described_class } - - let(:user) { create(:user) } - - let(:project) do - create(:project, - :repository, - create_template: { - user: user, - access: Gitlab::Access::MASTER, - path: 'merge_request_templates' - }) - end + let(:project) { create(:project, :repository, create_templates: :merge_request) } describe '.all' do it 'strips the md suffix' do - expect(subject.all(project).first.name).not_to end_with('.issue_template') + expect(described_class.all(project).first.name).not_to end_with('.issue_template') end it 'combines the globals and rest' do - all = subject.all(project).map(&:name) + all = described_class.all(project).map(&:name) expect(all).to include('bug') expect(all).to include('feature_proposal') - expect(all).to include('template_test') end end describe '.find' do it 'returns nil if the file does not exist' do - expect { subject.find('mepmep-yadida', project) }.to raise_error(Gitlab::Template::Finders::RepoTemplateFinder::FileNotFoundError) + expect { described_class.find('mepmep-yadida', project) }.to raise_error(Gitlab::Template::Finders::RepoTemplateFinder::FileNotFoundError) end it 'returns the merge request object of a valid file' do - ruby = subject.find('bug', project) + ruby = described_class.find('bug', project) expect(ruby).to be_a described_class expect(ruby.name).to eq('bug') @@ -44,21 +31,17 @@ describe Gitlab::Template::MergeRequestTemplate do describe '.by_category' do it 'return array of templates' do - all = subject.by_category('', project).map(&:name) + all = described_class.by_category('', project).map(&:name) expect(all).to include('bug') expect(all).to include('feature_proposal') - expect(all).to include('template_test') end context 'when repo is bare or empty' do let(:empty_project) { create(:project) } - before do - empty_project.add_user(user, Gitlab::Access::MASTER) - end - it "returns empty array" do - templates = subject.by_category('', empty_project) + templates = described_class.by_category('', empty_project) + expect(templates).to be_empty end end @@ -66,26 +49,23 @@ describe Gitlab::Template::MergeRequestTemplate do describe '#content' do it 'loads the full file' do - issue_template = subject.new('.gitlab/merge_request_templates/bug.md', project) + issue_template = described_class.new('.gitlab/merge_request_templates/bug.md', project) expect(issue_template.name).to eq 'bug' expect(issue_template.content).to eq('something valid') end it 'raises error when file is not found' do - issue_template = subject.new('.gitlab/merge_request_templates/bugnot.md', project) + issue_template = described_class.new('.gitlab/merge_request_templates/bugnot.md', project) expect { issue_template.content }.to raise_error(Gitlab::Template::Finders::RepoTemplateFinder::FileNotFoundError) end context "when repo is empty" do let(:empty_project) { create(:project) } - before do - empty_project.add_user(user, Gitlab::Access::MASTER) - end - it "raises file not found" do - issue_template = subject.new('.gitlab/merge_request_templates/not_existent.md', empty_project) + issue_template = described_class.new('.gitlab/merge_request_templates/not_existent.md', empty_project) + expect { issue_template.content }.to raise_error(Gitlab::Template::Finders::RepoTemplateFinder::FileNotFoundError) end end diff --git a/spec/lib/gitlab/url_blocker_spec.rb b/spec/lib/gitlab/url_blocker_spec.rb index f5b4882815f..f18823b61ef 100644 --- a/spec/lib/gitlab/url_blocker_spec.rb +++ b/spec/lib/gitlab/url_blocker_spec.rb @@ -20,6 +20,34 @@ describe Gitlab::UrlBlocker do expect(described_class.blocked_url?('https://gitlab.com:25/foo/foo.git')).to be true end + it 'returns true for a non-alphanumeric hostname' do + stub_resolv + + aggregate_failures do + expect(described_class).to be_blocked_url('ssh://-oProxyCommand=whoami/a') + + # The leading character here is a Unicode "soft hyphen" + expect(described_class).to be_blocked_url('ssh://ÂoProxyCommand=whoami/a') + + # Unicode alphanumerics are allowed + expect(described_class).not_to be_blocked_url('ssh://ÄŸitlab.com/a') + end + end + + it 'returns true for a non-alphanumeric username' do + stub_resolv + + aggregate_failures do + expect(described_class).to be_blocked_url('ssh://-oProxyCommand=whoami@example.com/a') + + # The leading character here is a Unicode "soft hyphen" + expect(described_class).to be_blocked_url('ssh://ÂoProxyCommand=whoami@example.com/a') + + # Unicode alphanumerics are allowed + expect(described_class).not_to be_blocked_url('ssh://ÄŸitlab@example.com/a') + end + end + it 'returns true for invalid URL' do expect(described_class.blocked_url?('http://:8080')).to be true end @@ -28,4 +56,10 @@ describe Gitlab::UrlBlocker do expect(described_class.blocked_url?('https://gitlab.com/foo/foo.git')).to be false end end + + # Resolv does not support resolving UTF-8 domain names + # See https://bugs.ruby-lang.org/issues/4270 + def stub_resolv + allow(Resolv).to receive(:getaddresses).and_return([]) + end end diff --git a/spec/lib/gitlab/workhorse_spec.rb b/spec/lib/gitlab/workhorse_spec.rb index 654397ccffb..e78892d4232 100644 --- a/spec/lib/gitlab/workhorse_spec.rb +++ b/spec/lib/gitlab/workhorse_spec.rb @@ -217,7 +217,9 @@ describe Gitlab::Workhorse do it 'includes a Repository param' do repo_param = { Repository: { storage_name: 'default', - relative_path: project.full_path + '.git' + relative_path: project.full_path + '.git', + git_object_directory: '', + git_alternate_object_directories: [] } } expect(subject).to include(repo_param) diff --git a/spec/lib/rspec_flaky/example_spec.rb b/spec/lib/rspec_flaky/example_spec.rb new file mode 100644 index 00000000000..5b4fd5ddf3e --- /dev/null +++ b/spec/lib/rspec_flaky/example_spec.rb @@ -0,0 +1,89 @@ +require 'spec_helper' + +describe RspecFlaky::Example do + let(:example_attrs) do + { + id: 'spec/foo/bar_spec.rb:2', + metadata: { + file_path: 'spec/foo/bar_spec.rb', + line_number: 2, + full_description: 'hello world' + }, + execution_result: double(status: 'passed', exception: 'BOOM!'), + attempts: 1 + } + end + let(:rspec_example) { double(example_attrs) } + + describe '#initialize' do + shared_examples 'a valid Example instance' do + it 'returns valid attributes' do + example = described_class.new(args) + + expect(example.example_id).to eq(example_attrs[:id]) + end + end + + context 'when given an Rspec::Core::Example that responds to #example' do + let(:args) { double(example: rspec_example) } + + it_behaves_like 'a valid Example instance' + end + + context 'when given an Rspec::Core::Example that does not respond to #example' do + let(:args) { rspec_example } + + it_behaves_like 'a valid Example instance' + end + end + + subject { described_class.new(rspec_example) } + + describe '#uid' do + it 'returns a hash of the full description' do + expect(subject.uid).to eq(Digest::MD5.hexdigest("#{subject.description}-#{subject.file}")) + end + end + + describe '#example_id' do + it 'returns the ID of the RSpec::Core::Example' do + expect(subject.example_id).to eq(rspec_example.id) + end + end + + describe '#attempts' do + it 'returns the attempts of the RSpec::Core::Example' do + expect(subject.attempts).to eq(rspec_example.attempts) + end + end + + describe '#file' do + it 'returns the metadata[:file_path] of the RSpec::Core::Example' do + expect(subject.file).to eq(rspec_example.metadata[:file_path]) + end + end + + describe '#line' do + it 'returns the metadata[:line_number] of the RSpec::Core::Example' do + expect(subject.line).to eq(rspec_example.metadata[:line_number]) + end + end + + describe '#description' do + it 'returns the metadata[:full_description] of the RSpec::Core::Example' do + expect(subject.description).to eq(rspec_example.metadata[:full_description]) + end + end + + describe '#status' do + it 'returns the execution_result.status of the RSpec::Core::Example' do + expect(subject.status).to eq(rspec_example.execution_result.status) + end + end + + describe '#exception' do + it 'returns the execution_result.exception of the RSpec::Core::Example' do + expect(subject.exception).to eq(rspec_example.execution_result.exception) + end + end +end diff --git a/spec/lib/rspec_flaky/flaky_example_spec.rb b/spec/lib/rspec_flaky/flaky_example_spec.rb new file mode 100644 index 00000000000..cbfc1e538ab --- /dev/null +++ b/spec/lib/rspec_flaky/flaky_example_spec.rb @@ -0,0 +1,104 @@ +require 'spec_helper' + +describe RspecFlaky::FlakyExample do + let(:flaky_example_attrs) do + { + example_id: 'spec/foo/bar_spec.rb:2', + file: 'spec/foo/bar_spec.rb', + line: 2, + description: 'hello world', + first_flaky_at: 1234, + last_flaky_at: 2345, + last_attempts_count: 2, + flaky_reports: 1 + } + end + let(:example_attrs) do + { + uid: 'abc123', + example_id: flaky_example_attrs[:example_id], + file: flaky_example_attrs[:file], + line: flaky_example_attrs[:line], + description: flaky_example_attrs[:description], + status: 'passed', + exception: 'BOOM!', + attempts: flaky_example_attrs[:last_attempts_count] + } + end + let(:example) { double(example_attrs) } + + describe '#initialize' do + shared_examples 'a valid FlakyExample instance' do + it 'returns valid attributes' do + flaky_example = described_class.new(args) + + expect(flaky_example.uid).to eq(flaky_example_attrs[:uid]) + expect(flaky_example.example_id).to eq(flaky_example_attrs[:example_id]) + end + end + + context 'when given an Rspec::Example' do + let(:args) { example } + + it_behaves_like 'a valid FlakyExample instance' + end + + context 'when given a hash' do + let(:args) { flaky_example_attrs } + + it_behaves_like 'a valid FlakyExample instance' + end + end + + describe '#to_h' do + before do + # Stub these env variables otherwise specs don't behave the same on the CI + stub_env('CI_PROJECT_URL', nil) + stub_env('CI_JOB_ID', nil) + end + + shared_examples 'a valid FlakyExample hash' do + let(:additional_attrs) { {} } + + it 'returns a valid hash' do + flaky_example = described_class.new(args) + final_hash = flaky_example_attrs + .merge(last_flaky_at: instance_of(Time), last_flaky_job: nil) + .merge(additional_attrs) + + expect(flaky_example.to_h).to match(hash_including(final_hash)) + end + end + + context 'when given an Rspec::Example' do + let(:args) { example } + + context 'when run locally' do + it_behaves_like 'a valid FlakyExample hash' do + let(:additional_attrs) do + { first_flaky_at: instance_of(Time) } + end + end + end + + context 'when run on the CI' do + before do + stub_env('CI_PROJECT_URL', 'https://gitlab.com/gitlab-org/gitlab-ce') + stub_env('CI_JOB_ID', 42) + end + + it_behaves_like 'a valid FlakyExample hash' do + let(:additional_attrs) do + { first_flaky_at: instance_of(Time), last_flaky_job: "https://gitlab.com/gitlab-org/gitlab-ce/-/jobs/42" } + end + end + end + end + + context 'when given a hash' do + let(:args) { flaky_example_attrs } + + it_behaves_like 'a valid FlakyExample hash' + end + end +end diff --git a/spec/lib/rspec_flaky/listener_spec.rb b/spec/lib/rspec_flaky/listener_spec.rb new file mode 100644 index 00000000000..0e193bf408b --- /dev/null +++ b/spec/lib/rspec_flaky/listener_spec.rb @@ -0,0 +1,178 @@ +require 'spec_helper' + +describe RspecFlaky::Listener do + let(:flaky_example_report) do + { + 'abc123' => { + example_id: 'spec/foo/bar_spec.rb:2', + file: 'spec/foo/bar_spec.rb', + line: 2, + description: 'hello world', + first_flaky_at: 1234, + last_flaky_at: instance_of(Time), + last_attempts_count: 2, + flaky_reports: 1, + last_flaky_job: nil + } + } + end + let(:example_attrs) do + { + id: 'spec/foo/baz_spec.rb:3', + metadata: { + file_path: 'spec/foo/baz_spec.rb', + line_number: 3, + full_description: 'hello GitLab' + }, + execution_result: double(status: 'passed', exception: nil) + } + end + + before do + # Stub these env variables otherwise specs don't behave the same on the CI + stub_env('CI_PROJECT_URL', nil) + stub_env('CI_JOB_ID', nil) + end + + describe '#initialize' do + shared_examples 'a valid Listener instance' do + let(:expected_all_flaky_examples) { {} } + + it 'returns a valid Listener instance' do + listener = described_class.new + + expect(listener.to_report(listener.all_flaky_examples)) + .to match(hash_including(expected_all_flaky_examples)) + expect(listener.new_flaky_examples).to eq({}) + end + end + + context 'when no report file exists' do + it_behaves_like 'a valid Listener instance' + end + + context 'when a report file exists and set by ALL_FLAKY_RSPEC_REPORT_PATH' do + let(:report_file) do + Tempfile.new(%w[rspec_flaky_report .json]).tap do |f| + f.write(JSON.pretty_generate(flaky_example_report)) + f.rewind + end + end + + before do + stub_env('ALL_FLAKY_RSPEC_REPORT_PATH', report_file.path) + end + + after do + report_file.close + report_file.unlink + end + + it_behaves_like 'a valid Listener instance' do + let(:expected_all_flaky_examples) { flaky_example_report } + end + end + end + + describe '#example_passed' do + let(:rspec_example) { double(example_attrs) } + let(:notification) { double(example: rspec_example) } + + shared_examples 'a non-flaky example' do + it 'does not change the flaky examples hash' do + expect { subject.example_passed(notification) } + .not_to change { subject.all_flaky_examples } + end + end + + describe 'when the RSpec example does not respond to attempts' do + it_behaves_like 'a non-flaky example' + end + + describe 'when the RSpec example has 1 attempt' do + let(:rspec_example) { double(example_attrs.merge(attempts: 1)) } + + it_behaves_like 'a non-flaky example' + end + + describe 'when the RSpec example has 2 attempts' do + let(:rspec_example) { double(example_attrs.merge(attempts: 2)) } + let(:expected_new_flaky_example) do + { + example_id: 'spec/foo/baz_spec.rb:3', + file: 'spec/foo/baz_spec.rb', + line: 3, + description: 'hello GitLab', + first_flaky_at: instance_of(Time), + last_flaky_at: instance_of(Time), + last_attempts_count: 2, + flaky_reports: 1, + last_flaky_job: nil + } + end + + it 'does not change the flaky examples hash' do + expect { subject.example_passed(notification) } + .to change { subject.all_flaky_examples } + + new_example = RspecFlaky::Example.new(rspec_example) + + expect(subject.all_flaky_examples[new_example.uid].to_h) + .to match(hash_including(expected_new_flaky_example)) + end + end + end + + describe '#dump_summary' do + let(:rspec_example) { double(example_attrs) } + let(:notification) { double(example: rspec_example) } + + context 'when a report file path is set by ALL_FLAKY_RSPEC_REPORT_PATH' do + let(:report_file_path) { Rails.root.join('tmp', 'rspec_flaky_report.json') } + + before do + stub_env('ALL_FLAKY_RSPEC_REPORT_PATH', report_file_path) + FileUtils.rm(report_file_path) if File.exist?(report_file_path) + end + + after do + FileUtils.rm(report_file_path) if File.exist?(report_file_path) + end + + context 'when FLAKY_RSPEC_GENERATE_REPORT == "false"' do + before do + stub_env('FLAKY_RSPEC_GENERATE_REPORT', 'false') + end + + it 'does not write the report file' do + subject.example_passed(notification) + + subject.dump_summary(nil) + + expect(File.exist?(report_file_path)).to be(false) + end + end + + context 'when FLAKY_RSPEC_GENERATE_REPORT == "true"' do + before do + stub_env('FLAKY_RSPEC_GENERATE_REPORT', 'true') + end + + it 'writes the report file' do + subject.example_passed(notification) + + subject.dump_summary(nil) + + expect(File.exist?(report_file_path)).to be(true) + end + end + end + end + + describe '#to_report' do + it 'transforms the internal hash to a JSON-ready hash' do + expect(subject.to_report('abc123' => RspecFlaky::FlakyExample.new(flaky_example_report['abc123']))) + .to match(hash_including(flaky_example_report)) + end + end +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/migrations/clean_upload_symlinks_spec.rb b/spec/migrations/clean_upload_symlinks_spec.rb index cecb3ddac53..26653b9c008 100644 --- a/spec/migrations/clean_upload_symlinks_spec.rb +++ b/spec/migrations/clean_upload_symlinks_spec.rb @@ -5,7 +5,7 @@ describe CleanUploadSymlinks do let(:migration) { described_class.new } let(:test_dir) { File.join(Rails.root, "tmp", "tests", "move_uploads_test") } let(:uploads_dir) { File.join(test_dir, "public", "uploads") } - let(:new_uploads_dir) { File.join(uploads_dir, "system") } + let(:new_uploads_dir) { File.join(uploads_dir, "-", "system") } let(:original_path) { File.join(new_uploads_dir, 'user') } let(:symlink_path) { File.join(uploads_dir, 'user') } diff --git a/spec/migrations/move_personal_snippets_files_spec.rb b/spec/migrations/move_personal_snippets_files_spec.rb index 8505c7bf3e3..1a319eccc0d 100644 --- a/spec/migrations/move_personal_snippets_files_spec.rb +++ b/spec/migrations/move_personal_snippets_files_spec.rb @@ -5,7 +5,7 @@ describe MovePersonalSnippetsFiles do let(:migration) { described_class.new } let(:test_dir) { File.join(Rails.root, "tmp", "tests", "move_snippet_files_test") } let(:uploads_dir) { File.join(test_dir, 'uploads') } - let(:new_uploads_dir) { File.join(uploads_dir, 'system') } + let(:new_uploads_dir) { File.join(uploads_dir, '-', 'system') } before do allow(CarrierWave).to receive(:root).and_return(test_dir) @@ -42,7 +42,7 @@ describe MovePersonalSnippetsFiles do describe 'updating the markdown' do it 'includes the new path when the file exists' do secret = "secret#{snippet.id}" - file_location = "/uploads/system/personal_snippet/#{snippet.id}/#{secret}/picture.jpg" + file_location = "/uploads/-/system/personal_snippet/#{snippet.id}/#{secret}/picture.jpg" migration.up @@ -60,7 +60,7 @@ describe MovePersonalSnippetsFiles do it 'updates the note markdown' do secret = "secret#{snippet.id}" - file_location = "/uploads/system/personal_snippet/#{snippet.id}/#{secret}/picture.jpg" + file_location = "/uploads/-/system/personal_snippet/#{snippet.id}/#{secret}/picture.jpg" markdown = markdown_linking_file('picture.jpg', snippet) note = create(:note_on_personal_snippet, noteable: snippet, note: "with #{markdown}") @@ -108,7 +108,7 @@ describe MovePersonalSnippetsFiles do it 'keeps the markdown as is when the file is missing' do secret = "secret#{snippet_with_missing_file.id}" - file_location = "/uploads/system/personal_snippet/#{snippet_with_missing_file.id}/#{secret}/picture.jpg" + file_location = "/uploads/-/system/personal_snippet/#{snippet_with_missing_file.id}/#{secret}/picture.jpg" migration.down @@ -167,7 +167,7 @@ describe MovePersonalSnippetsFiles do def markdown_linking_file(filename, snippet, in_new_path: false) markdown = "![#{filename.split('.')[0]}]" markdown += '(/uploads' - markdown += '/system' if in_new_path + markdown += '/-/system' if in_new_path markdown += "/#{model_file_path(filename, snippet)})" markdown end diff --git a/spec/migrations/move_system_upload_folder_spec.rb b/spec/migrations/move_system_upload_folder_spec.rb index b622b4e9536..d3180477db3 100644 --- a/spec/migrations/move_system_upload_folder_spec.rb +++ b/spec/migrations/move_system_upload_folder_spec.rb @@ -33,6 +33,15 @@ describe MoveSystemUploadFolder do expect(File.symlink?(File.join(test_base, 'system'))).to be_truthy expect(File.exist?(File.join(test_base, 'system', 'file'))).to be_truthy end + + it 'does not move if the target directory already exists' do + FileUtils.mkdir_p(File.join(test_base, '-', 'system')) + + expect(FileUtils).not_to receive(:mv) + expect(migration).to receive(:say).with(/already exists. No need to redo the move/) + + migration.up + end end describe '#down' do @@ -58,5 +67,14 @@ describe MoveSystemUploadFolder do expect(File.directory?(File.join(test_base, 'system'))).to be_truthy expect(File.symlink?(File.join(test_base, 'system'))).to be_falsey end + + it 'does not move if the old directory already exists' do + FileUtils.mkdir_p(File.join(test_base, 'system')) + + expect(FileUtils).not_to receive(:mv) + expect(migration).to receive(:say).with(/already exists and is not a symlink, no need to revert/) + + migration.down + end end end diff --git a/spec/migrations/move_uploads_to_system_dir_spec.rb b/spec/migrations/move_uploads_to_system_dir_spec.rb index 37d66452447..ca11a2004c5 100644 --- a/spec/migrations/move_uploads_to_system_dir_spec.rb +++ b/spec/migrations/move_uploads_to_system_dir_spec.rb @@ -5,7 +5,7 @@ describe MoveUploadsToSystemDir do let(:migration) { described_class.new } let(:test_dir) { File.join(Rails.root, "tmp", "move_uploads_test") } let(:uploads_dir) { File.join(test_dir, "public", "uploads") } - let(:new_uploads_dir) { File.join(uploads_dir, "system") } + let(:new_uploads_dir) { File.join(uploads_dir, "-", "system") } before do FileUtils.remove_dir(test_dir) if File.directory?(test_dir) diff --git a/spec/migrations/rename_system_namespaces_spec.rb b/spec/migrations/rename_system_namespaces_spec.rb deleted file mode 100644 index 747694cbe33..00000000000 --- a/spec/migrations/rename_system_namespaces_spec.rb +++ /dev/null @@ -1,254 +0,0 @@ -require "spec_helper" -require Rails.root.join("db", "migrate", "20170316163800_rename_system_namespaces.rb") - -describe RenameSystemNamespaces, truncate: true do - let(:migration) { described_class.new } - let(:test_dir) { File.join(Rails.root, "tmp", "tests", "rename_namespaces_test") } - let(:uploads_dir) { File.join(test_dir, "public", "uploads") } - let(:system_namespace) do - namespace = build(:namespace, path: "system") - namespace.save(validate: false) - namespace - end - - def save_invalid_routable(routable) - routable.__send__(:prepare_route) - routable.save(validate: false) - end - - before do - FileUtils.remove_dir(test_dir) if File.directory?(test_dir) - FileUtils.mkdir_p(uploads_dir) - FileUtils.remove_dir(TestEnv.repos_path) if File.directory?(TestEnv.repos_path) - allow(migration).to receive(:say) - allow(migration).to receive(:uploads_dir).and_return(uploads_dir) - end - - describe "#system_namespace" do - it "only root namespaces called with path `system`" do - system_namespace - system_namespace_with_parent = build(:namespace, path: 'system', parent: create(:namespace)) - system_namespace_with_parent.save(validate: false) - - expect(migration.system_namespace.id).to eq(system_namespace.id) - end - end - - describe "#up" do - before do - system_namespace - end - - it "doesn't break if there are no namespaces called system" do - Namespace.delete_all - - migration.up - end - - it "renames namespaces called system" do - migration.up - - expect(system_namespace.reload.path).to eq("system0") - end - - it "renames the route to the namespace" do - migration.up - - expect(system_namespace.reload.full_path).to eq("system0") - end - - it "renames the route for projects of the namespace" do - project = build(:project, :repository, path: "project-path", namespace: system_namespace) - save_invalid_routable(project) - - migration.up - - expect(project.route.reload.path).to eq("system0/project-path") - end - - it "doesn't touch routes of namespaces that look like system" do - namespace = create(:group, path: 'systemlookalike') - project = create(:project, :repository, namespace: namespace, path: 'the-project') - - migration.up - - expect(project.route.reload.path).to eq('systemlookalike/the-project') - expect(namespace.route.reload.path).to eq('systemlookalike') - end - - it "moves the the repository for a project in the namespace" do - project = build(:project, :repository, namespace: system_namespace, path: "system-project") - save_invalid_routable(project) - TestEnv.copy_repo(project, - bare_repo: TestEnv.factory_repo_path_bare, - refs: TestEnv::BRANCH_SHA) - expected_repo = File.join(TestEnv.repos_path, "system0", "system-project.git") - - migration.up - - expect(File.directory?(expected_repo)).to be(true) - end - - it "moves the uploads for the namespace" do - allow(migration).to receive(:move_namespace_folders).with(Settings.pages.path, "system", "system0") - expect(migration).to receive(:move_namespace_folders).with(uploads_dir, "system", "system0") - - migration.up - end - - it "moves the pages for the namespace" do - allow(migration).to receive(:move_namespace_folders).with(uploads_dir, "system", "system0") - expect(migration).to receive(:move_namespace_folders).with(Settings.pages.path, "system", "system0") - - migration.up - end - - describe "clears the markdown cache for projects in the system namespace" do - let!(:project) do - project = build(:project, :repository, namespace: system_namespace) - save_invalid_routable(project) - project - end - - it 'removes description_html from projects' do - migration.up - - expect(project.reload.description_html).to be_nil - end - - it 'removes issue descriptions' do - issue = create(:issue, project: project, description_html: 'Issue description') - - migration.up - - expect(issue.reload.description_html).to be_nil - end - - it 'removes merge request descriptions' do - merge_request = create(:merge_request, - source_project: project, - target_project: project, - description_html: 'MergeRequest description') - - migration.up - - expect(merge_request.reload.description_html).to be_nil - end - - it 'removes note html' do - note = create(:note, - project: project, - noteable: create(:issue, project: project), - note_html: 'note description') - - migration.up - - expect(note.reload.note_html).to be_nil - end - - it 'removes milestone description' do - milestone = create(:milestone, - project: project, - description_html: 'milestone description') - - migration.up - - expect(milestone.reload.description_html).to be_nil - end - end - - context "system namespace -> subgroup -> system0 project" do - it "updates the route of the project correctly" do - subgroup = build(:group, path: "subgroup", parent: system_namespace) - save_invalid_routable(subgroup) - project = build(:project, :repository, path: "system0", namespace: subgroup) - save_invalid_routable(project) - - migration.up - - expect(project.route.reload.path).to eq("system0/subgroup/system0") - end - end - end - - describe "#move_repositories" do - let(:namespace) { create(:group, name: "hello-group") } - it "moves a project for a namespace" do - create(:project, :repository, namespace: namespace, path: "hello-project") - expected_path = File.join(TestEnv.repos_path, "bye-group", "hello-project.git") - - migration.move_repositories(namespace, "hello-group", "bye-group") - - expect(File.directory?(expected_path)).to be(true) - end - - it "moves a namespace in a subdirectory correctly" do - child_namespace = create(:group, name: "sub-group", parent: namespace) - create(:project, :repository, namespace: child_namespace, path: "hello-project") - - expected_path = File.join(TestEnv.repos_path, "hello-group", "renamed-sub-group", "hello-project.git") - - migration.move_repositories(child_namespace, "hello-group/sub-group", "hello-group/renamed-sub-group") - - expect(File.directory?(expected_path)).to be(true) - end - - it "moves a parent namespace with subdirectories" do - child_namespace = create(:group, name: "sub-group", parent: namespace) - create(:project, :repository, namespace: child_namespace, path: "hello-project") - expected_path = File.join(TestEnv.repos_path, "renamed-group", "sub-group", "hello-project.git") - - migration.move_repositories(child_namespace, "hello-group", "renamed-group") - - expect(File.directory?(expected_path)).to be(true) - end - end - - describe "#move_namespace_folders" do - it "moves a namespace with files" do - source = File.join(uploads_dir, "parent-group", "sub-group") - FileUtils.mkdir_p(source) - destination = File.join(uploads_dir, "parent-group", "moved-group") - FileUtils.touch(File.join(source, "test.txt")) - expected_file = File.join(destination, "test.txt") - - migration.move_namespace_folders(uploads_dir, File.join("parent-group", "sub-group"), File.join("parent-group", "moved-group")) - - expect(File.exist?(expected_file)).to be(true) - end - - it "moves a parent namespace uploads" do - source = File.join(uploads_dir, "parent-group", "sub-group") - FileUtils.mkdir_p(source) - destination = File.join(uploads_dir, "moved-parent", "sub-group") - FileUtils.touch(File.join(source, "test.txt")) - expected_file = File.join(destination, "test.txt") - - migration.move_namespace_folders(uploads_dir, "parent-group", "moved-parent") - - expect(File.exist?(expected_file)).to be(true) - end - end - - describe "#child_ids_for_parent" do - it "collects child ids for all levels" do - parent = create(:group) - first_child = create(:group, parent: parent) - second_child = create(:group, parent: parent) - third_child = create(:group, parent: second_child) - all_ids = [parent.id, first_child.id, second_child.id, third_child.id] - - collected_ids = migration.child_ids_for_parent(parent, ids: [parent.id]) - - expect(collected_ids).to contain_exactly(*all_ids) - end - end - - describe "#remove_last_ocurrence" do - it "removes only the last occurance of a string" do - input = "this/is/system/namespace/with/system" - - expect(migration.remove_last_occurrence(input, "system")).to eq("this/is/system/namespace/with/") - end - end -end diff --git a/spec/migrations/update_upload_paths_to_system_spec.rb b/spec/migrations/update_upload_paths_to_system_spec.rb index 11412005b72..0a45c5ea32d 100644 --- a/spec/migrations/update_upload_paths_to_system_spec.rb +++ b/spec/migrations/update_upload_paths_to_system_spec.rb @@ -11,7 +11,7 @@ describe UpdateUploadPathsToSystem do describe "#uploads_to_switch_to_new_path" do it "contains only uploads with the old path for the correct models" do _upload_for_other_type = create(:upload, model: create(:ci_pipeline), path: "uploads/ci_pipeline/avatar.jpg") - _upload_with_system_path = create(:upload, model: create(:project), path: "uploads/system/project/avatar.jpg") + _upload_with_system_path = create(:upload, model: create(:project), path: "uploads/-/system/project/avatar.jpg") _upload_with_other_path = create(:upload, model: create(:project), path: "thelongsecretforafileupload/avatar.jpg") old_upload = create(:upload, model: create(:project), path: "uploads/project/avatar.jpg") group_upload = create(:upload, model: create(:group), path: "uploads/group/avatar.jpg") @@ -23,7 +23,7 @@ describe UpdateUploadPathsToSystem do describe "#uploads_to_switch_to_old_path" do it "contains only uploads with the new path for the correct models" do _upload_for_other_type = create(:upload, model: create(:ci_pipeline), path: "uploads/ci_pipeline/avatar.jpg") - upload_with_system_path = create(:upload, model: create(:project), path: "uploads/system/project/avatar.jpg") + upload_with_system_path = create(:upload, model: create(:project), path: "uploads/-/system/project/avatar.jpg") _upload_with_other_path = create(:upload, model: create(:project), path: "thelongsecretforafileupload/avatar.jpg") _old_upload = create(:upload, model: create(:project), path: "uploads/project/avatar.jpg") @@ -37,13 +37,13 @@ describe UpdateUploadPathsToSystem do migration.up - expect(old_upload.reload.path).to eq("uploads/system/project/avatar.jpg") + expect(old_upload.reload.path).to eq("uploads/-/system/project/avatar.jpg") end end describe "#down", truncate: true do it "updates the new system patsh to the old paths" do - new_upload = create(:upload, model: create(:project), path: "uploads/system/project/avatar.jpg") + new_upload = create(:upload, model: create(:project), path: "uploads/-/system/project/avatar.jpg") migration.down diff --git a/spec/models/appearance_spec.rb b/spec/models/appearance_spec.rb index 7cd3a84d592..b5d5d58697b 100644 --- a/spec/models/appearance_spec.rb +++ b/spec/models/appearance_spec.rb @@ -9,4 +9,39 @@ RSpec.describe Appearance do it { is_expected.to validate_presence_of(:description) } it { is_expected.to have_many(:uploads).dependent(:destroy) } + + describe '.current', :use_clean_rails_memory_store_caching do + let!(:appearance) { create(:appearance) } + + it 'returns the current appearance row' do + expect(described_class.current).to eq(appearance) + end + + it 'caches the result' do + expect(described_class).to receive(:first).once + + 2.times { described_class.current } + end + end + + describe '#flush_redis_cache' do + it 'flushes the cache in Redis' do + appearance = create(:appearance) + + expect(Rails.cache).to receive(:delete).with(described_class::CACHE_KEY) + + appearance.flush_redis_cache + end + end + + describe '#single_appearance_row' do + it 'adds an error when more than 1 row exists' do + create(:appearance) + + new_row = build(:appearance) + new_row.save + + expect(new_row.valid?).to eq(false) + end + end end diff --git a/spec/models/broadcast_message_spec.rb b/spec/models/broadcast_message_spec.rb index a8ca1d110e4..3369aef1d3e 100644 --- a/spec/models/broadcast_message_spec.rb +++ b/spec/models/broadcast_message_spec.rb @@ -20,7 +20,7 @@ describe BroadcastMessage do it { is_expected.not_to allow_value('000').for(:font) } end - describe '.current' do + describe '.current', :use_clean_rails_memory_store_caching do it 'returns message if time match' do message = create(:broadcast_message) @@ -45,6 +45,14 @@ describe BroadcastMessage do expect(described_class.current).to be_empty end + + it 'caches the output of the query' do + create(:broadcast_message) + + expect(described_class).to receive(:where).and_call_original.once + + 2.times { described_class.current } + end end describe '#active?' do @@ -102,4 +110,14 @@ describe BroadcastMessage do end end end + + describe '#flush_redis_cache' do + it 'flushes the Redis cache' do + message = create(:broadcast_message) + + expect(Rails.cache).to receive(:delete).with(described_class::CACHE_KEY) + + message.flush_redis_cache + end + end 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/event_collection_spec.rb b/spec/models/event_collection_spec.rb new file mode 100644 index 00000000000..e0a87c18cc7 --- /dev/null +++ b/spec/models/event_collection_spec.rb @@ -0,0 +1,51 @@ +require 'spec_helper' + +describe EventCollection do + describe '#to_a' do + let(:project) { create(:project_empty_repo) } + let(:projects) { Project.where(id: project.id) } + let(:user) { create(:user) } + + before do + 20.times do + event = create(:push_event, project: project, author: user) + + create(:push_event_payload, event: event) + end + + create(:closed_issue_event, project: project, author: user) + end + + it 'returns an Array of events' do + events = described_class.new(projects).to_a + + expect(events).to be_an_instance_of(Array) + end + + it 'applies a limit to the number of events' do + events = described_class.new(projects).to_a + + expect(events.length).to eq(20) + end + + it 'can paginate through events' do + events = described_class.new(projects, offset: 20).to_a + + expect(events.length).to eq(1) + end + + it 'returns an empty Array when crossing the maximum page number' do + events = described_class.new(projects, limit: 1, offset: 15).to_a + + expect(events).to be_empty + end + + it 'allows filtering of events using an EventFilter' do + filter = EventFilter.new(EventFilter.issue) + events = described_class.new(projects, filter: filter).to_a + + expect(events.length).to eq(1) + expect(events[0].action).to eq(Event::CLOSED) + end + end +end diff --git a/spec/models/event_spec.rb b/spec/models/event_spec.rb index d86bf1a90a9..ff3224dd298 100644 --- a/spec/models/event_spec.rb +++ b/spec/models/event_spec.rb @@ -304,27 +304,15 @@ describe Event do end end - def create_push_event(project, user, attrs = {}) - data = { - before: Gitlab::Git::BLANK_SHA, - after: "0220c11b9a3e6c69dc8fd35321254ca9a7b98f7e", - ref: "refs/heads/master", - user_id: user.id, - user_name: user.name, - repository: { - name: project.name, - url: "localhost/rubinius", - description: "", - homepage: "localhost/rubinius", - private: true - } - } - - described_class.create({ - project: project, - action: described_class::PUSHED, - data: data, - author_id: user.id - }.merge!(attrs)) + def create_push_event(project, user) + event = create(:push_event, project: project, author: user) + + create(:push_event_payload, + event: event, + commit_to: '1cf19a015df3523caf0a1f9d40c98a267d6a2fc2', + commit_count: 0, + ref: 'master') + + event end end diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index 6d825ba68d1..9203f6562f2 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -57,18 +57,14 @@ describe Issue do end describe '#closed_at' do - after do - Timecop.return - end - - let!(:now) { Timecop.freeze(Time.now) } - it 'sets closed_at to Time.now when issue is closed' do issue = create(:issue, state: 'opened') + expect(issue.closed_at).to be_nil + issue.close - expect(issue.closed_at).to eq(now) + expect(issue.closed_at).to be_present end end diff --git a/spec/models/members/project_member_spec.rb b/spec/models/members/project_member_spec.rb index f1d1f37c78a..fa3e80ba062 100644 --- a/spec/models/members/project_member_spec.rb +++ b/spec/models/members/project_member_spec.rb @@ -149,7 +149,7 @@ describe ProjectMember do describe 'notifications' do describe '#after_accept_request' do it 'calls NotificationService.new_project_member' do - member = create(:project_member, user: build_stubbed(:user), requested_at: Time.now) + member = create(:project_member, user: create(:user), requested_at: Time.now) expect_any_instance_of(NotificationService).to receive(:new_project_member) 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..d9ab44dc49f 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -485,7 +485,7 @@ describe Project do describe 'last_activity' do it 'alias last_activity to last_event' do - last_event = create(:event, project: project) + last_event = create(:event, :closed, project: project) expect(project.last_activity).to eq(last_event) end @@ -493,7 +493,7 @@ describe Project do describe 'last_activity_date' do it 'returns the creation date of the project\'s last event if present' do - new_event = create(:event, project: project, created_at: Time.now) + new_event = create(:event, :closed, project: project, created_at: Time.now) project.reload expect(project.last_activity_at.to_i).to eq(new_event.created_at.to_i) @@ -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/models/push_event_payload_spec.rb b/spec/models/push_event_payload_spec.rb new file mode 100644 index 00000000000..a049ad35584 --- /dev/null +++ b/spec/models/push_event_payload_spec.rb @@ -0,0 +1,16 @@ +require 'spec_helper' + +describe PushEventPayload do + describe 'saving payloads' do + it 'does not allow commit messages longer than 70 characters' do + event = create(:push_event) + payload = build(:push_event_payload, event: event) + + expect(payload).to be_valid + + payload.commit_title = 'a' * 100 + + expect(payload).not_to be_valid + end + end +end diff --git a/spec/models/push_event_spec.rb b/spec/models/push_event_spec.rb new file mode 100644 index 00000000000..532fb024261 --- /dev/null +++ b/spec/models/push_event_spec.rb @@ -0,0 +1,202 @@ +require 'spec_helper' + +describe PushEvent do + let(:payload) { PushEventPayload.new } + + let(:event) do + event = described_class.new + + allow(event).to receive(:push_event_payload).and_return(payload) + + event + end + + describe '.sti_name' do + it 'returns Event::PUSHED' do + expect(described_class.sti_name).to eq(Event::PUSHED) + end + end + + describe '#push?' do + it 'returns true' do + expect(event).to be_push + end + end + + describe '#push_with_commits?' do + it 'returns true when both the first and last commit are present' do + allow(event).to receive(:commit_from).and_return('123') + allow(event).to receive(:commit_to).and_return('456') + + expect(event).to be_push_with_commits + end + + it 'returns false when the first commit is missing' do + allow(event).to receive(:commit_to).and_return('456') + + expect(event).not_to be_push_with_commits + end + + it 'returns false when the last commit is missing' do + allow(event).to receive(:commit_from).and_return('123') + + expect(event).not_to be_push_with_commits + end + end + + describe '#tag?' do + it 'returns true when pushing to a tag' do + allow(payload).to receive(:tag?).and_return(true) + + expect(event).to be_tag + end + + it 'returns false when pushing to a branch' do + allow(payload).to receive(:tag?).and_return(false) + + expect(event).not_to be_tag + end + end + + describe '#branch?' do + it 'returns true when pushing to a branch' do + allow(payload).to receive(:branch?).and_return(true) + + expect(event).to be_branch + end + + it 'returns false when pushing to a tag' do + allow(payload).to receive(:branch?).and_return(false) + + expect(event).not_to be_branch + end + end + + describe '#valid_push?' do + it 'returns true if a ref exists' do + allow(payload).to receive(:ref).and_return('master') + + expect(event).to be_valid_push + end + + it 'returns false when no ref is present' do + expect(event).not_to be_valid_push + end + end + + describe '#new_ref?' do + it 'returns true when pushing a new ref' do + allow(payload).to receive(:created?).and_return(true) + + expect(event).to be_new_ref + end + + it 'returns false when pushing to an existing ref' do + allow(payload).to receive(:created?).and_return(false) + + expect(event).not_to be_new_ref + end + end + + describe '#rm_ref?' do + it 'returns true when removing an existing ref' do + allow(payload).to receive(:removed?).and_return(true) + + expect(event).to be_rm_ref + end + + it 'returns false when pushing to an existing ref' do + allow(payload).to receive(:removed?).and_return(false) + + expect(event).not_to be_rm_ref + end + end + + describe '#commit_from' do + it 'returns the first commit SHA' do + allow(payload).to receive(:commit_from).and_return('123') + + expect(event.commit_from).to eq('123') + end + end + + describe '#commit_to' do + it 'returns the last commit SHA' do + allow(payload).to receive(:commit_to).and_return('123') + + expect(event.commit_to).to eq('123') + end + end + + describe '#ref_name' do + it 'returns the name of the ref' do + allow(payload).to receive(:ref).and_return('master') + + expect(event.ref_name).to eq('master') + end + end + + describe '#ref_type' do + it 'returns the type of the ref' do + allow(payload).to receive(:ref_type).and_return('branch') + + expect(event.ref_type).to eq('branch') + end + end + + describe '#branch_name' do + it 'returns the name of the branch' do + allow(payload).to receive(:ref).and_return('master') + + expect(event.branch_name).to eq('master') + end + end + + describe '#tag_name' do + it 'returns the name of the tag' do + allow(payload).to receive(:ref).and_return('1.2') + + expect(event.tag_name).to eq('1.2') + end + end + + describe '#commit_title' do + it 'returns the commit message' do + allow(payload).to receive(:commit_title).and_return('foo') + + expect(event.commit_title).to eq('foo') + end + end + + describe '#commit_id' do + it 'returns the SHA of the last commit if present' do + allow(event).to receive(:commit_to).and_return('123') + + expect(event.commit_id).to eq('123') + end + + it 'returns the SHA of the first commit if the last commit is not present' do + allow(event).to receive(:commit_to).and_return(nil) + allow(event).to receive(:commit_from).and_return('123') + + expect(event.commit_id).to eq('123') + end + end + + describe '#commits_count' do + it 'returns the number of commits' do + allow(payload).to receive(:commit_count).and_return(1) + + expect(event.commits_count).to eq(1) + end + end + + describe '#validate_push_action' do + it 'adds an error when the action is not PUSHED' do + event.action = Event::CREATED + event.validate_push_action + + expect(event.errors.count).to eq(1) + end + end +end diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 62f40c12c0a..4926d5d6c49 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -961,6 +961,27 @@ describe Repository, models: true do end end + context 'when temporary ref failed to be created from other project' do + let(:target_project) { create(:project, :empty_repo) } + + before do + expect(target_project.repository).to receive(:run_git) + end + + it 'raises Rugged::ReferenceError' do + raise_reference_error = raise_error(Rugged::ReferenceError) do |err| + expect(err.cause).to be_nil + end + + expect do + GitOperationService.new(user, target_project.repository) + .with_branch('feature', + start_project: project, + &:itself) + end.to raise_reference_error + end + end + context 'when the update adds more than one commit' do let(:old_rev) { '33f3729a45c02fc67d00adb1b8bca394b0e761d9' } diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 6c8248eeb40..97bb91a6ac8 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -1291,7 +1291,7 @@ describe User do let!(:project2) { create(:project, forked_from_project: project3) } let!(:project3) { create(:project) } let!(:merge_request) { create(:merge_request, source_project: project2, target_project: project3, author: subject) } - let!(:push_event) { create(:event, :pushed, project: project1, target: project1, author: subject) } + let!(:push_event) { create(:push_event, project: project1, author: subject) } let!(:merge_event) { create(:event, :created, project: project3, target: merge_request, author: subject) } before do @@ -1333,10 +1333,18 @@ describe User do subject { create(:user) } let!(:project1) { create(:project, :repository) } let!(:project2) { create(:project, :repository, forked_from_project: project1) } - let!(:push_data) do - Gitlab::DataBuilder::Push.build_sample(project2, subject) + + let!(:push_event) do + event = create(:push_event, project: project2, author: subject) + + create(:push_event_payload, + event: event, + commit_to: '1cf19a015df3523caf0a1f9d40c98a267d6a2fc2', + commit_count: 0, + ref: 'master') + + event end - let!(:push_event) { create(:event, :pushed, project: project2, target: project1, author: subject, data: push_data) } before do project1.team << [subject, :master] @@ -1363,8 +1371,13 @@ describe User do expect(subject.recent_push(project1)).to eq(nil) expect(subject.recent_push(project2)).to eq(push_event) - push_data1 = Gitlab::DataBuilder::Push.build_sample(project1, subject) - push_event1 = create(:event, :pushed, project: project1, target: project1, author: subject, data: push_data1) + push_event1 = create(:push_event, project: project1, author: subject) + + create(:push_event_payload, + event: push_event1, + commit_to: '1cf19a015df3523caf0a1f9d40c98a267d6a2fc2', + commit_count: 0, + ref: 'master') expect(subject.recent_push([project1, project2])).to eq(push_event1) # Newest end diff --git a/spec/requests/api/events_spec.rb b/spec/requests/api/events_spec.rb index f1a26b6ce6c..a23d28994ce 100644 --- a/spec/requests/api/events_spec.rb +++ b/spec/requests/api/events_spec.rb @@ -59,6 +59,34 @@ describe API::Events do expect(json_response.size).to eq(1) end + context 'when the list of events includes push events' do + let(:event) do + create(:push_event, author: user, project: private_project) + end + + let!(:payload) { create(:push_event_payload, event: event) } + let(:payload_hash) { json_response[0]['push_data'] } + + before do + get api("/users/#{user.id}/events?action=pushed", user) + end + + it 'responds with HTTP 200 OK' do + expect(response).to have_http_status(200) + end + + it 'includes the push payload as a Hash' do + expect(payload_hash).to be_an_instance_of(Hash) + end + + it 'includes the push payload details' do + expect(payload_hash['commit_count']).to eq(payload.commit_count) + expect(payload_hash['action']).to eq(payload.action) + expect(payload_hash['ref_type']).to eq(payload.ref_type) + expect(payload_hash['commit_to']).to eq(payload.commit_to) + end + end + context 'when there are multiple events from different projects' do let(:second_note) { create(:note_on_issue, project: create(:project)) } 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/internal_spec.rb b/spec/requests/api/internal_spec.rb index 8a2de23716f..e9c30dba8d4 100644 --- a/spec/requests/api/internal_spec.rb +++ b/spec/requests/api/internal_spec.rb @@ -181,13 +181,12 @@ describe API::Internal do describe "POST /internal/allowed", :clean_gitlab_redis_shared_state do context "access granted" do - before do - project.team << [user, :developer] - Timecop.freeze + around do |example| + Timecop.freeze { example.run } end - after do - Timecop.return + before do + project.team << [user, :developer] end context 'with env passed as a JSON' do diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index 1e8eccb9b1c..0db645863fb 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -31,7 +31,7 @@ describe API::MergeRequests do it 'returns authentication error' do get api('/merge_requests') - expect(response).to have_http_status(401) + expect(response).to have_gitlab_http_status(401) end end @@ -43,7 +43,7 @@ describe API::MergeRequests do it 'returns an array of all merge requests' do get api('/merge_requests', user), scope: :all - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) expect(response).to include_pagination_headers expect(json_response).to be_an Array expect(json_response.map { |mr| mr['id'] }) @@ -56,7 +56,7 @@ describe API::MergeRequests do get api('/merge_requests', user), scope: :all - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) expect(response).to include_pagination_headers expect(json_response).to be_an Array expect(json_response.map { |mr| mr['id'] }) @@ -68,7 +68,7 @@ describe API::MergeRequests do get api('/merge_requests', user2) - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) expect(json_response).to be_an Array expect(json_response.length).to eq(1) expect(json_response.first['id']).to eq(merge_request3.id) @@ -79,7 +79,7 @@ describe API::MergeRequests do get api('/merge_requests', user), author_id: user2.id, scope: :all - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) expect(json_response).to be_an Array expect(json_response.length).to eq(1) expect(json_response.first['id']).to eq(merge_request3.id) @@ -90,7 +90,7 @@ describe API::MergeRequests do get api('/merge_requests', user), assignee_id: user2.id, scope: :all - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) expect(json_response).to be_an Array expect(json_response.length).to eq(1) expect(json_response.first['id']).to eq(merge_request3.id) @@ -101,7 +101,7 @@ describe API::MergeRequests do get api('/merge_requests', user2), scope: 'assigned-to-me' - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) expect(json_response).to be_an Array expect(json_response.length).to eq(1) expect(json_response.first['id']).to eq(merge_request3.id) @@ -112,7 +112,7 @@ describe API::MergeRequests do get api('/merge_requests', user2), scope: 'created-by-me' - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) expect(json_response).to be_an Array expect(json_response.length).to eq(1) expect(json_response.first['id']).to eq(merge_request3.id) @@ -125,7 +125,7 @@ describe API::MergeRequests do it "returns authentication error" do get api("/projects/#{project.id}/merge_requests") - expect(response).to have_http_status(401) + expect(response).to have_gitlab_http_status(401) end end @@ -145,7 +145,7 @@ describe API::MergeRequests do it "returns an array of all merge_requests" do get api("/projects/#{project.id}/merge_requests", user) - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) expect(response).to include_pagination_headers expect(json_response).to be_an Array expect(json_response.length).to eq(3) @@ -166,7 +166,7 @@ describe API::MergeRequests do it "returns an array of all merge_requests using simple mode" do get api("/projects/#{project.id}/merge_requests?view=simple", user) - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) expect(response).to include_pagination_headers expect(json_response.last.keys).to match_array(%w(id iid title web_url created_at description project_id state updated_at)) expect(json_response).to be_an Array @@ -182,7 +182,7 @@ describe API::MergeRequests do it "returns an array of all merge_requests" do get api("/projects/#{project.id}/merge_requests?state", user) - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) expect(response).to include_pagination_headers expect(json_response).to be_an Array expect(json_response.length).to eq(3) @@ -192,7 +192,7 @@ describe API::MergeRequests do it "returns an array of open merge_requests" do get api("/projects/#{project.id}/merge_requests?state=opened", user) - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) expect(response).to include_pagination_headers expect(json_response).to be_an Array expect(json_response.length).to eq(1) @@ -202,7 +202,7 @@ describe API::MergeRequests do it "returns an array of closed merge_requests" do get api("/projects/#{project.id}/merge_requests?state=closed", user) - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) expect(response).to include_pagination_headers expect(json_response).to be_an Array expect(json_response.length).to eq(1) @@ -212,7 +212,7 @@ describe API::MergeRequests do it "returns an array of merged merge_requests" do get api("/projects/#{project.id}/merge_requests?state=merged", user) - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) expect(response).to include_pagination_headers expect(json_response).to be_an Array expect(json_response.length).to eq(1) @@ -222,7 +222,7 @@ describe API::MergeRequests do it 'returns merge_request by "iids" array' do get api("/projects/#{project.id}/merge_requests", user), iids: [merge_request.iid, merge_request_closed.iid] - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) expect(json_response).to be_an Array expect(json_response.length).to eq(2) expect(json_response.first['title']).to eq merge_request_closed.title @@ -232,14 +232,14 @@ describe API::MergeRequests do it 'matches V4 response schema' do get api("/projects/#{project.id}/merge_requests", user) - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) expect(response).to match_response_schema('public_api/v4/merge_requests') end it 'returns an empty array if no issue matches milestone' do get api("/projects/#{project.id}/merge_requests", user), milestone: '1.0.0' - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) expect(json_response).to be_an Array expect(json_response.length).to eq(0) end @@ -247,7 +247,7 @@ describe API::MergeRequests do it 'returns an empty array if milestone does not exist' do get api("/projects/#{project.id}/merge_requests", user), milestone: 'foo' - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) expect(json_response).to be_an Array expect(json_response.length).to eq(0) end @@ -262,7 +262,7 @@ describe API::MergeRequests do it 'returns an array of merge requests matching state in milestone' do get api("/projects/#{project.id}/merge_requests", user), milestone: '0.9', state: 'closed' - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) expect(json_response).to be_an Array expect(json_response.length).to eq(1) expect(json_response.first['id']).to eq(merge_request_closed.id) @@ -271,7 +271,7 @@ describe API::MergeRequests do it 'returns an array of labeled merge requests' do get api("/projects/#{project.id}/merge_requests?labels=#{label.title}", user) - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) expect(json_response).to be_an Array expect(json_response.length).to eq(1) expect(json_response.first['labels']).to eq([label2.title, label.title]) @@ -280,7 +280,7 @@ describe API::MergeRequests do it 'returns an array of labeled merge requests where all labels match' do get api("/projects/#{project.id}/merge_requests?labels=#{label.title},foo,bar", user) - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) expect(json_response).to be_an Array expect(json_response.length).to eq(0) end @@ -288,7 +288,7 @@ describe API::MergeRequests do it 'returns an empty array if no merge request matches labels' do get api("/projects/#{project.id}/merge_requests?labels=foo,bar", user) - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) expect(json_response).to be_an Array expect(json_response.length).to eq(0) end @@ -307,7 +307,7 @@ describe API::MergeRequests do get api("/projects/#{project.id}/merge_requests?labels=#{bug_label.title}&milestone=#{milestone1.title}&state=merged", user) - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) expect(json_response).to be_an Array expect(json_response.length).to eq(1) expect(json_response.first['id']).to eq(mr2.id) @@ -322,7 +322,7 @@ describe API::MergeRequests do it "returns an array of merge_requests in ascending order" do get api("/projects/#{project.id}/merge_requests?sort=asc", user) - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) expect(response).to include_pagination_headers expect(json_response).to be_an Array expect(json_response.length).to eq(3) @@ -333,7 +333,7 @@ describe API::MergeRequests do it "returns an array of merge_requests in descending order" do get api("/projects/#{project.id}/merge_requests?sort=desc", user) - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) expect(response).to include_pagination_headers expect(json_response).to be_an Array expect(json_response.length).to eq(3) @@ -344,7 +344,7 @@ describe API::MergeRequests do it "returns an array of merge_requests ordered by updated_at" do get api("/projects/#{project.id}/merge_requests?order_by=updated_at", user) - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) expect(response).to include_pagination_headers expect(json_response).to be_an Array expect(json_response.length).to eq(3) @@ -355,7 +355,7 @@ describe API::MergeRequests do it "returns an array of merge_requests ordered by created_at" do get api("/projects/#{project.id}/merge_requests?order_by=created_at&sort=asc", user) - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) expect(response).to include_pagination_headers expect(json_response).to be_an Array expect(json_response.length).to eq(3) @@ -370,7 +370,7 @@ describe API::MergeRequests do it 'exposes known attributes' do get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user) - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) expect(json_response['id']).to eq(merge_request.id) expect(json_response['iid']).to eq(merge_request.iid) expect(json_response['project_id']).to eq(merge_request.project.id) @@ -398,7 +398,7 @@ describe API::MergeRequests do it "returns merge_request" do get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user) - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) expect(json_response['title']).to eq(merge_request.title) expect(json_response['iid']).to eq(merge_request.iid) expect(json_response['work_in_progress']).to eq(false) @@ -409,13 +409,13 @@ describe API::MergeRequests do it "returns a 404 error if merge_request_iid not found" do get api("/projects/#{project.id}/merge_requests/999", user) - expect(response).to have_http_status(404) + expect(response).to have_gitlab_http_status(404) end it "returns a 404 error if merge_request `id` is used instead of iid" do get api("/projects/#{project.id}/merge_requests/#{merge_request.id}", user) - expect(response).to have_http_status(404) + expect(response).to have_gitlab_http_status(404) end context 'Work in Progress' do @@ -423,7 +423,7 @@ describe API::MergeRequests do it "returns merge_request" do get api("/projects/#{project.id}/merge_requests/#{merge_request_wip.iid}", user) - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) expect(json_response['work_in_progress']).to eq(true) end end @@ -434,7 +434,7 @@ describe API::MergeRequests do get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/commits", user) commit = merge_request.commits.first - expect(response.status).to eq 200 + expect(response).to have_gitlab_http_status(200) expect(response).to include_pagination_headers expect(json_response).to be_an Array expect(json_response.size).to eq(merge_request.commits.size) @@ -444,13 +444,13 @@ describe API::MergeRequests do it 'returns a 404 when merge_request_iid not found' do get api("/projects/#{project.id}/merge_requests/999/commits", user) - expect(response).to have_http_status(404) + expect(response).to have_gitlab_http_status(404) end it 'returns a 404 when merge_request id is used instead of iid' do get api("/projects/#{project.id}/merge_requests/#{merge_request.id}/commits", user) - expect(response).to have_http_status(404) + expect(response).to have_gitlab_http_status(404) end end @@ -458,19 +458,19 @@ describe API::MergeRequests do it 'returns the change information of the merge_request' do get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/changes", user) - expect(response.status).to eq 200 + expect(response).to have_gitlab_http_status(200) expect(json_response['changes'].size).to eq(merge_request.diffs.size) end it 'returns a 404 when merge_request_iid not found' do get api("/projects/#{project.id}/merge_requests/999/changes", user) - expect(response).to have_http_status(404) + expect(response).to have_gitlab_http_status(404) end it 'returns a 404 when merge_request id is used instead of iid' do get api("/projects/#{project.id}/merge_requests/#{merge_request.id}/changes", user) - expect(response).to have_http_status(404) + expect(response).to have_gitlab_http_status(404) end end @@ -485,7 +485,7 @@ describe API::MergeRequests do labels: 'label, label2', milestone_id: milestone.id - expect(response).to have_http_status(201) + expect(response).to have_gitlab_http_status(201) expect(json_response['title']).to eq('Test merge_request') expect(json_response['labels']).to eq(%w(label label2)) expect(json_response['milestone']['id']).to eq(milestone.id) @@ -495,25 +495,25 @@ describe API::MergeRequests do it "returns 422 when source_branch equals target_branch" do post api("/projects/#{project.id}/merge_requests", user), title: "Test merge_request", source_branch: "master", target_branch: "master", author: user - expect(response).to have_http_status(422) + expect(response).to have_gitlab_http_status(422) end it "returns 400 when source_branch is missing" do post api("/projects/#{project.id}/merge_requests", user), title: "Test merge_request", target_branch: "master", author: user - expect(response).to have_http_status(400) + expect(response).to have_gitlab_http_status(400) end it "returns 400 when target_branch is missing" do post api("/projects/#{project.id}/merge_requests", user), title: "Test merge_request", source_branch: "markdown", author: user - expect(response).to have_http_status(400) + expect(response).to have_gitlab_http_status(400) end it "returns 400 when title is missing" do post api("/projects/#{project.id}/merge_requests", user), target_branch: 'master', source_branch: 'markdown' - expect(response).to have_http_status(400) + expect(response).to have_gitlab_http_status(400) end it 'allows special label names' do @@ -523,7 +523,7 @@ describe API::MergeRequests do target_branch: 'master', author: user, labels: 'label, label?, label&foo, ?, &' - expect(response.status).to eq(201) + expect(response).to have_gitlab_http_status(201) expect(json_response['labels']).to include 'label' expect(json_response['labels']).to include 'label?' expect(json_response['labels']).to include 'label&foo' @@ -549,7 +549,7 @@ describe API::MergeRequests do target_branch: 'master', author: user end.to change { MergeRequest.count }.by(0) - expect(response).to have_http_status(409) + expect(response).to have_gitlab_http_status(409) end end @@ -580,15 +580,17 @@ 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| - fork_project.team << [user2, :reporter] + before do + fork_project.add_reporter(user2) + + allow_any_instance_of(MergeRequest).to receive(:write_ref) end it "returns merge_request" do post api("/projects/#{fork_project.id}/merge_requests", user2), title: 'Test merge_request', source_branch: "feature_conflict", target_branch: "master", author: user2, target_project_id: project.id, description: 'Test description for Test merge_request' - expect(response).to have_http_status(201) + expect(response).to have_gitlab_http_status(201) expect(json_response['title']).to eq('Test merge_request') expect(json_response['description']).to eq('Test description for Test merge_request') end @@ -599,7 +601,7 @@ describe API::MergeRequests do expect(fork_project.forked_from_project).to eq(project) post api("/projects/#{fork_project.id}/merge_requests", user2), title: 'Test merge_request', source_branch: "master", target_branch: "master", author: user2, target_project_id: project.id - expect(response).to have_http_status(201) + expect(response).to have_gitlab_http_status(201) expect(json_response['title']).to eq('Test merge_request') end @@ -613,25 +615,25 @@ describe API::MergeRequests do author: user2, target_project_id: project.id - expect(response).to have_http_status(422) + expect(response).to have_gitlab_http_status(422) end it "returns 400 when source_branch is missing" do post api("/projects/#{fork_project.id}/merge_requests", user2), title: 'Test merge_request', target_branch: "master", author: user2, target_project_id: project.id - expect(response).to have_http_status(400) + expect(response).to have_gitlab_http_status(400) end it "returns 400 when target_branch is missing" do post api("/projects/#{fork_project.id}/merge_requests", user2), title: 'Test merge_request', target_branch: "master", author: user2, target_project_id: project.id - expect(response).to have_http_status(400) + expect(response).to have_gitlab_http_status(400) end it "returns 400 when title is missing" do post api("/projects/#{fork_project.id}/merge_requests", user2), target_branch: 'master', source_branch: 'markdown', author: user2, target_project_id: project.id - expect(response).to have_http_status(400) + expect(response).to have_gitlab_http_status(400) end context 'when target_branch is specified' do @@ -642,7 +644,7 @@ describe API::MergeRequests do source_branch: 'markdown', author: user, target_project_id: fork_project.id - expect(response).to have_http_status(422) + expect(response).to have_gitlab_http_status(422) end it 'returns 422 if targeting a different fork' do @@ -652,14 +654,14 @@ describe API::MergeRequests do source_branch: 'markdown', author: user2, target_project_id: unrelated_project.id - expect(response).to have_http_status(422) + expect(response).to have_gitlab_http_status(422) end end it "returns 201 when target_branch is specified and for the same project" do post api("/projects/#{fork_project.id}/merge_requests", user2), title: 'Test merge_request', target_branch: 'master', source_branch: 'markdown', author: user2, target_project_id: fork_project.id - expect(response).to have_http_status(201) + expect(response).to have_gitlab_http_status(201) end end end @@ -674,7 +676,7 @@ describe API::MergeRequests do it "denies the deletion of the merge request" do delete api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", developer) - expect(response).to have_http_status(403) + expect(response).to have_gitlab_http_status(403) end end @@ -682,19 +684,19 @@ describe API::MergeRequests do it "destroys the merge request owners can destroy" do delete api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user) - expect(response).to have_http_status(204) + expect(response).to have_gitlab_http_status(204) end it "returns 404 for an invalid merge request IID" do delete api("/projects/#{project.id}/merge_requests/12345", user) - expect(response).to have_http_status(404) + expect(response).to have_gitlab_http_status(404) end it "returns 404 if the merge request id is used instead of iid" do delete api("/projects/#{project.id}/merge_requests/#{merge_request.id}", user) - expect(response).to have_http_status(404) + expect(response).to have_gitlab_http_status(404) end end end @@ -705,7 +707,7 @@ describe API::MergeRequests do it "returns merge_request in case of success" do put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user) - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) end it "returns 406 if branch can't be merged" do @@ -714,21 +716,21 @@ describe API::MergeRequests do put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user) - expect(response).to have_http_status(406) + expect(response).to have_gitlab_http_status(406) expect(json_response['message']).to eq('Branch cannot be merged') end it "returns 405 if merge_request is not open" do merge_request.close put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user) - expect(response).to have_http_status(405) + expect(response).to have_gitlab_http_status(405) expect(json_response['message']).to eq('405 Method Not Allowed') end it "returns 405 if merge_request is a work in progress" do merge_request.update_attribute(:title, "WIP: #{merge_request.title}") put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user) - expect(response).to have_http_status(405) + expect(response).to have_gitlab_http_status(405) expect(json_response['message']).to eq('405 Method Not Allowed') end @@ -737,7 +739,7 @@ describe API::MergeRequests do put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user) - expect(response).to have_http_status(405) + expect(response).to have_gitlab_http_status(405) expect(json_response['message']).to eq('405 Method Not Allowed') end @@ -745,21 +747,21 @@ describe API::MergeRequests do user2 = create(:user) project.team << [user2, :reporter] put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user2) - expect(response).to have_http_status(401) + expect(response).to have_gitlab_http_status(401) expect(json_response['message']).to eq('401 Unauthorized') end it "returns 409 if the SHA parameter doesn't match" do put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user), sha: merge_request.diff_head_sha.reverse - expect(response).to have_http_status(409) + expect(response).to have_gitlab_http_status(409) expect(json_response['message']).to start_with('SHA does not match HEAD of source branch') end it "succeeds if the SHA parameter matches" do put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user), sha: merge_request.diff_head_sha - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) end it "enables merge when pipeline succeeds if the pipeline is active" do @@ -768,7 +770,7 @@ describe API::MergeRequests do put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user), merge_when_pipeline_succeeds: true - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) expect(json_response['title']).to eq('Test') expect(json_response['merge_when_pipeline_succeeds']).to eq(true) end @@ -780,7 +782,7 @@ describe API::MergeRequests do put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user), merge_when_pipeline_succeeds: true - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) expect(json_response['title']).to eq('Test') expect(json_response['merge_when_pipeline_succeeds']).to eq(true) end @@ -788,13 +790,13 @@ describe API::MergeRequests do it "returns 404 for an invalid merge request IID" do put api("/projects/#{project.id}/merge_requests/12345/merge", user) - expect(response).to have_http_status(404) + expect(response).to have_gitlab_http_status(404) end it "returns 404 if the merge request id is used instead of iid" do put api("/projects/#{project.id}/merge_requests/#{merge_request.id}/merge", user) - expect(response).to have_http_status(404) + expect(response).to have_gitlab_http_status(404) end end @@ -803,39 +805,39 @@ describe API::MergeRequests do it "returns merge_request" do put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user), state_event: "close" - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) expect(json_response['state']).to eq('closed') end end it "updates title and returns merge_request" do put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user), title: "New title" - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) expect(json_response['title']).to eq('New title') end it "updates description and returns merge_request" do put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user), description: "New description" - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) expect(json_response['description']).to eq('New description') end it "updates milestone_id and returns merge_request" do put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user), milestone_id: milestone.id - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) expect(json_response['milestone']['id']).to eq(milestone.id) end it "returns merge_request with renamed target_branch" do put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user), target_branch: "wiki" - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) expect(json_response['target_branch']).to eq('wiki') end it "returns merge_request that removes the source branch" do put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user), remove_source_branch: true - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) expect(json_response['force_remove_source_branch']).to be_truthy end @@ -856,7 +858,7 @@ describe API::MergeRequests do put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user), state_event: 'close', title: nil merge_request.reload - expect(response).to have_http_status(400) + expect(response).to have_gitlab_http_status(400) expect(merge_request.state).to eq('opened') end @@ -864,20 +866,20 @@ describe API::MergeRequests do put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user), state_event: 'close', target_branch: nil merge_request.reload - expect(response).to have_http_status(400) + expect(response).to have_gitlab_http_status(400) expect(merge_request.state).to eq('opened') end it "returns 404 for an invalid merge request IID" do put api("/projects/#{project.id}/merge_requests/12345", user), state_event: "close" - expect(response).to have_http_status(404) + expect(response).to have_gitlab_http_status(404) end it "returns 404 if the merge request id is used instead of iid" do put api("/projects/#{project.id}/merge_requests/#{merge_request.id}", user), state_event: "close" - expect(response).to have_http_status(404) + expect(response).to have_gitlab_http_status(404) end end @@ -890,7 +892,7 @@ describe API::MergeRequests do get api("/projects/#{project.id}/merge_requests/#{mr.iid}/closes_issues", user) - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) expect(response).to include_pagination_headers expect(json_response).to be_an Array expect(json_response.length).to eq(1) @@ -900,7 +902,7 @@ describe API::MergeRequests do it 'returns an empty array when there are no issues to be closed' do get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/closes_issues", user) - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) expect(response).to include_pagination_headers expect(json_response).to be_an Array expect(json_response.length).to eq(0) @@ -916,7 +918,7 @@ describe API::MergeRequests do get api("/projects/#{jira_project.id}/merge_requests/#{merge_request.iid}/closes_issues", user) - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) expect(response).to include_pagination_headers expect(json_response).to be_an Array expect(json_response.length).to eq(2) @@ -936,19 +938,19 @@ describe API::MergeRequests do get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/closes_issues", guest) - expect(response).to have_http_status(403) + expect(response).to have_gitlab_http_status(403) end it "returns 404 for an invalid merge request IID" do get api("/projects/#{project.id}/merge_requests/12345/closes_issues", user) - expect(response).to have_http_status(404) + expect(response).to have_gitlab_http_status(404) end it "returns 404 if the merge request id is used instead of iid" do get api("/projects/#{project.id}/merge_requests/#{merge_request.id}/closes_issues", user) - expect(response).to have_http_status(404) + expect(response).to have_gitlab_http_status(404) end end @@ -956,26 +958,26 @@ describe API::MergeRequests do it 'subscribes to a merge request' do post api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/subscribe", admin) - expect(response).to have_http_status(201) + expect(response).to have_gitlab_http_status(201) expect(json_response['subscribed']).to eq(true) end it 'returns 304 if already subscribed' do post api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/subscribe", user) - expect(response).to have_http_status(304) + expect(response).to have_gitlab_http_status(304) end it 'returns 404 if the merge request is not found' do post api("/projects/#{project.id}/merge_requests/123/subscribe", user) - expect(response).to have_http_status(404) + expect(response).to have_gitlab_http_status(404) end it 'returns 404 if the merge request id is used instead of iid' do post api("/projects/#{project.id}/merge_requests/#{merge_request.id}/subscribe", user) - expect(response).to have_http_status(404) + expect(response).to have_gitlab_http_status(404) end it 'returns 403 if user has no access to read code' do @@ -984,7 +986,7 @@ describe API::MergeRequests do post api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/subscribe", guest) - expect(response).to have_http_status(403) + expect(response).to have_gitlab_http_status(403) end end @@ -992,26 +994,26 @@ describe API::MergeRequests do it 'unsubscribes from a merge request' do post api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/unsubscribe", user) - expect(response).to have_http_status(201) + expect(response).to have_gitlab_http_status(201) expect(json_response['subscribed']).to eq(false) end it 'returns 304 if not subscribed' do post api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/unsubscribe", admin) - expect(response).to have_http_status(304) + expect(response).to have_gitlab_http_status(304) end it 'returns 404 if the merge request is not found' do post api("/projects/#{project.id}/merge_requests/123/unsubscribe", user) - expect(response).to have_http_status(404) + expect(response).to have_gitlab_http_status(404) end it 'returns 404 if the merge request id is used instead of iid' do post api("/projects/#{project.id}/merge_requests/#{merge_request.id}/unsubscribe", user) - expect(response).to have_http_status(404) + expect(response).to have_gitlab_http_status(404) end it 'returns 403 if user has no access to read code' do @@ -1020,7 +1022,7 @@ describe API::MergeRequests do post api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/unsubscribe", guest) - expect(response).to have_http_status(403) + expect(response).to have_gitlab_http_status(403) end end diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index 9baac12821f..6cb27d16fe5 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -8,8 +8,8 @@ describe API::Projects do let(:user2) { create(:user) } let(:user3) { create(:user) } let(:admin) { create(:admin) } - let(:project) { create(:project, creator_id: user.id, namespace: user.namespace) } - let(:project2) { create(:project, path: 'project2', creator_id: user.id, namespace: user.namespace) } + let(:project) { create(:project, namespace: user.namespace) } + let(:project2) { create(:project, path: 'project2', namespace: user.namespace) } let(:snippet) { create(:project_snippet, :public, author: user, project: project, title: 'example') } let(:project_member) { create(:project_member, :developer, user: user3, project: project) } let(:user4) { create(:user) } diff --git a/spec/requests/api/settings_spec.rb b/spec/requests/api/settings_spec.rb index c3ed5cd8ece..97275b80d03 100644 --- a/spec/requests/api/settings_spec.rb +++ b/spec/requests/api/settings_spec.rb @@ -43,7 +43,9 @@ describe API::Settings, 'Settings' do default_artifacts_expire_in: '2 days', help_page_text: 'custom help text', help_page_hide_commercial_content: true, - help_page_support_url: 'http://example.com/help' + help_page_support_url: 'http://example.com/help', + project_export_enabled: false + expect(response).to have_http_status(200) expect(json_response['default_projects_limit']).to eq(3) expect(json_response['password_authentication_enabled']).to be_falsey @@ -58,6 +60,7 @@ describe API::Settings, 'Settings' do expect(json_response['help_page_text']).to eq('custom help text') expect(json_response['help_page_hide_commercial_content']).to be_truthy expect(json_response['help_page_support_url']).to eq('http://example.com/help') + expect(json_response['project_export_enabled']).to be_falsey end end diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index 2dc7be22f8f..49739a1601a 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -217,9 +217,19 @@ describe API::Users do it "does not return the user's `is_admin` flag" do get api("/users/#{user.id}", user) + expect(response).to have_http_status(200) expect(json_response['is_admin']).to be_nil end + context 'when authenticated as admin' do + it 'includes the `is_admin` field' do + get api("/users/#{user.id}", admin) + + expect(response).to have_http_status(200) + expect(json_response['is_admin']).to be(false) + end + end + context 'for an anonymous user' do it "returns a user by id" do get api("/users/#{user.id}") 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..86f38dd4ec1 100644 --- a/spec/requests/api/v3/merge_requests_spec.rb +++ b/spec/requests/api/v3/merge_requests_spec.rb @@ -315,15 +315,17 @@ 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| - fork_project.team << [user2, :reporter] + before do + fork_project.add_reporter(user2) + + allow_any_instance_of(MergeRequest).to receive(:write_ref) end it "returns merge_request" do post v3_api("/projects/#{fork_project.id}/merge_requests", user2), title: 'Test merge_request', source_branch: "feature_conflict", target_branch: "master", author: user2, target_project_id: project.id, description: 'Test description for Test merge_request' - expect(response).to have_http_status(201) + expect(response).to have_gitlab_http_status(201) expect(json_response['title']).to eq('Test merge_request') expect(json_response['description']).to eq('Test description for Test merge_request') end @@ -334,7 +336,7 @@ describe API::MergeRequests do expect(fork_project.forked_from_project).to eq(project) post v3_api("/projects/#{fork_project.id}/merge_requests", user2), title: 'Test merge_request', source_branch: "master", target_branch: "master", author: user2, target_project_id: project.id - expect(response).to have_http_status(201) + expect(response).to have_gitlab_http_status(201) expect(json_response['title']).to eq('Test merge_request') end @@ -348,25 +350,25 @@ describe API::MergeRequests do author: user2, target_project_id: project.id - expect(response).to have_http_status(422) + expect(response).to have_gitlab_http_status(422) end it "returns 400 when source_branch is missing" do post v3_api("/projects/#{fork_project.id}/merge_requests", user2), title: 'Test merge_request', target_branch: "master", author: user2, target_project_id: project.id - expect(response).to have_http_status(400) + expect(response).to have_gitlab_http_status(400) end it "returns 400 when target_branch is missing" do post v3_api("/projects/#{fork_project.id}/merge_requests", user2), title: 'Test merge_request', target_branch: "master", author: user2, target_project_id: project.id - expect(response).to have_http_status(400) + expect(response).to have_gitlab_http_status(400) end it "returns 400 when title is missing" do post v3_api("/projects/#{fork_project.id}/merge_requests", user2), target_branch: 'master', source_branch: 'markdown', author: user2, target_project_id: project.id - expect(response).to have_http_status(400) + expect(response).to have_gitlab_http_status(400) end context 'when target_branch is specified' do @@ -377,7 +379,7 @@ describe API::MergeRequests do source_branch: 'markdown', author: user, target_project_id: fork_project.id - expect(response).to have_http_status(422) + expect(response).to have_gitlab_http_status(422) end it 'returns 422 if targeting a different fork' do @@ -387,14 +389,14 @@ describe API::MergeRequests do source_branch: 'markdown', author: user2, target_project_id: unrelated_project.id - expect(response).to have_http_status(422) + expect(response).to have_gitlab_http_status(422) end end it "returns 201 when target_branch is specified and for the same project" do post v3_api("/projects/#{fork_project.id}/merge_requests", user2), title: 'Test merge_request', target_branch: 'master', source_branch: 'markdown', author: user2, target_project_id: fork_project.id - expect(response).to have_http_status(201) + expect(response).to have_gitlab_http_status(201) end end end diff --git a/spec/requests/api/v3/users_spec.rb b/spec/requests/api/v3/users_spec.rb index bc0a4ab20a3..227b8d1b0c1 100644 --- a/spec/requests/api/v3/users_spec.rb +++ b/spec/requests/api/v3/users_spec.rb @@ -252,6 +252,31 @@ describe API::V3::Users do end context "as a user than can see the event's project" do + context 'when the list of events includes push events' do + let(:event) { create(:push_event, author: user, project: project) } + let!(:payload) { create(:push_event_payload, event: event) } + let(:payload_hash) { json_response[0]['push_data'] } + + before do + get api("/users/#{user.id}/events?action=pushed", user) + end + + it 'responds with HTTP 200 OK' do + expect(response).to have_http_status(200) + end + + it 'includes the push payload as a Hash' do + expect(payload_hash).to be_an_instance_of(Hash) + end + + it 'includes the push payload details' do + expect(payload_hash['commit_count']).to eq(payload.commit_count) + expect(payload_hash['action']).to eq(payload.action) + expect(payload_hash['ref_type']).to eq(payload.ref_type) + expect(payload_hash['commit_to']).to eq(payload.commit_to) + end + end + context 'joined event' do it 'returns the "joined" event' do get v3_api("/users/#{user.id}/events", user) diff --git a/spec/rubocop/cop/migration/safer_boolean_column_spec.rb b/spec/rubocop/cop/migration/safer_boolean_column_spec.rb new file mode 100644 index 00000000000..1006594499a --- /dev/null +++ b/spec/rubocop/cop/migration/safer_boolean_column_spec.rb @@ -0,0 +1,85 @@ +require 'spec_helper' + +require 'rubocop' +require 'rubocop/rspec/support' + +require_relative '../../../../rubocop/cop/migration/safer_boolean_column' + +describe RuboCop::Cop::Migration::SaferBooleanColumn do + include CopHelper + + subject(:cop) { described_class.new } + + context 'in migration' do + before do + allow(cop).to receive(:in_migration?).and_return(true) + end + + described_class::SMALL_TABLES.each do |table| + context "for the #{table} table" do + sources_and_offense = [ + ["add_column :#{table}, :column, :boolean, default: true", 'should disallow nulls'], + ["add_column :#{table}, :column, :boolean, default: false", 'should disallow nulls'], + ["add_column :#{table}, :column, :boolean, default: nil", 'should have a default and should disallow nulls'], + ["add_column :#{table}, :column, :boolean, null: false", 'should have a default'], + ["add_column :#{table}, :column, :boolean, null: true", 'should have a default and should disallow nulls'], + ["add_column :#{table}, :column, :boolean", 'should have a default and should disallow nulls'], + ["add_column :#{table}, :column, :boolean, default: nil, null: false", 'should have a default'], + ["add_column :#{table}, :column, :boolean, default: nil, null: true", 'should have a default and should disallow nulls'], + ["add_column :#{table}, :column, :boolean, default: false, null: true", 'should disallow nulls'] + ] + + sources_and_offense.each do |source, offense| + context "given the source \"#{source}\"" do + it "registers the offense matching \"#{offense}\"" do + inspect_source(cop, source) + + aggregate_failures do + expect(cop.offenses.first.message).to match(offense) + end + end + end + end + + inoffensive_sources = [ + "add_column :#{table}, :column, :boolean, default: true, null: false", + "add_column :#{table}, :column, :boolean, default: false, null: false" + ] + + inoffensive_sources.each do |source| + context "given the source \"#{source}\"" do + it "registers no offense" do + inspect_source(cop, source) + + aggregate_failures do + expect(cop.offenses).to be_empty + end + end + end + end + end + end + + it 'registers no offense for tables not listed in SMALL_TABLES' do + inspect_source(cop, "add_column :large_table, :column, :boolean") + + expect(cop.offenses).to be_empty + end + + it 'registers no offense for non-boolean columns' do + table = described_class::SMALL_TABLES.sample + inspect_source(cop, "add_column :#{table}, :column, :string") + + expect(cop.offenses).to be_empty + end + end + + context 'outside of migration' do + it 'registers no offense' do + table = described_class::SMALL_TABLES.sample + inspect_source(cop, "add_column :#{table}, :column, :boolean") + + expect(cop.offenses).to be_empty + end + end +end diff --git a/spec/serializers/analytics_build_entity_spec.rb b/spec/serializers/analytics_build_entity_spec.rb index 9f26d5cd09a..1ff4908972a 100644 --- a/spec/serializers/analytics_build_entity_spec.rb +++ b/spec/serializers/analytics_build_entity_spec.rb @@ -13,12 +13,8 @@ describe AnalyticsBuildEntity do subject { entity.as_json } - before do - Timecop.freeze - end - - after do - Timecop.return + around do |example| + Timecop.freeze { example.run } end it 'contains the URL' do diff --git a/spec/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb index 730df4e0336..53d4fcfed18 100644 --- a/spec/services/ci/create_pipeline_service_spec.rb +++ b/spec/services/ci/create_pipeline_service_spec.rb @@ -66,7 +66,7 @@ describe Ci::CreatePipelineService do context 'when there is no pipeline for source branch' do it "does not update merge request head pipeline" do - merge_request = create(:merge_request, source_branch: 'other_branch', target_branch: "branch_1", source_project: project) + merge_request = create(:merge_request, source_branch: 'feature', target_branch: "branch_1", source_project: project) head_pipeline = pipeline diff --git a/spec/services/create_deployment_service_spec.rb b/spec/services/create_deployment_service_spec.rb index 049b082277a..08267d6e6a0 100644 --- a/spec/services/create_deployment_service_spec.rb +++ b/spec/services/create_deployment_service_spec.rb @@ -20,6 +20,10 @@ describe CreateDeploymentService do let(:service) { described_class.new(job) } + before do + allow_any_instance_of(Deployment).to receive(:create_ref) + end + describe '#execute' do subject { service.execute } diff --git a/spec/services/event_create_service_spec.rb b/spec/services/event_create_service_spec.rb index 42adb044190..02d7ddeb86b 100644 --- a/spec/services/event_create_service_spec.rb +++ b/spec/services/event_create_service_spec.rb @@ -117,12 +117,52 @@ describe EventCreateService do let(:project) { create(:project) } let(:user) { create(:user) } + let(:push_data) do + { + commits: [ + { + id: '1cf19a015df3523caf0a1f9d40c98a267d6a2fc2', + message: 'This is a commit' + } + ], + before: '0000000000000000000000000000000000000000', + after: '1cf19a015df3523caf0a1f9d40c98a267d6a2fc2', + total_commits_count: 1, + ref: 'refs/heads/my-branch' + } + end + it 'creates a new event' do - expect { service.push(project, user, {}) }.to change { Event.count } + expect { service.push(project, user, push_data) }.to change { Event.count } + end + + it 'creates the push event payload' do + expect(PushEventPayloadService).to receive(:new) + .with(an_instance_of(PushEvent), push_data) + .and_call_original + + service.push(project, user, push_data) end it 'updates user last activity' do - expect { service.push(project, user, {}) }.to change { user_activity(user) } + expect { service.push(project, user, push_data) } + .to change { user_activity(user) } + end + + it 'does not create any event data when an error is raised' do + payload_service = double(:service) + + allow(payload_service).to receive(:execute) + .and_raise(RuntimeError) + + allow(PushEventPayloadService).to receive(:new) + .and_return(payload_service) + + expect { service.push(project, user, push_data) } + .to raise_error(RuntimeError) + + expect(Event.count).to eq(0) + expect(PushEventPayload.count).to eq(0) end end diff --git a/spec/services/git_push_service_spec.rb b/spec/services/git_push_service_spec.rb index a6449a3c9f5..8485605b398 100644 --- a/spec/services/git_push_service_spec.rb +++ b/spec/services/git_push_service_spec.rb @@ -141,10 +141,13 @@ describe GitPushService, services: true do let!(:push_data) { push_data_from_service(project, user, oldrev, newrev, ref) } let(:event) { Event.find_by_action(Event::PUSHED) } - it { expect(event).not_to be_nil } + it { expect(event).to be_an_instance_of(PushEvent) } it { expect(event.project).to eq(project) } it { expect(event.action).to eq(Event::PUSHED) } - it { expect(event.data).to eq(push_data) } + it { expect(event.push_event_payload).to be_an_instance_of(PushEventPayload) } + it { expect(event.push_event_payload.commit_from).to eq(oldrev) } + it { expect(event.push_event_payload.commit_to).to eq(newrev) } + it { expect(event.push_event_payload.ref).to eq('master') } context "Updates merge requests" do it "when pushing a new branch for the first time" do diff --git a/spec/services/issues/resolve_discussions_spec.rb b/spec/services/issues/resolve_discussions_spec.rb index fac66791ffb..67a86c50fc1 100644 --- a/spec/services/issues/resolve_discussions_spec.rb +++ b/spec/services/issues/resolve_discussions_spec.rb @@ -20,7 +20,7 @@ describe Issues::ResolveDiscussions do describe "for resolving discussions" do let(:discussion) { create(:diff_note_on_merge_request, project: project, note: "Almost done").to_discussion } let(:merge_request) { discussion.noteable } - let(:other_merge_request) { create(:merge_request, source_project: project, source_branch: "other") } + let(:other_merge_request) { create(:merge_request, source_project: project, source_branch: "fix") } describe "#merge_request_for_resolving_discussion" do let(:service) { DummyService.new(project, user, merge_request_to_resolve_discussions_of: merge_request.iid) } 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/merge_requests/get_urls_service_spec.rb b/spec/services/merge_requests/get_urls_service_spec.rb index 672d86e4028..25599dea19f 100644 --- a/spec/services/merge_requests/get_urls_service_spec.rb +++ b/spec/services/merge_requests/get_urls_service_spec.rb @@ -3,7 +3,7 @@ require "spec_helper" describe MergeRequests::GetUrlsService do let(:project) { create(:project, :public, :repository) } let(:service) { described_class.new(project) } - let(:source_branch) { "my_branch" } + let(:source_branch) { "merge-test" } let(:new_merge_request_url) { "http://#{Gitlab.config.gitlab.host}/#{project.namespace.name}/#{project.path}/merge_requests/new?merge_request%5Bsource_branch%5D=#{source_branch}" } let(:show_merge_request_url) { "http://#{Gitlab.config.gitlab.host}/#{project.namespace.name}/#{project.path}/merge_requests/#{merge_request.iid}" } let(:new_branch_changes) { "#{Gitlab::Git::BLANK_SHA} 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/#{source_branch}" } @@ -111,9 +111,9 @@ describe MergeRequests::GetUrlsService do end context 'pushing new branch and existing branch (with merge request created) at once' do - let!(:merge_request) { create(:merge_request, source_project: project, source_branch: "existing_branch") } + let!(:merge_request) { create(:merge_request, source_project: project, source_branch: "markdown") } let(:new_branch_changes) { "#{Gitlab::Git::BLANK_SHA} 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/new_branch" } - let(:existing_branch_changes) { "d14d6c0abdd253381df51a723d58691b2ee1ab08 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/existing_branch" } + let(:existing_branch_changes) { "d14d6c0abdd253381df51a723d58691b2ee1ab08 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/markdown" } let(:changes) { "#{new_branch_changes}\n#{existing_branch_changes}" } let(:new_merge_request_url) { "http://#{Gitlab.config.gitlab.host}/#{project.namespace.name}/#{project.path}/merge_requests/new?merge_request%5Bsource_branch%5D=new_branch" } @@ -124,7 +124,7 @@ describe MergeRequests::GetUrlsService do url: new_merge_request_url, new_merge_request: true }, { - branch_name: "existing_branch", + branch_name: "markdown", url: show_merge_request_url, new_merge_request: false }]) diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index bd8ff5aaaa7..44b2d28d1d4 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 @@ -80,12 +80,16 @@ describe NotificationService, :mailer do describe 'Keys' do describe '#new_key' do - let!(:key) { create(:personal_key) } + let(:key_options) { {} } + let!(:key) { create(:personal_key, key_options) } it { expect(notification.new_key(key)).to be_truthy } + it { should_email(key.user) } - it 'sends email to key owner' do - expect { notification.new_key(key) }.to change { ActionMailer::Base.deliveries.size }.by(1) + describe 'never emails the ghost user' do + let(:key_options) { { user: User.ghost } } + + it { should_not_email_anyone } end end end @@ -1173,19 +1177,39 @@ describe NotificationService, :mailer do end end - describe '#project_exported' do - it do - notification.project_exported(project, @u_disabled) + context 'user with notifications disabled' do + describe '#project_exported' do + it do + notification.project_exported(project, @u_disabled) + + should_not_email_anyone + end + end + + describe '#project_not_exported' do + it do + notification.project_not_exported(project, @u_disabled, ['error']) - should_only_email(@u_disabled) + should_not_email_anyone + end end end - describe '#project_not_exported' do - it do - notification.project_not_exported(project, @u_disabled, ['error']) + context 'user with notifications enabled' do + describe '#project_exported' do + it do + notification.project_exported(project, @u_participating) - should_only_email(@u_disabled) + should_only_email(@u_participating) + end + end + + describe '#project_not_exported' do + it do + notification.project_not_exported(project, @u_participating, ['error']) + + should_only_email(@u_participating) + end end end end @@ -1196,7 +1220,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 @@ -1209,6 +1233,35 @@ describe NotificationService, :mailer do end.to change { ActionMailer::Base.deliveries.size }.by(1) end end + + describe '#new_group_member' do + let(:group) { create(:group) } + let(:added_user) { create(:user) } + + def create_member! + GroupMember.create( + group: group, + user: added_user, + access_level: Gitlab::Access::GUEST + ) + end + + it 'sends a notification' do + create_member! + should_only_email(added_user) + end + + describe 'when notifications are disabled' do + before do + create_global_setting_for(added_user, :disabled) + end + + it 'does not send a notification' do + create_member! + should_not_email_anyone + end + end + end end describe 'ProjectMember' do @@ -1216,7 +1269,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 @@ -1228,6 +1281,31 @@ describe NotificationService, :mailer do end.to change { ActionMailer::Base.deliveries.size }.by(1) end end + + describe '#new_project_member' do + let(:project) { create(:project) } + let(:added_user) { create(:user) } + + def create_member! + create(:project_member, user: added_user, project: project) + end + + it do + create_member! + should_only_email(added_user) + end + + describe 'when notifications are disabled' do + before do + create_global_setting_for(added_user, :disabled) + end + + it do + create_member! + should_not_email_anyone + end + end + end end context 'guest user in private project' do 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/push_event_payload_service_spec.rb b/spec/services/push_event_payload_service_spec.rb new file mode 100644 index 00000000000..81956200bff --- /dev/null +++ b/spec/services/push_event_payload_service_spec.rb @@ -0,0 +1,218 @@ +require 'spec_helper' + +describe PushEventPayloadService do + let(:event) { create(:push_event) } + + describe '#execute' do + let(:push_data) do + { + commits: [ + { + id: '1cf19a015df3523caf0a1f9d40c98a267d6a2fc2', + message: 'This is a commit' + } + ], + before: '0000000000000000000000000000000000000000', + after: '1cf19a015df3523caf0a1f9d40c98a267d6a2fc2', + total_commits_count: 1, + ref: 'refs/heads/my-branch' + } + end + + it 'creates a new PushEventPayload row' do + payload = described_class.new(event, push_data).execute + + expect(payload.commit_count).to eq(1) + expect(payload.action).to eq('created') + expect(payload.ref_type).to eq('branch') + expect(payload.commit_from).to be_nil + expect(payload.commit_to).to eq(push_data[:after]) + expect(payload.ref).to eq('my-branch') + expect(payload.commit_title).to eq('This is a commit') + expect(payload.event_id).to eq(event.id) + end + + it 'sets the push_event_payload association of the used event' do + payload = described_class.new(event, push_data).execute + + expect(event.push_event_payload).to eq(payload) + end + end + + describe '#commit_title' do + it 'returns nil if no commits were pushed' do + service = described_class.new(event, commits: []) + + expect(service.commit_title).to be_nil + end + + it 'returns a String limited to 70 characters' do + service = described_class.new(event, commits: [{ message: 'a' * 100 }]) + + expect(service.commit_title).to eq(('a' * 67) + '...') + end + + it 'does not truncate the commit message if it is shorter than 70 characters' do + service = described_class.new(event, commits: [{ message: 'Hello' }]) + + expect(service.commit_title).to eq('Hello') + end + + it 'includes the first line of a commit message if the message spans multiple lines' do + service = described_class + .new(event, commits: [{ message: "Hello\n\nworld" }]) + + expect(service.commit_title).to eq('Hello') + end + end + + describe '#commit_from_id' do + it 'returns nil when creating a new ref' do + service = described_class.new(event, before: Gitlab::Git::BLANK_SHA) + + expect(service.commit_from_id).to be_nil + end + + it 'returns the ID of the first commit when pushing to an existing ref' do + service = described_class.new(event, before: '123') + + expect(service.commit_from_id).to eq('123') + end + end + + describe '#commit_to_id' do + it 'returns nil when removing an existing ref' do + service = described_class.new(event, after: Gitlab::Git::BLANK_SHA) + + expect(service.commit_to_id).to be_nil + end + end + + describe '#commit_count' do + it 'returns the number of commits' do + service = described_class.new(event, total_commits_count: 1) + + expect(service.commit_count).to eq(1) + end + + it 'raises when the push data does not contain the commits count' do + service = described_class.new(event, {}) + + expect { service.commit_count }.to raise_error(KeyError) + end + end + + describe '#ref' do + it 'returns the name of the ref' do + service = described_class.new(event, ref: 'refs/heads/foo') + + expect(service.ref).to eq('refs/heads/foo') + end + + it 'raises when the push data does not contain the ref name' do + service = described_class.new(event, {}) + + expect { service.ref }.to raise_error(KeyError) + end + end + + describe '#revision_before' do + it 'returns the revision from before the push' do + service = described_class.new(event, before: 'foo') + + expect(service.revision_before).to eq('foo') + end + + it 'raises when the push data does not contain the before revision' do + service = described_class.new(event, {}) + + expect { service.revision_before }.to raise_error(KeyError) + end + end + + describe '#revision_after' do + it 'returns the revision from after the push' do + service = described_class.new(event, after: 'foo') + + expect(service.revision_after).to eq('foo') + end + + it 'raises when the push data does not contain the after revision' do + service = described_class.new(event, {}) + + expect { service.revision_after }.to raise_error(KeyError) + end + end + + describe '#trimmed_ref' do + it 'returns the ref name without its prefix' do + service = described_class.new(event, ref: 'refs/heads/foo') + + expect(service.trimmed_ref).to eq('foo') + end + end + + describe '#create?' do + it 'returns true when creating a new ref' do + service = described_class.new(event, before: Gitlab::Git::BLANK_SHA) + + expect(service.create?).to eq(true) + end + + it 'returns false when pushing to an existing ref' do + service = described_class.new(event, before: 'foo') + + expect(service.create?).to eq(false) + end + end + + describe '#remove?' do + it 'returns true when removing an existing ref' do + service = described_class.new(event, after: Gitlab::Git::BLANK_SHA) + + expect(service.remove?).to eq(true) + end + + it 'returns false pushing to an existing ref' do + service = described_class.new(event, after: 'foo') + + expect(service.remove?).to eq(false) + end + end + + describe '#action' do + it 'returns :created when creating a ref' do + service = described_class.new(event, before: Gitlab::Git::BLANK_SHA) + + expect(service.action).to eq(:created) + end + + it 'returns :removed when removing an existing ref' do + service = described_class.new(event, + before: '123', + after: Gitlab::Git::BLANK_SHA) + + expect(service.action).to eq(:removed) + end + + it 'returns :pushed when pushing to an existing ref' do + service = described_class.new(event, before: '123', after: '456') + + expect(service.action).to eq(:pushed) + end + end + + describe '#ref_type' do + it 'returns :tag for a tag' do + service = described_class.new(event, ref: 'refs/tags/1.2') + + expect(service.ref_type).to eq(:tag) + end + + it 'returns :branch for a branch' do + service = described_class.new(event, ref: 'refs/heads/master') + + expect(service.ref_type).to eq(:branch) + end + end +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/simplecov_env.rb b/spec/simplecov_env.rb index ac2c89b3ff9..25ddf932d42 100644 --- a/spec/simplecov_env.rb +++ b/spec/simplecov_env.rb @@ -36,18 +36,25 @@ module SimpleCovEnv track_files '{app,lib}/**/*.rb' add_filter '/vendor/ruby/' + add_filter 'app/controllers/sherlock/' add_filter 'config/initializers/' + add_filter 'db/fixtures/' + add_filter 'lib/gitlab/sidekiq_middleware/' + add_filter 'lib/system_check/' add_group 'Controllers', 'app/controllers' - add_group 'Models', 'app/models' - add_group 'Mailers', 'app/mailers' - add_group 'Helpers', 'app/helpers' - add_group 'Workers', %w(app/jobs app/workers) - add_group 'Libraries', 'lib' - add_group 'Services', 'app/services' - add_group 'Finders', 'app/finders' - add_group 'Uploaders', 'app/uploaders' - add_group 'Validators', 'app/validators' + add_group 'Finders', 'app/finders' + add_group 'Helpers', 'app/helpers' + add_group 'Libraries', 'lib' + add_group 'Mailers', 'app/mailers' + add_group 'Models', 'app/models' + add_group 'Policies', 'app/policies' + add_group 'Presenters', 'app/presenters' + add_group 'Serializers', 'app/serializers' + add_group 'Services', 'app/services' + add_group 'Uploaders', 'app/uploaders' + add_group 'Validators', 'app/validators' + add_group 'Workers', %w(app/jobs app/workers) merge_timeout 365.days end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 55691f6716f..3eea39d4bf4 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -70,7 +70,14 @@ RSpec.configure do |config| config.raise_errors_for_deprecations! + if ENV['CI'] + # This includes the first try, i.e. tests will be run 4 times before failing. + config.default_retry_count = 4 + config.reporter.register_listener(RspecFlaky::Listener.new, :example_passed, :dump_summary) + end + config.before(:suite) do + Timecop.safe_mode = true TestEnv.init end @@ -98,12 +105,6 @@ RSpec.configure do |config| reset_delivered_emails! end - if ENV['CI'] - config.around(:each) do |ex| - ex.run_with_retry retry: 2 - end - end - config.around(:each, :use_clean_rails_memory_store_caching) do |example| caching_store = Rails.cache Rails.cache = ActiveSupport::Cache::MemoryStore.new diff --git a/spec/support/features/reportable_note_shared_examples.rb b/spec/support/features/reportable_note_shared_examples.rb index 27e079c01dd..cb483ae9a5a 100644 --- a/spec/support/features/reportable_note_shared_examples.rb +++ b/spec/support/features/reportable_note_shared_examples.rb @@ -7,15 +7,18 @@ shared_examples 'reportable note' do let(:more_actions_selector) { '.more-actions.dropdown' } let(:abuse_report_path) { new_abuse_report_path(user_id: note.author.id, ref_url: noteable_note_url(note)) } + it 'has an edit button' do + expect(comment).to have_selector('.js-note-edit') + end + it 'has a `More actions` dropdown' do expect(comment).to have_selector(more_actions_selector) end - it 'dropdown has Edit, Report and Delete links' do + it 'dropdown has Report and Delete links' do dropdown = comment.find(more_actions_selector) open_dropdown(dropdown) - expect(dropdown).to have_button('Edit comment') expect(dropdown).to have_link('Report as abuse', href: abuse_report_path) expect(dropdown).to have_link('Delete comment', href: note_url(note, project)) end diff --git a/spec/support/issuables_list_metadata_shared_examples.rb b/spec/support/issuables_list_metadata_shared_examples.rb index a60d3b0d22d..75982432ab4 100644 --- a/spec/support/issuables_list_metadata_shared_examples.rb +++ b/spec/support/issuables_list_metadata_shared_examples.rb @@ -2,12 +2,12 @@ shared_examples 'issuables list meta-data' do |issuable_type, action = nil| before do @issuable_ids = [] - 2.times do |n| + %w[fix improve/awesome].each do |source_branch| issuable = if issuable_type == :issue create(issuable_type, project: project) else - create(issuable_type, source_project: project, source_branch: "#{n}-feature") + create(issuable_type, source_project: project, source_branch: source_branch) end @issuable_ids << issuable.id 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) diff --git a/spec/tasks/gitlab/gitaly_rake_spec.rb b/spec/tasks/gitlab/gitaly_rake_spec.rb index 43ac1a72152..b29d63c7d67 100644 --- a/spec/tasks/gitlab/gitaly_rake_spec.rb +++ b/spec/tasks/gitlab/gitaly_rake_spec.rb @@ -54,17 +54,17 @@ describe 'gitlab:gitaly namespace rake task' do before do FileUtils.mkdir_p(clone_path) expect(Dir).to receive(:chdir).with(clone_path).and_call_original + allow(Bundler).to receive(:bundle_path).and_return('/fake/bundle_path') end context 'gmake is available' do before do expect(main_object).to receive(:checkout_or_clone_version) - allow(main_object).to receive(:run_command!).with(command_preamble + ['gmake']).and_return(true) end it 'calls gmake in the gitaly directory' do expect(Gitlab::Popen).to receive(:popen).with(%w[which gmake]).and_return(['/usr/bin/gmake', 0]) - expect(main_object).to receive(:run_command!).with(command_preamble + ['gmake']).and_return(true) + expect(main_object).to receive(:run_command!).with(command_preamble + %w[gmake BUNDLE_PATH=/fake/bundle_path]).and_return(true) run_rake_task('gitlab:gitaly:install', clone_path) end @@ -73,15 +73,26 @@ describe 'gitlab:gitaly namespace rake task' do context 'gmake is not available' do before do expect(main_object).to receive(:checkout_or_clone_version) - allow(main_object).to receive(:run_command!).with(command_preamble + ['make']).and_return(true) + expect(Gitlab::Popen).to receive(:popen).with(%w[which gmake]).and_return(['', 42]) end it 'calls make in the gitaly directory' do - expect(Gitlab::Popen).to receive(:popen).with(%w[which gmake]).and_return(['', 42]) - expect(main_object).to receive(:run_command!).with(command_preamble + ['make']).and_return(true) + expect(main_object).to receive(:run_command!).with(command_preamble + %w[make BUNDLE_PATH=/fake/bundle_path]).and_return(true) run_rake_task('gitlab:gitaly:install', clone_path) end + + context 'when Rails.env is not "test"' do + before do + allow(Rails.env).to receive(:test?).and_return(false) + end + + it 'calls make in the gitaly directory without BUNDLE_PATH' do + expect(main_object).to receive(:run_command!).with(command_preamble + ['make']).and_return(true) + + run_rake_task('gitlab:gitaly:install', clone_path) + end + end end end end diff --git a/spec/uploaders/file_mover_spec.rb b/spec/uploaders/file_mover_spec.rb index d7c1b390f9a..0cf462e9553 100644 --- a/spec/uploaders/file_mover_spec.rb +++ b/spec/uploaders/file_mover_spec.rb @@ -4,11 +4,11 @@ describe FileMover do let(:filename) { 'banana_sample.gif' } let(:file) { fixture_file_upload(Rails.root.join('spec', 'fixtures', filename)) } let(:temp_description) do - 'test ![banana_sample](/uploads/system/temp/secret55/banana_sample.gif) same ![banana_sample]'\ - '(/uploads/system/temp/secret55/banana_sample.gif)' + 'test ![banana_sample](/uploads/-/system/temp/secret55/banana_sample.gif) same ![banana_sample]'\ + '(/uploads/-/system/temp/secret55/banana_sample.gif)' end let(:temp_file_path) { File.join('secret55', filename).to_s } - let(:file_path) { File.join('uploads', 'system', 'personal_snippet', snippet.id.to_s, 'secret55', filename).to_s } + let(:file_path) { File.join('uploads', '-', 'system', 'personal_snippet', snippet.id.to_s, 'secret55', filename).to_s } let(:snippet) { create(:personal_snippet, description: temp_description) } @@ -28,8 +28,8 @@ describe FileMover do expect(snippet.reload.description) .to eq( - "test ![banana_sample](/uploads/system/personal_snippet/#{snippet.id}/secret55/banana_sample.gif)"\ - " same ![banana_sample](/uploads/system/personal_snippet/#{snippet.id}/secret55/banana_sample.gif)" + "test ![banana_sample](/uploads/-/system/personal_snippet/#{snippet.id}/secret55/banana_sample.gif)"\ + " same ![banana_sample](/uploads/-/system/personal_snippet/#{snippet.id}/secret55/banana_sample.gif)" ) end @@ -50,8 +50,8 @@ describe FileMover do expect(snippet.reload.description) .to eq( - "test ![banana_sample](/uploads/system/temp/secret55/banana_sample.gif)"\ - " same ![banana_sample](/uploads/system/temp/secret55/banana_sample.gif)" + "test ![banana_sample](/uploads/-/system/temp/secret55/banana_sample.gif)"\ + " same ![banana_sample](/uploads/-/system/temp/secret55/banana_sample.gif)" ) end diff --git a/spec/uploaders/personal_file_uploader_spec.rb b/spec/uploaders/personal_file_uploader_spec.rb index e505edc75ce..cbafa9f478d 100644 --- a/spec/uploaders/personal_file_uploader_spec.rb +++ b/spec/uploaders/personal_file_uploader_spec.rb @@ -10,7 +10,7 @@ describe PersonalFileUploader do dynamic_segment = "personal_snippet/#{snippet.id}" - expect(described_class.absolute_path(upload)).to end_with("/system/#{dynamic_segment}/secret/foo.jpg") + expect(described_class.absolute_path(upload)).to end_with("/-/system/#{dynamic_segment}/secret/foo.jpg") end end @@ -19,7 +19,7 @@ describe PersonalFileUploader do uploader = described_class.new(snippet, 'secret') allow(uploader).to receive(:file).and_return(double(extension: 'txt', filename: 'file_name')) - expected_url = "/uploads/system/personal_snippet/#{snippet.id}/secret/file_name" + expected_url = "/uploads/-/system/personal_snippet/#{snippet.id}/secret/file_name" expect(uploader.to_h).to eq( alt: 'file_name', diff --git a/spec/views/projects/edit.html.haml_spec.rb b/spec/views/projects/edit.html.haml_spec.rb index 94899e26292..1af422941d7 100644 --- a/spec/views/projects/edit.html.haml_spec.rb +++ b/spec/views/projects/edit.html.haml_spec.rb @@ -11,14 +11,26 @@ describe 'projects/edit' do allow(controller).to receive(:current_user).and_return(user) allow(view).to receive_messages(current_user: user, can?: true) - allow(Gitlab.config.lfs).to receive(:enabled).and_return(true) end context 'LFS enabled setting' do it 'displays the correct elements' do + allow(Gitlab.config.lfs).to receive(:enabled).and_return(true) + render + expect(rendered).to have_select('project_lfs_enabled') expect(rendered).to have_content('Git Large File Storage') end end + + context 'project export disabled' do + it 'does not display the project export option' do + stub_application_setting(project_export_enabled?: false) + + render + + expect(rendered).not_to have_content('Export project') + end + end end diff --git a/spec/views/projects/notes/_more_actions_dropdown.html.haml_spec.rb b/spec/views/projects/notes/_more_actions_dropdown.html.haml_spec.rb index aea20d826d0..9c0be249a50 100644 --- a/spec/views/projects/notes/_more_actions_dropdown.html.haml_spec.rb +++ b/spec/views/projects/notes/_more_actions_dropdown.html.haml_spec.rb @@ -24,18 +24,16 @@ describe 'projects/notes/_more_actions_dropdown' do expect(rendered).not_to have_selector('.dropdown.more-actions') end - it 'shows Report as abuse, Edit and Delete buttons if editable and not current users comment' do + it 'shows Report as abuse and Delete buttons if editable and not current users comment' do render 'projects/notes/more_actions_dropdown', current_user: not_author_user, note_editable: true, note: note expect(rendered).to have_link('Report as abuse') - expect(rendered).to have_button('Edit comment') expect(rendered).to have_link('Delete comment') end - it 'shows Edit and Delete buttons if editable and current users comment' do + it 'shows Delete button if editable and current users comment' do render 'projects/notes/more_actions_dropdown', current_user: author_user, note_editable: true, note: note - expect(rendered).to have_button('Edit comment') expect(rendered).to have_link('Delete comment') end end diff --git a/spec/workers/prune_old_events_worker_spec.rb b/spec/workers/prune_old_events_worker_spec.rb index 35e1518a35e..ea974355050 100644 --- a/spec/workers/prune_old_events_worker_spec.rb +++ b/spec/workers/prune_old_events_worker_spec.rb @@ -2,9 +2,11 @@ require 'spec_helper' describe PruneOldEventsWorker do describe '#perform' do - let!(:expired_event) { create(:event, author_id: 0, created_at: 13.months.ago) } - let!(:not_expired_event) { create(:event, author_id: 0, created_at: 1.day.ago) } - let!(:exactly_12_months_event) { create(:event, author_id: 0, created_at: 12.months.ago) } + let(:user) { create(:user) } + + let!(:expired_event) { create(:event, :closed, author: user, created_at: 13.months.ago) } + let!(:not_expired_event) { create(:event, :closed, author: user, created_at: 1.day.ago) } + let!(:exactly_12_months_event) { create(:event, :closed, author: user, created_at: 12.months.ago) } it 'prunes events older than 12 months' do expect { subject.perform }.to change { Event.count }.by(-1) |