diff options
Diffstat (limited to 'spec')
125 files changed, 1871 insertions, 754 deletions
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/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/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:  and "\ "text: [text.txt](/uploads#{text_file})" @@ -208,8 +208,8 @@ describe SnippetsController do snippet = subject expected_description = "Description with picture: "\ - " and "\ - "text: [text.txt](/uploads/system/personal_snippet/#{snippet.id}/secret78/text.txt)" + " 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/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/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/features/admin/admin_settings_spec.rb b/spec/features/admin/admin_settings_spec.rb index c9591a7d854..dbb0ae9c86e 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 @@ -72,7 +74,7 @@ feature 'Admin updates settings' do context 'sign-in restrictions', :js do it 'de-activates oauth sign-in source' do find('.btn', text: 'GitLab.com').click - + expect(find('.btn', text: 'GitLab.com')).not_to have_css('.active') end 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/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/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/bulk_assignment_labels_spec.rb b/spec/features/issues/bulk_assignment_labels_spec.rb index 847e3856ba5..b2229b44f99 100644 --- a/spec/features/issues/bulk_assignment_labels_spec.rb +++ b/spec/features/issues/bulk_assignment_labels_spec.rb @@ -353,7 +353,7 @@ feature 'Issues > Labels bulk assignment' do context 'cannot bulk assign labels' do it do - expect(page).not_to have_button 'Edit Issues' + expect(page).not_to have_button 'Edit issues' expect(page).not_to have_css '.check-all-issues' expect(page).not_to have_css '.issue-check' end @@ -411,7 +411,7 @@ feature 'Issues > Labels bulk assignment' do def enable_bulk_update visit project_issues_path(project) - click_button 'Edit Issues' + click_button 'Edit issues' end def disable_bulk_update 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/update_issues_spec.rb b/spec/features/issues/update_issues_spec.rb index 5a7c4f54cb6..bcc6e9bab0f 100644 --- a/spec/features/issues/update_issues_spec.rb +++ b/spec/features/issues/update_issues_spec.rb @@ -14,7 +14,7 @@ feature 'Multiple issue updating from issues#index', :js do it 'sets to closed' do visit project_issues_path(project) - click_button 'Edit Issues' + click_button 'Edit issues' find('#check-all-issues').click find('.js-issue-status').click @@ -27,7 +27,7 @@ feature 'Multiple issue updating from issues#index', :js do create_closed visit project_issues_path(project, state: 'closed') - click_button 'Edit Issues' + click_button 'Edit issues' find('#check-all-issues').click find('.js-issue-status').click @@ -41,7 +41,7 @@ feature 'Multiple issue updating from issues#index', :js do it 'updates to current user' do visit project_issues_path(project) - click_button 'Edit Issues' + click_button 'Edit issues' find('#check-all-issues').click click_update_assignee_button @@ -57,7 +57,7 @@ feature 'Multiple issue updating from issues#index', :js do create_assigned visit project_issues_path(project) - click_button 'Edit Issues' + click_button 'Edit issues' find('#check-all-issues').click click_update_assignee_button @@ -73,7 +73,7 @@ feature 'Multiple issue updating from issues#index', :js do it 'updates milestone' do visit project_issues_path(project) - click_button 'Edit Issues' + click_button 'Edit issues' find('#check-all-issues').click find('.issues-bulk-update .js-milestone-select').click @@ -89,7 +89,7 @@ feature 'Multiple issue updating from issues#index', :js do expect(first('.issue')).to have_content milestone.title - click_button 'Edit Issues' + click_button 'Edit issues' find('#check-all-issues').click find('.issues-bulk-update .js-milestone-select').click diff --git a/spec/features/issues_spec.rb b/spec/features/issues_spec.rb index a5bb642221c..3ffc80622f5 100644 --- a/spec/features/issues_spec.rb +++ b/spec/features/issues_spec.rb @@ -708,7 +708,7 @@ describe 'Issues' do end describe 'confidential issue#show', js: true do - it 'shows confidential sibebar information as confidential and can be turned off' do + it 'shows confidential sibebar information as confidential and can be turned off' do issue = create(:issue, :confidential, project: project) visit project_issue_path(project, issue) @@ -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/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/update_merge_requests_spec.rb b/spec/features/merge_requests/update_merge_requests_spec.rb index cf30a687df8..e6dc284cba7 100644 --- a/spec/features/merge_requests/update_merge_requests_spec.rb +++ b/spec/features/merge_requests/update_merge_requests_spec.rb @@ -98,7 +98,7 @@ feature 'Multiple merge requests updating from merge_requests#index' do end def change_status(text) - click_button 'Edit Merge Requests' + click_button 'Edit merge requests' find('#check-all-issues').click find('.js-issue-status').click find('.dropdown-menu-status a', text: text).click @@ -106,7 +106,7 @@ feature 'Multiple merge requests updating from merge_requests#index' do end def change_assignee(text) - click_button 'Edit Merge Requests' + click_button 'Edit merge requests' find('#check-all-issues').click find('.js-update-assignee').click wait_for_requests @@ -119,7 +119,7 @@ feature 'Multiple merge requests updating from merge_requests#index' do end def change_milestone(text) - click_button 'Edit Merge Requests' + click_button 'Edit merge requests' find('#check-all-issues').click find('.issues-bulk-update .js-milestone-select').click find('.dropdown-menu-milestone a', text: text).click 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_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/milestones/show_spec.rb b/spec/features/milestones/show_spec.rb index 20303359c46..624f13922ed 100644 --- a/spec/features/milestones/show_spec.rb +++ b/spec/features/milestones/show_spec.rb @@ -8,7 +8,7 @@ describe 'Milestone show' do let(:issue_params) { { project: project, assignees: [user], author: user, milestone: milestone, labels: labels } } before do - project.add_user(user, :developer) + project.add_user(user, :developer) sign_in(user) end diff --git a/spec/features/projects/files/undo_template_spec.rb b/spec/features/projects/files/undo_template_spec.rb index 4238d25e9ee..9bcd5beabb8 100644 --- a/spec/features/projects/files/undo_template_spec.rb +++ b/spec/features/projects/files/undo_template_spec.rb @@ -20,7 +20,7 @@ feature 'Template Undo Button', js: true do end end - context 'creating a non-matching file' do + context 'creating a non-matching file' do before do visit project_new_blob_path(project, 'master') select_file_template_type('LICENSE') diff --git a/spec/features/projects/user_edits_files_spec.rb b/spec/features/projects/user_edits_files_spec.rb index 8c9fc8821e6..3129aad8473 100644 --- a/spec/features/projects/user_edits_files_spec.rb +++ b/spec/features/projects/user_edits_files_spec.rb @@ -20,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') @@ -35,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') @@ -49,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) @@ -65,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') @@ -92,6 +104,8 @@ describe 'User edits files' do "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')") expect(evaluate_script('ace.edit("editor").getValue()')).to eq('*.rbca') @@ -105,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/protected_tags_spec.rb b/spec/features/protected_tags_spec.rb index c9ba1a8c088..8abd4403065 100644 --- a/spec/features/protected_tags_spec.rb +++ b/spec/features/protected_tags_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -feature 'Projected Tags', js: true do +feature 'Protected Tags', js: true do let(:user) { create(:user, :admin) } let(:project) { create(:project, :repository) } 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/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/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/helpers/projects_helper_spec.rb b/spec/helpers/projects_helper_spec.rb index 37a5e6b474e..d1efa318d14 100644 --- a/spec/helpers/projects_helper_spec.rb +++ b/spec/helpers/projects_helper_spec.rb @@ -432,9 +432,7 @@ describe ProjectsHelper do end describe '#any_projects?' do - before do - create(:project) - end + let!(:project) { create(:project) } it 'returns true when projects will be returned' do expect(helper.any_projects?(Project.all)).to eq(true) @@ -444,6 +442,14 @@ describe ProjectsHelper do expect(helper.any_projects?(Project.none)).to eq(false) end + it 'returns true when using a non-empty Array' do + expect(helper.any_projects?([project])).to eq(true) + end + + it 'returns false when using an empty Array' do + expect(helper.any_projects?([])).to eq(false) + end + it 'only executes a single query when a LIMIT is applied' do relation = Project.limit(1) recorder = ActiveRecord::QueryRecorder.new do diff --git a/spec/javascripts/fixtures/prometheus_service.rb b/spec/javascripts/fixtures/prometheus_service.rb index 7a46e47bb15..7968c9425f2 100644 --- a/spec/javascripts/fixtures/prometheus_service.rb +++ b/spec/javascripts/fixtures/prometheus_service.rb @@ -7,7 +7,7 @@ describe Projects::ServicesController, '(JavaScript fixtures)', type: :controlle let(:namespace) { create(:namespace, name: 'frontend-fixtures' )} let(:project) { create(:project_empty_repo, namespace: namespace, path: 'services-project') } let!(:service) { create(:prometheus_service, project: project) } - + render_views before(:all) do diff --git a/spec/javascripts/fixtures/services.rb b/spec/javascripts/fixtures/services.rb index 0a3c64d5d31..80915c32a74 100644 --- a/spec/javascripts/fixtures/services.rb +++ b/spec/javascripts/fixtures/services.rb @@ -7,7 +7,6 @@ describe Projects::ServicesController, '(JavaScript fixtures)', type: :controlle let(:namespace) { create(:namespace, name: 'frontend-fixtures' )} let(:project) { create(:project_empty_repo, namespace: namespace, path: 'services-project') } let!(:service) { create(:custom_issue_tracker_service, project: project, title: 'Custom Issue Tracker') } - render_views diff --git a/spec/javascripts/fly_out_nav_spec.js b/spec/javascripts/fly_out_nav_spec.js index e44d874ad2b..2e81a1b056b 100644 --- a/spec/javascripts/fly_out_nav_spec.js +++ b/spec/javascripts/fly_out_nav_spec.js @@ -1,10 +1,16 @@ import Cookies from 'js-cookie'; import { calculateTop, - hideSubLevelItems, showSubLevelItems, canShowSubItems, canShowActiveSubItems, + mouseEnterTopItems, + mouseLeaveTopItem, + setOpenMenu, + mousePos, + getHideSubItemsInterval, + documentMouseMove, + getHeaderHeight, } from '~/fly_out_nav'; import bp from '~/breakpoints'; @@ -18,11 +24,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 +58,153 @@ 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: 150px;"></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, + }); + console.log(el); + + 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 +233,7 @@ describe('Fly out sidebar navigation', () => { ).not.toBe('block'); }); - it('does not shows sub-items', () => { + it('shows sub-items', () => { showSubLevelItems(el); expect( @@ -146,7 +247,7 @@ describe('Fly out sidebar navigation', () => { expect( subItems.style.transform, - ).toBe(`translate3d(0px, ${Math.floor(el.getBoundingClientRect().top)}px, 0px)`); + ).toBe(`translate3d(0px, ${Math.floor(el.getBoundingClientRect().top) - getHeaderHeight()}px, 0px)`); }); it('sets is-above when element is above', () => { diff --git a/spec/javascripts/gpg_badges_spec.js b/spec/javascripts/gpg_badges_spec.js new file mode 100644 index 00000000000..7a826487bf9 --- /dev/null +++ b/spec/javascripts/gpg_badges_spec.js @@ -0,0 +1,48 @@ +import GpgBadges from '~/gpg_badges'; + +describe('GpgBadges', () => { + const dummyCommitSha = 'n0m0rec0ffee'; + const dummyBadgeHtml = 'dummy html'; + const dummyResponse = { + signatures: [{ + commit_sha: dummyCommitSha, + html: dummyBadgeHtml, + }], + }; + + beforeEach(() => { + setFixtures(` + <div class="parent-container"> + <div class="js-loading-gpg-badge" data-commit-sha="${dummyCommitSha}"></div> + </div> + `); + }); + + it('displays a loading spinner', () => { + spyOn($, 'get').and.returnValue({ + done() { + // intentionally left blank + }, + }); + + GpgBadges.fetch(); + + expect(document.querySelector('.js-loading-gpg-badge:empty')).toBe(null); + const spinners = document.querySelectorAll('.js-loading-gpg-badge i.fa.fa-spinner.fa-spin'); + expect(spinners.length).toBe(1); + }); + + it('replaces the loading spinner', () => { + spyOn($, 'get').and.returnValue({ + done(callback) { + callback(dummyResponse); + }, + }); + + GpgBadges.fetch(); + + expect(document.querySelector('.js-loading-gpg-badge')).toBe(null); + const parentContainer = document.querySelector('.parent-container'); + expect(parentContainer.innerHTML.trim()).toEqual(dummyBadgeHtml); + }); +}); diff --git a/spec/javascripts/pipeline_schedules/pipeline_schedule_callout_spec.js b/spec/javascripts/pipeline_schedules/pipeline_schedule_callout_spec.js index 6120d224ac0..ed481cb60a1 100644 --- a/spec/javascripts/pipeline_schedules/pipeline_schedule_callout_spec.js +++ b/spec/javascripts/pipeline_schedules/pipeline_schedule_callout_spec.js @@ -1,6 +1,6 @@ import Vue from 'vue'; import Cookies from 'js-cookie'; -import PipelineSchedulesCallout from '~/pipeline_schedules/components/pipeline_schedules_callout'; +import PipelineSchedulesCallout from '~/pipeline_schedules/components/pipeline_schedules_callout.vue'; const PipelineSchedulesCalloutComponent = Vue.extend(PipelineSchedulesCallout); const cookieKey = 'pipeline_schedules_callout_dismissed'; diff --git a/spec/javascripts/repo/components/repo_commit_section_spec.js b/spec/javascripts/repo/components/repo_commit_section_spec.js index db2b7d51626..249a2f36fcd 100644 --- a/spec/javascripts/repo/components/repo_commit_section_spec.js +++ b/spec/javascripts/repo/components/repo_commit_section_spec.js @@ -1,57 +1,57 @@ import Vue from 'vue'; import repoCommitSection from '~/repo/components/repo_commit_section.vue'; import RepoStore from '~/repo/stores/repo_store'; -import RepoHelper from '~/repo/helpers/repo_helper'; import Api from '~/api'; describe('RepoCommitSection', () => { const branch = 'master'; const projectUrl = 'projectUrl'; - const openedFiles = [{ + const changedFiles = [{ id: 0, changed: true, url: `/namespace/${projectUrl}/blob/${branch}/dir/file0.ext`, + path: 'dir/file0.ext', newContent: 'a', }, { id: 1, changed: true, url: `/namespace/${projectUrl}/blob/${branch}/dir/file1.ext`, + path: 'dir/file1.ext', newContent: 'b', - }, { + }]; + const openedFiles = changedFiles.concat([{ id: 2, url: `/namespace/${projectUrl}/blob/${branch}/dir/file2.ext`, + path: 'dir/file2.ext', changed: false, - }]; + }]); RepoStore.projectUrl = projectUrl; - function createComponent() { + function createComponent(el) { const RepoCommitSection = Vue.extend(repoCommitSection); - return new RepoCommitSection().$mount(); + return new RepoCommitSection().$mount(el); } it('renders a commit section', () => { RepoStore.isCommitable = true; + RepoStore.currentBranch = branch; RepoStore.targetBranch = branch; RepoStore.openedFiles = openedFiles; - spyOn(RepoHelper, 'getBranch').and.returnValue(branch); - const vm = createComponent(); - const changedFiles = [...vm.$el.querySelectorAll('.changed-files > li')]; + const changedFileElements = [...vm.$el.querySelectorAll('.changed-files > li')]; const commitMessage = vm.$el.querySelector('#commit-message'); - const submitCommit = vm.$el.querySelector('.submit-commit'); + const submitCommit = vm.$refs.submitCommit; const targetBranch = vm.$el.querySelector('.target-branch'); expect(vm.$el.querySelector(':scope > form')).toBeTruthy(); - expect(vm.$el.querySelector('.staged-files').textContent).toEqual('Staged files (2)'); - expect(changedFiles.length).toEqual(2); + expect(vm.$el.querySelector('.staged-files').textContent.trim()).toEqual('Staged files (2)'); + expect(changedFileElements.length).toEqual(2); - changedFiles.forEach((changedFile, i) => { - const filePath = RepoHelper.getFilePathFromFullPath(openedFiles[i].url, branch); - - expect(changedFile.textContent).toEqual(filePath); + changedFileElements.forEach((changedFile, i) => { + expect(changedFile.textContent.trim()).toEqual(changedFiles[i].path); }); expect(commitMessage.tagName).toEqual('TEXTAREA'); @@ -59,9 +59,9 @@ describe('RepoCommitSection', () => { expect(submitCommit.type).toEqual('submit'); expect(submitCommit.disabled).toBeTruthy(); expect(submitCommit.querySelector('.fa-spinner.fa-spin')).toBeFalsy(); - expect(vm.$el.querySelector('.commit-summary').textContent).toEqual('Commit 2 files'); - expect(targetBranch.querySelector(':scope > label').textContent).toEqual('Target branch'); - expect(targetBranch.querySelector('.help-block').textContent).toEqual(branch); + expect(vm.$el.querySelector('.commit-summary').textContent.trim()).toEqual('Commit 2 files'); + expect(targetBranch.querySelector(':scope > label').textContent.trim()).toEqual('Target branch'); + expect(targetBranch.querySelector('.help-block').textContent.trim()).toEqual(branch); }); it('does not render if not isCommitable', () => { @@ -89,14 +89,20 @@ describe('RepoCommitSection', () => { const projectId = 'projectId'; const commitMessage = 'commitMessage'; RepoStore.isCommitable = true; + RepoStore.currentBranch = branch; + RepoStore.targetBranch = branch; RepoStore.openedFiles = openedFiles; RepoStore.projectId = projectId; - spyOn(RepoHelper, 'getBranch').and.returnValue(branch); + // We need to append to body to get form `submit` events working + // Otherwise we run into, "Form submission canceled because the form is not connected" + // See https://html.spec.whatwg.org/multipage/form-control-infrastructure.html#form-submission-algorithm + const el = document.createElement('div'); + document.body.appendChild(el); - const vm = createComponent(); + const vm = createComponent(el); const commitMessageEl = vm.$el.querySelector('#commit-message'); - const submitCommit = vm.$el.querySelector('.submit-commit'); + const submitCommit = vm.$refs.submitCommit; vm.commitMessage = commitMessage; @@ -124,10 +130,8 @@ describe('RepoCommitSection', () => { expect(actions[1].action).toEqual('update'); expect(actions[0].content).toEqual(openedFiles[0].newContent); expect(actions[1].content).toEqual(openedFiles[1].newContent); - expect(actions[0].file_path) - .toEqual(RepoHelper.getFilePathFromFullPath(openedFiles[0].url, branch)); - expect(actions[1].file_path) - .toEqual(RepoHelper.getFilePathFromFullPath(openedFiles[1].url, branch)); + expect(actions[0].file_path).toEqual(openedFiles[0].path); + expect(actions[1].file_path).toEqual(openedFiles[1].path); done(); }); @@ -140,7 +144,6 @@ describe('RepoCommitSection', () => { const vm = { submitCommitsLoading: true, changedFiles: new Array(10), - openedFiles: new Array(10), commitMessage: 'commitMessage', editMode: true, }; @@ -149,7 +152,6 @@ describe('RepoCommitSection', () => { expect(vm.submitCommitsLoading).toEqual(false); expect(vm.changedFiles).toEqual([]); - expect(vm.openedFiles).toEqual([]); expect(vm.commitMessage).toEqual(''); expect(vm.editMode).toEqual(false); }); diff --git a/spec/javascripts/repo/components/repo_edit_button_spec.js b/spec/javascripts/repo/components/repo_edit_button_spec.js index df2f9697acc..29dc2d21e4b 100644 --- a/spec/javascripts/repo/components/repo_edit_button_spec.js +++ b/spec/javascripts/repo/components/repo_edit_button_spec.js @@ -12,18 +12,22 @@ describe('RepoEditButton', () => { it('renders an edit button that toggles the view state', (done) => { RepoStore.isCommitable = true; RepoStore.changedFiles = []; + RepoStore.binary = false; + RepoStore.openedFiles = [{}, {}]; const vm = createComponent(); expect(vm.$el.tagName).toEqual('BUTTON'); expect(vm.$el.textContent).toMatch('Edit'); - spyOn(vm, 'editClicked').and.callThrough(); + spyOn(vm, 'editCancelClicked').and.callThrough(); + spyOn(vm, 'toggleProjectRefsForm'); vm.$el.click(); Vue.nextTick(() => { - expect(vm.editClicked).toHaveBeenCalled(); + expect(vm.editCancelClicked).toHaveBeenCalled(); + expect(vm.toggleProjectRefsForm).toHaveBeenCalled(); expect(vm.$el.textContent).toMatch('Cancel edit'); done(); }); @@ -38,14 +42,10 @@ describe('RepoEditButton', () => { }); describe('methods', () => { - describe('editClicked', () => { - it('sets dialog to open when there are changedFiles', () => { + describe('editCancelClicked', () => { + it('sets dialog to open when there are changedFiles'); - }); - - it('toggles editMode and calls toggleBlobView', () => { - - }); + it('toggles editMode and calls toggleBlobView'); }); }); }); diff --git a/spec/javascripts/repo/components/repo_editor_spec.js b/spec/javascripts/repo/components/repo_editor_spec.js index 35e0c995163..85d55d171f9 100644 --- a/spec/javascripts/repo/components/repo_editor_spec.js +++ b/spec/javascripts/repo/components/repo_editor_spec.js @@ -1,26 +1,49 @@ import Vue from 'vue'; import repoEditor from '~/repo/components/repo_editor.vue'; -import RepoStore from '~/repo/stores/repo_store'; describe('RepoEditor', () => { - function createComponent() { + beforeEach(() => { const RepoEditor = Vue.extend(repoEditor); - return new RepoEditor().$mount(); - } + this.vm = new RepoEditor().$mount(); + }); + + it('renders an ide container', (done) => { + this.vm.openedFiles = ['idiidid']; + this.vm.binary = false; - it('renders an ide container', () => { - const monacoInstance = jasmine.createSpyObj('monacoInstance', ['onMouseUp', 'onKeyUp', 'setModel', 'updateOptions']); - const monaco = { - editor: jasmine.createSpyObj('editor', ['create']), - }; - RepoStore.monaco = monaco; + Vue.nextTick(() => { + expect(this.vm.shouldHideEditor).toBe(false); + expect(this.vm.$el.id).toEqual('ide'); + expect(this.vm.$el.tagName).toBe('DIV'); + done(); + }); + }); - monaco.editor.create.and.returnValue(monacoInstance); - spyOn(repoEditor.watch, 'blobRaw'); + describe('when there are no open files', () => { + it('does not render the ide', (done) => { + this.vm.openedFiles = []; + + Vue.nextTick(() => { + expect(this.vm.shouldHideEditor).toBe(true); + expect(this.vm.$el.tagName).not.toBeDefined(); + done(); + }); + }); + }); - const vm = createComponent(); + describe('when open file is binary and not raw', () => { + it('does not render the IDE', (done) => { + this.vm.binary = true; + this.vm.activeFile = { + raw: false, + }; - expect(vm.$el.id).toEqual('ide'); + Vue.nextTick(() => { + expect(this.vm.shouldHideEditor).toBe(true); + expect(this.vm.$el.tagName).not.toBeDefined(); + done(); + }); + }); }); }); diff --git a/spec/javascripts/repo/components/repo_file_buttons_spec.js b/spec/javascripts/repo/components/repo_file_buttons_spec.js index e1f25e4485f..dfab51710c3 100644 --- a/spec/javascripts/repo/components/repo_file_buttons_spec.js +++ b/spec/javascripts/repo/components/repo_file_buttons_spec.js @@ -23,6 +23,7 @@ describe('RepoFileButtons', () => { RepoStore.activeFile = activeFile; RepoStore.activeFileLabel = activeFileLabel; RepoStore.editMode = true; + RepoStore.binary = false; const vm = createComponent(); const raw = vm.$el.querySelector('.raw'); @@ -31,13 +32,13 @@ describe('RepoFileButtons', () => { expect(vm.$el.id).toEqual('repo-file-buttons'); expect(raw.href).toMatch(`/${activeFile.raw_path}`); - expect(raw.textContent).toEqual('Raw'); + expect(raw.textContent.trim()).toEqual('Raw'); expect(blame.href).toMatch(`/${activeFile.blame_path}`); - expect(blame.textContent).toEqual('Blame'); + expect(blame.textContent.trim()).toEqual('Blame'); expect(history.href).toMatch(`/${activeFile.commits_path}`); - expect(history.textContent).toEqual('History'); - expect(vm.$el.querySelector('.permalink').textContent).toEqual('Permalink'); - expect(vm.$el.querySelector('.preview').textContent).toEqual(activeFileLabel); + expect(history.textContent.trim()).toEqual('History'); + expect(vm.$el.querySelector('.permalink').textContent.trim()).toEqual('Permalink'); + expect(vm.$el.querySelector('.preview').textContent.trim()).toEqual(activeFileLabel); }); it('triggers rawPreviewToggle on preview click', () => { @@ -71,12 +72,4 @@ describe('RepoFileButtons', () => { expect(vm.$el.querySelector('.preview')).toBeFalsy(); }); - - it('does not render if not isMini', () => { - RepoStore.openedFiles = []; - - const vm = createComponent(); - - expect(vm.$el.innerHTML).toBeFalsy(); - }); }); diff --git a/spec/javascripts/repo/components/repo_file_spec.js b/spec/javascripts/repo/components/repo_file_spec.js index 90616ae13ca..518a2d25ecf 100644 --- a/spec/javascripts/repo/components/repo_file_spec.js +++ b/spec/javascripts/repo/components/repo_file_spec.js @@ -39,9 +39,9 @@ describe('RepoFile', () => { expect(vm.$el.querySelector(`.${file.icon}`).style.marginLeft).toEqual('100px'); expect(name.title).toEqual(file.url); expect(name.href).toMatch(`/${file.url}`); - expect(name.textContent).toEqual(file.name); - expect(vm.$el.querySelector('.commit-message').textContent).toBe(file.lastCommitMessage); - expect(vm.$el.querySelector('.commit-update').textContent).toBe(updated); + expect(name.textContent.trim()).toEqual(file.name); + expect(vm.$el.querySelector('.commit-message').textContent.trim()).toBe(file.lastCommitMessage); + expect(vm.$el.querySelector('.commit-update').textContent.trim()).toBe(updated); expect(fileIcon.classList.contains(file.icon)).toBeTruthy(); expect(fileIcon.style.marginLeft).toEqual(`${file.level * 10}px`); }); diff --git a/spec/javascripts/repo/components/repo_loading_file_spec.js b/spec/javascripts/repo/components/repo_loading_file_spec.js index d84f4c5609e..a030314d749 100644 --- a/spec/javascripts/repo/components/repo_loading_file_spec.js +++ b/spec/javascripts/repo/components/repo_loading_file_spec.js @@ -13,7 +13,7 @@ describe('RepoLoadingFile', () => { function assertLines(lines) { lines.forEach((line, n) => { const index = n + 1; - expect(line.classList.contains(`line-of-code-${index}`)).toBeTruthy(); + expect(line.classList.contains(`skeleton-line-${index}`)).toBeTruthy(); }); } diff --git a/spec/javascripts/repo/components/repo_sidebar_spec.js b/spec/javascripts/repo/components/repo_sidebar_spec.js index 0d216c9c026..abcff8e537e 100644 --- a/spec/javascripts/repo/components/repo_sidebar_spec.js +++ b/spec/javascripts/repo/components/repo_sidebar_spec.js @@ -1,4 +1,6 @@ import Vue from 'vue'; +import Helper from '~/repo/helpers/repo_helper'; +import RepoService from '~/repo/services/repo_service'; import RepoStore from '~/repo/stores/repo_store'; import repoSidebar from '~/repo/components/repo_sidebar.vue'; @@ -13,6 +15,7 @@ describe('RepoSidebar', () => { RepoStore.files = [{ id: 0, }]; + RepoStore.openedFiles = []; const vm = createComponent(); const thead = vm.$el.querySelector('thead'); const tbody = vm.$el.querySelector('tbody'); @@ -58,4 +61,51 @@ describe('RepoSidebar', () => { expect(vm.$el.querySelector('tbody .prev-directory')).toBeTruthy(); }); + + describe('methods', () => { + describe('fileClicked', () => { + it('should fetch data for new file', () => { + spyOn(Helper, 'getContent').and.callThrough(); + const file1 = { + id: 0, + url: '', + }; + RepoStore.files = [file1]; + RepoStore.isRoot = true; + const vm = createComponent(); + + vm.fileClicked(file1); + + expect(Helper.getContent).toHaveBeenCalledWith(file1); + }); + + it('should hide files in directory if already open', () => { + spyOn(RepoStore, 'removeChildFilesOfTree').and.callThrough(); + const file1 = { + id: 0, + type: 'tree', + url: '', + opened: true, + }; + RepoStore.files = [file1]; + RepoStore.isRoot = true; + const vm = createComponent(); + + vm.fileClicked(file1); + + expect(RepoStore.removeChildFilesOfTree).toHaveBeenCalledWith(file1); + }); + }); + + describe('goToPreviousDirectoryClicked', () => { + it('should hide files in directory if already open', () => { + const prevUrl = 'foo/bar'; + const vm = createComponent(); + + vm.goToPreviousDirectoryClicked(prevUrl); + + expect(RepoService.url).toEqual(prevUrl); + }); + }); + }); }); diff --git a/spec/javascripts/repo/components/repo_tab_spec.js b/spec/javascripts/repo/components/repo_tab_spec.js index f3572804b4a..d2a790ad73a 100644 --- a/spec/javascripts/repo/components/repo_tab_spec.js +++ b/spec/javascripts/repo/components/repo_tab_spec.js @@ -12,7 +12,6 @@ describe('RepoTab', () => { it('renders a close link and a name link', () => { const tab = { - loading: false, url: 'url', name: 'name', }; @@ -22,38 +21,21 @@ describe('RepoTab', () => { const close = vm.$el.querySelector('.close'); const name = vm.$el.querySelector(`a[title="${tab.url}"]`); - spyOn(vm, 'xClicked'); + spyOn(vm, 'closeTab'); spyOn(vm, 'tabClicked'); expect(close.querySelector('.fa-times')).toBeTruthy(); - expect(name.textContent).toEqual(tab.name); + expect(name.textContent.trim()).toEqual(tab.name); close.click(); name.click(); - expect(vm.xClicked).toHaveBeenCalledWith(tab); + expect(vm.closeTab).toHaveBeenCalledWith(tab); expect(vm.tabClicked).toHaveBeenCalledWith(tab); }); - it('renders a spinner if tab is loading', () => { - const tab = { - loading: true, - url: 'url', - }; - const vm = createComponent({ - tab, - }); - const close = vm.$el.querySelector('.close'); - const name = vm.$el.querySelector(`a[title="${tab.url}"]`); - - expect(close).toBeFalsy(); - expect(name).toBeFalsy(); - expect(vm.$el.querySelector('.fa.fa-spinner.fa-spin')).toBeTruthy(); - }); - it('renders an fa-circle icon if tab is changed', () => { const tab = { - loading: false, url: 'url', name: 'name', changed: true, @@ -66,22 +48,22 @@ describe('RepoTab', () => { }); describe('methods', () => { - describe('xClicked', () => { + describe('closeTab', () => { const vm = jasmine.createSpyObj('vm', ['$emit']); it('returns undefined and does not $emit if file is changed', () => { const file = { changed: true }; - const returnVal = repoTab.methods.xClicked.call(vm, file); + const returnVal = repoTab.methods.closeTab.call(vm, file); expect(returnVal).toBeUndefined(); expect(vm.$emit).not.toHaveBeenCalled(); }); - it('$emits xclicked event with file obj', () => { + it('$emits tabclosed event with file obj', () => { const file = { changed: false }; - repoTab.methods.xClicked.call(vm, file); + repoTab.methods.closeTab.call(vm, file); - expect(vm.$emit).toHaveBeenCalledWith('xclicked', file); + expect(vm.$emit).toHaveBeenCalledWith('tabclosed', file); }); }); }); diff --git a/spec/javascripts/repo/components/repo_tabs_spec.js b/spec/javascripts/repo/components/repo_tabs_spec.js index fdb12cfc00f..a02b54efafc 100644 --- a/spec/javascripts/repo/components/repo_tabs_spec.js +++ b/spec/javascripts/repo/components/repo_tabs_spec.js @@ -18,44 +18,25 @@ describe('RepoTabs', () => { it('renders a list of tabs', () => { RepoStore.openedFiles = openedFiles; - RepoStore.tabsOverflow = true; const vm = createComponent(); const tabs = [...vm.$el.querySelectorAll(':scope > li')]; expect(vm.$el.id).toEqual('tabs'); - expect(vm.$el.classList.contains('overflown')).toBeTruthy(); expect(tabs.length).toEqual(3); expect(tabs[0].classList.contains('active')).toBeTruthy(); expect(tabs[1].classList.contains('active')).toBeFalsy(); expect(tabs[2].classList.contains('tabs-divider')).toBeTruthy(); }); - it('does not render a tabs list if not isMini', () => { - RepoStore.openedFiles = []; - - const vm = createComponent(); - - expect(vm.$el.innerHTML).toBeFalsy(); - }); - - it('does not apply overflown class if not tabsOverflow', () => { - RepoStore.openedFiles = openedFiles; - RepoStore.tabsOverflow = false; - - const vm = createComponent(); - - expect(vm.$el.classList.contains('overflown')).toBeFalsy(); - }); - describe('methods', () => { - describe('xClicked', () => { + describe('tabClosed', () => { it('calls removeFromOpenedFiles with file obj', () => { const file = {}; spyOn(RepoStore, 'removeFromOpenedFiles'); - repoTabs.methods.xClicked(file); + repoTabs.methods.tabClosed(file); expect(RepoStore.removeFromOpenedFiles).toHaveBeenCalledWith(file); }); diff --git a/spec/javascripts/sidebar/confidential_issue_sidebar_spec.js b/spec/javascripts/sidebar/confidential_issue_sidebar_spec.js index 90eac1ed1ab..2e16adffb5b 100644 --- a/spec/javascripts/sidebar/confidential_issue_sidebar_spec.js +++ b/spec/javascripts/sidebar/confidential_issue_sidebar_spec.js @@ -41,7 +41,7 @@ describe('Confidential Issue Sidebar Block', () => { ).toBe(true); expect( - vm2.$el.innerHTML.includes('None'), + vm2.$el.innerHTML.includes('This issue is not confidential'), ).toBe(true); }); 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/ci/trace/stream_spec.rb b/spec/lib/gitlab/ci/trace/stream_spec.rb index ebe5af56160..e5555546fa8 100644 --- a/spec/lib/gitlab/ci/trace/stream_spec.rb +++ b/spec/lib/gitlab/ci/trace/stream_spec.rb @@ -295,7 +295,7 @@ describe Gitlab::Ci::Trace::Stream do end context 'malicious regexp' do - let(:data) { malicious_text } + let(:data) { malicious_text } let(:regex) { malicious_regexp } include_examples 'malicious regexp' diff --git a/spec/lib/gitlab/cycle_analytics/base_event_fetcher_spec.rb b/spec/lib/gitlab/cycle_analytics/base_event_fetcher_spec.rb index 854aaa34c73..0560c47f03f 100644 --- a/spec/lib/gitlab/cycle_analytics/base_event_fetcher_spec.rb +++ b/spec/lib/gitlab/cycle_analytics/base_event_fetcher_spec.rb @@ -6,10 +6,10 @@ describe Gitlab::CycleAnalytics::BaseEventFetcher do let(:user) { create(:user, :admin) } let(:start_time_attrs) { Issue.arel_table[:created_at] } let(:end_time_attrs) { [Issue::Metrics.arel_table[:first_associated_with_milestone_at]] } - let(:options) do + let(:options) do { start_time_attrs: start_time_attrs, end_time_attrs: end_time_attrs, - from: 30.days.ago } + from: 30.days.ago } end subject do diff --git a/spec/lib/gitlab/git/commit_spec.rb b/spec/lib/gitlab/git/commit_spec.rb index c531d4b055f..ac33cd8a2c9 100644 --- a/spec/lib/gitlab/git/commit_spec.rb +++ b/spec/lib/gitlab/git/commit_spec.rb @@ -310,8 +310,8 @@ describe Gitlab::Git::Commit, seed_helper: true do commits.map(&:id) end - it 'has 33 elements' do - expect(subject.size).to eq(33) + it 'has 34 elements' do + expect(subject.size).to eq(34) end it 'includes the expected commits' do diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index 04c86a2c1a7..4ef5d9070a2 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -289,7 +289,13 @@ describe Gitlab::Git::Repository, seed_helper: true do it { expect(submodule_url('six')).to eq('git://github.com/randx/six.git') } end - context 'no submodules at commit' do + context 'no .gitmodules at commit' do + let(:ref) { '9596bc54a6f0c0c98248fe97077eb5ccf48a98d0' } + + it { expect(submodule_url('six')).to eq(nil) } + end + + context 'no gitlink entry' do let(:ref) { '6d39438' } it { expect(submodule_url('six')).to eq(nil) } @@ -986,7 +992,7 @@ describe Gitlab::Git::Repository, seed_helper: true do describe '#branch_count' do it 'returns the number of branches' do - expect(repository.branch_count).to eq(9) + expect(repository.branch_count).to eq(10) end end @@ -1002,7 +1008,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 diff --git a/spec/lib/gitlab/git/storage/circuit_breaker_spec.rb b/spec/lib/gitlab/git/storage/circuit_breaker_spec.rb index 7256402b010..9d1763b96ad 100644 --- a/spec/lib/gitlab/git/storage/circuit_breaker_spec.rb +++ b/spec/lib/gitlab/git/storage/circuit_breaker_spec.rb @@ -175,11 +175,7 @@ describe Gitlab::Git::Storage::CircuitBreaker, clean_gitlab_redis_shared_state: describe '#track_storage_inaccessible' do around do |example| - Timecop.freeze - - example.run - - Timecop.return + 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 a0e5e401359..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 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/key_fingerprint_spec.rb b/spec/lib/gitlab/key_fingerprint_spec.rb index f5fd5a96bc9..d643dc5342d 100644 --- a/spec/lib/gitlab/key_fingerprint_spec.rb +++ b/spec/lib/gitlab/key_fingerprint_spec.rb @@ -30,8 +30,8 @@ describe Gitlab::KeyFingerprint, lib: true do MD5_FINGERPRINTS = { rsa: '06:b2:8a:92:df:0e:11:2c:ca:7b:8f:a4:ba:6e:4b:fd', - ecdsa: '45:ff:5b:98:9a:b6:8a:41:13:c1:30:8b:09:5e:7b:4e', - ed25519: '2e:65:6a:c8:cf:bf:b2:8b:9a:bd:6d:9f:11:5c:12:16', + ecdsa: '45:ff:5b:98:9a:b6:8a:41:13:c1:30:8b:09:5e:7b:4e', + ed25519: '2e:65:6a:c8:cf:bf:b2:8b:9a:bd:6d:9f:11:5c:12:16', dss: '57:98:86:02:5f:9c:f4:9b:ad:5a:1e:51:92:0e:fd:2b' }.freeze diff --git a/spec/lib/gitlab/ldap/auth_hash_spec.rb b/spec/lib/gitlab/ldap/auth_hash_spec.rb index 57a91193004..8370adf9211 100644 --- a/spec/lib/gitlab/ldap/auth_hash_spec.rb +++ b/spec/lib/gitlab/ldap/auth_hash_spec.rb @@ -4,8 +4,8 @@ describe Gitlab::LDAP::AuthHash do let(:auth_hash) do described_class.new( OmniAuth::AuthHash.new( - uid: '123456', - provider: 'ldapmain', + uid: '123456', + provider: 'ldapmain', info: info, extra: { raw_info: raw_info @@ -33,11 +33,11 @@ describe Gitlab::LDAP::AuthHash do context "without overridden attributes" do it "has the correct username" do - expect(auth_hash.username).to eq("123456") + expect(auth_hash.username).to eq("123456") end it "has the correct name" do - expect(auth_hash.name).to eq("Smith, J.") + expect(auth_hash.name).to eq("Smith, J.") end end @@ -54,11 +54,11 @@ describe Gitlab::LDAP::AuthHash do end it "has the correct username" do - expect(auth_hash.username).to eq("johnsmith@example.com") + expect(auth_hash.username).to eq("johnsmith@example.com") end it "has the correct name" do - expect(auth_hash.name).to eq("John Smith") + expect(auth_hash.name).to eq("John Smith") end end end 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/o_auth/user_spec.rb b/spec/lib/gitlab/o_auth/user_spec.rb index 15edb820908..2cf0f7516de 100644 --- a/spec/lib/gitlab/o_auth/user_spec.rb +++ b/spec/lib/gitlab/o_auth/user_spec.rb @@ -481,7 +481,7 @@ describe Gitlab::OAuth::User do email: 'admin@othermail.com' } end - + it 'generates the username with a counter' do expect(gl_user.username).to eq('admin1') end diff --git a/spec/lib/gitlab/project_template_spec.rb b/spec/lib/gitlab/project_template_spec.rb index 12e75cdd5d0..d19bd611919 100644 --- a/spec/lib/gitlab/project_template_spec.rb +++ b/spec/lib/gitlab/project_template_spec.rb @@ -4,7 +4,9 @@ describe Gitlab::ProjectTemplate do describe '.all' do it 'returns a all templates' do expected = [ - described_class.new('rails', 'Ruby on Rails') + described_class.new('rails', 'Ruby on Rails'), + described_class.new('spring', 'Spring'), + described_class.new('express', 'NodeJS Express') ] expect(described_class.all).to be_an(Array) diff --git a/spec/lib/gitlab/prometheus/additional_metrics_parser_spec.rb b/spec/lib/gitlab/prometheus/additional_metrics_parser_spec.rb index d7df4e35c31..5589db92b1d 100644 --- a/spec/lib/gitlab/prometheus/additional_metrics_parser_spec.rb +++ b/spec/lib/gitlab/prometheus/additional_metrics_parser_spec.rb @@ -24,7 +24,7 @@ describe Gitlab::Prometheus::AdditionalMetricsParser do queries: [{ query_range: 'query_range_empty' }] - group: group_b priority: 1 - metrics: + metrics: - title: title required_metrics: ['metric_a'] weight: 1 @@ -148,7 +148,7 @@ describe Gitlab::Prometheus::AdditionalMetricsParser do - group: group_a priority: 1 metrics: - - title: + - title: required_metrics: [] weight: 1 queries: [] diff --git a/spec/lib/gitlab/redis/wrapper_spec.rb b/spec/lib/gitlab/redis/wrapper_spec.rb index e1becd0a614..0c22a0d62cc 100644 --- a/spec/lib/gitlab/redis/wrapper_spec.rb +++ b/spec/lib/gitlab/redis/wrapper_spec.rb @@ -17,4 +17,11 @@ describe Gitlab::Redis::Wrapper do let(:class_redis_url) { Gitlab::Redis::Wrapper::DEFAULT_REDIS_URL } include_examples "redis_shared_examples" + + describe '.config_file_path' do + it 'returns the absolute path to the configuration file' do + expect(described_class.config_file_path('foo.yml')) + .to eq Rails.root.join('config', 'foo.yml').to_s + end + end end diff --git a/spec/lib/gitlab/shell_spec.rb b/spec/lib/gitlab/shell_spec.rb index 2345874cf10..cfadee0bcf5 100644 --- a/spec/lib/gitlab/shell_spec.rb +++ b/spec/lib/gitlab/shell_spec.rb @@ -94,28 +94,41 @@ describe Gitlab::Shell do end describe 'projects commands' do - let(:projects_path) { 'tmp/tests/shell-projects-test/bin/gitlab-projects' } + let(:gitlab_shell_path) { File.expand_path('tmp/tests/gitlab-shell') } + let(:projects_path) { File.join(gitlab_shell_path, 'bin/gitlab-projects') } + let(:gitlab_shell_hooks_path) { File.join(gitlab_shell_path, 'hooks') } before do - allow(Gitlab.config.gitlab_shell).to receive(:path).and_return('tmp/tests/shell-projects-test') + allow(Gitlab.config.gitlab_shell).to receive(:path).and_return(gitlab_shell_path) + allow(Gitlab.config.gitlab_shell).to receive(:hooks_path).and_return(gitlab_shell_hooks_path) allow(Gitlab.config.gitlab_shell).to receive(:git_timeout).and_return(800) end describe '#add_repository' do - it 'returns true when the command succeeds' do - expect(Gitlab::Popen).to receive(:popen) - .with([projects_path, 'add-project', 'current/storage', 'project/path.git'], - nil, popen_vars).and_return([nil, 0]) + it 'creates a repository' do + created_path = File.join(TestEnv.repos_path, 'project', 'path.git') + hooks_path = File.join(created_path, 'hooks') + + begin + result = gitlab_shell.add_repository(TestEnv.repos_path, 'project/path') + + repo_stat = File.stat(created_path) rescue nil + hooks_stat = File.lstat(hooks_path) rescue nil + hooks_dir = File.realpath(hooks_path) + ensure + FileUtils.rm_rf(created_path) + end - expect(gitlab_shell.add_repository('current/storage', 'project/path')).to be true + expect(result).to be_truthy + expect(repo_stat.mode & 0o777).to eq(0o770) + expect(hooks_stat.symlink?).to be_truthy + expect(hooks_dir).to eq(gitlab_shell_hooks_path) end it 'returns false when the command fails' do - expect(Gitlab::Popen).to receive(:popen) - .with([projects_path, 'add-project', 'current/storage', 'project/path.git'], - nil, popen_vars).and_return(["error", 1]) + expect(FileUtils).to receive(:mkdir_p).and_raise(Errno::EEXIST) - expect(gitlab_shell.add_repository('current/storage', 'project/path')).to be false + expect(gitlab_shell.add_repository('current/storage', 'project/path')).to be_falsy end end diff --git a/spec/lib/gitlab/utils_spec.rb b/spec/lib/gitlab/utils_spec.rb index 111c873f79c..92787bb262e 100644 --- a/spec/lib/gitlab/utils_spec.rb +++ b/spec/lib/gitlab/utils_spec.rb @@ -1,7 +1,21 @@ require 'spec_helper' describe Gitlab::Utils do - delegate :to_boolean, :boolean_to_yes_no, to: :described_class + delegate :to_boolean, :boolean_to_yes_no, :slugify, to: :described_class + + describe '.slugify' do + { + 'TEST' => 'test', + 'project_with_underscores' => 'project-with-underscores', + 'namespace/project' => 'namespace-project', + 'a' * 70 => 'a' * 63, + 'test_trailing_' => 'test-trailing' + }.each do |original, expected| + it "slugifies #{original} to #{expected}" do + expect(slugify(original)).to eq(expected) + end + end + end describe '.to_boolean' do it 'accepts booleans' do diff --git a/spec/lib/gitlab/workhorse_spec.rb b/spec/lib/gitlab/workhorse_spec.rb index 654397ccffb..b66afafa174 100644 --- a/spec/lib/gitlab/workhorse_spec.rb +++ b/spec/lib/gitlab/workhorse_spec.rb @@ -202,7 +202,6 @@ describe Gitlab::Workhorse do context 'when Gitaly is enabled' do let(:gitaly_params) do { - GitalyAddress: Gitlab::GitalyClient.address('default'), GitalyServer: { address: Gitlab::GitalyClient.address('default'), token: Gitlab::GitalyClient.token('default') @@ -217,7 +216,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/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/migrate_old_artifacts_spec.rb b/spec/migrations/migrate_old_artifacts_spec.rb index cfe1ca481b2..81366d15b34 100644 --- a/spec/migrations/migrate_old_artifacts_spec.rb +++ b/spec/migrations/migrate_old_artifacts_spec.rb @@ -10,7 +10,7 @@ describe MigrateOldArtifacts do before do allow(Gitlab.config.artifacts).to receive(:path).and_return(directory) end - + after do FileUtils.remove_entry_secure(directory) end @@ -95,7 +95,7 @@ describe MigrateOldArtifacts do FileUtils.copy( Rails.root.join('spec/fixtures/ci_build_artifacts.zip'), File.join(legacy_path(build), "ci_build_artifacts.zip")) - + FileUtils.copy( Rails.root.join('spec/fixtures/ci_build_artifacts_metadata.gz'), File.join(legacy_path(build), "ci_build_artifacts_metadata.gz")) 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/remove_duplicate_mr_events_spec.rb b/spec/migrations/remove_duplicate_mr_events_spec.rb new file mode 100644 index 00000000000..e393374028f --- /dev/null +++ b/spec/migrations/remove_duplicate_mr_events_spec.rb @@ -0,0 +1,26 @@ +require 'spec_helper' +require Rails.root.join('db', 'post_migrate', '20170815060945_remove_duplicate_mr_events.rb') + +describe RemoveDuplicateMrEvents, truncate: true do + let(:migration) { described_class.new } + + describe '#up' do + let(:user) { create(:user) } + let(:merge_requests) { create_list(:merge_request, 2) } + let(:issue) { create(:issue) } + let!(:events) do + [ + create(:event, :created, author: user, target: merge_requests.first), + create(:event, :created, author: user, target: merge_requests.first), + create(:event, :updated, author: user, target: merge_requests.first), + create(:event, :created, author: user, target: merge_requests.second), + create(:event, :created, author: user, target: issue), + create(:event, :created, author: user, target: issue) + ] + end + + it 'removes duplicated merge request create records' do + expect { migration.up }.to change { Event.count }.from(6).to(5) + end + end +end 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/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/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 86afa856ea7..767f0ad9e65 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -1220,7 +1220,7 @@ describe Ci::Build do { key: 'CI_PROJECT_ID', value: project.id.to_s, public: true }, { key: 'CI_PROJECT_NAME', value: project.path, public: true }, { key: 'CI_PROJECT_PATH', value: project.full_path, public: true }, - { key: 'CI_PROJECT_PATH_SLUG', value: project.full_path.parameterize, public: true }, + { key: 'CI_PROJECT_PATH_SLUG', value: project.full_path_slug, public: true }, { key: 'CI_PROJECT_NAMESPACE', value: project.namespace.full_path, public: true }, { key: 'CI_PROJECT_URL', value: project.web_url, public: true }, { key: 'CI_PIPELINE_ID', value: pipeline.id.to_s, public: true }, 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/namespace_spec.rb b/spec/models/namespace_spec.rb index 1a00c50690c..69286eff984 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -315,6 +315,20 @@ describe Namespace do end end + describe '#self_and_ancestors', :nested_groups do + let(:group) { create(:group) } + let(:nested_group) { create(:group, parent: group) } + let(:deep_nested_group) { create(:group, parent: nested_group) } + let(:very_deep_nested_group) { create(:group, parent: deep_nested_group) } + + it 'returns the correct ancestors' do + expect(very_deep_nested_group.self_and_ancestors).to contain_exactly(group, nested_group, deep_nested_group, very_deep_nested_group) + expect(deep_nested_group.self_and_ancestors).to contain_exactly(group, nested_group, deep_nested_group) + expect(nested_group.self_and_ancestors).to contain_exactly(group, nested_group) + expect(group.self_and_ancestors).to contain_exactly(group) + end + end + describe '#descendants', :nested_groups do let!(:group) { create(:group, path: 'git_lab') } let!(:nested_group) { create(:group, parent: group) } @@ -331,6 +345,22 @@ describe Namespace do end end + describe '#self_and_descendants', :nested_groups do + let!(:group) { create(:group, path: 'git_lab') } + let!(:nested_group) { create(:group, parent: group) } + let!(:deep_nested_group) { create(:group, parent: nested_group) } + let!(:very_deep_nested_group) { create(:group, parent: deep_nested_group) } + let!(:another_group) { create(:group, path: 'gitllab') } + let!(:another_group_nested) { create(:group, path: 'foo', parent: another_group) } + + it 'returns the correct descendants' do + expect(very_deep_nested_group.self_and_descendants).to contain_exactly(very_deep_nested_group) + expect(deep_nested_group.self_and_descendants).to contain_exactly(deep_nested_group, very_deep_nested_group) + expect(nested_group.self_and_descendants).to contain_exactly(nested_group, deep_nested_group, very_deep_nested_group) + expect(group.self_and_descendants).to contain_exactly(group, nested_group, deep_nested_group, very_deep_nested_group) + end + end + describe '#users_with_descendants', :nested_groups do let(:user_a) { create(:user) } let(:user_b) { create(:user) } diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index d9ab44dc49f..eba71ba2f72 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -2310,4 +2310,14 @@ describe Project do end end end + + describe '#forks_count' do + it 'returns the number of forks' do + project = build(:project) + + allow(project.forks).to receive(:count).and_return(1) + + expect(project.forks_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/requests/api/files_spec.rb b/spec/requests/api/files_spec.rb index 55c998b13b8..ea97c556430 100644 --- a/spec/requests/api/files_spec.rb +++ b/spec/requests/api/files_spec.rb @@ -33,6 +33,15 @@ describe API::Files do expect(Base64.decode64(json_response['content']).lines.first).to eq("require 'fileutils'\n") end + it 'returns json when file has txt extension' do + file_path = "bar%2Fbranch-test.txt" + + get api(route(file_path), current_user), params + + expect(response).to have_http_status(200) + expect(response.content_type).to eq('application/json') + end + it 'returns file by commit sha' do # This file is deleted on HEAD file_path = "files%2Fjs%2Fcommit%2Ejs%2Ecoffee" @@ -220,6 +229,7 @@ describe API::Files do post api(route("new_file_with_author%2Etxt"), user), valid_params expect(response).to have_http_status(201) + expect(response.content_type).to eq('application/json') last_commit = project.repository.commit.raw expect(last_commit.author_email).to eq(author_email) expect(last_commit.author_name).to eq(author_name) 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 9a6072e7eb7..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 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 6cb27d16fe5..a89a58ff713 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -1065,6 +1065,14 @@ describe API::Projects do expect(project_fork_target.forked?).to be_truthy end + it 'refreshes the forks count cachce' do + expect(project_fork_source.forks_count).to be_zero + + post api("/projects/#{project_fork_target.id}/fork/#{project_fork_source.id}", admin) + + expect(project_fork_source.forks_count).to eq(1) + end + it 'fails if forked_from project which does not exist' do post api("/projects/#{project_fork_target.id}/fork/9999", admin) expect(response).to have_http_status(404) diff --git a/spec/requests/api/protected_branches_spec.rb b/spec/requests/api/protected_branches_spec.rb index e4f9c47fb33..1aa8a95780e 100644 --- a/spec/requests/api/protected_branches_spec.rb +++ b/spec/requests/api/protected_branches_spec.rb @@ -96,7 +96,7 @@ describe API::ProtectedBranches do describe 'POST /projects/:id/protected_branches' do let(:branch_name) { 'new_branch' } - context 'when authenticated as a master' do + context 'when authenticated as a master' do before do project.add_master(user) end @@ -221,7 +221,7 @@ describe API::ProtectedBranches do context 'when branch has a wildcard in its name' do let(:protected_name) { 'feature*' } - + it "unprotects a wildcard branch" do delete api("/projects/#{project.id}/protected_branches/#{branch_name}", user) diff --git a/spec/requests/api/settings_spec.rb b/spec/requests/api/settings_spec.rb index c3ed5cd8ece..737c028ad53 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/merge_requests_spec.rb b/spec/requests/api/v3/merge_requests_spec.rb index ec684e7b9cd..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 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/projects_spec.rb b/spec/requests/api/v3/projects_spec.rb index fca5b5b5d82..a514166274a 100644 --- a/spec/requests/api/v3/projects_spec.rb +++ b/spec/requests/api/v3/projects_spec.rb @@ -1004,6 +1004,14 @@ describe API::V3::Projects do expect(project_fork_target.forked?).to be_truthy end + it 'refreshes the forks count cachce' do + expect(project_fork_source.forks_count).to be_zero + + post v3_api("/projects/#{project_fork_target.id}/fork/#{project_fork_source.id}", admin) + + expect(project_fork_source.forks_count).to eq(1) + end + it 'fails if forked_from project which does not exist' do post v3_api("/projects/#{project_fork_target.id}/fork/9999", admin) expect(response).to have_http_status(404) diff --git a/spec/requests/ci/api/builds_spec.rb b/spec/requests/ci/api/builds_spec.rb index ebd67eb1e94..7ccba4ba3ec 100644 --- a/spec/requests/ci/api/builds_spec.rb +++ b/spec/requests/ci/api/builds_spec.rb @@ -130,7 +130,7 @@ describe Ci::API::Builds do register_builds info: { platform: :darwin } expect(response).to have_http_status(201) - + expect(json_response["options"]).to be_empty end end 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/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/merge_requests/create_service_spec.rb b/spec/services/merge_requests/create_service_spec.rb index 8fef480274d..a1f3bec42cc 100644 --- a/spec/services/merge_requests/create_service_spec.rb +++ b/spec/services/merge_requests/create_service_spec.rb @@ -48,6 +48,16 @@ describe MergeRequests::CreateService do expect(Todo.where(attributes).count).to be_zero end + it 'creates exactly 1 create MR event' do + attributes = { + action: Event::CREATED, + target_id: @merge_request.id, + target_type: @merge_request.class.name + } + + expect(Event.where(attributes).count).to eq(1) + end + context 'when merge request is assigned to someone' do let(:opts) do { 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 64981c199e4..44b2d28d1d4 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -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 @@ -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 @@ -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/destroy_service_spec.rb b/spec/services/projects/destroy_service_spec.rb index 85b05ef6d05..c867139d1de 100644 --- a/spec/services/projects/destroy_service_spec.rb +++ b/spec/services/projects/destroy_service_spec.rb @@ -142,13 +142,13 @@ describe Projects::DestroyService do context 'when `execute` raises unexpected error' do before do expect_any_instance_of(Project) - .to receive(:destroy!).and_raise(Exception.new("Other error message")) + .to receive(:destroy!).and_raise(Exception.new('Other error message')) end it 'allows error to bubble up and rolls back project deletion' do expect do Sidekiq::Testing.inline! { destroy_project(project, user, {}) } - end.to raise_error + end.to raise_error(Exception, 'Other error message') expect(project.reload.pending_delete).to be(false) expect(project.delete_error).to include("Other error message") diff --git a/spec/services/projects/fork_service_spec.rb b/spec/services/projects/fork_service_spec.rb index c90536ba346..21c4b30734c 100644 --- a/spec/services/projects/fork_service_spec.rb +++ b/spec/services/projects/fork_service_spec.rb @@ -50,6 +50,14 @@ describe Projects::ForkService do expect(@from_project.avatar.file).to be_exists end + + it 'flushes the forks count cache of the source project' do + expect(@from_project.forks_count).to be_zero + + fork_project(@from_project, @to_user) + + expect(@from_project.forks_count).to eq(1) + end end end diff --git a/spec/services/projects/forks_count_service_spec.rb b/spec/services/projects/forks_count_service_spec.rb new file mode 100644 index 00000000000..cf299c5d09b --- /dev/null +++ b/spec/services/projects/forks_count_service_spec.rb @@ -0,0 +1,40 @@ +require 'spec_helper' + +describe Projects::ForksCountService do + let(:project) { build(:project, id: 42) } + let(:service) { described_class.new(project) } + + describe '#count' do + it 'returns the number of forks' do + allow(service).to receive(:uncached_count).and_return(1) + + expect(service.count).to eq(1) + end + + it 'caches the forks count', :use_clean_rails_memory_store_caching do + expect(service).to receive(:uncached_count).once.and_return(1) + + 2.times { service.count } + end + end + + describe '#refresh_cache', :use_clean_rails_memory_store_caching do + it 'refreshes the cache' do + expect(service).to receive(:uncached_count).once.and_return(1) + + service.refresh_cache + + expect(service.count).to eq(1) + end + end + + describe '#delete_cache', :use_clean_rails_memory_store_caching do + it 'removes the cache' do + expect(service).to receive(:uncached_count).twice.and_return(1) + + service.count + service.delete_cache + service.count + end + end +end diff --git a/spec/services/projects/unlink_fork_service_spec.rb b/spec/services/projects/unlink_fork_service_spec.rb index 2ae8d5f7c54..4f1ab697460 100644 --- a/spec/services/projects/unlink_fork_service_spec.rb +++ b/spec/services/projects/unlink_fork_service_spec.rb @@ -29,4 +29,14 @@ describe Projects::UnlinkForkService do subject.execute end + + it 'refreshes the forks count cache of the source project' do + source = fork_project.forked_from_project + + expect(source.forks_count).to eq(1) + + subject.execute + + expect(source.forks_count).to be_zero + end end diff --git a/spec/services/web_hook_service_spec.rb b/spec/services/web_hook_service_spec.rb index 365cb6b8f09..0726e135b20 100644 --- a/spec/services/web_hook_service_spec.rb +++ b/spec/services/web_hook_service_spec.rb @@ -144,7 +144,7 @@ describe WebHookService do describe '#async_execute' do let(:system_hook) { create(:system_hook) } - + it 'enqueue WebHookWorker' do expect(Sidekiq::Client).to receive(:enqueue).with(WebHookWorker, project_hook.id, data, 'push_hooks') 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 0ba6ed56314..3eea39d4bf4 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -8,6 +8,7 @@ require File.expand_path("../../config/environment", __FILE__) require 'rspec/rails' require 'shoulda/matchers' require 'rspec/retry' +require 'rspec-parameterized' rspec_profiling_is_configured = ENV['RSPEC_PROFILING_POSTGRES_URL'].present? || @@ -69,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 @@ -97,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/generate-seed-repo-rb b/spec/support/generate-seed-repo-rb index c89389b90ca..ef3c8e7087f 100755 --- a/spec/support/generate-seed-repo-rb +++ b/spec/support/generate-seed-repo-rb @@ -1,16 +1,16 @@ #!/usr/bin/env ruby -# +# # # generate-seed-repo-rb -# +# # This script generates the seed_repo.rb file used by lib/gitlab/git # tests. The seed_repo.rb file needs to be updated anytime there is a # Git push to https://gitlab.com/gitlab-org/gitlab-git-test. -# +# # Usage: -# +# # ./spec/support/generate-seed-repo-rb > spec/support/seed_repo.rb -# -# +# +# require 'erb' require 'tempfile' diff --git a/spec/support/gitlab-git-test.git/objects/3e/20715310a699808282e772720b9c04a0696bcc b/spec/support/gitlab-git-test.git/objects/3e/20715310a699808282e772720b9c04a0696bcc Binary files differnew file mode 100644 index 00000000000..86bf37ac887 --- /dev/null +++ b/spec/support/gitlab-git-test.git/objects/3e/20715310a699808282e772720b9c04a0696bcc diff --git a/spec/support/gitlab-git-test.git/objects/95/96bc54a6f0c0c98248fe97077eb5ccf48a98d0 b/spec/support/gitlab-git-test.git/objects/95/96bc54a6f0c0c98248fe97077eb5ccf48a98d0 new file mode 100644 index 00000000000..d90cb028e9b --- /dev/null +++ b/spec/support/gitlab-git-test.git/objects/95/96bc54a6f0c0c98248fe97077eb5ccf48a98d0 @@ -0,0 +1,2 @@ +xOn1 䜯 9&O "noYD6ՒҪ?j;wQ G rN(HPrArR7tpM#Mc NrsI +%p>۫pz?Y3XBB̰GB4
p?kv۞y~W])[a<CP_
\ No newline at end of file diff --git a/spec/support/gitlab-git-test.git/packed-refs b/spec/support/gitlab-git-test.git/packed-refs index ce5ab1f705b..507e4ce785a 100644 --- a/spec/support/gitlab-git-test.git/packed-refs +++ b/spec/support/gitlab-git-test.git/packed-refs @@ -8,6 +8,7 @@ 46e1395e609395de004cacd4b142865ab0e52a29 refs/heads/gitattributes-updated 4b4918a572fa86f9771e5ba40fbd48e1eb03e2c6 refs/heads/master 5937ac0a7beb003549fc5fd26fc247adbce4a52e refs/heads/merge-test +9596bc54a6f0c0c98248fe97077eb5ccf48a98d0 refs/heads/missing-gitmodules f4e6814c3e4e7a0de82a9e7cd20c626cc963a2f8 refs/tags/v1.0.0 ^6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 8a2a6eb295bb170b34c24c76c49ed0e9b2eaf34b refs/tags/v1.1.0 diff --git a/spec/support/gpg_helpers.rb b/spec/support/gpg_helpers.rb index 96ea6f28b30..65b38626a51 100644 --- a/spec/support/gpg_helpers.rb +++ b/spec/support/gpg_helpers.rb @@ -1,4 +1,6 @@ module GpgHelpers + SIGNED_COMMIT_SHA = '8a852d50dda17cc8fd1408d2fd0c5b0f24c76ca4'.freeze + module User1 extend self 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/matchers/access_matchers_for_controller.rb b/spec/support/matchers/access_matchers_for_controller.rb index ff60bd0c0ae..bb6b7c63ee9 100644 --- a/spec/support/matchers/access_matchers_for_controller.rb +++ b/spec/support/matchers/access_matchers_for_controller.rb @@ -1,6 +1,6 @@ # AccessMatchersForController # -# For testing authorize_xxx in controller. +# For testing authorize_xxx in controller. module AccessMatchersForController extend RSpec::Matchers::DSL include Warden::Test::Helpers diff --git a/spec/support/seed_repo.rb b/spec/support/seed_repo.rb index cfe7fc980a8..b4868e82cd7 100644 --- a/spec/support/seed_repo.rb +++ b/spec/support/seed_repo.rb @@ -97,6 +97,7 @@ module SeedRepo gitattributes-updated master merge-test + missing-gitmodules ].freeze TAGS = %w[ v1.0.0 diff --git a/spec/tasks/gitlab/gitaly_rake_spec.rb b/spec/tasks/gitlab/gitaly_rake_spec.rb index 43ac1a72152..1e9b20435ec 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  same ![banana_sample]'\ - '(/uploads/system/temp/secret55/banana_sample.gif)' + 'test  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 "\ - " same " + "test "\ + " same " ) end @@ -50,8 +50,8 @@ describe FileMover do expect(snippet.reload.description) .to eq( - "test "\ - " same " + "test "\ + " same " ) 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/commits/_commit.html.haml_spec.rb b/spec/views/projects/commits/_commit.html.haml_spec.rb new file mode 100644 index 00000000000..4c247361bd7 --- /dev/null +++ b/spec/views/projects/commits/_commit.html.haml_spec.rb @@ -0,0 +1,22 @@ +require 'spec_helper' + +describe 'projects/commits/_commit.html.haml' do + context 'with a singed commit' do + let(:project) { create(:project, :repository) } + let(:repository) { project.repository } + let(:ref) { GpgHelpers::SIGNED_COMMIT_SHA } + let(:commit) { repository.commit(ref) } + + it 'does not display a loading spinner for GPG status' do + render partial: 'projects/commits/commit', locals: { + project: project, + ref: ref, + commit: commit + } + + within '.gpg-status-box' do + expect(page).not_to have_css('i.fa.fa-spinner.fa-spin') + end + end + end +end 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 |