diff options
Diffstat (limited to 'spec')
153 files changed, 3803 insertions, 1212 deletions
diff --git a/spec/controllers/concerns/send_file_upload_spec.rb b/spec/controllers/concerns/send_file_upload_spec.rb index 379b2d6b935..a07113a6156 100644 --- a/spec/controllers/concerns/send_file_upload_spec.rb +++ b/spec/controllers/concerns/send_file_upload_spec.rb @@ -53,19 +53,38 @@ describe SendFileUpload do end context 'with attachment' do - let(:params) { { attachment: 'test.js' } } + let(:filename) { 'test.js' } + let(:params) { { attachment: filename } } it 'sends a file with content-type of text/plain' do + # Notice the filename= is omitted from the disposition; this is because + # Rails 5 will append this header in send_file expected_params = { content_type: 'text/plain', filename: 'test.js', - disposition: 'attachment' + disposition: "attachment; filename*=UTF-8''test.js" } expect(controller).to receive(:send_file).with(uploader.path, expected_params) subject end + context 'with non-ASCII encoded filename' do + let(:filename) { 'テスト.txt' } + + # Notice the filename= is omitted from the disposition; this is because + # Rails 5 will append this header in send_file + it 'sends content-disposition for non-ASCII encoded filenames' do + expected_params = { + filename: filename, + disposition: "attachment; filename*=UTF-8''%E3%83%86%E3%82%B9%E3%83%88.txt" + } + expect(controller).to receive(:send_file).with(uploader.path, expected_params) + + subject + end + end + context 'with a proxied file in object storage' do before do stub_uploads_object_storage(uploader: uploader_class) @@ -76,7 +95,7 @@ describe SendFileUpload do it 'sends a file with a custom type' do headers = double - expected_headers = %r(response-content-disposition=attachment%3Bfilename%3D%22test.js%22&response-content-type=application/ecmascript) + expected_headers = %r(response-content-disposition=attachment%3B%20filename%3D%22test.js%22%3B%20filename%2A%3DUTF-8%27%27test.js&response-content-type=application/ecmascript) expect(Gitlab::Workhorse).to receive(:send_url).with(expected_headers).and_call_original expect(headers).to receive(:store).with(Gitlab::Workhorse::SEND_DATA_HEADER, /^send-url:/) diff --git a/spec/controllers/omniauth_callbacks_controller_spec.rb b/spec/controllers/omniauth_callbacks_controller_spec.rb index 59463462e5a..232a5e2793b 100644 --- a/spec/controllers/omniauth_callbacks_controller_spec.rb +++ b/spec/controllers/omniauth_callbacks_controller_spec.rb @@ -45,6 +45,29 @@ describe OmniauthCallbacksController, type: :controller do end end + context 'when sign in fails' do + include RoutesHelpers + + let(:extern_uid) { 'my-uid' } + let(:provider) { :saml } + + def stub_route_as(path) + allow(@routes).to receive(:generate_extras) { [path, []] } + end + + it 'it calls through to the failure handler' do + request.env['omniauth.error'] = OneLogin::RubySaml::ValidationError.new("Fingerprint mismatch") + request.env['omniauth.error.strategy'] = OmniAuth::Strategies::SAML.new(nil) + stub_route_as('/users/auth/saml/callback') + + ForgeryProtection.with_forgery_protection do + post :failure + end + + expect(flash[:alert]).to match(/Fingerprint mismatch/) + end + end + context 'when a redirect fragment is provided' do let(:provider) { :jwt } let(:extern_uid) { 'my-uid' } diff --git a/spec/controllers/projects/artifacts_controller_spec.rb b/spec/controllers/projects/artifacts_controller_spec.rb index bd10de45b67..29df00e6bb0 100644 --- a/spec/controllers/projects/artifacts_controller_spec.rb +++ b/spec/controllers/projects/artifacts_controller_spec.rb @@ -26,8 +26,15 @@ describe Projects::ArtifactsController do end context 'when no file type is supplied' do + let(:filename) { job.artifacts_file.filename } + it 'sends the artifacts file' do - expect(controller).to receive(:send_file).with(job.artifacts_file.path, hash_including(disposition: 'attachment')).and_call_original + # Notice the filename= is omitted from the disposition; this is because + # Rails 5 will append this header in send_file + expect(controller).to receive(:send_file) + .with( + job.artifacts_file.file.path, + hash_including(disposition: %Q(attachment; filename*=UTF-8''#{filename}))).and_call_original download_artifact end @@ -46,6 +53,7 @@ describe Projects::ArtifactsController do context 'when codequality file type is supplied' do let(:file_type) { 'codequality' } + let(:filename) { job.job_artifacts_codequality.filename } context 'when file is stored locally' do before do @@ -53,7 +61,11 @@ describe Projects::ArtifactsController do end it 'sends the codequality report' do - expect(controller).to receive(:send_file).with(job.job_artifacts_codequality.file.path, hash_including(disposition: 'attachment')).and_call_original + # Notice the filename= is omitted from the disposition; this is because + # Rails 5 will append this header in send_file + expect(controller).to receive(:send_file) + .with(job.job_artifacts_codequality.file.path, + hash_including(disposition: %Q(attachment; filename*=UTF-8''#{filename}))).and_call_original download_artifact(file_type: file_type) end diff --git a/spec/controllers/projects/environments_controller_spec.rb b/spec/controllers/projects/environments_controller_spec.rb index a4d494a820f..aa97a417a98 100644 --- a/spec/controllers/projects/environments_controller_spec.rb +++ b/spec/controllers/projects/environments_controller_spec.rb @@ -422,6 +422,79 @@ describe Projects::EnvironmentsController do end end + describe 'GET #search' do + before do + create(:environment, name: 'staging', project: project) + create(:environment, name: 'review/patch-1', project: project) + create(:environment, name: 'review/patch-2', project: project) + end + + let(:query) { 'pro' } + + it 'responds with status code 200' do + get :search, params: environment_params(format: :json, query: query) + + expect(response).to have_gitlab_http_status(:ok) + end + + it 'returns matched results' do + get :search, params: environment_params(format: :json, query: query) + + expect(json_response).to contain_exactly('production') + end + + context 'when query is review' do + let(:query) { 'review' } + + it 'returns matched results' do + get :search, params: environment_params(format: :json, query: query) + + expect(json_response).to contain_exactly('review/patch-1', 'review/patch-2') + end + end + + context 'when query is empty' do + let(:query) { '' } + + it 'returns matched results' do + get :search, params: environment_params(format: :json, query: query) + + expect(json_response) + .to contain_exactly('production', 'staging', 'review/patch-1', 'review/patch-2') + end + end + + context 'when query is review/patch-3' do + let(:query) { 'review/patch-3' } + + it 'responds with status code 204' do + get :search, params: environment_params(format: :json, query: query) + + expect(response).to have_gitlab_http_status(:no_content) + end + end + + context 'when query is partially matched in the middle of environment name' do + let(:query) { 'patch' } + + it 'responds with status code 204' do + get :search, params: environment_params(format: :json, query: query) + + expect(response).to have_gitlab_http_status(:no_content) + end + end + + context 'when query contains a wildcard character' do + let(:query) { 'review%' } + + it 'prevents wildcard injection' do + get :search, params: environment_params(format: :json, query: query) + + expect(response).to have_gitlab_http_status(:no_content) + end + end + end + def environment_params(opts = {}) opts.reverse_merge(namespace_id: project.namespace, project_id: project, diff --git a/spec/controllers/projects/error_tracking_controller_spec.rb b/spec/controllers/projects/error_tracking_controller_spec.rb index 6464398cea1..844c61f1ace 100644 --- a/spec/controllers/projects/error_tracking_controller_spec.rb +++ b/spec/controllers/projects/error_tracking_controller_spec.rb @@ -107,8 +107,11 @@ describe Projects::ErrorTrackingController do let(:http_status) { :no_content } before do - expect(list_issues_service).to receive(:execute) - .and_return(status: :error, message: error_message, http_status: http_status) + expect(list_issues_service).to receive(:execute).and_return( + status: :error, + message: error_message, + http_status: http_status + ) end it 'returns http_status with message' do @@ -122,6 +125,113 @@ describe Projects::ErrorTrackingController do end end + describe 'POST #list_projects' do + context 'with insufficient permissions' do + before do + project.add_guest(user) + end + + it 'returns 404' do + post :list_projects, params: list_projects_params + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'with an anonymous user' do + before do + sign_out(user) + end + + it 'redirects to sign-in page' do + post :list_projects, params: list_projects_params + + expect(response).to have_gitlab_http_status(:unauthorized) + end + end + + context 'with authorized user' do + let(:list_projects_service) { spy(:list_projects_service) } + let(:sentry_project) { build(:error_tracking_project) } + + let(:permitted_params) do + ActionController::Parameters.new( + list_projects_params[:error_tracking_setting] + ).permit! + end + + before do + allow(ErrorTracking::ListProjectsService) + .to receive(:new).with(project, user, permitted_params) + .and_return(list_projects_service) + end + + context 'service result is successful' do + before do + expect(list_projects_service).to receive(:execute) + .and_return(status: :success, projects: [sentry_project]) + end + + it 'returns a list of projects' do + post :list_projects, params: list_projects_params + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('error_tracking/list_projects') + expect(json_response['projects']).to eq([sentry_project].as_json) + end + end + + context 'service result is erroneous' do + let(:error_message) { 'error message' } + + context 'without http_status' do + before do + expect(list_projects_service).to receive(:execute) + .and_return(status: :error, message: error_message) + end + + it 'returns 400 with message' do + get :list_projects, params: list_projects_params + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']).to eq(error_message) + end + end + + context 'with explicit http_status' do + let(:http_status) { :no_content } + + before do + expect(list_projects_service).to receive(:execute).and_return( + status: :error, + message: error_message, + http_status: http_status + ) + end + + it 'returns http_status with message' do + get :list_projects, params: list_projects_params + + expect(response).to have_gitlab_http_status(http_status) + expect(json_response['message']).to eq(error_message) + end + end + end + end + + private + + def list_projects_params(opts = {}) + project_params( + format: :json, + error_tracking_setting: { + api_host: 'gitlab.com', + token: 'token' + } + ) + end + end + private def project_params(opts = {}) diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index ca5ff9b1e3b..79f97aa4170 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -387,6 +387,23 @@ describe Projects::MergeRequestsController do end end + context 'when a squash commit message is passed' do + let(:message) { 'My custom squash commit message' } + + it 'passes the same message to SquashService' do + params = { squash: '1', squash_commit_message: message } + + expect_next_instance_of(MergeRequests::SquashService, project, user, params.merge(merge_request: merge_request)) do |squash_service| + expect(squash_service).to receive(:execute).and_return({ + status: :success, + squash_sha: SecureRandom.hex(20) + }) + end + + merge_with_sha(params) + end + end + context 'when the pipeline succeeds is passed' do let!(:head_pipeline) do create(:ci_empty_pipeline, project: project, sha: merge_request.diff_head_sha, ref: merge_request.source_branch, head_pipeline_of: merge_request) diff --git a/spec/controllers/projects/services_controller_spec.rb b/spec/controllers/projects/services_controller_spec.rb index 4a5d2bdecb7..601a292bf54 100644 --- a/spec/controllers/projects/services_controller_spec.rb +++ b/spec/controllers/projects/services_controller_spec.rb @@ -152,6 +152,16 @@ describe Projects::ServicesController do expect(service.namespace).not_to eq('updated_namespace') end end + + context 'when activating JIRA service from a template' do + let(:template_service) { create(:jira_service, project: project, template: true) } + + it 'activate JIRA service from template' do + put :update, params: { namespace_id: project.namespace, project_id: project, id: service.to_param, service: { active: true } } + + expect(flash[:notice]).to eq 'JIRA activated.' + end + end end describe "GET #edit" do diff --git a/spec/factories/commits.rb b/spec/factories/commits.rb index 818f7b046f6..2bcc4b6cf52 100644 --- a/spec/factories/commits.rb +++ b/spec/factories/commits.rb @@ -16,14 +16,24 @@ FactoryBot.define do commit end + project + skip_create # Commits cannot be persisted + initialize_with do new(git_commit, project) end after(:build) do |commit, evaluator| allow(commit).to receive(:author).and_return(evaluator.author || build_stubbed(:author)) + allow(commit).to receive(:parent_ids).and_return([]) + end + + trait :merge_commit do + after(:build) do |commit| + allow(commit).to receive(:parent_ids).and_return(Array.new(2) { SecureRandom.hex(20) }) + end end trait :without_author do diff --git a/spec/features/admin/admin_users_spec.rb b/spec/features/admin/admin_users_spec.rb index 931095936a6..b1c6f308bc6 100644 --- a/spec/features/admin/admin_users_spec.rb +++ b/spec/features/admin/admin_users_spec.rb @@ -1,13 +1,13 @@ require 'spec_helper' describe "Admin::Users" do - include Spec::Support::Helpers::Features::ListRowsHelpers + include Spec::Support::Helpers::Features::ResponsiveTableHelpers let!(:user) do create(:omniauth_user, provider: 'twitter', extern_uid: '123456') end - let!(:current_user) { create(:admin) } + let!(:current_user) { create(:admin, last_activity_on: 5.days.ago) } before do sign_in(current_user) @@ -25,6 +25,8 @@ describe "Admin::Users" do it "has users list" do expect(page).to have_content(current_user.email) expect(page).to have_content(current_user.name) + expect(page).to have_content(current_user.created_at.strftime("%e %b, %Y")) + expect(page).to have_content(current_user.last_activity_on.strftime("%e %b, %Y")) expect(page).to have_content(user.email) expect(page).to have_content(user.name) expect(page).to have_link('Block', href: block_admin_user_path(user)) @@ -32,10 +34,24 @@ describe "Admin::Users" do expect(page).to have_button('Delete user and contributions') end + describe "view extra user information", :js do + it 'does not have the user popover open' do + expect(page).not_to have_selector('#__BV_popover_1__') + end + + it 'shows the user popover on hover' do + first_user_link = page.first('.js-user-link') + + first_user_link.hover + + expect(page).to have_selector('#__BV_popover_1__') + end + end + describe 'search and sort' do before do - create(:user, name: 'Foo Bar') - create(:user, name: 'Foo Baz') + create(:user, name: 'Foo Bar', last_activity_on: 3.days.ago) + create(:user, name: 'Foo Baz', last_activity_on: 2.days.ago) create(:user, name: 'Dmitriy') end @@ -75,6 +91,24 @@ describe "Admin::Users" do expect(first_row.text).to include('Foo Bar') expect(second_row.text).to include('Foo Baz') end + + it 'sorts users by recent last activity' do + visit admin_users_path(search_query: 'Foo') + + sort_by('Recent last activity') + + expect(first_row.text).to include('Foo Baz') + expect(second_row.text).to include('Foo Bar') + end + + it 'sorts users by oldest last activity' do + visit admin_users_path(search_query: 'Foo') + + sort_by('Oldest last activity') + + expect(first_row.text).to include('Foo Bar') + expect(second_row.text).to include('Foo Baz') + end end describe 'Two-factor Authentication filters' do diff --git a/spec/features/markdown/copy_as_gfm_spec.rb b/spec/features/markdown/copy_as_gfm_spec.rb index 16754035076..60ddb02da2c 100644 --- a/spec/features/markdown/copy_as_gfm_spec.rb +++ b/spec/features/markdown/copy_as_gfm_spec.rb @@ -843,6 +843,7 @@ describe 'Copy as GFM', :js do def verify(selector, gfm, target: nil) html = html_for_selector(selector) output_gfm = html_to_gfm(html, 'transformCodeSelection', target: target) + wait_for_requests expect(output_gfm.strip).to eq(gfm.strip) end end @@ -861,6 +862,9 @@ describe 'Copy as GFM', :js do def html_to_gfm(html, transformer = 'transformGFMSelection', target: nil) js = <<~JS (function(html) { + // Setting it off so the import already starts + window.CopyAsGFM.nodeToGFM(document.createElement('div')); + var transformer = window.CopyAsGFM[#{transformer.inspect}]; var node = document.createElement('div'); @@ -875,9 +879,18 @@ describe 'Copy as GFM', :js do node = transformer(node, target); if (!node) return null; - return window.CopyAsGFM.nodeToGFM(node); + + window.gfmCopytestRes = null; + window.CopyAsGFM.nodeToGFM(node) + .then((res) => { + window.gfmCopytestRes = res; + }); })("#{escape_javascript(html)}") JS - page.evaluate_script(js) + page.execute_script(js) + + loop until page.evaluate_script('window.gfmCopytestRes !== null') + + page.evaluate_script('window.gfmCopytestRes') end end diff --git a/spec/features/markdown/markdown_spec.rb b/spec/features/markdown/markdown_spec.rb index 3b37ede8579..8815643ca96 100644 --- a/spec/features/markdown/markdown_spec.rb +++ b/spec/features/markdown/markdown_spec.rb @@ -13,7 +13,7 @@ require 'erb' # # Raw Markdown # -> `markdown` helper -# -> Redcarpet::Render::GitlabHTML converts Markdown to HTML +# -> CommonMark::Render::GitlabHTML converts Markdown to HTML # -> Post-process HTML # -> `gfm` helper # -> HTML::Pipeline @@ -324,31 +324,6 @@ describe 'GitLab Markdown', :aggregate_failures do end end - context 'Redcarpet documents' do - before do - allow_any_instance_of(Banzai::Filter::MarkdownFilter).to receive(:engine).and_return('Redcarpet') - @html = markdown(@feat.raw_markdown) - end - - it 'processes certain elements differently' do - aggregate_failures 'parses superscript' do - expect(doc).to have_selector('sup', count: 3) - end - - aggregate_failures 'permits style attribute in th elements' do - expect(doc.at_css('th:contains("Header")')['style']).to eq 'text-align: center' - expect(doc.at_css('th:contains("Row")')['style']).to eq 'text-align: right' - expect(doc.at_css('th:contains("Example")')['style']).to eq 'text-align: left' - end - - aggregate_failures 'permits style attribute in td elements' do - expect(doc.at_css('td:contains("Foo")')['style']).to eq 'text-align: center' - expect(doc.at_css('td:contains("Bar")')['style']).to eq 'text-align: right' - expect(doc.at_css('td:contains("Baz")')['style']).to eq 'text-align: left' - end - end - end - # Fake a `current_user` helper def current_user @feat.user diff --git a/spec/features/merge_request/user_posts_notes_spec.rb b/spec/features/merge_request/user_posts_notes_spec.rb index ee5f5377ca6..1bbcf455ac7 100644 --- a/spec/features/merge_request/user_posts_notes_spec.rb +++ b/spec/features/merge_request/user_posts_notes_spec.rb @@ -66,6 +66,38 @@ describe 'Merge request > User posts notes', :js do is_expected.to have_css('.js-note-text', visible: true) end end + + describe 'when reply_to_individual_notes feature flag is not set' do + before do + stub_feature_flags(reply_to_individual_notes: false) + visit project_merge_request_path(project, merge_request) + end + + it 'does not show a reply button' do + expect(page).to have_no_selector('.js-reply-button') + end + end + + describe 'when reply_to_individual_notes feature flag is set' do + before do + stub_feature_flags(reply_to_individual_notes: true) + visit project_merge_request_path(project, merge_request) + end + + it 'shows a reply button' do + reply_button = find('.js-reply-button', match: :first) + + expect(reply_button).to have_selector('.ic-comment') + end + + it 'shows reply placeholder when clicking reply button' do + reply_button = find('.js-reply-button', match: :first) + + reply_button.click + + expect(page).to have_selector('.discussion-reply-holder') + end + end end describe 'when previewing a note' do diff --git a/spec/features/merge_request/user_resolves_conflicts_spec.rb b/spec/features/merge_request/user_resolves_conflicts_spec.rb index 50c723776a3..16c058ab6bd 100644 --- a/spec/features/merge_request/user_resolves_conflicts_spec.rb +++ b/spec/features/merge_request/user_resolves_conflicts_spec.rb @@ -37,6 +37,8 @@ describe 'Merge request > User resolves conflicts', :js do click_on 'Changes' wait_for_requests + find('.js-toggle-tree-list').click + within find('.diff-file', text: 'files/ruby/popen.rb') do expect(page).to have_selector('.line_content.new', text: "vars = { 'PWD' => path }") expect(page).to have_selector('.line_content.new', text: "options = { chdir: path }") diff --git a/spec/features/merge_request/user_sees_versions_spec.rb b/spec/features/merge_request/user_sees_versions_spec.rb index 63d8decc2d2..aa91ade46ca 100644 --- a/spec/features/merge_request/user_sees_versions_spec.rb +++ b/spec/features/merge_request/user_sees_versions_spec.rb @@ -42,7 +42,7 @@ describe 'Merge request > User sees versions', :js do expect(page).to have_content 'latest version' end - expect(page).to have_content '8 changed files' + expect(page).to have_content '8 Files' end it_behaves_like 'allows commenting', @@ -76,7 +76,7 @@ describe 'Merge request > User sees versions', :js do end it 'shows comments that were last relevant at that version' do - expect(page).to have_content '5 changed files' + expect(page).to have_content '5 Files' position = Gitlab::Diff::Position.new( old_path: ".gitmodules", @@ -120,8 +120,15 @@ describe 'Merge request > User sees versions', :js do diff_id: merge_request_diff3.id, start_sha: '6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9' ) - expect(page).to have_content '4 changed files' - expect(page).to have_content '15 additions 6 deletions' + expect(page).to have_content '4 Files' + + additions_content = page.find('.diff-stats.is-compare-versions-header .diff-stats-group svg.ic-file-addition') + .ancestor('.diff-stats-group').text + deletions_content = page.find('.diff-stats.is-compare-versions-header .diff-stats-group svg.ic-file-deletion') + .ancestor('.diff-stats-group').text + + expect(additions_content).to eq '15' + expect(deletions_content).to eq '6' position = Gitlab::Diff::Position.new( old_path: ".gitmodules", @@ -141,8 +148,14 @@ describe 'Merge request > User sees versions', :js do end it 'show diff between new and old version' do - expect(page).to have_content '4 changed files' - expect(page).to have_content '15 additions 6 deletions' + additions_content = page.find('.diff-stats.is-compare-versions-header .diff-stats-group svg.ic-file-addition') + .ancestor('.diff-stats-group').text + deletions_content = page.find('.diff-stats.is-compare-versions-header .diff-stats-group svg.ic-file-deletion') + .ancestor('.diff-stats-group').text + + expect(page).to have_content '4 Files' + expect(additions_content).to eq '15' + expect(deletions_content).to eq '6' end it 'returns to latest version when "Show latest version" button is clicked' do @@ -150,7 +163,7 @@ describe 'Merge request > User sees versions', :js do page.within '.mr-version-dropdown' do expect(page).to have_content 'latest version' end - expect(page).to have_content '8 changed files' + expect(page).to have_content '8 Files' end it_behaves_like 'allows commenting', @@ -176,7 +189,7 @@ describe 'Merge request > User sees versions', :js do find('.btn-default').click click_link 'version 1' end - expect(page).to have_content '0 changed files' + expect(page).to have_content '0 Files' end end @@ -202,7 +215,7 @@ describe 'Merge request > User sees versions', :js do expect(page).to have_content 'version 1' end - expect(page).to have_content '0 changed files' + expect(page).to have_content '0 Files' end end diff --git a/spec/features/merge_requests/user_squashes_merge_request_spec.rb b/spec/features/merge_requests/user_squashes_merge_request_spec.rb index 47f9f10815c..bf9c55cf22c 100644 --- a/spec/features/merge_requests/user_squashes_merge_request_spec.rb +++ b/spec/features/merge_requests/user_squashes_merge_request_spec.rb @@ -14,7 +14,7 @@ describe 'User squashes a merge request', :js do latest_master_commits = project.repository.commits_between(original_head.sha, 'master').map(&:raw) squash_commit = an_object_having_attributes(sha: a_string_matching(/\h{40}/), - message: "Csv\n", + message: a_string_starting_with(project.merge_requests.first.default_squash_commit_message), author_name: user.name, committer_name: user.name) diff --git a/spec/features/projects/artifacts/user_downloads_artifacts_spec.rb b/spec/features/projects/artifacts/user_downloads_artifacts_spec.rb index 554f0b49052..5cb015e80be 100644 --- a/spec/features/projects/artifacts/user_downloads_artifacts_spec.rb +++ b/spec/features/projects/artifacts/user_downloads_artifacts_spec.rb @@ -7,7 +7,7 @@ describe "User downloads artifacts" do shared_examples "downloading" do it "downloads the zip" do - expect(page.response_headers["Content-Disposition"]).to eq(%Q{attachment; filename="#{job.artifacts_file.filename}"}) + expect(page.response_headers["Content-Disposition"]).to eq(%Q{attachment; filename*=UTF-8''#{job.artifacts_file.filename}; filename="#{job.artifacts_file.filename}"}) expect(page.response_headers['Content-Transfer-Encoding']).to eq("binary") expect(page.response_headers['Content-Type']).to eq("application/zip") expect(page.source.b).to eq(job.artifacts_file.file.read.b) diff --git a/spec/features/projects/blobs/blob_show_spec.rb b/spec/features/projects/blobs/blob_show_spec.rb index e2f9e7e9cc5..3edcc7ac2cd 100644 --- a/spec/features/projects/blobs/blob_show_spec.rb +++ b/spec/features/projects/blobs/blob_show_spec.rb @@ -5,8 +5,8 @@ describe 'File blob', :js do let(:project) { create(:project, :public, :repository) } - def visit_blob(path, anchor: nil, ref: 'master', legacy_render: nil) - visit project_blob_path(project, File.join(ref, path), anchor: anchor, legacy_render: legacy_render) + def visit_blob(path, anchor: nil, ref: 'master') + visit project_blob_path(project, File.join(ref, path), anchor: anchor) wait_for_requests end @@ -171,21 +171,6 @@ describe 'File blob', :js do end end end - - context 'when rendering legacy markdown' do - before do - visit_blob('files/commonmark/file.md', legacy_render: 1) - - wait_for_requests - end - - it 'renders using RedCarpet' do - aggregate_failures do - expect(page).to have_content("sublist") - expect(page).to have_xpath("//ol//li//ul") - end - end - end end context 'Markdown file (stored in LFS)' do diff --git a/spec/features/projects/blobs/edit_spec.rb b/spec/features/projects/blobs/edit_spec.rb index d5b20605860..6e6c299ee2e 100644 --- a/spec/features/projects/blobs/edit_spec.rb +++ b/spec/features/projects/blobs/edit_spec.rb @@ -81,17 +81,6 @@ describe 'Editing file blob', :js do expect(page).to have_content("sublist") expect(page).not_to have_xpath("//ol//li//ul") end - - it 'renders content with RedCarpet when legacy_render is set' do - visit project_edit_blob_path(project, tree_join(branch, readme_file_path), legacy_render: 1) - fill_editor(content: "1. one\\n - sublist\\n") - click_link 'Preview' - wait_for_requests - - # the above generates a sublist list in RedCarpet - expect(page).to have_content("sublist") - expect(page).to have_xpath("//ol//li//ul") - end end end diff --git a/spec/features/projects/jobs_spec.rb b/spec/features/projects/jobs_spec.rb index 24830b2bd3e..65ce872363f 100644 --- a/spec/features/projects/jobs_spec.rb +++ b/spec/features/projects/jobs_spec.rb @@ -220,7 +220,7 @@ describe 'Jobs', :clean_gitlab_redis_shared_state do artifact_request = requests.find { |req| req.url.match(%r{artifacts/download}) } - expect(artifact_request.response_headers["Content-Disposition"]).to eq(%Q{attachment; filename="#{job.artifacts_file.filename}"}) + expect(artifact_request.response_headers["Content-Disposition"]).to eq(%Q{attachment; filename*=UTF-8''#{job.artifacts_file.filename}; filename="#{job.artifacts_file.filename}"}) expect(artifact_request.response_headers['Content-Transfer-Encoding']).to eq("binary") expect(artifact_request.response_headers['Content-Type']).to eq("image/gif") expect(artifact_request.body).to eq(job.artifacts_file.file.read.b) diff --git a/spec/features/projects/serverless/functions_spec.rb b/spec/features/projects/serverless/functions_spec.rb index 766c63725b3..aa71669de98 100644 --- a/spec/features/projects/serverless/functions_spec.rb +++ b/spec/features/projects/serverless/functions_spec.rb @@ -1,6 +1,10 @@ +# frozen_string_literal: true + require 'spec_helper' describe 'Functions', :js do + include KubernetesHelpers + let(:project) { create(:project) } let(:user) { create(:user) } @@ -34,11 +38,14 @@ describe 'Functions', :js do end context 'when the user has a cluster and knative installed and visits the serverless page' do - let!(:cluster) { create(:cluster, :project, :provided_by_gcp) } + let(:cluster) { create(:cluster, :project, :provided_by_gcp) } + let(:service) { cluster.platform_kubernetes } let(:knative) { create(:clusters_applications_knative, :installed, cluster: cluster) } let(:project) { knative.cluster.project } before do + stub_kubeclient_knative_services + stub_kubeclient_service_pods visit project_serverless_functions_path(project) end diff --git a/spec/features/projects/wiki/markdown_preview_spec.rb b/spec/features/projects/wiki/markdown_preview_spec.rb index f505023d1d0..3b469fee867 100644 --- a/spec/features/projects/wiki/markdown_preview_spec.rb +++ b/spec/features/projects/wiki/markdown_preview_spec.rb @@ -175,20 +175,6 @@ describe 'Projects > Wiki > User previews markdown changes', :js do expect(page).to have_content("sublist") expect(page).not_to have_xpath("//ol//li//ul") end - - it 'renders content with RedCarpet when legacy_render is set' do - wiki_page = create(:wiki_page, - wiki: project.wiki, - attrs: { title: 'home', content: "Empty content" }) - visit(project_wiki_edit_path(project, wiki_page, legacy_render: 1)) - - fill_in :wiki_content, with: "1. one\n - sublist\n" - click_on "Preview" - - # the above generates a sublist list in RedCarpet - expect(page).to have_content("sublist") - expect(page).to have_xpath("//ol//li//ul") - end end end diff --git a/spec/features/projects_spec.rb b/spec/features/projects_spec.rb index f7efc3f325c..bc36c6f948f 100644 --- a/spec/features/projects_spec.rb +++ b/spec/features/projects_spec.rb @@ -110,16 +110,23 @@ describe 'Project' do it 'shows project topics' do project.update_attribute(:tag_list, 'topic1') + visit path + expect(page).to have_css('.home-panel-topic-list') - expect(page).to have_content('topic1') + expect(page).to have_link('Topic1', href: explore_projects_path(tag: 'topic1')) end it 'shows up to 3 project tags' do project.update_attribute(:tag_list, 'topic1, topic2, topic3, topic4') + visit path + expect(page).to have_css('.home-panel-topic-list') - expect(page).to have_content('topic1, topic2, topic3 + 1 more') + expect(page).to have_link('Topic1', href: explore_projects_path(tag: 'topic1')) + expect(page).to have_link('Topic2', href: explore_projects_path(tag: 'topic2')) + expect(page).to have_link('Topic3', href: explore_projects_path(tag: 'topic3')) + expect(page).to have_content('+ 1 more') end end diff --git a/spec/features/snippets/show_spec.rb b/spec/features/snippets/show_spec.rb index 367a479f62a..74fdfcf492e 100644 --- a/spec/features/snippets/show_spec.rb +++ b/spec/features/snippets/show_spec.rb @@ -79,35 +79,6 @@ describe 'Snippet', :js do expect(page).not_to have_xpath("//ol//li//ul") end end - - context 'when rendering legacy markdown' do - before do - visit snippet_path(snippet, legacy_render: 1) - - wait_for_requests - end - - it 'renders using RedCarpet' do - expect(page).to have_content("sublist") - expect(page).to have_xpath("//ol//li//ul") - end - end - - context 'with cached CommonMark html' do - let(:snippet) { create(:personal_snippet, :public, file_name: file_name, content: content, cached_markdown_version: CacheMarkdownField::CACHE_COMMONMARK_VERSION) } - - it 'renders correctly' do - expect(page).not_to have_xpath("//ol//li//ul") - end - end - - context 'with cached Redcarpet html' do - let(:snippet) { create(:personal_snippet, :public, file_name: file_name, content: content, cached_markdown_version: CacheMarkdownField::CACHE_REDCARPET_VERSION) } - - it 'renders correctly' do - expect(page).to have_xpath("//ol//li//ul") - end - end end context 'switching to the simple viewer' do diff --git a/spec/features/task_lists_spec.rb b/spec/features/task_lists_spec.rb index b549f2b5c62..6fe840dccf6 100644 --- a/spec/features/task_lists_spec.rb +++ b/spec/features/task_lists_spec.rb @@ -36,19 +36,6 @@ describe 'Task Lists' do MARKDOWN end - let(:nested_tasks_markdown_redcarpet) do - <<-EOT.strip_heredoc - - [ ] Task a - - [x] Task a.1 - - [ ] Task a.2 - - [ ] Task b - - 1. [ ] Task 1 - 1. [ ] Task 1.1 - 1. [x] Task 1.2 - EOT - end - let(:nested_tasks_markdown) do <<-EOT.strip_heredoc - [ ] Task a @@ -153,61 +140,6 @@ describe 'Task Lists' do expect(page).to have_content("1 of 1 task completed") end end - - shared_examples 'shared nested tasks' do - before do - allow(Banzai::Filter::MarkdownFilter).to receive(:engine).and_return('Redcarpet') - visit_issue(project, issue) - end - it 'renders' do - expect(page).to have_selector('ul.task-list', count: 2) - expect(page).to have_selector('li.task-list-item', count: 7) - expect(page).to have_selector('ul input[checked]', count: 1) - expect(page).to have_selector('ol input[checked]', count: 1) - end - - it 'solves tasks' do - expect(page).to have_content("2 of 7 tasks completed") - - page.find('li.task-list-item', text: 'Task b').find('input').click - wait_for_requests - page.find('li.task-list-item ul li.task-list-item', text: 'Task a.2').find('input').click - wait_for_requests - page.find('li.task-list-item ol li.task-list-item', text: 'Task 1.1').find('input').click - wait_for_requests - - expect(page).to have_content("5 of 7 tasks completed") - - visit_issue(project, issue) # reload to see new system notes - - expect(page).to have_content('marked the task Task b as complete') - expect(page).to have_content('marked the task Task a.2 as complete') - expect(page).to have_content('marked the task Task 1.1 as complete') - end - end - - describe 'nested tasks', :js do - let(:cache_version) { CacheMarkdownField::CACHE_COMMONMARK_VERSION } - let!(:issue) do - create(:issue, description: nested_tasks_markdown, author: user, project: project, - cached_markdown_version: cache_version) - end - - before do - visit_issue(project, issue) - end - - context 'with Redcarpet' do - let(:cache_version) { CacheMarkdownField::CACHE_REDCARPET_VERSION } - let(:nested_tasks_markdown) { nested_tasks_markdown_redcarpet } - - it_behaves_like 'shared nested tasks' - end - - context 'with CommonMark' do - it_behaves_like 'shared nested tasks' - end - end end describe 'for Notes' do diff --git a/spec/features/users/overview_spec.rb b/spec/features/users/overview_spec.rb index 3708f0ee477..3db9ae7a951 100644 --- a/spec/features/users/overview_spec.rb +++ b/spec/features/users/overview_spec.rb @@ -34,7 +34,7 @@ describe 'Overview tab on a user profile', :js do it 'does not show any entries in the list of activities' do page.within('.activities-block') do expect(page).to have_selector('.loading', visible: false) - expect(page).to have_content('No activities found') + expect(page).to have_content('Join or create a group to start contributing by commenting on issues or submitting merge requests!') expect(page).not_to have_selector('.event-item') end end @@ -96,7 +96,7 @@ describe 'Overview tab on a user profile', :js do it 'it shows an empty project list with an info message' do page.within('.projects-block') do expect(page).to have_selector('.loading', visible: false) - expect(page).to have_content('This user doesn\'t have any personal projects') + expect(page).to have_content('You haven\'t created any personal projects.') expect(page).not_to have_selector('.project-row') end end diff --git a/spec/finders/issues_finder_spec.rb b/spec/finders/issues_finder_spec.rb index 682fae06434..34cb09942be 100644 --- a/spec/finders/issues_finder_spec.rb +++ b/spec/finders/issues_finder_spec.rb @@ -314,6 +314,14 @@ describe IssuesFinder do end end + context 'filtering by issue term in title' do + let(:params) { { search: 'git', in: 'title' } } + + it 'returns issues with title match for search term' do + expect(issues).to contain_exactly(issue1) + end + end + context 'filtering by issues iids' do let(:params) { { iids: issue3.iid } } diff --git a/spec/fixtures/api/schemas/entities/merge_request_widget.json b/spec/fixtures/api/schemas/entities/merge_request_widget.json index 1bd39a46830..67c209f3fc3 100644 --- a/spec/fixtures/api/schemas/entities/merge_request_widget.json +++ b/spec/fixtures/api/schemas/entities/merge_request_widget.json @@ -44,7 +44,7 @@ "merge_user": { "type": ["object", "null"] }, "diff_head_sha": { "type": ["string", "null"] }, "diff_head_commit_short_id": { "type": ["string", "null"] }, - "merge_commit_message": { "type": ["string", "null"] }, + "default_merge_commit_message": { "type": ["string", "null"] }, "pipeline": { "type": ["object", "null"] }, "merge_pipeline": { "type": ["object", "null"] }, "work_in_progress": { "type": "boolean" }, @@ -102,7 +102,9 @@ "new_blob_path": { "type": ["string", "null"] }, "merge_check_path": { "type": "string" }, "ci_environments_status_path": { "type": "string" }, - "merge_commit_message_with_description": { "type": "string" }, + "default_merge_commit_message_with_description": { "type": "string" }, + "default_squash_commit_message": { "type": "string" }, + "commits_without_merge_commits": { "type": "array" }, "diverged_commits_count": { "type": "integer" }, "commit_change_content_path": { "type": "string" }, "merge_commit_path": { "type": ["string", "null"] }, diff --git a/spec/fixtures/api/schemas/public_api/v4/group_labels.json b/spec/fixtures/api/schemas/public_api/v4/group_labels.json new file mode 100644 index 00000000000..f6c327abfdd --- /dev/null +++ b/spec/fixtures/api/schemas/public_api/v4/group_labels.json @@ -0,0 +1,18 @@ +{ + "type": "array", + "items": { + "type": "object", + "properties" : { + "id" : { "type": "integer" }, + "name" : { "type": "string "}, + "color" : { "type": "string "}, + "description" : { "type": "string "}, + "open_issues_count" : { "type": "integer "}, + "closed_issues_count" : { "type": "integer "}, + "open_merge_requests_count" : { "type": "integer "}, + "subscribed" : { "type": "boolean" }, + "priority" : { "type": "null" } + }, + "additionalProperties": false + } +} diff --git a/spec/fixtures/markdown.md.erb b/spec/fixtures/markdown.md.erb index e5d01c3bd03..bbeacf1707b 100644 --- a/spec/fixtures/markdown.md.erb +++ b/spec/fixtures/markdown.md.erb @@ -6,7 +6,7 @@ started. ## Markdown -GitLab uses [Redcarpet](http://git.io/ld_NVQ) to parse all Markdown into +GitLab uses [Commonmark](https://git.io/fhDag) to parse all Markdown into HTML. It has some special features. Let's try 'em out! diff --git a/spec/helpers/issuables_helper_spec.rb b/spec/helpers/issuables_helper_spec.rb index af319e5ebfe..8b82dea2524 100644 --- a/spec/helpers/issuables_helper_spec.rb +++ b/spec/helpers/issuables_helper_spec.rb @@ -188,7 +188,6 @@ describe IssuablesHelper do issuableRef: "##{issue.iid}", markdownPreviewPath: "/#{@project.full_path}/preview_markdown", markdownDocsPath: '/help/user/markdown', - markdownVersion: CacheMarkdownField::CACHE_COMMONMARK_VERSION, issuableTemplates: [], lockVersion: issue.lock_version, projectPath: @project.path, diff --git a/spec/helpers/markup_helper_spec.rb b/spec/helpers/markup_helper_spec.rb index a0c0af94fa5..c3956ba08fd 100644 --- a/spec/helpers/markup_helper_spec.rb +++ b/spec/helpers/markup_helper_spec.rb @@ -212,17 +212,6 @@ describe MarkupHelper do helper.render_wiki_content(@wiki) end - it 'uses Wiki pipeline for markdown files with RedCarpet if feature disabled' do - stub_feature_flags(commonmark_for_repositories: false) - allow(@wiki).to receive(:format).and_return(:markdown) - - expect(helper).to receive(:markdown_unsafe).with('wiki content', - pipeline: :wiki, project: project, project_wiki: @wiki, page_slug: "nested/page", - issuable_state_filter_enabled: true, markdown_engine: :redcarpet) - - helper.render_wiki_content(@wiki) - end - it "uses Asciidoctor for asciidoc files" do allow(@wiki).to receive(:format).and_return(:asciidoc) @@ -273,16 +262,6 @@ describe MarkupHelper do it 'defaults to CommonMark' do expect(helper.markup('foo.md', 'x^2')).to include('x^2') end - - it 'honors markdown_engine for RedCarpet' do - expect(helper.markup('foo.md', 'x^2', { markdown_engine: :redcarpet })).to include('x<sup>2</sup>') - end - - it 'uses RedCarpet if feature disabled' do - stub_feature_flags(commonmark_for_repositories: false) - - expect(helper.markup('foo.md', 'x^2', { markdown_engine: :redcarpet })).to include('x<sup>2</sup>') - end end describe '#first_line_in_markdown' do diff --git a/spec/helpers/projects_helper_spec.rb b/spec/helpers/projects_helper_spec.rb index 10f61731206..49895b0680b 100644 --- a/spec/helpers/projects_helper_spec.rb +++ b/spec/helpers/projects_helper_spec.rb @@ -372,21 +372,16 @@ describe ProjectsHelper do end context 'when project has external wiki' do - before do - allow(project).to receive(:has_external_wiki?).and_return(true) - end - it 'includes external wiki tab' do + project.create_external_wiki_service(active: true, properties: { 'external_wiki_url' => 'https://gitlab.com' }) + is_expected.to include(:external_wiki) end end context 'when project does not have external wiki' do - before do - allow(project).to receive(:has_external_wiki?).and_return(false) - end - it 'does not include external wiki tab' do + expect(project.external_wiki).to be_nil is_expected.not_to include(:external_wiki) end end @@ -540,18 +535,6 @@ describe ProjectsHelper do end end - describe '#legacy_render_context' do - it 'returns the redcarpet engine' do - params = { legacy_render: '1' } - - expect(helper.legacy_render_context(params)).to include(markdown_engine: :redcarpet) - end - - it 'returns nothing' do - expect(helper.legacy_render_context({})).to be_empty - end - end - describe '#explore_projects_tab?' do subject { helper.explore_projects_tab? } diff --git a/spec/helpers/users_helper_spec.rb b/spec/helpers/users_helper_spec.rb index 34d9115a1f6..f3649495493 100644 --- a/spec/helpers/users_helper_spec.rb +++ b/spec/helpers/users_helper_spec.rb @@ -51,7 +51,7 @@ describe UsersHelper do false | 'mockRegexPattern' | { user_internal_regex_pattern: nil, user_internal_regex_options: nil } true | nil | { user_internal_regex_pattern: nil, user_internal_regex_options: nil } true | '' | { user_internal_regex_pattern: nil, user_internal_regex_options: nil } - true | 'mockRegexPattern' | { user_internal_regex_pattern: 'mockRegexPattern', user_internal_regex_options: 'gi' } + true | 'mockRegexPattern' | { user_internal_regex_pattern: 'mockRegexPattern', user_internal_regex_options: 'i' } end with_them do @@ -100,4 +100,72 @@ describe UsersHelper do end end end + + describe '#user_badges_in_admin_section' do + before do + allow(helper).to receive(:current_user).and_return(user) + end + + context 'with a blocked user' do + it "returns the blocked badge" do + blocked_user = create(:user, state: 'blocked') + + badges = helper.user_badges_in_admin_section(blocked_user) + + expect(badges).to eq([text: "Blocked", variant: "danger"]) + end + end + + context 'with an admin user' do + it "returns the admin badge" do + admin_user = create(:admin) + + badges = helper.user_badges_in_admin_section(admin_user) + + expect(badges).to eq([text: "Admin", variant: "success"]) + end + end + + context 'with an external user' do + it 'returns the external badge' do + external_user = create(:user, external: true) + + badges = helper.user_badges_in_admin_section(external_user) + + expect(badges).to eq([text: "External", variant: "secondary"]) + end + end + + context 'with the current user' do + it 'returns the "It\'s You" badge' do + badges = helper.user_badges_in_admin_section(user) + + expect(badges).to eq([text: "It's you!", variant: nil]) + end + end + + context 'with an external blocked admin' do + it 'returns the blocked, admin and external badges' do + user = create(:admin, state: 'blocked', external: true) + + badges = helper.user_badges_in_admin_section(user) + + expect(badges).to eq([ + { text: "Blocked", variant: "danger" }, + { text: "Admin", variant: "success" }, + { text: "External", variant: "secondary" } + ]) + end + end + + context 'get badges for normal user' do + it 'returns no badges' do + user = create(:user) + + badges = helper.user_badges_in_admin_section(user) + + expect(badges).to be_empty + end + end + end end diff --git a/spec/javascripts/api_spec.js b/spec/javascripts/api_spec.js index 9d55c615450..1e9470970ff 100644 --- a/spec/javascripts/api_spec.js +++ b/spec/javascripts/api_spec.js @@ -49,6 +49,22 @@ describe('Api', () => { }); }); + describe('groupMembers', () => { + it('fetches group members', done => { + const groupId = '54321'; + const expectedUrl = `${dummyUrlRoot}/api/${dummyApiVersion}/groups/${groupId}/members`; + const expectedData = [{ id: 7 }]; + mock.onGet(expectedUrl).reply(200, expectedData); + + Api.groupMembers(groupId) + .then(({ data }) => { + expect(data).toEqual(expectedData); + }) + .then(done) + .catch(done.fail); + }); + }); + describe('groups', () => { it('fetches groups', done => { const query = 'dummy query'; diff --git a/spec/javascripts/behaviors/copy_as_gfm_spec.js b/spec/javascripts/behaviors/copy_as_gfm_spec.js index 6179a02ce16..ca849f75860 100644 --- a/spec/javascripts/behaviors/copy_as_gfm_spec.js +++ b/spec/javascripts/behaviors/copy_as_gfm_spec.js @@ -1,4 +1,4 @@ -import { CopyAsGFM } from '~/behaviors/markdown/copy_as_gfm'; +import initCopyAsGFM, { CopyAsGFM } from '~/behaviors/markdown/copy_as_gfm'; describe('CopyAsGFM', () => { describe('CopyAsGFM.pasteGFM', () => { @@ -79,27 +79,46 @@ describe('CopyAsGFM', () => { return clipboardData; }; + beforeAll(done => { + initCopyAsGFM(); + + // Fake call to nodeToGfm so the import of lazy bundle happened + CopyAsGFM.nodeToGFM(document.createElement('div')) + .then(() => { + done(); + }) + .catch(done.fail); + }); + beforeEach(() => spyOn(clipboardData, 'setData')); describe('list handling', () => { - it('uses correct gfm for unordered lists', () => { + it('uses correct gfm for unordered lists', done => { const selection = stubSelection('<li>List Item1</li><li>List Item2</li>\n', 'UL'); + spyOn(window, 'getSelection').and.returnValue(selection); simulateCopy(); - const expectedGFM = '* List Item1\n\n* List Item2'; + setTimeout(() => { + const expectedGFM = '* List Item1\n\n* List Item2'; - expect(clipboardData.setData).toHaveBeenCalledWith('text/x-gfm', expectedGFM); + expect(clipboardData.setData).toHaveBeenCalledWith('text/x-gfm', expectedGFM); + done(); + }); }); - it('uses correct gfm for ordered lists', () => { + it('uses correct gfm for ordered lists', done => { const selection = stubSelection('<li>List Item1</li><li>List Item2</li>\n', 'OL'); + spyOn(window, 'getSelection').and.returnValue(selection); simulateCopy(); - const expectedGFM = '1. List Item1\n\n1. List Item2'; + setTimeout(() => { + const expectedGFM = '1. List Item1\n\n1. List Item2'; - expect(clipboardData.setData).toHaveBeenCalledWith('text/x-gfm', expectedGFM); + expect(clipboardData.setData).toHaveBeenCalledWith('text/x-gfm', expectedGFM); + done(); + }); }); }); }); diff --git a/spec/javascripts/behaviors/shortcuts/shortcuts_issuable_spec.js b/spec/javascripts/behaviors/shortcuts/shortcuts_issuable_spec.js index fe827bb1e18..4843a0386b5 100644 --- a/spec/javascripts/behaviors/shortcuts/shortcuts_issuable_spec.js +++ b/spec/javascripts/behaviors/shortcuts/shortcuts_issuable_spec.js @@ -3,17 +3,26 @@ */ import $ from 'jquery'; -import initCopyAsGFM from '~/behaviors/markdown/copy_as_gfm'; +import initCopyAsGFM, { CopyAsGFM } from '~/behaviors/markdown/copy_as_gfm'; import ShortcutsIssuable from '~/behaviors/shortcuts/shortcuts_issuable'; -initCopyAsGFM(); - const FORM_SELECTOR = '.js-main-target-form .js-vue-comment-form'; describe('ShortcutsIssuable', function() { const fixtureName = 'snippets/show.html.raw'; preloadFixtures(fixtureName); + beforeAll(done => { + initCopyAsGFM(); + + // Fake call to nodeToGfm so the import of lazy bundle happened + CopyAsGFM.nodeToGFM(document.createElement('div')) + .then(() => { + done(); + }) + .catch(done.fail); + }); + beforeEach(() => { loadFixtures(fixtureName); $('body').append( @@ -63,17 +72,22 @@ describe('ShortcutsIssuable', function() { stubSelection('<p>Selected text.</p>'); }); - it('leaves existing input intact', () => { + it('leaves existing input intact', done => { $(FORM_SELECTOR).val('This text was already here.'); expect($(FORM_SELECTOR).val()).toBe('This text was already here.'); ShortcutsIssuable.replyWithSelectedText(true); - expect($(FORM_SELECTOR).val()).toBe('This text was already here.\n\n> Selected text.\n\n'); + setTimeout(() => { + expect($(FORM_SELECTOR).val()).toBe( + 'This text was already here.\n\n> Selected text.\n\n', + ); + done(); + }); }); - it('triggers `input`', () => { + it('triggers `input`', done => { let triggered = false; $(FORM_SELECTOR).on('input', () => { triggered = true; @@ -81,36 +95,48 @@ describe('ShortcutsIssuable', function() { ShortcutsIssuable.replyWithSelectedText(true); - expect(triggered).toBe(true); + setTimeout(() => { + expect(triggered).toBe(true); + done(); + }); }); - it('triggers `focus`', () => { + it('triggers `focus`', done => { const spy = spyOn(document.querySelector(FORM_SELECTOR), 'focus'); ShortcutsIssuable.replyWithSelectedText(true); - expect(spy).toHaveBeenCalled(); + setTimeout(() => { + expect(spy).toHaveBeenCalled(); + done(); + }); }); }); describe('with a one-line selection', () => { - it('quotes the selection', () => { + it('quotes the selection', done => { stubSelection('<p>This text has been selected.</p>'); ShortcutsIssuable.replyWithSelectedText(true); - expect($(FORM_SELECTOR).val()).toBe('> This text has been selected.\n\n'); + setTimeout(() => { + expect($(FORM_SELECTOR).val()).toBe('> This text has been selected.\n\n'); + done(); + }); }); }); describe('with a multi-line selection', () => { - it('quotes the selected lines as a group', () => { + it('quotes the selected lines as a group', done => { stubSelection( '<p>Selected line one.</p>\n<p>Selected line two.</p>\n<p>Selected line three.</p>', ); ShortcutsIssuable.replyWithSelectedText(true); - expect($(FORM_SELECTOR).val()).toBe( - '> Selected line one.\n>\n> Selected line two.\n>\n> Selected line three.\n\n', - ); + setTimeout(() => { + expect($(FORM_SELECTOR).val()).toBe( + '> Selected line one.\n>\n> Selected line two.\n>\n> Selected line three.\n\n', + ); + done(); + }); }); }); @@ -119,17 +145,23 @@ describe('ShortcutsIssuable', function() { stubSelection('<p>Selected text.</p>', true); }); - it('does not add anything to the input', () => { + it('does not add anything to the input', done => { ShortcutsIssuable.replyWithSelectedText(true); - expect($(FORM_SELECTOR).val()).toBe(''); + setTimeout(() => { + expect($(FORM_SELECTOR).val()).toBe(''); + done(); + }); }); - it('triggers `focus`', () => { + it('triggers `focus`', done => { const spy = spyOn(document.querySelector(FORM_SELECTOR), 'focus'); ShortcutsIssuable.replyWithSelectedText(true); - expect(spy).toHaveBeenCalled(); + setTimeout(() => { + expect(spy).toHaveBeenCalled(); + done(); + }); }); }); @@ -138,20 +170,26 @@ describe('ShortcutsIssuable', function() { stubSelection('<div class="md">Selected text.</div><p>Invalid selected text.</p>', true); }); - it('only adds the valid part to the input', () => { + it('only adds the valid part to the input', done => { ShortcutsIssuable.replyWithSelectedText(true); - expect($(FORM_SELECTOR).val()).toBe('> Selected text.\n\n'); + setTimeout(() => { + expect($(FORM_SELECTOR).val()).toBe('> Selected text.\n\n'); + done(); + }); }); - it('triggers `focus`', () => { + it('triggers `focus`', done => { const spy = spyOn(document.querySelector(FORM_SELECTOR), 'focus'); ShortcutsIssuable.replyWithSelectedText(true); - expect(spy).toHaveBeenCalled(); + setTimeout(() => { + expect(spy).toHaveBeenCalled(); + done(); + }); }); - it('triggers `input`', () => { + it('triggers `input`', done => { let triggered = false; $(FORM_SELECTOR).on('input', () => { triggered = true; @@ -159,7 +197,10 @@ describe('ShortcutsIssuable', function() { ShortcutsIssuable.replyWithSelectedText(true); - expect(triggered).toBe(true); + setTimeout(() => { + expect(triggered).toBe(true); + done(); + }); }); }); @@ -183,20 +224,26 @@ describe('ShortcutsIssuable', function() { }); }); - it('adds the quoted selection to the input', () => { + it('adds the quoted selection to the input', done => { ShortcutsIssuable.replyWithSelectedText(true); - expect($(FORM_SELECTOR).val()).toBe('> *Selected text.*\n\n'); + setTimeout(() => { + expect($(FORM_SELECTOR).val()).toBe('> *Selected text.*\n\n'); + done(); + }); }); - it('triggers `focus`', () => { + it('triggers `focus`', done => { const spy = spyOn(document.querySelector(FORM_SELECTOR), 'focus'); ShortcutsIssuable.replyWithSelectedText(true); - expect(spy).toHaveBeenCalled(); + setTimeout(() => { + expect(spy).toHaveBeenCalled(); + done(); + }); }); - it('triggers `input`', () => { + it('triggers `input`', done => { let triggered = false; $(FORM_SELECTOR).on('input', () => { triggered = true; @@ -204,7 +251,10 @@ describe('ShortcutsIssuable', function() { ShortcutsIssuable.replyWithSelectedText(true); - expect(triggered).toBe(true); + setTimeout(() => { + expect(triggered).toBe(true); + done(); + }); }); }); @@ -228,17 +278,23 @@ describe('ShortcutsIssuable', function() { }); }); - it('does not add anything to the input', () => { + it('does not add anything to the input', done => { ShortcutsIssuable.replyWithSelectedText(true); - expect($(FORM_SELECTOR).val()).toBe(''); + setTimeout(() => { + expect($(FORM_SELECTOR).val()).toBe(''); + done(); + }); }); - it('triggers `focus`', () => { + it('triggers `focus`', done => { const spy = spyOn(document.querySelector(FORM_SELECTOR), 'focus'); ShortcutsIssuable.replyWithSelectedText(true); - expect(spy).toHaveBeenCalled(); + setTimeout(() => { + expect(spy).toHaveBeenCalled(); + done(); + }); }); }); }); diff --git a/spec/javascripts/diffs/components/compare_versions_spec.js b/spec/javascripts/diffs/components/compare_versions_spec.js index 2f0385454d7..e886f962d2f 100644 --- a/spec/javascripts/diffs/components/compare_versions_spec.js +++ b/spec/javascripts/diffs/components/compare_versions_spec.js @@ -10,6 +10,10 @@ describe('CompareVersions', () => { const targetBranch = { branchName: 'tmp-wine-dev', versionIndex: -1 }; beforeEach(() => { + store.state.diffs.addedLines = 10; + store.state.diffs.removedLines = 20; + store.state.diffs.diffFiles.push('test'); + vm = createComponentWithStore(Vue.extend(CompareVersionsComponent), store, { mergeRequestDiffs: diffsMockData, mergeRequestDiff: diffsMockData[0], diff --git a/spec/javascripts/diffs/components/diff_file_header_spec.js b/spec/javascripts/diffs/components/diff_file_header_spec.js index b77907ff26f..787a81fd88f 100644 --- a/spec/javascripts/diffs/components/diff_file_header_spec.js +++ b/spec/javascripts/diffs/components/diff_file_header_spec.js @@ -24,6 +24,10 @@ describe('diff_file_header', () => { beforeEach(() => { const diffFile = diffDiscussionMock.diff_file; + + diffFile.added_lines = 2; + diffFile.removed_lines = 1; + props = { diffFile: { ...diffFile }, canCurrentUserFork: false, diff --git a/spec/javascripts/diffs/components/diff_stats_spec.js b/spec/javascripts/diffs/components/diff_stats_spec.js new file mode 100644 index 00000000000..984b3026209 --- /dev/null +++ b/spec/javascripts/diffs/components/diff_stats_spec.js @@ -0,0 +1,33 @@ +import { shallowMount } from '@vue/test-utils'; +import DiffStats from '~/diffs/components/diff_stats.vue'; + +describe('diff_stats', () => { + it('does not render a group if diffFileLengths is not passed in', () => { + const wrapper = shallowMount(DiffStats, { + propsData: { + addedLines: 1, + removedLines: 2, + }, + }); + const groups = wrapper.findAll('.diff-stats-group'); + + expect(groups.length).toBe(2); + }); + + it('shows amount of files changed, lines added and lines removed when passed all props', () => { + const wrapper = shallowMount(DiffStats, { + propsData: { + addedLines: 100, + removedLines: 200, + diffFilesLength: 300, + }, + }); + const additions = wrapper.find('icon-stub[name="file-addition"]').element.parentNode; + const deletions = wrapper.find('icon-stub[name="file-deletion"]').element.parentNode; + const filesChanged = wrapper.find('icon-stub[name="doc-code"]').element.parentNode; + + expect(additions.textContent).toContain('100'); + expect(deletions.textContent).toContain('200'); + expect(filesChanged.textContent).toContain('300'); + }); +}); diff --git a/spec/javascripts/diffs/components/tree_list_spec.js b/spec/javascripts/diffs/components/tree_list_spec.js index 08b0b4f9e45..9e556698f34 100644 --- a/spec/javascripts/diffs/components/tree_list_spec.js +++ b/spec/javascripts/diffs/components/tree_list_spec.js @@ -35,12 +35,6 @@ describe('Diffs tree list component', () => { vm.$destroy(); }); - it('renders diff stats', () => { - expect(vm.$el.textContent).toContain('1 changed file'); - expect(vm.$el.textContent).toContain('10 additions'); - expect(vm.$el.textContent).toContain('20 deletions'); - }); - it('renders empty text', () => { expect(vm.$el.textContent).toContain('No files found'); }); @@ -83,17 +77,6 @@ describe('Diffs tree list component', () => { expect(vm.$el.querySelectorAll('.file-row')[1].textContent).toContain('app'); }); - it('filters tree list to blobs matching search', done => { - vm.search = 'app/index'; - - vm.$nextTick(() => { - expect(vm.$el.querySelectorAll('.file-row').length).toBe(1); - expect(vm.$el.querySelectorAll('.file-row')[0].textContent).toContain('index.js'); - - done(); - }); - }); - it('calls toggleTreeOpen when clicking folder', () => { spyOn(vm.$store, 'dispatch').and.stub(); @@ -130,14 +113,4 @@ describe('Diffs tree list component', () => { }); }); }); - - describe('clearSearch', () => { - it('resets search', () => { - vm.search = 'test'; - - vm.$el.querySelector('.tree-list-clear-icon').click(); - - expect(vm.search).toBe(''); - }); - }); }); diff --git a/spec/javascripts/diffs/store/getters_spec.js b/spec/javascripts/diffs/store/getters_spec.js index 190ca1230ca..4f69dc92ab8 100644 --- a/spec/javascripts/diffs/store/getters_spec.js +++ b/spec/javascripts/diffs/store/getters_spec.js @@ -242,7 +242,11 @@ describe('Diffs Module Getters', () => { }, }; - expect(getters.allBlobs(localState)).toEqual([ + expect( + getters.allBlobs(localState, { + flatBlobsList: getters.flatBlobsList(localState), + }), + ).toEqual([ { isHeader: true, path: '/', diff --git a/spec/javascripts/helpers/vue_test_utils_helper.js b/spec/javascripts/helpers/vue_test_utils_helper.js new file mode 100644 index 00000000000..19e27388eeb --- /dev/null +++ b/spec/javascripts/helpers/vue_test_utils_helper.js @@ -0,0 +1,19 @@ +/* eslint-disable import/prefer-default-export */ + +const vNodeContainsText = (vnode, text) => + (vnode.text && vnode.text.includes(text)) || + (vnode.children && vnode.children.filter(child => vNodeContainsText(child, text)).length); + +/** + * Determines whether a `shallowMount` Wrapper contains text + * within one of it's slots. This will also work on Wrappers + * acquired with `find()`, but only if it's parent Wrapper + * was shallowMounted. + * NOTE: Prefer checking the rendered output of a component + * wherever possible using something like `text()` instead. + * @param {Wrapper} shallowWrapper - Vue test utils wrapper (shallowMounted) + * @param {String} slotName + * @param {String} text + */ +export const shallowWrapperContainsSlotText = (shallowWrapper, slotName, text) => + !!shallowWrapper.vm.$slots[slotName].filter(vnode => vNodeContainsText(vnode, text)).length; diff --git a/spec/javascripts/helpers/vue_test_utils_helper_spec.js b/spec/javascripts/helpers/vue_test_utils_helper_spec.js new file mode 100644 index 00000000000..41714066da5 --- /dev/null +++ b/spec/javascripts/helpers/vue_test_utils_helper_spec.js @@ -0,0 +1,48 @@ +import { shallowMount } from '@vue/test-utils'; +import { shallowWrapperContainsSlotText } from './vue_test_utils_helper'; + +describe('Vue test utils helpers', () => { + describe('shallowWrapperContainsSlotText', () => { + const mockText = 'text'; + const mockSlot = `<div>${mockText}</div>`; + let mockComponent; + + beforeEach(() => { + mockComponent = shallowMount( + { + render(h) { + h(`<div>mockedComponent</div>`); + }, + }, + { + slots: { + default: mockText, + namedSlot: mockSlot, + }, + }, + ); + }); + + it('finds text within shallowWrapper default slot', () => { + expect(shallowWrapperContainsSlotText(mockComponent, 'default', mockText)).toBe(true); + }); + + it('finds text within shallowWrapper named slot', () => { + expect(shallowWrapperContainsSlotText(mockComponent, 'namedSlot', mockText)).toBe(true); + }); + + it('returns false when text is not present', () => { + const searchText = 'absent'; + + expect(shallowWrapperContainsSlotText(mockComponent, 'default', searchText)).toBe(false); + expect(shallowWrapperContainsSlotText(mockComponent, 'namedSlot', searchText)).toBe(false); + }); + + it('searches with case-sensitivity', () => { + const searchText = mockText.toUpperCase(); + + expect(shallowWrapperContainsSlotText(mockComponent, 'default', searchText)).toBe(false); + expect(shallowWrapperContainsSlotText(mockComponent, 'namedSlot', searchText)).toBe(false); + }); + }); +}); diff --git a/spec/javascripts/ide/components/ide_spec.js b/spec/javascripts/ide/components/ide_spec.js index 55f40be0e4e..dc5790f6562 100644 --- a/spec/javascripts/ide/components/ide_spec.js +++ b/spec/javascripts/ide/components/ide_spec.js @@ -1,5 +1,4 @@ import Vue from 'vue'; -import Mousetrap from 'mousetrap'; import store from '~/ide/stores'; import ide from '~/ide/components/ide.vue'; import { createComponentWithStore } from 'spec/helpers/vue_mount_component_helper'; @@ -72,73 +71,6 @@ describe('ide component', () => { }); }); - describe('file finder', () => { - beforeEach(done => { - spyOn(vm, 'toggleFileFinder'); - - vm.$store.state.fileFindVisible = true; - - vm.$nextTick(done); - }); - - it('calls toggleFileFinder on `t` key press', done => { - Mousetrap.trigger('t'); - - vm.$nextTick() - .then(() => { - expect(vm.toggleFileFinder).toHaveBeenCalled(); - }) - .then(done) - .catch(done.fail); - }); - - it('calls toggleFileFinder on `command+p` key press', done => { - Mousetrap.trigger('command+p'); - - vm.$nextTick() - .then(() => { - expect(vm.toggleFileFinder).toHaveBeenCalled(); - }) - .then(done) - .catch(done.fail); - }); - - it('calls toggleFileFinder on `ctrl+p` key press', done => { - Mousetrap.trigger('ctrl+p'); - - vm.$nextTick() - .then(() => { - expect(vm.toggleFileFinder).toHaveBeenCalled(); - }) - .then(done) - .catch(done.fail); - }); - - it('always allows `command+p` to trigger toggleFileFinder', () => { - expect( - vm.mousetrapStopCallback(null, vm.$el.querySelector('.dropdown-input-field'), 'command+p'), - ).toBe(false); - }); - - it('always allows `ctrl+p` to trigger toggleFileFinder', () => { - expect( - vm.mousetrapStopCallback(null, vm.$el.querySelector('.dropdown-input-field'), 'ctrl+p'), - ).toBe(false); - }); - - it('onlys handles `t` when focused in input-field', () => { - expect( - vm.mousetrapStopCallback(null, vm.$el.querySelector('.dropdown-input-field'), 't'), - ).toBe(true); - }); - - it('stops callback in monaco editor', () => { - setFixtures('<div class="inputarea"></div>'); - - expect(vm.mousetrapStopCallback(null, document.querySelector('.inputarea'), 't')).toBe(true); - }); - }); - it('shows error message when set', done => { expect(vm.$el.querySelector('.flash-container')).toBe(null); diff --git a/spec/javascripts/lib/utils/common_utils_spec.js b/spec/javascripts/lib/utils/common_utils_spec.js index e3fd9604474..3eff3f655ee 100644 --- a/spec/javascripts/lib/utils/common_utils_spec.js +++ b/spec/javascripts/lib/utils/common_utils_spec.js @@ -232,6 +232,21 @@ describe('common_utils', () => { }); }); + describe('debounceByAnimationFrame', () => { + it('debounces a function to allow a maximum of one call per animation frame', done => { + const spy = jasmine.createSpy('spy'); + const debouncedSpy = commonUtils.debounceByAnimationFrame(spy); + window.requestAnimationFrame(() => { + debouncedSpy(); + debouncedSpy(); + window.requestAnimationFrame(() => { + expect(spy).toHaveBeenCalledTimes(1); + done(); + }); + }); + }); + }); + describe('getParameterByName', () => { beforeEach(() => { window.history.pushState({}, null, '?scope=all&p=2'); diff --git a/spec/javascripts/lib/utils/file_upload_spec.js b/spec/javascripts/lib/utils/file_upload_spec.js index 92c9cc70aaf..8f7092f63de 100644 --- a/spec/javascripts/lib/utils/file_upload_spec.js +++ b/spec/javascripts/lib/utils/file_upload_spec.js @@ -9,28 +9,56 @@ describe('File upload', () => { <span class="js-filename"></span> </form> `); + }); + + describe('when there is a matching button and input', () => { + beforeEach(() => { + fileUpload('.js-button', '.js-input'); + }); + + it('clicks file input after clicking button', () => { + const btn = document.querySelector('.js-button'); + const input = document.querySelector('.js-input'); + + spyOn(input, 'click'); + + btn.click(); + + expect(input.click).toHaveBeenCalled(); + }); + + it('updates file name text', () => { + const input = document.querySelector('.js-input'); - fileUpload('.js-button', '.js-input'); + input.value = 'path/to/file/index.js'; + + input.dispatchEvent(new CustomEvent('change')); + + expect(document.querySelector('.js-filename').textContent).toEqual('index.js'); + }); }); - it('clicks file input after clicking button', () => { - const btn = document.querySelector('.js-button'); + it('fails gracefully when there is no matching button', () => { const input = document.querySelector('.js-input'); + const btn = document.querySelector('.js-button'); + fileUpload('.js-not-button', '.js-input'); spyOn(input, 'click'); btn.click(); - expect(input.click).toHaveBeenCalled(); + expect(input.click).not.toHaveBeenCalled(); }); - it('updates file name text', () => { + it('fails gracefully when there is no matching input', () => { const input = document.querySelector('.js-input'); + const btn = document.querySelector('.js-button'); + fileUpload('.js-button', '.js-not-input'); - input.value = 'path/to/file/index.js'; + spyOn(input, 'click'); - input.dispatchEvent(new CustomEvent('change')); + btn.click(); - expect(document.querySelector('.js-filename').textContent).toEqual('index.js'); + expect(input.click).not.toHaveBeenCalled(); }); }); diff --git a/spec/javascripts/lib/utils/grammar_spec.js b/spec/javascripts/lib/utils/grammar_spec.js new file mode 100644 index 00000000000..377b2ffb48c --- /dev/null +++ b/spec/javascripts/lib/utils/grammar_spec.js @@ -0,0 +1,35 @@ +import * as grammar from '~/lib/utils/grammar'; + +describe('utils/grammar', () => { + describe('toNounSeriesText', () => { + it('with empty items returns empty string', () => { + expect(grammar.toNounSeriesText([])).toBe(''); + }); + + it('with single item returns item', () => { + const items = ['Lorem Ipsum']; + + expect(grammar.toNounSeriesText(items)).toBe(items[0]); + }); + + it('with 2 items returns item1 and item2', () => { + const items = ['Dolar', 'Sit Amit']; + + expect(grammar.toNounSeriesText(items)).toBe(`${items[0]} and ${items[1]}`); + }); + + it('with 3 items returns comma separated series', () => { + const items = ['Lorem', 'Ipsum', 'dolar']; + const expected = 'Lorem, Ipsum, and dolar'; + + expect(grammar.toNounSeriesText(items)).toBe(expected); + }); + + it('with 6 items returns comma separated series', () => { + const items = ['Lorem', 'ipsum', 'dolar', 'sit', 'amit', 'consectetur']; + const expected = 'Lorem, ipsum, dolar, sit, amit, and consectetur'; + + expect(grammar.toNounSeriesText(items)).toBe(expected); + }); + }); +}); diff --git a/spec/javascripts/lib/utils/icon_utils_spec.js b/spec/javascripts/lib/utils/icon_utils_spec.js new file mode 100644 index 00000000000..3fd3940efe8 --- /dev/null +++ b/spec/javascripts/lib/utils/icon_utils_spec.js @@ -0,0 +1,67 @@ +import MockAdapter from 'axios-mock-adapter'; +import axios from '~/lib/utils/axios_utils'; +import * as iconUtils from '~/lib/utils/icon_utils'; + +describe('Icon utils', () => { + describe('getSvgIconPathContent', () => { + let spriteIcons; + + beforeAll(() => { + spriteIcons = gon.sprite_icons; + gon.sprite_icons = 'mockSpriteIconsEndpoint'; + }); + + afterAll(() => { + gon.sprite_icons = spriteIcons; + }); + + let axiosMock; + let mockEndpoint; + let getIcon; + const mockName = 'mockIconName'; + const mockPath = 'mockPath'; + + beforeEach(() => { + axiosMock = new MockAdapter(axios); + mockEndpoint = axiosMock.onGet(gon.sprite_icons); + getIcon = iconUtils.getSvgIconPathContent(mockName); + }); + + afterEach(() => { + axiosMock.restore(); + }); + + it('extracts svg icon path content from sprite icons', done => { + mockEndpoint.replyOnce( + 200, + `<svg><symbol id="${mockName}"><path d="${mockPath}"/></symbol></svg>`, + ); + getIcon + .then(path => { + expect(path).toBe(mockPath); + done(); + }) + .catch(done.fail); + }); + + it('returns null if icon path content does not exist', done => { + mockEndpoint.replyOnce(200, ``); + getIcon + .then(path => { + expect(path).toBe(null); + done(); + }) + .catch(done.fail); + }); + + it('returns null if an http error occurs', done => { + mockEndpoint.replyOnce(500); + getIcon + .then(path => { + expect(path).toBe(null); + done(); + }) + .catch(done.fail); + }); + }); +}); diff --git a/spec/javascripts/monitoring/charts/area_spec.js b/spec/javascripts/monitoring/charts/area_spec.js new file mode 100644 index 00000000000..0b36fc9f5f7 --- /dev/null +++ b/spec/javascripts/monitoring/charts/area_spec.js @@ -0,0 +1,220 @@ +import { shallowMount } from '@vue/test-utils'; +import { GlAreaChart } from '@gitlab/ui/dist/charts'; +import { shallowWrapperContainsSlotText } from 'spec/helpers/vue_test_utils_helper'; +import Area from '~/monitoring/components/charts/area.vue'; +import MonitoringStore from '~/monitoring/stores/monitoring_store'; +import MonitoringMock, { deploymentData } from '../mock_data'; + +describe('Area component', () => { + const mockWidgets = 'mockWidgets'; + let mockGraphData; + let areaChart; + let spriteSpy; + + beforeEach(() => { + const store = new MonitoringStore(); + store.storeMetrics(MonitoringMock.data); + store.storeDeploymentData(deploymentData); + + [mockGraphData] = store.groups[0].metrics; + + areaChart = shallowMount(Area, { + propsData: { + graphData: mockGraphData, + containerWidth: 0, + deploymentData: store.deploymentData, + }, + slots: { + default: mockWidgets, + }, + }); + + spriteSpy = spyOnDependency(Area, 'getSvgIconPathContent').and.callFake( + () => new Promise(resolve => resolve()), + ); + }); + + afterEach(() => { + areaChart.destroy(); + }); + + it('renders chart title', () => { + expect(areaChart.find({ ref: 'graphTitle' }).text()).toBe(mockGraphData.title); + }); + + it('contains graph widgets from slot', () => { + expect(areaChart.find({ ref: 'graphWidgets' }).text()).toBe(mockWidgets); + }); + + describe('wrapped components', () => { + describe('GitLab UI area chart', () => { + let glAreaChart; + + beforeEach(() => { + glAreaChart = areaChart.find(GlAreaChart); + }); + + it('is a Vue instance', () => { + expect(glAreaChart.isVueInstance()).toBe(true); + }); + + it('receives data properties needed for proper chart render', () => { + const props = glAreaChart.props(); + + expect(props.data).toBe(areaChart.vm.chartData); + expect(props.option).toBe(areaChart.vm.chartOptions); + expect(props.formatTooltipText).toBe(areaChart.vm.formatTooltipText); + expect(props.thresholds).toBe(areaChart.props('alertData')); + }); + + it('recieves a tooltip title', () => { + const mockTitle = 'mockTitle'; + areaChart.vm.tooltip.title = mockTitle; + + expect(shallowWrapperContainsSlotText(glAreaChart, 'tooltipTitle', mockTitle)).toBe(true); + }); + + it('recieves tooltip content', () => { + const mockContent = 'mockContent'; + areaChart.vm.tooltip.content = mockContent; + + expect(shallowWrapperContainsSlotText(glAreaChart, 'tooltipContent', mockContent)).toBe( + true, + ); + }); + + describe('when tooltip is showing deployment data', () => { + beforeEach(() => { + areaChart.vm.tooltip.isDeployment = true; + }); + + it('uses deployment title', () => { + expect(shallowWrapperContainsSlotText(glAreaChart, 'tooltipTitle', 'Deployed')).toBe( + true, + ); + }); + + it('renders commit sha in tooltip content', () => { + const mockSha = 'mockSha'; + areaChart.vm.tooltip.sha = mockSha; + + expect(shallowWrapperContainsSlotText(glAreaChart, 'tooltipContent', mockSha)).toBe(true); + }); + }); + }); + }); + + describe('methods', () => { + describe('formatTooltipText', () => { + const mockDate = deploymentData[0].created_at; + const generateSeriesData = type => ({ + seriesData: [ + { + componentSubType: type, + value: [mockDate, 5.55555], + }, + ], + value: mockDate, + }); + + describe('series is of line type', () => { + beforeEach(() => { + areaChart.vm.formatTooltipText(generateSeriesData('line')); + }); + + it('formats tooltip title', () => { + expect(areaChart.vm.tooltip.title).toBe('31 May 2017, 9:23PM'); + }); + + it('formats tooltip content', () => { + expect(areaChart.vm.tooltip.content).toBe('CPU (Cores) 5.556'); + }); + }); + + describe('series is of scatter type', () => { + beforeEach(() => { + areaChart.vm.formatTooltipText(generateSeriesData('scatter')); + }); + + it('formats tooltip title', () => { + expect(areaChart.vm.tooltip.title).toBe('31 May 2017, 9:23PM'); + }); + + it('formats tooltip sha', () => { + expect(areaChart.vm.tooltip.sha).toBe('f5bcd1d9'); + }); + }); + }); + + describe('getScatterSymbol', () => { + beforeEach(() => { + areaChart.vm.getScatterSymbol(); + }); + + it('gets rocket svg path content for use as deployment data symbol', () => { + expect(spriteSpy).toHaveBeenCalledWith('rocket'); + }); + }); + + describe('onResize', () => { + const mockWidth = 233; + const mockHeight = 144; + + beforeEach(() => { + spyOn(Element.prototype, 'getBoundingClientRect').and.callFake(() => ({ + width: mockWidth, + height: mockHeight, + })); + areaChart.vm.onResize(); + }); + + it('sets area chart width', () => { + expect(areaChart.vm.width).toBe(mockWidth); + }); + + it('sets area chart height', () => { + expect(areaChart.vm.height).toBe(mockHeight); + }); + }); + }); + + describe('computed', () => { + describe('chartData', () => { + it('utilizes all data points', () => { + expect(Object.keys(areaChart.vm.chartData)).toEqual(['Cores']); + expect(areaChart.vm.chartData.Cores.length).toBe(297); + }); + + it('creates valid data', () => { + const data = areaChart.vm.chartData.Cores; + + expect( + data.filter(([time, value]) => new Date(time).getTime() > 0 && typeof value === 'number') + .length, + ).toBe(data.length); + }); + }); + + describe('scatterSeries', () => { + it('utilizes deployment data', () => { + expect(areaChart.vm.scatterSeries.data).toEqual([ + ['2017-05-31T21:23:37.881Z', 0], + ['2017-05-30T20:08:04.629Z', 0], + ['2017-05-30T17:42:38.409Z', 0], + ]); + }); + }); + + describe('xAxisLabel', () => { + it('constructs a label for the chart x-axis', () => { + expect(areaChart.vm.xAxisLabel).toBe('Core Usage'); + }); + }); + + describe('yAxisLabel', () => { + it('constructs a label for the chart y-axis', () => { + expect(areaChart.vm.yAxisLabel).toBe('CPU (Cores)'); + }); + }); + }); +}); diff --git a/spec/javascripts/monitoring/dashboard_spec.js b/spec/javascripts/monitoring/dashboard_spec.js index 565b87de248..b1778029a77 100644 --- a/spec/javascripts/monitoring/dashboard_spec.js +++ b/spec/javascripts/monitoring/dashboard_spec.js @@ -25,15 +25,22 @@ export default propsData; describe('Dashboard', () => { let DashboardComponent; + let mock; beforeEach(() => { setFixtures(` <div class="prometheus-graphs"></div> - <div class="nav-sidebar"></div> + <div class="layout-page"></div> `); + + mock = new MockAdapter(axios); DashboardComponent = Vue.extend(Dashboard); }); + afterEach(() => { + mock.restore(); + }); + describe('no metrics are available yet', () => { it('shows a getting started empty state when no metrics are present', () => { const component = new DashboardComponent({ @@ -47,16 +54,10 @@ describe('Dashboard', () => { }); describe('requests information to the server', () => { - let mock; beforeEach(() => { - mock = new MockAdapter(axios); mock.onGet(mockApiEndpoint).reply(200, metricsGroupsAPIResponse); }); - afterEach(() => { - mock.restore(); - }); - it('shows up a loading state', done => { const component = new DashboardComponent({ el: document.querySelector('.prometheus-graphs'), @@ -152,28 +153,25 @@ describe('Dashboard', () => { }); describe('when the window resizes', () => { - let mock; beforeEach(() => { - mock = new MockAdapter(axios); mock.onGet(mockApiEndpoint).reply(200, metricsGroupsAPIResponse); jasmine.clock().install(); }); afterEach(() => { - mock.restore(); jasmine.clock().uninstall(); }); - it('rerenders the dashboard when the sidebar is resized', done => { + it('sets elWidth to page width when the sidebar is resized', done => { const component = new DashboardComponent({ el: document.querySelector('.prometheus-graphs'), propsData: { ...propsData, hasMetrics: true, showPanels: false }, }); - expect(component.forceRedraw).toEqual(0); + expect(component.elWidth).toEqual(0); - const navSidebarEl = document.querySelector('.nav-sidebar'); - navSidebarEl.classList.add('nav-sidebar-collapsed'); + const pageLayoutEl = document.querySelector('.layout-page'); + pageLayoutEl.classList.add('page-with-icon-sidebar'); Vue.nextTick() .then(() => { @@ -181,7 +179,7 @@ describe('Dashboard', () => { return Vue.nextTick(); }) .then(() => { - expect(component.forceRedraw).toEqual(component.elWidth); + expect(component.elWidth).toEqual(pageLayoutEl.clientWidth); done(); }) .catch(done.fail); diff --git a/spec/javascripts/monitoring/mock_data.js b/spec/javascripts/monitoring/mock_data.js index b4e2cd75d47..ffc7148fde2 100644 --- a/spec/javascripts/monitoring/mock_data.js +++ b/spec/javascripts/monitoring/mock_data.js @@ -326,6 +326,7 @@ export const metricsGroupsAPIResponse = { { id: 6, title: 'CPU usage', + y_label: 'CPU', weight: 1, queries: [ { diff --git a/spec/javascripts/notes/components/discussion_reply_placeholder_spec.js b/spec/javascripts/notes/components/discussion_reply_placeholder_spec.js new file mode 100644 index 00000000000..07a366cf339 --- /dev/null +++ b/spec/javascripts/notes/components/discussion_reply_placeholder_spec.js @@ -0,0 +1,34 @@ +import ReplyPlaceholder from '~/notes/components/discussion_reply_placeholder.vue'; +import { shallowMount, createLocalVue } from '@vue/test-utils'; + +const localVue = createLocalVue(); + +describe('ReplyPlaceholder', () => { + let wrapper; + + beforeEach(() => { + wrapper = shallowMount(ReplyPlaceholder, { + localVue, + }); + }); + + afterEach(() => { + wrapper.destroy(); + }); + + it('emits onClick even on button click', () => { + const button = wrapper.find({ ref: 'button' }); + + button.trigger('click'); + + expect(wrapper.emitted()).toEqual({ + onClick: [[]], + }); + }); + + it('should render reply button', () => { + const button = wrapper.find({ ref: 'button' }); + + expect(button.text()).toEqual('Reply...'); + }); +}); diff --git a/spec/javascripts/notes/components/note_actions/reply_button_spec.js b/spec/javascripts/notes/components/note_actions/reply_button_spec.js new file mode 100644 index 00000000000..11e1664a3f4 --- /dev/null +++ b/spec/javascripts/notes/components/note_actions/reply_button_spec.js @@ -0,0 +1,46 @@ +import Vuex from 'vuex'; +import { createLocalVue, mount } from '@vue/test-utils'; +import ReplyButton from '~/notes/components/note_actions/reply_button.vue'; + +describe('ReplyButton', () => { + const noteId = 'dummy-note-id'; + + let wrapper; + let convertToDiscussion; + + beforeEach(() => { + const localVue = createLocalVue(); + convertToDiscussion = jasmine.createSpy('convertToDiscussion'); + + localVue.use(Vuex); + const store = new Vuex.Store({ + actions: { + convertToDiscussion, + }, + }); + + wrapper = mount(ReplyButton, { + propsData: { + noteId, + }, + store, + sync: false, + localVue, + }); + }); + + afterEach(() => { + wrapper.destroy(); + }); + + it('dispatches convertToDiscussion with note ID on click', () => { + const button = wrapper.find({ ref: 'button' }); + + button.trigger('click'); + + expect(convertToDiscussion).toHaveBeenCalledTimes(1); + const [, payload] = convertToDiscussion.calls.argsFor(0); + + expect(payload).toBe(noteId); + }); +}); diff --git a/spec/javascripts/notes/components/note_actions_spec.js b/spec/javascripts/notes/components/note_actions_spec.js index b102b7aecf7..0c1962912b4 100644 --- a/spec/javascripts/notes/components/note_actions_spec.js +++ b/spec/javascripts/notes/components/note_actions_spec.js @@ -2,14 +2,38 @@ import Vue from 'vue'; import { shallowMount, createLocalVue } from '@vue/test-utils'; import createStore from '~/notes/stores'; import noteActions from '~/notes/components/note_actions.vue'; +import { TEST_HOST } from 'spec/test_constants'; import { userDataMock } from '../mock_data'; describe('noteActions', () => { let wrapper; let store; + let props; + + const createWrapper = propsData => { + const localVue = createLocalVue(); + return shallowMount(noteActions, { + store, + propsData, + localVue, + sync: false, + }); + }; beforeEach(() => { store = createStore(); + props = { + accessLevel: 'Maintainer', + authorId: 26, + canDelete: true, + canEdit: true, + canAwardEmoji: true, + canReportAsAbuse: true, + noteId: '539', + noteUrl: `${TEST_HOST}/group/project/merge_requests/1#note_1`, + reportAbusePath: `${TEST_HOST}/abuse_reports/new?ref_url=http%3A%2F%2Flocalhost%3A3000%2Fgitlab-org%2Fgitlab-ce%2Fissues%2F7%23note_539&user_id=26`, + showReply: false, + }; }); afterEach(() => { @@ -17,31 +41,10 @@ describe('noteActions', () => { }); describe('user is logged in', () => { - let props; - beforeEach(() => { - props = { - accessLevel: 'Maintainer', - authorId: 26, - canDelete: true, - canEdit: true, - canAwardEmoji: true, - canReportAsAbuse: true, - noteId: '539', - noteUrl: 'https://localhost:3000/group/project/merge_requests/1#note_1', - reportAbusePath: - '/abuse_reports/new?ref_url=http%3A%2F%2Flocalhost%3A3000%2Fgitlab-org%2Fgitlab-ce%2Fissues%2F7%23note_539&user_id=26', - }; - store.dispatch('setUserData', userDataMock); - const localVue = createLocalVue(); - wrapper = shallowMount(noteActions, { - store, - propsData: props, - localVue, - sync: false, - }); + wrapper = createWrapper(props); }); it('should render access level badge', () => { @@ -91,28 +94,14 @@ describe('noteActions', () => { }); describe('user is not logged in', () => { - let props; - beforeEach(() => { store.dispatch('setUserData', {}); - props = { - accessLevel: 'Maintainer', - authorId: 26, + wrapper = createWrapper({ + ...props, canDelete: false, canEdit: false, canAwardEmoji: false, canReportAsAbuse: false, - noteId: '539', - noteUrl: 'https://localhost:3000/group/project/merge_requests/1#note_1', - reportAbusePath: - '/abuse_reports/new?ref_url=http%3A%2F%2Flocalhost%3A3000%2Fgitlab-org%2Fgitlab-ce%2Fissues%2F7%23note_539&user_id=26', - }; - const localVue = createLocalVue(); - wrapper = shallowMount(noteActions, { - store, - propsData: props, - localVue, - sync: false, }); }); @@ -124,4 +113,88 @@ describe('noteActions', () => { expect(wrapper.find('.more-actions').exists()).toBe(false); }); }); + + describe('with feature flag replyToIndividualNotes enabled', () => { + beforeEach(() => { + gon.features = { + replyToIndividualNotes: true, + }; + }); + + afterEach(() => { + gon.features = {}; + }); + + describe('for showReply = true', () => { + beforeEach(() => { + wrapper = createWrapper({ + ...props, + showReply: true, + }); + }); + + it('shows a reply button', () => { + const replyButton = wrapper.find({ ref: 'replyButton' }); + + expect(replyButton.exists()).toBe(true); + }); + }); + + describe('for showReply = false', () => { + beforeEach(() => { + wrapper = createWrapper({ + ...props, + showReply: false, + }); + }); + + it('does not show a reply button', () => { + const replyButton = wrapper.find({ ref: 'replyButton' }); + + expect(replyButton.exists()).toBe(false); + }); + }); + }); + + describe('with feature flag replyToIndividualNotes disabled', () => { + beforeEach(() => { + gon.features = { + replyToIndividualNotes: false, + }; + }); + + afterEach(() => { + gon.features = {}; + }); + + describe('for showReply = true', () => { + beforeEach(() => { + wrapper = createWrapper({ + ...props, + showReply: true, + }); + }); + + it('does not show a reply button', () => { + const replyButton = wrapper.find({ ref: 'replyButton' }); + + expect(replyButton.exists()).toBe(false); + }); + }); + + describe('for showReply = false', () => { + beforeEach(() => { + wrapper = createWrapper({ + ...props, + showReply: false, + }); + }); + + it('does not show a reply button', () => { + const replyButton = wrapper.find({ ref: 'replyButton' }); + + expect(replyButton.exists()).toBe(false); + }); + }); + }); }); diff --git a/spec/javascripts/notes/components/note_app_spec.js b/spec/javascripts/notes/components/note_app_spec.js index 22bee049f9c..d5c0bf6b25d 100644 --- a/spec/javascripts/notes/components/note_app_spec.js +++ b/spec/javascripts/notes/components/note_app_spec.js @@ -1,57 +1,49 @@ import $ from 'jquery'; import _ from 'underscore'; import Vue from 'vue'; -import notesApp from '~/notes/components/notes_app.vue'; +import { mount, createLocalVue } from '@vue/test-utils'; +import NotesApp from '~/notes/components/notes_app.vue'; import service from '~/notes/services/notes_service'; import createStore from '~/notes/stores'; import '~/behaviors/markdown/render_gfm'; -import { mountComponentWithStore } from 'spec/helpers'; import * as mockData from '../mock_data'; -const vueMatchers = { - toIncludeElement() { - return { - compare(vm, selector) { - const result = { - pass: vm.$el.querySelector(selector) !== null, - }; - return result; - }, - }; - }, -}; - describe('note_app', () => { let mountComponent; - let vm; + let wrapper; let store; beforeEach(() => { - jasmine.addMatchers(vueMatchers); $('body').attr('data-page', 'projects:merge_requests:show'); - setFixtures('<div class="js-vue-notes-event"><div id="app"></div></div>'); - - const IssueNotesApp = Vue.extend(notesApp); - store = createStore(); mountComponent = data => { - const props = data || { + const propsData = data || { noteableData: mockData.noteableDataMock, notesData: mockData.notesDataMock, userData: mockData.userDataMock, }; - - return mountComponentWithStore(IssueNotesApp, { - props, - store, - el: document.getElementById('app'), - }); + const localVue = createLocalVue(); + + return mount( + { + components: { + NotesApp, + }, + template: '<div class="js-vue-notes-event"><notes-app v-bind="$attrs" /></div>', + }, + { + propsData, + store, + localVue, + sync: false, + }, + ); }; }); afterEach(() => { - vm.$destroy(); + wrapper.destroy(); }); describe('set data', () => { @@ -65,7 +57,7 @@ describe('note_app', () => { beforeEach(() => { Vue.http.interceptors.push(responseInterceptor); - vm = mountComponent(); + wrapper = mountComponent(); }); afterEach(() => { @@ -73,26 +65,26 @@ describe('note_app', () => { }); it('should set notes data', () => { - expect(vm.$store.state.notesData).toEqual(mockData.notesDataMock); + expect(store.state.notesData).toEqual(mockData.notesDataMock); }); it('should set issue data', () => { - expect(vm.$store.state.noteableData).toEqual(mockData.noteableDataMock); + expect(store.state.noteableData).toEqual(mockData.noteableDataMock); }); it('should set user data', () => { - expect(vm.$store.state.userData).toEqual(mockData.userDataMock); + expect(store.state.userData).toEqual(mockData.userDataMock); }); it('should fetch discussions', () => { - expect(vm.$store.state.discussions).toEqual([]); + expect(store.state.discussions).toEqual([]); }); }); describe('render', () => { beforeEach(() => { Vue.http.interceptors.push(mockData.individualNoteInterceptor); - vm = mountComponent(); + wrapper = mountComponent(); }); afterEach(() => { @@ -107,51 +99,50 @@ describe('note_app', () => { setTimeout(() => { expect( - vm.$el.querySelector('.main-notes-list .note-header-author-name').textContent.trim(), + wrapper + .find('.main-notes-list .note-header-author-name') + .text() + .trim(), ).toEqual(note.author.name); - expect(vm.$el.querySelector('.main-notes-list .note-text').innerHTML).toEqual( - note.note_html, - ); + expect(wrapper.find('.main-notes-list .note-text').html()).toContain(note.note_html); done(); }, 0); }); it('should render form', () => { - expect(vm.$el.querySelector('.js-main-target-form').tagName).toEqual('FORM'); - expect( - vm.$el.querySelector('.js-main-target-form textarea').getAttribute('placeholder'), - ).toEqual('Write a comment or drag your files here…'); + expect(wrapper.find('.js-main-target-form').name()).toEqual('form'); + expect(wrapper.find('.js-main-target-form textarea').attributes('placeholder')).toEqual( + 'Write a comment or drag your files here…', + ); }); it('should not render form when commenting is disabled', () => { store.state.commentsDisabled = true; - vm = mountComponent(); + wrapper = mountComponent(); - expect(vm.$el.querySelector('.js-main-target-form')).toEqual(null); + expect(wrapper.find('.js-main-target-form').exists()).toBe(false); }); it('should render form comment button as disabled', () => { - expect(vm.$el.querySelector('.js-note-new-discussion').getAttribute('disabled')).toEqual( - 'disabled', - ); + expect(wrapper.find('.js-note-new-discussion').attributes('disabled')).toEqual('disabled'); }); }); describe('while fetching data', () => { beforeEach(() => { - vm = mountComponent(); + wrapper = mountComponent(); }); it('renders skeleton notes', () => { - expect(vm).toIncludeElement('.animation-container'); + expect(wrapper.find('.animation-container').exists()).toBe(true); }); it('should render form', () => { - expect(vm.$el.querySelector('.js-main-target-form').tagName).toEqual('FORM'); - expect( - vm.$el.querySelector('.js-main-target-form textarea').getAttribute('placeholder'), - ).toEqual('Write a comment or drag your files here…'); + expect(wrapper.find('.js-main-target-form').name()).toEqual('form'); + expect(wrapper.find('.js-main-target-form textarea').attributes('placeholder')).toEqual( + 'Write a comment or drag your files here…', + ); }); }); @@ -160,9 +151,9 @@ describe('note_app', () => { beforeEach(done => { Vue.http.interceptors.push(mockData.individualNoteInterceptor); spyOn(service, 'updateNote').and.callThrough(); - vm = mountComponent(); + wrapper = mountComponent(); setTimeout(() => { - vm.$el.querySelector('.js-note-edit').click(); + wrapper.find('.js-note-edit').trigger('click'); Vue.nextTick(done); }, 0); }); @@ -175,12 +166,12 @@ describe('note_app', () => { }); it('renders edit form', () => { - expect(vm).toIncludeElement('.js-vue-issue-note-form'); + expect(wrapper.find('.js-vue-issue-note-form').exists()).toBe(true); }); it('calls the service to update the note', done => { - vm.$el.querySelector('.js-vue-issue-note-form').value = 'this is a note'; - vm.$el.querySelector('.js-vue-issue-save').click(); + wrapper.find('.js-vue-issue-note-form').value = 'this is a note'; + wrapper.find('.js-vue-issue-save').trigger('click'); expect(service.updateNote).toHaveBeenCalled(); // Wait for the requests to finish before destroying @@ -194,10 +185,10 @@ describe('note_app', () => { beforeEach(done => { Vue.http.interceptors.push(mockData.discussionNoteInterceptor); spyOn(service, 'updateNote').and.callThrough(); - vm = mountComponent(); + wrapper = mountComponent(); setTimeout(() => { - vm.$el.querySelector('.js-note-edit').click(); + wrapper.find('.js-note-edit').trigger('click'); Vue.nextTick(done); }, 0); }); @@ -210,12 +201,12 @@ describe('note_app', () => { }); it('renders edit form', () => { - expect(vm).toIncludeElement('.js-vue-issue-note-form'); + expect(wrapper.find('.js-vue-issue-note-form').exists()).toBe(true); }); it('updates the note and resets the edit form', done => { - vm.$el.querySelector('.js-vue-issue-note-form').value = 'this is a note'; - vm.$el.querySelector('.js-vue-issue-save').click(); + wrapper.find('.js-vue-issue-note-form').value = 'this is a note'; + wrapper.find('.js-vue-issue-save').trigger('click'); expect(service.updateNote).toHaveBeenCalled(); // Wait for the requests to finish before destroying @@ -228,30 +219,36 @@ describe('note_app', () => { describe('new note form', () => { beforeEach(() => { - vm = mountComponent(); + wrapper = mountComponent(); }); it('should render markdown docs url', () => { const { markdownDocsPath } = mockData.notesDataMock; - expect(vm.$el.querySelector(`a[href="${markdownDocsPath}"]`).textContent.trim()).toEqual( - 'Markdown', - ); + expect( + wrapper + .find(`a[href="${markdownDocsPath}"]`) + .text() + .trim(), + ).toEqual('Markdown'); }); it('should render quick action docs url', () => { const { quickActionsDocsPath } = mockData.notesDataMock; - expect(vm.$el.querySelector(`a[href="${quickActionsDocsPath}"]`).textContent.trim()).toEqual( - 'quick actions', - ); + expect( + wrapper + .find(`a[href="${quickActionsDocsPath}"]`) + .text() + .trim(), + ).toEqual('quick actions'); }); }); describe('edit form', () => { beforeEach(() => { Vue.http.interceptors.push(mockData.individualNoteInterceptor); - vm = mountComponent(); + wrapper = mountComponent(); }); afterEach(() => { @@ -260,12 +257,15 @@ describe('note_app', () => { it('should render markdown docs url', done => { setTimeout(() => { - vm.$el.querySelector('.js-note-edit').click(); + wrapper.find('.js-note-edit').trigger('click'); const { markdownDocsPath } = mockData.notesDataMock; Vue.nextTick(() => { expect( - vm.$el.querySelector(`.edit-note a[href="${markdownDocsPath}"]`).textContent.trim(), + wrapper + .find(`.edit-note a[href="${markdownDocsPath}"]`) + .text() + .trim(), ).toEqual('Markdown is supported'); done(); }); @@ -274,13 +274,11 @@ describe('note_app', () => { it('should not render quick actions docs url', done => { setTimeout(() => { - vm.$el.querySelector('.js-note-edit').click(); + wrapper.find('.js-note-edit').trigger('click'); const { quickActionsDocsPath } = mockData.notesDataMock; Vue.nextTick(() => { - expect(vm.$el.querySelector(`.edit-note a[href="${quickActionsDocsPath}"]`)).toEqual( - null, - ); + expect(wrapper.find(`.edit-note a[href="${quickActionsDocsPath}"]`).exists()).toBe(false); done(); }); }, 0); @@ -295,12 +293,19 @@ describe('note_app', () => { noteId: 1, }, }); + const toggleAwardAction = jasmine.createSpy('toggleAward'); + wrapper.vm.$store.hotUpdate({ + actions: { + toggleAward: toggleAwardAction, + }, + }); - spyOn(vm.$store, 'dispatch'); + wrapper.vm.$parent.$el.dispatchEvent(toggleAwardEvent); - vm.$el.parentElement.dispatchEvent(toggleAwardEvent); + expect(toggleAwardAction).toHaveBeenCalledTimes(1); + const [, payload] = toggleAwardAction.calls.argsFor(0); - expect(vm.$store.dispatch).toHaveBeenCalledWith('toggleAward', { + expect(payload).toEqual({ awardName: 'test', noteId: 1, }); diff --git a/spec/javascripts/notes/components/noteable_discussion_spec.js b/spec/javascripts/notes/components/noteable_discussion_spec.js index c4b7eb17393..2eae22e095f 100644 --- a/spec/javascripts/notes/components/noteable_discussion_spec.js +++ b/spec/javascripts/notes/components/noteable_discussion_spec.js @@ -1,6 +1,7 @@ import { shallowMount, createLocalVue } from '@vue/test-utils'; import createStore from '~/notes/stores'; import noteableDiscussion from '~/notes/components/noteable_discussion.vue'; +import ReplyPlaceholder from '~/notes/components/discussion_reply_placeholder.vue'; import '~/behaviors/markdown/render_gfm'; import { noteableDataMock, discussionMock, notesDataMock } from '../mock_data'; import mockDiffFile from '../../diffs/mock_data/diff_file'; @@ -57,27 +58,23 @@ describe('noteable_discussion component', () => { }); describe('actions', () => { - it('should render reply button', () => { - expect( - wrapper - .find('.js-vue-discussion-reply') - .text() - .trim(), - ).toEqual('Reply...'); - }); - it('should toggle reply form', done => { - wrapper.find('.js-vue-discussion-reply').trigger('click'); + const replyPlaceholder = wrapper.find(ReplyPlaceholder); - wrapper.vm.$nextTick(() => { - expect(wrapper.vm.isReplying).toEqual(true); + wrapper.vm + .$nextTick() + .then(() => { + expect(wrapper.vm.isReplying).toEqual(false); - // There is a watcher for `isReplying` which will init autosave in the next tick - wrapper.vm.$nextTick(() => { + replyPlaceholder.vm.$emit('onClick'); + }) + .then(() => wrapper.vm.$nextTick()) + .then(() => { + expect(wrapper.vm.isReplying).toEqual(true); expect(wrapper.vm.$refs.noteForm).not.toBeNull(); - done(); - }); - }); + }) + .then(done) + .catch(done.fail); }); it('does not render jump to discussion button', () => { diff --git a/spec/javascripts/notes/mock_data.js b/spec/javascripts/notes/mock_data.js index 7ae45c40c28..348743081eb 100644 --- a/spec/javascripts/notes/mock_data.js +++ b/spec/javascripts/notes/mock_data.js @@ -165,7 +165,6 @@ export const note = { report_abuse_path: '/abuse_reports/new?ref_url=http%3A%2F%2Flocalhost%3A3000%2Fgitlab-org%2Fgitlab-ce%2Fissues%2F7%23note_546&user_id=1', path: '/gitlab-org/gitlab-ce/notes/546', - cached_markdown_version: 11, }; export const discussionMock = { diff --git a/spec/javascripts/notes/stores/actions_spec.js b/spec/javascripts/notes/stores/actions_spec.js index 2e3cd5e8f36..73f960dd21e 100644 --- a/spec/javascripts/notes/stores/actions_spec.js +++ b/spec/javascripts/notes/stores/actions_spec.js @@ -585,4 +585,18 @@ describe('Actions Notes Store', () => { ); }); }); + + describe('convertToDiscussion', () => { + it('commits CONVERT_TO_DISCUSSION with noteId', done => { + const noteId = 'dummy-note-id'; + testAction( + actions.convertToDiscussion, + noteId, + {}, + [{ type: 'CONVERT_TO_DISCUSSION', payload: noteId }], + [], + done, + ); + }); + }); }); diff --git a/spec/javascripts/notes/stores/mutation_spec.js b/spec/javascripts/notes/stores/mutation_spec.js index b6b2c7d60a5..4f8d3069bb5 100644 --- a/spec/javascripts/notes/stores/mutation_spec.js +++ b/spec/javascripts/notes/stores/mutation_spec.js @@ -517,4 +517,27 @@ describe('Notes Store mutations', () => { ); }); }); + + describe('CONVERT_TO_DISCUSSION', () => { + let discussion; + let state; + + beforeEach(() => { + discussion = { + id: 42, + individual_note: true, + }; + state = { discussions: [discussion] }; + }); + + it('toggles individual_note', () => { + mutations.CONVERT_TO_DISCUSSION(state, discussion.id); + + expect(discussion.individual_note).toBe(false); + }); + + it('throws if discussion was not found', () => { + expect(() => mutations.CONVERT_TO_DISCUSSION(state, 99)).toThrow(); + }); + }); }); diff --git a/spec/javascripts/serverless/components/environment_row_spec.js b/spec/javascripts/serverless/components/environment_row_spec.js new file mode 100644 index 00000000000..bdf7a714910 --- /dev/null +++ b/spec/javascripts/serverless/components/environment_row_spec.js @@ -0,0 +1,81 @@ +import Vue from 'vue'; + +import environmentRowComponent from '~/serverless/components/environment_row.vue'; +import mountComponent from 'spec/helpers/vue_mount_component_helper'; +import ServerlessStore from '~/serverless/stores/serverless_store'; + +import { mockServerlessFunctions, mockServerlessFunctionsDiffEnv } from '../mock_data'; + +const createComponent = (env, envName) => + mountComponent(Vue.extend(environmentRowComponent), { env, envName }); + +describe('environment row component', () => { + describe('default global cluster case', () => { + let vm; + + beforeEach(() => { + const store = new ServerlessStore(false, '/cluster_path', 'help_path'); + store.updateFunctionsFromServer(mockServerlessFunctions); + vm = createComponent(store.state.functions['*'], '*'); + }); + + it('has the correct envId', () => { + expect(vm.envId).toEqual('env-global'); + vm.$destroy(); + }); + + it('is open by default', () => { + expect(vm.isOpenClass).toEqual({ 'is-open': true }); + vm.$destroy(); + }); + + it('generates correct output', () => { + expect(vm.$el.querySelectorAll('li').length).toEqual(2); + expect(vm.$el.id).toEqual('env-global'); + expect(vm.$el.classList.contains('is-open')).toBe(true); + expect(vm.$el.querySelector('div.title').innerHTML.trim()).toEqual('*'); + + vm.$destroy(); + }); + + it('opens and closes correctly', () => { + expect(vm.isOpen).toBe(true); + + vm.toggleOpen(); + Vue.nextTick(() => { + expect(vm.isOpen).toBe(false); + }); + + vm.$destroy(); + }); + }); + + describe('default named cluster case', () => { + let vm; + + beforeEach(() => { + const store = new ServerlessStore(false, '/cluster_path', 'help_path'); + store.updateFunctionsFromServer(mockServerlessFunctionsDiffEnv); + vm = createComponent(store.state.functions.test, 'test'); + }); + + it('has the correct envId', () => { + expect(vm.envId).toEqual('env-test'); + vm.$destroy(); + }); + + it('is open by default', () => { + expect(vm.isOpenClass).toEqual({ 'is-open': true }); + vm.$destroy(); + }); + + it('generates correct output', () => { + expect(vm.$el.querySelectorAll('li').length).toEqual(1); + expect(vm.$el.id).toEqual('env-test'); + expect(vm.$el.classList.contains('is-open')).toBe(true); + expect(vm.$el.querySelector('div.title').innerHTML.trim()).toEqual('test'); + + vm.$destroy(); + }); + }); +}); diff --git a/spec/javascripts/serverless/components/function_row_spec.js b/spec/javascripts/serverless/components/function_row_spec.js new file mode 100644 index 00000000000..6933a8f6c87 --- /dev/null +++ b/spec/javascripts/serverless/components/function_row_spec.js @@ -0,0 +1,33 @@ +import Vue from 'vue'; + +import functionRowComponent from '~/serverless/components/function_row.vue'; +import mountComponent from 'spec/helpers/vue_mount_component_helper'; + +import { mockServerlessFunction } from '../mock_data'; + +const createComponent = func => mountComponent(Vue.extend(functionRowComponent), { func }); + +describe('functionRowComponent', () => { + it('Parses the function details correctly', () => { + const vm = createComponent(mockServerlessFunction); + + expect(vm.$el.querySelector('b').innerHTML).toEqual(mockServerlessFunction.name); + expect(vm.$el.querySelector('span').innerHTML).toEqual(mockServerlessFunction.image); + expect(vm.$el.querySelector('time').getAttribute('data-original-title')).not.toBe(null); + expect(vm.$el.querySelector('div.url-text-field').innerHTML).toEqual( + mockServerlessFunction.url, + ); + + vm.$destroy(); + }); + + it('handles clicks correctly', () => { + const vm = createComponent(mockServerlessFunction); + + expect(vm.checkClass(vm.$el.querySelector('p'))).toBe(true); // check somewhere inside the row + expect(vm.checkClass(vm.$el.querySelector('svg'))).toBe(false); // check a button image + expect(vm.checkClass(vm.$el.querySelector('div.url-text-field'))).toBe(false); // check the url bar + + vm.$destroy(); + }); +}); diff --git a/spec/javascripts/serverless/components/functions_spec.js b/spec/javascripts/serverless/components/functions_spec.js new file mode 100644 index 00000000000..85cfe71281f --- /dev/null +++ b/spec/javascripts/serverless/components/functions_spec.js @@ -0,0 +1,68 @@ +import Vue from 'vue'; + +import functionsComponent from '~/serverless/components/functions.vue'; +import mountComponent from 'spec/helpers/vue_mount_component_helper'; +import ServerlessStore from '~/serverless/stores/serverless_store'; + +import { mockServerlessFunctions } from '../mock_data'; + +const createComponent = ( + functions, + installed = true, + loadingData = true, + hasFunctionData = true, +) => { + const component = Vue.extend(functionsComponent); + + return mountComponent(component, { + functions, + installed, + clustersPath: '/testClusterPath', + helpPath: '/helpPath', + loadingData, + hasFunctionData, + }); +}; + +describe('functionsComponent', () => { + it('should render empty state when Knative is not installed', () => { + const vm = createComponent({}, false); + + expect(vm.$el.querySelector('div.row').classList.contains('js-empty-state')).toBe(true); + expect(vm.$el.querySelector('h4.state-title').innerHTML.trim()).toEqual( + 'Getting started with serverless', + ); + + vm.$destroy(); + }); + + it('should render a loading component', () => { + const vm = createComponent({}); + + expect(vm.$el.querySelector('.gl-responsive-table-row')).not.toBe(null); + expect(vm.$el.querySelector('div.animation-container')).not.toBe(null); + }); + + it('should render empty state when there is no function data', () => { + const vm = createComponent({}, true, false, false); + + expect( + vm.$el.querySelector('.empty-state, .js-empty-state').classList.contains('js-empty-state'), + ).toBe(true); + + expect(vm.$el.querySelector('h4.state-title').innerHTML.trim()).toEqual( + 'No functions available', + ); + + vm.$destroy(); + }); + + it('should render the functions list', () => { + const store = new ServerlessStore(false, '/cluster_path', 'help_path'); + store.updateFunctionsFromServer(mockServerlessFunctions); + const vm = createComponent(store.state.functions, true, false); + + expect(vm.$el.querySelector('div.groups-list-tree-container')).not.toBe(null); + expect(vm.$el.querySelector('#env-global').classList.contains('has-children')).toBe(true); + }); +}); diff --git a/spec/javascripts/serverless/components/url_spec.js b/spec/javascripts/serverless/components/url_spec.js new file mode 100644 index 00000000000..21a879a49bb --- /dev/null +++ b/spec/javascripts/serverless/components/url_spec.js @@ -0,0 +1,28 @@ +import Vue from 'vue'; + +import urlComponent from '~/serverless/components/url.vue'; +import mountComponent from 'spec/helpers/vue_mount_component_helper'; + +const createComponent = uri => { + const component = Vue.extend(urlComponent); + + return mountComponent(component, { + uri, + }); +}; + +describe('urlComponent', () => { + it('should render correctly', () => { + const uri = 'http://testfunc.apps.example.com'; + const vm = createComponent(uri); + + expect(vm.$el.classList.contains('clipboard-group')).toBe(true); + expect(vm.$el.querySelector('.js-clipboard-btn').getAttribute('data-clipboard-text')).toEqual( + uri, + ); + + expect(vm.$el.querySelector('.url-text-field').innerHTML).toEqual(uri); + + vm.$destroy(); + }); +}); diff --git a/spec/javascripts/serverless/mock_data.js b/spec/javascripts/serverless/mock_data.js new file mode 100644 index 00000000000..ecd393b174c --- /dev/null +++ b/spec/javascripts/serverless/mock_data.js @@ -0,0 +1,79 @@ +export const mockServerlessFunctions = [ + { + name: 'testfunc1', + namespace: 'tm-example', + environment_scope: '*', + cluster_id: 46, + detail_url: '/testuser/testproj/serverless/functions/*/testfunc1', + podcount: null, + created_at: '2019-02-05T01:01:23Z', + url: 'http://testfunc1.tm-example.apps.example.com', + description: 'A test service', + image: 'knative-test-container-buildtemplate', + }, + { + name: 'testfunc2', + namespace: 'tm-example', + environment_scope: '*', + cluster_id: 46, + detail_url: '/testuser/testproj/serverless/functions/*/testfunc2', + podcount: null, + created_at: '2019-02-05T01:01:23Z', + url: 'http://testfunc2.tm-example.apps.example.com', + description: 'A second test service\nThis one with additional descriptions', + image: 'knative-test-echo-buildtemplate', + }, +]; + +export const mockServerlessFunctionsDiffEnv = [ + { + name: 'testfunc1', + namespace: 'tm-example', + environment_scope: '*', + cluster_id: 46, + detail_url: '/testuser/testproj/serverless/functions/*/testfunc1', + podcount: null, + created_at: '2019-02-05T01:01:23Z', + url: 'http://testfunc1.tm-example.apps.example.com', + description: 'A test service', + image: 'knative-test-container-buildtemplate', + }, + { + name: 'testfunc2', + namespace: 'tm-example', + environment_scope: 'test', + cluster_id: 46, + detail_url: '/testuser/testproj/serverless/functions/*/testfunc2', + podcount: null, + created_at: '2019-02-05T01:01:23Z', + url: 'http://testfunc2.tm-example.apps.example.com', + description: 'A second test service\nThis one with additional descriptions', + image: 'knative-test-echo-buildtemplate', + }, +]; + +export const mockServerlessFunction = { + name: 'testfunc1', + namespace: 'tm-example', + environment_scope: '*', + cluster_id: 46, + detail_url: '/testuser/testproj/serverless/functions/*/testfunc1', + podcount: '3', + created_at: '2019-02-05T01:01:23Z', + url: 'http://testfunc1.tm-example.apps.example.com', + description: 'A test service', + image: 'knative-test-container-buildtemplate', +}; + +export const mockMultilineServerlessFunction = { + name: 'testfunc1', + namespace: 'tm-example', + environment_scope: '*', + cluster_id: 46, + detail_url: '/testuser/testproj/serverless/functions/*/testfunc1', + podcount: '3', + created_at: '2019-02-05T01:01:23Z', + url: 'http://testfunc1.tm-example.apps.example.com', + description: 'testfunc1\nA test service line\\nWith additional services', + image: 'knative-test-container-buildtemplate', +}; diff --git a/spec/javascripts/serverless/stores/serverless_store_spec.js b/spec/javascripts/serverless/stores/serverless_store_spec.js new file mode 100644 index 00000000000..72fd903d7d1 --- /dev/null +++ b/spec/javascripts/serverless/stores/serverless_store_spec.js @@ -0,0 +1,36 @@ +import ServerlessStore from '~/serverless/stores/serverless_store'; +import { mockServerlessFunctions, mockServerlessFunctionsDiffEnv } from '../mock_data'; + +describe('Serverless Functions Store', () => { + let store; + + beforeEach(() => { + store = new ServerlessStore(false, '/cluster_path', 'help_path'); + }); + + describe('#updateFunctionsFromServer', () => { + it('should pass an empty hash object', () => { + store.updateFunctionsFromServer(); + + expect(store.state.functions).toEqual({}); + }); + + it('should group functions to one global environment', () => { + const mockServerlessData = mockServerlessFunctions; + store.updateFunctionsFromServer(mockServerlessData); + + expect(Object.keys(store.state.functions)).toEqual(jasmine.objectContaining(['*'])); + expect(store.state.functions['*'].length).toEqual(2); + }); + + it('should group functions to multiple environments', () => { + const mockServerlessData = mockServerlessFunctionsDiffEnv; + store.updateFunctionsFromServer(mockServerlessData); + + expect(Object.keys(store.state.functions)).toEqual(jasmine.objectContaining(['*'])); + expect(store.state.functions['*'].length).toEqual(1); + expect(store.state.functions.test.length).toEqual(1); + expect(store.state.functions.test[0].name).toEqual('testfunc2'); + }); + }); +}); diff --git a/spec/javascripts/vue_mr_widget/mock_data.js b/spec/javascripts/vue_mr_widget/mock_data.js index 072e98fc0e8..75b197fb2ba 100644 --- a/spec/javascripts/vue_mr_widget/mock_data.js +++ b/spec/javascripts/vue_mr_widget/mock_data.js @@ -58,7 +58,7 @@ export default { merge_user: null, diff_head_sha: '104096c51715e12e7ae41f9333e9fa35b73f385d', diff_head_commit_short_id: '104096c5', - merge_commit_message: + default_merge_commit_message: "Merge branch 'daaaa' into 'master'\n\nUpdate README.md\n\nSee merge request !22", pipeline: { id: 172, @@ -213,7 +213,7 @@ export default { merge_check_path: '/root/acets-app/merge_requests/22/merge_check', ci_environments_status_url: '/root/acets-app/merge_requests/22/ci_environments_status', project_archived: false, - merge_commit_message_with_description: + default_merge_commit_message_with_description: "Merge branch 'daaaa' into 'master'\n\nUpdate README.md\n\nSee merge request !22", diverged_commits_count: 0, only_allow_merge_if_pipeline_succeeds: false, diff --git a/spec/javascripts/ide/components/file_finder/index_spec.js b/spec/javascripts/vue_shared/components/file_finder/index_spec.js index 15ef8c31f91..bae4741f652 100644 --- a/spec/javascripts/ide/components/file_finder/index_spec.js +++ b/spec/javascripts/vue_shared/components/file_finder/index_spec.js @@ -1,54 +1,51 @@ import Vue from 'vue'; -import store from '~/ide/stores'; -import FindFileComponent from '~/ide/components/file_finder/index.vue'; +import Mousetrap from 'mousetrap'; +import FindFileComponent from '~/vue_shared/components/file_finder/index.vue'; import { UP_KEY_CODE, DOWN_KEY_CODE, ENTER_KEY_CODE, ESC_KEY_CODE } from '~/lib/utils/keycodes'; -import router from '~/ide/ide_router'; -import { file, resetStore } from '../../helpers'; -import { mountComponentWithStore } from '../../../helpers/vue_mount_component_helper'; +import { file } from 'spec/ide/helpers'; +import timeoutPromise from 'spec/helpers/set_timeout_promise_helper'; -describe('IDE File finder item spec', () => { +describe('File finder item spec', () => { const Component = Vue.extend(FindFileComponent); let vm; - beforeEach(done => { - setFixtures('<div id="app"></div>'); - - vm = mountComponentWithStore(Component, { - store, - el: '#app', - props: { - index: 0, + function createComponent(props) { + vm = new Component({ + propsData: { + files: [], + visible: true, + loading: false, + ...props, }, }); - setTimeout(done); + vm.$mount('#app'); + } + + beforeEach(() => { + setFixtures('<div id="app"></div>'); }); afterEach(() => { vm.$destroy(); - - resetStore(vm.$store); }); describe('with entries', () => { beforeEach(done => { - Vue.set(vm.$store.state.entries, 'folder', { - ...file('folder'), - path: 'folder', - type: 'folder', - }); - - Vue.set(vm.$store.state.entries, 'index.js', { - ...file('index.js'), - path: 'index.js', - type: 'blob', - url: '/index.jsurl', - }); - - Vue.set(vm.$store.state.entries, 'component.js', { - ...file('component.js'), - path: 'component.js', - type: 'blob', + createComponent({ + files: [ + { + ...file('index.js'), + path: 'index.js', + type: 'blob', + url: '/index.jsurl', + }, + { + ...file('component.js'), + path: 'component.js', + type: 'blob', + }, + ], }); setTimeout(done); @@ -56,13 +53,14 @@ describe('IDE File finder item spec', () => { it('renders list of blobs', () => { expect(vm.$el.textContent).toContain('index.js'); + expect(vm.$el.textContent).toContain('component.js'); expect(vm.$el.textContent).not.toContain('folder'); }); it('filters entries', done => { vm.searchText = 'index'; - vm.$nextTick(() => { + setTimeout(() => { expect(vm.$el.textContent).toContain('index.js'); expect(vm.$el.textContent).not.toContain('component.js'); @@ -73,8 +71,8 @@ describe('IDE File finder item spec', () => { it('shows clear button when searchText is not empty', done => { vm.searchText = 'index'; - vm.$nextTick(() => { - expect(vm.$el.querySelector('.dropdown-input-clear').classList).toContain('show'); + setTimeout(() => { + expect(vm.$el.querySelector('.dropdown-input').classList).toContain('has-value'); expect(vm.$el.querySelector('.dropdown-input-search').classList).toContain('hidden'); done(); @@ -84,11 +82,11 @@ describe('IDE File finder item spec', () => { it('clear button resets searchText', done => { vm.searchText = 'index'; - vm.$nextTick() + timeoutPromise() .then(() => { vm.$el.querySelector('.dropdown-input-clear').click(); }) - .then(vm.$nextTick) + .then(timeoutPromise) .then(() => { expect(vm.searchText).toBe(''); }) @@ -100,11 +98,11 @@ describe('IDE File finder item spec', () => { spyOn(vm.$refs.searchInput, 'focus'); vm.searchText = 'index'; - vm.$nextTick() + timeoutPromise() .then(() => { vm.$el.querySelector('.dropdown-input-clear').click(); }) - .then(vm.$nextTick) + .then(timeoutPromise) .then(() => { expect(vm.$refs.searchInput.focus).toHaveBeenCalled(); }) @@ -116,7 +114,7 @@ describe('IDE File finder item spec', () => { it('returns 1 when no filtered entries exist', done => { vm.searchText = 'testing 123'; - vm.$nextTick(() => { + setTimeout(() => { expect(vm.listShowCount).toBe(1); done(); @@ -136,7 +134,7 @@ describe('IDE File finder item spec', () => { it('returns 33 when entries dont exist', done => { vm.searchText = 'testing 123'; - vm.$nextTick(() => { + setTimeout(() => { expect(vm.listHeight).toBe(33); done(); @@ -148,7 +146,7 @@ describe('IDE File finder item spec', () => { it('returns length of filtered blobs', done => { vm.searchText = 'index'; - vm.$nextTick(() => { + setTimeout(() => { expect(vm.filteredBlobsLength).toBe(1); done(); @@ -162,7 +160,7 @@ describe('IDE File finder item spec', () => { vm.focusedIndex = 1; vm.searchText = 'test'; - vm.$nextTick(() => { + setTimeout(() => { expect(vm.focusedIndex).toBe(0); done(); @@ -170,16 +168,16 @@ describe('IDE File finder item spec', () => { }); }); - describe('fileFindVisible', () => { + describe('visible', () => { it('returns searchText when false', done => { vm.searchText = 'test'; - vm.$store.state.fileFindVisible = true; + vm.visible = true; - vm.$nextTick() + timeoutPromise() .then(() => { - vm.$store.state.fileFindVisible = false; + vm.visible = false; }) - .then(vm.$nextTick) + .then(timeoutPromise) .then(() => { expect(vm.searchText).toBe(''); }) @@ -191,20 +189,19 @@ describe('IDE File finder item spec', () => { describe('openFile', () => { beforeEach(() => { - spyOn(router, 'push'); - spyOn(vm, 'toggleFileFinder'); + spyOn(vm, '$emit'); }); it('closes file finder', () => { - vm.openFile(vm.$store.state.entries['index.js']); + vm.openFile(vm.files[0]); - expect(vm.toggleFileFinder).toHaveBeenCalled(); + expect(vm.$emit).toHaveBeenCalledWith('toggle', false); }); it('pushes to router', () => { - vm.openFile(vm.$store.state.entries['index.js']); + vm.openFile(vm.files[0]); - expect(router.push).toHaveBeenCalledWith('/project/index.jsurl'); + expect(vm.$emit).toHaveBeenCalledWith('click', vm.files[0]); }); }); @@ -217,8 +214,8 @@ describe('IDE File finder item spec', () => { vm.$refs.searchInput.dispatchEvent(event); - vm.$nextTick(() => { - expect(vm.openFile).toHaveBeenCalledWith(vm.$store.state.entries['index.js']); + setTimeout(() => { + expect(vm.openFile).toHaveBeenCalledWith(vm.files[0]); done(); }); @@ -228,12 +225,12 @@ describe('IDE File finder item spec', () => { const event = new CustomEvent('keyup'); event.keyCode = ESC_KEY_CODE; - spyOn(vm, 'toggleFileFinder'); + spyOn(vm, '$emit'); vm.$refs.searchInput.dispatchEvent(event); - vm.$nextTick(() => { - expect(vm.toggleFileFinder).toHaveBeenCalled(); + setTimeout(() => { + expect(vm.$emit).toHaveBeenCalledWith('toggle', false); done(); }); @@ -287,18 +284,85 @@ describe('IDE File finder item spec', () => { }); describe('without entries', () => { - it('renders loading text when loading', done => { - store.state.loading = true; - - vm.$nextTick(() => { - expect(vm.$el.textContent).toContain('Loading...'); - - done(); + it('renders loading text when loading', () => { + createComponent({ + loading: true, }); + + expect(vm.$el.textContent).toContain('Loading...'); }); it('renders no files text', () => { + createComponent(); + expect(vm.$el.textContent).toContain('No files found.'); }); }); + + describe('keyboard shortcuts', () => { + beforeEach(done => { + createComponent(); + + spyOn(vm, 'toggle'); + + vm.$nextTick(done); + }); + + it('calls toggle on `t` key press', done => { + Mousetrap.trigger('t'); + + vm.$nextTick() + .then(() => { + expect(vm.toggle).toHaveBeenCalled(); + }) + .then(done) + .catch(done.fail); + }); + + it('calls toggle on `command+p` key press', done => { + Mousetrap.trigger('command+p'); + + vm.$nextTick() + .then(() => { + expect(vm.toggle).toHaveBeenCalled(); + }) + .then(done) + .catch(done.fail); + }); + + it('calls toggle on `ctrl+p` key press', done => { + Mousetrap.trigger('ctrl+p'); + + vm.$nextTick() + .then(() => { + expect(vm.toggle).toHaveBeenCalled(); + }) + .then(done) + .catch(done.fail); + }); + + it('always allows `command+p` to trigger toggle', () => { + expect( + vm.mousetrapStopCallback(null, vm.$el.querySelector('.dropdown-input-field'), 'command+p'), + ).toBe(false); + }); + + it('always allows `ctrl+p` to trigger toggle', () => { + expect( + vm.mousetrapStopCallback(null, vm.$el.querySelector('.dropdown-input-field'), 'ctrl+p'), + ).toBe(false); + }); + + it('onlys handles `t` when focused in input-field', () => { + expect( + vm.mousetrapStopCallback(null, vm.$el.querySelector('.dropdown-input-field'), 't'), + ).toBe(true); + }); + + it('stops callback in monaco editor', () => { + setFixtures('<div class="inputarea"></div>'); + + expect(vm.mousetrapStopCallback(null, document.querySelector('.inputarea'), 't')).toBe(true); + }); + }); }); diff --git a/spec/javascripts/ide/components/file_finder/item_spec.js b/spec/javascripts/vue_shared/components/file_finder/item_spec.js index 0f1116c6912..c1511643a9d 100644 --- a/spec/javascripts/ide/components/file_finder/item_spec.js +++ b/spec/javascripts/vue_shared/components/file_finder/item_spec.js @@ -1,9 +1,9 @@ import Vue from 'vue'; -import ItemComponent from '~/ide/components/file_finder/item.vue'; -import { file } from '../../helpers'; +import ItemComponent from '~/vue_shared/components/file_finder/item.vue'; +import { file } from 'spec/ide/helpers'; import createComponent from '../../../helpers/vue_mount_component_helper'; -describe('IDE File finder item spec', () => { +describe('File finder item spec', () => { const Component = Vue.extend(ItemComponent); let vm; let localFile; diff --git a/spec/lib/api/helpers_spec.rb b/spec/lib/api/helpers_spec.rb index e1aea82653d..08165f147bb 100644 --- a/spec/lib/api/helpers_spec.rb +++ b/spec/lib/api/helpers_spec.rb @@ -179,7 +179,7 @@ describe API::Helpers do context 'when blob name is not null' do it 'returns disposition with the blob name' do - expect(send_git_blob['Content-Disposition']).to eq 'inline; filename="foobar"' + expect(send_git_blob['Content-Disposition']).to eq %q(inline; filename="foobar"; filename*=UTF-8''foobar) end end end diff --git a/spec/lib/banzai/filter/autolink_filter_spec.rb b/spec/lib/banzai/filter/autolink_filter_spec.rb index 6217381c491..4972c4b4bd2 100644 --- a/spec/lib/banzai/filter/autolink_filter_spec.rb +++ b/spec/lib/banzai/filter/autolink_filter_spec.rb @@ -121,6 +121,13 @@ describe Banzai::Filter::AutolinkFilter do expect(doc.to_s).to eq("See #{link}") end + it 'does not autolink bad URLs after we remove trailing punctuation' do + link = 'http://]' + doc = filter("See #{link}") + + expect(doc.to_s).to eq("See #{link}") + end + it 'does not include trailing punctuation' do ['.', ', ok?', '...', '?', '!', ': is that ok?'].each do |trailing_punctuation| doc = filter("See #{link}#{trailing_punctuation}") diff --git a/spec/lib/banzai/filter/markdown_filter_spec.rb b/spec/lib/banzai/filter/markdown_filter_spec.rb index 4c4e821deab..83fcda29680 100644 --- a/spec/lib/banzai/filter/markdown_filter_spec.rb +++ b/spec/lib/banzai/filter/markdown_filter_spec.rb @@ -10,12 +10,6 @@ describe Banzai::Filter::MarkdownFilter do filter('test') end - it 'uses Redcarpet' do - expect_any_instance_of(Banzai::Filter::MarkdownEngines::Redcarpet).to receive(:render).and_return('test') - - filter('test', { markdown_engine: :redcarpet }) - end - it 'uses CommonMark' do expect_any_instance_of(Banzai::Filter::MarkdownEngines::CommonMark).to receive(:render).and_return('test') @@ -47,24 +41,6 @@ describe Banzai::Filter::MarkdownFilter do expect(result).to start_with('<pre><code lang="日">') end end - - context 'using Redcarpet' do - before do - stub_const('Banzai::Filter::MarkdownFilter::DEFAULT_ENGINE', :redcarpet) - end - - it 'adds language to lang attribute when specified' do - result = filter("```html\nsome code\n```") - - expect(result).to start_with("\n<pre><code lang=\"html\">") - end - - it 'does not add language to lang attribute when not specified' do - result = filter("```\nsome code\n```") - - expect(result).to start_with("\n<pre><code>") - end - end end describe 'source line position' do @@ -85,18 +61,6 @@ describe Banzai::Filter::MarkdownFilter do expect(result).to eq '<p>test</p>' end end - - context 'using Redcarpet' do - before do - stub_const('Banzai::Filter::MarkdownFilter::DEFAULT_ENGINE', :redcarpet) - end - - it 'does not support data-sourcepos' do - result = filter('test') - - expect(result).to eq '<p>test</p>' - end - end end describe 'footnotes in tables' do diff --git a/spec/lib/banzai/filter/spaced_link_filter_spec.rb b/spec/lib/banzai/filter/spaced_link_filter_spec.rb index 1ad7f3ff567..76d7644d76c 100644 --- a/spec/lib/banzai/filter/spaced_link_filter_spec.rb +++ b/spec/lib/banzai/filter/spaced_link_filter_spec.rb @@ -26,11 +26,6 @@ describe Banzai::Filter::SpacedLinkFilter do expect(doc.at_css('p')).to be_nil end - it 'does nothing when markdown_engine is redcarpet' do - exp = act = link - expect(filter(act, markdown_engine: :redcarpet).to_html).to eq exp - end - it 'does nothing with empty text' do link = '[](page slug)' doc = filter("See #{link}") diff --git a/spec/lib/banzai/object_renderer_spec.rb b/spec/lib/banzai/object_renderer_spec.rb index 209a547c3b3..3b52f6666d0 100644 --- a/spec/lib/banzai/object_renderer_spec.rb +++ b/spec/lib/banzai/object_renderer_spec.rb @@ -11,7 +11,7 @@ describe Banzai::ObjectRenderer do ) end - let(:object) { Note.new(note: 'hello', note_html: '<p dir="auto">hello</p>', cached_markdown_version: CacheMarkdownField::CACHE_COMMONMARK_VERSION) } + let(:object) { Note.new(note: 'hello', note_html: '<p dir="auto">hello</p>', cached_markdown_version: CacheMarkdownField::CACHE_COMMONMARK_VERSION << 16) } describe '#render' do context 'with cache' do diff --git a/spec/lib/gitlab/auth_spec.rb b/spec/lib/gitlab/auth_spec.rb index 236808c0b69..a4a6338961e 100644 --- a/spec/lib/gitlab/auth_spec.rb +++ b/spec/lib/gitlab/auth_spec.rb @@ -19,7 +19,7 @@ describe Gitlab::Auth do it 'optional_scopes contains all non-default scopes' do stub_container_registry_config(enabled: true) - expect(subject.optional_scopes).to eq %i[read_user sudo read_repository read_registry openid] + expect(subject.optional_scopes).to eq %i[read_user sudo read_repository read_registry openid profile email] end context 'registry_scopes' do diff --git a/spec/lib/gitlab/bare_repository_import/repository_spec.rb b/spec/lib/gitlab/bare_repository_import/repository_spec.rb index afd8f5da39f..a07c5371134 100644 --- a/spec/lib/gitlab/bare_repository_import/repository_spec.rb +++ b/spec/lib/gitlab/bare_repository_import/repository_spec.rb @@ -61,7 +61,7 @@ describe ::Gitlab::BareRepositoryImport::Repository do let(:wiki_path) { File.join(root_path, "#{hashed_path}.wiki.git") } before do - gitlab_shell.create_repository(repository_storage, hashed_path) + gitlab_shell.create_repository(repository_storage, hashed_path, 'group/project') Gitlab::GitalyClient::StorageSettings.allow_disk_access do repository = Rugged::Repository.new(repo_path) repository.config['gitlab.fullpath'] = 'to/repo' diff --git a/spec/lib/gitlab/bitbucket_import/importer_spec.rb b/spec/lib/gitlab/bitbucket_import/importer_spec.rb index 0def685f177..c432cc223b9 100644 --- a/spec/lib/gitlab/bitbucket_import/importer_spec.rb +++ b/spec/lib/gitlab/bitbucket_import/importer_spec.rb @@ -218,7 +218,7 @@ describe Gitlab::BitbucketImport::Importer do describe 'wiki import' do it 'is skipped when the wiki exists' do expect(project.wiki).to receive(:repository_exists?) { true } - expect(importer.gitlab_shell).not_to receive(:import_repository) + expect(importer.gitlab_shell).not_to receive(:import_wiki_repository) importer.execute @@ -227,11 +227,7 @@ describe Gitlab::BitbucketImport::Importer do it 'imports to the project disk_path' do expect(project.wiki).to receive(:repository_exists?) { false } - expect(importer.gitlab_shell).to receive(:import_repository).with( - project.repository_storage, - project.wiki.disk_path, - project.import_url + '/wiki' - ) + expect(importer.gitlab_shell).to receive(:import_wiki_repository) importer.execute diff --git a/spec/lib/gitlab/bitbucket_import/wiki_formatter_spec.rb b/spec/lib/gitlab/bitbucket_import/wiki_formatter_spec.rb new file mode 100644 index 00000000000..795fd069ab2 --- /dev/null +++ b/spec/lib/gitlab/bitbucket_import/wiki_formatter_spec.rb @@ -0,0 +1,29 @@ +require 'spec_helper' + +describe Gitlab::BitbucketImport::WikiFormatter do + let(:project) do + create(:project, + namespace: create(:namespace, path: 'gitlabhq'), + import_url: 'https://xxx@bitbucket.org/gitlabhq/sample.gitlabhq.git') + end + + subject(:wiki) { described_class.new(project) } + + describe '#disk_path' do + it 'appends .wiki to disk path' do + expect(wiki.disk_path).to eq project.wiki.disk_path + end + end + + describe '#full_path' do + it 'appends .wiki to project path' do + expect(wiki.full_path).to eq project.wiki.full_path + end + end + + describe '#import_url' do + it 'returns URL of the wiki repository' do + expect(wiki.import_url).to eq 'https://xxx@bitbucket.org/gitlabhq/sample.gitlabhq.git/wiki' + end + end +end diff --git a/spec/lib/gitlab/email/handler/create_note_handler_spec.rb b/spec/lib/gitlab/email/handler/create_note_handler_spec.rb index e5420ea6bea..50e473c459e 100644 --- a/spec/lib/gitlab/email/handler/create_note_handler_spec.rb +++ b/spec/lib/gitlab/email/handler/create_note_handler_spec.rb @@ -155,11 +155,7 @@ describe Gitlab::Email::Handler::CreateNoteHandler do it_behaves_like "checks permissions on noteable" end - context "when everything is fine" do - before do - setup_attachment - end - + shared_examples 'a reply to existing comment' do it "creates a comment" do expect { receiver.execute }.to change { noteable.notes.count }.by(1) new_note = noteable.notes.last @@ -168,7 +164,21 @@ describe Gitlab::Email::Handler::CreateNoteHandler do expect(new_note.position).to eq(note.position) expect(new_note.note).to include("I could not disagree more.") expect(new_note.in_reply_to?(note)).to be_truthy + + if note.part_of_discussion? + expect(new_note.discussion_id).to eq(note.discussion_id) + else + expect(new_note.discussion_id).not_to eq(note.discussion_id) + end end + end + + context "when everything is fine" do + before do + setup_attachment + end + + it_behaves_like 'a reply to existing comment' it "adds all attachments" do receiver.execute @@ -207,4 +217,10 @@ describe Gitlab::Email::Handler::CreateNoteHandler do end end end + + context "when note is not a discussion" do + let(:note) { create(:note_on_merge_request, project: project) } + + it_behaves_like 'a reply to existing comment' + end end diff --git a/spec/lib/gitlab/git/blame_spec.rb b/spec/lib/gitlab/git/blame_spec.rb index e704d1c673c..0010c0304eb 100644 --- a/spec/lib/gitlab/git/blame_spec.rb +++ b/spec/lib/gitlab/git/blame_spec.rb @@ -2,7 +2,7 @@ require "spec_helper" describe Gitlab::Git::Blame, :seed_helper do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '', 'group/project') } let(:blame) do Gitlab::Git::Blame.new(repository, SeedRepo::Commit::ID, "CONTRIBUTING.md") end diff --git a/spec/lib/gitlab/git/blob_spec.rb b/spec/lib/gitlab/git/blob_spec.rb index 1bcec04d28f..a1b5cea88c0 100644 --- a/spec/lib/gitlab/git/blob_spec.rb +++ b/spec/lib/gitlab/git/blob_spec.rb @@ -3,7 +3,7 @@ require "spec_helper" describe Gitlab::Git::Blob, :seed_helper do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '', 'group/project') } let(:rugged) do Rugged::Repository.new(File.join(TestEnv.repos_path, TEST_REPO_PATH)) end diff --git a/spec/lib/gitlab/git/branch_spec.rb b/spec/lib/gitlab/git/branch_spec.rb index 0df282d0ae3..0764e525ede 100644 --- a/spec/lib/gitlab/git/branch_spec.rb +++ b/spec/lib/gitlab/git/branch_spec.rb @@ -1,7 +1,7 @@ require "spec_helper" describe Gitlab::Git::Branch, :seed_helper do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '', 'group/project') } let(:rugged) do Rugged::Repository.new(File.join(TestEnv.repos_path, repository.relative_path)) end @@ -64,7 +64,7 @@ describe Gitlab::Git::Branch, :seed_helper do context 'with active, stale and future branches' do let(:repository) do - Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') + Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '', 'group/project') end let(:user) { create(:user) } diff --git a/spec/lib/gitlab/git/commit_spec.rb b/spec/lib/gitlab/git/commit_spec.rb index db68062e433..2611ebed25b 100644 --- a/spec/lib/gitlab/git/commit_spec.rb +++ b/spec/lib/gitlab/git/commit_spec.rb @@ -3,7 +3,7 @@ require "spec_helper" describe Gitlab::Git::Commit, :seed_helper do include GitHelpers - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '', 'group/project') } let(:rugged_repo) do Rugged::Repository.new(File.join(TestEnv.repos_path, TEST_REPO_PATH)) end @@ -146,7 +146,7 @@ describe Gitlab::Git::Commit, :seed_helper do end context 'with broken repo' do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_BROKEN_REPO_PATH, '') } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_BROKEN_REPO_PATH, '', 'group/project') } it 'returns nil' do expect(described_class.find(repository, SeedRepo::Commit::ID)).to be_nil diff --git a/spec/lib/gitlab/git/compare_spec.rb b/spec/lib/gitlab/git/compare_spec.rb index 771c71a16a9..65dfb93d0db 100644 --- a/spec/lib/gitlab/git/compare_spec.rb +++ b/spec/lib/gitlab/git/compare_spec.rb @@ -1,7 +1,7 @@ require "spec_helper" describe Gitlab::Git::Compare, :seed_helper do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '', 'group/project') } let(:compare) { Gitlab::Git::Compare.new(repository, SeedRepo::BigCommit::ID, SeedRepo::Commit::ID, straight: false) } let(:compare_straight) { Gitlab::Git::Compare.new(repository, SeedRepo::BigCommit::ID, SeedRepo::Commit::ID, straight: true) } diff --git a/spec/lib/gitlab/git/diff_spec.rb b/spec/lib/gitlab/git/diff_spec.rb index 8a4415506c4..1d22329b670 100644 --- a/spec/lib/gitlab/git/diff_spec.rb +++ b/spec/lib/gitlab/git/diff_spec.rb @@ -1,7 +1,7 @@ require "spec_helper" describe Gitlab::Git::Diff, :seed_helper do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '', 'group/project') } let(:gitaly_diff) do Gitlab::GitalyClient::Diff.new( from_path: '.gitmodules', diff --git a/spec/lib/gitlab/git/remote_repository_spec.rb b/spec/lib/gitlab/git/remote_repository_spec.rb index 53ed7c5a13a..e166628d4ca 100644 --- a/spec/lib/gitlab/git/remote_repository_spec.rb +++ b/spec/lib/gitlab/git/remote_repository_spec.rb @@ -1,15 +1,15 @@ require 'spec_helper' describe Gitlab::Git::RemoteRepository, :seed_helper do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '', 'group/project') } subject { described_class.new(repository) } describe '#empty?' do using RSpec::Parameterized::TableSyntax where(:repository, :result) do - Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') | false - Gitlab::Git::Repository.new('default', 'does-not-exist.git', '') | true + Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '', 'group/project') | false + Gitlab::Git::Repository.new('default', 'does-not-exist.git', '', 'group/project') | true end with_them do @@ -44,11 +44,11 @@ describe Gitlab::Git::RemoteRepository, :seed_helper do using RSpec::Parameterized::TableSyntax where(:other_repository, :result) do - repository | true - Gitlab::Git::Repository.new(repository.storage, repository.relative_path, '') | true - Gitlab::Git::Repository.new('broken', TEST_REPO_PATH, '') | false - Gitlab::Git::Repository.new(repository.storage, 'wrong/relative-path.git', '') | false - Gitlab::Git::Repository.new('broken', 'wrong/relative-path.git', '') | false + repository | true + Gitlab::Git::Repository.new(repository.storage, repository.relative_path, '', 'group/project') | true + Gitlab::Git::Repository.new('broken', TEST_REPO_PATH, '', 'group/project') | false + Gitlab::Git::Repository.new(repository.storage, 'wrong/relative-path.git', '', 'group/project') | false + Gitlab::Git::Repository.new('broken', 'wrong/relative-path.git', '', 'group/project') | false end with_them do diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index a19e3e84f83..cf9e0cccc71 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -19,8 +19,10 @@ describe Gitlab::Git::Repository, :seed_helper do end end - let(:mutable_repository) { Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') } - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } + let(:mutable_repository) { Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '', 'group/project') } + let(:mutable_repository_path) { File.join(TestEnv.repos_path, mutable_repository.relative_path) } + let(:mutable_repository_rugged) { Rugged::Repository.new(mutable_repository_path) } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '', 'group/project') } let(:repository_path) { File.join(TestEnv.repos_path, repository.relative_path) } let(:repository_rugged) { Rugged::Repository.new(repository_path) } let(:storage_path) { TestEnv.repos_path } @@ -434,13 +436,13 @@ describe Gitlab::Git::Repository, :seed_helper do describe '#fetch_repository_as_mirror' do let(:new_repository) do - Gitlab::Git::Repository.new('default', 'my_project.git', '') + Gitlab::Git::Repository.new('default', 'my_project.git', '', 'group/project') end subject { new_repository.fetch_repository_as_mirror(repository) } before do - Gitlab::Shell.new.create_repository('default', 'my_project') + Gitlab::Shell.new.create_repository('default', 'my_project', 'group/project') end after do @@ -497,6 +499,48 @@ describe Gitlab::Git::Repository, :seed_helper do end end + describe '#search_files_by_content' do + let(:repository) { mutable_repository } + let(:repository_rugged) { mutable_repository_rugged } + + before do + repository.create_branch('search-files-by-content-branch', 'master') + new_commit_edit_new_file_on_branch(repository_rugged, 'encoding/CHANGELOG', 'search-files-by-content-branch', 'committing something', 'search-files-by-content change') + new_commit_edit_new_file_on_branch(repository_rugged, 'anotherfile', 'search-files-by-content-branch', 'committing something', 'search-files-by-content change') + end + + after do + ensure_seeds + end + + shared_examples 'search files by content' do + it 'should have 2 items' do + expect(search_results.size).to eq(2) + end + + it 'should have the correct matching line' do + expect(search_results).to contain_exactly("search-files-by-content-branch:encoding/CHANGELOG\u00001\u0000search-files-by-content change\n", + "search-files-by-content-branch:anotherfile\u00001\u0000search-files-by-content change\n") + end + end + + it_should_behave_like 'search files by content' do + let(:search_results) do + repository.search_files_by_content('search-files-by-content', 'search-files-by-content-branch') + end + end + + it_should_behave_like 'search files by content' do + let(:search_results) do + repository.gitaly_repository_client.search_files_by_content( + 'search-files-by-content-branch', + 'search-files-by-content', + chunked_response: false + ) + end + end + end + describe '#find_remote_root_ref' do it 'gets the remote root ref from GitalyClient' do expect_any_instance_of(Gitlab::GitalyClient::RemoteService) @@ -544,7 +588,7 @@ describe Gitlab::Git::Repository, :seed_helper do # Add new commits so that there's a renamed file in the commit history @commit_with_old_name_id = new_commit_edit_old_file(repository_rugged).oid @rename_commit_id = new_commit_move_file(repository_rugged).oid - @commit_with_new_name_id = new_commit_edit_new_file(repository_rugged).oid + @commit_with_new_name_id = new_commit_edit_new_file(repository_rugged, "encoding/CHANGELOG", "Edit encoding/CHANGELOG", "I'm a new changelog with different text").oid end after do @@ -1230,7 +1274,7 @@ describe Gitlab::Git::Repository, :seed_helper do end describe '#gitattribute' do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_GITATTRIBUTES_REPO_PATH, '') } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_GITATTRIBUTES_REPO_PATH, '', 'group/project') } after do ensure_seeds @@ -1249,7 +1293,7 @@ describe Gitlab::Git::Repository, :seed_helper do end context 'without gitattributes file' do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '', 'group/project') } it 'returns nil' do expect(repository.gitattribute("README.md", 'gitlab-language')).to eq(nil) @@ -1513,7 +1557,7 @@ describe Gitlab::Git::Repository, :seed_helper do context 'repository does not exist' do it 'raises NoRepository and does not call Gitaly WriteConfig' do - repository = Gitlab::Git::Repository.new('default', 'does/not/exist.git', '') + repository = Gitlab::Git::Repository.new('default', 'does/not/exist.git', '', 'group/project') expect(repository.gitaly_repository_client).not_to receive(:write_config) @@ -1803,7 +1847,7 @@ describe Gitlab::Git::Repository, :seed_helper do out: '/dev/null', err: '/dev/null') - empty_repo = described_class.new('default', 'empty-repo.git', '') + empty_repo = described_class.new('default', 'empty-repo.git', '', 'group/empty-repo') expect(empty_repo.checksum).to eq '0000000000000000000000000000000000000000' end @@ -1818,13 +1862,13 @@ describe Gitlab::Git::Repository, :seed_helper do File.truncate(File.join(storage_path, 'non-valid.git/HEAD'), 0) - non_valid = described_class.new('default', 'non-valid.git', '') + non_valid = described_class.new('default', 'non-valid.git', '', 'a/non-valid') expect { non_valid.checksum }.to raise_error(Gitlab::Git::Repository::InvalidRepository) end it 'raises Gitlab::Git::Repository::NoRepository error when there is no repo' do - broken_repo = described_class.new('default', 'a/path.git', '') + broken_repo = described_class.new('default', 'a/path.git', '', 'a/path') expect { broken_repo.checksum }.to raise_error(Gitlab::Git::Repository::NoRepository) end @@ -1964,7 +2008,7 @@ describe Gitlab::Git::Repository, :seed_helper do end # Build the options hash that's passed to Rugged::Commit#create - def commit_options(repo, index, message) + def commit_options(repo, index, target, ref, message) options = {} options[:tree] = index.write_tree(repo) options[:author] = { @@ -1978,8 +2022,8 @@ describe Gitlab::Git::Repository, :seed_helper do time: Time.gm(2014, "mar", 3, 20, 15, 1) } options[:message] ||= message - options[:parents] = repo.empty? ? [] : [repo.head.target].compact - options[:update_ref] = "HEAD" + options[:parents] = repo.empty? ? [] : [target].compact + options[:update_ref] = ref options end @@ -1995,6 +2039,8 @@ describe Gitlab::Git::Repository, :seed_helper do options = commit_options( repo, index, + repo.head.target, + "HEAD", "Edit CHANGELOG in its original location" ) @@ -2003,19 +2049,24 @@ describe Gitlab::Git::Repository, :seed_helper do end # Writes a new commit to the repo and returns a Rugged::Commit. Replaces the - # contents of encoding/CHANGELOG with new text. - def new_commit_edit_new_file(repo) - oid = repo.write("I'm a new changelog with different text", :blob) + # contents of the specified file_path with new text. + def new_commit_edit_new_file(repo, file_path, commit_message, text, branch = repo.head) + oid = repo.write(text, :blob) index = repo.index - index.read_tree(repo.head.target.tree) - index.add(path: "encoding/CHANGELOG", oid: oid, mode: 0100644) - - options = commit_options(repo, index, "Edit encoding/CHANGELOG") - + index.read_tree(branch.target.tree) + index.add(path: file_path, oid: oid, mode: 0100644) + options = commit_options(repo, index, branch.target, branch.canonical_name, commit_message) sha = Rugged::Commit.create(repo, options) repo.lookup(sha) end + # Writes a new commit to the repo and returns a Rugged::Commit. Replaces the + # contents of encoding/CHANGELOG with new text. + def new_commit_edit_new_file_on_branch(repo, file_path, branch_name, commit_message, text) + branch = repo.branches[branch_name] + new_commit_edit_new_file(repo, file_path, commit_message, text, branch) + end + # Writes a new commit to the repo and returns a Rugged::Commit. Moves the # CHANGELOG file to the encoding/ directory. def new_commit_move_file(repo) @@ -2027,7 +2078,7 @@ describe Gitlab::Git::Repository, :seed_helper do index.add(path: "encoding/CHANGELOG", oid: oid, mode: 0100644) index.remove("CHANGELOG") - options = commit_options(repo, index, "Move CHANGELOG to encoding/") + options = commit_options(repo, index, repo.head.target, "HEAD", "Move CHANGELOG to encoding/") sha = Rugged::Commit.create(repo, options) repo.lookup(sha) diff --git a/spec/lib/gitlab/git/tag_spec.rb b/spec/lib/gitlab/git/tag_spec.rb index b51e3879f49..4c0291f64f0 100644 --- a/spec/lib/gitlab/git/tag_spec.rb +++ b/spec/lib/gitlab/git/tag_spec.rb @@ -1,7 +1,7 @@ require "spec_helper" describe Gitlab::Git::Tag, :seed_helper do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '', 'group/project') } describe '#tags' do describe 'first tag' do diff --git a/spec/lib/gitlab/git/tree_spec.rb b/spec/lib/gitlab/git/tree_spec.rb index bec875fb03d..4a4d69490a3 100644 --- a/spec/lib/gitlab/git/tree_spec.rb +++ b/spec/lib/gitlab/git/tree_spec.rb @@ -1,7 +1,7 @@ require "spec_helper" describe Gitlab::Git::Tree, :seed_helper do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '', 'group/project') } context :repo do let(:tree) { Gitlab::Git::Tree.where(repository, SeedRepo::Commit::ID) } diff --git a/spec/lib/gitlab/gitaly_client/remote_service_spec.rb b/spec/lib/gitlab/gitaly_client/remote_service_spec.rb index aff47599ad6..d5508dbff5d 100644 --- a/spec/lib/gitlab/gitaly_client/remote_service_spec.rb +++ b/spec/lib/gitlab/gitaly_client/remote_service_spec.rb @@ -33,7 +33,7 @@ describe Gitlab::GitalyClient::RemoteService do end describe '#fetch_internal_remote' do - let(:remote_repository) { Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') } + let(:remote_repository) { Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '', 'group/project') } it 'sends an fetch_internal_remote message and returns the result value' do expect_any_instance_of(Gitaly::RemoteService::Stub) diff --git a/spec/lib/gitlab/gitaly_client/util_spec.rb b/spec/lib/gitlab/gitaly_client/util_spec.rb index 550db6db6d9..78a5e195ad1 100644 --- a/spec/lib/gitlab/gitaly_client/util_spec.rb +++ b/spec/lib/gitlab/gitaly_client/util_spec.rb @@ -7,6 +7,7 @@ describe Gitlab::GitalyClient::Util do let(:gl_repository) { 'project-1' } let(:git_object_directory) { '.git/objects' } let(:git_alternate_object_directory) { ['/dir/one', '/dir/two'] } + let(:gl_project_path) { 'namespace/myproject' } let(:git_env) do { 'GIT_OBJECT_DIRECTORY_RELATIVE' => git_object_directory, @@ -15,7 +16,7 @@ describe Gitlab::GitalyClient::Util do end subject do - described_class.repository(repository_storage, relative_path, gl_repository) + described_class.repository(repository_storage, relative_path, gl_repository, gl_project_path) end it 'creates a Gitaly::Repository with the given data' do @@ -27,6 +28,7 @@ describe Gitlab::GitalyClient::Util do expect(subject.gl_repository).to eq(gl_repository) expect(subject.git_object_directory).to eq(git_object_directory) expect(subject.git_alternate_object_directories).to eq(git_alternate_object_directory) + expect(subject.gl_project_path).to eq(gl_project_path) end end end diff --git a/spec/lib/gitlab/github_import/importer/repository_importer_spec.rb b/spec/lib/gitlab/github_import/importer/repository_importer_spec.rb index 77f5b2ffa37..47233ea6ee2 100644 --- a/spec/lib/gitlab/github_import/importer/repository_importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer/repository_importer_spec.rb @@ -5,6 +5,14 @@ describe Gitlab::GithubImport::Importer::RepositoryImporter do let(:import_state) { double(:import_state) } let(:client) { double(:client) } + let(:wiki) do + double( + :wiki, + disk_path: 'foo.wiki', + full_path: 'group/foo.wiki' + ) + end + let(:project) do double( :project, @@ -15,7 +23,9 @@ describe Gitlab::GithubImport::Importer::RepositoryImporter do repository: repository, create_wiki: true, import_state: import_state, - lfs_enabled?: true + full_path: 'group/foo', + lfs_enabled?: true, + wiki: wiki ) end @@ -195,7 +205,7 @@ describe Gitlab::GithubImport::Importer::RepositoryImporter do it 'imports the wiki repository' do expect(importer.gitlab_shell) .to receive(:import_repository) - .with('foo', 'foo.wiki', 'foo.wiki.git') + .with('foo', 'foo.wiki', 'foo.wiki.git', 'group/foo.wiki') expect(importer.import_wiki_repository).to eq(true) end diff --git a/spec/lib/gitlab/legacy_github_import/wiki_formatter_spec.rb b/spec/lib/gitlab/legacy_github_import/wiki_formatter_spec.rb index 7723533aee2..7519707293c 100644 --- a/spec/lib/gitlab/legacy_github_import/wiki_formatter_spec.rb +++ b/spec/lib/gitlab/legacy_github_import/wiki_formatter_spec.rb @@ -10,11 +10,17 @@ describe Gitlab::LegacyGithubImport::WikiFormatter do subject(:wiki) { described_class.new(project) } describe '#disk_path' do - it 'appends .wiki to project path' do + it 'appends .wiki to disk path' do expect(wiki.disk_path).to eq project.wiki.disk_path end end + describe '#full_path' do + it 'appends .wiki to project path' do + expect(wiki.full_path).to eq project.wiki.full_path + end + end + describe '#import_url' do it 'returns URL of the wiki repository' do expect(wiki.import_url).to eq 'https://xxx@github.com/gitlabhq/sample.gitlabhq.wiki.git' diff --git a/spec/lib/gitlab/metrics/samplers/unicorn_sampler_spec.rb b/spec/lib/gitlab/metrics/samplers/unicorn_sampler_spec.rb index 771b633a2b9..4b03f3c2532 100644 --- a/spec/lib/gitlab/metrics/samplers/unicorn_sampler_spec.rb +++ b/spec/lib/gitlab/metrics/samplers/unicorn_sampler_spec.rb @@ -37,7 +37,7 @@ describe Gitlab::Metrics::Samplers::UnicornSampler do end it 'updates metrics type unix and with addr' do - labels = { type: 'unix', address: socket_address } + labels = { socket_type: 'unix', socket_address: socket_address } expect(subject).to receive_message_chain(:unicorn_active_connections, :set).with(labels, 'active') expect(subject).to receive_message_chain(:unicorn_queued_connections, :set).with(labels, 'queued') @@ -69,7 +69,7 @@ describe Gitlab::Metrics::Samplers::UnicornSampler do end it 'updates metrics type unix and with addr' do - labels = { type: 'tcp', address: tcp_socket_address } + labels = { socket_type: 'tcp', socket_address: tcp_socket_address } expect(subject).to receive_message_chain(:unicorn_active_connections, :set).with(labels, 'active') expect(subject).to receive_message_chain(:unicorn_queued_connections, :set).with(labels, 'queued') diff --git a/spec/lib/gitlab/shell_spec.rb b/spec/lib/gitlab/shell_spec.rb index 6ce9d515a0f..033e1bf81a1 100644 --- a/spec/lib/gitlab/shell_spec.rb +++ b/spec/lib/gitlab/shell_spec.rb @@ -412,7 +412,7 @@ describe Gitlab::Shell do end it 'creates a repository' do - expect(gitlab_shell.create_repository(repository_storage, repo_name)).to be_truthy + expect(gitlab_shell.create_repository(repository_storage, repo_name, repo_name)).to be_truthy expect(File.stat(created_path).mode & 0o777).to eq(0o770) @@ -427,7 +427,7 @@ describe Gitlab::Shell do # should cause #create_repository to fail. FileUtils.touch(created_path) - expect(gitlab_shell.create_repository(repository_storage, repo_name)).to be_falsy + expect(gitlab_shell.create_repository(repository_storage, repo_name, repo_name)).to be_falsy end end @@ -474,13 +474,10 @@ describe Gitlab::Shell do end describe '#fork_repository' do + let(:target_project) { create(:project) } + subject do - gitlab_shell.fork_repository( - project.repository_storage, - project.disk_path, - 'nfs-file05', - 'fork/path' - ) + gitlab_shell.fork_repository(project, target_project) end it 'returns true when the command succeeds' do @@ -505,7 +502,7 @@ describe Gitlab::Shell do it 'returns true when the command succeeds' do expect_any_instance_of(Gitlab::GitalyClient::RepositoryService).to receive(:import_repository).with(import_url) - result = gitlab_shell.import_repository(project.repository_storage, project.disk_path, import_url) + result = gitlab_shell.import_repository(project.repository_storage, project.disk_path, import_url, project.full_path) expect(result).to be_truthy end @@ -516,7 +513,7 @@ describe Gitlab::Shell do expect_any_instance_of(Gitlab::Shell::GitalyGitlabProjects).to receive(:output) { 'error'} expect do - gitlab_shell.import_repository(project.repository_storage, project.disk_path, import_url) + gitlab_shell.import_repository(project.repository_storage, project.disk_path, import_url, project.full_path) end.to raise_error(Gitlab::Shell::Error, "error") end end diff --git a/spec/models/application_record_spec.rb b/spec/models/application_record_spec.rb index 68aed387bfc..fd25132ed3a 100644 --- a/spec/models/application_record_spec.rb +++ b/spec/models/application_record_spec.rb @@ -10,4 +10,27 @@ describe ApplicationRecord do expect(User.id_in(records.last(2).map(&:id))).to eq(records.last(2)) end end + + describe '.safe_find_or_create_by' do + it 'creates the user avoiding race conditions' do + expect(Suggestion).to receive(:find_or_create_by).and_raise(ActiveRecord::RecordNotUnique) + allow(Suggestion).to receive(:find_or_create_by).and_call_original + + expect { Suggestion.safe_find_or_create_by(build(:suggestion).attributes) } + .to change { Suggestion.count }.by(1) + end + end + + describe '.safe_find_or_create_by!' do + it 'creates a record using safe_find_or_create_by' do + expect(Suggestion).to receive(:find_or_create_by).and_call_original + + expect(Suggestion.safe_find_or_create_by!(build(:suggestion).attributes)) + .to be_a(Suggestion) + end + + it 'raises a validation error if the record was not persisted' do + expect { Suggestion.find_or_create_by!(note: nil) }.to raise_error(ActiveRecord::RecordInvalid) + end + end end diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb index 96aa9a82b71..789e14e8a20 100644 --- a/spec/models/application_setting_spec.rb +++ b/spec/models/application_setting_spec.rb @@ -70,6 +70,13 @@ describe ApplicationSetting do .is_greater_than(0) end + it do + is_expected.to validate_numericality_of(:local_markdown_version) + .only_integer + .is_greater_than_or_equal_to(0) + .is_less_than(65536) + end + context 'key restrictions' do it 'supports all key types' do expect(described_class::SUPPORTED_KEY_TYPES).to contain_exactly(:rsa, :dsa, :ecdsa, :ed25519) diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 8a1bbb26e57..47865e4d08f 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -1844,6 +1844,26 @@ describe Ci::Build do context 'when there is no environment' do it { is_expected.to be_nil } end + + context 'when build has a start environment' do + let(:build) { create(:ci_build, :deploy_to_production, pipeline: pipeline) } + + it 'does not expand environment name' do + expect(build).not_to receive(:expanded_environment_name) + + subject + end + end + + context 'when build has a stop environment' do + let(:build) { create(:ci_build, :stop_review_app, pipeline: pipeline) } + + it 'expands environment name' do + expect(build).to receive(:expanded_environment_name) + + subject + end + end end describe '#play' do diff --git a/spec/models/clusters/applications/cert_manager_spec.rb b/spec/models/clusters/applications/cert_manager_spec.rb index 8e14abe098d..79a06c35459 100644 --- a/spec/models/clusters/applications/cert_manager_spec.rb +++ b/spec/models/clusters/applications/cert_manager_spec.rb @@ -4,20 +4,8 @@ describe Clusters::Applications::CertManager do let(:cert_manager) { create(:clusters_applications_cert_managers) } include_examples 'cluster application core specs', :clusters_applications_cert_managers - - describe '#make_installing!' do - before do - application.make_installing! - end - - context 'application install previously errored with older version' do - let(:application) { create(:clusters_applications_cert_managers, :scheduled, version: 'v0.4.0') } - - it 'updates the application version' do - expect(application.reload.version).to eq('v0.5.2') - end - end - end + include_examples 'cluster application status specs', :clusters_applications_cert_managers + include_examples 'cluster application initial status specs' describe '#install_command' do let(:cluster_issuer_file) { { "cluster_issuer.yaml": "---\napiVersion: certmanager.k8s.io/v1alpha1\nkind: ClusterIssuer\nmetadata:\n name: letsencrypt-prod\nspec:\n acme:\n server: https://acme-v02.api.letsencrypt.org/directory\n email: admin@example.com\n privateKeySecretRef:\n name: letsencrypt-prod\n http01: {}\n" } } diff --git a/spec/models/clusters/applications/ingress_spec.rb b/spec/models/clusters/applications/ingress_spec.rb index 52c347229c6..6d48131d1cc 100644 --- a/spec/models/clusters/applications/ingress_spec.rb +++ b/spec/models/clusters/applications/ingress_spec.rb @@ -8,6 +8,7 @@ describe Clusters::Applications::Ingress do include_examples 'cluster application core specs', :clusters_applications_ingress include_examples 'cluster application status specs', :clusters_applications_ingress include_examples 'cluster application helm specs', :clusters_applications_ingress + include_examples 'cluster application initial status specs' before do allow(ClusterWaitForIngressIpAddressWorker).to receive(:perform_in) @@ -26,20 +27,6 @@ describe Clusters::Applications::Ingress do it { is_expected.to contain_exactly(cluster) } end - describe '#make_installing!' do - before do - application.make_installing! - end - - context 'application install previously errored with older version' do - let(:application) { create(:clusters_applications_ingress, :scheduled, version: '0.22.0') } - - it 'updates the application version' do - expect(application.reload.version).to eq('1.1.2') - end - end - end - describe '#make_installed!' do before do application.make_installed! diff --git a/spec/models/clusters/applications/jupyter_spec.rb b/spec/models/clusters/applications/jupyter_spec.rb index 391e5425384..b73a243f6e0 100644 --- a/spec/models/clusters/applications/jupyter_spec.rb +++ b/spec/models/clusters/applications/jupyter_spec.rb @@ -2,6 +2,7 @@ require 'rails_helper' describe Clusters::Applications::Jupyter do include_examples 'cluster application core specs', :clusters_applications_jupyter + include_examples 'cluster application status specs', :clusters_applications_jupyter include_examples 'cluster application helm specs', :clusters_applications_jupyter it { is_expected.to belong_to(:oauth_application) } @@ -26,20 +27,6 @@ describe Clusters::Applications::Jupyter do end end - describe '#make_installing!' do - before do - application.make_installing! - end - - context 'application install previously errored with older version' do - let(:application) { create(:clusters_applications_jupyter, :scheduled, version: 'v0.5') } - - it 'updates the application version' do - expect(application.reload.version).to eq('v0.6') - end - end - end - describe '#install_command' do let!(:ingress) { create(:clusters_applications_ingress, :installed, external_ip: '127.0.0.1') } let!(:jupyter) { create(:clusters_applications_jupyter, cluster: ingress.cluster) } diff --git a/spec/models/clusters/applications/knative_spec.rb b/spec/models/clusters/applications/knative_spec.rb index 35818be8deb..5519615d52d 100644 --- a/spec/models/clusters/applications/knative_spec.rb +++ b/spec/models/clusters/applications/knative_spec.rb @@ -9,6 +9,7 @@ describe Clusters::Applications::Knative do include_examples 'cluster application core specs', :clusters_applications_knative include_examples 'cluster application status specs', :clusters_applications_knative include_examples 'cluster application helm specs', :clusters_applications_knative + include_examples 'cluster application initial status specs' before do allow(ClusterWaitForIngressIpAddressWorker).to receive(:perform_in) @@ -34,20 +35,6 @@ describe Clusters::Applications::Knative do it { is_expected.to contain_exactly(cluster) } end - describe '#make_installing!' do - before do - application.make_installing! - end - - context 'application install previously errored with older version' do - let(:application) { create(:clusters_applications_knative, :scheduled, version: '0.2.2') } - - it 'updates the application version' do - expect(application.reload.version).to eq('0.2.2') - end - end - end - describe '#make_installed' do subject { described_class.installed } diff --git a/spec/models/clusters/applications/prometheus_spec.rb b/spec/models/clusters/applications/prometheus_spec.rb index e50ba67c493..073fbded8ac 100644 --- a/spec/models/clusters/applications/prometheus_spec.rb +++ b/spec/models/clusters/applications/prometheus_spec.rb @@ -6,6 +6,7 @@ describe Clusters::Applications::Prometheus do include_examples 'cluster application core specs', :clusters_applications_prometheus include_examples 'cluster application status specs', :clusters_applications_prometheus include_examples 'cluster application helm specs', :clusters_applications_prometheus + include_examples 'cluster application initial status specs' describe '.installed' do subject { described_class.installed } @@ -19,20 +20,6 @@ describe Clusters::Applications::Prometheus do it { is_expected.to contain_exactly(cluster) } end - describe '#make_installing!' do - before do - application.make_installing! - end - - context 'application install previously errored with older version' do - let(:application) { create(:clusters_applications_prometheus, :scheduled, version: '6.7.2') } - - it 'updates the application version' do - expect(application.reload.version).to eq('6.7.3') - end - end - end - describe 'transition to installed' do let(:project) { create(:project) } let(:cluster) { create(:cluster, :with_installed_helm, projects: [project]) } diff --git a/spec/models/clusters/applications/runner_spec.rb b/spec/models/clusters/applications/runner_spec.rb index 8ad41e997c2..96b7b02dbaf 100644 --- a/spec/models/clusters/applications/runner_spec.rb +++ b/spec/models/clusters/applications/runner_spec.rb @@ -6,23 +6,10 @@ describe Clusters::Applications::Runner do include_examples 'cluster application core specs', :clusters_applications_runner include_examples 'cluster application status specs', :clusters_applications_runner include_examples 'cluster application helm specs', :clusters_applications_runner + include_examples 'cluster application initial status specs' it { is_expected.to belong_to(:runner) } - describe '#make_installing!' do - before do - application.make_installing! - end - - context 'application install previously errored with older version' do - let(:application) { create(:clusters_applications_runner, :scheduled, version: '0.1.30') } - - it 'updates the application version' do - expect(application.reload.version).to eq('0.1.45') - end - end - end - describe '.installed' do subject { described_class.installed } diff --git a/spec/models/commit_collection_spec.rb b/spec/models/commit_collection_spec.rb index 005005b236b..12e59b35428 100644 --- a/spec/models/commit_collection_spec.rb +++ b/spec/models/commit_collection_spec.rb @@ -35,6 +35,17 @@ describe CommitCollection do end end + describe '#without_merge_commits' do + it 'returns all commits except merge commits' do + collection = described_class.new(project, [ + build(:commit), + build(:commit, :merge_commit) + ]) + + expect(collection.without_merge_commits.size).to eq(1) + end + end + describe '#with_pipeline_status' do it 'sets the pipeline status for every commit so no additional queries are necessary' do create( diff --git a/spec/models/concerns/cache_markdown_field_spec.rb b/spec/models/concerns/cache_markdown_field_spec.rb index 925e2ab0955..447279f19a8 100644 --- a/spec/models/concerns/cache_markdown_field_spec.rb +++ b/spec/models/concerns/cache_markdown_field_spec.rb @@ -60,6 +60,10 @@ describe CacheMarkdownField do changes_applied end end + + def has_attribute?(attr_name) + attribute_names.include?(attr_name) + end end def thing_subclass(new_attr) @@ -72,7 +76,8 @@ describe CacheMarkdownField do let(:updated_markdown) { '`Bar`' } let(:updated_html) { '<p dir="auto"><code>Bar</code></p>' } - let(:thing) { ThingWithMarkdownFields.new(foo: markdown, foo_html: html, cached_markdown_version: CacheMarkdownField::CACHE_COMMONMARK_VERSION) } + let(:thing) { ThingWithMarkdownFields.new(foo: markdown, foo_html: html, cached_markdown_version: cache_version) } + let(:cache_version) { CacheMarkdownField::CACHE_COMMONMARK_VERSION << 16 } before do stub_commonmark_sourcepos_disabled @@ -93,24 +98,19 @@ describe CacheMarkdownField do it { expect(thing.foo).to eq(markdown) } it { expect(thing.foo_html).to eq(html) } it { expect(thing.foo_html_changed?).not_to be_truthy } - it { expect(thing.cached_markdown_version).to eq(CacheMarkdownField::CACHE_COMMONMARK_VERSION) } + it { expect(thing.cached_markdown_version).to eq(cache_version) } end context 'a changed markdown field' do - shared_examples 'with cache version' do |cache_version| - let(:thing) { ThingWithMarkdownFields.new(foo: markdown, foo_html: html, cached_markdown_version: cache_version) } - - before do - thing.foo = updated_markdown - thing.save - end + let(:thing) { ThingWithMarkdownFields.new(foo: markdown, foo_html: html, cached_markdown_version: cache_version - 1) } - it { expect(thing.foo_html).to eq(updated_html) } - it { expect(thing.cached_markdown_version).to eq(cache_version) } + before do + thing.foo = updated_markdown + thing.save end - it_behaves_like 'with cache version', CacheMarkdownField::CACHE_REDCARPET_VERSION - it_behaves_like 'with cache version', CacheMarkdownField::CACHE_COMMONMARK_VERSION + it { expect(thing.foo_html).to eq(updated_html) } + it { expect(thing.cached_markdown_version).to eq(cache_version) } end context 'when a markdown field is set repeatedly to an empty string' do @@ -143,22 +143,17 @@ describe CacheMarkdownField do end context 'a non-markdown field changed' do - shared_examples 'with cache version' do |cache_version| - let(:thing) { ThingWithMarkdownFields.new(foo: markdown, foo_html: html, cached_markdown_version: cache_version) } + let(:thing) { ThingWithMarkdownFields.new(foo: markdown, foo_html: html, cached_markdown_version: cache_version - 1) } - before do - thing.bar = 'OK' - thing.save - end - - it { expect(thing.bar).to eq('OK') } - it { expect(thing.foo).to eq(markdown) } - it { expect(thing.foo_html).to eq(html) } - it { expect(thing.cached_markdown_version).to eq(cache_version) } + before do + thing.bar = 'OK' + thing.save end - it_behaves_like 'with cache version', CacheMarkdownField::CACHE_REDCARPET_VERSION - it_behaves_like 'with cache version', CacheMarkdownField::CACHE_COMMONMARK_VERSION + it { expect(thing.bar).to eq('OK') } + it { expect(thing.foo).to eq(markdown) } + it { expect(thing.foo_html).to eq(html) } + it { expect(thing.cached_markdown_version).to eq(cache_version) } end context 'version is out of date' do @@ -169,109 +164,84 @@ describe CacheMarkdownField do end it { expect(thing.foo_html).to eq(updated_html) } - it { expect(thing.cached_markdown_version).to eq(CacheMarkdownField::CACHE_COMMONMARK_VERSION) } + it { expect(thing.cached_markdown_version).to eq(cache_version) } end describe '#cached_html_up_to_date?' do - shared_examples 'with cache version' do |cache_version| - let(:thing) { ThingWithMarkdownFields.new(foo: markdown, foo_html: html, cached_markdown_version: cache_version) } - - subject { thing.cached_html_up_to_date?(:foo) } - - it 'returns false when the version is absent' do - thing.cached_markdown_version = nil - - is_expected.to be_falsy - end - - it 'returns false when the version is too early' do - thing.cached_markdown_version -= 1 + let(:thing) { ThingWithMarkdownFields.new(foo: markdown, foo_html: html, cached_markdown_version: cache_version) } - is_expected.to be_falsy - end + subject { thing.cached_html_up_to_date?(:foo) } - it 'returns false when the version is too late' do - thing.cached_markdown_version += 1 + it 'returns false when the version is absent' do + thing.cached_markdown_version = nil - is_expected.to be_falsy - end + is_expected.to be_falsy + end - it 'returns true when the version is just right' do - thing.cached_markdown_version = cache_version + it 'returns false when the cached version is too old' do + thing.cached_markdown_version = cache_version - 1 - is_expected.to be_truthy - end + is_expected.to be_falsy + end - it 'returns false if markdown has been changed but html has not' do - thing.foo = updated_html + it 'returns false when the cached version is in future' do + thing.cached_markdown_version = cache_version + 1 - is_expected.to be_falsy - end + is_expected.to be_falsy + end - it 'returns true if markdown has not been changed but html has' do - thing.foo_html = updated_html + it 'returns false when the local version was bumped' do + allow(Gitlab::CurrentSettings.current_application_settings).to receive(:local_markdown_version).and_return(2) + thing.cached_markdown_version = cache_version - is_expected.to be_truthy - end + is_expected.to be_falsy + end - it 'returns true if markdown and html have both been changed' do - thing.foo = updated_markdown - thing.foo_html = updated_html + it 'returns true when the local version is default' do + thing.cached_markdown_version = cache_version - is_expected.to be_truthy - end + is_expected.to be_truthy + end - it 'returns false if the markdown field is set but the html is not' do - thing.foo_html = nil + it 'returns true when the cached version is just right' do + allow(Gitlab::CurrentSettings.current_application_settings).to receive(:local_markdown_version).and_return(2) + thing.cached_markdown_version = cache_version + 2 - is_expected.to be_falsy - end + is_expected.to be_truthy end - it_behaves_like 'with cache version', CacheMarkdownField::CACHE_REDCARPET_VERSION - it_behaves_like 'with cache version', CacheMarkdownField::CACHE_COMMONMARK_VERSION - end - - describe '#latest_cached_markdown_version' do - subject { thing.latest_cached_markdown_version } + it 'returns false if markdown has been changed but html has not' do + thing.foo = updated_html - it 'returns redcarpet version' do - thing.cached_markdown_version = CacheMarkdownField::CACHE_COMMONMARK_VERSION_START - 1 - is_expected.to eq(CacheMarkdownField::CACHE_REDCARPET_VERSION) + is_expected.to be_falsy end - it 'returns commonmark version' do - thing.cached_markdown_version = CacheMarkdownField::CACHE_COMMONMARK_VERSION_START + 1 - is_expected.to eq(CacheMarkdownField::CACHE_COMMONMARK_VERSION) - end + it 'returns true if markdown has not been changed but html has' do + thing.foo_html = updated_html - it 'returns default version when version is nil' do - thing.cached_markdown_version = nil - is_expected.to eq(CacheMarkdownField::CACHE_COMMONMARK_VERSION) + is_expected.to be_truthy end - end - describe '#legacy_markdown?' do - subject { thing.legacy_markdown? } + it 'returns true if markdown and html have both been changed' do + thing.foo = updated_markdown + thing.foo_html = updated_html - it 'returns true for redcarpet versions' do - thing.cached_markdown_version = CacheMarkdownField::CACHE_COMMONMARK_VERSION_START - 1 is_expected.to be_truthy end - it 'returns false for commonmark versions' do - thing.cached_markdown_version = CacheMarkdownField::CACHE_COMMONMARK_VERSION_START - is_expected.to be_falsey - end + it 'returns false if the markdown field is set but the html is not' do + thing.foo_html = nil - it 'returns false if nil' do - thing.cached_markdown_version = nil - is_expected.to be_falsey + is_expected.to be_falsy end + end - it 'returns false if 0' do - thing.cached_markdown_version = 0 - is_expected.to be_falsey + describe '#latest_cached_markdown_version' do + subject { thing.latest_cached_markdown_version } + + it 'returns default version' do + thing.cached_markdown_version = nil + is_expected.to eq(cache_version) end end @@ -298,44 +268,39 @@ describe CacheMarkdownField do thing.cached_markdown_version = nil thing.refresh_markdown_cache - expect(thing.cached_markdown_version).to eq(CacheMarkdownField::CACHE_COMMONMARK_VERSION) + expect(thing.cached_markdown_version).to eq(cache_version) end end describe '#refresh_markdown_cache!' do - shared_examples 'with cache version' do |cache_version| - let(:thing) { ThingWithMarkdownFields.new(foo: markdown, foo_html: html, cached_markdown_version: cache_version) } + let(:thing) { ThingWithMarkdownFields.new(foo: markdown, foo_html: html, cached_markdown_version: cache_version) } - before do - thing.foo = updated_markdown - end + before do + thing.foo = updated_markdown + end - it 'fills all html fields' do - thing.refresh_markdown_cache! + it 'fills all html fields' do + thing.refresh_markdown_cache! - expect(thing.foo_html).to eq(updated_html) - expect(thing.foo_html_changed?).to be_truthy - expect(thing.baz_html_changed?).to be_truthy - end + expect(thing.foo_html).to eq(updated_html) + expect(thing.foo_html_changed?).to be_truthy + expect(thing.baz_html_changed?).to be_truthy + end - it 'skips saving if not persisted' do - expect(thing).to receive(:persisted?).and_return(false) - expect(thing).not_to receive(:update_columns) + it 'skips saving if not persisted' do + expect(thing).to receive(:persisted?).and_return(false) + expect(thing).not_to receive(:update_columns) - thing.refresh_markdown_cache! - end + thing.refresh_markdown_cache! + end - it 'saves the changes using #update_columns' do - expect(thing).to receive(:persisted?).and_return(true) - expect(thing).to receive(:update_columns) - .with("foo_html" => updated_html, "baz_html" => "", "cached_markdown_version" => cache_version) + it 'saves the changes using #update_columns' do + expect(thing).to receive(:persisted?).and_return(true) + expect(thing).to receive(:update_columns) + .with("foo_html" => updated_html, "baz_html" => "", "cached_markdown_version" => cache_version) - thing.refresh_markdown_cache! - end + thing.refresh_markdown_cache! end - - it_behaves_like 'with cache version', CacheMarkdownField::CACHE_REDCARPET_VERSION - it_behaves_like 'with cache version', CacheMarkdownField::CACHE_COMMONMARK_VERSION end describe '#banzai_render_context' do @@ -384,7 +349,7 @@ describe CacheMarkdownField do expect(thing.foo_html).to eq(updated_html) expect(thing.baz_html).to eq(updated_html) - expect(thing.cached_markdown_version).to eq(CacheMarkdownField::CACHE_COMMONMARK_VERSION) + expect(thing.cached_markdown_version).to eq(cache_version) end end @@ -404,24 +369,8 @@ describe CacheMarkdownField do expect(thing.foo_html).to eq(updated_html) expect(thing.baz_html).to eq(updated_html) - expect(thing.cached_markdown_version).to eq(CacheMarkdownField::CACHE_COMMONMARK_VERSION) + expect(thing.cached_markdown_version).to eq(cache_version) end end end - - describe CacheMarkdownField::MarkdownEngine do - subject { lambda { |version| CacheMarkdownField::MarkdownEngine.from_version(version) } } - - it 'returns :common_mark as a default' do - expect(subject.call(nil)).to eq :common_mark - end - - it 'returns :common_mark' do - expect(subject.call(CacheMarkdownField::CACHE_COMMONMARK_VERSION)).to eq :common_mark - end - - it 'returns :redcarpet' do - expect(subject.call(CacheMarkdownField::CACHE_REDCARPET_VERSION)).to eq :redcarpet - end - end end diff --git a/spec/models/concerns/issuable_spec.rb b/spec/models/concerns/issuable_spec.rb index 5753c646106..41159348e04 100644 --- a/spec/models/concerns/issuable_spec.rb +++ b/spec/models/concerns/issuable_spec.rb @@ -139,6 +139,78 @@ describe Issuable do it 'returns issues with a matching description for a query shorter than 3 chars' do expect(issuable_class.full_search(searchable_issue2.description.downcase)).to eq([searchable_issue2]) end + + context 'when matching columns is "title"' do + it 'returns issues with a matching title' do + expect(issuable_class.full_search(searchable_issue.title, matched_columns: 'title')) + .to eq([searchable_issue]) + end + + it 'returns no issues with a matching description' do + expect(issuable_class.full_search(searchable_issue.description, matched_columns: 'title')) + .to be_empty + end + end + + context 'when matching columns is "description"' do + it 'returns no issues with a matching title' do + expect(issuable_class.full_search(searchable_issue.title, matched_columns: 'description')) + .to be_empty + end + + it 'returns issues with a matching description' do + expect(issuable_class.full_search(searchable_issue.description, matched_columns: 'description')) + .to eq([searchable_issue]) + end + end + + context 'when matching columns is "title,description"' do + it 'returns issues with a matching title' do + expect(issuable_class.full_search(searchable_issue.title, matched_columns: 'title,description')) + .to eq([searchable_issue]) + end + + it 'returns issues with a matching description' do + expect(issuable_class.full_search(searchable_issue.description, matched_columns: 'title,description')) + .to eq([searchable_issue]) + end + end + + context 'when matching columns is nil"' do + it 'returns issues with a matching title' do + expect(issuable_class.full_search(searchable_issue.title, matched_columns: nil)) + .to eq([searchable_issue]) + end + + it 'returns issues with a matching description' do + expect(issuable_class.full_search(searchable_issue.description, matched_columns: nil)) + .to eq([searchable_issue]) + end + end + + context 'when matching columns is "invalid"' do + it 'returns issues with a matching title' do + expect(issuable_class.full_search(searchable_issue.title, matched_columns: 'invalid')) + .to eq([searchable_issue]) + end + + it 'returns issues with a matching description' do + expect(issuable_class.full_search(searchable_issue.description, matched_columns: 'invalid')) + .to eq([searchable_issue]) + end + end + + context 'when matching columns is "title,invalid"' do + it 'returns issues with a matching title' do + expect(issuable_class.full_search(searchable_issue.title, matched_columns: 'title,invalid')) + .to eq([searchable_issue]) + end + + it 'returns no issues with a matching description' do + expect(issuable_class.full_search(searchable_issue.description, matched_columns: 'title,invalid')) + .to be_empty + end + end end describe '.to_ability_name' do diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index 9a3f1f1c5a1..2d554326f05 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -41,6 +41,76 @@ describe Environment do end end + describe '.for_name_like' do + subject { project.environments.for_name_like(query, limit: limit) } + + let!(:environment) { create(:environment, name: 'production', project: project) } + let(:query) { 'pro' } + let(:limit) { 5 } + + it 'returns a found name' do + is_expected.to include(environment) + end + + context 'when query is production' do + let(:query) { 'production' } + + it 'returns a found name' do + is_expected.to include(environment) + end + end + + context 'when query is productionA' do + let(:query) { 'productionA' } + + it 'returns empty array' do + is_expected.to be_empty + end + end + + context 'when query is empty' do + let(:query) { '' } + + it 'returns a found name' do + is_expected.to include(environment) + end + end + + context 'when query is nil' do + let(:query) { } + + it 'raises an error' do + expect { subject }.to raise_error(NoMethodError) + end + end + + context 'when query is partially matched in the middle of environment name' do + let(:query) { 'duction' } + + it 'returns empty array' do + is_expected.to be_empty + end + end + + context 'when query contains a wildcard character' do + let(:query) { 'produc%' } + + it 'prevents wildcard injection' do + is_expected.to be_empty + end + end + end + + describe '.pluck_names' do + subject { described_class.pluck_names } + + let!(:environment) { create(:environment, name: 'production', project: project) } + + it 'plucks names' do + is_expected.to eq(%w[production]) + end + end + describe '#expire_etag_cache' do let(:store) { Gitlab::EtagCaching::Store.new } diff --git a/spec/models/gpg_signature_spec.rb b/spec/models/gpg_signature_spec.rb index cdd7dea2064..e90319c39b1 100644 --- a/spec/models/gpg_signature_spec.rb +++ b/spec/models/gpg_signature_spec.rb @@ -23,6 +23,41 @@ RSpec.describe GpgSignature do it { is_expected.to validate_presence_of(:gpg_key_primary_keyid) } end + describe '.safe_create!' do + let(:attributes) do + { + commit_sha: commit_sha, + project: project, + gpg_key_primary_keyid: gpg_key.keyid + } + end + + it 'finds a signature by commit sha if it existed' do + gpg_signature + + expect(described_class.safe_create!(commit_sha: commit_sha)).to eq(gpg_signature) + end + + it 'creates a new signature if it was not found' do + expect { described_class.safe_create!(attributes) }.to change { described_class.count }.by(1) + end + + it 'assigns the correct attributes when creating' do + signature = described_class.safe_create!(attributes) + + expect(signature.project).to eq(project) + expect(signature.commit_sha).to eq(commit_sha) + expect(signature.gpg_key_primary_keyid).to eq(gpg_key.keyid) + end + + it 'does not raise an error in case of a race condition' do + expect(described_class).to receive(:find_or_create_by).and_raise(ActiveRecord::RecordNotUnique) + allow(described_class).to receive(:find_or_create_by).and_call_original + + described_class.safe_create!(attributes) + end + end + describe '#commit' do it 'fetches the commit through the project' do expect_any_instance_of(Project).to receive(:commit).with(commit_sha).and_return(commit) diff --git a/spec/models/merge_request_diff_spec.rb b/spec/models/merge_request_diff_spec.rb index 33e984dc399..1849d3bac12 100644 --- a/spec/models/merge_request_diff_spec.rb +++ b/spec/models/merge_request_diff_spec.rb @@ -46,7 +46,7 @@ describe MergeRequestDiff do it { expect(first_diff.reload).not_to be_latest } end - describe '#diffs' do + shared_examples_for 'merge request diffs' do let(:merge_request) { create(:merge_request, :with_diffs) } let!(:diff) { merge_request.merge_request_diff.reload } @@ -91,98 +91,110 @@ describe MergeRequestDiff do diff.diffs.diff_files end end - end - describe '#raw_diffs' do - context 'when the :ignore_whitespace_change option is set' do - it 'creates a new compare object instead of loading from the DB' do - expect(diff_with_commits).not_to receive(:load_diffs) - expect(diff_with_commits.compare).to receive(:diffs).and_call_original + describe '#raw_diffs' do + context 'when the :ignore_whitespace_change option is set' do + it 'creates a new compare object instead of using preprocessed data' do + expect(diff_with_commits).not_to receive(:load_diffs) + expect(diff_with_commits.compare).to receive(:diffs).and_call_original - diff_with_commits.raw_diffs(ignore_whitespace_change: true) + diff_with_commits.raw_diffs(ignore_whitespace_change: true) + end end - end - context 'when the raw diffs are empty' do - before do - MergeRequestDiffFile.where(merge_request_diff_id: diff_with_commits.id).delete_all - end + context 'when the raw diffs are empty' do + before do + MergeRequestDiffFile.where(merge_request_diff_id: diff_with_commits.id).delete_all + end - it 'returns an empty DiffCollection' do - expect(diff_with_commits.raw_diffs).to be_a(Gitlab::Git::DiffCollection) - expect(diff_with_commits.raw_diffs).to be_empty + it 'returns an empty DiffCollection' do + expect(diff_with_commits.raw_diffs).to be_a(Gitlab::Git::DiffCollection) + expect(diff_with_commits.raw_diffs).to be_empty + end end - end - context 'when the raw diffs exist' do - it 'returns the diffs' do - expect(diff_with_commits.raw_diffs).to be_a(Gitlab::Git::DiffCollection) - expect(diff_with_commits.raw_diffs).not_to be_empty - end + context 'when the raw diffs exist' do + it 'returns the diffs' do + expect(diff_with_commits.raw_diffs).to be_a(Gitlab::Git::DiffCollection) + expect(diff_with_commits.raw_diffs).not_to be_empty + end - context 'when the :paths option is set' do - let(:diffs) { diff_with_commits.raw_diffs(paths: ['files/ruby/popen.rb', 'files/ruby/popen.rb']) } + context 'when the :paths option is set' do + let(:diffs) { diff_with_commits.raw_diffs(paths: ['files/ruby/popen.rb', 'files/ruby/popen.rb']) } - it 'only returns diffs that match the (old path, new path) given' do - expect(diffs.map(&:new_path)).to contain_exactly('files/ruby/popen.rb') - end + it 'only returns diffs that match the (old path, new path) given' do + expect(diffs.map(&:new_path)).to contain_exactly('files/ruby/popen.rb') + end - it 'only serializes diff files found by query' do - expect(diff_with_commits.merge_request_diff_files.count).to be > 10 - expect_any_instance_of(MergeRequestDiffFile).to receive(:to_hash).once + it 'only serializes diff files found by query' do + expect(diff_with_commits.merge_request_diff_files.count).to be > 10 + expect_any_instance_of(MergeRequestDiffFile).to receive(:to_hash).once - diffs - end + diffs + end - it 'uses the diffs from the DB' do - expect(diff_with_commits).to receive(:load_diffs) + it 'uses the preprocessed diffs' do + expect(diff_with_commits).to receive(:load_diffs) - diffs + diffs + end end end end - end - describe '#save_diffs' do - it 'saves collected state' do - mr_diff = create(:merge_request).merge_request_diff + describe '#save_diffs' do + it 'saves collected state' do + mr_diff = create(:merge_request).merge_request_diff - expect(mr_diff.collected?).to be_truthy - end + expect(mr_diff.collected?).to be_truthy + end - it 'saves overflow state' do - allow(Commit).to receive(:max_diff_options) - .and_return(max_lines: 0, max_files: 0) + it 'saves overflow state' do + allow(Commit).to receive(:max_diff_options) + .and_return(max_lines: 0, max_files: 0) - mr_diff = create(:merge_request).merge_request_diff + mr_diff = create(:merge_request).merge_request_diff - expect(mr_diff.overflow?).to be_truthy - end + expect(mr_diff.overflow?).to be_truthy + end - it 'saves empty state' do - allow_any_instance_of(described_class).to receive_message_chain(:compare, :commits) - .and_return([]) + it 'saves empty state' do + allow_any_instance_of(described_class).to receive_message_chain(:compare, :commits) + .and_return([]) - mr_diff = create(:merge_request).merge_request_diff + mr_diff = create(:merge_request).merge_request_diff - expect(mr_diff.empty?).to be_truthy - end + expect(mr_diff.empty?).to be_truthy + end - it 'expands collapsed diffs before saving' do - mr_diff = create(:merge_request, source_branch: 'expand-collapse-lines', target_branch: 'master').merge_request_diff - diff_file = mr_diff.merge_request_diff_files.find_by(new_path: 'expand-collapse/file-5.txt') + it 'expands collapsed diffs before saving' do + mr_diff = create(:merge_request, source_branch: 'expand-collapse-lines', target_branch: 'master').merge_request_diff + diff_file = mr_diff.merge_request_diff_files.find_by(new_path: 'expand-collapse/file-5.txt') - expect(diff_file.diff).not_to be_empty + expect(diff_file.diff).not_to be_empty + end + + it 'saves binary diffs correctly' do + path = 'files/images/icn-time-tracking.pdf' + mr_diff = create(:merge_request, source_branch: 'add-pdf-text-binary', target_branch: 'master').merge_request_diff + diff_file = mr_diff.merge_request_diff_files.find_by(new_path: path) + + expect(diff_file).to be_binary + expect(diff_file.diff).to eq(mr_diff.compare.diffs(paths: [path]).to_a.first.diff) + end end + end - it 'saves binary diffs correctly' do - path = 'files/images/icn-time-tracking.pdf' - mr_diff = create(:merge_request, source_branch: 'add-pdf-text-binary', target_branch: 'master').merge_request_diff - diff_file = mr_diff.merge_request_diff_files.find_by(new_path: path) + describe 'internal diffs configured' do + include_examples 'merge request diffs' + end - expect(diff_file).to be_binary - expect(diff_file.diff).to eq(mr_diff.compare.diffs(paths: [path]).to_a.first.diff) + describe 'external diffs configured' do + before do + stub_external_diffs_setting(enabled: true) end + + include_examples 'merge request diffs' end describe '#commit_shas' do @@ -245,4 +257,55 @@ describe MergeRequestDiff do expect(subject.modified_paths).to eq(%w{foo bar baz}) end end + + describe '#opening_external_diff' do + subject(:diff) { diff_with_commits } + + context 'external diffs disabled' do + it { expect(diff.external_diff).not_to be_exists } + + it 'yields nil' do + expect { |b| diff.opening_external_diff(&b) }.to yield_with_args(nil) + end + end + + context 'external diffs enabled' do + let(:test_dir) { 'tmp/tests/external-diffs' } + + around do |example| + FileUtils.mkdir_p(test_dir) + + begin + example.run + ensure + FileUtils.rm_rf(test_dir) + end + end + + before do + stub_external_diffs_setting(enabled: true, storage_path: test_dir) + end + + it { expect(diff.external_diff).to be_exists } + + it 'yields an open file' do + expect { |b| diff.opening_external_diff(&b) }.to yield_with_args(File) + end + + it 'is re-entrant' do + outer_file_a = + diff.opening_external_diff do |outer_file| + diff.opening_external_diff do |inner_file| + expect(outer_file).to eq(inner_file) + end + + outer_file + end + + diff.opening_external_diff do |outer_file_b| + expect(outer_file_a).not_to eq(outer_file_b) + end + end + end + end end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index b62f973ad1e..afa87b8a62d 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -82,6 +82,38 @@ describe MergeRequest do end end + describe '#default_squash_commit_message' do + let(:project) { subject.project } + + def commit_collection(commit_hashes) + raw_commits = commit_hashes.map { |raw| Commit.from_hash(raw, project) } + + CommitCollection.new(project, raw_commits) + end + + it 'returns the oldest multiline commit message' do + commits = commit_collection([ + { message: 'Singleline', parent_ids: [] }, + { message: "Second multiline\nCommit message", parent_ids: [] }, + { message: "First multiline\nCommit message", parent_ids: [] } + ]) + + expect(subject).to receive(:commits).and_return(commits) + + expect(subject.default_squash_commit_message).to eq("First multiline\nCommit message") + end + + it 'returns the merge request title if there are no multiline commits' do + commits = commit_collection([ + { message: 'Singleline', parent_ids: [] } + ]) + + expect(subject).to receive(:commits).and_return(commits) + + expect(subject.default_squash_commit_message).to eq(subject.title) + end + end + describe 'modules' do subject { described_class } @@ -920,18 +952,18 @@ describe MergeRequest do end end - describe '#merge_commit_message' do + describe '#default_merge_commit_message' do it 'includes merge information as the title' do request = build(:merge_request, source_branch: 'source', target_branch: 'target') - expect(request.merge_commit_message) + expect(request.default_merge_commit_message) .to match("Merge branch 'source' into 'target'\n\n") end it 'includes its title in the body' do request = build(:merge_request, title: 'Remove all technical debt') - expect(request.merge_commit_message) + expect(request.default_merge_commit_message) .to match("Remove all technical debt\n\n") end @@ -943,34 +975,34 @@ describe MergeRequest do allow(subject.project).to receive(:default_branch).and_return(subject.target_branch) subject.cache_merge_request_closes_issues! - expect(subject.merge_commit_message) + expect(subject.default_merge_commit_message) .to match("Closes #{issue.to_reference}") end it 'includes its reference in the body' do request = build_stubbed(:merge_request) - expect(request.merge_commit_message) + expect(request.default_merge_commit_message) .to match("See merge request #{request.to_reference(full: true)}") end it 'excludes multiple linebreak runs when description is blank' do request = build(:merge_request, title: 'Title', description: nil) - expect(request.merge_commit_message).not_to match("Title\n\n\n\n") + expect(request.default_merge_commit_message).not_to match("Title\n\n\n\n") end it 'includes its description in the body' do request = build(:merge_request, description: 'By removing all code') - expect(request.merge_commit_message(include_description: true)) + expect(request.default_merge_commit_message(include_description: true)) .to match("By removing all code\n\n") end it 'does not includes its description in the body' do request = build(:merge_request, description: 'By removing all code') - expect(request.merge_commit_message) + expect(request.default_merge_commit_message) .not_to match("By removing all code\n\n") end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index ae137aa7b78..c1767ed0535 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -1765,7 +1765,7 @@ describe Project do context 'using a regular repository' do it 'creates the repository' do expect(shell).to receive(:create_repository) - .with(project.repository_storage, project.disk_path) + .with(project.repository_storage, project.disk_path, project.full_path) .and_return(true) expect(project.repository).to receive(:after_create) @@ -1775,7 +1775,7 @@ describe Project do it 'adds an error if the repository could not be created' do expect(shell).to receive(:create_repository) - .with(project.repository_storage, project.disk_path) + .with(project.repository_storage, project.disk_path, project.full_path) .and_return(false) expect(project.repository).not_to receive(:after_create) @@ -1808,7 +1808,7 @@ describe Project do .and_return(false) allow(shell).to receive(:create_repository) - .with(project.repository_storage, project.disk_path) + .with(project.repository_storage, project.disk_path, project.full_path) .and_return(true) expect(project).to receive(:create_repository).with(force: true) @@ -1832,7 +1832,7 @@ describe Project do .and_return(false) expect(shell).to receive(:create_repository) - .with(project.repository_storage, project.disk_path) + .with(project.repository_storage, project.disk_path, project.full_path) .and_return(true) project.ensure_repository diff --git a/spec/models/project_wiki_spec.rb b/spec/models/project_wiki_spec.rb index 48a43801b9f..3ccc706edf2 100644 --- a/spec/models/project_wiki_spec.rb +++ b/spec/models/project_wiki_spec.rb @@ -7,7 +7,7 @@ describe ProjectWiki do let(:repository) { project.repository } let(:gitlab_shell) { Gitlab::Shell.new } let(:project_wiki) { described_class.new(project, user) } - let(:raw_repository) { Gitlab::Git::Repository.new(project.repository_storage, subject.disk_path + '.git', 'foo') } + let(:raw_repository) { Gitlab::Git::Repository.new(project.repository_storage, subject.disk_path + '.git', 'foo', 'group/project.wiki') } let(:commit) { project_wiki.repository.head_commit } subject { project_wiki } @@ -75,7 +75,7 @@ describe ProjectWiki do # Create a fresh project which will not have a wiki project_wiki = described_class.new(create(:project), user) gitlab_shell = double(:gitlab_shell) - allow(gitlab_shell).to receive(:create_repository) + allow(gitlab_shell).to receive(:create_wiki_repository) allow(project_wiki).to receive(:gitlab_shell).and_return(gitlab_shell) expect { project_wiki.send(:wiki) }.to raise_exception(ProjectWiki::CouldNotCreateWikiError) diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 4978c43c9b5..f78760bf567 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -2291,6 +2291,7 @@ describe Repository do expect(subject).to be_a(Gitlab::Git::Repository) expect(subject.relative_path).to eq(project.disk_path + '.git') expect(subject.gl_repository).to eq("project-#{project.id}") + expect(subject.gl_project_path).to eq(project.full_path) end context 'with a wiki repository' do @@ -2300,6 +2301,7 @@ describe Repository do expect(subject).to be_a(Gitlab::Git::Repository) expect(subject.relative_path).to eq(project.disk_path + '.wiki.git') expect(subject.gl_repository).to eq("wiki-#{project.id}") + expect(subject.gl_project_path).to eq(project.full_path) end end end diff --git a/spec/models/resource_label_event_spec.rb b/spec/models/resource_label_event_spec.rb index 3797960ac3d..7eeb2fae57d 100644 --- a/spec/models/resource_label_event_spec.rb +++ b/spec/models/resource_label_event_spec.rb @@ -81,14 +81,14 @@ RSpec.describe ResourceLabelEvent, type: :model do expect(subject.outdated_markdown?).to be true end - it 'returns true markdown is outdated' do - subject.attributes = { cached_markdown_version: 0 } + it 'returns true if markdown is outdated' do + subject.attributes = { cached_markdown_version: ((CacheMarkdownField::CACHE_COMMONMARK_VERSION - 1) << 16) | 0 } expect(subject.outdated_markdown?).to be true end it 'returns false if label and reference are set' do - subject.attributes = { reference: 'whatever', cached_markdown_version: CacheMarkdownField::CACHE_COMMONMARK_VERSION } + subject.attributes = { reference: 'whatever', cached_markdown_version: CacheMarkdownField::CACHE_COMMONMARK_VERSION << 16 } expect(subject.outdated_markdown?).to be false end diff --git a/spec/models/sent_notification_spec.rb b/spec/models/sent_notification_spec.rb index 677613b7980..6c35ed8f649 100644 --- a/spec/models/sent_notification_spec.rb +++ b/spec/models/sent_notification_spec.rb @@ -36,19 +36,41 @@ describe SentNotification do end end + shared_examples 'a successful sent notification' do + it 'creates a new SentNotification' do + expect { subject }.to change { described_class.count }.by(1) + end + end + describe '.record' do let(:issue) { create(:issue) } - it 'creates a new SentNotification' do - expect { described_class.record(issue, user.id) }.to change { described_class.count }.by(1) - end + subject { described_class.record(issue, user.id) } + + it_behaves_like 'a successful sent notification' end describe '.record_note' do - let(:note) { create(:diff_note_on_merge_request) } + subject { described_class.record_note(note, note.author.id) } - it 'creates a new SentNotification' do - expect { described_class.record_note(note, note.author.id) }.to change { described_class.count }.by(1) + context 'for a discussion note' do + let(:note) { create(:diff_note_on_merge_request) } + + it_behaves_like 'a successful sent notification' + + it 'sets in_reply_to_discussion_id' do + expect(subject.in_reply_to_discussion_id).to eq(note.discussion_id) + end + end + + context 'for an individual note' do + let(:note) { create(:note_on_merge_request) } + + it_behaves_like 'a successful sent notification' + + it 'does not set in_reply_to_discussion_id' do + expect(subject.in_reply_to_discussion_id).to be_nil + end end end diff --git a/spec/models/ssh_host_key_spec.rb b/spec/models/ssh_host_key_spec.rb index 23a94334172..4c677569561 100644 --- a/spec/models/ssh_host_key_spec.rb +++ b/spec/models/ssh_host_key_spec.rb @@ -56,6 +56,29 @@ describe SshHostKey do end end + describe '.find_by' do + let(:project) { create(:project) } + let(:url) { 'ssh://invalid.invalid:2222' } + + let(:finding_id) { [project.id, url].join(':') } + + it 'accepts a string key' do + result = described_class.find_by('id' => finding_id) + + expect(result).to be_a(described_class) + expect(result.project).to eq(project) + expect(result.url.to_s).to eq(url) + end + + it 'accepts a symbol key' do + result = described_class.find_by(id: finding_id) + + expect(result).to be_a(described_class) + expect(result.project).to eq(project) + expect(result.url.to_s).to eq(url) + end + end + describe '#fingerprints', :use_clean_rails_memory_store_caching do it 'returns an array of indexed fingerprints when the cache is filled' do stub_reactive_cache(ssh_host_key, known_hosts: known_hosts) diff --git a/spec/presenters/blob_presenter_spec.rb b/spec/presenters/blob_presenter_spec.rb index e85e7a41017..bb1db9a3d51 100644 --- a/spec/presenters/blob_presenter_spec.rb +++ b/spec/presenters/blob_presenter_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe BlobPresenter, :seed_helper do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '', 'group/project') } let(:git_blob) do Gitlab::Git::Blob.find( diff --git a/spec/requests/api/files_spec.rb b/spec/requests/api/files_spec.rb index 9b32dc78274..1ad536258ba 100644 --- a/spec/requests/api/files_spec.rb +++ b/spec/requests/api/files_spec.rb @@ -191,7 +191,7 @@ describe API::Files do get api(url, current_user), params: params - expect(headers['Content-Disposition']).to eq('inline; filename="popen.rb"') + expect(headers['Content-Disposition']).to eq(%q(inline; filename="popen.rb"; filename*=UTF-8''popen.rb)) end context 'when mandatory params are not given' do diff --git a/spec/requests/api/group_labels_spec.rb b/spec/requests/api/group_labels_spec.rb new file mode 100644 index 00000000000..3769f8b78e4 --- /dev/null +++ b/spec/requests/api/group_labels_spec.rb @@ -0,0 +1,258 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe API::GroupLabels do + let(:user) { create(:user) } + let(:group) { create(:group) } + let!(:group_member) { create(:group_member, group: group, user: user) } + let!(:label1) { create(:group_label, title: 'feature', group: group) } + let!(:label2) { create(:group_label, title: 'bug', group: group) } + + describe 'GET :id/labels' do + it 'returns all available labels for the group' do + get api("/groups/#{group.id}/labels", user) + + expect(response).to have_gitlab_http_status(200) + expect(response).to match_response_schema('public_api/v4/group_labels') + expect(response).to include_pagination_headers + expect(json_response).to be_an Array + expect(json_response.size).to eq(2) + expect(json_response.map {|r| r['name'] }).to contain_exactly('feature', 'bug') + end + end + + describe 'POST /groups/:id/labels' do + it 'returns created label when all params are given' do + post api("/groups/#{group.id}/labels", user), + params: { + name: 'Foo', + color: '#FFAABB', + description: 'test' + } + + expect(response).to have_gitlab_http_status(201) + expect(json_response['name']).to eq('Foo') + expect(json_response['color']).to eq('#FFAABB') + expect(json_response['description']).to eq('test') + end + + it 'returns created label when only required params are given' do + post api("/groups/#{group.id}/labels", user), + params: { + name: 'Foo & Bar', + color: '#FFAABB' + } + + expect(response.status).to eq(201) + expect(json_response['name']).to eq('Foo & Bar') + expect(json_response['color']).to eq('#FFAABB') + expect(json_response['description']).to be_nil + end + + it 'returns a 400 bad request if name not given' do + post api("/groups/#{group.id}/labels", user), params: { color: '#FFAABB' } + + expect(response).to have_gitlab_http_status(400) + end + + it 'returns a 400 bad request if color is not given' do + post api("/groups/#{group.id}/labels", user), params: { name: 'Foobar' } + + expect(response).to have_gitlab_http_status(400) + end + + it 'returns 409 if label already exists' do + post api("/groups/#{group.id}/labels", user), + params: { + name: label1.name, + color: '#FFAABB' + } + + expect(response).to have_gitlab_http_status(409) + expect(json_response['message']).to eq('Label already exists') + end + end + + describe 'DELETE /groups/:id/labels' do + it 'returns 204 for existing label' do + delete api("/groups/#{group.id}/labels", user), params: { name: label1.name } + + expect(response).to have_gitlab_http_status(204) + end + + it 'returns 404 for non existing label' do + delete api("/groups/#{group.id}/labels", user), params: { name: 'label2' } + + expect(response).to have_gitlab_http_status(404) + expect(json_response['message']).to eq('404 Label Not Found') + end + + it 'returns 400 for wrong parameters' do + delete api("/groups/#{group.id}/labels", user) + + expect(response).to have_gitlab_http_status(400) + end + + it "does not delete parent's group labels", :nested_groups do + subgroup = create(:group, parent: group) + subgroup_label = create(:group_label, title: 'feature', group: subgroup) + + delete api("/groups/#{subgroup.id}/labels", user), params: { name: subgroup_label.name } + + expect(response).to have_gitlab_http_status(204) + expect(subgroup.labels.size).to eq(0) + expect(group.labels).to include(label1) + end + + it_behaves_like '412 response' do + let(:request) { api("/groups/#{group.id}/labels", user) } + let(:params) { { name: label1.name } } + end + end + + describe 'PUT /groups/:id/labels' do + it 'returns 200 if name and colors and description are changed' do + put api("/groups/#{group.id}/labels", user), + params: { + name: label1.name, + new_name: 'New Label', + color: '#FFFFFF', + description: 'test' + } + + expect(response).to have_gitlab_http_status(200) + expect(json_response['name']).to eq('New Label') + expect(json_response['color']).to eq('#FFFFFF') + expect(json_response['description']).to eq('test') + end + + it "does not update parent's group label", :nested_groups do + subgroup = create(:group, parent: group) + subgroup_label = create(:group_label, title: 'feature', group: subgroup) + + put api("/groups/#{subgroup.id}/labels", user), + params: { + name: subgroup_label.name, + new_name: 'New Label' + } + + expect(response).to have_gitlab_http_status(200) + expect(subgroup.labels[0].name).to eq('New Label') + expect(label1.name).to eq('feature') + end + + it 'returns 404 if label does not exist' do + put api("/groups/#{group.id}/labels", user), + params: { + name: 'label2', + new_name: 'label3' + } + + expect(response).to have_gitlab_http_status(404) + end + + it 'returns 400 if no label name given' do + put api("/groups/#{group.id}/labels", user), params: { new_name: label1.name } + + expect(response).to have_gitlab_http_status(400) + expect(json_response['error']).to eq('name is missing') + end + + it 'returns 400 if no new parameters given' do + put api("/groups/#{group.id}/labels", user), params: { name: label1.name } + + expect(response).to have_gitlab_http_status(400) + expect(json_response['error']).to eq('new_name, color, description are missing, '\ + 'at least one parameter must be provided') + end + end + + describe 'POST /groups/:id/labels/:label_id/subscribe' do + context 'when label_id is a label title' do + it 'subscribes to the label' do + post api("/groups/#{group.id}/labels/#{label1.title}/subscribe", user) + + expect(response).to have_gitlab_http_status(201) + expect(json_response['name']).to eq(label1.title) + expect(json_response['subscribed']).to be_truthy + end + end + + context 'when label_id is a label ID' do + it 'subscribes to the label' do + post api("/groups/#{group.id}/labels/#{label1.id}/subscribe", user) + + expect(response).to have_gitlab_http_status(201) + expect(json_response['name']).to eq(label1.title) + expect(json_response['subscribed']).to be_truthy + end + end + + context 'when user is already subscribed to label' do + before do + label1.subscribe(user) + end + + it 'returns 304' do + post api("/groups/#{group.id}/labels/#{label1.id}/subscribe", user) + + expect(response).to have_gitlab_http_status(304) + end + end + + context 'when label ID is not found' do + it 'returns 404 error' do + post api("/groups/#{group.id}/labels/1234/subscribe", user) + + expect(response).to have_gitlab_http_status(404) + end + end + end + + describe 'POST /groups/:id/labels/:label_id/unsubscribe' do + before do + label1.subscribe(user) + end + + context 'when label_id is a label title' do + it 'unsubscribes from the label' do + post api("/groups/#{group.id}/labels/#{label1.title}/unsubscribe", user) + + expect(response).to have_gitlab_http_status(201) + expect(json_response['name']).to eq(label1.title) + expect(json_response['subscribed']).to be_falsey + end + end + + context 'when label_id is a label ID' do + it 'unsubscribes from the label' do + post api("/groups/#{group.id}/labels/#{label1.id}/unsubscribe", user) + + expect(response).to have_gitlab_http_status(201) + expect(json_response['name']).to eq(label1.title) + expect(json_response['subscribed']).to be_falsey + end + end + + context 'when user is already unsubscribed from label' do + before do + label1.unsubscribe(user) + end + + it 'returns 304' do + post api("/groups/#{group.id}/labels/#{label1.id}/unsubscribe", user) + + expect(response).to have_gitlab_http_status(304) + end + end + + context 'when label ID is not found' do + it 'returns 404 error' do + post api("/groups/#{group.id}/labels/1234/unsubscribe", user) + + expect(response).to have_gitlab_http_status(404) + end + end + end +end diff --git a/spec/requests/api/issues_spec.rb b/spec/requests/api/issues_spec.rb index e0f1e303e96..04908378a24 100644 --- a/spec/requests/api/issues_spec.rb +++ b/spec/requests/api/issues_spec.rb @@ -208,6 +208,18 @@ describe API::Issues do expect_paginated_array_response(issue.id) end + it 'returns issues matching given search string for title and scoped in title' do + get api("/issues", user), params: { search: issue.title, in: 'title' } + + expect_paginated_array_response(issue.id) + end + + it 'returns an empty array if no issue matches given search string for title and scoped in description' do + get api("/issues", user), params: { search: issue.title, in: 'description' } + + expect_paginated_array_response([]) + end + it 'returns issues matching given search string for description' do get api("/issues", user), params: { search: issue.description } diff --git a/spec/requests/api/jobs_spec.rb b/spec/requests/api/jobs_spec.rb index 97aa71bf231..3defe8bbf51 100644 --- a/spec/requests/api/jobs_spec.rb +++ b/spec/requests/api/jobs_spec.rb @@ -403,7 +403,7 @@ describe API::Jobs do shared_examples 'downloads artifact' do let(:download_headers) do { 'Content-Transfer-Encoding' => 'binary', - 'Content-Disposition' => 'attachment; filename=ci_build_artifacts.zip' } + 'Content-Disposition' => %q(attachment; filename="ci_build_artifacts.zip"; filename*=UTF-8''ci_build_artifacts.zip) } end it 'returns specific job artifacts' do @@ -555,7 +555,7 @@ describe API::Jobs do let(:download_headers) do { 'Content-Transfer-Encoding' => 'binary', 'Content-Disposition' => - "attachment; filename=#{job.artifacts_file.filename}" } + %Q(attachment; filename="#{job.artifacts_file.filename}"; filename*=UTF-8''#{job.artifacts_file.filename}) } end it { expect(response).to have_http_status(:ok) } diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index 4d42bc39ac3..0f5f6e38819 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -260,6 +260,18 @@ describe API::MergeRequests do expect_response_ordered_exactly(merge_request) end + it 'returns merge requests matching given search string for title and scoped in title' do + get api("/merge_requests", user), params: { search: merge_request.title, in: 'title' } + + expect_response_ordered_exactly(merge_request) + end + + it 'returns an empty array if no merge reques matches given search string for description and scoped in title' do + get api("/merge_requests", user), params: { search: merge_request.description, in: 'title' } + + expect_response_contain_exactly + end + it 'returns merge requests for project matching given search string for description' do get api("/merge_requests", user), params: { project_id: project.id, search: merge_request.description } @@ -939,6 +951,29 @@ describe API::MergeRequests do expect(response).to have_gitlab_http_status(404) end + + describe "the squash_commit_message param" do + let(:squash_commit) do + project.repository.commits_between(json_response['diff_refs']['start_sha'], json_response['merge_commit_sha']).first + end + + it "results in a specific squash commit message when set" do + squash_commit_message = 'My custom squash commit message' + + put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user), params: { + squash: true, + squash_commit_message: squash_commit_message + } + + expect(squash_commit.message.chomp).to eq(squash_commit_message) + end + + it "results in a default squash commit message when not set" do + put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user), params: { squash: true } + + expect(squash_commit.message).to eq(merge_request.default_squash_commit_message) + end + end end describe "PUT /projects/:id/merge_requests/:merge_request_iid" do diff --git a/spec/requests/api/releases_spec.rb b/spec/requests/api/releases_spec.rb index 811e23fb854..1f317971a66 100644 --- a/spec/requests/api/releases_spec.rb +++ b/spec/requests/api/releases_spec.rb @@ -127,6 +127,31 @@ describe API::Releases do .to match_array(release.sources.map(&:url)) end + context "when release description contains confidential issue's link" do + let(:confidential_issue) do + create(:issue, + :confidential, + project: project, + title: 'A vulnerability') + end + + let!(:release) do + create(:release, + project: project, + tag: 'v0.1', + sha: commit.id, + author: maintainer, + description: "This is confidential #{confidential_issue.to_reference}") + end + + it "does not expose confidential issue's title" do + get api("/projects/#{project.id}/releases/v0.1", maintainer) + + expect(json_response['description_html']).to include(confidential_issue.to_reference) + expect(json_response['description_html']).not_to include('A vulnerability') + end + end + context 'when release has link asset' do let!(:link) do create(:release_link, diff --git a/spec/requests/api/runner_spec.rb b/spec/requests/api/runner_spec.rb index ed0108c846a..d7ddd97e8c8 100644 --- a/spec/requests/api/runner_spec.rb +++ b/spec/requests/api/runner_spec.rb @@ -1584,7 +1584,7 @@ describe API::Runner, :clean_gitlab_redis_shared_state do context 'when artifacts are stored locally' do let(:download_headers) do { 'Content-Transfer-Encoding' => 'binary', - 'Content-Disposition' => 'attachment; filename=ci_build_artifacts.zip' } + 'Content-Disposition' => %q(attachment; filename="ci_build_artifacts.zip"; filename*=UTF-8''ci_build_artifacts.zip) } end before do diff --git a/spec/requests/api/settings_spec.rb b/spec/requests/api/settings_spec.rb index 45fb1562e84..f33eb5b9e02 100644 --- a/spec/requests/api/settings_spec.rb +++ b/spec/requests/api/settings_spec.rb @@ -64,7 +64,8 @@ describe API::Settings, 'Settings' do performance_bar_allowed_group_path: group.full_path, instance_statistics_visibility_private: true, diff_max_patch_bytes: 150_000, - default_branch_protection: Gitlab::Access::PROTECTION_DEV_CAN_MERGE + default_branch_protection: Gitlab::Access::PROTECTION_DEV_CAN_MERGE, + local_markdown_version: 3 } expect(response).to have_gitlab_http_status(200) @@ -90,6 +91,7 @@ describe API::Settings, 'Settings' do expect(json_response['instance_statistics_visibility_private']).to be(true) expect(json_response['diff_max_patch_bytes']).to eq(150_000) expect(json_response['default_branch_protection']).to eq(Gitlab::Access::PROTECTION_DEV_CAN_MERGE) + expect(json_response['local_markdown_version']).to eq(3) end end diff --git a/spec/requests/openid_connect_spec.rb b/spec/requests/openid_connect_spec.rb index 2b148c1b563..2a455523e2c 100644 --- a/spec/requests/openid_connect_spec.rb +++ b/spec/requests/openid_connect_spec.rb @@ -35,7 +35,7 @@ describe 'OpenID Connect requests' do 'name' => 'Alice', 'nickname' => 'alice', 'email' => 'public@example.com', - 'email_verified' => true, + 'email_verified' => false, 'website' => 'https://example.com', 'profile' => 'http://localhost/alice', 'picture' => "http://localhost/uploads/-/system/user/avatar/#{user.id}/dk.png", @@ -111,6 +111,18 @@ describe 'OpenID Connect requests' do it 'does not include any unknown claims' do expect(json_response.keys).to eq %w[sub sub_legacy] + user_info_claims.keys end + + it 'includes email and email_verified claims' do + expect(json_response.keys).to include('email', 'email_verified') + end + + it 'has public email in email claim' do + expect(json_response['email']).to eq(user.public_email) + end + + it 'has false in email_verified claim' do + expect(json_response['email_verified']).to eq(false) + end end context 'ID token payload' do @@ -175,7 +187,35 @@ describe 'OpenID Connect requests' do expect(response).to have_gitlab_http_status(200) expect(json_response['issuer']).to eq('http://localhost') expect(json_response['jwks_uri']).to eq('http://www.example.com/oauth/discovery/keys') - expect(json_response['scopes_supported']).to eq(%w[api read_user sudo read_repository openid]) + expect(json_response['scopes_supported']).to eq(%w[api read_user sudo read_repository openid profile email]) + end + end + + context 'Application with OpenID and email scopes' do + let(:application) { create :oauth_application, scopes: 'openid email' } + + it 'token response includes an ID token' do + request_access_token! + + expect(json_response).to include 'id_token' + end + + context 'UserInfo payload' do + before do + request_user_info! + end + + it 'includes the email and email_verified claims' do + expect(json_response.keys).to include('email', 'email_verified') + end + + it 'has private email in email claim' do + expect(json_response['email']).to eq(user.email) + end + + it 'has true in email_verified claim' do + expect(json_response['email_verified']).to eq(true) + end end end end diff --git a/spec/requests/user_activity_spec.rb b/spec/requests/user_activity_spec.rb new file mode 100644 index 00000000000..15666e00b9f --- /dev/null +++ b/spec/requests/user_activity_spec.rb @@ -0,0 +1,114 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'Update of user activity' do + let(:user) { create(:user, last_activity_on: nil) } + + before do + group = create(:group, name: 'group') + project = create(:project, :public, namespace: group, name: 'project') + + create(:issue, project: project, iid: 10) + create(:merge_request, source_project: project, iid: 15) + + project.add_maintainer(user) + end + + paths_to_visit = [ + '/group', + '/group/project', + '/groups/group/-/issues', + '/groups/group/-/boards', + '/dashboard/projects', + '/dashboard/snippets', + '/dashboard/groups', + '/dashboard/todos', + '/group/project/issues', + '/group/project/issues/10', + '/group/project/merge_requests', + '/group/project/merge_requests/15' + ] + + context 'without an authenticated user' do + it 'does not set the last activity cookie' do + get "/group/project" + + expect(response.cookies['user_last_activity_on']).to be_nil + end + end + + context 'with an authenticated user' do + before do + login_as(user) + end + + context 'with a POST request' do + it 'does not set the last activity cookie' do + post "/group/project/archive" + + expect(response.cookies['user_last_activity_on']).to be_nil + end + end + + paths_to_visit.each do |path| + context "on GET to #{path}" do + it 'updates the last activity date' do + expect(Users::ActivityService).to receive(:new).and_call_original + + get path + + expect(user.last_activity_on).to eq(Date.today) + end + + context 'when calling it twice' do + it 'updates last_activity_on just once' do + expect(Users::ActivityService).to receive(:new).once.and_call_original + + 2.times do + get path + end + end + end + + context 'when last_activity_on is nil' do + before do + user.update_attribute(:last_activity_on, nil) + end + + it 'updates the last activity date' do + expect(user.last_activity_on).to be_nil + + get path + + expect(user.last_activity_on).to eq(Date.today) + end + end + + context 'when last_activity_on is stale' do + before do + user.update_attribute(:last_activity_on, 2.days.ago.to_date) + end + + it 'updates the last activity date' do + get path + + expect(user.last_activity_on).to eq(Date.today) + end + end + + context 'when last_activity_on is up to date' do + before do + user.update_attribute(:last_activity_on, Date.today) + end + + it 'does not try to update it' do + expect(Users::ActivityService).not_to receive(:new) + + get path + end + end + end + end + end +end diff --git a/spec/serializers/merge_request_widget_commit_entity_spec.rb b/spec/serializers/merge_request_widget_commit_entity_spec.rb new file mode 100644 index 00000000000..ce83978c49a --- /dev/null +++ b/spec/serializers/merge_request_widget_commit_entity_spec.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe MergeRequestWidgetCommitEntity do + let(:project) { create(:project, :repository) } + let(:commit) { project.commit } + let(:request) { double('request') } + + let(:entity) do + described_class.new(commit, request: request) + end + + context 'as json' do + subject { entity.as_json } + + it { expect(subject[:message]).to eq(commit.safe_message) } + it { expect(subject[:short_id]).to eq(commit.short_id) } + it { expect(subject[:title]).to eq(commit.title) } + end +end diff --git a/spec/serializers/merge_request_widget_entity_spec.rb b/spec/serializers/merge_request_widget_entity_spec.rb index 376698a16df..4dbd79f2fc0 100644 --- a/spec/serializers/merge_request_widget_entity_spec.rb +++ b/spec/serializers/merge_request_widget_entity_spec.rb @@ -188,9 +188,14 @@ describe MergeRequestWidgetEntity do .to eq("/#{resource.project.full_path}/merge_requests/#{resource.iid}.diff") end - it 'has merge_commit_message_with_description' do - expect(subject[:merge_commit_message_with_description]) - .to eq(resource.merge_commit_message(include_description: true)) + it 'has default_merge_commit_message_with_description' do + expect(subject[:default_merge_commit_message_with_description]) + .to eq(resource.default_merge_commit_message(include_description: true)) + end + + it 'has default_squash_commit_message' do + expect(subject[:default_squash_commit_message]) + .to eq(resource.default_squash_commit_message) end describe 'new_blob_path' do @@ -272,4 +277,15 @@ describe MergeRequestWidgetEntity do expect(entity[:rebase_path]).to be_nil end end + + describe 'commits_without_merge_commits' do + it 'should not include merge commits' do + # Mock all but the first 5 commits to be merge commits + resource.commits.each_with_index do |commit, i| + expect(commit).to receive(:merge_commit?).at_least(:once).and_return(i > 4) + end + + expect(subject[:commits_without_merge_commits].size).to eq(5) + end + end end diff --git a/spec/services/error_tracking/list_projects_service_spec.rb b/spec/services/error_tracking/list_projects_service_spec.rb new file mode 100644 index 00000000000..ee9c59e3f65 --- /dev/null +++ b/spec/services/error_tracking/list_projects_service_spec.rb @@ -0,0 +1,149 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe ErrorTracking::ListProjectsService do + set(:user) { create(:user) } + set(:project) { create(:project) } + + let(:sentry_url) { 'https://sentrytest.gitlab.com/api/0/projects/sentry-org/sentry-project' } + let(:token) { 'test-token' } + let(:new_api_host) { 'https://gitlab.com/' } + let(:new_token) { 'new-token' } + let(:params) { ActionController::Parameters.new(api_host: new_api_host, token: new_token) } + + let(:error_tracking_setting) do + create(:project_error_tracking_setting, api_url: sentry_url, token: token, project: project) + end + + subject { described_class.new(project, user, params) } + + before do + project.add_reporter(user) + end + + describe '#execute' do + let(:result) { subject.execute } + + context 'with authorized user' do + before do + expect(project).to receive(:error_tracking_setting).at_least(:once) + .and_return(error_tracking_setting) + end + + context 'set model attributes to new values' do + let(:new_api_url) { new_api_host + 'api/0/projects/' } + + before do + expect(error_tracking_setting).to receive(:list_sentry_projects) + .and_return({ projects: [] }) + end + + it 'uses new api_url and token' do + subject.execute + + expect(error_tracking_setting.api_url).to eq(new_api_url) + expect(error_tracking_setting.token).to eq(new_token) + error_tracking_setting.reload + expect(error_tracking_setting.api_url).to eq(sentry_url) + expect(error_tracking_setting.token).to eq(token) + end + end + + context 'sentry client raises exception' do + before do + expect(error_tracking_setting).to receive(:list_sentry_projects) + .and_raise(Sentry::Client::Error, 'Sentry response error: 500') + end + + it 'returns error response' do + expect(result[:message]).to eq('Sentry response error: 500') + expect(result[:http_status]).to eq(:bad_request) + end + end + + context 'with invalid url' do + let(:params) do + ActionController::Parameters.new( + api_host: 'https://localhost', + token: new_token + ) + end + + before do + error_tracking_setting.enabled = false + end + + it 'returns error' do + expect(result[:message]).to start_with('Api url is blocked') + expect(error_tracking_setting).not_to be_valid + end + end + + context 'when list_sentry_projects returns projects' do + let(:projects) { [:list, :of, :projects] } + + before do + expect(error_tracking_setting) + .to receive(:list_sentry_projects).and_return(projects: projects) + end + + it 'returns the projects' do + expect(result).to eq(status: :success, projects: projects) + end + end + end + + context 'with unauthorized user' do + before do + project.add_guest(user) + end + + it 'returns error' do + expect(result).to include(status: :error, message: 'access denied') + end + end + + context 'with error tracking disabled' do + before do + expect(project).to receive(:error_tracking_setting).at_least(:once) + .and_return(error_tracking_setting) + expect(error_tracking_setting) + .to receive(:list_sentry_projects).and_return(projects: []) + + error_tracking_setting.enabled = false + end + + it 'ignores enabled flag' do + expect(result).to include(status: :success, projects: []) + end + end + + context 'error_tracking_setting is nil' do + let(:error_tracking_setting) { build(:project_error_tracking_setting) } + let(:new_api_url) { new_api_host + 'api/0/projects/' } + + before do + expect(project).to receive(:build_error_tracking_setting).once + .and_return(error_tracking_setting) + + expect(error_tracking_setting).to receive(:list_sentry_projects) + .and_return(projects: [:project1, :project2]) + end + + it 'builds a new error_tracking_setting' do + expect(project.error_tracking_setting).to be_nil + + expect(result[:projects]).to eq([:project1, :project2]) + + expect(error_tracking_setting.api_url).to eq(new_api_url) + expect(error_tracking_setting.token).to eq(new_token) + expect(error_tracking_setting.enabled).to be true + expect(error_tracking_setting.persisted?).to be false + expect(error_tracking_setting.project_id).not_to be_nil + + expect(project.error_tracking_setting).to be_nil + end + end + end +end diff --git a/spec/services/merge_requests/create_service_spec.rb b/spec/services/merge_requests/create_service_spec.rb index 4e64b0c9414..b46aa65818d 100644 --- a/spec/services/merge_requests/create_service_spec.rb +++ b/spec/services/merge_requests/create_service_spec.rb @@ -197,6 +197,24 @@ describe MergeRequests::CreateService do expect(merge_request.actual_head_pipeline).to be_merge_request end + context 'when there are no commits between source branch and target branch' do + let(:opts) do + { + title: 'Awesome merge_request', + description: 'please fix', + source_branch: 'not-merged-branch', + target_branch: 'master' + } + end + + it 'does not create a merge request pipeline' do + expect(merge_request).to be_persisted + + merge_request.reload + expect(merge_request.merge_request_pipelines.count).to eq(0) + end + end + context "when branch pipeline was created before a merge request pipline has been created" do before do create(:ci_pipeline, project: merge_request.source_project, diff --git a/spec/services/merge_requests/merge_service_spec.rb b/spec/services/merge_requests/merge_service_spec.rb index 5d96b5ce27c..04a62aa454d 100644 --- a/spec/services/merge_requests/merge_service_spec.rb +++ b/spec/services/merge_requests/merge_service_spec.rb @@ -258,7 +258,7 @@ describe MergeRequests::MergeService do it 'logs and saves error if there is an error when squashing' do error_message = 'Failed to squash. Should be done manually' - allow_any_instance_of(MergeRequests::SquashService).to receive(:squash).and_return(nil) + allow_any_instance_of(MergeRequests::SquashService).to receive(:squash!).and_return(nil) merge_request.update(squash: true) service.execute(merge_request) diff --git a/spec/services/merge_requests/refresh_service_spec.rb b/spec/services/merge_requests/refresh_service_spec.rb index 1169ed5f9f2..9e9dc5a576c 100644 --- a/spec/services/merge_requests/refresh_service_spec.rb +++ b/spec/services/merge_requests/refresh_service_spec.rb @@ -150,11 +150,15 @@ describe MergeRequests::RefreshService do } end - it 'create merge request pipeline' do + it 'create merge request pipeline with commits' do expect { subject } .to change { @merge_request.merge_request_pipelines.count }.by(1) .and change { @fork_merge_request.merge_request_pipelines.count }.by(1) - .and change { @another_merge_request.merge_request_pipelines.count }.by(1) + .and change { @another_merge_request.merge_request_pipelines.count }.by(0) + + expect(@merge_request.has_commits?).to be_truthy + expect(@fork_merge_request.has_commits?).to be_truthy + expect(@another_merge_request.has_commits?).to be_falsy end context "when branch pipeline was created before a merge request pipline has been created" do diff --git a/spec/services/merge_requests/squash_service_spec.rb b/spec/services/merge_requests/squash_service_spec.rb index 53bce15735c..2713652873e 100644 --- a/spec/services/merge_requests/squash_service_spec.rb +++ b/spec/services/merge_requests/squash_service_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe MergeRequests::SquashService do include GitHelpers - let(:service) { described_class.new(project, user, {}) } + let(:service) { described_class.new(project, user, { merge_request: merge_request }) } let(:user) { project.owner } let(:project) { create(:project, :repository) } let(:repository) { project.repository.raw } @@ -31,32 +31,49 @@ describe MergeRequests::SquashService do shared_examples 'the squash succeeds' do it 'returns the squashed commit SHA' do - result = service.execute(merge_request) + result = service.execute expect(result).to match(status: :success, squash_sha: a_string_matching(/\h{40}/)) expect(result[:squash_sha]).not_to eq(merge_request.diff_head_sha) end it 'cleans up the temporary directory' do - service.execute(merge_request) + service.execute expect(File.exist?(squash_dir_path)).to be(false) end it 'does not keep the branch push event' do - expect { service.execute(merge_request) }.not_to change { Event.count } + expect { service.execute }.not_to change { Event.count } + end + + context 'when there is a single commit in the merge request' do + before do + expect(merge_request).to receive(:commits_count).at_least(:once).and_return(1) + end + + it 'will skip performing the squash, as the outcome would be the same' do + expect(merge_request.target_project.repository).not_to receive(:squash) + + service.execute + end + + it 'will still perform the squash when a custom squash commit message has been provided' do + service = described_class.new(project, user, { merge_request: merge_request, squash_commit_message: 'A custom commit message' }) + + expect(merge_request.target_project.repository).to receive(:squash).and_return('sha') + + service.execute + end end context 'the squashed commit' do - let(:squash_sha) { service.execute(merge_request)[:squash_sha] } + let(:squash_sha) { service.execute[:squash_sha] } let(:squash_commit) { project.repository.commit(squash_sha) } - it 'copies the author info and message from the merge request' do + it 'copies the author info from the merge request' do expect(squash_commit.author_name).to eq(merge_request.author.name) expect(squash_commit.author_email).to eq(merge_request.author.email) - - # Commit messages have a trailing newline, but titles don't. - expect(squash_commit.message.chomp).to eq(merge_request.title) end it 'sets the current user as the committer' do @@ -72,21 +89,37 @@ describe MergeRequests::SquashService do expect(squash_diff.patch.length).to eq(mr_diff.patch.length) expect(squash_commit.sha).not_to eq(merge_request.diff_head_sha) end + + it 'has a default squash commit message if no message was provided' do + expect(squash_commit.message.chomp).to eq(merge_request.default_squash_commit_message.chomp) + end + + context 'if a message was provided' do + let(:service) { described_class.new(project, user, { merge_request: merge_request, squash_commit_message: message }) } + let(:message) { 'My custom message' } + let(:squash_sha) { service.execute[:squash_sha] } + + it 'has the same message as the message provided' do + expect(squash_commit.message.chomp).to eq(message) + end + end end end describe '#execute' do context 'when there is only one commit in the merge request' do + let(:merge_request) { merge_request_with_one_commit } + it 'returns that commit SHA' do - result = service.execute(merge_request_with_one_commit) + result = service.execute - expect(result).to match(status: :success, squash_sha: merge_request_with_one_commit.diff_head_sha) + expect(result).to match(status: :success, squash_sha: merge_request.diff_head_sha) end it 'does not perform any git actions' do expect(repository).not_to receive(:popen) - service.execute(merge_request_with_one_commit) + service.execute end end @@ -116,12 +149,11 @@ describe MergeRequests::SquashService do expect(service).to receive(:log_error).with(log_error) expect(service).to receive(:log_error).with(error) - service.execute(merge_request) + service.execute end it 'returns an error' do - expect(service.execute(merge_request)).to match(status: :error, - message: a_string_including('squash')) + expect(service.execute).to match(status: :error, message: a_string_including('squash')) end end end @@ -131,23 +163,22 @@ describe MergeRequests::SquashService do let(:error) { 'A test error' } before do - allow(merge_request).to receive(:commits_count).and_raise(error) + allow(merge_request.target_project.repository).to receive(:squash).and_raise(error) end it 'logs the MR reference and exception' do expect(service).to receive(:log_error).with(a_string_including("#{project.full_path}#{merge_request.to_reference}")) expect(service).to receive(:log_error).with(error) - service.execute(merge_request) + service.execute end it 'returns an error' do - expect(service.execute(merge_request)).to match(status: :error, - message: a_string_including('squash')) + expect(service.execute).to match(status: :error, message: a_string_including('squash')) end it 'cleans up the temporary directory' do - service.execute(merge_request) + service.execute expect(File.exist?(squash_dir_path)).to be(false) end diff --git a/spec/services/notes/build_service_spec.rb b/spec/services/notes/build_service_spec.rb index 9aaccb4bffe..af4daff336b 100644 --- a/spec/services/notes/build_service_spec.rb +++ b/spec/services/notes/build_service_spec.rb @@ -123,6 +123,46 @@ describe Notes::BuildService do end end + context 'when replying to individual note' do + let(:note) { create(:note_on_issue) } + + subject { described_class.new(project, author, note: 'Test', in_reply_to_discussion_id: note.discussion_id).execute } + + shared_examples 'an individual note reply' do + it 'builds another individual note' do + expect(subject).to be_valid + expect(subject).to be_a(Note) + expect(subject.discussion_id).not_to eq(note.discussion_id) + end + end + + context 'when reply_to_individual_notes is disabled' do + before do + stub_feature_flags(reply_to_individual_notes: false) + end + + it_behaves_like 'an individual note reply' + end + + context 'when reply_to_individual_notes is enabled' do + before do + stub_feature_flags(reply_to_individual_notes: true) + end + + it 'sets the note up to be in reply to that note' do + expect(subject).to be_valid + expect(subject).to be_a(DiscussionNote) + expect(subject.discussion_id).to eq(note.discussion_id) + end + + context 'when noteable does not support replies' do + let(:note) { create(:note_on_commit) } + + it_behaves_like 'an individual note reply' + end + end + end + it 'builds a note without saving it' do new_note = described_class.new(project, author, diff --git a/spec/services/notes/create_service_spec.rb b/spec/services/notes/create_service_spec.rb index 1b9ba42cfd6..48f1d696ff6 100644 --- a/spec/services/notes/create_service_spec.rb +++ b/spec/services/notes/create_service_spec.rb @@ -278,5 +278,42 @@ describe Notes::CreateService do expect(note.note).to eq(':smile:') end end + + context 'reply to individual note' do + let(:existing_note) { create(:note_on_issue, noteable: issue, project: project) } + let(:reply_opts) { opts.merge(in_reply_to_discussion_id: existing_note.discussion_id) } + + subject { described_class.new(project, user, reply_opts).execute } + + context 'when reply_to_individual_notes is disabled' do + before do + stub_feature_flags(reply_to_individual_notes: false) + end + + it 'creates an individual note' do + expect(subject.type).to eq(nil) + expect(subject.discussion_id).not_to eq(existing_note.discussion_id) + end + + it 'does not convert existing note' do + expect { subject }.not_to change { existing_note.reload.type } + end + end + + context 'when reply_to_individual_notes is enabled' do + before do + stub_feature_flags(reply_to_individual_notes: true) + end + + it 'creates a DiscussionNote in reply to existing note' do + expect(subject).to be_a(DiscussionNote) + expect(subject.discussion_id).to eq(existing_note.discussion_id) + end + + it 'converts existing note to DiscussionNote' do + expect { subject }.to change { existing_note.reload.type }.from(nil).to('DiscussionNote') + end + end + end end end diff --git a/spec/services/preview_markdown_service_spec.rb b/spec/services/preview_markdown_service_spec.rb index 458cb8f1f31..85515d548a7 100644 --- a/spec/services/preview_markdown_service_spec.rb +++ b/spec/services/preview_markdown_service_spec.rb @@ -114,23 +114,4 @@ describe PreviewMarkdownService do expect(result[:commands]).to eq 'Tags this commit to v1.2.3 with "Stable release".' end end - - it 'sets correct markdown engine' do - service = described_class.new(project, user, { markdown_version: CacheMarkdownField::CACHE_REDCARPET_VERSION }) - result = service.execute - - expect(result[:markdown_engine]).to eq :redcarpet - - service = described_class.new(project, user, { markdown_version: CacheMarkdownField::CACHE_COMMONMARK_VERSION }) - result = service.execute - - expect(result[:markdown_engine]).to eq :common_mark - end - - it 'honors the legacy_render parameter' do - service = described_class.new(project, user, { legacy_render: '1' }) - result = service.execute - - expect(result[:markdown_engine]).to eq :redcarpet - end end diff --git a/spec/services/projects/create_service_spec.rb b/spec/services/projects/create_service_spec.rb index 54ce33dd103..d1b110b9806 100644 --- a/spec/services/projects/create_service_spec.rb +++ b/spec/services/projects/create_service_spec.rb @@ -116,7 +116,7 @@ describe Projects::CreateService, '#execute' do def wiki_repo(project) relative_path = ProjectWiki.new(project).disk_path + '.git' - Gitlab::Git::Repository.new(project.repository_storage, relative_path, 'foobar') + Gitlab::Git::Repository.new(project.repository_storage, relative_path, 'foobar', project.full_path) end end @@ -198,7 +198,7 @@ describe Projects::CreateService, '#execute' do context 'with legacy storage' do before do - gitlab_shell.create_repository(repository_storage, "#{user.namespace.full_path}/existing") + gitlab_shell.create_repository(repository_storage, "#{user.namespace.full_path}/existing", 'group/project') end after do @@ -234,7 +234,7 @@ describe Projects::CreateService, '#execute' do end before do - gitlab_shell.create_repository(repository_storage, hashed_path) + gitlab_shell.create_repository(repository_storage, hashed_path, 'group/project') end after do diff --git a/spec/services/projects/fork_service_spec.rb b/spec/services/projects/fork_service_spec.rb index 26e8d829345..23ec29cce7b 100644 --- a/spec/services/projects/fork_service_spec.rb +++ b/spec/services/projects/fork_service_spec.rb @@ -119,7 +119,7 @@ describe Projects::ForkService do let(:repository_storage_path) { Gitlab.config.repositories.storages[repository_storage].legacy_disk_path } before do - gitlab_shell.create_repository(repository_storage, "#{@to_user.namespace.full_path}/#{@from_project.path}") + gitlab_shell.create_repository(repository_storage, "#{@to_user.namespace.full_path}/#{@from_project.path}", "#{@to_user.namespace.full_path}/#{@from_project.path}") end after do diff --git a/spec/services/projects/transfer_service_spec.rb b/spec/services/projects/transfer_service_spec.rb index 766276fdba3..aae50d5307f 100644 --- a/spec/services/projects/transfer_service_spec.rb +++ b/spec/services/projects/transfer_service_spec.rb @@ -201,7 +201,7 @@ describe Projects::TransferService do before do group.add_owner(user) - unless gitlab_shell.create_repository(repository_storage, "#{group.full_path}/#{project.path}") + unless gitlab_shell.create_repository(repository_storage, "#{group.full_path}/#{project.path}", project.full_path) raise 'failed to add repository' end diff --git a/spec/services/projects/update_pages_configuration_service_spec.rb b/spec/services/projects/update_pages_configuration_service_spec.rb index e4d4e6ff3dd..7f5ef3129d7 100644 --- a/spec/services/projects/update_pages_configuration_service_spec.rb +++ b/spec/services/projects/update_pages_configuration_service_spec.rb @@ -2,23 +2,41 @@ require 'spec_helper' describe Projects::UpdatePagesConfigurationService do let(:project) { create(:project) } - subject { described_class.new(project) } + let(:service) { described_class.new(project) } describe "#update" do let(:file) { Tempfile.new('pages-test') } + subject { service.execute } + after do file.close file.unlink end - it 'updates the .update file' do - # Access this reference to ensure scoping works - Projects::Settings # rubocop:disable Lint/Void - expect(subject).to receive(:pages_config_file).and_return(file.path) - expect(subject).to receive(:reload_daemon).and_call_original + before do + allow(service).to receive(:pages_config_file).and_return(file.path) + end + + context 'when configuration changes' do + it 'updates the .update file' do + expect(service).to receive(:reload_daemon).and_call_original + + expect(subject).to include(status: :success, reload: true) + end + end + + context 'when configuration does not change' do + before do + # we set the configuration + service.execute + end + + it 'does not update the .update file' do + expect(service).not_to receive(:reload_daemon) - expect(subject.execute).to eq({ status: :success }) + expect(subject).to include(status: :success, reload: false) + end end end end diff --git a/spec/services/projects/update_service_spec.rb b/spec/services/projects/update_service_spec.rb index 8adfc63222e..90eaea9c872 100644 --- a/spec/services/projects/update_service_spec.rb +++ b/spec/services/projects/update_service_spec.rb @@ -232,7 +232,7 @@ describe Projects::UpdateService do let(:project) { create(:project, :legacy_storage, :repository, creator: user, namespace: user.namespace) } before do - gitlab_shell.create_repository(repository_storage, "#{user.namespace.full_path}/existing") + gitlab_shell.create_repository(repository_storage, "#{user.namespace.full_path}/existing", user.namespace.full_path) end after do diff --git a/spec/services/task_list_toggle_service_spec.rb b/spec/services/task_list_toggle_service_spec.rb index 750ac4c40ba..cc64dd25085 100644 --- a/spec/services/task_list_toggle_service_spec.rb +++ b/spec/services/task_list_toggle_service_spec.rb @@ -3,7 +3,6 @@ require 'spec_helper' describe TaskListToggleService do - let(:sourcepos) { true } let(:markdown) do <<-EOT.strip_heredoc * [ ] Task 1 @@ -40,87 +39,47 @@ describe TaskListToggleService do EOT end - shared_examples 'task lists' do - it 'checks Task 1' do - toggler = described_class.new(markdown, markdown_html, - index: 1, toggle_as_checked: true, - line_source: '* [ ] Task 1', line_number: 1, - sourcepos: sourcepos) + it 'checks Task 1' do + toggler = described_class.new(markdown, markdown_html, + toggle_as_checked: true, + line_source: '* [ ] Task 1', line_number: 1) - expect(toggler.execute).to be_truthy - expect(toggler.updated_markdown.lines[0]).to eq "* [x] Task 1\n" - expect(toggler.updated_markdown_html).to include('disabled checked> Task 1') - end - - it 'unchecks Item 1' do - toggler = described_class.new(markdown, markdown_html, - index: 3, toggle_as_checked: false, - line_source: '1. [X] Item 1', line_number: 6, - sourcepos: sourcepos) - - expect(toggler.execute).to be_truthy - expect(toggler.updated_markdown.lines[5]).to eq "1. [ ] Item 1\n" - expect(toggler.updated_markdown_html).to include('disabled> Item 1') - end - - it 'returns false if line_source does not match the text' do - toggler = described_class.new(markdown, markdown_html, - index: 2, toggle_as_checked: false, - line_source: '* [x] Task Added', line_number: 2, - sourcepos: sourcepos) - - expect(toggler.execute).to be_falsey - end + expect(toggler.execute).to be_truthy + expect(toggler.updated_markdown.lines[0]).to eq "* [x] Task 1\n" + expect(toggler.updated_markdown_html).to include('disabled checked> Task 1') + end - it 'returns false if markdown is nil' do - toggler = described_class.new(nil, markdown_html, - index: 2, toggle_as_checked: false, - line_source: '* [x] Task Added', line_number: 2, - sourcepos: sourcepos) + it 'unchecks Item 1' do + toggler = described_class.new(markdown, markdown_html, + toggle_as_checked: false, + line_source: '1. [X] Item 1', line_number: 6) - expect(toggler.execute).to be_falsey - end + expect(toggler.execute).to be_truthy + expect(toggler.updated_markdown.lines[5]).to eq "1. [ ] Item 1\n" + expect(toggler.updated_markdown_html).to include('disabled> Item 1') + end - it 'returns false if markdown_html is nil' do - toggler = described_class.new(markdown, nil, - index: 2, toggle_as_checked: false, - line_source: '* [x] Task Added', line_number: 2, - sourcepos: sourcepos) + it 'returns false if line_source does not match the text' do + toggler = described_class.new(markdown, markdown_html, + toggle_as_checked: false, + line_source: '* [x] Task Added', line_number: 2) - expect(toggler.execute).to be_falsey - end + expect(toggler.execute).to be_falsey end - context 'when using sourcepos' do - it_behaves_like 'task lists' + it 'returns false if markdown is nil' do + toggler = described_class.new(nil, markdown_html, + toggle_as_checked: false, + line_source: '* [x] Task Added', line_number: 2) + + expect(toggler.execute).to be_falsey end - context 'when using checkbox indexing' do - let(:sourcepos) { false } - let(:markdown_html) do - <<-EOT.strip_heredoc - <ul class="task-list" dir="auto"> - <li class="task-list-item"> - <input type="checkbox" class="task-list-item-checkbox" disabled> Task 1 - </li> - <li class="task-list-item"> - <input type="checkbox" class="task-list-item-checkbox" disabled checked> Task 2 - </li> - </ul> - <p dir="auto">A paragraph</p> - <ol class="task-list" dir="auto"> - <li class="task-list-item"> - <input type="checkbox" class="task-list-item-checkbox" disabled checked> Item 1 - <ul class="task-list"> - <li class="task-list-item"> - <input type="checkbox" class="task-list-item-checkbox" disabled> Sub-item 1 - </li> - </ul> - </li> - </ol> - EOT - end + it 'returns false if markdown_html is nil' do + toggler = described_class.new(markdown, nil, + toggle_as_checked: false, + line_source: '* [x] Task Added', line_number: 2) - it_behaves_like 'task lists' + expect(toggler.execute).to be_falsey end end diff --git a/spec/support/helpers/features/responsive_table_helpers.rb b/spec/support/helpers/features/responsive_table_helpers.rb new file mode 100644 index 00000000000..7a175219fe9 --- /dev/null +++ b/spec/support/helpers/features/responsive_table_helpers.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true +# These helpers allow you to access rows in a responsive table +# +# Usage: +# describe "..." do +# include Spec::Support::Helpers::Features::ResponsiveTableHelpers +# ... +# +# expect(first_row.text).to include("John Doe") +# expect(second_row.text).to include("John Smith") +# +# Note: +# index starts at 1 as index 0 is expected to be the table header +# +# +module Spec + module Support + module Helpers + module Features + module ResponsiveTableHelpers + def first_row + page.all('.gl-responsive-table-row')[1] + end + + def second_row + page.all('.gl-responsive-table-row')[2] + end + end + end + end + end +end diff --git a/spec/support/helpers/stub_configuration.rb b/spec/support/helpers/stub_configuration.rb index 2851cd9733c..ff21bbe28ca 100644 --- a/spec/support/helpers/stub_configuration.rb +++ b/spec/support/helpers/stub_configuration.rb @@ -56,6 +56,10 @@ module StubConfiguration allow(Gitlab.config.lfs).to receive_messages(to_settings(messages)) end + def stub_external_diffs_setting(messages) + allow(Gitlab.config.external_diffs).to receive_messages(to_settings(messages)) + end + def stub_artifacts_setting(messages) allow(Gitlab.config.artifacts).to receive_messages(to_settings(messages)) end diff --git a/spec/support/helpers/stub_object_storage.rb b/spec/support/helpers/stub_object_storage.rb index 58b5c6a6435..e0c50e533a6 100644 --- a/spec/support/helpers/stub_object_storage.rb +++ b/spec/support/helpers/stub_object_storage.rb @@ -42,6 +42,13 @@ module StubObjectStorage **params) end + def stub_external_diffs_object_storage(uploader = described_class, **params) + stub_object_storage_uploader(config: Gitlab.config.external_diffs.object_store, + uploader: uploader, + remote_directory: 'external_diffs', + **params) + end + def stub_lfs_object_storage(**params) stub_object_storage_uploader(config: Gitlab.config.lfs.object_store, uploader: LfsObjectUploader, diff --git a/spec/support/shared_examples/controllers/repository_lfs_file_load_examples.rb b/spec/support/shared_examples/controllers/repository_lfs_file_load_examples.rb index a3d31e26498..982e0317f7f 100644 --- a/spec/support/shared_examples/controllers/repository_lfs_file_load_examples.rb +++ b/spec/support/shared_examples/controllers/repository_lfs_file_load_examples.rb @@ -28,7 +28,13 @@ shared_examples 'repository lfs file load' do end it 'serves the file' do - expect(controller).to receive(:send_file).with("#{LfsObjectUploader.root}/91/ef/f75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897", filename: filename, disposition: 'attachment') + # Notice the filename= is omitted from the disposition; this is because + # Rails 5 will append this header in send_file + expect(controller).to receive(:send_file) + .with( + "#{LfsObjectUploader.root}/91/ef/f75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897", + filename: filename, + disposition: %Q(attachment; filename*=UTF-8''#{filename})) subject @@ -56,7 +62,7 @@ shared_examples 'repository lfs file load' do file_uri = URI.parse(response.location) params = CGI.parse(file_uri.query) - expect(params["response-content-disposition"].first).to eq "attachment;filename=\"#{filename}\"" + expect(params["response-content-disposition"].first).to eq(%q(attachment; filename="lfs_object.iso"; filename*=UTF-8''lfs_object.iso)) end end end diff --git a/spec/support/shared_examples/models/cluster_application_initial_status.rb b/spec/support/shared_examples/models/cluster_application_initial_status.rb new file mode 100644 index 00000000000..9775d87953c --- /dev/null +++ b/spec/support/shared_examples/models/cluster_application_initial_status.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +shared_examples 'cluster application initial status specs' do + describe '#status' do + let(:cluster) { create(:cluster, :provided_by_gcp) } + + subject { described_class.new(cluster: cluster) } + + context 'when application helm is scheduled' do + before do + create(:clusters_applications_helm, :scheduled, cluster: cluster) + end + + it 'defaults to :not_installable' do + expect(subject.status_name).to be(:not_installable) + end + end + + context 'when application is scheduled' do + before do + create(:clusters_applications_helm, :installed, cluster: cluster) + end + + it 'sets a default status' do + expect(subject.status_name).to be(:installable) + end + end + end +end diff --git a/spec/support/shared_examples/models/cluster_application_status_shared_examples.rb b/spec/support/shared_examples/models/cluster_application_status_shared_examples.rb index c391cc48f4e..554f2e747bc 100644 --- a/spec/support/shared_examples/models/cluster_application_status_shared_examples.rb +++ b/spec/support/shared_examples/models/cluster_application_status_shared_examples.rb @@ -7,26 +7,6 @@ shared_examples 'cluster application status specs' do |application_name| it 'sets a default status' do expect(subject.status_name).to be(:not_installable) end - - context 'when application helm is scheduled' do - before do - create(:clusters_applications_helm, :scheduled, cluster: cluster) - end - - it 'defaults to :not_installable' do - expect(subject.status_name).to be(:not_installable) - end - end - - context 'when application is scheduled' do - before do - create(:clusters_applications_helm, :installed, cluster: cluster) - end - - it 'sets a default status' do - expect(subject.status_name).to be(:installable) - end - end end describe 'status state machine' do @@ -58,6 +38,16 @@ shared_examples 'cluster application status specs' do |application_name| expect(subject.cluster.application_helm.version).to eq(Gitlab::Kubernetes::Helm::HELM_VERSION) end + + it 'sets the correct version of the application' do + subject.update!(version: '0.0.0') + + subject.make_installed! + + subject.reload + + expect(subject.version).to eq(subject.class.const_get(:VERSION)) + end end describe '#make_updated' do @@ -78,6 +68,16 @@ shared_examples 'cluster application status specs' do |application_name| expect(subject.cluster.application_helm.version).to eq(Gitlab::Kubernetes::Helm::HELM_VERSION) end + + it 'updates the version for the application' do + subject.update!(version: '0.0.0') + + subject.make_updated! + + subject.reload + + expect(subject.version).to eq(subject.class.const_get(:VERSION)) + end end describe '#make_errored' do diff --git a/spec/uploaders/external_diff_uploader_spec.rb b/spec/uploaders/external_diff_uploader_spec.rb new file mode 100644 index 00000000000..1c959770dc4 --- /dev/null +++ b/spec/uploaders/external_diff_uploader_spec.rb @@ -0,0 +1,67 @@ +require 'spec_helper' + +describe ExternalDiffUploader do + let(:diff) { create(:merge_request).merge_request_diff } + let(:path) { Gitlab.config.external_diffs.storage_path } + + subject(:uploader) { described_class.new(diff, :external_diff) } + + it_behaves_like "builds correct paths", + store_dir: %r[merge_request_diffs/mr-\d+], + cache_dir: %r[/external-diffs/tmp/cache], + work_dir: %r[/external-diffs/tmp/work] + + context "object store is REMOTE" do + before do + stub_external_diffs_object_storage + end + + include_context 'with storage', described_class::Store::REMOTE + + it_behaves_like "builds correct paths", + store_dir: %r[merge_request_diffs/mr-\d+] + end + + describe 'migration to object storage' do + context 'with object storage disabled' do + it "is skipped" do + expect(ObjectStorage::BackgroundMoveWorker).not_to receive(:perform_async) + + diff + end + end + + context 'with object storage enabled' do + before do + stub_external_diffs_setting(enabled: true) + stub_external_diffs_object_storage(background_upload: true) + end + + it 'is scheduled to run after creation' do + expect(ObjectStorage::BackgroundMoveWorker).to receive(:perform_async).with(described_class.name, 'MergeRequestDiff', :external_diff, kind_of(Numeric)) + + diff + end + end + end + + describe 'remote file' do + context 'with object storage enabled' do + before do + stub_external_diffs_setting(enabled: true) + stub_external_diffs_object_storage + + diff.update!(external_diff_store: described_class::Store::REMOTE) + end + + it 'can store file remotely' do + allow(ObjectStorage::BackgroundMoveWorker).to receive(:perform_async) + + diff + + expect(diff.external_diff_store).to eq(described_class::Store::REMOTE) + expect(diff.external_diff.path).not_to be_blank + end + end + end +end diff --git a/spec/validators/js_regex_validator_spec.rb b/spec/validators/js_regex_validator_spec.rb index aeb55cdc0e5..4d3bafaf267 100644 --- a/spec/validators/js_regex_validator_spec.rb +++ b/spec/validators/js_regex_validator_spec.rb @@ -12,8 +12,6 @@ describe JsRegexValidator do '' | [] '(?#comment)' | ['Regex Pattern (?#comment) can not be expressed in Javascript'] '(?(a)b|c)' | ['invalid conditional pattern: /(?(a)b|c)/i'] - '[a-z&&[^uo]]' | ["Dropped unsupported set intersection '[a-z&&[^uo]]' at index 0", - "Dropped unsupported nested negative set data '[^uo]' at index 6"] end with_them do diff --git a/spec/workers/mail_scheduler/notification_service_worker_spec.rb b/spec/workers/mail_scheduler/notification_service_worker_spec.rb index 1033557ee88..5cfba01850c 100644 --- a/spec/workers/mail_scheduler/notification_service_worker_spec.rb +++ b/spec/workers/mail_scheduler/notification_service_worker_spec.rb @@ -9,6 +9,10 @@ describe MailScheduler::NotificationServiceWorker do ActiveJob::Arguments.serialize(args) end + def deserialize(args) + ActiveJob::Arguments.deserialize(args) + end + describe '#perform' do it 'deserializes arguments from global IDs' do expect(worker.notification_service).to receive(method).with(key) @@ -42,13 +46,48 @@ describe MailScheduler::NotificationServiceWorker do end end - describe '.perform_async' do + describe '.perform_async', :sidekiq do + around do |example| + Sidekiq::Testing.fake! { example.run } + end + it 'serializes arguments as global IDs when scheduling' do - Sidekiq::Testing.fake! do - described_class.perform_async(method, key) + described_class.perform_async(method, key) + + expect(described_class.jobs.count).to eq(1) + expect(described_class.jobs.first).to include('args' => [method, *serialize(key)]) + end + + context 'with ActiveController::Parameters' do + let(:parameters) { ActionController::Parameters.new(hash) } + + let(:hash) do + { + "nested" => { + "hash" => true + } + } + end + + context 'when permitted' do + before do + parameters.permit! + end + + it 'serializes as a serializable Hash' do + described_class.perform_async(method, parameters) - expect(described_class.jobs.count).to eq(1) - expect(described_class.jobs.first).to include('args' => [method, *serialize(key)]) + expect(described_class.jobs.count).to eq(1) + expect(deserialize(described_class.jobs.first['args'])) + .to eq([method, hash]) + end + end + + context 'when not permitted' do + it 'fails to serialize' do + expect { described_class.perform_async(method, parameters) } + .to raise_error(ActionController::UnfilteredParameters) + end end end end diff --git a/spec/workers/repository_fork_worker_spec.rb b/spec/workers/repository_fork_worker_spec.rb index 781f91ac9ca..31bfe88d0bd 100644 --- a/spec/workers/repository_fork_worker_spec.rb +++ b/spec/workers/repository_fork_worker_spec.rb @@ -24,12 +24,7 @@ describe RepositoryForkWorker do end def expect_fork_repository - expect(shell).to receive(:fork_repository).with( - 'default', - project.disk_path, - forked_project.repository_storage, - forked_project.disk_path - ) + expect(shell).to receive(:fork_repository).with(project, forked_project) end describe 'when a worker was reset without cleanup' do |