diff options
Diffstat (limited to 'spec')
54 files changed, 2757 insertions, 67 deletions
diff --git a/spec/controllers/import/github_controller_spec.rb b/spec/controllers/import/github_controller_spec.rb index ebfbf54182b..4f96567192d 100644 --- a/spec/controllers/import/github_controller_spec.rb +++ b/spec/controllers/import/github_controller_spec.rb @@ -124,8 +124,8 @@ describe Import::GithubController do context "when the GitHub user and GitLab user's usernames match" do it "takes the current user's namespace" do expect(Gitlab::GithubImport::ProjectCreator). - to receive(:new).with(github_repo, user.namespace, user, access_params). - and_return(double(execute: true)) + to receive(:new).with(github_repo, github_repo.name, user.namespace, user, access_params). + and_return(double(execute: true)) post :create, format: :js end @@ -136,8 +136,8 @@ describe Import::GithubController do it "takes the current user's namespace" do expect(Gitlab::GithubImport::ProjectCreator). - to receive(:new).with(github_repo, user.namespace, user, access_params). - and_return(double(execute: true)) + to receive(:new).with(github_repo, github_repo.name, user.namespace, user, access_params). + and_return(double(execute: true)) post :create, format: :js end @@ -158,8 +158,8 @@ describe Import::GithubController do context "when the namespace is owned by the GitLab user" do it "takes the existing namespace" do expect(Gitlab::GithubImport::ProjectCreator). - to receive(:new).with(github_repo, existing_namespace, user, access_params). - and_return(double(execute: true)) + to receive(:new).with(github_repo, github_repo.name, existing_namespace, user, access_params). + and_return(double(execute: true)) post :create, format: :js end @@ -171,9 +171,10 @@ describe Import::GithubController do existing_namespace.save end - it "doesn't create a project" do + it "creates a project using user's namespace" do expect(Gitlab::GithubImport::ProjectCreator). - not_to receive(:new) + to receive(:new).with(github_repo, github_repo.name, user.namespace, user, access_params). + and_return(double(execute: true)) post :create, format: :js end @@ -186,15 +187,15 @@ describe Import::GithubController do expect(Gitlab::GithubImport::ProjectCreator). to receive(:new).and_return(double(execute: true)) - expect { post :create, format: :js }.to change(Namespace, :count).by(1) + expect { post :create, target_namespace: github_repo.name, format: :js }.to change(Namespace, :count).by(1) end it "takes the new namespace" do expect(Gitlab::GithubImport::ProjectCreator). - to receive(:new).with(github_repo, an_instance_of(Group), user, access_params). + to receive(:new).with(github_repo, github_repo.name, an_instance_of(Group), user, access_params). and_return(double(execute: true)) - post :create, format: :js + post :create, target_namespace: github_repo.name, format: :js end end @@ -212,13 +213,34 @@ describe Import::GithubController do it "takes the current user's namespace" do expect(Gitlab::GithubImport::ProjectCreator). - to receive(:new).with(github_repo, user.namespace, user, access_params). + to receive(:new).with(github_repo, github_repo.name, user.namespace, user, access_params). and_return(double(execute: true)) post :create, format: :js end end end + + context 'user has chosen a namespace and name for the project' do + let(:test_namespace) { create(:namespace, name: 'test_namespace', owner: user) } + let(:test_name) { 'test_name' } + + it 'takes the selected namespace and name' do + expect(Gitlab::GithubImport::ProjectCreator). + to receive(:new).with(github_repo, test_name, test_namespace, user, access_params). + and_return(double(execute: true)) + + post :create, { target_namespace: test_namespace.name, new_name: test_name, format: :js } + end + + it 'takes the selected name and default namespace' do + expect(Gitlab::GithubImport::ProjectCreator). + to receive(:new).with(github_repo, test_name, user.namespace, user, access_params). + and_return(double(execute: true)) + + post :create, { new_name: test_name, format: :js } + end + end end end end diff --git a/spec/controllers/sent_notifications_controller_spec.rb b/spec/controllers/sent_notifications_controller_spec.rb index 9ced397bd4a..191e290a118 100644 --- a/spec/controllers/sent_notifications_controller_spec.rb +++ b/spec/controllers/sent_notifications_controller_spec.rb @@ -1,25 +1,108 @@ require 'rails_helper' describe SentNotificationsController, type: :controller do - let(:user) { create(:user) } - let(:issue) { create(:issue, author: user) } - let(:sent_notification) { create(:sent_notification, noteable: issue) } + let(:user) { create(:user) } + let(:project) { create(:empty_project) } + let(:sent_notification) { create(:sent_notification, noteable: issue, recipient: user) } - describe 'GET #unsubscribe' do - it 'returns a 404 when calling without existing id' do - get(:unsubscribe, id: '0' * 32) + let(:issue) do + create(:issue, project: project, author: user) do |issue| + issue.subscriptions.create(user: user, subscribed: true) + end + end + + describe 'GET unsubscribe' do + context 'when the user is not logged in' do + context 'when the force param is passed' do + before { get(:unsubscribe, id: sent_notification.reply_key, force: true) } + + it 'unsubscribes the user' do + expect(issue.subscribed?(user)).to be_falsey + end + + it 'sets the flash message' do + expect(controller).to set_flash[:notice].to(/unsubscribed/).now + end + + it 'redirects to the login page' do + expect(response).to redirect_to(new_user_session_path) + end + end + + context 'when the force param is not passed' do + before { get(:unsubscribe, id: sent_notification.reply_key) } + + it 'does not unsubscribe the user' do + expect(issue.subscribed?(user)).to be_truthy + end - expect(response.status).to be 404 + it 'does not set the flash message' do + expect(controller).not_to set_flash[:notice] + end + + it 'redirects to the login page' do + expect(response).to render_template :unsubscribe + end + end end - context 'calling with id' do - it 'shows a flash message to the user' do - get(:unsubscribe, id: sent_notification.reply_key) + context 'when the user is logged in' do + before { sign_in(user) } + + context 'when the ID passed does not exist' do + before { get(:unsubscribe, id: sent_notification.reply_key.reverse) } + + it 'does not unsubscribe the user' do + expect(issue.subscribed?(user)).to be_truthy + end + + it 'does not set the flash message' do + expect(controller).not_to set_flash[:notice] + end + + it 'returns a 404' do + expect(response).to have_http_status(:not_found) + end + end + + context 'when the force param is passed' do + before { get(:unsubscribe, id: sent_notification.reply_key, force: true) } + + it 'unsubscribes the user' do + expect(issue.subscribed?(user)).to be_falsey + end + + it 'sets the flash message' do + expect(controller).to set_flash[:notice].to(/unsubscribed/).now + end + + it 'redirects to the issue page' do + expect(response). + to redirect_to(namespace_project_issue_path(project.namespace, project, issue)) + end + end + + context 'when the force param is not passed' do + let(:merge_request) do + create(:merge_request, source_project: project, author: user) do |merge_request| + merge_request.subscriptions.create(user: user, subscribed: true) + end + end + let(:sent_notification) { create(:sent_notification, noteable: merge_request, recipient: user) } + before { get(:unsubscribe, id: sent_notification.reply_key) } + + it 'unsubscribes the user' do + expect(merge_request.subscribed?(user)).to be_falsey + end - expect(response.status).to be 302 + it 'sets the flash message' do + expect(controller).to set_flash[:notice].to(/unsubscribed/).now + end - expect(response).to redirect_to new_user_session_path - expect(controller).to set_flash[:notice].to(/unsubscribed/).now + it 'redirects to the merge request page' do + expect(response). + to redirect_to(namespace_project_merge_request_path(project.namespace, project, merge_request)) + end end end end diff --git a/spec/factories/deployments.rb b/spec/factories/deployments.rb index 82591604fcb..6f24bf58d14 100644 --- a/spec/factories/deployments.rb +++ b/spec/factories/deployments.rb @@ -3,11 +3,12 @@ FactoryGirl.define do sha '97de212e80737a608d939f648d959671fb0a0142' ref 'master' tag false + project nil environment factory: :environment after(:build) do |deployment, evaluator| - deployment.project = deployment.environment.project + deployment.project ||= deployment.environment.project end end end diff --git a/spec/features/dashboard_issues_spec.rb b/spec/features/dashboard_issues_spec.rb index 3fb1cb37544..fc914022a59 100644 --- a/spec/features/dashboard_issues_spec.rb +++ b/spec/features/dashboard_issues_spec.rb @@ -21,6 +21,9 @@ describe "Dashboard Issues filtering", feature: true, js: true do click_link 'No Milestone' + page.within '.issues-state-filters' do + expect(page).to have_selector('.active .badge', text: '1') + end expect(page).to have_selector('.issue', count: 1) end @@ -29,6 +32,9 @@ describe "Dashboard Issues filtering", feature: true, js: true do click_link 'Any Milestone' + page.within '.issues-state-filters' do + expect(page).to have_selector('.active .badge', text: '2') + end expect(page).to have_selector('.issue', count: 2) end @@ -39,6 +45,9 @@ describe "Dashboard Issues filtering", feature: true, js: true do click_link milestone.title end + page.within '.issues-state-filters' do + expect(page).to have_selector('.active .badge', text: '1') + end expect(page).to have_selector('.issue', count: 1) end end diff --git a/spec/features/issues/filter_issues_spec.rb b/spec/features/issues/filter_issues_spec.rb index 69fda27cc61..72f39e2fbca 100644 --- a/spec/features/issues/filter_issues_spec.rb +++ b/spec/features/issues/filter_issues_spec.rb @@ -206,7 +206,7 @@ describe 'Filter issues', feature: true do context 'only text', js: true do it 'filters issues by searched text' do - fill_in 'issue_search', with: 'Bug' + fill_in 'issuable_search', with: 'Bug' page.within '.issues-list' do expect(page).to have_selector('.issue', count: 2) @@ -214,7 +214,7 @@ describe 'Filter issues', feature: true do end it 'does not show any issues' do - fill_in 'issue_search', with: 'testing' + fill_in 'issuable_search', with: 'testing' page.within '.issues-list' do expect(page).not_to have_selector('.issue') @@ -224,12 +224,16 @@ describe 'Filter issues', feature: true do context 'text and dropdown options', js: true do it 'filters by text and label' do - fill_in 'issue_search', with: 'Bug' + fill_in 'issuable_search', with: 'Bug' page.within '.issues-list' do expect(page).to have_selector('.issue', count: 2) end + page.within '.issues-state-filters' do + expect(page).to have_selector('.active .badge', text: '2') + end + click_button 'Label' page.within '.labels-filter' do click_link 'bug' @@ -239,15 +243,23 @@ describe 'Filter issues', feature: true do page.within '.issues-list' do expect(page).to have_selector('.issue', count: 1) end + + page.within '.issues-state-filters' do + expect(page).to have_selector('.active .badge', text: '1') + end end it 'filters by text and milestone' do - fill_in 'issue_search', with: 'Bug' + fill_in 'issuable_search', with: 'Bug' page.within '.issues-list' do expect(page).to have_selector('.issue', count: 2) end + page.within '.issues-state-filters' do + expect(page).to have_selector('.active .badge', text: '2') + end + click_button 'Milestone' page.within '.milestone-filter' do click_link '8' @@ -256,15 +268,23 @@ describe 'Filter issues', feature: true do page.within '.issues-list' do expect(page).to have_selector('.issue', count: 1) end + + page.within '.issues-state-filters' do + expect(page).to have_selector('.active .badge', text: '1') + end end it 'filters by text and assignee' do - fill_in 'issue_search', with: 'Bug' + fill_in 'issuable_search', with: 'Bug' page.within '.issues-list' do expect(page).to have_selector('.issue', count: 2) end + page.within '.issues-state-filters' do + expect(page).to have_selector('.active .badge', text: '2') + end + click_button 'Assignee' page.within '.dropdown-menu-assignee' do click_link user.name @@ -273,15 +293,23 @@ describe 'Filter issues', feature: true do page.within '.issues-list' do expect(page).to have_selector('.issue', count: 1) end + + page.within '.issues-state-filters' do + expect(page).to have_selector('.active .badge', text: '1') + end end it 'filters by text and author' do - fill_in 'issue_search', with: 'Bug' + fill_in 'issuable_search', with: 'Bug' page.within '.issues-list' do expect(page).to have_selector('.issue', count: 2) end + page.within '.issues-state-filters' do + expect(page).to have_selector('.active .badge', text: '2') + end + click_button 'Author' page.within '.dropdown-menu-author' do click_link user.name @@ -290,6 +318,10 @@ describe 'Filter issues', feature: true do page.within '.issues-list' do expect(page).to have_selector('.issue', count: 1) end + + page.within '.issues-state-filters' do + expect(page).to have_selector('.active .badge', text: '1') + end end end end diff --git a/spec/features/issues/reset_filters_spec.rb b/spec/features/issues/reset_filters_spec.rb index 41f218eaa8b..f4d0f13c3d5 100644 --- a/spec/features/issues/reset_filters_spec.rb +++ b/spec/features/issues/reset_filters_spec.rb @@ -37,7 +37,7 @@ feature 'Issues filter reset button', feature: true, js: true do context 'when a text search has been conducted' do it 'resets the text search filter' do - visit_issues(project, issue_search: 'Bug') + visit_issues(project, search: 'Bug') expect(page).to have_css('.issue', count: 1) reset_filters @@ -67,7 +67,7 @@ feature 'Issues filter reset button', feature: true, js: true do context 'when all filters have been applied' do it 'resets all filters' do - visit_issues(project, assignee_id: user.id, author_id: user.id, milestone_title: milestone.title, label_name: bug.name, issue_search: 'Bug') + visit_issues(project, assignee_id: user.id, author_id: user.id, milestone_title: milestone.title, label_name: bug.name, search: 'Bug') expect(page).to have_css('.issue', count: 0) reset_filters diff --git a/spec/features/merge_requests/merge_request_versions_spec.rb b/spec/features/merge_requests/merge_request_versions_spec.rb index 9e759de3752..22d9e42119d 100644 --- a/spec/features/merge_requests/merge_request_versions_spec.rb +++ b/spec/features/merge_requests/merge_request_versions_spec.rb @@ -21,7 +21,7 @@ feature 'Merge Request versions', js: true, feature: true do describe 'switch between versions' do before do page.within '.mr-version-dropdown' do - find('.btn-link').click + find('.btn-default').click click_link 'version 1' end end @@ -42,12 +42,12 @@ feature 'Merge Request versions', js: true, feature: true do describe 'compare with older version' do before do page.within '.mr-version-compare-dropdown' do - find('.btn-link').click + find('.btn-default').click click_link 'version 1' end end - it 'should has correct value in the compare dropdown' do + it 'should have correct value in the compare dropdown' do page.within '.mr-version-compare-dropdown' do expect(page).to have_content 'version 1' end @@ -64,5 +64,13 @@ feature 'Merge Request versions', js: true, feature: true do it 'show diff between new and old version' do expect(page).to have_content '4 changed files with 15 additions and 6 deletions' end + + it 'should return to latest version when "Show latest version" button is clicked' do + click_link 'Show latest version' + page.within '.mr-version-dropdown' do + expect(page).to have_content 'latest version' + end + expect(page).to have_content '8 changed files' + end end end diff --git a/spec/features/projects/gfm_autocomplete_load_spec.rb b/spec/features/projects/gfm_autocomplete_load_spec.rb new file mode 100644 index 00000000000..1921ea6d8ae --- /dev/null +++ b/spec/features/projects/gfm_autocomplete_load_spec.rb @@ -0,0 +1,21 @@ +require 'spec_helper' + +describe 'GFM autocomplete loading', feature: true, js: true do + let(:project) { create(:project) } + + before do + login_as :admin + + visit namespace_project_path(project.namespace, project) + end + + it 'does not load on project#show' do + expect(evaluate_script('GitLab.GfmAutoComplete.dataSource')).to eq('') + end + + it 'loads on new issue page' do + visit new_namespace_project_issue_path(project.namespace, project) + + expect(evaluate_script('GitLab.GfmAutoComplete.dataSource')).not_to eq('') + end +end diff --git a/spec/features/projects/import_export/export_file_spec.rb b/spec/features/projects/import_export/export_file_spec.rb new file mode 100644 index 00000000000..7e2c701e401 --- /dev/null +++ b/spec/features/projects/import_export/export_file_spec.rb @@ -0,0 +1,80 @@ +require 'spec_helper' + +# Integration test that exports a file using the Import/Export feature +# It looks up for any sensitive word inside the JSON, so if a sensitive word is found +# we''l have to either include it adding the model that includes it to the +safe_list+ +# or make sure the attribute is blacklisted in the +import_export.yml+ configuration +feature 'Import/Export - project export integration test', feature: true, js: true do + include Select2Helper + include ExportFileHelper + + let(:user) { create(:admin) } + let(:export_path) { "#{Dir::tmpdir}/import_file_spec" } + let(:config_hash) { YAML.load_file(Gitlab::ImportExport.config_file).deep_stringify_keys } + + let(:sensitive_words) { %w[pass secret token key] } + let(:safe_list) do + { + token: [ProjectHook, Ci::Trigger], + key: [Project, Ci::Variable, :yaml_variables] + } + end + let(:safe_hashes) { { yaml_variables: %w[key value public] } } + + let(:project) { setup_project } + + background do + allow_any_instance_of(Gitlab::ImportExport).to receive(:storage_path).and_return(export_path) + end + + after do + FileUtils.rm_rf(export_path, secure: true) + end + + context 'admin user' do + before do + login_as(user) + end + + scenario 'exports a project successfully' do + visit edit_namespace_project_path(project.namespace, project) + + expect(page).to have_content('Export project') + + click_link 'Export project' + + visit edit_namespace_project_path(project.namespace, project) + + expect(page).to have_content('Download export') + + in_directory_with_expanded_export(project) do |exit_status, tmpdir| + expect(exit_status).to eq(0) + + project_json_path = File.join(tmpdir, 'project.json') + expect(File).to exist(project_json_path) + + project_hash = JSON.parse(IO.read(project_json_path)) + + sensitive_words.each do |sensitive_word| + found = find_sensitive_attributes(sensitive_word, project_hash) + + expect(found).to be_nil, failure_message(found.try(:key_found), found.try(:parent), sensitive_word) + end + end + end + + def failure_message(key_found, parent, sensitive_word) + <<-MSG + Found a new sensitive word <#{key_found}>, which is part of the hash #{parent.inspect} + + If you think this information shouldn't get exported, please exclude the model or attribute in IMPORT_EXPORT_CONFIG. + + Otherwise, please add the exception to +safe_list+ in CURRENT_SPEC using #{sensitive_word} as the key and the + correspondent hash or model as the value. + + IMPORT_EXPORT_CONFIG: #{Gitlab::ImportExport.config_file} + CURRENT_SPEC: #{__FILE__} + MSG + end + end +end diff --git a/spec/features/projects/import_export/import_file_spec.rb b/spec/features/projects/import_export/import_file_spec.rb index f707ccf4e93..09cd6369881 100644 --- a/spec/features/projects/import_export/import_file_spec.rb +++ b/spec/features/projects/import_export/import_file_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -feature 'project import', feature: true, js: true do +feature 'Import/Export - project import integration test', feature: true, js: true do include Select2Helper let(:admin) { create(:admin) } diff --git a/spec/features/unsubscribe_links_spec.rb b/spec/features/unsubscribe_links_spec.rb new file mode 100644 index 00000000000..cc40671787c --- /dev/null +++ b/spec/features/unsubscribe_links_spec.rb @@ -0,0 +1,75 @@ +require 'spec_helper' + +describe 'Unsubscribe links', feature: true do + include Warden::Test::Helpers + + let(:recipient) { create(:user) } + let(:author) { create(:user) } + let(:project) { create(:empty_project, :public) } + let(:params) { { title: 'A bug!', description: 'Fix it!', assignee: recipient } } + let(:issue) { Issues::CreateService.new(project, author, params).execute } + + let(:mail) { ActionMailer::Base.deliveries.last } + let(:body) { Capybara::Node::Simple.new(mail.default_part_body.to_s) } + let(:header_link) { mail.header['List-Unsubscribe'] } + let(:body_link) { body.find_link('unsubscribe')['href'] } + + before do + perform_enqueued_jobs { issue } + end + + context 'when logged out' do + context 'when visiting the link from the body' do + it 'shows the unsubscribe confirmation page and redirects to root path when confirming' do + visit body_link + + expect(current_path).to eq unsubscribe_sent_notification_path(SentNotification.last) + expect(page).to have_text(%(Unsubscribe from issue #{issue.title} (#{issue.to_reference}))) + expect(page).to have_text(%(Are you sure you want to unsubscribe from issue #{issue.title} (#{issue.to_reference})?)) + expect(issue.subscribed?(recipient)).to be_truthy + + click_link 'Unsubscribe' + + expect(issue.subscribed?(recipient)).to be_falsey + expect(current_path).to eq new_user_session_path + end + + it 'shows the unsubscribe confirmation page and redirects to root path when canceling' do + visit body_link + + expect(current_path).to eq unsubscribe_sent_notification_path(SentNotification.last) + expect(issue.subscribed?(recipient)).to be_truthy + + click_link 'Cancel' + + expect(issue.subscribed?(recipient)).to be_truthy + expect(current_path).to eq new_user_session_path + end + end + + it 'unsubscribes from the issue when visiting the link from the header' do + visit header_link + + expect(page).to have_text('unsubscribed') + expect(issue.subscribed?(recipient)).to be_falsey + end + end + + context 'when logged in' do + before { login_as(recipient) } + + it 'unsubscribes from the issue when visiting the link from the email body' do + visit body_link + + expect(page).to have_text('unsubscribed') + expect(issue.subscribed?(recipient)).to be_falsey + end + + it 'unsubscribes from the issue when visiting the link from the header' do + visit header_link + + expect(page).to have_text('unsubscribed') + expect(issue.subscribed?(recipient)).to be_falsey + end + end +end diff --git a/spec/finders/issues_finder_spec.rb b/spec/finders/issues_finder_spec.rb index ec8809e6926..40bccb8e50b 100644 --- a/spec/finders/issues_finder_spec.rb +++ b/spec/finders/issues_finder_spec.rb @@ -7,8 +7,8 @@ describe IssuesFinder do let(:project2) { create(:empty_project) } let(:milestone) { create(:milestone, project: project1) } let(:label) { create(:label, project: project2) } - let(:issue1) { create(:issue, author: user, assignee: user, project: project1, milestone: milestone) } - let(:issue2) { create(:issue, author: user, assignee: user, project: project2) } + let(:issue1) { create(:issue, author: user, assignee: user, project: project1, milestone: milestone, title: 'gitlab') } + let(:issue2) { create(:issue, author: user, assignee: user, project: project2, description: 'gitlab') } let(:issue3) { create(:issue, author: user2, assignee: user2, project: project2) } let!(:label_link) { create(:label_link, label: label, target: issue2) } @@ -127,6 +127,22 @@ describe IssuesFinder do end end + context 'filtering by issue term' do + let(:params) { { search: 'git' } } + + it 'returns issues with title and description match for search term' do + expect(issues).to contain_exactly(issue1, issue2) + end + end + + context 'filtering by issue iid' do + let(:params) { { search: issue3.to_reference } } + + it 'returns issue with iid match' do + expect(issues).to contain_exactly(issue3) + end + end + context 'when the user is unauthorized' do let(:search_user) { nil } diff --git a/spec/lib/ci/mask_secret_spec.rb b/spec/lib/ci/mask_secret_spec.rb index 518de76911c..3101bed20fb 100644 --- a/spec/lib/ci/mask_secret_spec.rb +++ b/spec/lib/ci/mask_secret_spec.rb @@ -5,15 +5,23 @@ describe Ci::MaskSecret, lib: true do describe '#mask' do it 'masks exact number of characters' do - expect(subject.mask('token', 'oke')).to eq('txxxn') + expect(mask('token', 'oke')).to eq('txxxn') end it 'masks multiple occurrences' do - expect(subject.mask('token token token', 'oke')).to eq('txxxn txxxn txxxn') + expect(mask('token token token', 'oke')).to eq('txxxn txxxn txxxn') end it 'does not mask if not found' do - expect(subject.mask('token', 'not')).to eq('token') + expect(mask('token', 'not')).to eq('token') + end + + it 'does support null token' do + expect(mask('token', nil)).to eq('token') + end + + def mask(value, token) + subject.mask!(value.dup, token) end end end diff --git a/spec/lib/gitlab/git_access_spec.rb b/spec/lib/gitlab/git_access_spec.rb index ed43646330f..de68e32e5b4 100644 --- a/spec/lib/gitlab/git_access_spec.rb +++ b/spec/lib/gitlab/git_access_spec.rb @@ -343,7 +343,7 @@ describe Gitlab::GitAccess, lib: true do end context 'to private project' do - let(:project) { create(:project, :internal) } + let(:project) { create(:project) } it { expect(subject).not_to be_allowed } end diff --git a/spec/lib/gitlab/git_spec.rb b/spec/lib/gitlab/git_spec.rb new file mode 100644 index 00000000000..219198eff60 --- /dev/null +++ b/spec/lib/gitlab/git_spec.rb @@ -0,0 +1,45 @@ +require 'spec_helper' + +describe Gitlab::Git, lib: true do + let(:committer_email) { FFaker::Internet.email } + + # I have to remove periods from the end of the name + # This happened when the user's name had a suffix (i.e. "Sr.") + # This seems to be what git does under the hood. For example, this commit: + # + # $ git commit --author='Foo Sr. <foo@example.com>' -m 'Where's my trailing period?' + # + # results in this: + # + # $ git show --pretty + # ... + # Author: Foo Sr <foo@example.com> + # ... + let(:committer_name) { FFaker::Name.name.chomp("\.") } + + describe 'committer_hash' do + it "returns a hash containing the given email and name" do + committer_hash = Gitlab::Git::committer_hash(email: committer_email, name: committer_name) + + expect(committer_hash[:email]).to eq(committer_email) + expect(committer_hash[:name]).to eq(committer_name) + expect(committer_hash[:time]).to be_a(Time) + end + + context 'when email is nil' do + it "returns nil" do + committer_hash = Gitlab::Git::committer_hash(email: nil, name: committer_name) + + expect(committer_hash).to be_nil + end + end + + context 'when name is nil' do + it "returns nil" do + committer_hash = Gitlab::Git::committer_hash(email: committer_email, name: nil) + + expect(committer_hash).to be_nil + end + end + end +end diff --git a/spec/lib/gitlab/github_import/project_creator_spec.rb b/spec/lib/gitlab/github_import/project_creator_spec.rb index 014ee462e5c..ab06b7bc5bb 100644 --- a/spec/lib/gitlab/github_import/project_creator_spec.rb +++ b/spec/lib/gitlab/github_import/project_creator_spec.rb @@ -13,7 +13,7 @@ describe Gitlab::GithubImport::ProjectCreator, lib: true do ) end - subject(:service) { described_class.new(repo, namespace, user, github_access_token: 'asdffg') } + subject(:service) { described_class.new(repo, repo.name, namespace, user, github_access_token: 'asdffg') } before do namespace.add_owner(user) diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml new file mode 100644 index 00000000000..30968ba2d5f --- /dev/null +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -0,0 +1,175 @@ +--- +issues: +- subscriptions +- award_emoji +- author +- assignee +- updated_by +- milestone +- notes +- label_links +- labels +- todos +- user_agent_detail +- moved_to +- events +events: +- author +- project +- target +notes: +- award_emoji +- project +- noteable +- author +- updated_by +- resolved_by +- todos +- events +label_links: +- target +- label +label: +- subscriptions +- project +- lists +- label_links +- issues +- merge_requests +milestone: +- project +- issues +- labels +- merge_requests +- participants +- events +snippets: +- author +- project +- notes +releases: +- project +project_members: +- created_by +- user +- source +- project +merge_requests: +- subscriptions +- award_emoji +- author +- assignee +- updated_by +- milestone +- notes +- label_links +- labels +- todos +- target_project +- source_project +- merge_user +- merge_request_diffs +- merge_request_diff +- events +merge_request_diff: +- merge_request +pipelines: +- project +- user +- statuses +- builds +- trigger_requests +statuses: +- project +- pipeline +- user +variables: +- project +triggers: +- project +- trigger_requests +deploy_keys: +- user +- deploy_keys_projects +- projects +services: +- project +- service_hook +hooks: +- project +protected_branches: +- project +- merge_access_levels +- push_access_levels +project: +- taggings +- base_tags +- tag_taggings +- tags +- creator +- group +- namespace +- board +- last_event +- services +- campfire_service +- drone_ci_service +- emails_on_push_service +- builds_email_service +- irker_service +- pivotaltracker_service +- hipchat_service +- flowdock_service +- assembla_service +- asana_service +- gemnasium_service +- slack_service +- buildkite_service +- bamboo_service +- teamcity_service +- pushover_service +- jira_service +- redmine_service +- custom_issue_tracker_service +- bugzilla_service +- gitlab_issue_tracker_service +- external_wiki_service +- forked_project_link +- forked_from_project +- forked_project_links +- forks +- merge_requests +- fork_merge_requests +- issues +- labels +- events +- milestones +- notes +- snippets +- hooks +- protected_branches +- project_members +- users +- requesters +- deploy_keys_projects +- deploy_keys +- users_star_projects +- starrers +- releases +- lfs_objects_projects +- lfs_objects +- project_group_links +- invited_groups +- todos +- notification_settings +- import_data +- commit_statuses +- pipelines +- builds +- runner_projects +- runners +- variables +- triggers +- environments +- deployments +- project_feature
\ No newline at end of file diff --git a/spec/lib/gitlab/import_export/attribute_configuration_spec.rb b/spec/lib/gitlab/import_export/attribute_configuration_spec.rb new file mode 100644 index 00000000000..2ba344092ce --- /dev/null +++ b/spec/lib/gitlab/import_export/attribute_configuration_spec.rb @@ -0,0 +1,55 @@ +require 'spec_helper' + +# Part of the test security suite for the Import/Export feature +# Checks whether there are new attributes in models that are currently being exported as part of the +# project Import/Export feature. +# If there are new attributes, these will have to either be added to this spec in case we want them +# to be included as part of the export, or blacklist them using the import_export.yml configuration file. +# Likewise, new models added to import_export.yml, will need to be added with their correspondent attributes +# to this spec. +describe 'Import/Export attribute configuration', lib: true do + include ConfigurationHelper + + let(:config_hash) { YAML.load_file(Gitlab::ImportExport.config_file).deep_stringify_keys } + let(:relation_names) do + names = names_from_tree(config_hash['project_tree']) + + # Remove duplicated or add missing models + # - project is not part of the tree, so it has to be added manually. + # - milestone, labels have both singular and plural versions in the tree, so remove the duplicates. + names.flatten.uniq - ['milestones', 'labels'] + ['project'] + end + + let(:safe_attributes_file) { 'spec/lib/gitlab/import_export/safe_model_attributes.yml' } + let(:safe_model_attributes) { YAML.load_file(safe_attributes_file) } + + it 'has no new columns' do + relation_names.each do |relation_name| + relation_class = relation_class_for_name(relation_name) + + expect(safe_model_attributes[relation_class.to_s]).not_to be_nil, "Expected exported class #{relation_class.to_s} to exist in safe_model_attributes" + + current_attributes = parsed_attributes(relation_name, relation_class.attribute_names) + safe_attributes = safe_model_attributes[relation_class.to_s] + new_attributes = current_attributes - safe_attributes + + expect(new_attributes).to be_empty, failure_message(relation_class.to_s, new_attributes) + end + end + + def failure_message(relation_class, new_attributes) + <<-MSG + It looks like #{relation_class}, which is exported using the project Import/Export, has new attributes: #{new_attributes.join(',')} + + Please add the attribute(s) to SAFE_MODEL_ATTRIBUTES if you consider this can be exported. + Otherwise, please blacklist the attribute(s) in IMPORT_EXPORT_CONFIG by adding it to its correspondent + model in the +excluded_attributes+ section. + + SAFE_MODEL_ATTRIBUTES: #{File.expand_path(safe_attributes_file)} + IMPORT_EXPORT_CONFIG: #{Gitlab::ImportExport.config_file} + MSG + end + + class Author < User + end +end diff --git a/spec/lib/gitlab/import_export/model_configuration_spec.rb b/spec/lib/gitlab/import_export/model_configuration_spec.rb new file mode 100644 index 00000000000..9b492d1b9c7 --- /dev/null +++ b/spec/lib/gitlab/import_export/model_configuration_spec.rb @@ -0,0 +1,57 @@ +require 'spec_helper' + +# Part of the test security suite for the Import/Export feature +# Finds if a new model has been added that can potentially be part of the Import/Export +# If it finds a new model, it will show a +failure_message+ with the options available. +describe 'Import/Export model configuration', lib: true do + include ConfigurationHelper + + let(:config_hash) { YAML.load_file(Gitlab::ImportExport.config_file).deep_stringify_keys } + let(:model_names) do + names = names_from_tree(config_hash['project_tree']) + + # Remove duplicated or add missing models + # - project is not part of the tree, so it has to be added manually. + # - milestone, labels have both singular and plural versions in the tree, so remove the duplicates. + # - User, Author... Models we do not care about for checking models + names.flatten.uniq - ['milestones', 'labels', 'user', 'author'] + ['project'] + end + + let(:all_models_yml) { 'spec/lib/gitlab/import_export/all_models.yml' } + let(:all_models) { YAML.load_file(all_models_yml) } + let(:current_models) { setup_models } + + it 'has no new models' do + model_names.each do |model_name| + new_models = Array(current_models[model_name]) - Array(all_models[model_name]) + expect(new_models).to be_empty, failure_message(model_name.classify, new_models) + end + end + + # List of current models between models, in the format of + # {model: [model_2, model3], ...} + def setup_models + all_models_hash = {} + + model_names.each do |model_name| + model_class = relation_class_for_name(model_name) + + all_models_hash[model_name] = associations_for(model_class) - ['project'] + end + + all_models_hash + end + + def failure_message(parent_model_name, new_models) + <<-MSG + New model(s) <#{new_models.join(',')}> have been added, related to #{parent_model_name}, which is exported by + the Import/Export feature. + + If you think this model should be included in the export, please add it to IMPORT_EXPORT_CONFIG. + Definitely add it to MODELS_JSON to signal that you've handled this error and to prevent it from showing up in the future. + + MODELS_JSON: #{File.expand_path(all_models_yml)} + IMPORT_EXPORT_CONFIG: #{Gitlab::ImportExport.config_file} + MSG + end +end diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml new file mode 100644 index 00000000000..f2d272ca7e2 --- /dev/null +++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml @@ -0,0 +1,310 @@ +--- +Issue: +- id +- title +- assignee_id +- author_id +- project_id +- created_at +- updated_at +- position +- branch_name +- description +- state +- iid +- updated_by_id +- confidential +- deleted_at +- due_date +- moved_to_id +- lock_version +- milestone_id +- weight +Event: +- id +- target_type +- target_id +- title +- data +- project_id +- created_at +- updated_at +- action +- author_id +Note: +- id +- note +- noteable_type +- author_id +- created_at +- updated_at +- project_id +- attachment +- line_code +- commit_id +- noteable_id +- system +- st_diff +- updated_by_id +- type +- position +- original_position +- resolved_at +- resolved_by_id +- discussion_id +- original_discussion_id +LabelLink: +- id +- label_id +- target_id +- target_type +- created_at +- updated_at +Label: +- id +- title +- color +- project_id +- created_at +- updated_at +- template +- description +- priority +Milestone: +- id +- title +- project_id +- description +- due_date +- created_at +- updated_at +- state +- iid +ProjectSnippet: +- id +- title +- content +- author_id +- project_id +- created_at +- updated_at +- file_name +- type +- visibility_level +Release: +- id +- tag +- description +- project_id +- created_at +- updated_at +ProjectMember: +- id +- access_level +- source_id +- source_type +- user_id +- notification_level +- type +- created_at +- updated_at +- created_by_id +- invite_email +- invite_token +- invite_accepted_at +- requested_at +- expires_at +User: +- id +- username +- email +MergeRequest: +- id +- target_branch +- source_branch +- source_project_id +- author_id +- assignee_id +- title +- created_at +- updated_at +- state +- merge_status +- target_project_id +- iid +- description +- position +- locked_at +- updated_by_id +- merge_error +- merge_params +- merge_when_build_succeeds +- merge_user_id +- merge_commit_sha +- deleted_at +- in_progress_merge_commit_sha +- lock_version +- milestone_id +- approvals_before_merge +- rebase_commit_sha +MergeRequestDiff: +- id +- state +- st_commits +- merge_request_id +- created_at +- updated_at +- base_commit_sha +- real_size +- head_commit_sha +- start_commit_sha +Ci::Pipeline: +- id +- project_id +- ref +- sha +- before_sha +- push_data +- created_at +- updated_at +- tag +- yaml_errors +- committed_at +- gl_project_id +- status +- started_at +- finished_at +- duration +- user_id +CommitStatus: +- id +- project_id +- status +- finished_at +- trace +- created_at +- updated_at +- started_at +- runner_id +- coverage +- commit_id +- commands +- job_id +- name +- deploy +- options +- allow_failure +- stage +- trigger_request_id +- stage_idx +- tag +- ref +- user_id +- type +- target_url +- description +- artifacts_file +- gl_project_id +- artifacts_metadata +- erased_by_id +- erased_at +- artifacts_expire_at +- environment +- artifacts_size +- when +- yaml_variables +- queued_at +Ci::Variable: +- id +- project_id +- key +- value +- encrypted_value +- encrypted_value_salt +- encrypted_value_iv +- gl_project_id +Ci::Trigger: +- id +- token +- project_id +- deleted_at +- created_at +- updated_at +- gl_project_id +DeployKey: +- id +- user_id +- created_at +- updated_at +- key +- title +- type +- fingerprint +- public +Service: +- id +- type +- title +- project_id +- created_at +- updated_at +- active +- properties +- template +- push_events +- issues_events +- merge_requests_events +- tag_push_events +- note_events +- pipeline_events +- build_events +- category +- default +- wiki_page_events +- confidential_issues_events +ProjectHook: +- id +- url +- project_id +- created_at +- updated_at +- type +- service_id +- push_events +- issues_events +- merge_requests_events +- tag_push_events +- note_events +- pipeline_events +- enable_ssl_verification +- build_events +- wiki_page_events +- token +- group_id +- confidential_issues_events +ProtectedBranch: +- id +- project_id +- name +- created_at +- updated_at +Project: +- description +- issues_enabled +- merge_requests_enabled +- wiki_enabled +- snippets_enabled +- visibility_level +- archived +Author: +- name +ProjectFeature: +- id +- project_id +- merge_requests_access_level +- issues_access_level +- wiki_access_level +- snippets_access_level +- builds_access_level +- created_at +- updated_at
\ No newline at end of file diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb index eae9c060c38..0363bc74939 100644 --- a/spec/mailers/notify_spec.rb +++ b/spec/mailers/notify_spec.rb @@ -861,7 +861,7 @@ describe Notify do subject { Notify.repository_push_email(project.id, author_id: user.id, ref: 'refs/heads/master', action: :create) } it_behaves_like 'it should not have Gmail Actions links' - it_behaves_like "a user cannot unsubscribe through footer link" + it_behaves_like 'a user cannot unsubscribe through footer link' it_behaves_like 'an email with X-GitLab headers containing project details' it_behaves_like 'an email that contains a header with author username' @@ -914,7 +914,7 @@ describe Notify do subject { Notify.repository_push_email(project.id, author_id: user.id, ref: 'refs/heads/master', action: :delete) } it_behaves_like 'it should not have Gmail Actions links' - it_behaves_like "a user cannot unsubscribe through footer link" + it_behaves_like 'a user cannot unsubscribe through footer link' it_behaves_like 'an email with X-GitLab headers containing project details' it_behaves_like 'an email that contains a header with author username' @@ -936,7 +936,7 @@ describe Notify do subject { Notify.repository_push_email(project.id, author_id: user.id, ref: 'refs/tags/v1.0', action: :delete) } it_behaves_like 'it should not have Gmail Actions links' - it_behaves_like "a user cannot unsubscribe through footer link" + it_behaves_like 'a user cannot unsubscribe through footer link' it_behaves_like 'an email with X-GitLab headers containing project details' it_behaves_like 'an email that contains a header with author username' @@ -964,7 +964,7 @@ describe Notify do subject { Notify.repository_push_email(project.id, author_id: user.id, ref: 'refs/heads/master', action: :push, compare: compare, reverse_compare: false, diff_refs: diff_refs, send_from_committer_email: send_from_committer_email) } it_behaves_like 'it should not have Gmail Actions links' - it_behaves_like "a user cannot unsubscribe through footer link" + it_behaves_like 'a user cannot unsubscribe through footer link' it_behaves_like 'an email with X-GitLab headers containing project details' it_behaves_like 'an email that contains a header with author username' @@ -1066,7 +1066,7 @@ describe Notify do subject { Notify.repository_push_email(project.id, author_id: user.id, ref: 'refs/heads/master', action: :push, compare: compare, diff_refs: diff_refs) } it_behaves_like 'it should show Gmail Actions View Commit link' - it_behaves_like "a user cannot unsubscribe through footer link" + it_behaves_like 'a user cannot unsubscribe through footer link' it_behaves_like 'an email with X-GitLab headers containing project details' it_behaves_like 'an email that contains a header with author username' diff --git a/spec/mailers/shared/notify.rb b/spec/mailers/shared/notify.rb index 93de5850ba2..56872da9a8f 100644 --- a/spec/mailers/shared/notify.rb +++ b/spec/mailers/shared/notify.rb @@ -169,10 +169,18 @@ shared_examples 'it should show Gmail Actions View Commit link' do end shared_examples 'an unsubscribeable thread' do + it 'has a List-Unsubscribe header' do + is_expected.to have_header 'List-Unsubscribe', /unsubscribe/ + end + it { is_expected.to have_body_text /unsubscribe/ } end shared_examples 'a user cannot unsubscribe through footer link' do + it 'does not have a List-Unsubscribe header' do + is_expected.not_to have_header 'List-Unsubscribe', /unsubscribe/ + end + it { is_expected.not_to have_body_text /unsubscribe/ } end diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index f1857f846dc..550a890797e 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -187,6 +187,37 @@ describe Ci::Pipeline, models: true do end end + describe "merge request metrics" do + let(:project) { FactoryGirl.create :project } + let(:pipeline) { FactoryGirl.create(:ci_empty_pipeline, status: 'created', project: project, ref: 'master', sha: project.repository.commit('master').id) } + let!(:merge_request) { create(:merge_request, source_project: project, source_branch: pipeline.ref) } + + context 'when transitioning to running' do + it 'records the build start time' do + time = Time.now + Timecop.freeze(time) { build.run } + + expect(merge_request.reload.metrics.latest_build_started_at).to be_within(1.second).of(time) + end + + it 'clears the build end time' do + build.run + + expect(merge_request.reload.metrics.latest_build_finished_at).to be_nil + end + end + + context 'when transitioning to success' do + it 'records the build end time' do + build.run + time = Time.now + Timecop.freeze(time) { build.success } + + expect(merge_request.reload.metrics.latest_build_finished_at).to be_within(1.second).of(time) + end + end + end + def create_build(name, queued_at = current, started_from = 0) create(:ci_build, name: name, @@ -468,4 +499,28 @@ describe Ci::Pipeline, models: true do stage_idx: stage_idx) end end + + describe "#merge_requests" do + let(:project) { FactoryGirl.create :project } + let(:pipeline) { FactoryGirl.create(:ci_empty_pipeline, status: 'created', project: project, ref: 'master', sha: project.repository.commit('master').id) } + + it "returns merge requests whose `diff_head_sha` matches the pipeline's SHA" do + merge_request = create(:merge_request, source_project: project, source_branch: pipeline.ref) + + expect(pipeline.merge_requests).to eq([merge_request]) + end + + it "doesn't return merge requests whose source branch doesn't match the pipeline's ref" do + create(:merge_request, source_project: project, source_branch: 'feature', target_branch: 'master') + + expect(pipeline.merge_requests).to be_empty + end + + it "doesn't return merge requests whose `diff_head_sha` doesn't match the pipeline's SHA" do + create(:merge_request, source_project: project, source_branch: pipeline.ref) + allow_any_instance_of(MergeRequest).to receive(:diff_head_sha) { '97de212e80737a608d939f648d959671fb0a0142b' } + + expect(pipeline.merge_requests).to be_empty + end + end end diff --git a/spec/models/cycle_analytics/code_spec.rb b/spec/models/cycle_analytics/code_spec.rb new file mode 100644 index 00000000000..b9381e33914 --- /dev/null +++ b/spec/models/cycle_analytics/code_spec.rb @@ -0,0 +1,42 @@ +require 'spec_helper' + +describe 'CycleAnalytics#code', feature: true do + extend CycleAnalyticsHelpers::TestGeneration + + let(:project) { create(:project) } + let(:from_date) { 10.days.ago } + let(:user) { create(:user, :admin) } + subject { CycleAnalytics.new(project, from: from_date) } + + generate_cycle_analytics_spec( + phase: :code, + data_fn: -> (context) { { issue: context.create(:issue, project: context.project) } }, + start_time_conditions: [["issue mentioned in a commit", + -> (context, data) do + context.create_commit_referencing_issue(data[:issue]) + end]], + end_time_conditions: [["merge request that closes issue is created", + -> (context, data) do + context.create_merge_request_closing_issue(data[:issue]) + end]], + post_fn: -> (context, data) do + context.merge_merge_requests_closing_issue(data[:issue]) + context.deploy_master + end) + + context "when a regular merge request (that doesn't close the issue) is created" do + it "returns nil" do + 5.times do + issue = create(:issue, project: project) + + create_commit_referencing_issue(issue) + create_merge_request_closing_issue(issue, message: "Closes nothing") + + merge_merge_requests_closing_issue(issue) + deploy_master + end + + expect(subject.code).to be_nil + end + end +end diff --git a/spec/models/cycle_analytics/issue_spec.rb b/spec/models/cycle_analytics/issue_spec.rb new file mode 100644 index 00000000000..e9cc71254ab --- /dev/null +++ b/spec/models/cycle_analytics/issue_spec.rb @@ -0,0 +1,50 @@ +require 'spec_helper' + +describe 'CycleAnalytics#issue', models: true do + extend CycleAnalyticsHelpers::TestGeneration + + let(:project) { create(:project) } + let(:from_date) { 10.days.ago } + let(:user) { create(:user, :admin) } + subject { CycleAnalytics.new(project, from: from_date) } + + generate_cycle_analytics_spec( + phase: :issue, + data_fn: -> (context) { { issue: context.build(:issue, project: context.project) } }, + start_time_conditions: [["issue created", -> (context, data) { data[:issue].save }]], + end_time_conditions: [["issue associated with a milestone", + -> (context, data) do + if data[:issue].persisted? + data[:issue].update(milestone: context.create(:milestone, project: context.project)) + end + end], + ["list label added to issue", + -> (context, data) do + if data[:issue].persisted? + data[:issue].update(label_ids: [context.create(:label, lists: [context.create(:list)]).id]) + end + end]], + post_fn: -> (context, data) do + if data[:issue].persisted? + context.create_merge_request_closing_issue(data[:issue].reload) + context.merge_merge_requests_closing_issue(data[:issue]) + context.deploy_master + end + end) + + context "when a regular label (instead of a list label) is added to the issue" do + it "returns nil" do + 5.times do + regular_label = create(:label) + issue = create(:issue, project: project) + issue.update(label_ids: [regular_label.id]) + + create_merge_request_closing_issue(issue) + merge_merge_requests_closing_issue(issue) + deploy_master + end + + expect(subject.issue).to be_nil + end + end +end diff --git a/spec/models/cycle_analytics/plan_spec.rb b/spec/models/cycle_analytics/plan_spec.rb new file mode 100644 index 00000000000..5b8c96dc992 --- /dev/null +++ b/spec/models/cycle_analytics/plan_spec.rb @@ -0,0 +1,52 @@ +require 'spec_helper' + +describe 'CycleAnalytics#plan', feature: true do + extend CycleAnalyticsHelpers::TestGeneration + + let(:project) { create(:project) } + let(:from_date) { 10.days.ago } + let(:user) { create(:user, :admin) } + subject { CycleAnalytics.new(project, from: from_date) } + + generate_cycle_analytics_spec( + phase: :plan, + data_fn: -> (context) do + { + issue: context.create(:issue, project: context.project), + branch_name: context.random_git_name + } + end, + start_time_conditions: [["issue associated with a milestone", + -> (context, data) do + data[:issue].update(milestone: context.create(:milestone, project: context.project)) + end], + ["list label added to issue", + -> (context, data) do + data[:issue].update(label_ids: [context.create(:label, lists: [context.create(:list)]).id]) + end]], + end_time_conditions: [["issue mentioned in a commit", + -> (context, data) do + context.create_commit_referencing_issue(data[:issue], branch_name: data[:branch_name]) + end]], + post_fn: -> (context, data) do + context.create_merge_request_closing_issue(data[:issue], source_branch: data[:branch_name]) + context.merge_merge_requests_closing_issue(data[:issue]) + context.deploy_master + end) + + context "when a regular label (instead of a list label) is added to the issue" do + it "returns nil" do + branch_name = random_git_name + label = create(:label) + issue = create(:issue, project: project) + issue.update(label_ids: [label.id]) + create_commit_referencing_issue(issue, branch_name: branch_name) + + create_merge_request_closing_issue(issue, source_branch: branch_name) + merge_merge_requests_closing_issue(issue) + deploy_master + + expect(subject.issue).to be_nil + end + end +end diff --git a/spec/models/cycle_analytics/production_spec.rb b/spec/models/cycle_analytics/production_spec.rb new file mode 100644 index 00000000000..1f5e5cab92d --- /dev/null +++ b/spec/models/cycle_analytics/production_spec.rb @@ -0,0 +1,54 @@ +require 'spec_helper' + +describe 'CycleAnalytics#production', feature: true do + extend CycleAnalyticsHelpers::TestGeneration + + let(:project) { create(:project) } + let(:from_date) { 10.days.ago } + let(:user) { create(:user, :admin) } + subject { CycleAnalytics.new(project, from: from_date) } + + generate_cycle_analytics_spec( + phase: :production, + data_fn: -> (context) { { issue: context.build(:issue, project: context.project) } }, + start_time_conditions: [["issue is created", -> (context, data) { data[:issue].save }]], + before_end_fn: lambda do |context, data| + context.create_merge_request_closing_issue(data[:issue]) + context.merge_merge_requests_closing_issue(data[:issue]) + end, + end_time_conditions: + [["merge request that closes issue is deployed to production", -> (context, data) { context.deploy_master }], + ["production deploy happens after merge request is merged (along with other changes)", + lambda do |context, data| + # Make other changes on master + sha = context.project.repository.commit_file(context.user, context.random_git_name, "content", "commit message", 'master', false) + context.project.repository.commit(sha) + + context.deploy_master + end]]) + + context "when a regular merge request (that doesn't close the issue) is merged and deployed" do + it "returns nil" do + 5.times do + merge_request = create(:merge_request) + MergeRequests::MergeService.new(project, user).execute(merge_request) + deploy_master + end + + expect(subject.production).to be_nil + end + end + + context "when the deployment happens to a non-production environment" do + it "returns nil" do + 5.times do + issue = create(:issue, project: project) + merge_request = create_merge_request_closing_issue(issue) + MergeRequests::MergeService.new(project, user).execute(merge_request) + deploy_master(environment: 'staging') + end + + expect(subject.production).to be_nil + end + end +end diff --git a/spec/models/cycle_analytics/review_spec.rb b/spec/models/cycle_analytics/review_spec.rb new file mode 100644 index 00000000000..b6e26d8f261 --- /dev/null +++ b/spec/models/cycle_analytics/review_spec.rb @@ -0,0 +1,35 @@ +require 'spec_helper' + +describe 'CycleAnalytics#review', feature: true do + extend CycleAnalyticsHelpers::TestGeneration + + let(:project) { create(:project) } + let(:from_date) { 10.days.ago } + let(:user) { create(:user, :admin) } + subject { CycleAnalytics.new(project, from: from_date) } + + generate_cycle_analytics_spec( + phase: :review, + data_fn: -> (context) { { issue: context.create(:issue, project: context.project) } }, + start_time_conditions: [["merge request that closes issue is created", + -> (context, data) do + context.create_merge_request_closing_issue(data[:issue]) + end]], + end_time_conditions: [["merge request that closes issue is merged", + -> (context, data) do + context.merge_merge_requests_closing_issue(data[:issue]) + end]], + post_fn: -> (context, data) { context.deploy_master }) + + context "when a regular merge request (that doesn't close the issue) is created and merged" do + it "returns nil" do + 5.times do + MergeRequests::MergeService.new(project, user).execute(create(:merge_request)) + + deploy_master + end + + expect(subject.review).to be_nil + end + end +end diff --git a/spec/models/cycle_analytics/staging_spec.rb b/spec/models/cycle_analytics/staging_spec.rb new file mode 100644 index 00000000000..af1c4477ddb --- /dev/null +++ b/spec/models/cycle_analytics/staging_spec.rb @@ -0,0 +1,64 @@ +require 'spec_helper' + +describe 'CycleAnalytics#staging', feature: true do + extend CycleAnalyticsHelpers::TestGeneration + + let(:project) { create(:project) } + let(:from_date) { 10.days.ago } + let(:user) { create(:user, :admin) } + subject { CycleAnalytics.new(project, from: from_date) } + + generate_cycle_analytics_spec( + phase: :staging, + data_fn: lambda do |context| + issue = context.create(:issue, project: context.project) + { issue: issue, merge_request: context.create_merge_request_closing_issue(issue) } + end, + start_time_conditions: [["merge request that closes issue is merged", + -> (context, data) do + context.merge_merge_requests_closing_issue(data[:issue]) + end ]], + end_time_conditions: [["merge request that closes issue is deployed to production", + -> (context, data) do + context.deploy_master + end], + ["production deploy happens after merge request is merged (along with other changes)", + lambda do |context, data| + # Make other changes on master + sha = context.project.repository.commit_file( + context.user, + context.random_git_name, + "content", + "commit message", + 'master', + false) + context.project.repository.commit(sha) + + context.deploy_master + end]]) + + context "when a regular merge request (that doesn't close the issue) is merged and deployed" do + it "returns nil" do + 5.times do + merge_request = create(:merge_request) + MergeRequests::MergeService.new(project, user).execute(merge_request) + deploy_master + end + + expect(subject.staging).to be_nil + end + end + + context "when the deployment happens to a non-production environment" do + it "returns nil" do + 5.times do + issue = create(:issue, project: project) + merge_request = create_merge_request_closing_issue(issue) + MergeRequests::MergeService.new(project, user).execute(merge_request) + deploy_master(environment: 'staging') + end + + expect(subject.staging).to be_nil + end + end +end diff --git a/spec/models/cycle_analytics/summary_spec.rb b/spec/models/cycle_analytics/summary_spec.rb new file mode 100644 index 00000000000..743bc2da33f --- /dev/null +++ b/spec/models/cycle_analytics/summary_spec.rb @@ -0,0 +1,53 @@ +require 'spec_helper' + +describe CycleAnalytics::Summary, models: true do + let(:project) { create(:project) } + let(:from) { Time.now } + let(:user) { create(:user, :admin) } + subject { described_class.new(project, from: from) } + + describe "#new_issues" do + it "finds the number of issues created after the 'from date'" do + Timecop.freeze(5.days.ago) { create(:issue, project: project) } + Timecop.freeze(5.days.from_now) { create(:issue, project: project) } + + expect(subject.new_issues).to eq(1) + end + + it "doesn't find issues from other projects" do + Timecop.freeze(5.days.from_now) { create(:issue, project: create(:project)) } + + expect(subject.new_issues).to eq(0) + end + end + + describe "#commits" do + it "finds the number of commits created after the 'from date'" do + Timecop.freeze(5.days.ago) { create_commit("Test message", project, user, 'master') } + Timecop.freeze(5.days.from_now) { create_commit("Test message", project, user, 'master') } + + expect(subject.commits).to eq(1) + end + + it "doesn't find commits from other projects" do + Timecop.freeze(5.days.from_now) { create_commit("Test message", create(:project), user, 'master') } + + expect(subject.commits).to eq(0) + end + end + + describe "#deploys" do + it "finds the number of deploys made created after the 'from date'" do + Timecop.freeze(5.days.ago) { create(:deployment, project: project) } + Timecop.freeze(5.days.from_now) { create(:deployment, project: project) } + + expect(subject.deploys).to eq(1) + end + + it "doesn't find commits from other projects" do + Timecop.freeze(5.days.from_now) { create(:deployment, project: create(:project)) } + + expect(subject.deploys).to eq(0) + end + end +end diff --git a/spec/models/cycle_analytics/test_spec.rb b/spec/models/cycle_analytics/test_spec.rb new file mode 100644 index 00000000000..89ace0b2742 --- /dev/null +++ b/spec/models/cycle_analytics/test_spec.rb @@ -0,0 +1,94 @@ +require 'spec_helper' + +describe 'CycleAnalytics#test', feature: true do + extend CycleAnalyticsHelpers::TestGeneration + + let(:project) { create(:project) } + let(:from_date) { 10.days.ago } + let(:user) { create(:user, :admin) } + subject { CycleAnalytics.new(project, from: from_date) } + + generate_cycle_analytics_spec( + phase: :test, + data_fn: lambda do |context| + issue = context.create(:issue, project: context.project) + merge_request = context.create_merge_request_closing_issue(issue) + pipeline = context.create(:ci_pipeline, ref: merge_request.source_branch, sha: merge_request.diff_head_sha, project: context.project) + { pipeline: pipeline, issue: issue } + end, + start_time_conditions: [["pipeline is started", -> (context, data) { data[:pipeline].run! }]], + end_time_conditions: [["pipeline is finished", -> (context, data) { data[:pipeline].succeed! }]], + post_fn: -> (context, data) do + context.merge_merge_requests_closing_issue(data[:issue]) + context.deploy_master + end) + + context "when the pipeline is for a regular merge request (that doesn't close an issue)" do + it "returns nil" do + 5.times do + issue = create(:issue, project: project) + merge_request = create_merge_request_closing_issue(issue) + pipeline = create(:ci_pipeline, ref: "refs/heads/#{merge_request.source_branch}", sha: merge_request.diff_head_sha) + + pipeline.run! + pipeline.succeed! + + merge_merge_requests_closing_issue(issue) + deploy_master + end + + expect(subject.test).to be_nil + end + end + + context "when the pipeline is not for a merge request" do + it "returns nil" do + 5.times do + pipeline = create(:ci_pipeline, ref: "refs/heads/master", sha: project.repository.commit('master').sha) + + pipeline.run! + pipeline.succeed! + + deploy_master + end + + expect(subject.test).to be_nil + end + end + + context "when the pipeline is dropped (failed)" do + it "returns nil" do + 5.times do + issue = create(:issue, project: project) + merge_request = create_merge_request_closing_issue(issue) + pipeline = create(:ci_pipeline, ref: "refs/heads/#{merge_request.source_branch}", sha: merge_request.diff_head_sha) + + pipeline.run! + pipeline.drop! + + merge_merge_requests_closing_issue(issue) + deploy_master + end + + expect(subject.test).to be_nil + end + end + + context "when the pipeline is cancelled" do + it "returns nil" do + 5.times do + issue = create(:issue, project: project) + merge_request = create_merge_request_closing_issue(issue) + pipeline = create(:ci_pipeline, ref: "refs/heads/#{merge_request.source_branch}", sha: merge_request.diff_head_sha) + + pipeline.run! + pipeline.cancel! + + merge_merge_requests_closing_issue(issue) + deploy_master + end + + expect(subject.test).to be_nil + end + end +end diff --git a/spec/models/issue/metrics_spec.rb b/spec/models/issue/metrics_spec.rb new file mode 100644 index 00000000000..e170b087ebc --- /dev/null +++ b/spec/models/issue/metrics_spec.rb @@ -0,0 +1,55 @@ +require 'spec_helper' + +describe Issue::Metrics, models: true do + let(:project) { create(:project) } + + subject { create(:issue, project: project) } + + describe "when recording the default set of issue metrics on issue save" do + context "milestones" do + it "records the first time an issue is associated with a milestone" do + time = Time.now + Timecop.freeze(time) { subject.update(milestone: create(:milestone)) } + metrics = subject.metrics + + expect(metrics).to be_present + expect(metrics.first_associated_with_milestone_at).to be_within(1.second).of(time) + end + + it "does not record the second time an issue is associated with a milestone" do + time = Time.now + Timecop.freeze(time) { subject.update(milestone: create(:milestone)) } + Timecop.freeze(time + 2.hours) { subject.update(milestone: nil) } + Timecop.freeze(time + 6.hours) { subject.update(milestone: create(:milestone)) } + metrics = subject.metrics + + expect(metrics).to be_present + expect(metrics.first_associated_with_milestone_at).to be_within(1.second).of(time) + end + end + + context "list labels" do + it "records the first time an issue is associated with a list label" do + list_label = create(:label, lists: [create(:list)]) + time = Time.now + Timecop.freeze(time) { subject.update(label_ids: [list_label.id]) } + metrics = subject.metrics + + expect(metrics).to be_present + expect(metrics.first_added_to_board_at).to be_within(1.second).of(time) + end + + it "does not record the second time an issue is associated with a list label" do + time = Time.now + first_list_label = create(:label, lists: [create(:list)]) + Timecop.freeze(time) { subject.update(label_ids: [first_list_label.id]) } + second_list_label = create(:label, lists: [create(:list)]) + Timecop.freeze(time + 5.hours) { subject.update(label_ids: [second_list_label.id]) } + metrics = subject.metrics + + expect(metrics).to be_present + expect(metrics.first_added_to_board_at).to be_within(1.second).of(time) + end + end + end +end diff --git a/spec/models/merge_request/metrics_spec.rb b/spec/models/merge_request/metrics_spec.rb new file mode 100644 index 00000000000..a79dd215d41 --- /dev/null +++ b/spec/models/merge_request/metrics_spec.rb @@ -0,0 +1,18 @@ +require 'spec_helper' + +describe MergeRequest::Metrics, models: true do + let(:project) { create(:project) } + + subject { create(:merge_request, source_project: project) } + + describe "when recording the default set of metrics on merge request save" do + it "records the merge time" do + time = Time.now + Timecop.freeze(time) { subject.mark_as_merged } + metrics = subject.metrics + + expect(metrics).to be_present + expect(metrics.merged_at).to be_within(1.second).of(time) + end + end +end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 06feeb1bbba..433aba7747b 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -495,15 +495,62 @@ describe MergeRequest, models: true do end describe '#all_pipelines' do - let!(:pipelines) do - subject.merge_request_diff.commits.map do |commit| - create(:ci_empty_pipeline, project: subject.source_project, sha: commit.id, ref: subject.source_branch) + shared_examples 'returning pipelines with proper ordering' do + let!(:all_pipelines) do + subject.all_commits_sha.map do |sha| + create(:ci_empty_pipeline, + project: subject.source_project, + sha: sha, + ref: subject.source_branch) + end + end + + it 'returns all pipelines' do + expect(subject.all_pipelines).not_to be_empty + expect(subject.all_pipelines).to eq(all_pipelines.reverse) + end + end + + context 'with single merge_request_diffs' do + it_behaves_like 'returning pipelines with proper ordering' + end + + context 'with multiple irrelevant merge_request_diffs' do + before do + subject.update(target_branch: 'markdown') + end + + it_behaves_like 'returning pipelines with proper ordering' + end + + context 'with unsaved merge request' do + subject { build(:merge_request) } + + let!(:pipeline) do + create(:ci_empty_pipeline, + project: subject.project, + sha: subject.diff_head_sha, + ref: subject.source_branch) + end + + it 'returns pipelines from diff_head_sha' do + expect(subject.all_pipelines).to contain_exactly(pipeline) end end + end + + describe '#all_commits_sha' do + let(:all_commits_sha) do + subject.merge_request_diffs.flat_map(&:commits).map(&:sha).uniq + end + + before do + subject.update(target_branch: 'markdown') + end - it 'returns a pipelines from source projects with proper ordering' do - expect(subject.all_pipelines).not_to be_empty - expect(subject.all_pipelines).to eq(pipelines.reverse) + it 'returns all SHA from all merge_request_diffs' do + expect(subject.merge_request_diffs.size).to eq(2) + expect(subject.all_commits_sha).to eq(all_commits_sha) end end @@ -701,16 +748,57 @@ describe MergeRequest, models: true do end end - describe "#environments" do + describe '#environments' do let(:project) { create(:project) } let(:merge_request) { create(:merge_request, source_project: project) } - it 'selects deployed environments' do - environments = create_list(:environment, 3, project: project) - create(:deployment, environment: environments.first, sha: project.commit('master').id) - create(:deployment, environment: environments.second, sha: project.commit('feature').id) + context 'with multiple environments' do + let(:environments) { create_list(:environment, 3, project: project) } - expect(merge_request.environments).to eq [environments.first] + before do + create(:deployment, environment: environments.first, ref: 'master', sha: project.commit('master').id) + create(:deployment, environment: environments.second, ref: 'feature', sha: project.commit('feature').id) + end + + it 'selects deployed environments' do + expect(merge_request.environments).to contain_exactly(environments.first) + end + end + + context 'with environments on source project' do + let(:source_project) do + create(:project) do |fork_project| + fork_project.create_forked_project_link(forked_to_project_id: fork_project.id, forked_from_project_id: project.id) + end + end + + let(:merge_request) do + create(:merge_request, + source_project: source_project, source_branch: 'feature', + target_project: project) + end + + let(:source_environment) { create(:environment, project: source_project) } + + before do + create(:deployment, environment: source_environment, ref: 'feature', sha: merge_request.diff_head_sha) + end + + it 'selects deployed environments' do + expect(merge_request.environments).to contain_exactly(source_environment) + end + + context 'with environments on target project' do + let(:target_environment) { create(:environment, project: project) } + + before do + create(:deployment, environment: target_environment, tag: true, sha: merge_request.diff_head_sha) + end + + it 'selects deployed environments' do + expect(merge_request.environments).to contain_exactly(source_environment, target_environment) + end + end end context 'without a diff_head_commit' do diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index a388ff703a6..83f61f0af0a 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -1647,6 +1647,47 @@ describe Project, models: true do end end + describe '#environments_for' do + let(:project) { create(:project) } + let(:environment) { create(:environment, project: project) } + + context 'tagged deployment' do + before do + create(:deployment, environment: environment, ref: '1.0', tag: true, sha: project.commit.id) + end + + it 'returns environment when with_tags is set' do + expect(project.environments_for('master', project.commit, with_tags: true)).to contain_exactly(environment) + end + + it 'does not return environment when no with_tags is set' do + expect(project.environments_for('master', project.commit)).to be_empty + end + + it 'does not return environment when commit is not part of deployment' do + expect(project.environments_for('master', project.commit('feature'))).to be_empty + end + end + + context 'branch deployment' do + before do + create(:deployment, environment: environment, ref: 'master', sha: project.commit.id) + end + + it 'returns environment when ref is set' do + expect(project.environments_for('master', project.commit)).to contain_exactly(environment) + end + + it 'does not environment when ref is different' do + expect(project.environments_for('feature', project.commit)).to be_empty + end + + it 'does not return environment when commit is not part of deployment' do + expect(project.environments_for('master', project.commit('feature'))).to be_empty + end + end + end + def enable_lfs allow(Gitlab.config.lfs).to receive(:enabled).and_return(true) end diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 94681004c96..db29f4d353b 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -16,6 +16,21 @@ describe Repository, models: true do merge_commit_id = repository.merge(user, merge_request, commit_options) repository.commit(merge_commit_id) end + let(:author_email) { FFaker::Internet.email } + + # I have to remove periods from the end of the name + # This happened when the user's name had a suffix (i.e. "Sr.") + # This seems to be what git does under the hood. For example, this commit: + # + # $ git commit --author='Foo Sr. <foo@example.com>' -m 'Where's my trailing period?' + # + # results in this: + # + # $ git show --pretty + # ... + # Author: Foo Sr <foo@example.com> + # ... + let(:author_name) { FFaker::Name.name.chomp("\.") } describe '#branch_names_contains' do subject { repository.branch_names_contains(sample_commit.id) } @@ -132,7 +147,31 @@ describe Repository, models: true do end end - describe :commit_file do + describe "#commit_dir" do + it "commits a change that creates a new directory" do + expect do + repository.commit_dir(user, 'newdir', 'Create newdir', 'master') + end.to change { repository.commits('master').count }.by(1) + + newdir = repository.tree('master', 'newdir') + expect(newdir.path).to eq('newdir') + end + + context "when an author is specified" do + it "uses the given email/name to set the commit's author" do + expect do + repository.commit_dir(user, "newdir", "Add newdir", 'master', author_email: author_email, author_name: author_name) + end.to change { repository.commits('master').count }.by(1) + + last_commit = repository.commit + + expect(last_commit.author_email).to eq(author_email) + expect(last_commit.author_name).to eq(author_name) + end + end + end + + describe "#commit_file" do it 'commits change to a file successfully' do expect do repository.commit_file(user, 'CHANGELOG', 'Changelog!', @@ -144,9 +183,23 @@ describe Repository, models: true do expect(blob.data).to eq('Changelog!') end + + context "when an author is specified" do + it "uses the given email/name to set the commit's author" do + expect do + repository.commit_file(user, "README", 'README!', 'Add README', + 'master', true, author_email: author_email, author_name: author_name) + end.to change { repository.commits('master').count }.by(1) + + last_commit = repository.commit + + expect(last_commit.author_email).to eq(author_email) + expect(last_commit.author_name).to eq(author_name) + end + end end - describe :update_file do + describe "#update_file" do it 'updates filename successfully' do expect do repository.update_file(user, 'NEWLICENSE', 'Copyright!', @@ -160,6 +213,85 @@ describe Repository, models: true do expect(files).not_to include('LICENSE') expect(files).to include('NEWLICENSE') end + + context "when an author is specified" do + it "uses the given email/name to set the commit's author" do + repository.commit_file(user, "README", 'README!', 'Add README', 'master', true) + + expect do + repository.update_file(user, 'README', "Updated README!", + branch: 'master', + previous_path: 'README', + message: 'Update README', + author_email: author_email, + author_name: author_name) + end.to change { repository.commits('master').count }.by(1) + + last_commit = repository.commit + + expect(last_commit.author_email).to eq(author_email) + expect(last_commit.author_name).to eq(author_name) + end + end + end + + describe "#remove_file" do + it 'removes file successfully' do + repository.commit_file(user, "README", 'README!', 'Add README', 'master', true) + + expect do + repository.remove_file(user, "README", "Remove README", 'master') + end.to change { repository.commits('master').count }.by(1) + + expect(repository.blob_at('master', 'README')).to be_nil + end + + context "when an author is specified" do + it "uses the given email/name to set the commit's author" do + repository.commit_file(user, "README", 'README!', 'Add README', 'master', true) + + expect do + repository.remove_file(user, "README", "Remove README", 'master', author_email: author_email, author_name: author_name) + end.to change { repository.commits('master').count }.by(1) + + last_commit = repository.commit + + expect(last_commit.author_email).to eq(author_email) + expect(last_commit.author_name).to eq(author_name) + end + end + end + + describe '#get_committer_and_author' do + it 'returns the committer and author data' do + options = repository.get_committer_and_author(user) + expect(options[:committer][:email]).to eq(user.email) + expect(options[:author][:email]).to eq(user.email) + end + + context 'when the email/name are given' do + it 'returns an object containing the email/name' do + options = repository.get_committer_and_author(user, email: author_email, name: author_name) + expect(options[:author][:email]).to eq(author_email) + expect(options[:author][:name]).to eq(author_name) + end + end + + context 'when the email is given but the name is not' do + it 'returns the committer as the author' do + options = repository.get_committer_and_author(user, email: author_email) + expect(options[:author][:email]).to eq(user.email) + expect(options[:author][:name]).to eq(user.name) + end + end + + context 'when the name is given but the email is not' do + it 'returns nil' do + options = repository.get_committer_and_author(user, name: author_name) + expect(options[:author][:email]).to eq(user.email) + expect(options[:author][:name]).to eq(user.name) + end + end end describe "search_files" do diff --git a/spec/policies/project_policy_spec.rb b/spec/policies/project_policy_spec.rb index eda1cafd65e..a7a06744428 100644 --- a/spec/policies/project_policy_spec.rb +++ b/spec/policies/project_policy_spec.rb @@ -33,4 +33,17 @@ describe ProjectPolicy, models: true do it 'returns increasing permissions for each level' do expect(users_permissions).to eq(users_permissions.sort.uniq) end + + it 'does not include the read_issue permission when the issue author is not a member of the private project' do + project = create(:project, :private) + issue = create(:issue, project: project) + user = issue.author + + expect(project.team.member?(issue.author)).to eq(false) + + expect(BasePolicy.class_for(project).abilities(user, project).can_set). + not_to include(:read_issue) + + expect(Ability.allowed?(user, :read_issue, project)).to be_falsy + end end diff --git a/spec/requests/api/api_helpers_spec.rb b/spec/requests/api/api_helpers_spec.rb index bbdf8f03c2b..e66faeed705 100644 --- a/spec/requests/api/api_helpers_spec.rb +++ b/spec/requests/api/api_helpers_spec.rb @@ -36,11 +36,36 @@ describe API::Helpers, api: true do params.delete(API::Helpers::SUDO_PARAM) end + def warden_authenticate_returns(value) + warden = double("warden", authenticate: value) + env['warden'] = warden + end + + def doorkeeper_guard_returns(value) + allow_any_instance_of(self.class).to receive(:doorkeeper_guard){ value } + end + def error!(message, status) raise Exception end describe ".current_user" do + subject { current_user } + + describe "when authenticating via Warden" do + before { doorkeeper_guard_returns false } + + context "fails" do + it { is_expected.to be_nil } + end + + context "succeeds" do + before { warden_authenticate_returns user } + + it { is_expected.to eq(user) } + end + end + describe "when authenticating using a user's private token" do it "returns nil for an invalid token" do env[API::Helpers::PRIVATE_TOKEN_HEADER] = 'invalid token' diff --git a/spec/requests/api/files_spec.rb b/spec/requests/api/files_spec.rb index 2d1213df8a7..050d0dd082d 100644 --- a/spec/requests/api/files_spec.rb +++ b/spec/requests/api/files_spec.rb @@ -5,6 +5,21 @@ describe API::API, api: true do let(:user) { create(:user) } let!(:project) { create(:project, namespace: user.namespace ) } let(:file_path) { 'files/ruby/popen.rb' } + let(:author_email) { FFaker::Internet.email } + + # I have to remove periods from the end of the name + # This happened when the user's name had a suffix (i.e. "Sr.") + # This seems to be what git does under the hood. For example, this commit: + # + # $ git commit --author='Foo Sr. <foo@example.com>' -m 'Where's my trailing period?' + # + # results in this: + # + # $ git show --pretty + # ... + # Author: Foo Sr <foo@example.com> + # ... + let(:author_name) { FFaker::Name.name.chomp("\.") } before { project.team << [user, :developer] } @@ -16,6 +31,7 @@ describe API::API, api: true do } get api("/projects/#{project.id}/repository/files", user), params + expect(response).to have_http_status(200) expect(json_response['file_path']).to eq(file_path) expect(json_response['file_name']).to eq('popen.rb') @@ -25,6 +41,7 @@ describe API::API, api: true do it "returns a 400 bad request if no params given" do get api("/projects/#{project.id}/repository/files", user) + expect(response).to have_http_status(400) end @@ -35,6 +52,7 @@ describe API::API, api: true do } get api("/projects/#{project.id}/repository/files", user), params + expect(response).to have_http_status(404) end end @@ -51,12 +69,17 @@ describe API::API, api: true do it "creates a new file in project repo" do post api("/projects/#{project.id}/repository/files", user), valid_params + expect(response).to have_http_status(201) expect(json_response['file_path']).to eq('newfile.rb') + last_commit = project.repository.commit.raw + expect(last_commit.author_email).to eq(user.email) + expect(last_commit.author_name).to eq(user.name) end it "returns a 400 bad request if no params given" do post api("/projects/#{project.id}/repository/files", user) + expect(response).to have_http_status(400) end @@ -65,8 +88,22 @@ describe API::API, api: true do and_return(false) post api("/projects/#{project.id}/repository/files", user), valid_params + expect(response).to have_http_status(400) end + + context "when specifying an author" do + it "creates a new file with the specified author" do + valid_params.merge!(author_email: author_email, author_name: author_name) + + post api("/projects/#{project.id}/repository/files", user), valid_params + + expect(response).to have_http_status(201) + last_commit = project.repository.commit.raw + expect(last_commit.author_email).to eq(author_email) + expect(last_commit.author_name).to eq(author_name) + end + end end describe "PUT /projects/:id/repository/files" do @@ -81,14 +118,32 @@ describe API::API, api: true do it "updates existing file in project repo" do put api("/projects/#{project.id}/repository/files", user), valid_params + expect(response).to have_http_status(200) expect(json_response['file_path']).to eq(file_path) + last_commit = project.repository.commit.raw + expect(last_commit.author_email).to eq(user.email) + expect(last_commit.author_name).to eq(user.name) end it "returns a 400 bad request if no params given" do put api("/projects/#{project.id}/repository/files", user) + expect(response).to have_http_status(400) end + + context "when specifying an author" do + it "updates a file with the specified author" do + valid_params.merge!(author_email: author_email, author_name: author_name, content: "New content") + + put api("/projects/#{project.id}/repository/files", user), valid_params + + expect(response).to have_http_status(200) + last_commit = project.repository.commit.raw + expect(last_commit.author_email).to eq(author_email) + expect(last_commit.author_name).to eq(author_name) + end + end end describe "DELETE /projects/:id/repository/files" do @@ -102,12 +157,17 @@ describe API::API, api: true do it "deletes existing file in project repo" do delete api("/projects/#{project.id}/repository/files", user), valid_params + expect(response).to have_http_status(200) expect(json_response['file_path']).to eq(file_path) + last_commit = project.repository.commit.raw + expect(last_commit.author_email).to eq(user.email) + expect(last_commit.author_name).to eq(user.name) end it "returns a 400 bad request if no params given" do delete api("/projects/#{project.id}/repository/files", user) + expect(response).to have_http_status(400) end @@ -115,8 +175,22 @@ describe API::API, api: true do allow_any_instance_of(Repository).to receive(:remove_file).and_return(false) delete api("/projects/#{project.id}/repository/files", user), valid_params + expect(response).to have_http_status(400) end + + context "when specifying an author" do + it "removes a file with the specified author" do + valid_params.merge!(author_email: author_email, author_name: author_name) + + delete api("/projects/#{project.id}/repository/files", user), valid_params + + expect(response).to have_http_status(200) + last_commit = project.repository.commit.raw + expect(last_commit.author_email).to eq(author_email) + expect(last_commit.author_name).to eq(author_name) + end + end end describe "POST /projects/:id/repository/files with binary file" do @@ -143,6 +217,7 @@ describe API::API, api: true do it "remains unchanged" do get api("/projects/#{project.id}/repository/files", user), get_params + expect(response).to have_http_status(200) expect(json_response['file_path']).to eq(file_path) expect(json_response['file_name']).to eq(file_path) diff --git a/spec/requests/git_http_spec.rb b/spec/requests/git_http_spec.rb index e3922bec689..74516686921 100644 --- a/spec/requests/git_http_spec.rb +++ b/spec/requests/git_http_spec.rb @@ -335,7 +335,7 @@ describe 'Git HTTP requests', lib: true do project.team << [user, :reporter] end - shared_examples 'can download code only from own projects' do + shared_examples 'can download code only' do it 'downloads get status 200' do clone_get "#{project.path_with_namespace}.git", user: 'gitlab-ci-token', password: build.token @@ -353,7 +353,7 @@ describe 'Git HTTP requests', lib: true do context 'administrator' do let(:user) { create(:admin) } - it_behaves_like 'can download code only from own projects' + it_behaves_like 'can download code only' it 'downloads from other project get status 403' do clone_get "#{other_project.path_with_namespace}.git", user: 'gitlab-ci-token', password: build.token @@ -365,7 +365,7 @@ describe 'Git HTTP requests', lib: true do context 'regular user' do let(:user) { create(:user) } - it_behaves_like 'can download code only from own projects' + it_behaves_like 'can download code only' it 'downloads from other project get status 404' do clone_get "#{other_project.path_with_namespace}.git", user: 'gitlab-ci-token', password: build.token diff --git a/spec/services/create_deployment_service_spec.rb b/spec/services/create_deployment_service_spec.rb index 41b897f36cd..343b4385bf2 100644 --- a/spec/services/create_deployment_service_spec.rb +++ b/spec/services/create_deployment_service_spec.rb @@ -169,4 +169,83 @@ describe CreateDeploymentService, services: true do end end end + + describe "merge request metrics" do + let(:params) do + { + environment: 'production', + ref: 'master', + tag: false, + sha: '97de212e80737a608d939f648d959671fb0a0142b', + } + end + + let(:merge_request) { create(:merge_request, target_branch: 'master', source_branch: 'feature', source_project: project) } + + context "while updating the 'first_deployed_to_production_at' time" do + before { merge_request.mark_as_merged } + + context "for merge requests merged before the current deploy" do + it "sets the time if the deploy's environment is 'production'" do + time = Time.now + Timecop.freeze(time) { service.execute } + + expect(merge_request.reload.metrics.first_deployed_to_production_at).to be_within(1.second).of(time) + end + + it "doesn't set the time if the deploy's environment is not 'production'" do + staging_params = params.merge(environment: 'staging') + service = described_class.new(project, user, staging_params) + service.execute + + expect(merge_request.reload.metrics.first_deployed_to_production_at).to be_nil + end + + it 'does not raise errors if the merge request does not have a metrics record' do + merge_request.metrics.destroy + + expect(merge_request.reload.metrics).to be_nil + expect { service.execute }.not_to raise_error + end + end + + context "for merge requests merged before the previous deploy" do + context "if the 'first_deployed_to_production_at' time is already set" do + it "does not overwrite the older 'first_deployed_to_production_at' time" do + # Previous deploy + time = Time.now + Timecop.freeze(time) { service.execute } + + expect(merge_request.reload.metrics.first_deployed_to_production_at).to be_within(1.second).of(time) + + # Current deploy + service = described_class.new(project, user, params) + Timecop.freeze(time + 12.hours) { service.execute } + + expect(merge_request.reload.metrics.first_deployed_to_production_at).to be_within(1.second).of(time) + end + end + + context "if the 'first_deployed_to_production_at' time is not already set" do + it "does not overwrite the older 'first_deployed_to_production_at' time" do + # Previous deploy + time = 5.minutes.from_now + Timecop.freeze(time) { service.execute } + + expect(merge_request.reload.metrics.merged_at).to be < merge_request.reload.metrics.first_deployed_to_production_at + + merge_request.reload.metrics.update(first_deployed_to_production_at: nil) + + expect(merge_request.reload.metrics.first_deployed_to_production_at).to be_nil + + # Current deploy + service = described_class.new(project, user, params) + Timecop.freeze(time + 12.hours) { service.execute } + + expect(merge_request.reload.metrics.first_deployed_to_production_at).to be_nil + end + end + end + end + end end diff --git a/spec/services/git_push_service_spec.rb b/spec/services/git_push_service_spec.rb index 22724434a7f..22991c5bc86 100644 --- a/spec/services/git_push_service_spec.rb +++ b/spec/services/git_push_service_spec.rb @@ -339,6 +339,43 @@ describe GitPushService, services: true do end end + describe "issue metrics" do + let(:issue) { create :issue, project: project } + let(:commit_author) { create :user } + let(:commit) { project.commit } + let(:commit_time) { Time.now } + + before do + project.team << [commit_author, :developer] + project.team << [user, :developer] + + allow(commit).to receive_messages( + safe_message: "this commit \n mentions #{issue.to_reference}", + references: [issue], + author_name: commit_author.name, + author_email: commit_author.email, + committed_date: commit_time + ) + + allow(project.repository).to receive(:commits_between).and_return([commit]) + end + + context "while saving the 'first_mentioned_in_commit_at' metric for an issue" do + it 'sets the metric for referenced issues' do + execute_service(project, user, @oldrev, @newrev, @ref) + + expect(issue.reload.metrics.first_mentioned_in_commit_at).to be_within(1.second).of(commit_time) + end + + it 'does not set the metric for non-referenced issues' do + non_referenced_issue = create(:issue, project: project) + execute_service(project, user, @oldrev, @newrev, @ref) + + expect(non_referenced_issue.reload.metrics.first_mentioned_in_commit_at).to be_nil + end + end + end + describe "closing issues from pushed commits containing a closing reference" do let(:issue) { create :issue, project: project } let(:other_issue) { create :issue, project: project } diff --git a/spec/services/merge_requests/create_service_spec.rb b/spec/services/merge_requests/create_service_spec.rb index c1e4f8bd96b..b8142889075 100644 --- a/spec/services/merge_requests/create_service_spec.rb +++ b/spec/services/merge_requests/create_service_spec.rb @@ -83,5 +83,34 @@ describe MergeRequests::CreateService, services: true do } end end + + context 'while saving references to issues that the created merge request closes' do + let(:first_issue) { create(:issue, project: project) } + let(:second_issue) { create(:issue, project: project) } + + let(:opts) do + { + title: 'Awesome merge_request', + source_branch: 'feature', + target_branch: 'master', + force_remove_source_branch: '1' + } + end + + before do + project.team << [user, :master] + project.team << [assignee, :developer] + end + + it 'creates a `MergeRequestsClosingIssues` record for each issue' do + issue_closing_opts = opts.merge(description: "Closes #{first_issue.to_reference} and #{second_issue.to_reference}") + service = described_class.new(project, user, issue_closing_opts) + allow(service).to receive(:execute_hooks) + merge_request = service.execute + + issue_ids = MergeRequestsClosingIssues.where(merge_request: merge_request).pluck(:issue_id) + expect(issue_ids).to match_array([first_issue.id, second_issue.id]) + end + end end end diff --git a/spec/services/merge_requests/refresh_service_spec.rb b/spec/services/merge_requests/refresh_service_spec.rb index fff86480c6d..a162df5fc34 100644 --- a/spec/services/merge_requests/refresh_service_spec.rb +++ b/spec/services/merge_requests/refresh_service_spec.rb @@ -174,6 +174,58 @@ describe MergeRequests::RefreshService, services: true do end end + context 'merge request metrics' do + let(:issue) { create :issue, project: @project } + let(:commit_author) { create :user } + let(:commit) { project.commit } + + before do + project.team << [commit_author, :developer] + project.team << [user, :developer] + + allow(commit).to receive_messages( + safe_message: "Closes #{issue.to_reference}", + references: [issue], + author_name: commit_author.name, + author_email: commit_author.email, + committed_date: Time.now + ) + + allow_any_instance_of(MergeRequest).to receive(:commits).and_return([commit]) + end + + context 'when the merge request is sourced from the same project' do + it 'creates a `MergeRequestsClosingIssues` record for each issue closed by a commit' do + merge_request = create(:merge_request, target_branch: 'master', source_branch: 'feature', source_project: @project) + refresh_service = service.new(@project, @user) + allow(refresh_service).to receive(:execute_hooks) + refresh_service.execute(@oldrev, @newrev, 'refs/heads/feature') + + issue_ids = MergeRequestsClosingIssues.where(merge_request: merge_request).pluck(:issue_id) + expect(issue_ids).to eq([issue.id]) + end + end + + context 'when the merge request is sourced from a different project' do + it 'creates a `MergeRequestsClosingIssues` record for each issue closed by a commit' do + forked_project = create(:project) + create(:forked_project_link, forked_to_project: forked_project, forked_from_project: @project) + + merge_request = create(:merge_request, + target_branch: 'master', + source_branch: 'feature', + target_project: @project, + source_project: forked_project) + refresh_service = service.new(@project, @user) + allow(refresh_service).to receive(:execute_hooks) + refresh_service.execute(@oldrev, @newrev, 'refs/heads/feature') + + issue_ids = MergeRequestsClosingIssues.where(merge_request: merge_request).pluck(:issue_id) + expect(issue_ids).to eq([issue.id]) + end + end + end + def reload_mrs @merge_request.reload @fork_merge_request.reload diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb index 6dfeb581975..33db34c0f62 100644 --- a/spec/services/merge_requests/update_service_spec.rb +++ b/spec/services/merge_requests/update_service_spec.rb @@ -263,5 +263,42 @@ describe MergeRequests::UpdateService, services: true do end end end + + context 'while saving references to issues that the updated merge request closes' do + let(:first_issue) { create(:issue, project: project) } + let(:second_issue) { create(:issue, project: project) } + + it 'creates a `MergeRequestsClosingIssues` record for each issue' do + issue_closing_opts = { description: "Closes #{first_issue.to_reference} and #{second_issue.to_reference}" } + service = described_class.new(project, user, issue_closing_opts) + allow(service).to receive(:execute_hooks) + service.execute(merge_request) + + issue_ids = MergeRequestsClosingIssues.where(merge_request: merge_request).pluck(:issue_id) + expect(issue_ids).to match_array([first_issue.id, second_issue.id]) + end + + it 'removes `MergeRequestsClosingIssues` records when issues are not closed anymore' do + opts = { + title: 'Awesome merge_request', + description: "Closes #{first_issue.to_reference} and #{second_issue.to_reference}", + source_branch: 'feature', + target_branch: 'master', + force_remove_source_branch: '1' + } + + merge_request = MergeRequests::CreateService.new(project, user, opts).execute + + issue_ids = MergeRequestsClosingIssues.where(merge_request: merge_request).pluck(:issue_id) + expect(issue_ids).to match_array([first_issue.id, second_issue.id]) + + service = described_class.new(project, user, description: "not closing any issues") + allow(service).to receive(:execute_hooks) + service.execute(merge_request.reload) + + issue_ids = MergeRequestsClosingIssues.where(merge_request: merge_request).pluck(:issue_id) + expect(issue_ids).to be_empty + end + end end end diff --git a/spec/services/notes/slash_commands_service_spec.rb b/spec/services/notes/slash_commands_service_spec.rb index 4f231aab161..d1099884a02 100644 --- a/spec/services/notes/slash_commands_service_spec.rb +++ b/spec/services/notes/slash_commands_service_spec.rb @@ -122,6 +122,75 @@ describe Notes::SlashCommandsService, services: true do end end + describe '.noteable_update_service' do + include_context 'note on noteable' + + it 'returns Issues::UpdateService for a note on an issue' do + note = create(:note_on_issue, project: project) + + expect(described_class.noteable_update_service(note)).to eq(Issues::UpdateService) + end + + it 'returns Issues::UpdateService for a note on a merge request' do + note = create(:note_on_merge_request, project: project) + + expect(described_class.noteable_update_service(note)).to eq(MergeRequests::UpdateService) + end + + it 'returns nil for a note on a commit' do + note = create(:note_on_commit, project: project) + + expect(described_class.noteable_update_service(note)).to be_nil + end + end + + describe '.supported?' do + include_context 'note on noteable' + + let(:note) { create(:note_on_issue, project: project) } + + context 'with no current_user' do + it 'returns false' do + expect(described_class.supported?(note, nil)).to be_falsy + end + end + + context 'when current_user cannot update the noteable' do + it 'returns false' do + user = create(:user) + + expect(described_class.supported?(note, user)).to be_falsy + end + end + + context 'when current_user can update the noteable' do + it 'returns true' do + expect(described_class.supported?(note, master)).to be_truthy + end + + context 'with a note on a commit' do + let(:note) { create(:note_on_commit, project: project) } + + it 'returns false' do + expect(described_class.supported?(note, nil)).to be_falsy + end + end + end + end + + describe '#supported?' do + include_context 'note on noteable' + + it 'delegates to the class method' do + service = described_class.new(project, master) + note = create(:note_on_issue, project: project) + + expect(described_class).to receive(:supported?).with(note, master) + + service.supported?(note) + end + end + describe '#execute' do let(:service) { described_class.new(project, master) } diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index f81a58899fd..0d152534c38 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -379,6 +379,7 @@ describe NotificationService, services: true do it "emails subscribers of the issue's labels" do subscriber = create(:user) label = create(:label, issues: [issue]) + issue.reload label.toggle_subscription(subscriber) notification.new_issue(issue, @u_disabled) @@ -399,6 +400,7 @@ describe NotificationService, services: true do project.team << [guest, :guest] label = create(:label, issues: [confidential_issue]) + confidential_issue.reload label.toggle_subscription(non_member) label.toggle_subscription(author) label.toggle_subscription(assignee) diff --git a/spec/support/cycle_analytics_helpers.rb b/spec/support/cycle_analytics_helpers.rb new file mode 100644 index 00000000000..e8e760a6187 --- /dev/null +++ b/spec/support/cycle_analytics_helpers.rb @@ -0,0 +1,64 @@ +module CycleAnalyticsHelpers + def create_commit_referencing_issue(issue, branch_name: random_git_name) + project.repository.add_branch(user, branch_name, 'master') + create_commit("Commit for ##{issue.iid}", issue.project, user, branch_name) + end + + def create_commit(message, project, user, branch_name) + filename = random_git_name + oldrev = project.repository.commit(branch_name).sha + + options = { + committer: project.repository.user_to_committer(user), + author: project.repository.user_to_committer(user), + commit: { message: message, branch: branch_name, update_ref: true }, + file: { content: "content", path: filename, update: false } + } + + commit_sha = Gitlab::Git::Blob.commit(project.repository, options) + project.repository.commit(commit_sha) + + GitPushService.new(project, + user, + oldrev: oldrev, + newrev: commit_sha, + ref: 'refs/heads/master').execute + end + + def create_merge_request_closing_issue(issue, message: nil, source_branch: nil) + if !source_branch || project.repository.commit(source_branch).blank? + source_branch = random_git_name + project.repository.add_branch(user, source_branch, 'master') + end + + sha = project.repository.commit_file(user, random_git_name, "content", "commit message", source_branch, false) + project.repository.commit(sha) + + opts = { + title: 'Awesome merge_request', + description: message || "Fixes #{issue.to_reference}", + source_branch: source_branch, + target_branch: 'master' + } + + MergeRequests::CreateService.new(project, user, opts).execute + end + + def merge_merge_requests_closing_issue(issue) + merge_requests = issue.closed_by_merge_requests + merge_requests.each { |merge_request| MergeRequests::MergeService.new(project, user).execute(merge_request) } + end + + def deploy_master(environment: 'production') + CreateDeploymentService.new(project, user, { + environment: environment, + ref: 'master', + tag: false, + sha: project.repository.commit('master').sha + }).execute + end +end + +RSpec.configure do |config| + config.include CycleAnalyticsHelpers +end diff --git a/spec/support/cycle_analytics_helpers/test_generation.rb b/spec/support/cycle_analytics_helpers/test_generation.rb new file mode 100644 index 00000000000..8e19a6c92e2 --- /dev/null +++ b/spec/support/cycle_analytics_helpers/test_generation.rb @@ -0,0 +1,161 @@ +# rubocop:disable Metrics/AbcSize + +# Note: The ABC size is large here because we have a method generating test cases with +# multiple nested contexts. This shouldn't count as a violation. + +module CycleAnalyticsHelpers + module TestGeneration + # Generate the most common set of specs that all cycle analytics phases need to have. + # + # Arguments: + # + # phase: Which phase are we testing? Will call `CycleAnalytics.new.send(phase)` for the final assertion + # data_fn: A function that returns a hash, constituting initial data for the test case + # start_time_conditions: An array of `conditions`. Each condition is an tuple of `condition_name` and `condition_fn`. `condition_fn` is called with + # `context` (no lexical scope, so need to do `context.create` for factories, for example) and `data` (from the `data_fn`). + # Each `condition_fn` is expected to implement a case which consitutes the start of the given cycle analytics phase. + # end_time_conditions: An array of `conditions`. Each condition is an tuple of `condition_name` and `condition_fn`. `condition_fn` is called with + # `context` (no lexical scope, so need to do `context.create` for factories, for example) and `data` (from the `data_fn`). + # Each `condition_fn` is expected to implement a case which consitutes the end of the given cycle analytics phase. + # before_end_fn: This function is run before calling the end time conditions. Used for setup that needs to be run between the start and end conditions. + # post_fn: Code that needs to be run after running the end time conditions. + + def generate_cycle_analytics_spec(phase:, data_fn:, start_time_conditions:, end_time_conditions:, before_end_fn: nil, post_fn: nil) + combinations_of_start_time_conditions = (1..start_time_conditions.size).flat_map { |size| start_time_conditions.combination(size).to_a } + combinations_of_end_time_conditions = (1..end_time_conditions.size).flat_map { |size| end_time_conditions.combination(size).to_a } + + scenarios = combinations_of_start_time_conditions.product(combinations_of_end_time_conditions) + scenarios.each do |start_time_conditions, end_time_conditions| + context "start condition: #{start_time_conditions.map(&:first).to_sentence}" do + context "end condition: #{end_time_conditions.map(&:first).to_sentence}" do + it "finds the median of available durations between the two conditions" do + time_differences = Array.new(5) do |index| + data = data_fn[self] + start_time = (index * 10).days.from_now + end_time = start_time + rand(1..5).days + + start_time_conditions.each do |condition_name, condition_fn| + Timecop.freeze(start_time) { condition_fn[self, data] } + end + + # Run `before_end_fn` at the midpoint between `start_time` and `end_time` + Timecop.freeze(start_time + (end_time - start_time) / 2) { before_end_fn[self, data] } if before_end_fn + + end_time_conditions.each do |condition_name, condition_fn| + Timecop.freeze(end_time) { condition_fn[self, data] } + end + + Timecop.freeze(end_time + 1.day) { post_fn[self, data] } if post_fn + + end_time - start_time + end + + median_time_difference = time_differences.sort[2] + expect(subject.send(phase)).to be_within(5).of(median_time_difference) + end + + context "when the data belongs to another project" do + let(:other_project) { create(:project) } + + it "returns nil" do + # Use a stub to "trick" the data/condition functions + # into using another project. This saves us from having to + # define separate data/condition functions for this particular + # test case. + allow(self).to receive(:project) { other_project } + + 5.times do + data = data_fn[self] + start_time = Time.now + end_time = rand(1..10).days.from_now + + start_time_conditions.each do |condition_name, condition_fn| + Timecop.freeze(start_time) { condition_fn[self, data] } + end + + end_time_conditions.each do |condition_name, condition_fn| + Timecop.freeze(end_time) { condition_fn[self, data] } + end + + Timecop.freeze(end_time + 1.day) { post_fn[self, data] } if post_fn + end + + # Turn off the stub before checking assertions + allow(self).to receive(:project).and_call_original + + expect(subject.send(phase)).to be_nil + end + end + + context "when the end condition happens before the start condition" do + it 'returns nil' do + data = data_fn[self] + start_time = Time.now + end_time = start_time + rand(1..5).days + + # Run `before_end_fn` at the midpoint between `start_time` and `end_time` + Timecop.freeze(start_time + (end_time - start_time) / 2) { before_end_fn[self, data] } if before_end_fn + + end_time_conditions.each do |condition_name, condition_fn| + Timecop.freeze(start_time) { condition_fn[self, data] } + end + + start_time_conditions.each do |condition_name, condition_fn| + Timecop.freeze(end_time) { condition_fn[self, data] } + end + + Timecop.freeze(end_time + 1.day) { post_fn[self, data] } if post_fn + + expect(subject.send(phase)).to be_nil + end + end + end + end + + context "start condition NOT PRESENT: #{start_time_conditions.map(&:first).to_sentence}" do + context "end condition: #{end_time_conditions.map(&:first).to_sentence}" do + it "returns nil" do + 5.times do + data = data_fn[self] + end_time = rand(1..10).days.from_now + + end_time_conditions.each_with_index do |(condition_name, condition_fn), index| + Timecop.freeze(end_time + index.days) { condition_fn[self, data] } + end + + Timecop.freeze(end_time + 1.day) { post_fn[self, data] } if post_fn + end + + expect(subject.send(phase)).to be_nil + end + end + end + + context "start condition: #{start_time_conditions.map(&:first).to_sentence}" do + context "end condition NOT PRESENT: #{end_time_conditions.map(&:first).to_sentence}" do + it "returns nil" do + 5.times do + data = data_fn[self] + start_time = Time.now + + start_time_conditions.each do |condition_name, condition_fn| + Timecop.freeze(start_time) { condition_fn[self, data] } + end + + post_fn[self, data] if post_fn + end + + expect(subject.send(phase)).to be_nil + end + end + end + end + + context "when none of the start / end conditions are matched" do + it "returns nil" do + expect(subject.send(phase)).to be_nil + end + end + end + end +end diff --git a/spec/support/git_helpers.rb b/spec/support/git_helpers.rb new file mode 100644 index 00000000000..93422390ef7 --- /dev/null +++ b/spec/support/git_helpers.rb @@ -0,0 +1,9 @@ +module GitHelpers + def random_git_name + "#{FFaker::Product.brand}-#{FFaker::Product.brand}-#{rand(1000)}" + end +end + +RSpec.configure do |config| + config.include GitHelpers +end diff --git a/spec/support/import_export/configuration_helper.rb b/spec/support/import_export/configuration_helper.rb new file mode 100644 index 00000000000..f752508d48c --- /dev/null +++ b/spec/support/import_export/configuration_helper.rb @@ -0,0 +1,29 @@ +module ConfigurationHelper + # Returns a list of models from hashes/arrays contained in +project_tree+ + def names_from_tree(project_tree) + project_tree.map do |branch_or_model| + branch_or_model = branch_or_model.to_s if branch_or_model.is_a?(Symbol) + + branch_or_model.is_a?(String) ? branch_or_model : names_from_tree(branch_or_model) + end + end + + def relation_class_for_name(relation_name) + relation_name = Gitlab::ImportExport::RelationFactory::OVERRIDES[relation_name.to_sym] || relation_name + relation_name.to_s.classify.constantize + end + + def parsed_attributes(relation_name, attributes) + excluded_attributes = config_hash['excluded_attributes'][relation_name] + included_attributes = config_hash['included_attributes'][relation_name] + + attributes = attributes - JSON[excluded_attributes.to_json] if excluded_attributes + attributes = attributes & JSON[included_attributes.to_json] if included_attributes + + attributes + end + + def associations_for(safe_model) + safe_model.reflect_on_all_associations.map { |assoc| assoc.name.to_s } + end +end diff --git a/spec/support/import_export/export_file_helper.rb b/spec/support/import_export/export_file_helper.rb new file mode 100644 index 00000000000..be0772d6a4a --- /dev/null +++ b/spec/support/import_export/export_file_helper.rb @@ -0,0 +1,133 @@ +require './spec/support/import_export/configuration_helper' + +module ExportFileHelper + include ConfigurationHelper + + ObjectWithParent = Struct.new(:object, :parent, :key_found) + + def setup_project + project = create(:project, :public) + + create(:release, project: project) + + issue = create(:issue, assignee: user, project: project) + snippet = create(:project_snippet, project: project) + label = create(:label, project: project) + milestone = create(:milestone, project: project) + merge_request = create(:merge_request, source_project: project, milestone: milestone) + commit_status = create(:commit_status, project: project) + + create(:label_link, label: label, target: issue) + + ci_pipeline = create(:ci_pipeline, + project: project, + sha: merge_request.diff_head_sha, + ref: merge_request.source_branch, + statuses: [commit_status]) + + create(:ci_build, pipeline: ci_pipeline, project: project) + create(:milestone, project: project) + create(:note, noteable: issue, project: project) + create(:note, noteable: merge_request, project: project) + create(:note, noteable: snippet, project: project) + create(:note_on_commit, + author: user, + project: project, + commit_id: ci_pipeline.sha) + + create(:event, target: milestone, project: project, action: Event::CREATED, author: user) + create(:project_member, :master, user: user, project: project) + create(:ci_variable, project: project) + create(:ci_trigger, project: project) + key = create(:deploy_key) + key.projects << project + create(:service, project: project) + create(:project_hook, project: project, token: 'token') + create(:protected_branch, project: project) + + project + end + + # Expands the compressed file for an exported project into +tmpdir+ + def in_directory_with_expanded_export(project) + Dir.mktmpdir do |tmpdir| + export_file = project.export_project_path + _output, exit_status = Gitlab::Popen.popen(%W{tar -zxf #{export_file} -C #{tmpdir}}) + + yield(exit_status, tmpdir) + end + end + + # Recursively finds key/values including +key+ as part of the key, inside a nested hash + def deep_find_with_parent(sensitive_key_word, object, found = nil) + sensitive_key_found = object_contains_key?(object, sensitive_key_word) + + # Returns the parent object and the object found containing a sensitive word as part of the key + if sensitive_key_found && object[sensitive_key_found] + ObjectWithParent.new(object[sensitive_key_found], object, sensitive_key_found) + elsif object.is_a?(Enumerable) + # Recursively lookup for keys containing sensitive words in a Hash or Array + object_with_parent = nil + + object.find do |*hash_or_array| + object_with_parent = deep_find_with_parent(sensitive_key_word, hash_or_array.last, found) + end + + object_with_parent + end + end + + # Return true if the hash has a key containing a sensitive word + def object_contains_key?(object, sensitive_key_word) + return false unless object.is_a?(Hash) + + object.keys.find { |key| key.include?(sensitive_key_word) } + end + + # Returns the offended ObjectWithParent object if a sensitive word is found inside a hash, + # excluding the whitelisted safe hashes. + def find_sensitive_attributes(sensitive_word, project_hash) + loop do + object_with_parent = deep_find_with_parent(sensitive_word, project_hash) + + return nil unless object_with_parent && object_with_parent.object + + if is_safe_hash?(object_with_parent.parent, sensitive_word) + # It's in the safe list, remove hash and keep looking + object_with_parent.parent.delete(object_with_parent.key_found) + else + return object_with_parent + end + + nil + end + end + + # Returns true if it's one of the excluded models in +safe_list+ + def is_safe_hash?(parent, sensitive_word) + return false unless parent && safe_list[sensitive_word.to_sym] + + # Extra attributes that appear in a model but not in the exported hash. + excluded_attributes = ['type'] + + safe_list[sensitive_word.to_sym].each do |model| + # Check whether this is a hash attribute inside a model + if model.is_a?(Symbol) + return true if (safe_hashes[model] - parent.keys).empty? + else + return true if safe_model?(model, excluded_attributes, parent) + end + end + + false + end + + # Compares model attributes with those those found in the hash + # and returns true if there is a match, ignoring some excluded attributes. + def safe_model?(model, excluded_attributes, parent) + excluded_attributes += associations_for(model) + parsed_model_attributes = parsed_attributes(model.name.underscore, model.attribute_names) + + (parsed_model_attributes - parent.keys - excluded_attributes).empty? + end +end diff --git a/spec/views/projects/merge_requests/_heading.html.haml_spec.rb b/spec/views/projects/merge_requests/_heading.html.haml_spec.rb index 733b2dfa7ff..21f49d396e7 100644 --- a/spec/views/projects/merge_requests/_heading.html.haml_spec.rb +++ b/spec/views/projects/merge_requests/_heading.html.haml_spec.rb @@ -15,6 +15,8 @@ describe 'projects/merge_requests/widget/_heading' do assign(:merge_request, merge_request) assign(:project, project) + allow(view).to receive(:can?).and_return(true) + render end diff --git a/spec/views/projects/notes/_form.html.haml_spec.rb b/spec/views/projects/notes/_form.html.haml_spec.rb new file mode 100644 index 00000000000..932d6756ad2 --- /dev/null +++ b/spec/views/projects/notes/_form.html.haml_spec.rb @@ -0,0 +1,36 @@ +require 'spec_helper' + +describe 'projects/notes/_form' do + include Devise::TestHelpers + + let(:user) { create(:user) } + let(:project) { create(:empty_project) } + + before do + project.team << [user, :master] + assign(:project, project) + assign(:note, note) + + allow(view).to receive(:current_user).and_return(user) + + render + end + + %w[issue merge_request].each do |noteable| + context "with a note on #{noteable}" do + let(:note) { build(:"note_on_#{noteable}", project: project) } + + it 'says that only markdown is supported, not slash commands' do + expect(rendered).to have_content('Styling with Markdown and slash commands are supported') + end + end + end + + context 'with a note on a commit' do + let(:note) { build(:note_on_commit, project: project) } + + it 'says that only markdown is supported, not slash commands' do + expect(rendered).to have_content('Styling with Markdown is supported') + end + end +end |