diff options
author | Matija Čupić <matteeyah@gmail.com> | 2018-06-06 22:29:12 +0300 |
---|---|---|
committer | Matija Čupić <matteeyah@gmail.com> | 2018-06-06 22:29:12 +0300 |
commit | 9efb1875b7b001f0bda8afea60be7145459eb496 (patch) | |
tree | 6b1eb349cc5ad120c55f8e5af10df7b062b0c6b4 /spec | |
parent | 44be58836c826dfc0fbfa6d58641d34f84e292fb (diff) | |
parent | dd6aade3bf54a6d72a5b98daa34d0798b158399f (diff) |
Merge branch 'master' into 38542-application-control-panel-in-settings-page
Diffstat (limited to 'spec')
169 files changed, 5268 insertions, 843 deletions
diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb index b048da1991c..683c57c96f8 100644 --- a/spec/controllers/application_controller_spec.rb +++ b/spec/controllers/application_controller_spec.rb @@ -477,4 +477,28 @@ describe ApplicationController do end end end + + describe '#access_denied' do + controller(described_class) do + def index + access_denied!(params[:message]) + end + end + + before do + sign_in user + end + + it 'renders a 404 without a message' do + get :index + + expect(response).to have_gitlab_http_status(404) + end + + it 'renders a 403 when a message is passed to access denied' do + get :index, message: 'None shall pass' + + expect(response).to have_gitlab_http_status(403) + end + end end diff --git a/spec/controllers/concerns/controller_with_cross_project_access_check_spec.rb b/spec/controllers/concerns/controller_with_cross_project_access_check_spec.rb index 27f558e1b5d..d20471ef603 100644 --- a/spec/controllers/concerns/controller_with_cross_project_access_check_spec.rb +++ b/spec/controllers/concerns/controller_with_cross_project_access_check_spec.rb @@ -43,13 +43,13 @@ describe ControllerWithCrossProjectAccessCheck do end end - it 'renders a 404 with trying to access a cross project page' do + it 'renders a 403 with trying to access a cross project page' do message = "This page is unavailable because you are not allowed to read "\ "information across multiple projects." get :index - expect(response).to have_gitlab_http_status(404) + expect(response).to have_gitlab_http_status(403) expect(response.body).to match(/#{message}/) end @@ -119,7 +119,7 @@ describe ControllerWithCrossProjectAccessCheck do get :index - expect(response).to have_gitlab_http_status(404) + expect(response).to have_gitlab_http_status(403) end it 'is executed when the `unless` condition returns true' do @@ -127,19 +127,19 @@ describe ControllerWithCrossProjectAccessCheck do get :index - expect(response).to have_gitlab_http_status(404) + expect(response).to have_gitlab_http_status(403) end it 'does not skip the check on an action that is not skipped' do get :show, id: 'hello' - expect(response).to have_gitlab_http_status(404) + expect(response).to have_gitlab_http_status(403) end it 'does not skip the check on an action that was not defined to skip' do get :edit, id: 'hello' - expect(response).to have_gitlab_http_status(404) + expect(response).to have_gitlab_http_status(403) end end end diff --git a/spec/controllers/graphql_controller_spec.rb b/spec/controllers/graphql_controller_spec.rb new file mode 100644 index 00000000000..1449036e148 --- /dev/null +++ b/spec/controllers/graphql_controller_spec.rb @@ -0,0 +1,69 @@ +require 'spec_helper' + +describe GraphqlController do + describe 'execute' do + let(:user) { nil } + + before do + sign_in(user) if user + + run_test_query! + end + + subject { query_response } + + context 'graphql is disabled by feature flag' do + let(:user) { nil } + + before do + stub_feature_flags(graphql: false) + end + + it 'returns 404' do + run_test_query! + + expect(response).to have_gitlab_http_status(404) + end + end + + context 'signed out' do + let(:user) { nil } + + it 'runs the query with current_user: nil' do + is_expected.to eq('echo' => 'nil says: test success') + end + end + + context 'signed in' do + let(:user) { create(:user, username: 'Simon') } + + it 'runs the query with current_user set' do + is_expected.to eq('echo' => '"Simon" says: test success') + end + end + + context 'invalid variables' do + it 'returns an error' do + run_test_query!(variables: "This is not JSON") + + expect(response).to have_gitlab_http_status(422) + expect(json_response['errors'].first['message']).not_to be_nil + end + end + end + + # Chosen to exercise all the moving parts in GraphqlController#execute + def run_test_query!(variables: { 'text' => 'test success' }) + query = <<~QUERY + query Echo($text: String) { + echo(text: $text) + } + QUERY + + post :execute, query: query, operationName: 'Echo', variables: variables + end + + def query_response + json_response['data'] + end +end diff --git a/spec/controllers/groups/shared_projects_controller_spec.rb b/spec/controllers/groups/shared_projects_controller_spec.rb index d8fa41abb18..003c8c262e7 100644 --- a/spec/controllers/groups/shared_projects_controller_spec.rb +++ b/spec/controllers/groups/shared_projects_controller_spec.rb @@ -38,7 +38,7 @@ describe Groups::SharedProjectsController do end it 'allows filtering shared projects' do - project = create(:project, :archived, namespace: user.namespace, name: "Searching for") + project = create(:project, namespace: user.namespace, name: "Searching for") share_project(project) get_shared_projects(filter: 'search') @@ -55,5 +55,14 @@ describe Groups::SharedProjectsController do expect(json_project_ids).to eq([second_project.id, shared_project.id]) end + + it 'does not include archived projects' do + archived_project = create(:project, :archived, namespace: user.namespace) + share_project(archived_project) + + get_shared_projects + + expect(json_project_ids).to contain_exactly(shared_project.id) + end end end diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index 6e8de6db9c3..6e710c9b20b 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -80,6 +80,16 @@ describe Projects::MergeRequestsController do )) end end + + context "that is invalid" do + let(:merge_request) { create(:invalid_merge_request, target_project: project, source_project: project) } + + it "renders merge request page" do + go(format: :html) + + expect(response).to be_success + end + end end describe 'as json' do @@ -106,6 +116,16 @@ describe Projects::MergeRequestsController do expect(response).to match_response_schema('entities/merge_request_widget') end end + + context "that is invalid" do + let(:merge_request) { create(:invalid_merge_request, target_project: project, source_project: project) } + + it "renders merge request page" do + go(format: :json) + + expect(response).to be_success + end + end end describe "as diff" do diff --git a/spec/controllers/projects/milestones_controller_spec.rb b/spec/controllers/projects/milestones_controller_spec.rb index 548c5ef36e7..02b30f9bc6d 100644 --- a/spec/controllers/projects/milestones_controller_spec.rb +++ b/spec/controllers/projects/milestones_controller_spec.rb @@ -57,19 +57,36 @@ describe Projects::MilestonesController do context "as json" do let!(:group) { create(:group, :public) } let!(:group_milestone) { create(:milestone, group: group) } - let!(:group_member) { create(:group_member, group: group, user: user) } - before do - project.update(namespace: group) - get :index, namespace_id: project.namespace.id, project_id: project.id, format: :json + context 'with a single group ancestor' do + before do + project.update(namespace: group) + get :index, namespace_id: project.namespace.id, project_id: project.id, format: :json + end + + it "queries projects milestones and groups milestones" do + milestones = assigns(:milestones) + + expect(milestones.count).to eq(2) + expect(milestones).to match_array([milestone, group_milestone]) + end end - it "queries projects milestones and groups milestones" do - milestones = assigns(:milestones) + context 'with nested groups', :nested_groups do + let!(:subgroup) { create(:group, :public, parent: group) } + let!(:subgroup_milestone) { create(:milestone, group: subgroup) } + + before do + project.update(namespace: subgroup) + get :index, namespace_id: project.namespace.id, project_id: project.id, format: :json + end + + it "queries projects milestones and all ancestors milestones" do + milestones = assigns(:milestones) - expect(milestones.count).to eq(2) - expect(milestones.where(project_id: nil).first).to eq(group_milestone) - expect(milestones.where(group_id: nil).first).to eq(milestone) + expect(milestones.count).to eq(3) + expect(milestones).to match_array([milestone, group_milestone, subgroup_milestone]) + end end end end diff --git a/spec/controllers/projects/pipelines_controller_spec.rb b/spec/controllers/projects/pipelines_controller_spec.rb index 9e7bc20a6d1..92886e93077 100644 --- a/spec/controllers/projects/pipelines_controller_spec.rb +++ b/spec/controllers/projects/pipelines_controller_spec.rb @@ -17,44 +17,103 @@ describe Projects::PipelinesController do describe 'GET index.json' do before do - %w(pending running created success).each_with_index do |status, index| - sha = project.commit("HEAD~#{index}") - create(:ci_empty_pipeline, status: status, project: project, sha: sha) + %w(pending running success failed canceled).each_with_index do |status, index| + create_pipeline(status, project.commit("HEAD~#{index}")) end end - subject do - get :index, namespace_id: project.namespace, project_id: project, format: :json + context 'when using persisted stages', :request_store do + before do + stub_feature_flags(ci_pipeline_persisted_stages: true) + end + + it 'returns serialized pipelines', :request_store do + queries = ActiveRecord::QueryRecorder.new do + get_pipelines_index_json + end + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('pipeline') + + expect(json_response).to include('pipelines') + expect(json_response['pipelines'].count).to eq 5 + expect(json_response['count']['all']).to eq '5' + expect(json_response['count']['running']).to eq '1' + expect(json_response['count']['pending']).to eq '1' + expect(json_response['count']['finished']).to eq '3' + + json_response.dig('pipelines', 0, 'details', 'stages').tap do |stages| + expect(stages.count).to eq 3 + end + + expect(queries.count).to be + end end - it 'returns JSON with serialized pipelines' do - subject + context 'when using legacy stages', :request_store do + before do + stub_feature_flags(ci_pipeline_persisted_stages: false) + end - expect(response).to have_gitlab_http_status(:ok) - expect(response).to match_response_schema('pipeline') + it 'returns JSON with serialized pipelines', :request_store do + queries = ActiveRecord::QueryRecorder.new do + get_pipelines_index_json + end + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('pipeline') - expect(json_response).to include('pipelines') - expect(json_response['pipelines'].count).to eq 4 - expect(json_response['count']['all']).to eq '4' - expect(json_response['count']['running']).to eq '1' - expect(json_response['count']['pending']).to eq '1' - expect(json_response['count']['finished']).to eq '1' + expect(json_response).to include('pipelines') + expect(json_response['pipelines'].count).to eq 5 + expect(json_response['count']['all']).to eq '5' + expect(json_response['count']['running']).to eq '1' + expect(json_response['count']['pending']).to eq '1' + expect(json_response['count']['finished']).to eq '3' + + json_response.dig('pipelines', 0, 'details', 'stages').tap do |stages| + expect(stages.count).to eq 3 + end + + expect(queries.count).to be_within(3).of(30) + end end it 'does not include coverage data for the pipelines' do - subject + get_pipelines_index_json expect(json_response['pipelines'][0]).not_to include('coverage') end context 'when performing gitaly calls', :request_store do it 'limits the Gitaly requests' do - expect { subject }.to change { Gitlab::GitalyClient.get_request_count }.by(3) + expect { get_pipelines_index_json } + .to change { Gitlab::GitalyClient.get_request_count }.by(2) end end + + def get_pipelines_index_json + get :index, namespace_id: project.namespace, + project_id: project, + format: :json + end + + def create_pipeline(status, sha) + pipeline = create(:ci_empty_pipeline, status: status, + project: project, + sha: sha) + + create_build(pipeline, 'build', 1, 'build') + create_build(pipeline, 'test', 2, 'test') + create_build(pipeline, 'deploy', 3, 'deploy') + end + + def create_build(pipeline, stage, stage_idx, name) + status = %w[created running pending success failed canceled].sample + create(:ci_build, pipeline: pipeline, stage: stage, stage_idx: stage_idx, name: name, status: status) + end end - describe 'GET show JSON' do + describe 'GET show.json' do let(:pipeline) { create(:ci_pipeline_with_one_job, project: project) } it 'returns the pipeline' do @@ -67,6 +126,14 @@ describe Projects::PipelinesController do end context 'when the pipeline has multiple stages and groups', :request_store do + let(:project) { create(:project, :repository) } + + let(:pipeline) do + create(:ci_empty_pipeline, project: project, + user: user, + sha: project.commit.id) + end + before do create_build('build', 0, 'build') create_build('test', 1, 'rspec 0') @@ -74,11 +141,6 @@ describe Projects::PipelinesController do create_build('post deploy', 3, 'pages 0') end - let(:project) { create(:project, :repository) } - let(:pipeline) do - create(:ci_empty_pipeline, project: project, user: user, sha: project.commit.id) - end - it 'does not perform N + 1 queries' do control_count = ActiveRecord::QueryRecorder.new { get_pipeline_json }.count @@ -90,6 +152,7 @@ describe Projects::PipelinesController do create_build('post deploy', 3, 'pages 2') new_count = ActiveRecord::QueryRecorder.new { get_pipeline_json }.count + expect(new_count).to be_within(12).of(control_count) end end diff --git a/spec/controllers/search_controller_spec.rb b/spec/controllers/search_controller_spec.rb index 30c06ddf744..416a09e1684 100644 --- a/spec/controllers/search_controller_spec.rb +++ b/spec/controllers/search_controller_spec.rb @@ -32,7 +32,7 @@ describe SearchController do it 'still blocks searches without a project_id' do get :show, search: 'hello' - expect(response).to have_gitlab_http_status(404) + expect(response).to have_gitlab_http_status(403) end it 'allows searches with a project_id' do diff --git a/spec/controllers/users/terms_controller_spec.rb b/spec/controllers/users/terms_controller_spec.rb index a744463413c..0d77e91a67d 100644 --- a/spec/controllers/users/terms_controller_spec.rb +++ b/spec/controllers/users/terms_controller_spec.rb @@ -1,6 +1,7 @@ require 'spec_helper' describe Users::TermsController do + include TermsHelper let(:user) { create(:user) } let(:term) { create(:term) } @@ -15,10 +16,25 @@ describe Users::TermsController do expect(response).to have_gitlab_http_status(:redirect) end - it 'shows terms when they exist' do - term + context 'when terms exist' do + before do + stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false') + term + end + + it 'shows terms when they exist' do + get :index + + expect(response).to have_gitlab_http_status(:success) + end - expect(response).to have_gitlab_http_status(:success) + it 'shows a message when the user already accepted the terms' do + accept_terms(user) + + get :index + + expect(controller).to set_flash.now[:notice].to(/already accepted/) + end end end diff --git a/spec/factories/merge_requests.rb b/spec/factories/merge_requests.rb index fab0ec22450..3441ce1b8cb 100644 --- a/spec/factories/merge_requests.rb +++ b/spec/factories/merge_requests.rb @@ -54,6 +54,11 @@ FactoryBot.define do state :opened end + trait :invalid do + source_branch "feature_one" + target_branch "feature_two" + end + trait :locked do state :locked end @@ -98,6 +103,7 @@ FactoryBot.define do factory :merged_merge_request, traits: [:merged] factory :closed_merge_request, traits: [:closed] factory :reopened_merge_request, traits: [:opened] + factory :invalid_merge_request, traits: [:invalid] factory :merge_request_with_diffs, traits: [:with_diffs] factory :merge_request_with_diff_notes do after(:create) do |mr| diff --git a/spec/factories/term_agreements.rb b/spec/factories/term_agreements.rb index 557599e663d..3c4eebd0196 100644 --- a/spec/factories/term_agreements.rb +++ b/spec/factories/term_agreements.rb @@ -3,4 +3,12 @@ FactoryBot.define do term user end + + trait :declined do + accepted false + end + + trait :accepted do + accepted true + end end diff --git a/spec/features/markdown/markdown_spec.rb b/spec/features/markdown/markdown_spec.rb index f13d78d24e3..c86ba8c50a5 100644 --- a/spec/features/markdown/markdown_spec.rb +++ b/spec/features/markdown/markdown_spec.rb @@ -24,7 +24,7 @@ require 'erb' # # See the MarkdownFeature class for setup details. -describe 'GitLab Markdown' do +describe 'GitLab Markdown', :aggregate_failures do include Capybara::Node::Matchers include MarkupHelper include MarkdownMatchers @@ -44,112 +44,102 @@ describe 'GitLab Markdown' do # Shared behavior that all pipelines should exhibit shared_examples 'all pipelines' do - describe 'Redcarpet extensions' do - it 'does not parse emphasis inside of words' do + it 'includes Redcarpet extensions' do + aggregate_failures 'does not parse emphasis inside of words' do expect(doc.to_html).not_to match('foo<em>bar</em>baz') end - it 'parses table Markdown' do - aggregate_failures do - expect(doc).to have_selector('th:contains("Header")') - expect(doc).to have_selector('th:contains("Row")') - expect(doc).to have_selector('th:contains("Example")') - end + aggregate_failures 'parses table Markdown' do + expect(doc).to have_selector('th:contains("Header")') + expect(doc).to have_selector('th:contains("Row")') + expect(doc).to have_selector('th:contains("Example")') end - it 'allows Markdown in tables' do + aggregate_failures 'allows Markdown in tables' do expect(doc.at_css('td:contains("Baz")').children.to_html) .to eq '<strong>Baz</strong>' end - it 'parses fenced code blocks' do - aggregate_failures do - expect(doc).to have_selector('pre.code.highlight.js-syntax-highlight.c') - expect(doc).to have_selector('pre.code.highlight.js-syntax-highlight.python') - end + aggregate_failures 'parses fenced code blocks' do + expect(doc).to have_selector('pre.code.highlight.js-syntax-highlight.c') + expect(doc).to have_selector('pre.code.highlight.js-syntax-highlight.python') end - it 'parses mermaid code block' do - aggregate_failures do - expect(doc).to have_selector('pre[lang=mermaid] > code.js-render-mermaid') - end + aggregate_failures 'parses mermaid code block' do + expect(doc).to have_selector('pre[lang=mermaid] > code.js-render-mermaid') end - it 'parses strikethroughs' do + aggregate_failures 'parses strikethroughs' do expect(doc).to have_selector(%{del:contains("and this text doesn't")}) end - it 'parses superscript' do + aggregate_failures 'parses superscript' do expect(doc).to have_selector('sup', count: 2) end end - describe 'SanitizationFilter' do - it 'permits b elements' do + it 'includes SanitizationFilter' do + aggregate_failures 'permits b elements' do expect(doc).to have_selector('b:contains("b tag")') end - it 'permits em elements' do + aggregate_failures 'permits em elements' do expect(doc).to have_selector('em:contains("em tag")') end - it 'permits code elements' do + aggregate_failures 'permits code elements' do expect(doc).to have_selector('code:contains("code tag")') end - it 'permits kbd elements' do + aggregate_failures 'permits kbd elements' do expect(doc).to have_selector('kbd:contains("s")') end - it 'permits strike elements' do + aggregate_failures 'permits strike elements' do expect(doc).to have_selector('strike:contains(Emoji)') end - it 'permits img elements' do + aggregate_failures 'permits img elements' do expect(doc).to have_selector('img[data-src*="smile.png"]') end - it 'permits br elements' do + aggregate_failures 'permits br elements' do expect(doc).to have_selector('br') end - it 'permits hr elements' do + aggregate_failures 'permits hr elements' do expect(doc).to have_selector('hr') end - it 'permits span elements' do + aggregate_failures 'permits span elements' do expect(doc).to have_selector('span:contains("span tag")') end - it 'permits details elements' do + aggregate_failures 'permits details elements' do expect(doc).to have_selector('details:contains("Hiding the details")') end - it 'permits summary elements' do + aggregate_failures 'permits summary elements' do expect(doc).to have_selector('details summary:contains("collapsible")') end - it 'permits style attribute in th elements' do - aggregate_failures 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 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 - it 'permits style attribute in td elements' do - aggregate_failures 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 + 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 - it 'removes `rel` attribute from links' do + aggregate_failures 'removes `rel` attribute from links' do expect(doc).not_to have_selector('a[rel="bookmark"]') end - it "removes `href` from `a` elements if it's fishy" do + aggregate_failures "removes `href` from `a` elements if it's fishy" do expect(doc).not_to have_selector('a[href*="javascript"]') end end @@ -176,26 +166,26 @@ describe 'GitLab Markdown' do end end - describe 'ExternalLinkFilter' do - it 'adds nofollow to external link' do + it 'includes ExternalLinkFilter' do + aggregate_failures 'adds nofollow to external link' do link = doc.at_css('a:contains("Google")') expect(link.attr('rel')).to include('nofollow') end - it 'adds noreferrer to external link' do + aggregate_failures 'adds noreferrer to external link' do link = doc.at_css('a:contains("Google")') expect(link.attr('rel')).to include('noreferrer') end - it 'adds _blank to target attribute for external links' do + aggregate_failures 'adds _blank to target attribute for external links' do link = doc.at_css('a:contains("Google")') expect(link.attr('target')).to match('_blank') end - it 'ignores internal link' do + aggregate_failures 'ignores internal link' do link = doc.at_css('a:contains("GitLab Root")') expect(link.attr('rel')).not_to match 'nofollow' @@ -219,24 +209,24 @@ describe 'GitLab Markdown' do it_behaves_like 'all pipelines' - it 'includes RelativeLinkFilter' do - expect(doc).to parse_relative_links - end + it 'includes custom filters' do + aggregate_failures 'RelativeLinkFilter' do + expect(doc).to parse_relative_links + end - it 'includes EmojiFilter' do - expect(doc).to parse_emoji - end + aggregate_failures 'EmojiFilter' do + expect(doc).to parse_emoji + end - it 'includes TableOfContentsFilter' do - expect(doc).to create_header_links - end + aggregate_failures 'TableOfContentsFilter' do + expect(doc).to create_header_links + end - it 'includes AutolinkFilter' do - expect(doc).to create_autolinks - end + aggregate_failures 'AutolinkFilter' do + expect(doc).to create_autolinks + end - it 'includes all reference filters' do - aggregate_failures do + aggregate_failures 'all reference filters' do expect(doc).to reference_users expect(doc).to reference_issues expect(doc).to reference_merge_requests @@ -246,22 +236,22 @@ describe 'GitLab Markdown' do expect(doc).to reference_labels expect(doc).to reference_milestones end - end - it 'includes TaskListFilter' do - expect(doc).to parse_task_lists - end + aggregate_failures 'TaskListFilter' do + expect(doc).to parse_task_lists + end - it 'includes InlineDiffFilter' do - expect(doc).to parse_inline_diffs - end + aggregate_failures 'InlineDiffFilter' do + expect(doc).to parse_inline_diffs + end - it 'includes VideoLinkFilter' do - expect(doc).to parse_video_links - end + aggregate_failures 'VideoLinkFilter' do + expect(doc).to parse_video_links + end - it 'includes ColorFilter' do - expect(doc).to parse_colors + aggregate_failures 'ColorFilter' do + expect(doc).to parse_colors + end end end @@ -280,24 +270,24 @@ describe 'GitLab Markdown' do it_behaves_like 'all pipelines' - it 'includes RelativeLinkFilter' do - expect(doc).not_to parse_relative_links - end + it 'includes custom filters' do + aggregate_failures 'RelativeLinkFilter' do + expect(doc).not_to parse_relative_links + end - it 'includes EmojiFilter' do - expect(doc).to parse_emoji - end + aggregate_failures 'EmojiFilter' do + expect(doc).to parse_emoji + end - it 'includes TableOfContentsFilter' do - expect(doc).to create_header_links - end + aggregate_failures 'TableOfContentsFilter' do + expect(doc).to create_header_links + end - it 'includes AutolinkFilter' do - expect(doc).to create_autolinks - end + aggregate_failures 'AutolinkFilter' do + expect(doc).to create_autolinks + end - it 'includes all reference filters' do - aggregate_failures do + aggregate_failures 'all reference filters' do expect(doc).to reference_users expect(doc).to reference_issues expect(doc).to reference_merge_requests @@ -307,26 +297,26 @@ describe 'GitLab Markdown' do expect(doc).to reference_labels expect(doc).to reference_milestones end - end - it 'includes TaskListFilter' do - expect(doc).to parse_task_lists - end + aggregate_failures 'TaskListFilter' do + expect(doc).to parse_task_lists + end - it 'includes GollumTagsFilter' do - expect(doc).to parse_gollum_tags - end + aggregate_failures 'GollumTagsFilter' do + expect(doc).to parse_gollum_tags + end - it 'includes InlineDiffFilter' do - expect(doc).to parse_inline_diffs - end + aggregate_failures 'InlineDiffFilter' do + expect(doc).to parse_inline_diffs + end - it 'includes VideoLinkFilter' do - expect(doc).to parse_video_links - end + aggregate_failures 'VideoLinkFilter' do + expect(doc).to parse_video_links + end - it 'includes ColorFilter' do - expect(doc).to parse_colors + aggregate_failures 'ColorFilter' do + expect(doc).to parse_colors + end end end diff --git a/spec/features/merge_request/maintainer_edits_fork_spec.rb b/spec/features/merge_request/maintainer_edits_fork_spec.rb index a3323da1b1f..1808d0c0a0c 100644 --- a/spec/features/merge_request/maintainer_edits_fork_spec.rb +++ b/spec/features/merge_request/maintainer_edits_fork_spec.rb @@ -14,7 +14,7 @@ describe 'a maintainer edits files on a source-branch of an MR from a fork', :js source_branch: 'fix', target_branch: 'master', author: author, - allow_maintainer_to_push: true) + allow_collaboration: true) end before do diff --git a/spec/features/merge_request/user_allows_a_maintainer_to_push_spec.rb b/spec/features/merge_request/user_allows_commits_from_memebers_who_can_merge_spec.rb index eb41d7de8ed..0af37d76539 100644 --- a/spec/features/merge_request/user_allows_a_maintainer_to_push_spec.rb +++ b/spec/features/merge_request/user_allows_commits_from_memebers_who_can_merge_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe 'create a merge request that allows maintainers to push', :js do +describe 'create a merge request, allowing commits from members who can merge to the target branch', :js do include ProjectForksHelper let(:user) { create(:user) } let(:target_project) { create(:project, :public, :repository) } @@ -21,16 +21,16 @@ describe 'create a merge request that allows maintainers to push', :js do sign_in(user) end - it 'allows setting maintainer push possible' do + it 'allows setting possible' do visit_new_merge_request - check 'Allow edits from maintainers' + check 'Allow commits from members who can merge to the target branch' click_button 'Submit merge request' wait_for_requests - expect(page).to have_content('Allows edits from maintainers') + expect(page).to have_content('Allows commits from members who can merge to the target branch') end it 'shows a message when one of the projects is private' do @@ -57,12 +57,12 @@ describe 'create a merge request that allows maintainers to push', :js do visit_new_merge_request - expect(page).not_to have_content('Allows edits from maintainers') + expect(page).not_to have_content('Allows commits from members who can merge to the target branch') end end - context 'when a maintainer tries to edit the option' do - let(:maintainer) { create(:user) } + context 'when a member who can merge tries to edit the option' do + let(:member) { create(:user) } let(:merge_request) do create(:merge_request, source_project: source_project, @@ -71,15 +71,15 @@ describe 'create a merge request that allows maintainers to push', :js do end before do - target_project.add_master(maintainer) + target_project.add_master(member) - sign_in(maintainer) + sign_in(member) end - it 'it hides the option from maintainers' do + it 'it hides the option from members' do visit edit_project_merge_request_path(target_project, merge_request) - expect(page).not_to have_content('Allows edits from maintainers') + expect(page).not_to have_content('Allows commits from members who can merge to the target branch') end end end diff --git a/spec/features/projects/import_export/import_file_spec.rb b/spec/features/projects/import_export/import_file_spec.rb index 60fe30bd898..d0912e645bc 100644 --- a/spec/features/projects/import_export/import_file_spec.rb +++ b/spec/features/projects/import_export/import_file_spec.rb @@ -87,11 +87,13 @@ feature 'Import/Export - project import integration test', :js do def wiki_exists?(project) wiki = ProjectWiki.new(project) - File.exist?(wiki.repository.path_to_repo) && !wiki.repository.empty? + wiki.repository.exists? && !wiki.repository.empty? end def project_hook_exists?(project) - Gitlab::Git::Hook.new('post-receive', project.repository.raw_repository).exists? + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + Gitlab::Git::Hook.new('post-receive', project.repository.raw_repository).exists? + end end def click_import_project_tab diff --git a/spec/features/projects/jobs/user_browses_job_spec.rb b/spec/features/projects/jobs/user_browses_job_spec.rb index bff5bbe99af..ce0b38b7239 100644 --- a/spec/features/projects/jobs/user_browses_job_spec.rb +++ b/spec/features/projects/jobs/user_browses_job_spec.rb @@ -32,8 +32,6 @@ describe 'User browses a job', :js do page.within('.erased') do expect(page).to have_content('Job has been erased') end - - expect(build.project.running_or_pending_build_count).to eq(build.project.builds.running_or_pending.count(:all)) end context 'with a failed job' do diff --git a/spec/features/projects/settings/user_manages_group_links_spec.rb b/spec/features/projects/settings/user_manages_group_links_spec.rb index fdf42797091..92ce2ca83c7 100644 --- a/spec/features/projects/settings/user_manages_group_links_spec.rb +++ b/spec/features/projects/settings/user_manages_group_links_spec.rb @@ -30,7 +30,7 @@ describe 'Projects > Settings > User manages group links' do click_link('Share with group') select2(group_market.id, from: '#link_group_id') - select('Master', from: 'link_group_access') + select('Maintainer', from: 'link_group_access') click_button('Share') diff --git a/spec/features/projects/settings/user_manages_project_members_spec.rb b/spec/features/projects/settings/user_manages_project_members_spec.rb index 8af95522165..d3003753ae6 100644 --- a/spec/features/projects/settings/user_manages_project_members_spec.rb +++ b/spec/features/projects/settings/user_manages_project_members_spec.rb @@ -62,7 +62,7 @@ describe 'Projects > Settings > User manages project members' do page.within('.project-members-groups') do expect(page).to have_content('OpenSource') - expect(first('.group_member')).to have_content('Master') + expect(first('.group_member')).to have_content('Maintainer') end end end diff --git a/spec/features/protected_branches_spec.rb b/spec/features/protected_branches_spec.rb index 0c28a853b54..4c0f9971425 100644 --- a/spec/features/protected_branches_spec.rb +++ b/spec/features/protected_branches_spec.rb @@ -72,7 +72,7 @@ feature 'Protected Branches', :js do click_link 'No one' find(".js-allowed-to-push").click wait_for_requests - click_link 'Developers + Masters' + click_link 'Developers + Maintainers' end visit project_protected_branches_path(project) @@ -82,7 +82,7 @@ feature 'Protected Branches', :js do expect(page.find(".dropdown-toggle-text")).to have_content("No one") end page.within(".js-allowed-to-push") do - expect(page.find(".dropdown-toggle-text")).to have_content("Developers + Masters") + expect(page.find(".dropdown-toggle-text")).to have_content("Developers + Maintainers") end end end diff --git a/spec/features/runners_spec.rb b/spec/features/runners_spec.rb index 9ce7d538004..fe0b03a7e00 100644 --- a/spec/features/runners_spec.rb +++ b/spec/features/runners_spec.rb @@ -184,7 +184,7 @@ feature 'Runners' do given(:group) { create :group } - context 'as project and group master' do + context 'as project and group maintainer' do background do group.add_master(user) end @@ -197,13 +197,13 @@ feature 'Runners' do expect(page).to have_content 'This group does not provide any group Runners yet' - expect(page).to have_content 'Group masters can register group runners in the Group CI/CD settings' - expect(page).not_to have_content 'Ask your group master to setup a group Runner' + expect(page).to have_content 'Group maintainers can register group runners in the Group CI/CD settings' + expect(page).not_to have_content 'Ask your group maintainer to setup a group Runner' end end end - context 'as project master' do + context 'as project maintainer' do context 'project without a group' do given(:project) { create :project } @@ -223,8 +223,8 @@ feature 'Runners' do expect(page).to have_content 'This group does not provide any group Runners yet.' - expect(page).not_to have_content 'Group masters can register group runners in the Group CI/CD settings' - expect(page).to have_content 'Ask your group master to setup a group Runner.' + expect(page).not_to have_content 'Group maintainers can register group runners in the Group CI/CD settings' + expect(page).to have_content 'Ask your group maintainer to setup a group Runner.' end end diff --git a/spec/features/users/terms_spec.rb b/spec/features/users/terms_spec.rb index 1efa5cd5490..af407c52917 100644 --- a/spec/features/users/terms_spec.rb +++ b/spec/features/users/terms_spec.rb @@ -39,6 +39,22 @@ describe 'Users > Terms' do end end + context 'when the user has already accepted the terms' do + before do + accept_terms(user) + end + + it 'allows the user to continue to the app' do + visit terms_path + + expect(page).to have_content "You have already accepted the Terms of Service as #{user.to_reference}" + + click_link 'Continue' + + expect(current_path).to eq(root_path) + end + end + context 'terms were enforced while session is active', :js do let(:project) { create(:project) } diff --git a/spec/fixtures/api/schemas/entities/merge_request_basic.json b/spec/fixtures/api/schemas/entities/merge_request_basic.json index 46031961cca..f7bc137c90c 100644 --- a/spec/fixtures/api/schemas/entities/merge_request_basic.json +++ b/spec/fixtures/api/schemas/entities/merge_request_basic.json @@ -13,6 +13,7 @@ "assignee_id": { "type": ["integer", "null"] }, "subscribed": { "type": ["boolean", "null"] }, "participants": { "type": "array" }, + "allow_collaboration": { "type": "boolean"}, "allow_maintainer_to_push": { "type": "boolean"} }, "additionalProperties": false diff --git a/spec/fixtures/api/schemas/entities/merge_request_widget.json b/spec/fixtures/api/schemas/entities/merge_request_widget.json index 7be8c9e3e67..ee5588fa6c6 100644 --- a/spec/fixtures/api/schemas/entities/merge_request_widget.json +++ b/spec/fixtures/api/schemas/entities/merge_request_widget.json @@ -31,7 +31,7 @@ "source_project_id": { "type": "integer" }, "target_branch": { "type": "string" }, "target_project_id": { "type": "integer" }, - "allow_maintainer_to_push": { "type": "boolean"}, + "allow_collaboration": { "type": "boolean"}, "metrics": { "oneOf": [ { "type": "null" }, diff --git a/spec/fixtures/api/schemas/public_api/v4/merge_requests.json b/spec/fixtures/api/schemas/public_api/v4/merge_requests.json index f97461ce9cc..f7adc4e0b91 100644 --- a/spec/fixtures/api/schemas/public_api/v4/merge_requests.json +++ b/spec/fixtures/api/schemas/public_api/v4/merge_requests.json @@ -82,6 +82,7 @@ "human_time_estimate": { "type": ["string", "null"] }, "human_total_time_spent": { "type": ["string", "null"] } }, + "allow_collaboration": { "type": ["boolean", "null"] }, "allow_maintainer_to_push": { "type": ["boolean", "null"] } }, "required": [ diff --git a/spec/graphql/gitlab_schema_spec.rb b/spec/graphql/gitlab_schema_spec.rb new file mode 100644 index 00000000000..b892f6b44ed --- /dev/null +++ b/spec/graphql/gitlab_schema_spec.rb @@ -0,0 +1,33 @@ +require 'spec_helper' + +describe GitlabSchema do + it 'uses batch loading' do + expect(field_instrumenters).to include(BatchLoader::GraphQL) + end + + it 'enables the preload instrumenter' do + expect(field_instrumenters).to include(BatchLoader::GraphQL) + end + + it 'enables the authorization instrumenter' do + expect(field_instrumenters).to include(instance_of(::Gitlab::Graphql::Authorize::Instrumentation)) + end + + it 'enables using presenters' do + expect(field_instrumenters).to include(instance_of(::Gitlab::Graphql::Present::Instrumentation)) + end + + it 'has the base mutation' do + pending('Adding an empty mutation breaks the documentation explorer') + + expect(described_class.mutation).to eq(::Types::MutationType.to_graphql) + end + + it 'has the base query' do + expect(described_class.query).to eq(::Types::QueryType.to_graphql) + end + + def field_instrumenters + described_class.instrumenters[:field] + end +end diff --git a/spec/graphql/resolvers/merge_request_resolver_spec.rb b/spec/graphql/resolvers/merge_request_resolver_spec.rb new file mode 100644 index 00000000000..af015533209 --- /dev/null +++ b/spec/graphql/resolvers/merge_request_resolver_spec.rb @@ -0,0 +1,58 @@ +require 'spec_helper' + +describe Resolvers::MergeRequestResolver do + include GraphqlHelpers + + set(:project) { create(:project, :repository) } + set(:merge_request_1) { create(:merge_request, :simple, source_project: project, target_project: project) } + set(:merge_request_2) { create(:merge_request, :rebased, source_project: project, target_project: project) } + + set(:other_project) { create(:project, :repository) } + set(:other_merge_request) { create(:merge_request, source_project: other_project, target_project: other_project) } + + let(:full_path) { project.full_path } + let(:iid_1) { merge_request_1.iid } + let(:iid_2) { merge_request_2.iid } + + let(:other_full_path) { other_project.full_path } + let(:other_iid) { other_merge_request.iid } + + describe '#resolve' do + it 'batch-resolves merge requests by target project full path and IID' do + path = full_path # avoid database query + + result = batch(max_queries: 2) do + [resolve_mr(path, iid_1), resolve_mr(path, iid_2)] + end + + expect(result).to contain_exactly(merge_request_1, merge_request_2) + end + + it 'can batch-resolve merge requests from different projects' do + path = project.full_path # avoid database queries + other_path = other_full_path + + result = batch(max_queries: 3) do + [resolve_mr(path, iid_1), resolve_mr(path, iid_2), resolve_mr(other_path, other_iid)] + end + + expect(result).to contain_exactly(merge_request_1, merge_request_2, other_merge_request) + end + + it 'resolves an unknown iid to nil' do + result = batch { resolve_mr(full_path, -1) } + + expect(result).to be_nil + end + + it 'resolves a known iid for an unknown full_path to nil' do + result = batch { resolve_mr('unknown/project', iid_1) } + + expect(result).to be_nil + end + end + + def resolve_mr(full_path, iid) + resolve(described_class, args: { full_path: full_path, iid: iid }) + end +end diff --git a/spec/graphql/resolvers/project_resolver_spec.rb b/spec/graphql/resolvers/project_resolver_spec.rb new file mode 100644 index 00000000000..d4990c6492c --- /dev/null +++ b/spec/graphql/resolvers/project_resolver_spec.rb @@ -0,0 +1,32 @@ +require 'spec_helper' + +describe Resolvers::ProjectResolver do + include GraphqlHelpers + + set(:project1) { create(:project) } + set(:project2) { create(:project) } + + set(:other_project) { create(:project) } + + describe '#resolve' do + it 'batch-resolves projects by full path' do + paths = [project1.full_path, project2.full_path] + + result = batch(max_queries: 1) do + paths.map { |path| resolve_project(path) } + end + + expect(result).to contain_exactly(project1, project2) + end + + it 'resolves an unknown full_path to nil' do + result = batch { resolve_project('unknown/project') } + + expect(result).to be_nil + end + end + + def resolve_project(full_path) + resolve(described_class, args: { full_path: full_path }) + end +end diff --git a/spec/graphql/types/project_type_spec.rb b/spec/graphql/types/project_type_spec.rb new file mode 100644 index 00000000000..e0f89105b86 --- /dev/null +++ b/spec/graphql/types/project_type_spec.rb @@ -0,0 +1,5 @@ +require 'spec_helper' + +describe GitlabSchema.types['Project'] do + it { expect(described_class.graphql_name).to eq('Project') } +end diff --git a/spec/graphql/types/query_type_spec.rb b/spec/graphql/types/query_type_spec.rb new file mode 100644 index 00000000000..8488252fd59 --- /dev/null +++ b/spec/graphql/types/query_type_spec.rb @@ -0,0 +1,37 @@ +require 'spec_helper' + +describe GitlabSchema.types['Query'] do + it 'is called Query' do + expect(described_class.graphql_name).to eq('Query') + end + + it { is_expected.to have_graphql_fields(:project, :merge_request, :echo) } + + describe 'project field' do + subject { described_class.fields['project'] } + + it 'finds projects by full path' do + is_expected.to have_graphql_arguments(:full_path) + is_expected.to have_graphql_type(Types::ProjectType) + is_expected.to have_graphql_resolver(Resolvers::ProjectResolver) + end + + it 'authorizes with read_project' do + is_expected.to require_graphql_authorizations(:read_project) + end + end + + describe 'merge_request field' do + subject { described_class.fields['mergeRequest'] } + + it 'finds MRs by project and IID' do + is_expected.to have_graphql_arguments(:full_path, :iid) + is_expected.to have_graphql_type(Types::MergeRequestType) + is_expected.to have_graphql_resolver(Resolvers::MergeRequestResolver) + end + + it 'authorizes with read_merge_request' do + is_expected.to require_graphql_authorizations(:read_merge_request) + end + end +end diff --git a/spec/graphql/types/time_type_spec.rb b/spec/graphql/types/time_type_spec.rb new file mode 100644 index 00000000000..4196d9d27d4 --- /dev/null +++ b/spec/graphql/types/time_type_spec.rb @@ -0,0 +1,16 @@ +require 'spec_helper' + +describe GitlabSchema.types['Time'] do + let(:iso) { "2018-06-04T15:23:50+02:00" } + let(:time) { Time.parse(iso) } + + it { expect(described_class.graphql_name).to eq('Time') } + + it 'coerces Time object into ISO 8601' do + expect(described_class.coerce_isolated_result(time)).to eq(iso) + end + + it 'coerces an ISO-time into Time object' do + expect(described_class.coerce_isolated_input(iso)).to eq(time) + end +end diff --git a/spec/helpers/projects_helper_spec.rb b/spec/helpers/projects_helper_spec.rb index f8877b6d1aa..d372e58f63d 100644 --- a/spec/helpers/projects_helper_spec.rb +++ b/spec/helpers/projects_helper_spec.rb @@ -5,9 +5,9 @@ describe ProjectsHelper do describe "#project_status_css_class" do it "returns appropriate class" do - expect(project_status_css_class("started")).to eq("active") - expect(project_status_css_class("failed")).to eq("danger") - expect(project_status_css_class("finished")).to eq("success") + expect(project_status_css_class("started")).to eq("table-active") + expect(project_status_css_class("failed")).to eq("table-danger") + expect(project_status_css_class("finished")).to eq("table-success") end end @@ -435,4 +435,46 @@ describe ProjectsHelper do expect(helper.send(:git_user_name)).to eq('John \"A\" Doe53') end end + + describe 'show_xcode_link' do + let!(:project) { create(:project) } + let(:mac_ua) { 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_13_4) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/66.0.3359.139 Safari/537.36' } + let(:ios_ua) { 'Mozilla/5.0 (iPad; CPU OS 5_1_1 like Mac OS X) AppleWebKit/534.46 (KHTML, like Gecko) Version/5.1 Mobile/9B206 Safari/7534.48.3' } + + context 'when the repository is xcode compatible' do + before do + allow(project.repository).to receive(:xcode_project?).and_return(true) + end + + it 'returns false if the visitor is not using macos' do + allow(helper).to receive(:browser).and_return(Browser.new(ios_ua)) + + expect(helper.show_xcode_link?(project)).to eq(false) + end + + it 'returns true if the visitor is using macos' do + allow(helper).to receive(:browser).and_return(Browser.new(mac_ua)) + + expect(helper.show_xcode_link?(project)).to eq(true) + end + end + + context 'when the repository is not xcode compatible' do + before do + allow(project.repository).to receive(:xcode_project?).and_return(false) + end + + it 'returns false if the visitor is not using macos' do + allow(helper).to receive(:browser).and_return(Browser.new(ios_ua)) + + expect(helper.show_xcode_link?(project)).to eq(false) + end + + it 'returns false if the visitor is using macos' do + allow(helper).to receive(:browser).and_return(Browser.new(mac_ua)) + + expect(helper.show_xcode_link?(project)).to eq(false) + end + end + end end diff --git a/spec/initializers/artifacts_direct_upload_support_spec.rb b/spec/initializers/artifacts_direct_upload_support_spec.rb deleted file mode 100644 index bfb71da3388..00000000000 --- a/spec/initializers/artifacts_direct_upload_support_spec.rb +++ /dev/null @@ -1,71 +0,0 @@ -require 'spec_helper' - -describe 'Artifacts direct upload support' do - subject do - load Rails.root.join('config/initializers/artifacts_direct_upload_support.rb') - end - - let(:connection) do - { provider: provider } - end - - before do - stub_artifacts_setting( - object_store: { - enabled: enabled, - direct_upload: direct_upload, - connection: connection - }) - end - - context 'when object storage is enabled' do - let(:enabled) { true } - - context 'when direct upload is enabled' do - let(:direct_upload) { true } - - context 'when provider is Google' do - let(:provider) { 'Google' } - - it 'succeeds' do - expect { subject }.not_to raise_error - end - end - - context 'when connection is empty' do - let(:connection) { nil } - - it 'raises an error' do - expect { subject }.to raise_error /object storage provider when 'direct_upload' of artifacts is used/ - end - end - - context 'when other provider is used' do - let(:provider) { 'AWS' } - - it 'raises an error' do - expect { subject }.to raise_error /object storage provider when 'direct_upload' of artifacts is used/ - end - end - end - - context 'when direct upload is disabled' do - let(:direct_upload) { false } - let(:provider) { 'AWS' } - - it 'succeeds' do - expect { subject }.not_to raise_error - end - end - end - - context 'when object storage is disabled' do - let(:enabled) { false } - let(:direct_upload) { false } - let(:provider) { 'AWS' } - - it 'succeeds' do - expect { subject }.not_to raise_error - end - end -end diff --git a/spec/initializers/direct_upload_support_spec.rb b/spec/initializers/direct_upload_support_spec.rb new file mode 100644 index 00000000000..e51d404e030 --- /dev/null +++ b/spec/initializers/direct_upload_support_spec.rb @@ -0,0 +1,90 @@ +require 'spec_helper' + +describe 'Direct upload support' do + subject do + load Rails.root.join('config/initializers/direct_upload_support.rb') + end + + where(:config_name) do + %w(lfs artifacts uploads) + end + + with_them do + let(:connection) do + { provider: provider } + end + + let(:object_store) do + { + enabled: enabled, + direct_upload: direct_upload, + connection: connection + } + end + + before do + allow(Gitlab.config).to receive_messages(to_settings(config_name => { + object_store: object_store + })) + end + + context 'when object storage is enabled' do + let(:enabled) { true } + + context 'when direct upload is enabled' do + let(:direct_upload) { true } + + context 'when provider is AWS' do + let(:provider) { 'AWS' } + + it 'succeeds' do + expect { subject }.not_to raise_error + end + end + + context 'when provider is Google' do + let(:provider) { 'Google' } + + it 'succeeds' do + expect { subject }.not_to raise_error + end + end + + context 'when connection is empty' do + let(:connection) { nil } + + it 'raises an error' do + expect { subject }.to raise_error /are supported as a object storage provider when 'direct_upload' is used/ + end + end + + context 'when other provider is used' do + let(:provider) { 'Rackspace' } + + it 'raises an error' do + expect { subject }.to raise_error /are supported as a object storage provider when 'direct_upload' is used/ + end + end + end + + context 'when direct upload is disabled' do + let(:direct_upload) { false } + let(:provider) { 'AWS' } + + it 'succeeds' do + expect { subject }.not_to raise_error + end + end + end + + context 'when object storage is disabled' do + let(:enabled) { false } + let(:direct_upload) { false } + let(:provider) { 'Rackspace' } + + it 'succeeds' do + expect { subject }.not_to raise_error + end + end + end +end diff --git a/spec/initializers/fog_google_https_private_urls_spec.rb b/spec/initializers/fog_google_https_private_urls_spec.rb index de3c157ab7b..08346b71fee 100644 --- a/spec/initializers/fog_google_https_private_urls_spec.rb +++ b/spec/initializers/fog_google_https_private_urls_spec.rb @@ -1,13 +1,13 @@ require 'spec_helper' -describe 'Fog::Storage::GoogleXML::File' do +describe 'Fog::Storage::GoogleXML::File', :fog_requests do let(:storage) do Fog.mock! - Fog::Storage.new({ - google_storage_access_key_id: "asdf", - google_storage_secret_access_key: "asdf", - provider: "Google" - }) + Fog::Storage.new( + google_storage_access_key_id: "asdf", + google_storage_secret_access_key: "asdf", + provider: "Google" + ) end let(:file) do diff --git a/spec/javascripts/ide/components/jobs/detail/description_spec.js b/spec/javascripts/ide/components/jobs/detail/description_spec.js new file mode 100644 index 00000000000..9b715a41499 --- /dev/null +++ b/spec/javascripts/ide/components/jobs/detail/description_spec.js @@ -0,0 +1,28 @@ +import Vue from 'vue'; +import Description from '~/ide/components/jobs/detail/description.vue'; +import mountComponent from '../../../../helpers/vue_mount_component_helper'; +import { jobs } from '../../../mock_data'; + +describe('IDE job description', () => { + const Component = Vue.extend(Description); + let vm; + + beforeEach(() => { + vm = mountComponent(Component, { + job: jobs[0], + }); + }); + + afterEach(() => { + vm.$destroy(); + }); + + it('renders job details', () => { + expect(vm.$el.textContent).toContain('#1'); + expect(vm.$el.textContent).toContain('test'); + }); + + it('renders CI icon', () => { + expect(vm.$el.querySelector('.ci-status-icon .ic-status_passed_borderless')).not.toBe(null); + }); +}); diff --git a/spec/javascripts/ide/components/jobs/detail/scroll_button_spec.js b/spec/javascripts/ide/components/jobs/detail/scroll_button_spec.js new file mode 100644 index 00000000000..fff382a107f --- /dev/null +++ b/spec/javascripts/ide/components/jobs/detail/scroll_button_spec.js @@ -0,0 +1,59 @@ +import Vue from 'vue'; +import ScrollButton from '~/ide/components/jobs/detail/scroll_button.vue'; +import mountComponent from '../../../../helpers/vue_mount_component_helper'; + +describe('IDE job log scroll button', () => { + const Component = Vue.extend(ScrollButton); + let vm; + + beforeEach(() => { + vm = mountComponent(Component, { + direction: 'up', + disabled: false, + }); + }); + + afterEach(() => { + vm.$destroy(); + }); + + describe('iconName', () => { + ['up', 'down'].forEach(direction => { + it(`returns icon name for ${direction}`, () => { + vm.direction = direction; + + expect(vm.iconName).toBe(`scroll_${direction}`); + }); + }); + }); + + describe('tooltipTitle', () => { + it('returns title for up', () => { + expect(vm.tooltipTitle).toBe('Scroll to top'); + }); + + it('returns title for down', () => { + vm.direction = 'down'; + + expect(vm.tooltipTitle).toBe('Scroll to bottom'); + }); + }); + + it('emits click event on click', () => { + spyOn(vm, '$emit'); + + vm.$el.querySelector('.btn-scroll').click(); + + expect(vm.$emit).toHaveBeenCalledWith('click'); + }); + + it('disables button when disabled is true', done => { + vm.disabled = true; + + vm.$nextTick(() => { + expect(vm.$el.querySelector('.btn-scroll').hasAttribute('disabled')).toBe(true); + + done(); + }); + }); +}); diff --git a/spec/javascripts/ide/components/jobs/detail_spec.js b/spec/javascripts/ide/components/jobs/detail_spec.js new file mode 100644 index 00000000000..641ba06f653 --- /dev/null +++ b/spec/javascripts/ide/components/jobs/detail_spec.js @@ -0,0 +1,180 @@ +import Vue from 'vue'; +import JobDetail from '~/ide/components/jobs/detail.vue'; +import { createStore } from '~/ide/stores'; +import { createComponentWithStore } from '../../../helpers/vue_mount_component_helper'; +import { jobs } from '../../mock_data'; + +describe('IDE jobs detail view', () => { + const Component = Vue.extend(JobDetail); + let vm; + + beforeEach(() => { + const store = createStore(); + + store.state.pipelines.detailJob = { + ...jobs[0], + isLoading: true, + output: 'testing', + rawPath: `${gl.TEST_HOST}/raw`, + }; + + vm = createComponentWithStore(Component, store); + + spyOn(vm, 'fetchJobTrace').and.returnValue(Promise.resolve()); + + vm = vm.$mount(); + + spyOn(vm.$refs.buildTrace, 'scrollTo'); + }); + + afterEach(() => { + vm.$destroy(); + }); + + it('calls fetchJobTrace on mount', () => { + expect(vm.fetchJobTrace).toHaveBeenCalled(); + }); + + it('scrolls to bottom on mount', done => { + setTimeout(() => { + expect(vm.$refs.buildTrace.scrollTo).toHaveBeenCalled(); + + done(); + }); + }); + + it('renders job output', () => { + expect(vm.$el.querySelector('.bash').textContent).toContain('testing'); + }); + + it('renders empty message output', done => { + vm.$store.state.pipelines.detailJob.output = ''; + + vm.$nextTick(() => { + expect(vm.$el.querySelector('.bash').textContent).toContain('No messages were logged'); + + done(); + }); + }); + + it('renders loading icon', () => { + expect(vm.$el.querySelector('.build-loader-animation')).not.toBe(null); + expect(vm.$el.querySelector('.build-loader-animation').style.display).toBe(''); + }); + + it('hide loading icon when isLoading is false', done => { + vm.$store.state.pipelines.detailJob.isLoading = false; + + vm.$nextTick(() => { + expect(vm.$el.querySelector('.build-loader-animation').style.display).toBe('none'); + + done(); + }); + }); + + it('resets detailJob when clicking header button', () => { + spyOn(vm, 'setDetailJob'); + + vm.$el.querySelector('.btn').click(); + + expect(vm.setDetailJob).toHaveBeenCalledWith(null); + }); + + it('renders raw path link', () => { + expect(vm.$el.querySelector('.controllers-buttons').getAttribute('href')).toBe( + `${gl.TEST_HOST}/raw`, + ); + }); + + describe('scroll buttons', () => { + it('triggers scrollDown when clicking down button', done => { + spyOn(vm, 'scrollDown'); + + vm.$el.querySelectorAll('.btn-scroll')[1].click(); + + vm.$nextTick(() => { + expect(vm.scrollDown).toHaveBeenCalled(); + + done(); + }); + }); + + it('triggers scrollUp when clicking up button', done => { + spyOn(vm, 'scrollUp'); + + vm.scrollPos = 1; + + vm + .$nextTick() + .then(() => vm.$el.querySelector('.btn-scroll').click()) + .then(() => vm.$nextTick()) + .then(() => { + expect(vm.scrollUp).toHaveBeenCalled(); + }) + .then(done) + .catch(done.fail); + }); + }); + + describe('scrollDown', () => { + it('scrolls build trace to bottom', () => { + spyOnProperty(vm.$refs.buildTrace, 'scrollHeight').and.returnValue(1000); + + vm.scrollDown(); + + expect(vm.$refs.buildTrace.scrollTo).toHaveBeenCalledWith(0, 1000); + }); + }); + + describe('scrollUp', () => { + it('scrolls build trace to top', () => { + vm.scrollUp(); + + expect(vm.$refs.buildTrace.scrollTo).toHaveBeenCalledWith(0, 0); + }); + }); + + describe('scrollBuildLog', () => { + beforeEach(() => { + spyOnProperty(vm.$refs.buildTrace, 'offsetHeight').and.returnValue(100); + spyOnProperty(vm.$refs.buildTrace, 'scrollHeight').and.returnValue(200); + }); + + it('sets scrollPos to bottom when at the bottom', done => { + spyOnProperty(vm.$refs.buildTrace, 'scrollTop').and.returnValue(100); + + vm.scrollBuildLog(); + + setTimeout(() => { + expect(vm.scrollPos).toBe(1); + + done(); + }); + }); + + it('sets scrollPos to top when at the top', done => { + spyOnProperty(vm.$refs.buildTrace, 'scrollTop').and.returnValue(0); + vm.scrollPos = 1; + + vm.scrollBuildLog(); + + setTimeout(() => { + expect(vm.scrollPos).toBe(0); + + done(); + }); + }); + + it('resets scrollPos when not at top or bottom', done => { + spyOnProperty(vm.$refs.buildTrace, 'scrollTop').and.returnValue(10); + + vm.scrollBuildLog(); + + setTimeout(() => { + expect(vm.scrollPos).toBe(''); + + done(); + }); + }); + }); +}); diff --git a/spec/javascripts/ide/components/jobs/item_spec.js b/spec/javascripts/ide/components/jobs/item_spec.js index 7c1dd4e475c..79e07f00e7b 100644 --- a/spec/javascripts/ide/components/jobs/item_spec.js +++ b/spec/javascripts/ide/components/jobs/item_spec.js @@ -26,4 +26,14 @@ describe('IDE jobs item', () => { it('renders CI icon', () => { expect(vm.$el.querySelector('.ic-status_passed_borderless')).not.toBe(null); }); + + it('does not render view logs button if not started', done => { + vm.job.started = false; + + vm.$nextTick(() => { + expect(vm.$el.querySelector('.btn')).toBe(null); + + done(); + }); + }); }); diff --git a/spec/javascripts/ide/components/merge_requests/dropdown_spec.js b/spec/javascripts/ide/components/merge_requests/dropdown_spec.js new file mode 100644 index 00000000000..74884c9a362 --- /dev/null +++ b/spec/javascripts/ide/components/merge_requests/dropdown_spec.js @@ -0,0 +1,47 @@ +import Vue from 'vue'; +import { createStore } from '~/ide/stores'; +import Dropdown from '~/ide/components/merge_requests/dropdown.vue'; +import { createComponentWithStore } from '../../../helpers/vue_mount_component_helper'; +import { mergeRequests } from '../../mock_data'; + +describe('IDE merge requests dropdown', () => { + const Component = Vue.extend(Dropdown); + let vm; + + beforeEach(() => { + const store = createStore(); + + vm = createComponentWithStore(Component, store, { show: false }).$mount(); + }); + + afterEach(() => { + vm.$destroy(); + }); + + it('does not render tabs when show is false', () => { + expect(vm.$el.querySelector('.nav-links')).toBe(null); + }); + + describe('when show is true', () => { + beforeEach(done => { + vm.show = true; + vm.$store.state.mergeRequests.assigned.mergeRequests.push(mergeRequests[0]); + + vm.$nextTick(done); + }); + + it('renders tabs', () => { + expect(vm.$el.querySelector('.nav-links')).not.toBe(null); + }); + + it('renders count for assigned & created data', () => { + expect(vm.$el.querySelector('.nav-links a').textContent).toContain('Created by me'); + expect(vm.$el.querySelector('.nav-links a .badge').textContent).toContain('0'); + + expect(vm.$el.querySelectorAll('.nav-links a')[1].textContent).toContain('Assigned to me'); + expect( + vm.$el.querySelectorAll('.nav-links a')[1].querySelector('.badge').textContent, + ).toContain('1'); + }); + }); +}); diff --git a/spec/javascripts/ide/components/merge_requests/item_spec.js b/spec/javascripts/ide/components/merge_requests/item_spec.js new file mode 100644 index 00000000000..51c4cddef2f --- /dev/null +++ b/spec/javascripts/ide/components/merge_requests/item_spec.js @@ -0,0 +1,61 @@ +import Vue from 'vue'; +import Item from '~/ide/components/merge_requests/item.vue'; +import mountCompontent from '../../../helpers/vue_mount_component_helper'; + +describe('IDE merge request item', () => { + const Component = Vue.extend(Item); + let vm; + + beforeEach(() => { + vm = mountCompontent(Component, { + item: { + iid: 1, + projectPathWithNamespace: 'gitlab-org/gitlab-ce', + title: 'Merge request title', + }, + currentId: '1', + currentProjectId: 'gitlab-org/gitlab-ce', + }); + }); + + afterEach(() => { + vm.$destroy(); + }); + + it('renders merge requests data', () => { + expect(vm.$el.textContent).toContain('Merge request title'); + expect(vm.$el.textContent).toContain('gitlab-org/gitlab-ce!1'); + }); + + it('renders icon if ID matches currentId', () => { + expect(vm.$el.querySelector('.ic-mobile-issue-close')).not.toBe(null); + }); + + it('does not render icon if ID does not match currentId', done => { + vm.currentId = '2'; + + vm.$nextTick(() => { + expect(vm.$el.querySelector('.ic-mobile-issue-close')).toBe(null); + + done(); + }); + }); + + it('does not render icon if project ID does not match', done => { + vm.currentProjectId = 'test/test'; + + vm.$nextTick(() => { + expect(vm.$el.querySelector('.ic-mobile-issue-close')).toBe(null); + + done(); + }); + }); + + it('emits click event on click', () => { + spyOn(vm, '$emit'); + + vm.$el.click(); + + expect(vm.$emit).toHaveBeenCalledWith('click', vm.item); + }); +}); diff --git a/spec/javascripts/ide/components/merge_requests/list_spec.js b/spec/javascripts/ide/components/merge_requests/list_spec.js new file mode 100644 index 00000000000..f4b393778dc --- /dev/null +++ b/spec/javascripts/ide/components/merge_requests/list_spec.js @@ -0,0 +1,126 @@ +import Vue from 'vue'; +import store from '~/ide/stores'; +import List from '~/ide/components/merge_requests/list.vue'; +import { createComponentWithStore } from '../../../helpers/vue_mount_component_helper'; +import { mergeRequests } from '../../mock_data'; +import { resetStore } from '../../helpers'; + +describe('IDE merge requests list', () => { + const Component = Vue.extend(List); + let vm; + + beforeEach(() => { + vm = createComponentWithStore(Component, store, { + type: 'created', + emptyText: 'empty text', + }); + + spyOn(vm, 'fetchMergeRequests'); + + vm.$mount(); + }); + + afterEach(() => { + vm.$destroy(); + + resetStore(vm.$store); + }); + + it('calls fetch on mounted', () => { + expect(vm.fetchMergeRequests).toHaveBeenCalledWith({ + type: 'created', + search: '', + }); + }); + + it('renders loading icon', done => { + vm.$store.state.mergeRequests.created.isLoading = true; + + vm.$nextTick(() => { + expect(vm.$el.querySelector('.loading-container')).not.toBe(null); + + done(); + }); + }); + + it('renders empty text when no merge requests exist', () => { + expect(vm.$el.textContent).toContain('empty text'); + }); + + it('renders no search results text when search is not empty', done => { + vm.search = 'testing'; + + vm.$nextTick(() => { + expect(vm.$el.textContent).toContain('No merge requests found'); + + done(); + }); + }); + + describe('with merge requests', () => { + beforeEach(done => { + vm.$store.state.mergeRequests.created.mergeRequests.push({ + ...mergeRequests[0], + projectPathWithNamespace: 'gitlab-org/gitlab-ce', + }); + + vm.$nextTick(done); + }); + + it('renders list', () => { + expect(vm.$el.querySelectorAll('li').length).toBe(1); + expect(vm.$el.querySelector('li').textContent).toContain(mergeRequests[0].title); + }); + + it('calls openMergeRequest when clicking merge request', done => { + spyOn(vm, 'openMergeRequest'); + vm.$el.querySelector('li button').click(); + + vm.$nextTick(() => { + expect(vm.openMergeRequest).toHaveBeenCalledWith({ + projectPath: 'gitlab-org/gitlab-ce', + id: 1, + }); + + done(); + }); + }); + }); + + describe('focusSearch', () => { + it('focuses search input when loading is false', done => { + spyOn(vm.$refs.searchInput, 'focus'); + + vm.$store.state.mergeRequests.created.isLoading = false; + vm.focusSearch(); + + vm.$nextTick(() => { + expect(vm.$refs.searchInput.focus).toHaveBeenCalled(); + + done(); + }); + }); + }); + + describe('searchMergeRequests', () => { + beforeEach(() => { + spyOn(vm, 'loadMergeRequests'); + + jasmine.clock().install(); + }); + + afterEach(() => { + jasmine.clock().uninstall(); + }); + + it('calls loadMergeRequests on input in search field', () => { + const event = new Event('input'); + + vm.$el.querySelector('input').dispatchEvent(event); + + jasmine.clock().tick(300); + + expect(vm.loadMergeRequests).toHaveBeenCalled(); + }); + }); +}); diff --git a/spec/javascripts/ide/mock_data.js b/spec/javascripts/ide/mock_data.js index dcf857f7e04..dd87a43f370 100644 --- a/spec/javascripts/ide/mock_data.js +++ b/spec/javascripts/ide/mock_data.js @@ -75,6 +75,7 @@ export const jobs = [ }, stage: 'test', duration: 1, + started: new Date(), }, { id: 2, @@ -86,6 +87,7 @@ export const jobs = [ }, stage: 'test', duration: 1, + started: new Date(), }, { id: 3, @@ -97,6 +99,7 @@ export const jobs = [ }, stage: 'test', duration: 1, + started: new Date(), }, { id: 4, @@ -108,6 +111,7 @@ export const jobs = [ }, stage: 'build', duration: 1, + started: new Date(), }, ]; diff --git a/spec/javascripts/ide/stores/modules/merge_requests/actions_spec.js b/spec/javascripts/ide/stores/modules/merge_requests/actions_spec.js index b571cfb963a..d178a44b76a 100644 --- a/spec/javascripts/ide/stores/modules/merge_requests/actions_spec.js +++ b/spec/javascripts/ide/stores/modules/merge_requests/actions_spec.js @@ -8,7 +8,9 @@ import actions, { receiveMergeRequestsSuccess, fetchMergeRequests, resetMergeRequests, + openMergeRequest, } from '~/ide/stores/modules/merge_requests/actions'; +import router from '~/ide/ide_router'; import { mergeRequests } from '../../../mock_data'; import testAction from '../../../../helpers/vuex_action_helper'; @@ -29,9 +31,9 @@ describe('IDE merge requests actions', () => { it('should should commit request', done => { testAction( requestMergeRequests, - null, + 'created', mockedState, - [{ type: types.REQUEST_MERGE_REQUESTS }], + [{ type: types.REQUEST_MERGE_REQUESTS, payload: 'created' }], [], done, ); @@ -48,16 +50,16 @@ describe('IDE merge requests actions', () => { it('should should commit error', done => { testAction( receiveMergeRequestsError, - null, + 'created', mockedState, - [{ type: types.RECEIVE_MERGE_REQUESTS_ERROR }], + [{ type: types.RECEIVE_MERGE_REQUESTS_ERROR, payload: 'created' }], [], done, ); }); it('creates flash message', () => { - receiveMergeRequestsError({ commit() {} }); + receiveMergeRequestsError({ commit() {} }, 'created'); expect(flashSpy).toHaveBeenCalled(); }); @@ -67,9 +69,14 @@ describe('IDE merge requests actions', () => { it('should commit received data', done => { testAction( receiveMergeRequestsSuccess, - 'data', + { type: 'created', data: 'data' }, mockedState, - [{ type: types.RECEIVE_MERGE_REQUESTS_SUCCESS, payload: 'data' }], + [ + { + type: types.RECEIVE_MERGE_REQUESTS_SUCCESS, + payload: { type: 'created', data: 'data' }, + }, + ], [], done, ); @@ -86,14 +93,14 @@ describe('IDE merge requests actions', () => { mock.onGet(/\/api\/v4\/merge_requests(.*)$/).replyOnce(200, mergeRequests); }); - it('calls API with params from state', () => { + it('calls API with params', () => { const apiSpy = spyOn(axios, 'get').and.callThrough(); - fetchMergeRequests({ dispatch() {}, state: mockedState }); + fetchMergeRequests({ dispatch() {}, state: mockedState }, { type: 'created' }); expect(apiSpy).toHaveBeenCalledWith(jasmine.anything(), { params: { - scope: 'assigned-to-me', + scope: 'created-by-me', state: 'opened', search: '', }, @@ -103,11 +110,14 @@ describe('IDE merge requests actions', () => { it('calls API with search', () => { const apiSpy = spyOn(axios, 'get').and.callThrough(); - fetchMergeRequests({ dispatch() {}, state: mockedState }, 'testing search'); + fetchMergeRequests( + { dispatch() {}, state: mockedState }, + { type: 'created', search: 'testing search' }, + ); expect(apiSpy).toHaveBeenCalledWith(jasmine.anything(), { params: { - scope: 'assigned-to-me', + scope: 'created-by-me', state: 'opened', search: 'testing search', }, @@ -117,7 +127,7 @@ describe('IDE merge requests actions', () => { it('dispatches request', done => { testAction( fetchMergeRequests, - null, + { type: 'created' }, mockedState, [], [ @@ -132,13 +142,16 @@ describe('IDE merge requests actions', () => { it('dispatches success with received data', done => { testAction( fetchMergeRequests, - null, + { type: 'created' }, mockedState, [], [ { type: 'requestMergeRequests' }, { type: 'resetMergeRequests' }, - { type: 'receiveMergeRequestsSuccess', payload: mergeRequests }, + { + type: 'receiveMergeRequestsSuccess', + payload: { type: 'created', data: mergeRequests }, + }, ], done, ); @@ -153,7 +166,7 @@ describe('IDE merge requests actions', () => { it('dispatches error', done => { testAction( fetchMergeRequests, - null, + { type: 'created' }, mockedState, [], [ @@ -171,12 +184,47 @@ describe('IDE merge requests actions', () => { it('commits reset', done => { testAction( resetMergeRequests, - null, + 'created', mockedState, - [{ type: types.RESET_MERGE_REQUESTS }], + [{ type: types.RESET_MERGE_REQUESTS, payload: 'created' }], [], done, ); }); }); + + describe('openMergeRequest', () => { + beforeEach(() => { + spyOn(router, 'push'); + }); + + it('commits reset mutations and actions', done => { + testAction( + openMergeRequest, + { projectPath: 'gitlab-org/gitlab-ce', id: '1' }, + mockedState, + [ + { type: 'CLEAR_PROJECTS' }, + { type: 'SET_CURRENT_MERGE_REQUEST', payload: '1' }, + { type: 'RESET_OPEN_FILES' }, + ], + [ + { type: 'pipelines/stopPipelinePolling' }, + { type: 'pipelines/clearEtagPoll' }, + { type: 'pipelines/resetLatestPipeline' }, + { type: 'setCurrentBranchId', payload: '' }, + ], + done, + ); + }); + + it('pushes new route', () => { + openMergeRequest( + { commit() {}, dispatch() {} }, + { projectPath: 'gitlab-org/gitlab-ce', id: '1' }, + ); + + expect(router.push).toHaveBeenCalledWith('/project/gitlab-org/gitlab-ce/merge_requests/1'); + }); + }); }); diff --git a/spec/javascripts/ide/stores/modules/merge_requests/mutations_spec.js b/spec/javascripts/ide/stores/modules/merge_requests/mutations_spec.js index 664d3914564..ea03131d90d 100644 --- a/spec/javascripts/ide/stores/modules/merge_requests/mutations_spec.js +++ b/spec/javascripts/ide/stores/modules/merge_requests/mutations_spec.js @@ -12,26 +12,29 @@ describe('IDE merge requests mutations', () => { describe(types.REQUEST_MERGE_REQUESTS, () => { it('sets loading to true', () => { - mutations[types.REQUEST_MERGE_REQUESTS](mockedState); + mutations[types.REQUEST_MERGE_REQUESTS](mockedState, 'created'); - expect(mockedState.isLoading).toBe(true); + expect(mockedState.created.isLoading).toBe(true); }); }); describe(types.RECEIVE_MERGE_REQUESTS_ERROR, () => { it('sets loading to false', () => { - mutations[types.RECEIVE_MERGE_REQUESTS_ERROR](mockedState); + mutations[types.RECEIVE_MERGE_REQUESTS_ERROR](mockedState, 'created'); - expect(mockedState.isLoading).toBe(false); + expect(mockedState.created.isLoading).toBe(false); }); }); describe(types.RECEIVE_MERGE_REQUESTS_SUCCESS, () => { it('sets merge requests', () => { gon.gitlab_url = gl.TEST_HOST; - mutations[types.RECEIVE_MERGE_REQUESTS_SUCCESS](mockedState, mergeRequests); + mutations[types.RECEIVE_MERGE_REQUESTS_SUCCESS](mockedState, { + type: 'created', + data: mergeRequests, + }); - expect(mockedState.mergeRequests).toEqual([ + expect(mockedState.created.mergeRequests).toEqual([ { id: 1, iid: 1, @@ -47,9 +50,9 @@ describe('IDE merge requests mutations', () => { it('clears merge request array', () => { mockedState.mergeRequests = ['test']; - mutations[types.RESET_MERGE_REQUESTS](mockedState); + mutations[types.RESET_MERGE_REQUESTS](mockedState, 'created'); - expect(mockedState.mergeRequests).toEqual([]); + expect(mockedState.created.mergeRequests).toEqual([]); }); }); }); diff --git a/spec/javascripts/ide/stores/modules/pipelines/actions_spec.js b/spec/javascripts/ide/stores/modules/pipelines/actions_spec.js index f26eaf9c81f..f2f8e780cd1 100644 --- a/spec/javascripts/ide/stores/modules/pipelines/actions_spec.js +++ b/spec/javascripts/ide/stores/modules/pipelines/actions_spec.js @@ -13,9 +13,15 @@ import actions, { receiveJobsSuccess, fetchJobs, toggleStageCollapsed, + setDetailJob, + requestJobTrace, + receiveJobTraceError, + receiveJobTraceSuccess, + fetchJobTrace, } from '~/ide/stores/modules/pipelines/actions'; import state from '~/ide/stores/modules/pipelines/state'; import * as types from '~/ide/stores/modules/pipelines/mutation_types'; +import { rightSidebarViews } from '~/ide/constants'; import testAction from '../../../../helpers/vuex_action_helper'; import { pipelines, jobs } from '../../../mock_data'; @@ -281,4 +287,133 @@ describe('IDE pipelines actions', () => { ); }); }); + + describe('setDetailJob', () => { + it('commits job', done => { + testAction( + setDetailJob, + 'job', + mockedState, + [{ type: types.SET_DETAIL_JOB, payload: 'job' }], + [{ type: 'setRightPane' }], + done, + ); + }); + + it('dispatches setRightPane as pipeline when job is null', done => { + testAction( + setDetailJob, + null, + mockedState, + [{ type: types.SET_DETAIL_JOB }], + [{ type: 'setRightPane', payload: rightSidebarViews.pipelines }], + done, + ); + }); + + it('dispatches setRightPane as job', done => { + testAction( + setDetailJob, + 'job', + mockedState, + [{ type: types.SET_DETAIL_JOB }], + [{ type: 'setRightPane', payload: rightSidebarViews.jobsDetail }], + done, + ); + }); + }); + + describe('requestJobTrace', () => { + it('commits request', done => { + testAction(requestJobTrace, null, mockedState, [{ type: types.REQUEST_JOB_TRACE }], [], done); + }); + }); + + describe('receiveJobTraceError', () => { + it('commits error', done => { + testAction( + receiveJobTraceError, + null, + mockedState, + [{ type: types.RECEIVE_JOB_TRACE_ERROR }], + [], + done, + ); + }); + + it('creates flash message', () => { + const flashSpy = spyOnDependency(actions, 'flash'); + + receiveJobTraceError({ commit() {} }); + + expect(flashSpy).toHaveBeenCalled(); + }); + }); + + describe('receiveJobTraceSuccess', () => { + it('commits data', done => { + testAction( + receiveJobTraceSuccess, + 'data', + mockedState, + [{ type: types.RECEIVE_JOB_TRACE_SUCCESS, payload: 'data' }], + [], + done, + ); + }); + }); + + describe('fetchJobTrace', () => { + beforeEach(() => { + mockedState.detailJob = { + path: `${gl.TEST_HOST}/project/builds`, + }; + }); + + describe('success', () => { + beforeEach(() => { + spyOn(axios, 'get').and.callThrough(); + mock.onGet(`${gl.TEST_HOST}/project/builds/trace`).replyOnce(200, { html: 'html' }); + }); + + it('dispatches request', done => { + testAction( + fetchJobTrace, + null, + mockedState, + [], + [ + { type: 'requestJobTrace' }, + { type: 'receiveJobTraceSuccess', payload: { html: 'html' } }, + ], + done, + ); + }); + + it('sends get request to correct URL', () => { + fetchJobTrace({ state: mockedState, dispatch() {} }); + + expect(axios.get).toHaveBeenCalledWith(`${gl.TEST_HOST}/project/builds/trace`, { + params: { format: 'json' }, + }); + }); + }); + + describe('error', () => { + beforeEach(() => { + mock.onGet(`${gl.TEST_HOST}/project/builds/trace`).replyOnce(500); + }); + + it('dispatches error', done => { + testAction( + fetchJobTrace, + null, + mockedState, + [], + [{ type: 'requestJobTrace' }, { type: 'receiveJobTraceError' }], + done, + ); + }); + }); + }); }); diff --git a/spec/javascripts/ide/stores/modules/pipelines/mutations_spec.js b/spec/javascripts/ide/stores/modules/pipelines/mutations_spec.js index 6285c01d483..eb7346bd5fc 100644 --- a/spec/javascripts/ide/stores/modules/pipelines/mutations_spec.js +++ b/spec/javascripts/ide/stores/modules/pipelines/mutations_spec.js @@ -147,6 +147,10 @@ describe('IDE pipelines mutations', () => { name: job.name, status: job.status, path: job.build_path, + rawPath: `${job.build_path}/raw`, + started: job.started, + isLoading: false, + output: '', })), ); }); @@ -171,4 +175,49 @@ describe('IDE pipelines mutations', () => { expect(mockedState.stages[0].isCollapsed).toBe(false); }); }); + + describe(types.SET_DETAIL_JOB, () => { + it('sets detail job', () => { + mutations[types.SET_DETAIL_JOB](mockedState, jobs[0]); + + expect(mockedState.detailJob).toEqual(jobs[0]); + }); + }); + + describe(types.REQUEST_JOB_TRACE, () => { + beforeEach(() => { + mockedState.detailJob = { ...jobs[0] }; + }); + + it('sets loading on detail job', () => { + mutations[types.REQUEST_JOB_TRACE](mockedState); + + expect(mockedState.detailJob.isLoading).toBe(true); + }); + }); + + describe(types.RECEIVE_JOB_TRACE_ERROR, () => { + beforeEach(() => { + mockedState.detailJob = { ...jobs[0], isLoading: true }; + }); + + it('sets loading to false on detail job', () => { + mutations[types.RECEIVE_JOB_TRACE_ERROR](mockedState); + + expect(mockedState.detailJob.isLoading).toBe(false); + }); + }); + + describe(types.RECEIVE_JOB_TRACE_SUCCESS, () => { + beforeEach(() => { + mockedState.detailJob = { ...jobs[0], isLoading: true }; + }); + + it('sets output on detail job', () => { + mutations[types.RECEIVE_JOB_TRACE_SUCCESS](mockedState, { html: 'html' }); + + expect(mockedState.detailJob.output).toBe('html'); + expect(mockedState.detailJob.isLoading).toBe(false); + }); + }); }); diff --git a/spec/javascripts/importer_status_spec.js b/spec/javascripts/importer_status_spec.js index 0575d02886d..63cdb3d5114 100644 --- a/spec/javascripts/importer_status_spec.js +++ b/spec/javascripts/importer_status_spec.js @@ -45,7 +45,25 @@ describe('Importer Status', () => { currentTarget: document.querySelector('.js-add-to-import'), }) .then(() => { - expect(document.querySelector('tr').classList.contains('active')).toEqual(true); + expect(document.querySelector('tr').classList.contains('table-active')).toEqual(true); + done(); + }) + .catch(done.fail); + }); + + it('shows error message after failed POST request', (done) => { + appendSetFixtures('<div class="flash-container"></div>'); + + mock.onPost(importUrl).reply(422, { + errors: 'You forgot your lunch', + }); + + instance.addToImport({ + currentTarget: document.querySelector('.js-add-to-import'), + }) + .then(() => { + const flashMessage = document.querySelector('.flash-text'); + expect(flashMessage.textContent.trim()).toEqual('An error occurred while importing project: You forgot your lunch'); done(); }) .catch(done.fail); diff --git a/spec/javascripts/jobs/header_spec.js b/spec/javascripts/jobs/header_spec.js index 4f861c39d3f..cef30a007db 100644 --- a/spec/javascripts/jobs/header_spec.js +++ b/spec/javascripts/jobs/header_spec.js @@ -13,6 +13,9 @@ describe('Job details header', () => { const threeWeeksAgo = new Date(); threeWeeksAgo.setDate(threeWeeksAgo.getDate() - 21); + const twoDaysAgo = new Date(); + twoDaysAgo.setDate(twoDaysAgo.getDate() - 2); + props = { job: { status: { @@ -31,7 +34,7 @@ describe('Job details header', () => { email: 'foo@bar.com', avatar_url: 'link', }, - started: '2018-01-08T09:48:27.319Z', + started: twoDaysAgo.toISOString(), new_issue_path: 'path', }, isLoading: false, @@ -69,7 +72,7 @@ describe('Job details header', () => { .querySelector('.header-main-content') .textContent.replace(/\s+/g, ' ') .trim(), - ).toEqual('failed Job #123 triggered 3 weeks ago by Foo'); + ).toEqual('failed Job #123 triggered 2 days ago by Foo'); }); it('should render new issue link', () => { diff --git a/spec/javascripts/labels_select_spec.js b/spec/javascripts/labels_select_spec.js index a2b89c0aef5..386e00bfd0c 100644 --- a/spec/javascripts/labels_select_spec.js +++ b/spec/javascripts/labels_select_spec.js @@ -40,5 +40,9 @@ describe('LabelsSelect', () => { it('generated label item template has correct label styles', () => { expect($labelEl.find('span.label').attr('style')).toBe(`background-color: ${label.color}; color: ${label.text_color};`); }); + + it('generated label item has a badge class', () => { + expect($labelEl.find('span').hasClass('badge')).toEqual(true); + }); }); }); diff --git a/spec/javascripts/monitoring/graph_spec.js b/spec/javascripts/monitoring/graph_spec.js index 220228e5c08..a46a387a534 100644 --- a/spec/javascripts/monitoring/graph_spec.js +++ b/spec/javascripts/monitoring/graph_spec.js @@ -18,9 +18,7 @@ const createComponent = propsData => { }).$mount(); }; -const convertedMetrics = convertDatesMultipleSeries( - singleRowMetricsMultipleSeries, -); +const convertedMetrics = convertDatesMultipleSeries(singleRowMetricsMultipleSeries); describe('Graph', () => { beforeEach(() => { @@ -36,7 +34,7 @@ describe('Graph', () => { projectPath, }); - expect(component.$el.querySelector('.text-center').innerText.trim()).toBe( + expect(component.$el.querySelector('.prometheus-graph-title').innerText.trim()).toBe( component.graphData.title, ); }); @@ -52,9 +50,7 @@ describe('Graph', () => { }); const transformedHeight = `${component.graphHeight - 100}`; - expect(component.axisTransform.indexOf(transformedHeight)).not.toEqual( - -1, - ); + expect(component.axisTransform.indexOf(transformedHeight)).not.toEqual(-1); }); it('outerViewBox gets a width and height property based on the DOM size of the element', () => { diff --git a/spec/javascripts/notes/components/note_actions_spec.js b/spec/javascripts/notes/components/note_actions_spec.js index 1dfe890e05e..c9e549d2096 100644 --- a/spec/javascripts/notes/components/note_actions_spec.js +++ b/spec/javascripts/notes/components/note_actions_spec.js @@ -20,7 +20,7 @@ describe('issue_note_actions component', () => { beforeEach(() => { props = { - accessLevel: 'Master', + accessLevel: 'Maintainer', authorId: 26, canDelete: true, canEdit: true, @@ -67,7 +67,7 @@ describe('issue_note_actions component', () => { beforeEach(() => { store.dispatch('setUserData', {}); props = { - accessLevel: 'Master', + accessLevel: 'Maintainer', authorId: 26, canDelete: false, canEdit: false, diff --git a/spec/javascripts/notes/mock_data.js b/spec/javascripts/notes/mock_data.js index bfe3a65feee..fa7adc32193 100644 --- a/spec/javascripts/notes/mock_data.js +++ b/spec/javascripts/notes/mock_data.js @@ -340,6 +340,79 @@ export const loggedOutnoteableData = { '/gitlab-org/gitlab-ce/preview_markdown?quick_actions_target_id=98&quick_actions_target_type=Issue', }; +export const collapseNotesMock = [ + { + expanded: true, + id: '0fb4e0e3f9276e55ff32eb4195add694aece4edd', + individual_note: true, + notes: [ + { + id: 1390, + attachment: null, + author: { + id: 1, + name: 'Root', + username: 'root', + state: 'active', + avatar_url: 'test', + path: '/root', + }, + created_at: '2018-02-26T18:07:41.071Z', + updated_at: '2018-02-26T18:07:41.071Z', + system: true, + system_note_icon_name: 'pencil', + noteable_id: 98, + noteable_type: 'Issue', + type: null, + human_access: 'Owner', + note: 'changed the description', + note_html: '<p dir="auto">changed the description</p>', + current_user: { can_edit: false }, + discussion_id: 'b97fb7bda470a65b3e009377a9032edec0a4dd05', + emoji_awardable: false, + path: '/h5bp/html5-boilerplate/notes/1057', + report_abuse_path: + '/abuse_reports/new?ref_url=http%3A%2F%2Flocalhost%3A3000%2Fh5bp%2Fhtml5-boilerplate%2Fissues%2F10%23note_1057&user_id=1', + }, + ], + }, + { + expanded: true, + id: 'ffde43f25984ad7f2b4275135e0e2846875336c0', + individual_note: true, + notes: [ + { + id: 1391, + attachment: null, + author: { + id: 1, + name: 'Root', + username: 'root', + state: 'active', + avatar_url: 'test', + path: '/root', + }, + created_at: '2018-02-26T18:13:24.071Z', + updated_at: '2018-02-26T18:13:24.071Z', + system: true, + system_note_icon_name: 'pencil', + noteable_id: 99, + noteable_type: 'Issue', + type: null, + human_access: 'Owner', + note: 'changed the description', + note_html: '<p dir="auto">changed the description</p>', + current_user: { can_edit: false }, + discussion_id: '3eb958b4d81dec207ec3537a2f3bd8b9f271bb34', + emoji_awardable: false, + path: '/h5bp/html5-boilerplate/notes/1057', + report_abuse_path: + '/abuse_reports/new?ref_url=http%3A%2F%2Flocalhost%3A3000%2Fh5bp%2Fhtml5-boilerplate%2Fissues%2F10%23note_1057&user_id=1', + }, + ], + }, +]; + export const INDIVIDUAL_NOTE_RESPONSE_MAP = { GET: { '/gitlab-org/gitlab-ce/issues/26/discussions.json': [ @@ -575,3 +648,508 @@ export function discussionNoteInterceptor(request, next) { }), ); } + +export const notesWithDescriptionChanges = [ + { + id: '39b271c2033e9ed43d8edb393702f65f7a830459', + reply_id: '39b271c2033e9ed43d8edb393702f65f7a830459', + expanded: true, + notes: [ + { + id: 901, + type: null, + attachment: null, + author: { + id: 1, + name: 'Administrator', + username: 'root', + state: 'active', + avatar_url: + 'https://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon', + path: '/root', + }, + created_at: '2018-05-29T12:05:36.117Z', + updated_at: '2018-05-29T12:05:36.117Z', + system: false, + noteable_id: 182, + noteable_type: 'Issue', + resolvable: false, + noteable_iid: 12, + note: + 'Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.', + note_html: + '<p dir="auto">Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.</p>', + current_user: { can_edit: true, can_award_emoji: true }, + resolved: false, + resolved_by: null, + discussion_id: '39b271c2033e9ed43d8edb393702f65f7a830459', + emoji_awardable: true, + award_emoji: [], + report_abuse_path: + '/abuse_reports/new?ref_url=http%3A%2F%2Flocalhost%3A3000%2Fgitlab-org%2Fgitlab-shell%2Fissues%2F12%23note_901&user_id=1', + human_access: 'Owner', + toggle_award_path: '/gitlab-org/gitlab-shell/notes/901/toggle_award_emoji', + path: '/gitlab-org/gitlab-shell/notes/901', + }, + ], + individual_note: true, + resolvable: false, + resolved: false, + diff_discussion: false, + }, + { + id: '4852335d7dc40b9ceb8fde1a2bb9c1b67e4c7795', + reply_id: '4852335d7dc40b9ceb8fde1a2bb9c1b67e4c7795', + expanded: true, + notes: [ + { + id: 902, + type: null, + attachment: null, + author: { + id: 1, + name: 'Administrator', + username: 'root', + state: 'active', + avatar_url: + 'https://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon', + path: '/root', + }, + created_at: '2018-05-29T12:05:58.694Z', + updated_at: '2018-05-29T12:05:58.694Z', + system: false, + noteable_id: 182, + noteable_type: 'Issue', + resolvable: false, + noteable_iid: 12, + note: + 'Varius vel pharetra vel turpis nunc eget lorem. Ipsum dolor sit amet consectetur adipiscing.', + note_html: + '<p dir="auto">Varius vel pharetra vel turpis nunc eget lorem. Ipsum dolor sit amet consectetur adipiscing.</p>', + current_user: { can_edit: true, can_award_emoji: true }, + resolved: false, + resolved_by: null, + discussion_id: '4852335d7dc40b9ceb8fde1a2bb9c1b67e4c7795', + emoji_awardable: true, + award_emoji: [], + report_abuse_path: + '/abuse_reports/new?ref_url=http%3A%2F%2Flocalhost%3A3000%2Fgitlab-org%2Fgitlab-shell%2Fissues%2F12%23note_902&user_id=1', + human_access: 'Owner', + toggle_award_path: '/gitlab-org/gitlab-shell/notes/902/toggle_award_emoji', + path: '/gitlab-org/gitlab-shell/notes/902', + }, + ], + individual_note: true, + resolvable: false, + resolved: false, + diff_discussion: false, + }, + { + id: '7f1feda384083eb31763366e6392399fde6f3f31', + reply_id: '7f1feda384083eb31763366e6392399fde6f3f31', + expanded: true, + notes: [ + { + id: 903, + type: null, + attachment: null, + author: { + id: 1, + name: 'Administrator', + username: 'root', + state: 'active', + avatar_url: + 'https://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon', + path: '/root', + }, + created_at: '2018-05-29T12:06:05.772Z', + updated_at: '2018-05-29T12:06:05.772Z', + system: true, + noteable_id: 182, + noteable_type: 'Issue', + resolvable: false, + noteable_iid: 12, + note: 'changed the description', + note_html: '<p dir="auto">changed the description</p>', + current_user: { can_edit: false, can_award_emoji: true }, + resolved: false, + resolved_by: null, + system_note_icon_name: 'pencil-square', + discussion_id: '7f1feda384083eb31763366e6392399fde6f3f31', + emoji_awardable: false, + report_abuse_path: + '/abuse_reports/new?ref_url=http%3A%2F%2Flocalhost%3A3000%2Fgitlab-org%2Fgitlab-shell%2Fissues%2F12%23note_903&user_id=1', + human_access: 'Owner', + path: '/gitlab-org/gitlab-shell/notes/903', + }, + ], + individual_note: true, + resolvable: false, + resolved: false, + diff_discussion: false, + }, + { + id: '091865fe3ae20f0045234a3d103e3b15e73405b5', + reply_id: '091865fe3ae20f0045234a3d103e3b15e73405b5', + expanded: true, + notes: [ + { + id: 904, + type: null, + attachment: null, + author: { + id: 1, + name: 'Administrator', + username: 'root', + state: 'active', + avatar_url: + 'https://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon', + path: '/root', + }, + created_at: '2018-05-29T12:06:16.112Z', + updated_at: '2018-05-29T12:06:16.112Z', + system: false, + noteable_id: 182, + noteable_type: 'Issue', + resolvable: false, + noteable_iid: 12, + note: 'Ullamcorper eget nulla facilisi etiam', + note_html: '<p dir="auto">Ullamcorper eget nulla facilisi etiam</p>', + current_user: { can_edit: true, can_award_emoji: true }, + resolved: false, + resolved_by: null, + discussion_id: '091865fe3ae20f0045234a3d103e3b15e73405b5', + emoji_awardable: true, + award_emoji: [], + report_abuse_path: + '/abuse_reports/new?ref_url=http%3A%2F%2Flocalhost%3A3000%2Fgitlab-org%2Fgitlab-shell%2Fissues%2F12%23note_904&user_id=1', + human_access: 'Owner', + toggle_award_path: '/gitlab-org/gitlab-shell/notes/904/toggle_award_emoji', + path: '/gitlab-org/gitlab-shell/notes/904', + }, + ], + individual_note: true, + resolvable: false, + resolved: false, + diff_discussion: false, + }, + { + id: 'a21cf2e804acc3c60d07e37d75e395f5a9a4d044', + reply_id: 'a21cf2e804acc3c60d07e37d75e395f5a9a4d044', + expanded: true, + notes: [ + { + id: 905, + type: null, + attachment: null, + author: { + id: 1, + name: 'Administrator', + username: 'root', + state: 'active', + avatar_url: + 'https://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon', + path: '/root', + }, + created_at: '2018-05-29T12:06:28.851Z', + updated_at: '2018-05-29T12:06:28.851Z', + system: true, + noteable_id: 182, + noteable_type: 'Issue', + resolvable: false, + noteable_iid: 12, + note: 'changed the description', + note_html: '<p dir="auto">changed the description</p>', + current_user: { can_edit: false, can_award_emoji: true }, + resolved: false, + resolved_by: null, + system_note_icon_name: 'pencil-square', + discussion_id: 'a21cf2e804acc3c60d07e37d75e395f5a9a4d044', + emoji_awardable: false, + report_abuse_path: + '/abuse_reports/new?ref_url=http%3A%2F%2Flocalhost%3A3000%2Fgitlab-org%2Fgitlab-shell%2Fissues%2F12%23note_905&user_id=1', + human_access: 'Owner', + path: '/gitlab-org/gitlab-shell/notes/905', + }, + ], + individual_note: true, + resolvable: false, + resolved: false, + diff_discussion: false, + }, + { + id: '70411b08cdfc01f24187a06d77daa33464cb2620', + reply_id: '70411b08cdfc01f24187a06d77daa33464cb2620', + expanded: true, + notes: [ + { + id: 906, + type: null, + attachment: null, + author: { + id: 1, + name: 'Administrator', + username: 'root', + state: 'active', + avatar_url: + 'https://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon', + path: '/root', + }, + created_at: '2018-05-29T12:20:02.925Z', + updated_at: '2018-05-29T12:20:02.925Z', + system: true, + noteable_id: 182, + noteable_type: 'Issue', + resolvable: false, + noteable_iid: 12, + note: 'changed the description', + note_html: '<p dir="auto">changed the description</p>', + current_user: { can_edit: false, can_award_emoji: true }, + resolved: false, + resolved_by: null, + system_note_icon_name: 'pencil-square', + discussion_id: '70411b08cdfc01f24187a06d77daa33464cb2620', + emoji_awardable: false, + report_abuse_path: + '/abuse_reports/new?ref_url=http%3A%2F%2Flocalhost%3A3000%2Fgitlab-org%2Fgitlab-shell%2Fissues%2F12%23note_906&user_id=1', + human_access: 'Owner', + path: '/gitlab-org/gitlab-shell/notes/906', + }, + ], + individual_note: true, + resolvable: false, + resolved: false, + diff_discussion: false, + }, +]; + +export const collapsedSystemNotes = [ + { + id: '39b271c2033e9ed43d8edb393702f65f7a830459', + reply_id: '39b271c2033e9ed43d8edb393702f65f7a830459', + expanded: true, + notes: [ + { + id: 901, + type: null, + attachment: null, + author: { + id: 1, + name: 'Administrator', + username: 'root', + state: 'active', + avatar_url: + 'https://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon', + path: '/root', + }, + created_at: '2018-05-29T12:05:36.117Z', + updated_at: '2018-05-29T12:05:36.117Z', + system: false, + noteable_id: 182, + noteable_type: 'Issue', + resolvable: false, + noteable_iid: 12, + note: + 'Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.', + note_html: + '<p dir="auto">Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.</p>', + current_user: { can_edit: true, can_award_emoji: true }, + resolved: false, + resolved_by: null, + discussion_id: '39b271c2033e9ed43d8edb393702f65f7a830459', + emoji_awardable: true, + award_emoji: [], + report_abuse_path: + '/abuse_reports/new?ref_url=http%3A%2F%2Flocalhost%3A3000%2Fgitlab-org%2Fgitlab-shell%2Fissues%2F12%23note_901&user_id=1', + human_access: 'Owner', + toggle_award_path: '/gitlab-org/gitlab-shell/notes/901/toggle_award_emoji', + path: '/gitlab-org/gitlab-shell/notes/901', + }, + ], + individual_note: true, + resolvable: false, + resolved: false, + diff_discussion: false, + }, + { + id: '4852335d7dc40b9ceb8fde1a2bb9c1b67e4c7795', + reply_id: '4852335d7dc40b9ceb8fde1a2bb9c1b67e4c7795', + expanded: true, + notes: [ + { + id: 902, + type: null, + attachment: null, + author: { + id: 1, + name: 'Administrator', + username: 'root', + state: 'active', + avatar_url: + 'https://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon', + path: '/root', + }, + created_at: '2018-05-29T12:05:58.694Z', + updated_at: '2018-05-29T12:05:58.694Z', + system: false, + noteable_id: 182, + noteable_type: 'Issue', + resolvable: false, + noteable_iid: 12, + note: + 'Varius vel pharetra vel turpis nunc eget lorem. Ipsum dolor sit amet consectetur adipiscing.', + note_html: + '<p dir="auto">Varius vel pharetra vel turpis nunc eget lorem. Ipsum dolor sit amet consectetur adipiscing.</p>', + current_user: { can_edit: true, can_award_emoji: true }, + resolved: false, + resolved_by: null, + discussion_id: '4852335d7dc40b9ceb8fde1a2bb9c1b67e4c7795', + emoji_awardable: true, + award_emoji: [], + report_abuse_path: + '/abuse_reports/new?ref_url=http%3A%2F%2Flocalhost%3A3000%2Fgitlab-org%2Fgitlab-shell%2Fissues%2F12%23note_902&user_id=1', + human_access: 'Owner', + toggle_award_path: '/gitlab-org/gitlab-shell/notes/902/toggle_award_emoji', + path: '/gitlab-org/gitlab-shell/notes/902', + }, + ], + individual_note: true, + resolvable: false, + resolved: false, + diff_discussion: false, + }, + { + id: '091865fe3ae20f0045234a3d103e3b15e73405b5', + reply_id: '091865fe3ae20f0045234a3d103e3b15e73405b5', + expanded: true, + notes: [ + { + id: 904, + type: null, + attachment: null, + author: { + id: 1, + name: 'Administrator', + username: 'root', + state: 'active', + avatar_url: + 'https://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon', + path: '/root', + }, + created_at: '2018-05-29T12:06:16.112Z', + updated_at: '2018-05-29T12:06:16.112Z', + system: false, + noteable_id: 182, + noteable_type: 'Issue', + resolvable: false, + noteable_iid: 12, + note: 'Ullamcorper eget nulla facilisi etiam', + note_html: '<p dir="auto">Ullamcorper eget nulla facilisi etiam</p>', + current_user: { can_edit: true, can_award_emoji: true }, + resolved: false, + resolved_by: null, + discussion_id: '091865fe3ae20f0045234a3d103e3b15e73405b5', + emoji_awardable: true, + award_emoji: [], + report_abuse_path: + '/abuse_reports/new?ref_url=http%3A%2F%2Flocalhost%3A3000%2Fgitlab-org%2Fgitlab-shell%2Fissues%2F12%23note_904&user_id=1', + human_access: 'Owner', + toggle_award_path: '/gitlab-org/gitlab-shell/notes/904/toggle_award_emoji', + path: '/gitlab-org/gitlab-shell/notes/904', + }, + ], + individual_note: true, + resolvable: false, + resolved: false, + diff_discussion: false, + }, + { + id: 'a21cf2e804acc3c60d07e37d75e395f5a9a4d044', + reply_id: 'a21cf2e804acc3c60d07e37d75e395f5a9a4d044', + expanded: true, + notes: [ + { + id: 905, + type: null, + attachment: null, + author: { + id: 1, + name: 'Administrator', + username: 'root', + state: 'active', + avatar_url: + 'https://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon', + path: '/root', + }, + created_at: '2018-05-29T12:06:28.851Z', + updated_at: '2018-05-29T12:06:28.851Z', + system: true, + noteable_id: 182, + noteable_type: 'Issue', + resolvable: false, + noteable_iid: 12, + note: 'changed the description', + note_html: '\n <p dir="auto">changed the description 2 times within 1 minute </p>', + current_user: { can_edit: false, can_award_emoji: true }, + resolved: false, + resolved_by: null, + system_note_icon_name: 'pencil-square', + discussion_id: 'a21cf2e804acc3c60d07e37d75e395f5a9a4d044', + emoji_awardable: false, + report_abuse_path: + '/abuse_reports/new?ref_url=http%3A%2F%2Flocalhost%3A3000%2Fgitlab-org%2Fgitlab-shell%2Fissues%2F12%23note_905&user_id=1', + human_access: 'Owner', + path: '/gitlab-org/gitlab-shell/notes/905', + times_updated: 2, + }, + ], + individual_note: true, + resolvable: false, + resolved: false, + diff_discussion: false, + }, + { + id: '70411b08cdfc01f24187a06d77daa33464cb2620', + reply_id: '70411b08cdfc01f24187a06d77daa33464cb2620', + expanded: true, + notes: [ + { + id: 906, + type: null, + attachment: null, + author: { + id: 1, + name: 'Administrator', + username: 'root', + state: 'active', + avatar_url: + 'https://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon', + path: '/root', + }, + created_at: '2018-05-29T12:20:02.925Z', + updated_at: '2018-05-29T12:20:02.925Z', + system: true, + noteable_id: 182, + noteable_type: 'Issue', + resolvable: false, + noteable_iid: 12, + note: 'changed the description', + note_html: '<p dir="auto">changed the description</p>', + current_user: { can_edit: false, can_award_emoji: true }, + resolved: false, + resolved_by: null, + system_note_icon_name: 'pencil-square', + discussion_id: '70411b08cdfc01f24187a06d77daa33464cb2620', + emoji_awardable: false, + report_abuse_path: + '/abuse_reports/new?ref_url=http%3A%2F%2Flocalhost%3A3000%2Fgitlab-org%2Fgitlab-shell%2Fissues%2F12%23note_906&user_id=1', + human_access: 'Owner', + path: '/gitlab-org/gitlab-shell/notes/906', + }, + ], + individual_note: true, + resolvable: false, + resolved: false, + diff_discussion: false, + }, +]; diff --git a/spec/javascripts/notes/stores/collapse_utils_spec.js b/spec/javascripts/notes/stores/collapse_utils_spec.js new file mode 100644 index 00000000000..06a6aab932a --- /dev/null +++ b/spec/javascripts/notes/stores/collapse_utils_spec.js @@ -0,0 +1,46 @@ +import { + isDescriptionSystemNote, + changeDescriptionNote, + getTimeDifferenceMinutes, + collapseSystemNotes, +} from '~/notes/stores/collapse_utils'; +import { + notesWithDescriptionChanges, + collapsedSystemNotes, +} from '../mock_data'; + +describe('Collapse utils', () => { + const mockSystemNote = { + note: 'changed the description', + note_html: '<p dir="auto">changed the description</p>', + system: true, + created_at: '2018-05-14T21:28:00.000Z', + }; + + it('checks if a system note is of a description type', () => { + expect(isDescriptionSystemNote(mockSystemNote)).toEqual(true); + }); + + it('returns false when a system note is not a description type', () => { + expect(isDescriptionSystemNote(Object.assign({}, mockSystemNote, { note: 'foo' }))).toEqual(false); + }); + + it('changes the description to contain the number of changed times', () => { + const changedNote = changeDescriptionNote(mockSystemNote, 3, 5); + + expect(changedNote.times_updated).toEqual(3); + expect(changedNote.note_html.trim()).toContain('<p dir="auto">changed the description 3 times within 5 minutes </p>'); + }); + + it('gets the time difference between two notes', () => { + const anotherSystemNote = { + created_at: '2018-05-14T21:33:00.000Z', + }; + + expect(getTimeDifferenceMinutes(mockSystemNote, anotherSystemNote)).toEqual(5); + }); + + it('collapses all description system notes made within 10 minutes or less from each other', () => { + expect(collapseSystemNotes(notesWithDescriptionChanges)).toEqual(collapsedSystemNotes); + }); +}); diff --git a/spec/javascripts/notes/stores/getters_spec.js b/spec/javascripts/notes/stores/getters_spec.js index 8b2a8d2cd7a..e5550580bf8 100644 --- a/spec/javascripts/notes/stores/getters_spec.js +++ b/spec/javascripts/notes/stores/getters_spec.js @@ -1,8 +1,9 @@ import * as getters from '~/notes/stores/getters'; -import { notesDataMock, userDataMock, noteableDataMock, individualNote } from '../mock_data'; +import { notesDataMock, userDataMock, noteableDataMock, individualNote, collapseNotesMock } from '../mock_data'; describe('Getters Notes Store', () => { let state; + beforeEach(() => { state = { notes: [individualNote], @@ -20,6 +21,22 @@ describe('Getters Notes Store', () => { }); }); + describe('Collapsed notes', () => { + const stateCollapsedNotes = { + notes: collapseNotesMock, + targetNoteHash: 'hash', + lastFetchedAt: 'timestamp', + + notesData: notesDataMock, + userData: userDataMock, + noteableData: noteableDataMock, + }; + + it('should return a single system note when a description was updated multiple times', () => { + expect(getters.notes(stateCollapsedNotes).length).toEqual(1); + }); + }); + describe('targetNoteHash', () => { it('should return `targetNoteHash`', () => { expect(getters.targetNoteHash(state)).toEqual('hash'); diff --git a/spec/javascripts/vue_shared/components/gl_modal_spec.js b/spec/javascripts/vue_shared/components/gl_modal_spec.js index 85cb1b90fc6..23be8d93b81 100644 --- a/spec/javascripts/vue_shared/components/gl_modal_spec.js +++ b/spec/javascripts/vue_shared/components/gl_modal_spec.js @@ -190,4 +190,37 @@ describe('GlModal', () => { }); }); }); + + describe('handling sizes', () => { + it('should render modal-sm', () => { + vm = mountComponent(modalComponent, { + modalSize: 'sm', + }); + + expect(vm.$el.querySelector('.modal-dialog').classList.contains('modal-sm')).toEqual(true); + }); + + it('should render modal-lg', () => { + vm = mountComponent(modalComponent, { + modalSize: 'lg', + }); + + expect(vm.$el.querySelector('.modal-dialog').classList.contains('modal-lg')).toEqual(true); + }); + + it('should not add modal size classes when md size is passed', () => { + vm = mountComponent(modalComponent, { + modalSize: 'md', + }); + + expect(vm.$el.querySelector('.modal-dialog').classList.contains('modal-md')).toEqual(false); + }); + + it('should not add modal size classes by default', () => { + vm = mountComponent(modalComponent, {}); + + expect(vm.$el.querySelector('.modal-dialog').classList.contains('modal-sm')).toEqual(false); + expect(vm.$el.querySelector('.modal-dialog').classList.contains('modal-lg')).toEqual(false); + }); + }); }); diff --git a/spec/javascripts/vue_shared/components/sidebar/labels_select/dropdown_value_spec.js b/spec/javascripts/vue_shared/components/sidebar/labels_select/dropdown_value_spec.js index 4397b00acfa..370a296bd8f 100644 --- a/spec/javascripts/vue_shared/components/sidebar/labels_select/dropdown_value_spec.js +++ b/spec/javascripts/vue_shared/components/sidebar/labels_select/dropdown_value_spec.js @@ -82,7 +82,7 @@ describe('DropdownValueComponent', () => { }); it('renders label element with tooltip and styles based on label details', () => { - const labelEl = vm.$el.querySelector('a span.label.color-label'); + const labelEl = vm.$el.querySelector('a span.badge.color-label'); expect(labelEl).not.toBeNull(); expect(labelEl.dataset.placement).toBe('bottom'); expect(labelEl.dataset.container).toBe('body'); diff --git a/spec/javascripts/vue_shared/components/table_pagination_spec.js b/spec/javascripts/vue_shared/components/table_pagination_spec.js index c63f15e5880..c36b607a34e 100644 --- a/spec/javascripts/vue_shared/components/table_pagination_spec.js +++ b/spec/javascripts/vue_shared/components/table_pagination_spec.js @@ -51,7 +51,7 @@ describe('Pagination component', () => { expect( component.$el.querySelector('.js-previous-button').classList.contains('disabled'), - ).toEqual(true); + ).toEqual(true); component.$el.querySelector('.js-previous-button a').click(); diff --git a/spec/lib/backup/files_spec.rb b/spec/lib/backup/files_spec.rb index 99872211a4e..63f2298357f 100644 --- a/spec/lib/backup/files_spec.rb +++ b/spec/lib/backup/files_spec.rb @@ -46,7 +46,9 @@ describe Backup::Files do end it 'calls tar command with unlink' do - expect(subject).to receive(:run_pipeline!).with([%w(gzip -cd), %w(tar --unlink-first --recursive-unlink -C /var/gitlab-registry -xf -)], any_args) + expect(subject).to receive(:tar).and_return('blabla-tar') + + expect(subject).to receive(:run_pipeline!).with([%w(gzip -cd), %w(blabla-tar --unlink-first --recursive-unlink -C /var/gitlab-registry -xf -)], any_args) subject.restore end end diff --git a/spec/lib/backup/manager_spec.rb b/spec/lib/backup/manager_spec.rb index 23c04a1a101..ca319679e80 100644 --- a/spec/lib/backup/manager_spec.rb +++ b/spec/lib/backup/manager_spec.rb @@ -274,16 +274,13 @@ describe Backup::Manager do } ) - # the Fog mock only knows about directories we create explicitly Fog.mock! + + # the Fog mock only knows about directories we create explicitly connection = ::Fog::Storage.new(Gitlab.config.backup.upload.connection.symbolize_keys) connection.directories.create(key: Gitlab.config.backup.upload.remote_directory) end - after do - Fog.unmock! - end - context 'target path' do it 'uses the tar filename by default' do expect_any_instance_of(Fog::Collection).to receive(:create) diff --git a/spec/lib/backup/repository_spec.rb b/spec/lib/backup/repository_spec.rb index f583b2021a2..92a27e308d2 100644 --- a/spec/lib/backup/repository_spec.rb +++ b/spec/lib/backup/repository_spec.rb @@ -34,7 +34,9 @@ describe Backup::Repository do let(:timestamp) { Time.utc(2017, 3, 22) } let(:temp_dirs) do Gitlab.config.repositories.storages.map do |name, storage| - File.join(storage.legacy_disk_path, '..', 'repositories.old.' + timestamp.to_i.to_s) + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + File.join(storage.legacy_disk_path, '..', 'repositories.old.' + timestamp.to_i.to_s) + end end end diff --git a/spec/lib/gitlab/background_migration/archive_legacy_traces_spec.rb b/spec/lib/gitlab/background_migration/archive_legacy_traces_spec.rb new file mode 100644 index 00000000000..877c061d11b --- /dev/null +++ b/spec/lib/gitlab/background_migration/archive_legacy_traces_spec.rb @@ -0,0 +1,59 @@ +require 'spec_helper' + +describe Gitlab::BackgroundMigration::ArchiveLegacyTraces, :migration, schema: 20180529152628 do + include TraceHelpers + + let(:namespaces) { table(:namespaces) } + let(:projects) { table(:projects) } + let(:builds) { table(:ci_builds) } + let(:job_artifacts) { table(:ci_job_artifacts) } + + before do + namespaces.create!(id: 123, name: 'gitlab1', path: 'gitlab1') + projects.create!(id: 123, name: 'gitlab1', path: 'gitlab1', namespace_id: 123) + @build = builds.create!(id: 1, project_id: 123, status: 'success', type: 'Ci::Build') + end + + context 'when trace file exsits at the right place' do + before do + create_legacy_trace(@build, 'trace in file') + end + + it 'correctly archive legacy traces' do + expect(job_artifacts.count).to eq(0) + expect(File.exist?(legacy_trace_path(@build))).to be_truthy + + described_class.new.perform(1, 1) + + expect(job_artifacts.count).to eq(1) + expect(File.exist?(legacy_trace_path(@build))).to be_falsy + expect(File.read(archived_trace_path(job_artifacts.first))).to eq('trace in file') + end + end + + context 'when trace file does not exsits at the right place' do + it 'does not raise errors nor create job artifact' do + expect { described_class.new.perform(1, 1) }.not_to raise_error + + expect(job_artifacts.count).to eq(0) + end + end + + context 'when trace data exsits in database' do + before do + create_legacy_trace_in_db(@build, 'trace in db') + end + + it 'correctly archive legacy traces' do + expect(job_artifacts.count).to eq(0) + expect(@build.read_attribute(:trace)).not_to be_empty + + described_class.new.perform(1, 1) + + @build.reload + expect(job_artifacts.count).to eq(1) + expect(@build.read_attribute(:trace)).to be_nil + expect(File.read(archived_trace_path(job_artifacts.first))).to eq('trace in db') + end + end +end diff --git a/spec/lib/gitlab/bare_repository_import/importer_spec.rb b/spec/lib/gitlab/bare_repository_import/importer_spec.rb index 5c8a19a53bc..468f6ff6d24 100644 --- a/spec/lib/gitlab/bare_repository_import/importer_spec.rb +++ b/spec/lib/gitlab/bare_repository_import/importer_spec.rb @@ -20,6 +20,13 @@ describe Gitlab::BareRepositoryImport::Importer, repository: true do Rainbow.enabled = @rainbow end + around do |example| + # TODO migrate BareRepositoryImport https://gitlab.com/gitlab-org/gitaly/issues/953 + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + example.run + end + end + shared_examples 'importing a repository' do describe '.execute' do it 'creates a project for a repository in storage' do diff --git a/spec/lib/gitlab/bare_repository_import/repository_spec.rb b/spec/lib/gitlab/bare_repository_import/repository_spec.rb index 1504826c7a5..afd8f5da39f 100644 --- a/spec/lib/gitlab/bare_repository_import/repository_spec.rb +++ b/spec/lib/gitlab/bare_repository_import/repository_spec.rb @@ -62,8 +62,10 @@ describe ::Gitlab::BareRepositoryImport::Repository do before do gitlab_shell.create_repository(repository_storage, hashed_path) - repository = Rugged::Repository.new(repo_path) - repository.config['gitlab.fullpath'] = 'to/repo' + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + repository = Rugged::Repository.new(repo_path) + repository.config['gitlab.fullpath'] = 'to/repo' + end end after do diff --git a/spec/lib/gitlab/checks/change_access_spec.rb b/spec/lib/gitlab/checks/change_access_spec.rb index 48e9902027c..1cb8143a9e9 100644 --- a/spec/lib/gitlab/checks/change_access_spec.rb +++ b/spec/lib/gitlab/checks/change_access_spec.rb @@ -52,7 +52,7 @@ describe Gitlab::Checks::ChangeAccess do context 'with protected tag' do let!(:protected_tag) { create(:protected_tag, project: project, name: 'v*') } - context 'as master' do + context 'as maintainer' do before do project.add_master(user) end @@ -138,7 +138,7 @@ describe Gitlab::Checks::ChangeAccess do context 'if the user is not allowed to delete protected branches' do it 'raises an error' do - expect { subject.exec }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You are not allowed to delete protected branches from this project. Only a project master or owner can delete a protected branch.') + expect { subject.exec }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You are not allowed to delete protected branches from this project. Only a project maintainer or owner can delete a protected branch.') end end diff --git a/spec/lib/gitlab/ci/pipeline/chain/populate_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/populate_spec.rb index 4d7d6951a51..c5a4d9b4778 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/populate_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/populate_spec.rb @@ -42,6 +42,10 @@ describe Gitlab::Ci::Pipeline::Chain::Populate do it 'correctly assigns user' do expect(pipeline.builds).to all(have_attributes(user: user)) end + + it 'has pipeline iid' do + expect(pipeline.iid).to be > 0 + end end context 'when pipeline is empty' do @@ -68,6 +72,10 @@ describe Gitlab::Ci::Pipeline::Chain::Populate do expect(pipeline.errors.to_a) .to include 'No stages / jobs for this pipeline.' end + + it 'wastes pipeline iid' do + expect(InternalId.ci_pipelines.where(project_id: project.id).last.last_value).to be > 0 + end end context 'when pipeline has validation errors' do @@ -87,6 +95,10 @@ describe Gitlab::Ci::Pipeline::Chain::Populate do expect(pipeline.errors.to_a) .to include 'Failed to build the pipeline!' end + + it 'wastes pipeline iid' do + expect(InternalId.ci_pipelines.where(project_id: project.id).last.last_value).to be > 0 + end end context 'when there is a seed blocks present' do @@ -111,6 +123,12 @@ describe Gitlab::Ci::Pipeline::Chain::Populate do expect(pipeline.variables.first.key).to eq 'VAR' expect(pipeline.variables.first.value).to eq '123' end + + it 'has pipeline iid' do + step.perform! + + expect(pipeline.iid).to be > 0 + end end context 'when seeds block tries to persist some resources' do @@ -121,6 +139,12 @@ describe Gitlab::Ci::Pipeline::Chain::Populate do it 'raises exception' do expect { step.perform! }.to raise_error(ActiveRecord::RecordNotSaved) end + + it 'wastes pipeline iid' do + expect { step.perform! }.to raise_error + + expect(InternalId.ci_pipelines.where(project_id: project.id).last.last_value).to be > 0 + end end end @@ -132,22 +156,39 @@ describe Gitlab::Ci::Pipeline::Chain::Populate do end end - context 'when using only/except build policies' do - let(:config) do - { rspec: { script: 'rspec', stage: 'test', only: ['master'] }, - prod: { script: 'cap prod', stage: 'deploy', only: ['tags'] } } - end + context 'when variables policy is specified' do + shared_examples_for 'a correct pipeline' do + it 'populates pipeline according to used policies' do + step.perform! - let(:pipeline) do - build(:ci_pipeline, ref: 'master', config: config) + expect(pipeline.stages.size).to eq 1 + expect(pipeline.stages.first.builds.size).to eq 1 + expect(pipeline.stages.first.builds.first.name).to eq 'rspec' + end end - it 'populates pipeline according to used policies' do - step.perform! + context 'when using only/except build policies' do + let(:config) do + { rspec: { script: 'rspec', stage: 'test', only: ['master'] }, + prod: { script: 'cap prod', stage: 'deploy', only: ['tags'] } } + end + + let(:pipeline) do + build(:ci_pipeline, ref: 'master', config: config) + end - expect(pipeline.stages.size).to eq 1 - expect(pipeline.stages.first.builds.size).to eq 1 - expect(pipeline.stages.first.builds.first.name).to eq 'rspec' + it_behaves_like 'a correct pipeline' + + context 'when variables expression is specified' do + context 'when pipeline iid is the subject' do + let(:config) do + { rspec: { script: 'rspec', only: { variables: ["$CI_PIPELINE_IID == '1'"] } }, + prod: { script: 'cap prod', only: { variables: ["$CI_PIPELINE_IID == '1000'"] } } } + end + + it_behaves_like 'a correct pipeline' + end + end end end end diff --git a/spec/lib/gitlab/ci/pipeline/preloader_spec.rb b/spec/lib/gitlab/ci/pipeline/preloader_spec.rb index 477c7477df0..40dfd893465 100644 --- a/spec/lib/gitlab/ci/pipeline/preloader_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/preloader_spec.rb @@ -3,18 +3,47 @@ require 'spec_helper' describe Gitlab::Ci::Pipeline::Preloader do - describe '.preload' do - it 'preloads the author of every pipeline commit' do - commit = double(:commit) - pipeline = double(:pipeline, commit: commit) + let(:stage) { double(:stage) } + let(:commit) { double(:commit) } - expect(commit) - .to receive(:lazy_author) + let(:pipeline) do + double(:pipeline, commit: commit, stages: [stage]) + end + + describe '.preload!' do + context 'when preloading multiple commits' do + let(:project) { create(:project, :repository) } + + it 'preloads all commits once' do + expect(Commit).to receive(:decorate).once.and_call_original + + pipelines = [build_pipeline(ref: 'HEAD'), + build_pipeline(ref: 'HEAD~1')] + + described_class.preload!(pipelines) + end + + def build_pipeline(ref:) + build_stubbed(:ci_pipeline, project: project, sha: project.commit(ref).id) + end + end + + it 'preloads commit authors and number of warnings' do + expect(commit).to receive(:lazy_author) + expect(pipeline).to receive(:number_of_warnings) + expect(stage).to receive(:number_of_warnings) + + described_class.preload!([pipeline]) + end + + it 'returns original collection' do + allow(commit).to receive(:lazy_author) + allow(pipeline).to receive(:number_of_warnings) + allow(stage).to receive(:number_of_warnings) - expect(pipeline) - .to receive(:number_of_warnings) + pipelines = [pipeline, pipeline] - described_class.preload([pipeline]) + expect(described_class.preload!(pipelines)).to eq pipelines end end end diff --git a/spec/lib/gitlab/ci/trace_spec.rb b/spec/lib/gitlab/ci/trace_spec.rb index e9d755c2021..d6510649dba 100644 --- a/spec/lib/gitlab/ci/trace_spec.rb +++ b/spec/lib/gitlab/ci/trace_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe Gitlab::Ci::Trace, :clean_gitlab_redis_cache do +describe Gitlab::Ci::Trace, :clean_gitlab_redis_shared_state do let(:build) { create(:ci_build) } let(:trace) { described_class.new(build) } diff --git a/spec/lib/gitlab/cycle_analytics/usage_data_spec.rb b/spec/lib/gitlab/cycle_analytics/usage_data_spec.rb index 56a316318cb..a785b17f682 100644 --- a/spec/lib/gitlab/cycle_analytics/usage_data_spec.rb +++ b/spec/lib/gitlab/cycle_analytics/usage_data_spec.rb @@ -3,7 +3,12 @@ require 'spec_helper' describe Gitlab::CycleAnalytics::UsageData do describe '#to_json' do before do - Timecop.freeze do + # Since git commits only have second precision, round up to the + # nearest second to ensure we have accurate median and standard + # deviation calculations. + current_time = Time.at(Time.now.to_i) + + Timecop.freeze(current_time) do user = create(:user, :admin) projects = create_list(:project, 2, :repository) @@ -37,13 +42,7 @@ describe Gitlab::CycleAnalytics::UsageData do expected_values.each_pair do |op, value| expect(stage_values).to have_key(op) - - if op == :missing - expect(stage_values[op]).to eq(value) - else - # delta is used because of git timings that Timecop does not stub - expect(stage_values[op].to_i).to be_within(5).of(value.to_i) - end + expect(stage_values[op]).to eq(value) end end end @@ -58,8 +57,8 @@ describe Gitlab::CycleAnalytics::UsageData do missing: 0 }, plan: { - average: 2, - sd: 2, + average: 1, + sd: 0, missing: 0 }, code: { diff --git a/spec/lib/gitlab/diff/file_spec.rb b/spec/lib/gitlab/diff/file_spec.rb index 0588fe935c3..f0e83ccfc7a 100644 --- a/spec/lib/gitlab/diff/file_spec.rb +++ b/spec/lib/gitlab/diff/file_spec.rb @@ -470,56 +470,69 @@ describe Gitlab::Diff::File do end describe '#diff_hunk' do - let(:raw_diff) do - <<EOS -@@ -6,12 +6,18 @@ module Popen - - def popen(cmd, path=nil) - unless cmd.is_a?(Array) -- raise "System commands must be given as an array of strings" -+ raise RuntimeError, "System commands must be given as an array of strings" - end - - path ||= Dir.pwd -- vars = { "PWD" => path } -- options = { chdir: path } -+ -+ vars = { -+ "PWD" => path -+ } -+ -+ options = { -+ chdir: path -+ } - - unless File.directory?(path) - FileUtils.mkdir_p(path) -@@ -19,6 +25,7 @@ module Popen - - @cmd_output = "" - @cmd_status = 0 -+ - Open3.popen3(vars, *cmd, options) do |stdin, stdout, stderr, wait_thr| - @cmd_output << stdout.read - @cmd_output << stderr.read -EOS - end - - it 'returns raw diff up to given line index' do - allow(diff_file).to receive(:raw_diff) { raw_diff } - diff_line = instance_double(Gitlab::Diff::Line, index: 5) - - diff_hunk = <<EOS -@@ -6,12 +6,18 @@ module Popen - - def popen(cmd, path=nil) - unless cmd.is_a?(Array) -- raise "System commands must be given as an array of strings" -+ raise RuntimeError, "System commands must be given as an array of strings" - end -EOS - - expect(diff_file.diff_hunk(diff_line)).to eq(diff_hunk) + context 'when first line is a match' do + let(:raw_diff) do + <<~EOS + --- a/files/ruby/popen.rb + +++ b/files/ruby/popen.rb + @@ -6,12 +6,18 @@ module Popen + + def popen(cmd, path=nil) + unless cmd.is_a?(Array) + - raise "System commands must be given as an array of strings" + + raise RuntimeError, "System commands must be given as an array of strings" + end + EOS + end + + it 'returns raw diff up to given line index' do + allow(diff_file).to receive(:raw_diff) { raw_diff } + diff_line = instance_double(Gitlab::Diff::Line, index: 4) + + diff_hunk = <<~EOS + @@ -6,12 +6,18 @@ module Popen + + def popen(cmd, path=nil) + unless cmd.is_a?(Array) + - raise "System commands must be given as an array of strings" + + raise RuntimeError, "System commands must be given as an array of strings" + EOS + + expect(diff_file.diff_hunk(diff_line)).to eq(diff_hunk.strip) + end + end + + context 'when first line is not a match' do + let(:raw_diff) do + <<~EOS + @@ -1,4 +1,4 @@ + -Copyright (c) 2011-2017 GitLab B.V. + +Copyright (c) 2011-2019 GitLab B.V. + + With regard to the GitLab Software: + + @@ -9,17 +9,21 @@ to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + copies of the Software, and to permit persons to whom the Software is + furnished to do so, subject to the following conditions: + EOS + end + + it 'returns raw diff up to given line index' do + allow(diff_file).to receive(:raw_diff) { raw_diff } + diff_line = instance_double(Gitlab::Diff::Line, index: 5) + + diff_hunk = <<~EOS + -Copyright (c) 2011-2017 GitLab B.V. + +Copyright (c) 2011-2019 GitLab B.V. + + With regard to the GitLab Software: + + @@ -9,17 +9,21 @@ to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + copies of the Software, and to permit persons to whom the Software is + EOS + + expect(diff_file.diff_hunk(diff_line)).to eq(diff_hunk.strip) + end end end end diff --git a/spec/lib/gitlab/file_finder_spec.rb b/spec/lib/gitlab/file_finder_spec.rb index 07cb10e563e..d6d9e4001a3 100644 --- a/spec/lib/gitlab/file_finder_spec.rb +++ b/spec/lib/gitlab/file_finder_spec.rb @@ -3,27 +3,11 @@ require 'spec_helper' describe Gitlab::FileFinder do describe '#find' do let(:project) { create(:project, :public, :repository) } - let(:finder) { described_class.new(project, project.default_branch) } - it 'finds by name' do - results = finder.find('files') - - filename, blob = results.find { |_, blob| blob.filename == 'files/images/wm.svg' } - expect(filename).to eq('files/images/wm.svg') - expect(blob).to be_a(Gitlab::SearchResults::FoundBlob) - expect(blob.ref).to eq(finder.ref) - expect(blob.data).not_to be_empty - end - - it 'finds by content' do - results = finder.find('files') - - filename, blob = results.find { |_, blob| blob.filename == 'CHANGELOG' } - - expect(filename).to eq('CHANGELOG') - expect(blob).to be_a(Gitlab::SearchResults::FoundBlob) - expect(blob.ref).to eq(finder.ref) - expect(blob.data).not_to be_empty + it_behaves_like 'file finder' do + subject { described_class.new(project, project.default_branch) } + let(:expected_file_by_name) { 'files/images/wm.svg' } + let(:expected_file_by_content) { 'CHANGELOG' } end end end diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index 7a9621d9c78..1744db1b17e 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -159,6 +159,7 @@ describe Gitlab::Git::Repository, seed_helper: true do let(:feature2) { 'feature2' } around do |example| + # discover_default_branch will be moved to gitaly-ruby Gitlab::GitalyClient::StorageSettings.allow_disk_access do example.run end @@ -255,7 +256,7 @@ describe Gitlab::Git::Repository, seed_helper: true do let(:expected_path) { File.join(storage_path, cache_key, expected_filename) } let(:expected_prefix) { "gitlab-git-test-#{ref}-#{SeedRepo::LastCommit::ID}" } - subject(:metadata) { repository.archive_metadata(ref, storage_path, format, append_sha: append_sha) } + subject(:metadata) { repository.archive_metadata(ref, storage_path, 'gitlab-git-test', format, append_sha: append_sha) } it 'sets CommitId to the commit SHA' do expect(metadata['CommitId']).to eq(SeedRepo::LastCommit::ID) @@ -373,6 +374,7 @@ describe Gitlab::Git::Repository, seed_helper: true do context '#submodules' do around do |example| + # TODO #submodules will be removed, has been migrated to gitaly Gitlab::GitalyClient::StorageSettings.allow_disk_access do example.run end @@ -1055,6 +1057,7 @@ describe Gitlab::Git::Repository, seed_helper: true do describe "#rugged_commits_between" do around do |example| + # TODO #rugged_commits_between will be removed, has been migrated to gitaly Gitlab::GitalyClient::StorageSettings.allow_disk_access do example.run end @@ -1703,6 +1706,7 @@ describe Gitlab::Git::Repository, seed_helper: true do let(:refs) { ['deadbeef', SeedRepo::RubyBlob::ID, '909e6157199'] } around do |example| + # TODO #batch_existence isn't used anywhere, can we remove it? Gitlab::GitalyClient::StorageSettings.allow_disk_access do example.run end @@ -2002,6 +2006,18 @@ describe Gitlab::Git::Repository, seed_helper: true do expect(config).to include("fullpath = #{repository_path}") end end + + 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', '') + + expect(repository.gitaly_repository_client).not_to receive(:write_config) + + expect do + repository.write_config(full_path: 'foo/bar.git') + end.to raise_error(Gitlab::Git::Repository::NoRepository) + end + end end context "when gitaly_write_config is enabled" do diff --git a/spec/lib/gitlab/git/rev_list_spec.rb b/spec/lib/gitlab/git/rev_list_spec.rb index 32ec1e029c8..70e90659b0f 100644 --- a/spec/lib/gitlab/git/rev_list_spec.rb +++ b/spec/lib/gitlab/git/rev_list_spec.rb @@ -9,9 +9,11 @@ describe Gitlab::Git::RevList do end def stub_popen_rev_list(*additional_args, with_lazy_block: true, output:) + repo_path = Gitlab::GitalyClient::StorageSettings.allow_disk_access { repository.path } + params = [ args_for_popen(additional_args), - repository.path, + repo_path, {}, hash_including(lazy_block: with_lazy_block ? anything : nil) ] @@ -86,7 +88,7 @@ describe Gitlab::Git::RevList do context '#all_objects' do it 'fetches list of all pushed objects using rev-list' do - stub_popen_rev_list('--all', '--objects', output: "sha1\nsha2") + stub_popen_rev_list('--all', '--objects', '--filter=blob:limit=200', output: "sha1\nsha2") expect { |b| rev_list.all_objects(&b) }.to yield_with_args(%w[sha1 sha2]) end diff --git a/spec/lib/gitlab/git_access_spec.rb b/spec/lib/gitlab/git_access_spec.rb index dfffea7797f..0d5f6a0b576 100644 --- a/spec/lib/gitlab/git_access_spec.rb +++ b/spec/lib/gitlab/git_access_spec.rb @@ -552,7 +552,7 @@ describe Gitlab::GitAccess do it 'returns not found' do project.add_guest(user) repo = project.repository - FileUtils.rm_rf(repo.path) + Gitlab::GitalyClient::StorageSettings.allow_disk_access { FileUtils.rm_rf(repo.path) } # Sanity check for rm_rf expect(repo.exists?).to eq(false) @@ -750,20 +750,22 @@ describe Gitlab::GitAccess do def merge_into_protected_branch @protected_branch_merge_commit ||= begin - stub_git_hooks - project.repository.add_branch(user, unprotected_branch, 'feature') - target_branch = project.repository.lookup('feature') - source_branch = project.repository.create_file( - user, - 'filename', - 'This is the file content', - message: 'This is a good commit message', - branch_name: unprotected_branch) - rugged = project.repository.rugged - author = { email: "email@example.com", time: Time.now, name: "Example Git User" } - - merge_index = rugged.merge_commits(target_branch, source_branch) - Rugged::Commit.create(rugged, author: author, committer: author, message: "commit message", parents: [target_branch, source_branch], tree: merge_index.write_tree(rugged)) + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + stub_git_hooks + project.repository.add_branch(user, unprotected_branch, 'feature') + target_branch = project.repository.lookup('feature') + source_branch = project.repository.create_file( + user, + 'filename', + 'This is the file content', + message: 'This is a good commit message', + branch_name: unprotected_branch) + rugged = project.repository.rugged + author = { email: "email@example.com", time: Time.now, name: "Example Git User" } + + merge_index = rugged.merge_commits(target_branch, source_branch) + Rugged::Commit.create(rugged, author: author, committer: author, message: "commit message", parents: [target_branch, source_branch], tree: merge_index.write_tree(rugged)) + end end end diff --git a/spec/lib/gitlab/github_import/importer/issue_importer_spec.rb b/spec/lib/gitlab/github_import/importer/issue_importer_spec.rb index d34ca0b76b8..81fe97c1e49 100644 --- a/spec/lib/gitlab/github_import/importer/issue_importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer/issue_importer_spec.rb @@ -180,12 +180,12 @@ describe Gitlab::GithubImport::Importer::IssueImporter, :clean_gitlab_redis_cach allow(importer.user_finder) .to receive(:user_id_for) - .ordered.with(issue.assignees[0]) + .with(issue.assignees[0]) .and_return(4) allow(importer.user_finder) .to receive(:user_id_for) - .ordered.with(issue.assignees[1]) + .with(issue.assignees[1]) .and_return(5) expect(Gitlab::Database) diff --git a/spec/lib/gitlab/github_import/importer/lfs_object_importer_spec.rb b/spec/lib/gitlab/github_import/importer/lfs_object_importer_spec.rb new file mode 100644 index 00000000000..4857f2afbe2 --- /dev/null +++ b/spec/lib/gitlab/github_import/importer/lfs_object_importer_spec.rb @@ -0,0 +1,23 @@ +require 'spec_helper' + +describe Gitlab::GithubImport::Importer::LfsObjectImporter do + let(:project) { create(:project) } + let(:download_link) { "http://www.gitlab.com/lfs_objects/oid" } + + let(:github_lfs_object) do + Gitlab::GithubImport::Representation::LfsObject.new( + oid: 'oid', download_link: download_link + ) + end + + let(:importer) { described_class.new(github_lfs_object, project, nil) } + + describe '#execute' do + it 'calls the LfsDownloadService with the lfs object attributes' do + expect_any_instance_of(Projects::LfsPointers::LfsDownloadService) + .to receive(:execute).with('oid', download_link) + + importer.execute + end + end +end diff --git a/spec/lib/gitlab/github_import/importer/lfs_objects_importer_spec.rb b/spec/lib/gitlab/github_import/importer/lfs_objects_importer_spec.rb new file mode 100644 index 00000000000..5f5c6b803c0 --- /dev/null +++ b/spec/lib/gitlab/github_import/importer/lfs_objects_importer_spec.rb @@ -0,0 +1,94 @@ +require 'spec_helper' + +describe Gitlab::GithubImport::Importer::LfsObjectsImporter do + let(:project) { double(:project, id: 4, import_source: 'foo/bar') } + let(:client) { double(:client) } + let(:download_link) { "http://www.gitlab.com/lfs_objects/oid" } + + let(:github_lfs_object) { ['oid', download_link] } + + describe '#parallel?' do + it 'returns true when running in parallel mode' do + importer = described_class.new(project, client) + expect(importer).to be_parallel + end + + it 'returns false when running in sequential mode' do + importer = described_class.new(project, client, parallel: false) + expect(importer).not_to be_parallel + end + end + + describe '#execute' do + context 'when running in parallel mode' do + it 'imports lfs objects in parallel' do + importer = described_class.new(project, client) + + expect(importer).to receive(:parallel_import) + + importer.execute + end + end + + context 'when running in sequential mode' do + it 'imports lfs objects in sequence' do + importer = described_class.new(project, client, parallel: false) + + expect(importer).to receive(:sequential_import) + + importer.execute + end + end + end + + describe '#sequential_import' do + it 'imports each lfs object in sequence' do + importer = described_class.new(project, client, parallel: false) + lfs_object_importer = double(:lfs_object_importer) + + allow(importer) + .to receive(:each_object_to_import) + .and_yield(['oid', download_link]) + + expect(Gitlab::GithubImport::Importer::LfsObjectImporter) + .to receive(:new) + .with( + an_instance_of(Gitlab::GithubImport::Representation::LfsObject), + project, + client + ) + .and_return(lfs_object_importer) + + expect(lfs_object_importer).to receive(:execute) + + importer.sequential_import + end + end + + describe '#parallel_import' do + it 'imports each lfs object in parallel' do + importer = described_class.new(project, client) + + allow(importer) + .to receive(:each_object_to_import) + .and_yield(github_lfs_object) + + expect(Gitlab::GithubImport::ImportLfsObjectWorker) + .to receive(:perform_async) + .with(project.id, an_instance_of(Hash), an_instance_of(String)) + + waiter = importer.parallel_import + + expect(waiter).to be_an_instance_of(Gitlab::JobWaiter) + expect(waiter.jobs_remaining).to eq(1) + end + end + + describe '#collection_options' do + it 'returns an empty Hash' do + importer = described_class.new(project, client) + + expect(importer.collection_options).to eq({}) + end + end +end diff --git a/spec/lib/gitlab/github_import/importer/pull_request_importer_spec.rb b/spec/lib/gitlab/github_import/importer/pull_request_importer_spec.rb index 35f3fdf8304..6686b7ce0b5 100644 --- a/spec/lib/gitlab/github_import/importer/pull_request_importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer/pull_request_importer_spec.rb @@ -40,13 +40,19 @@ describe Gitlab::GithubImport::Importer::PullRequestImporter, :clean_gitlab_redi describe '#execute' do it 'imports the pull request' do + mr = double(:merge_request, id: 10) + expect(importer) .to receive(:create_merge_request) - .and_return(10) + .and_return([mr, false]) + + expect(importer) + .to receive(:insert_git_data) + .with(mr, false) expect_any_instance_of(Gitlab::GithubImport::IssuableFinder) .to receive(:cache_database_id) - .with(10) + .with(mr.id) importer.execute end @@ -99,18 +105,11 @@ describe Gitlab::GithubImport::Importer::PullRequestImporter, :clean_gitlab_redi importer.create_merge_request end - it 'returns the ID of the created merge request' do - id = importer.create_merge_request - - expect(id).to be_a_kind_of(Numeric) - end - - it 'creates the merge request diffs' do - importer.create_merge_request - - mr = project.merge_requests.take + it 'returns the created merge request' do + mr, exists = importer.create_merge_request - expect(mr.merge_request_diffs.exists?).to eq(true) + expect(mr).to be_instance_of(MergeRequest) + expect(exists).to eq(false) end end @@ -217,5 +216,65 @@ describe Gitlab::GithubImport::Importer::PullRequestImporter, :clean_gitlab_redi expect { importer.create_merge_request }.not_to raise_error end end + + context 'when the merge request already exists' do + before do + allow(importer.user_finder) + .to receive(:author_id_for) + .with(pull_request) + .and_return([user.id, true]) + + allow(importer.user_finder) + .to receive(:assignee_id_for) + .with(pull_request) + .and_return(user.id) + end + + it 'returns the existing merge request' do + mr1, exists1 = importer.create_merge_request + mr2, exists2 = importer.create_merge_request + + expect(mr2).to eq(mr1) + expect(exists1).to eq(false) + expect(exists2).to eq(true) + end + end + end + + describe '#insert_git_data' do + before do + allow(importer.milestone_finder) + .to receive(:id_for) + .with(pull_request) + .and_return(milestone.id) + + allow(importer.user_finder) + .to receive(:author_id_for) + .with(pull_request) + .and_return([user.id, true]) + + allow(importer.user_finder) + .to receive(:assignee_id_for) + .with(pull_request) + .and_return(user.id) + end + + it 'creates the merge request diffs' do + mr, exists = importer.create_merge_request + + importer.insert_git_data(mr, exists) + + expect(mr.merge_request_diffs.exists?).to eq(true) + end + + it 'creates the merge request diff commits' do + mr, exists = importer.create_merge_request + + importer.insert_git_data(mr, exists) + + diff = mr.merge_request_diffs.take + + expect(diff.merge_request_diff_commits.exists?).to eq(true) + 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 cc9e4b67e72..d8f01dcb76b 100644 --- a/spec/lib/gitlab/github_import/importer/repository_importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer/repository_importer_spec.rb @@ -14,7 +14,8 @@ describe Gitlab::GithubImport::Importer::RepositoryImporter do disk_path: 'foo', repository: repository, create_wiki: true, - import_state: import_state + import_state: import_state, + lfs_enabled?: true ) end diff --git a/spec/lib/gitlab/gpg_spec.rb b/spec/lib/gitlab/gpg_spec.rb index ab9a166db00..47f37cae98f 100644 --- a/spec/lib/gitlab/gpg_spec.rb +++ b/spec/lib/gitlab/gpg_spec.rb @@ -74,6 +74,19 @@ describe Gitlab::Gpg do email: 'nannie.bernhard@example.com' }]) end + + it 'rejects non UTF-8 names and addresses' do + public_key = double(:key) + fingerprints = double(:fingerprints) + email = "\xEEch@test.com".force_encoding('ASCII-8BIT') + uid = double(:uid, name: 'Test User', email: email) + raw_key = double(:raw_key, uids: [uid]) + allow(Gitlab::Gpg::CurrentKeyChain).to receive(:fingerprints_from_key).with(public_key).and_return(fingerprints) + allow(GPGME::Key).to receive(:find).with(:public, anything).and_return([raw_key]) + + user_infos = described_class.user_infos_from_key(public_key) + expect(user_infos).to eq([]) + end end describe '.current_home_dir' do diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index a129855dbd8..2ea66479c1b 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -257,6 +257,7 @@ project: - import_data - commit_statuses - pipelines +- stages - builds - runner_projects - runners diff --git a/spec/lib/gitlab/import_export/fork_spec.rb b/spec/lib/gitlab/import_export/fork_spec.rb index 17e06a6a83f..71fd5a51c3b 100644 --- a/spec/lib/gitlab/import_export/fork_spec.rb +++ b/spec/lib/gitlab/import_export/fork_spec.rb @@ -41,8 +41,10 @@ describe 'forked project import' do after do FileUtils.rm_rf(export_path) - FileUtils.rm_rf(project_with_repo.repository.path_to_repo) - FileUtils.rm_rf(project.repository.path_to_repo) + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + FileUtils.rm_rf(project_with_repo.repository.path_to_repo) + FileUtils.rm_rf(project.repository.path_to_repo) + end end it 'can access the MR' do diff --git a/spec/lib/gitlab/import_export/repo_restorer_spec.rb b/spec/lib/gitlab/import_export/repo_restorer_spec.rb index dc806d036ff..013b8895f67 100644 --- a/spec/lib/gitlab/import_export/repo_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/repo_restorer_spec.rb @@ -23,8 +23,10 @@ describe Gitlab::ImportExport::RepoRestorer do after do FileUtils.rm_rf(export_path) - FileUtils.rm_rf(project_with_repo.repository.path_to_repo) - FileUtils.rm_rf(project.repository.path_to_repo) + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + FileUtils.rm_rf(project_with_repo.repository.path_to_repo) + FileUtils.rm_rf(project.repository.path_to_repo) + end end it 'restores the repo successfully' do @@ -34,7 +36,9 @@ describe Gitlab::ImportExport::RepoRestorer do it 'has the webhooks' do restorer.restore - expect(Gitlab::Git::Hook.new('post-receive', project.repository.raw_repository)).to exist + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + expect(Gitlab::Git::Hook.new('post-receive', project.repository.raw_repository)).to exist + end end end end diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml index 5e6311441f8..4354dca25ea 100644 --- a/spec/lib/gitlab/import_export/safe_model_attributes.yml +++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml @@ -170,7 +170,7 @@ MergeRequest: - last_edited_by_id - head_pipeline_id - discussion_locked -- allow_maintainer_to_push +- allow_collaboration MergeRequestDiff: - id - state @@ -242,6 +242,7 @@ Ci::Pipeline: - config_source - failure_reason - protected +- iid Ci::Stage: - id - name diff --git a/spec/lib/gitlab/import_sources_spec.rb b/spec/lib/gitlab/import_sources_spec.rb index f2fa315e3ec..10341486512 100644 --- a/spec/lib/gitlab/import_sources_spec.rb +++ b/spec/lib/gitlab/import_sources_spec.rb @@ -91,4 +91,23 @@ describe Gitlab::ImportSources do end end end + + describe 'imports_repository? checker' do + let(:allowed_importers) { %w[github gitlab_project] } + + it 'fails if any importer other than the allowed ones implements this method' do + current_importers = described_class.values.select { |kind| described_class.importer(kind).try(:imports_repository?) } + not_allowed_importers = current_importers - allowed_importers + + expect(not_allowed_importers).to be_empty, failure_message(not_allowed_importers) + end + + def failure_message(importers_class_names) + <<-MSG + It looks like the #{importers_class_names.join(', ')} importers implements its own way to import the repository. + That means that the lfs object download must be handled for each of them. You can use 'LfsImportService' and + 'LfsDownloadService' to implement it. After that, add the importer name to the list of allowed importers in this spec. + MSG + end + end end diff --git a/spec/lib/gitlab/legacy_github_import/project_creator_spec.rb b/spec/lib/gitlab/legacy_github_import/project_creator_spec.rb index eb1b13704ea..972b17d5b12 100644 --- a/spec/lib/gitlab/legacy_github_import/project_creator_spec.rb +++ b/spec/lib/gitlab/legacy_github_import/project_creator_spec.rb @@ -44,16 +44,44 @@ describe Gitlab::LegacyGithubImport::ProjectCreator do end context 'when GitHub project is public' do - before do - allow_any_instance_of(ApplicationSetting).to receive(:default_project_visibility).and_return(Gitlab::VisibilityLevel::INTERNAL) - end - - it 'sets project visibility to the default project visibility' do + it 'sets project visibility to public' do repo.private = false project = service.execute - expect(project.visibility_level).to eq(Gitlab::VisibilityLevel::INTERNAL) + expect(project.visibility_level).to eq(Gitlab::VisibilityLevel::PUBLIC) + end + end + + context 'when visibility level is restricted' do + context 'when GitHub project is private' do + before do + stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::PRIVATE]) + allow_any_instance_of(ApplicationSetting).to receive(:default_project_visibility).and_return(Gitlab::VisibilityLevel::INTERNAL) + end + + it 'sets project visibility to the default project visibility' do + repo.private = true + + project = service.execute + + expect(project.visibility_level).to eq(Gitlab::VisibilityLevel::INTERNAL) + end + end + + context 'when GitHub project is public' do + before do + stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::PUBLIC]) + allow_any_instance_of(ApplicationSetting).to receive(:default_project_visibility).and_return(Gitlab::VisibilityLevel::INTERNAL) + end + + it 'sets project visibility to the default project visibility' do + repo.private = false + + project = service.execute + + expect(project.visibility_level).to eq(Gitlab::VisibilityLevel::INTERNAL) + end end end diff --git a/spec/lib/gitlab/path_regex_spec.rb b/spec/lib/gitlab/path_regex_spec.rb index a40330d853f..e90e0aba0a4 100644 --- a/spec/lib/gitlab/path_regex_spec.rb +++ b/spec/lib/gitlab/path_regex_spec.rb @@ -90,11 +90,13 @@ describe Gitlab::PathRegex do let(:routes_not_starting_in_wildcard) { routes_without_format.select { |p| p !~ %r{^/[:*]} } } let(:top_level_words) do - words = routes_not_starting_in_wildcard.map do |route| - route.split('/')[1] - end.compact - - (words + ee_top_level_words + files_in_public + Array(API::API.prefix.to_s)).uniq + routes_not_starting_in_wildcard + .map { |route| route.split('/')[1] } + .concat(ee_top_level_words) + .concat(files_in_public) + .concat(Array(API::API.prefix.to_s)) + .compact + .uniq end let(:ee_top_level_words) do diff --git a/spec/lib/gitlab/project_search_results_spec.rb b/spec/lib/gitlab/project_search_results_spec.rb index e3f705d2299..50224bde722 100644 --- a/spec/lib/gitlab/project_search_results_spec.rb +++ b/spec/lib/gitlab/project_search_results_spec.rb @@ -22,47 +22,57 @@ describe Gitlab::ProjectSearchResults do it { expect(results.query).to eq('hello world') } end - describe 'blob search' do - let(:project) { create(:project, :public, :repository) } - - subject(:results) { described_class.new(user, project, 'files').objects('blobs') } - - context 'when repository is disabled' do - let(:project) { create(:project, :public, :repository, :repository_disabled) } + shared_examples 'general blob search' do |entity_type, blob_kind| + let(:query) { 'files' } + subject(:results) { described_class.new(user, project, query).objects(blob_type) } - it 'hides blobs from members' do + context "when #{entity_type} is disabled" do + let(:project) { disabled_project } + it "hides #{blob_kind} from members" do project.add_reporter(user) is_expected.to be_empty end - it 'hides blobs from non-members' do + it "hides #{blob_kind} from non-members" do is_expected.to be_empty end end - context 'when repository is internal' do - let(:project) { create(:project, :public, :repository, :repository_private) } + context "when #{entity_type} is internal" do + let(:project) { private_project } - it 'finds blobs for members' do + it "finds #{blob_kind} for members" do project.add_reporter(user) is_expected.not_to be_empty end - it 'hides blobs from non-members' do + it "hides #{blob_kind} from non-members" do is_expected.to be_empty end end it 'finds by name' do - expect(results.map(&:first)).to include('files/images/wm.svg') + expect(results.map(&:first)).to include(expected_file_by_name) end it 'finds by content' do - blob = results.select { |result| result.first == "CHANGELOG" }.flatten.last + blob = results.select { |result| result.first == expected_file_by_content }.flatten.last - expect(blob.filename).to eq("CHANGELOG") + expect(blob.filename).to eq(expected_file_by_content) + end + end + + describe 'blob search' do + let(:project) { create(:project, :public, :repository) } + + it_behaves_like 'general blob search', 'repository', 'blobs' do + let(:blob_type) { 'blobs' } + let(:disabled_project) { create(:project, :public, :repository, :repository_disabled) } + let(:private_project) { create(:project, :public, :repository, :repository_private) } + let(:expected_file_by_name) { 'files/images/wm.svg' } + let(:expected_file_by_content) { 'CHANGELOG' } end describe 'parsing results' do @@ -189,40 +199,18 @@ describe Gitlab::ProjectSearchResults do describe 'wiki search' do let(:project) { create(:project, :public, :wiki_repo) } let(:wiki) { build(:project_wiki, project: project) } - let!(:wiki_page) { wiki.create_page('Title', 'Content') } - - subject(:results) { described_class.new(user, project, 'Content').objects('wiki_blobs') } - - context 'when wiki is disabled' do - let(:project) { create(:project, :public, :wiki_repo, :wiki_disabled) } - it 'hides wiki blobs from members' do - project.add_reporter(user) - - is_expected.to be_empty - end - - it 'hides wiki blobs from non-members' do - is_expected.to be_empty - end - end - - context 'when wiki is internal' do - let(:project) { create(:project, :public, :wiki_repo, :wiki_private) } - - it 'finds wiki blobs for guest' do - project.add_guest(user) - - is_expected.not_to be_empty - end - - it 'hides wiki blobs from non-members' do - is_expected.to be_empty - end + before do + wiki.create_page('Files/Title', 'Content') + wiki.create_page('CHANGELOG', 'Files example') end - it 'finds by content' do - expect(results).to include("master:Title.md\x001\x00Content\n") + it_behaves_like 'general blob search', 'wiki', 'wiki blobs' do + let(:blob_type) { 'wiki_blobs' } + let(:disabled_project) { create(:project, :public, :wiki_repo, :wiki_disabled) } + let(:private_project) { create(:project, :public, :wiki_repo, :wiki_private) } + let(:expected_file_by_name) { 'Files/Title.md' } + let(:expected_file_by_content) { 'CHANGELOG.md' } end end diff --git a/spec/lib/gitlab/shell_spec.rb b/spec/lib/gitlab/shell_spec.rb index bf6ee4b0b59..14eae22a2ec 100644 --- a/spec/lib/gitlab/shell_spec.rb +++ b/spec/lib/gitlab/shell_spec.rb @@ -405,7 +405,11 @@ describe Gitlab::Shell do describe '#create_repository' do shared_examples '#create_repository' do let(:repository_storage) { 'default' } - let(:repository_storage_path) { Gitlab.config.repositories.storages[repository_storage].legacy_disk_path } + let(:repository_storage_path) do + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + Gitlab.config.repositories.storages[repository_storage].legacy_disk_path + end + end let(:repo_name) { 'project/path' } let(:created_path) { File.join(repository_storage_path, repo_name + '.git') } diff --git a/spec/lib/gitlab/sql/glob_spec.rb b/spec/lib/gitlab/sql/glob_spec.rb index f0bb4294d62..1cf8935bfe3 100644 --- a/spec/lib/gitlab/sql/glob_spec.rb +++ b/spec/lib/gitlab/sql/glob_spec.rb @@ -35,8 +35,9 @@ describe Gitlab::SQL::Glob do value = query("SELECT #{quote(string)} LIKE #{pattern}") .rows.flatten.first + check = Gitlab.rails5? ? true : 't' case value - when 't', 1 + when check, 1 true else false diff --git a/spec/lib/gitlab/user_access_spec.rb b/spec/lib/gitlab/user_access_spec.rb index 97b6069f64d..0469d984a40 100644 --- a/spec/lib/gitlab/user_access_spec.rb +++ b/spec/lib/gitlab/user_access_spec.rb @@ -142,7 +142,7 @@ describe Gitlab::UserAccess do target_project: canonical_project, source_project: project, source_branch: 'awesome-feature', - allow_maintainer_to_push: true + allow_collaboration: true ) end diff --git a/spec/lib/gitlab/utils/override_spec.rb b/spec/lib/gitlab/utils/override_spec.rb index 7c97cee982a..fc08ebcfc6d 100644 --- a/spec/lib/gitlab/utils/override_spec.rb +++ b/spec/lib/gitlab/utils/override_spec.rb @@ -1,7 +1,13 @@ -require 'spec_helper' +require 'fast_spec_helper' describe Gitlab::Utils::Override do - let(:base) { Struct.new(:good) } + let(:base) do + Struct.new(:good) do + def self.good + 0 + end + end + end let(:derived) { Class.new(base).tap { |m| m.extend described_class } } let(:extension) { Module.new.tap { |m| m.extend described_class } } @@ -9,6 +15,14 @@ describe Gitlab::Utils::Override do let(:prepending_class) { base.tap { |m| m.prepend extension } } let(:including_class) { base.tap { |m| m.include extension } } + let(:prepending_class_methods) do + base.tap { |m| m.singleton_class.prepend extension } + end + + let(:extending_class_methods) do + base.tap { |m| m.extend extension } + end + let(:klass) { subject } def good(mod) @@ -36,7 +50,7 @@ describe Gitlab::Utils::Override do shared_examples 'checking as intended' do it 'checks ok for overriding method' do good(subject) - result = klass.new(0).good + result = instance.good expect(result).to eq(1) described_class.verify! @@ -45,7 +59,25 @@ describe Gitlab::Utils::Override do it 'raises NotImplementedError when it is not overriding anything' do expect do bad(subject) - klass.new(0).bad + instance.bad + described_class.verify! + end.to raise_error(NotImplementedError) + end + end + + shared_examples 'checking as intended, nothing was overridden' do + it 'raises NotImplementedError because it is not overriding it' do + expect do + good(subject) + instance.good + described_class.verify! + end.to raise_error(NotImplementedError) + end + + it 'raises NotImplementedError when it is not overriding anything' do + expect do + bad(subject) + instance.bad described_class.verify! end.to raise_error(NotImplementedError) end @@ -54,7 +86,7 @@ describe Gitlab::Utils::Override do shared_examples 'nothing happened' do it 'does not complain when it is overriding something' do good(subject) - result = klass.new(0).good + result = instance.good expect(result).to eq(1) described_class.verify! @@ -62,7 +94,7 @@ describe Gitlab::Utils::Override do it 'does not complain when it is not overriding anything' do bad(subject) - result = klass.new(0).bad + result = instance.bad expect(result).to eq(true) described_class.verify! @@ -75,83 +107,97 @@ describe Gitlab::Utils::Override do end describe '#override' do - context 'when STATIC_VERIFICATION is set' do - before do - stub_env('STATIC_VERIFICATION', 'true') - end + context 'when instance is klass.new(0)' do + let(:instance) { klass.new(0) } - context 'when subject is a class' do - subject { derived } + context 'when STATIC_VERIFICATION is set' do + before do + stub_env('STATIC_VERIFICATION', 'true') + end - it_behaves_like 'checking as intended' - end + context 'when subject is a class' do + subject { derived } + + it_behaves_like 'checking as intended' + end + + context 'when subject is a module, and class is prepending it' do + subject { extension } + let(:klass) { prepending_class } + + it_behaves_like 'checking as intended' + end - context 'when subject is a module, and class is prepending it' do - subject { extension } - let(:klass) { prepending_class } + context 'when subject is a module, and class is including it' do + subject { extension } + let(:klass) { including_class } - it_behaves_like 'checking as intended' + it_behaves_like 'checking as intended, nothing was overridden' + end end - context 'when subject is a module, and class is including it' do - subject { extension } - let(:klass) { including_class } + context 'when STATIC_VERIFICATION is not set' do + before do + stub_env('STATIC_VERIFICATION', nil) + end - it 'raises NotImplementedError because it is not overriding it' do - expect do - good(subject) - klass.new(0).good - described_class.verify! - end.to raise_error(NotImplementedError) + context 'when subject is a class' do + subject { derived } + + it_behaves_like 'nothing happened' end - it 'raises NotImplementedError when it is not overriding anything' do - expect do - bad(subject) - klass.new(0).bad - described_class.verify! - end.to raise_error(NotImplementedError) + context 'when subject is a module, and class is prepending it' do + subject { extension } + let(:klass) { prepending_class } + + it_behaves_like 'nothing happened' end - end - end - end - context 'when STATIC_VERIFICATION is not set' do - before do - stub_env('STATIC_VERIFICATION', nil) - end + context 'when subject is a module, and class is including it' do + subject { extension } + let(:klass) { including_class } - context 'when subject is a class' do - subject { derived } + it 'does not complain when it is overriding something' do + good(subject) + result = instance.good - it_behaves_like 'nothing happened' - end + expect(result).to eq(0) + described_class.verify! + end - context 'when subject is a module, and class is prepending it' do - subject { extension } - let(:klass) { prepending_class } + it 'does not complain when it is not overriding anything' do + bad(subject) + result = instance.bad - it_behaves_like 'nothing happened' + expect(result).to eq(true) + described_class.verify! + end + end + end end - context 'when subject is a module, and class is including it' do - subject { extension } - let(:klass) { including_class } + context 'when instance is klass' do + let(:instance) { klass } - it 'does not complain when it is overriding something' do - good(subject) - result = klass.new(0).good + context 'when STATIC_VERIFICATION is set' do + before do + stub_env('STATIC_VERIFICATION', 'true') + end - expect(result).to eq(0) - described_class.verify! - end + context 'when subject is a module, and class is prepending it' do + subject { extension } + let(:klass) { prepending_class_methods } - it 'does not complain when it is not overriding anything' do - bad(subject) - result = klass.new(0).bad + it_behaves_like 'checking as intended' + end - expect(result).to eq(true) - described_class.verify! + context 'when subject is a module, and class is extending it' do + subject { extension } + let(:klass) { extending_class_methods } + + it_behaves_like 'checking as intended, nothing was overridden' + end end end end diff --git a/spec/lib/gitlab/wiki_file_finder_spec.rb b/spec/lib/gitlab/wiki_file_finder_spec.rb new file mode 100644 index 00000000000..025d1203dc5 --- /dev/null +++ b/spec/lib/gitlab/wiki_file_finder_spec.rb @@ -0,0 +1,20 @@ +require 'spec_helper' + +describe Gitlab::WikiFileFinder do + describe '#find' do + let(:project) { create(:project, :public, :wiki_repo) } + let(:wiki) { build(:project_wiki, project: project) } + + before do + wiki.create_page('Files/Title', 'Content') + wiki.create_page('CHANGELOG', 'Files example') + end + + it_behaves_like 'file finder' do + subject { described_class.new(project, project.wiki.default_branch) } + + let(:expected_file_by_name) { 'Files/Title.md' } + let(:expected_file_by_content) { 'CHANGELOG.md' } + end + end +end diff --git a/spec/lib/object_storage/direct_upload_spec.rb b/spec/lib/object_storage/direct_upload_spec.rb new file mode 100644 index 00000000000..e0569218d78 --- /dev/null +++ b/spec/lib/object_storage/direct_upload_spec.rb @@ -0,0 +1,168 @@ +require 'spec_helper' + +describe ObjectStorage::DirectUpload do + let(:credentials) do + { + provider: 'AWS', + aws_access_key_id: 'AWS_ACCESS_KEY_ID', + aws_secret_access_key: 'AWS_SECRET_ACCESS_KEY' + } + end + + let(:storage_url) { 'https://uploads.s3.amazonaws.com/' } + + let(:bucket_name) { 'uploads' } + let(:object_name) { 'tmp/uploads/my-file' } + let(:maximum_size) { 1.gigabyte } + + let(:direct_upload) { described_class.new(credentials, bucket_name, object_name, has_length: has_length, maximum_size: maximum_size) } + + before do + Fog.unmock! + end + + describe '#has_length' do + context 'is known' do + let(:has_length) { true } + let(:maximum_size) { nil } + + it "maximum size is not required" do + expect { direct_upload }.not_to raise_error + end + end + + context 'is unknown' do + let(:has_length) { false } + + context 'and maximum size is specified' do + let(:maximum_size) { 1.gigabyte } + + it "does not raise an error" do + expect { direct_upload }.not_to raise_error + end + end + + context 'and maximum size is not specified' do + let(:maximum_size) { nil } + + it "raises an error" do + expect { direct_upload }.to raise_error /maximum_size has to be specified if length is unknown/ + end + end + end + end + + describe '#to_hash' do + subject { direct_upload.to_hash } + + shared_examples 'a valid upload' do + it "returns valid structure" do + expect(subject).to have_key(:Timeout) + expect(subject[:GetURL]).to start_with(storage_url) + expect(subject[:StoreURL]).to start_with(storage_url) + expect(subject[:DeleteURL]).to start_with(storage_url) + end + end + + shared_examples 'a valid upload with multipart data' do + before do + stub_object_storage_multipart_init(storage_url, "myUpload") + end + + it_behaves_like 'a valid upload' + + it "returns valid structure" do + expect(subject).to have_key(:MultipartUpload) + expect(subject[:MultipartUpload]).to have_key(:PartSize) + expect(subject[:MultipartUpload][:PartURLs]).to all(start_with(storage_url)) + expect(subject[:MultipartUpload][:PartURLs]).to all(include('uploadId=myUpload')) + expect(subject[:MultipartUpload][:CompleteURL]).to start_with(storage_url) + expect(subject[:MultipartUpload][:CompleteURL]).to include('uploadId=myUpload') + expect(subject[:MultipartUpload][:AbortURL]).to start_with(storage_url) + expect(subject[:MultipartUpload][:AbortURL]).to include('uploadId=myUpload') + end + end + + shared_examples 'a valid upload without multipart data' do + it_behaves_like 'a valid upload' + + it "returns valid structure" do + expect(subject).not_to have_key(:MultipartUpload) + end + end + + context 'when AWS is used' do + context 'when length is known' do + let(:has_length) { true } + + it_behaves_like 'a valid upload without multipart data' + end + + context 'when length is unknown' do + let(:has_length) { false } + + it_behaves_like 'a valid upload with multipart data' do + context 'when maximum upload size is 10MB' do + let(:maximum_size) { 10.megabyte } + + it 'returns only 2 parts' do + expect(subject[:MultipartUpload][:PartURLs].length).to eq(2) + end + + it 'part size is mimimum, 5MB' do + expect(subject[:MultipartUpload][:PartSize]).to eq(5.megabyte) + end + end + + context 'when maximum upload size is 12MB' do + let(:maximum_size) { 12.megabyte } + + it 'returns only 3 parts' do + expect(subject[:MultipartUpload][:PartURLs].length).to eq(3) + end + + it 'part size is rounded-up to 5MB' do + expect(subject[:MultipartUpload][:PartSize]).to eq(5.megabyte) + end + end + + context 'when maximum upload size is 49GB' do + let(:maximum_size) { 49.gigabyte } + + it 'returns maximum, 100 parts' do + expect(subject[:MultipartUpload][:PartURLs].length).to eq(100) + end + + it 'part size is rounded-up to 5MB' do + expect(subject[:MultipartUpload][:PartSize]).to eq(505.megabyte) + end + end + end + end + end + + context 'when Google is used' do + let(:credentials) do + { + provider: 'Google', + google_storage_access_key_id: 'GOOGLE_ACCESS_KEY_ID', + google_storage_secret_access_key: 'GOOGLE_SECRET_ACCESS_KEY' + } + end + + let(:storage_url) { 'https://storage.googleapis.com/uploads/' } + + context 'when length is known' do + let(:has_length) { true } + + it_behaves_like 'a valid upload without multipart data' + end + + context 'when length is unknown' do + let(:has_length) { false } + + it_behaves_like 'a valid upload without multipart data' + end + end + end +end diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb index 69eafbe4bbe..775ca4ba0eb 100644 --- a/spec/mailers/notify_spec.rb +++ b/spec/mailers/notify_spec.rb @@ -68,7 +68,7 @@ describe Notify do end it 'contains the description' do - is_expected.to have_html_escaped_body_text issue.description + is_expected.to have_body_text issue.description end it 'does not add a reason header' do @@ -89,7 +89,7 @@ describe Notify do end it 'contains a link to note author' do - is_expected.to have_html_escaped_body_text(issue.author_name) + is_expected.to have_body_text(issue.author_name) is_expected.to have_body_text 'created an issue:' end end @@ -115,8 +115,8 @@ describe Notify do it 'has the correct subject and body' do aggregate_failures do is_expected.to have_referable_subject(issue, reply: true) - is_expected.to have_html_escaped_body_text(previous_assignee.name) - is_expected.to have_html_escaped_body_text(assignee.name) + is_expected.to have_body_text(previous_assignee.name) + is_expected.to have_body_text(assignee.name) is_expected.to have_body_text(project_issue_path(project, issue)) end end @@ -190,7 +190,7 @@ describe Notify do aggregate_failures do is_expected.to have_referable_subject(issue, reply: true) is_expected.to have_body_text(status) - is_expected.to have_html_escaped_body_text(current_user.name) + is_expected.to have_body_text(current_user.name) is_expected.to have_body_text(project_issue_path project, issue) end end @@ -243,7 +243,7 @@ describe Notify do end it 'contains the description' do - is_expected.to have_html_escaped_body_text merge_request.description + is_expected.to have_body_text merge_request.description end context 'when sent with a reason' do @@ -260,7 +260,7 @@ describe Notify do end it 'contains a link to note author' do - is_expected.to have_html_escaped_body_text merge_request.author_name + is_expected.to have_body_text merge_request.author_name is_expected.to have_body_text 'created a merge request:' end end @@ -286,9 +286,9 @@ describe Notify do it 'has the correct subject and body' do aggregate_failures do is_expected.to have_referable_subject(merge_request, reply: true) - is_expected.to have_html_escaped_body_text(previous_assignee.name) + is_expected.to have_body_text(previous_assignee.name) is_expected.to have_body_text(project_merge_request_path(project, merge_request)) - is_expected.to have_html_escaped_body_text(assignee.name) + is_expected.to have_body_text(assignee.name) end end @@ -358,7 +358,7 @@ describe Notify do aggregate_failures do is_expected.to have_referable_subject(merge_request, reply: true) is_expected.to have_body_text(status) - is_expected.to have_html_escaped_body_text(current_user.name) + is_expected.to have_body_text(current_user.name) is_expected.to have_body_text(project_merge_request_path(project, merge_request)) end end @@ -526,7 +526,7 @@ describe Notify do it 'has the correct subject and body' do is_expected.to have_referable_subject(project_snippet, reply: true) - is_expected.to have_html_escaped_body_text project_snippet_note.note + is_expected.to have_body_text project_snippet_note.note end end @@ -539,7 +539,7 @@ describe Notify do it 'has the correct subject and body' do is_expected.to have_subject("#{project.name} | Project was moved") - is_expected.to have_html_escaped_body_text project.full_name + is_expected.to have_body_text project.full_name is_expected.to have_body_text(project.ssh_url_to_repo) end end @@ -566,7 +566,7 @@ describe Notify do expect(to_emails).to eq([recipient.notification_email]) is_expected.to have_subject "Request to join the #{project.full_name} project" - is_expected.to have_html_escaped_body_text project.full_name + is_expected.to have_body_text project.full_name is_expected.to have_body_text project_project_members_url(project) is_expected.to have_body_text project_member.human_access end @@ -586,7 +586,7 @@ describe Notify do it 'contains all the useful information' do is_expected.to have_subject "Access to the #{project.full_name} project was denied" - is_expected.to have_html_escaped_body_text project.full_name + is_expected.to have_body_text project.full_name is_expected.to have_body_text project.web_url end end @@ -603,7 +603,7 @@ describe Notify do it 'contains all the useful information' do is_expected.to have_subject "Access to the #{project.full_name} project was granted" - is_expected.to have_html_escaped_body_text project.full_name + is_expected.to have_body_text project.full_name is_expected.to have_body_text project.web_url is_expected.to have_body_text project_member.human_access end @@ -633,7 +633,7 @@ describe Notify do it 'contains all the useful information' do is_expected.to have_subject "Invitation to join the #{project.full_name} project" - is_expected.to have_html_escaped_body_text project.full_name + is_expected.to have_body_text project.full_name is_expected.to have_body_text project.full_name is_expected.to have_body_text project_member.human_access is_expected.to have_body_text project_member.invite_token @@ -657,10 +657,10 @@ describe Notify do it 'contains all the useful information' do is_expected.to have_subject 'Invitation accepted' - is_expected.to have_html_escaped_body_text project.full_name + is_expected.to have_body_text project.full_name is_expected.to have_body_text project.web_url is_expected.to have_body_text project_member.invite_email - is_expected.to have_html_escaped_body_text invited_user.name + is_expected.to have_body_text invited_user.name end end @@ -680,7 +680,7 @@ describe Notify do it 'contains all the useful information' do is_expected.to have_subject 'Invitation declined' - is_expected.to have_html_escaped_body_text project.full_name + is_expected.to have_body_text project.full_name is_expected.to have_body_text project.web_url is_expected.to have_body_text project_member.invite_email end @@ -932,7 +932,7 @@ describe Notify do end it 'contains the message from the note' do - is_expected.to have_html_escaped_body_text note.note + is_expected.to have_body_text note.note end it 'contains an introduction' do @@ -991,7 +991,7 @@ describe Notify do expect(to_emails).to eq([recipient.notification_email]) is_expected.to have_subject "Request to join the #{group.name} group" - is_expected.to have_html_escaped_body_text group.name + is_expected.to have_body_text group.name is_expected.to have_body_text group_group_members_url(group) is_expected.to have_body_text group_member.human_access end @@ -1010,7 +1010,7 @@ describe Notify do it 'contains all the useful information' do is_expected.to have_subject "Access to the #{group.name} group was denied" - is_expected.to have_html_escaped_body_text group.name + is_expected.to have_body_text group.name is_expected.to have_body_text group.web_url end end @@ -1026,7 +1026,7 @@ describe Notify do it 'contains all the useful information' do is_expected.to have_subject "Access to the #{group.name} group was granted" - is_expected.to have_html_escaped_body_text group.name + is_expected.to have_body_text group.name is_expected.to have_body_text group.web_url is_expected.to have_body_text group_member.human_access end @@ -1056,7 +1056,7 @@ describe Notify do it 'contains all the useful information' do is_expected.to have_subject "Invitation to join the #{group.name} group" - is_expected.to have_html_escaped_body_text group.name + is_expected.to have_body_text group.name is_expected.to have_body_text group.web_url is_expected.to have_body_text group_member.human_access is_expected.to have_body_text group_member.invite_token @@ -1080,10 +1080,10 @@ describe Notify do it 'contains all the useful information' do is_expected.to have_subject 'Invitation accepted' - is_expected.to have_html_escaped_body_text group.name + is_expected.to have_body_text group.name is_expected.to have_body_text group.web_url is_expected.to have_body_text group_member.invite_email - is_expected.to have_html_escaped_body_text invited_user.name + is_expected.to have_body_text invited_user.name end end @@ -1103,7 +1103,7 @@ describe Notify do it 'contains all the useful information' do is_expected.to have_subject 'Invitation declined' - is_expected.to have_html_escaped_body_text group.name + is_expected.to have_body_text group.name is_expected.to have_body_text group.web_url is_expected.to have_body_text group_member.invite_email end @@ -1396,7 +1396,7 @@ describe Notify do it 'has the correct subject and body' do is_expected.to have_referable_subject(personal_snippet, reply: true) - is_expected.to have_html_escaped_body_text personal_snippet_note.note + is_expected.to have_body_text personal_snippet_note.note end end end diff --git a/spec/migrations/change_default_value_for_dsa_key_restriction_spec.rb b/spec/migrations/change_default_value_for_dsa_key_restriction_spec.rb new file mode 100644 index 00000000000..7e61ab9b52e --- /dev/null +++ b/spec/migrations/change_default_value_for_dsa_key_restriction_spec.rb @@ -0,0 +1,33 @@ +require 'spec_helper' +require Rails.root.join('db', 'migrate', '20180531220618_change_default_value_for_dsa_key_restriction.rb') + +describe ChangeDefaultValueForDsaKeyRestriction, :migration do + let(:application_settings) { table(:application_settings) } + + before do + application_settings.create! + end + + it 'changes the default value for dsa_key_restriction' do + expect(application_settings.first.dsa_key_restriction).to eq(0) + + migrate! + + application_settings.reset_column_information + new_setting = application_settings.create! + + expect(application_settings.count).to eq(2) + expect(new_setting.dsa_key_restriction).to eq(-1) + end + + it 'changes the existing setting' do + setting = application_settings.last + + expect(setting.dsa_key_restriction).to eq(0) + + migrate! + + expect(application_settings.count).to eq(1) + expect(setting.reload.dsa_key_restriction).to eq(-1) + end +end diff --git a/spec/migrations/migrate_object_storage_upload_sidekiq_queue_spec.rb b/spec/migrations/migrate_object_storage_upload_sidekiq_queue_spec.rb new file mode 100644 index 00000000000..1ee6c440cf4 --- /dev/null +++ b/spec/migrations/migrate_object_storage_upload_sidekiq_queue_spec.rb @@ -0,0 +1,33 @@ +require 'spec_helper' +require Rails.root.join('db', 'post_migrate', '20180603190921_migrate_object_storage_upload_sidekiq_queue.rb') + +describe MigrateObjectStorageUploadSidekiqQueue, :sidekiq, :redis do + include Gitlab::Database::MigrationHelpers + + context 'when there are jobs in the queue' do + it 'correctly migrates queue when migrating up' do + Sidekiq::Testing.disable! do + stubbed_worker(queue: 'object_storage_upload').perform_async('Something', [1]) + stubbed_worker(queue: 'object_storage:object_storage_background_move').perform_async('Something', [1]) + + described_class.new.up + + expect(sidekiq_queue_length('object_storage_upload')).to eq 0 + expect(sidekiq_queue_length('object_storage:object_storage_background_move')).to eq 2 + end + end + end + + context 'when there are no jobs in the queues' do + it 'does not raise error when migrating up' do + expect { described_class.new.up }.not_to raise_error + end + end + + def stubbed_worker(queue:) + Class.new do + include Sidekiq::Worker + sidekiq_options queue: queue + end + end +end diff --git a/spec/migrations/schedule_to_archive_legacy_traces_spec.rb b/spec/migrations/schedule_to_archive_legacy_traces_spec.rb new file mode 100644 index 00000000000..d3eac3c45ea --- /dev/null +++ b/spec/migrations/schedule_to_archive_legacy_traces_spec.rb @@ -0,0 +1,45 @@ +require 'spec_helper' +require Rails.root.join('db', 'post_migrate', '20180529152628_schedule_to_archive_legacy_traces') + +describe ScheduleToArchiveLegacyTraces, :migration do + include TraceHelpers + + let(:namespaces) { table(:namespaces) } + let(:projects) { table(:projects) } + let(:builds) { table(:ci_builds) } + let(:job_artifacts) { table(:ci_job_artifacts) } + + before do + namespaces.create!(id: 123, name: 'gitlab1', path: 'gitlab1') + projects.create!(id: 123, name: 'gitlab1', path: 'gitlab1', namespace_id: 123) + @build_success = builds.create!(id: 1, project_id: 123, status: 'success', type: 'Ci::Build') + @build_failed = builds.create!(id: 2, project_id: 123, status: 'failed', type: 'Ci::Build') + @builds_canceled = builds.create!(id: 3, project_id: 123, status: 'canceled', type: 'Ci::Build') + @build_running = builds.create!(id: 4, project_id: 123, status: 'running', type: 'Ci::Build') + + create_legacy_trace(@build_success, 'This job is done') + create_legacy_trace(@build_failed, 'This job is done') + create_legacy_trace(@builds_canceled, 'This job is done') + create_legacy_trace(@build_running, 'This job is not done yet') + end + + it 'correctly archive legacy traces' do + expect(job_artifacts.count).to eq(0) + expect(File.exist?(legacy_trace_path(@build_success))).to be_truthy + expect(File.exist?(legacy_trace_path(@build_failed))).to be_truthy + expect(File.exist?(legacy_trace_path(@builds_canceled))).to be_truthy + expect(File.exist?(legacy_trace_path(@build_running))).to be_truthy + + migrate! + + expect(job_artifacts.count).to eq(3) + expect(File.exist?(legacy_trace_path(@build_success))).to be_falsy + expect(File.exist?(legacy_trace_path(@build_failed))).to be_falsy + expect(File.exist?(legacy_trace_path(@builds_canceled))).to be_falsy + expect(File.exist?(legacy_trace_path(@build_running))).to be_truthy + expect(File.exist?(archived_trace_path(job_artifacts.where(job_id: @build_success.id).first))).to be_truthy + expect(File.exist?(archived_trace_path(job_artifacts.where(job_id: @build_failed.id).first))).to be_truthy + expect(File.exist?(archived_trace_path(job_artifacts.where(job_id: @builds_canceled.id).first))).to be_truthy + expect(job_artifacts.where(job_id: @build_running.id)).not_to be_exist + end +end diff --git a/spec/models/application_setting/term_spec.rb b/spec/models/application_setting/term_spec.rb index 1eddf3c56ff..aa49594f4d1 100644 --- a/spec/models/application_setting/term_spec.rb +++ b/spec/models/application_setting/term_spec.rb @@ -12,4 +12,41 @@ describe ApplicationSetting::Term do expect(described_class.latest).to eq(terms) end end + + describe '#accepted_by_user?' do + let(:user) { create(:user) } + let(:term) { create(:term) } + + it 'is true when the user accepted the terms' do + accept_terms(term, user) + + expect(term.accepted_by_user?(user)).to be(true) + end + + it 'is false when the user declined the terms' do + decline_terms(term, user) + + expect(term.accepted_by_user?(user)).to be(false) + end + + it 'does not cause a query when the user accepted the current terms' do + accept_terms(term, user) + + expect { term.accepted_by_user?(user) }.not_to exceed_query_limit(0) + end + + it 'returns false if the currently accepted terms are different' do + accept_terms(create(:term), user) + + expect(term.accepted_by_user?(user)).to be(false) + end + + def accept_terms(term, user) + Users::RespondToTermsService.new(user, term).execute(accepted: true) + end + + def decline_terms(term, user) + Users::RespondToTermsService.new(user, term).execute(accepted: false) + end + end end diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb index 968267a6d24..3e6656e0f12 100644 --- a/spec/models/application_setting_spec.rb +++ b/spec/models/application_setting_spec.rb @@ -110,10 +110,9 @@ describe ApplicationSetting do # Upgraded databases will have this sort of content context 'repository_storages is a String, not an Array' do before do - setting.__send__(:raw_write_attribute, :repository_storages, 'default') + described_class.where(id: setting.id).update_all(repository_storages: 'default') end - it { expect(setting.repository_storages_before_type_cast).to eq('default') } it { expect(setting.repository_storages).to eq(['default']) } end diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index b5eac913639..0a0d7d3fea9 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -116,6 +116,26 @@ describe Ci::Build do end end + describe '.with_live_trace' do + subject { described_class.with_live_trace } + + context 'when build has live trace' do + let!(:build) { create(:ci_build, :success, :trace_live) } + + it 'selects the build' do + is_expected.to eq([build]) + end + end + + context 'when build does not have live trace' do + let!(:build) { create(:ci_build, :success, :trace_artifact) } + + it 'does not select the build' do + is_expected.to be_empty + end + end + end + describe '#actionize' do context 'when build is a created' do before do @@ -1559,6 +1579,7 @@ describe Ci::Build do { key: 'CI_PROJECT_NAMESPACE', value: project.namespace.full_path, public: true }, { key: 'CI_PROJECT_URL', value: project.web_url, public: true }, { key: 'CI_PROJECT_VISIBILITY', value: 'private', public: true }, + { key: 'CI_PIPELINE_IID', value: pipeline.iid.to_s, public: true }, { key: 'CI_CONFIG_PATH', value: pipeline.ci_yaml_file_path, public: true }, { key: 'CI_PIPELINE_SOURCE', value: pipeline.source, public: true }, { key: 'CI_COMMIT_MESSAGE', value: pipeline.git_commit_message, public: true }, @@ -2505,4 +2526,76 @@ describe Ci::Build do end end end + + describe 'pages deployments' do + set(:build) { create(:ci_build, project: project, user: user) } + + context 'when job is "pages"' do + before do + build.name = 'pages' + end + + context 'when pages are enabled' do + before do + allow(Gitlab.config.pages).to receive_messages(enabled: true) + end + + it 'is marked as pages generator' do + expect(build).to be_pages_generator + end + + context 'job succeeds' do + it "calls pages worker" do + expect(PagesWorker).to receive(:perform_async).with(:deploy, build.id) + + build.success! + end + end + + context 'job fails' do + it "does not call pages worker" do + expect(PagesWorker).not_to receive(:perform_async) + + build.drop! + end + end + end + + context 'when pages are disabled' do + before do + allow(Gitlab.config.pages).to receive_messages(enabled: false) + end + + it 'is not marked as pages generator' do + expect(build).not_to be_pages_generator + end + + context 'job succeeds' do + it "does not call pages worker" do + expect(PagesWorker).not_to receive(:perform_async) + + build.success! + end + end + end + end + + context 'when job is not "pages"' do + before do + build.name = 'other-job' + end + + it 'is not marked as pages generator' do + expect(build).not_to be_pages_generator + end + + context 'job succeeds' do + it "does not call pages worker" do + expect(PagesWorker).not_to receive(:perform_async) + + build.success + end + end + end + end end diff --git a/spec/models/ci/group_spec.rb b/spec/models/ci/group_spec.rb index 51123e73fe6..838fa63cb1f 100644 --- a/spec/models/ci/group_spec.rb +++ b/spec/models/ci/group_spec.rb @@ -41,4 +41,55 @@ describe Ci::Group do end end end + + describe '.fabricate' do + let(:pipeline) { create(:ci_empty_pipeline) } + let(:stage) { create(:ci_stage_entity, pipeline: pipeline) } + + before do + create_build(:ci_build, name: 'rspec 0 2') + create_build(:ci_build, name: 'rspec 0 1') + create_build(:ci_build, name: 'spinach 0 1') + create_build(:commit_status, name: 'aaaaa') + end + + it 'returns an array of three groups' do + expect(stage.groups).to be_a Array + expect(stage.groups).to all(be_a described_class) + expect(stage.groups.size).to eq 3 + end + + it 'returns groups with correctly ordered statuses' do + expect(stage.groups.first.jobs.map(&:name)) + .to eq ['aaaaa'] + expect(stage.groups.second.jobs.map(&:name)) + .to eq ['rspec 0 1', 'rspec 0 2'] + expect(stage.groups.third.jobs.map(&:name)) + .to eq ['spinach 0 1'] + end + + it 'returns groups with correct names' do + expect(stage.groups.map(&:name)) + .to eq %w[aaaaa rspec spinach] + end + + context 'when a name is nil on legacy pipelines' do + before do + pipeline.builds.first.update_attribute(:name, nil) + end + + it 'returns an array of three groups' do + expect(stage.groups.map(&:name)) + .to eq ['', 'aaaaa', 'rspec', 'spinach'] + end + end + + def create_build(type, status: 'success', **opts) + create(type, pipeline: pipeline, + stage: stage.name, + status: status, + stage_id: stage.id, + **opts) + end + end end diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index f5295bec65b..2bae98dcbb8 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -35,6 +35,16 @@ describe Ci::Pipeline, :mailer do end end + describe 'modules' do + it_behaves_like 'AtomicInternalId', validate_presence: false do + let(:internal_id_attribute) { :iid } + let(:instance) { build(:ci_pipeline) } + let(:scope) { :project } + let(:scope_attrs) { { project: instance.project } } + let(:usage) { :ci_pipelines } + end + end + describe '#source' do context 'when creating new pipeline' do let(:pipeline) do @@ -195,7 +205,8 @@ describe Ci::Pipeline, :mailer do it 'includes all predefined variables in a valid order' do keys = subject.map { |variable| variable[:key] } - expect(keys).to eq %w[CI_CONFIG_PATH + expect(keys).to eq %w[CI_PIPELINE_IID + CI_CONFIG_PATH CI_PIPELINE_SOURCE CI_COMMIT_MESSAGE CI_COMMIT_TITLE @@ -526,6 +537,87 @@ describe Ci::Pipeline, :mailer do end end end + + describe '#stages' do + before do + create(:ci_stage_entity, project: project, + pipeline: pipeline, + name: 'build') + end + + it 'returns persisted stages' do + expect(pipeline.stages).not_to be_empty + expect(pipeline.stages).to all(be_persisted) + end + end + + describe '#ordered_stages' do + before do + create(:ci_stage_entity, project: project, + pipeline: pipeline, + position: 4, + name: 'deploy') + + create(:ci_build, project: project, + pipeline: pipeline, + stage: 'test', + stage_idx: 3, + name: 'test') + + create(:ci_build, project: project, + pipeline: pipeline, + stage: 'build', + stage_idx: 2, + name: 'build') + + create(:ci_stage_entity, project: project, + pipeline: pipeline, + position: 1, + name: 'sanity') + + create(:ci_stage_entity, project: project, + pipeline: pipeline, + position: 5, + name: 'cleanup') + end + + subject { pipeline.ordered_stages } + + context 'when using legacy stages' do + before do + stub_feature_flags(ci_pipeline_persisted_stages: false) + end + + it 'returns legacy stages in valid order' do + expect(subject.map(&:name)).to eq %w[build test] + end + end + + context 'when using persisted stages' do + before do + stub_feature_flags(ci_pipeline_persisted_stages: true) + end + + context 'when pipelines is not complete' do + it 'still returns legacy stages' do + expect(subject).to all(be_a Ci::LegacyStage) + expect(subject.map(&:name)).to eq %w[build test] + end + end + + context 'when pipeline is complete' do + before do + pipeline.succeed! + end + + it 'returns stages in valid order' do + expect(subject).to all(be_a Ci::Stage) + expect(subject.map(&:name)) + .to eq %w[sanity build test deploy cleanup] + end + end + end + end end describe 'state machine' do @@ -1170,6 +1262,43 @@ describe Ci::Pipeline, :mailer do end end + describe '#update_status' do + context 'when pipeline is empty' do + it 'updates does not change pipeline status' do + expect(pipeline.statuses.latest.status).to be_nil + + expect { pipeline.update_status } + .to change { pipeline.reload.status }.to 'skipped' + end + end + + context 'when updating status to pending' do + before do + allow(pipeline) + .to receive_message_chain(:statuses, :latest, :status) + .and_return(:running) + end + + it 'updates pipeline status to running' do + expect { pipeline.update_status } + .to change { pipeline.reload.status }.to 'running' + end + end + + context 'when statuses status was not recognized' do + before do + allow(pipeline) + .to receive(:latest_builds_status) + .and_return(:unknown) + end + + it 'raises an exception' do + expect { pipeline.update_status } + .to raise_error(HasStatus::UnknownStatusError) + end + end + end + describe '#detailed_status' do subject { pipeline.detailed_status(user) } diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index 0f072aa1719..f6433234573 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -549,7 +549,7 @@ describe Ci::Runner do end describe '#update_cached_info' do - let(:runner) { create(:ci_runner) } + let(:runner) { create(:ci_runner, :project) } subject { runner.update_cached_info(architecture: '18-bit') } @@ -570,17 +570,22 @@ describe Ci::Runner do runner.contacted_at = 2.hours.ago end - it 'updates database' do - expect_redis_update + context 'with invalid runner' do + before do + runner.projects = [] + end + + it 'still updates redis cache and database' do + expect(runner).to be_invalid - expect { subject }.to change { runner.reload.read_attribute(:contacted_at) } - .and change { runner.reload.read_attribute(:architecture) } + expect_redis_update + does_db_update + end end - it 'updates cache' do + it 'updates redis cache and database' do expect_redis_update - - subject + does_db_update end end @@ -590,6 +595,11 @@ describe Ci::Runner do expect(redis).to receive(:set).with(redis_key, anything, any_args) end end + + def does_db_update + expect { subject }.to change { runner.reload.read_attribute(:contacted_at) } + .and change { runner.reload.read_attribute(:architecture) } + end end describe '#destroy' do diff --git a/spec/models/ci/stage_spec.rb b/spec/models/ci/stage_spec.rb index a00db1d2bfc..22a4556c10c 100644 --- a/spec/models/ci/stage_spec.rb +++ b/spec/models/ci/stage_spec.rb @@ -65,7 +65,31 @@ describe Ci::Stage, :models do end end - context 'when stage is skipped' do + context 'when stage has only created builds' do + let(:stage) { create(:ci_stage_entity, status: :created) } + + before do + create(:ci_build, :created, stage_id: stage.id) + end + + it 'updates status to skipped' do + expect(stage.reload.status).to eq 'created' + end + end + + context 'when stage is skipped because of skipped builds' do + before do + create(:ci_build, :skipped, stage_id: stage.id) + end + + it 'updates status to skipped' do + expect { stage.update_status } + .to change { stage.reload.status } + .to 'skipped' + end + end + + context 'when stage is skipped because is empty' do it 'updates status to skipped' do expect { stage.update_status } .to change { stage.reload.status } @@ -86,9 +110,85 @@ describe Ci::Stage, :models do expect(stage.reload).to be_failed end end + + context 'when statuses status was not recognized' do + before do + allow(stage) + .to receive_message_chain(:statuses, :latest, :status) + .and_return(:unknown) + end + + it 'raises an exception' do + expect { stage.update_status } + .to raise_error(HasStatus::UnknownStatusError) + end + end + end + + describe '#detailed_status' do + using RSpec::Parameterized::TableSyntax + + let(:user) { create(:user) } + let(:stage) { create(:ci_stage_entity, status: :created) } + subject { stage.detailed_status(user) } + + where(:statuses, :label) do + %w[created] | :created + %w[success] | :passed + %w[pending] | :pending + %w[skipped] | :skipped + %w[canceled] | :canceled + %w[success failed] | :failed + %w[running pending] | :running + end + + with_them do + before do + statuses.each do |status| + create(:commit_status, project: stage.project, + pipeline: stage.pipeline, + stage_id: stage.id, + status: status) + + stage.update_status + end + end + + it 'has a correct label' do + expect(subject.label).to eq label.to_s + end + end + + context 'when stage has warnings' do + before do + create(:ci_build, project: stage.project, + pipeline: stage.pipeline, + stage_id: stage.id, + status: :failed, + allow_failure: true) + + stage.update_status + end + + it 'is passed with warnings' do + expect(subject.label).to eq 'passed with warnings' + end + end end - describe '#index' do + describe '#groups' do + before do + create(:ci_build, stage_id: stage.id, name: 'rspec 0 1') + create(:ci_build, stage_id: stage.id, name: 'rspec 0 2') + end + + it 'groups stage builds by name' do + expect(stage.groups).to be_one + expect(stage.groups.first.name).to eq 'rspec' + end + end + + describe '#position' do context 'when stage has been imported and does not have position index set' do before do stage.update_column(:position, nil) @@ -119,4 +219,42 @@ describe Ci::Stage, :models do end end end + + context 'when stage has warnings' do + before do + create(:ci_build, :failed, :allowed_to_fail, stage_id: stage.id) + end + + describe '#has_warnings?' do + it 'returns true' do + expect(stage).to have_warnings + end + end + + describe '#number_of_warnings' do + it 'returns a lazy stage warnings counter' do + lazy_queries = ActiveRecord::QueryRecorder.new do + stage.number_of_warnings + end + + synced_queries = ActiveRecord::QueryRecorder.new do + stage.number_of_warnings.to_i + end + + expect(lazy_queries.count).to eq 0 + expect(synced_queries.count).to eq 1 + + expect(stage.number_of_warnings.inspect).to include 'BatchLoader' + expect(stage.number_of_warnings).to eq 1 + end + end + end + + context 'when stage does not have warnings' do + describe '#has_warnings?' do + it 'returns false' do + expect(stage).not_to have_warnings + end + end + end end diff --git a/spec/models/deployment_spec.rb b/spec/models/deployment_spec.rb index aee70bcfb29..e01906f4b6c 100644 --- a/spec/models/deployment_spec.rb +++ b/spec/models/deployment_spec.rb @@ -20,6 +20,7 @@ describe Deployment do it_behaves_like 'AtomicInternalId' do let(:internal_id_attribute) { :iid } let(:instance) { build(:deployment) } + let(:scope) { :project } let(:scope_attrs) { { project: instance.project } } let(:usage) { :deployments } end diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index 128acf83686..e818fbeb9cf 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -17,6 +17,7 @@ describe Issue do it_behaves_like 'AtomicInternalId' do let(:internal_id_attribute) { :iid } let(:instance) { build(:issue) } + let(:scope) { :project } let(:scope_attrs) { { project: instance.project } } let(:usage) { :issues } end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 9ffa91fc265..3f028b3bd5c 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -16,7 +16,11 @@ describe MergeRequest do describe '#squash_in_progress?' do shared_examples 'checking whether a squash is in progress' do - let(:repo_path) { subject.source_project.repository.path } + let(:repo_path) do + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + subject.source_project.repository.path + end + end let(:squash_path) { File.join(repo_path, "gitlab-worktree", "squash-#{subject.id}") } before do @@ -84,6 +88,7 @@ describe MergeRequest do it_behaves_like 'AtomicInternalId' do let(:internal_id_attribute) { :iid } let(:instance) { build(:merge_request) } + let(:scope) { :target_project } let(:scope_attrs) { { project: instance.target_project } } let(:usage) { :merge_requests } end @@ -2196,7 +2201,11 @@ describe MergeRequest do describe '#rebase_in_progress?' do shared_examples 'checking whether a rebase is in progress' do - let(:repo_path) { subject.source_project.repository.path } + let(:repo_path) do + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + subject.source_project.repository.path + end + end let(:rebase_path) { File.join(repo_path, "gitlab-worktree", "rebase-#{subject.id}") } before do @@ -2236,25 +2245,25 @@ describe MergeRequest do end end - describe '#allow_maintainer_to_push' do + describe '#allow_collaboration' do let(:merge_request) do - build(:merge_request, source_branch: 'fixes', allow_maintainer_to_push: true) + build(:merge_request, source_branch: 'fixes', allow_collaboration: true) end it 'is false when pushing by a maintainer is not possible' do - expect(merge_request).to receive(:maintainer_push_possible?) { false } + expect(merge_request).to receive(:collaborative_push_possible?) { false } - expect(merge_request.allow_maintainer_to_push).to be_falsy + expect(merge_request.allow_collaboration).to be_falsy end it 'is true when pushing by a maintainer is possible' do - expect(merge_request).to receive(:maintainer_push_possible?) { true } + expect(merge_request).to receive(:collaborative_push_possible?) { true } - expect(merge_request.allow_maintainer_to_push).to be_truthy + expect(merge_request.allow_collaboration).to be_truthy end end - describe '#maintainer_push_possible?' do + describe '#collaborative_push_possible?' do let(:merge_request) do build(:merge_request, source_branch: 'fixes') end @@ -2266,14 +2275,14 @@ describe MergeRequest do it 'does not allow maintainer to push if the source project is the same as the target' do merge_request.target_project = merge_request.source_project = create(:project, :public) - expect(merge_request.maintainer_push_possible?).to be_falsy + expect(merge_request.collaborative_push_possible?).to be_falsy end it 'allows maintainer to push when both source and target are public' do merge_request.target_project = build(:project, :public) merge_request.source_project = build(:project, :public) - expect(merge_request.maintainer_push_possible?).to be_truthy + expect(merge_request.collaborative_push_possible?).to be_truthy end it 'is not available for protected branches' do @@ -2284,11 +2293,11 @@ describe MergeRequest do .with(merge_request.source_project, 'fixes') .and_return(true) - expect(merge_request.maintainer_push_possible?).to be_falsy + expect(merge_request.collaborative_push_possible?).to be_falsy end end - describe '#can_allow_maintainer_to_push?' do + describe '#can_allow_collaboration?' do let(:target_project) { create(:project, :public) } let(:source_project) { fork_project(target_project) } let(:merge_request) do @@ -2300,17 +2309,17 @@ describe MergeRequest do let(:user) { create(:user) } before do - allow(merge_request).to receive(:maintainer_push_possible?) { true } + allow(merge_request).to receive(:collaborative_push_possible?) { true } end it 'is false if the user does not have push access to the source project' do - expect(merge_request.can_allow_maintainer_to_push?(user)).to be_falsy + expect(merge_request.can_allow_collaboration?(user)).to be_falsy end it 'is true when the user has push access to the source project' do source_project.add_developer(user) - expect(merge_request.can_allow_maintainer_to_push?(user)).to be_truthy + expect(merge_request.can_allow_collaboration?(user)).to be_truthy end end diff --git a/spec/models/milestone_spec.rb b/spec/models/milestone_spec.rb index 4bb9717d33e..204d6b47832 100644 --- a/spec/models/milestone_spec.rb +++ b/spec/models/milestone_spec.rb @@ -6,6 +6,7 @@ describe Milestone do it_behaves_like 'AtomicInternalId' do let(:internal_id_attribute) { :iid } let(:instance) { build(:milestone, project: build(:project), group: nil) } + let(:scope) { :project } let(:scope_attrs) { { project: instance.project } } let(:usage) { :milestones } end @@ -15,6 +16,7 @@ describe Milestone do it_behaves_like 'AtomicInternalId' do let(:internal_id_attribute) { :iid } let(:instance) { build(:milestone, project: nil, group: build(:group)) } + let(:scope) { :group } let(:scope_attrs) { { namespace: instance.group } } let(:usage) { :milestones } end diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index 6f702d8d95e..18b01c3e6b7 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -301,12 +301,18 @@ describe Namespace do end def project_rugged(project) - project.repository.rugged + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + project.repository.rugged + end end end describe '#rm_dir', 'callback' do - let(:repository_storage_path) { Gitlab.config.repositories.storages.default.legacy_disk_path } + let(:repository_storage_path) do + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + Gitlab.config.repositories.storages.default.legacy_disk_path + end + end let(:path_in_dir) { File.join(repository_storage_path, namespace.full_path) } let(:deleted_path) { namespace.full_path.gsub(namespace.path, "#{namespace.full_path}+#{namespace.id}+deleted") } let(:deleted_path_in_dir) { File.join(repository_storage_path, deleted_path) } diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 9a76452a808..fe9d64c0e3b 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -3583,7 +3583,7 @@ describe Project do target_branch: 'target-branch', source_project: project, source_branch: 'awesome-feature-1', - allow_maintainer_to_push: true + allow_collaboration: true ) end @@ -3620,9 +3620,9 @@ describe Project do end end - describe '#branch_allows_maintainer_push?' do + describe '#branch_allows_collaboration_push?' do it 'allows access if the user can merge the merge request' do - expect(project.branch_allows_maintainer_push?(user, 'awesome-feature-1')) + expect(project.branch_allows_collaboration?(user, 'awesome-feature-1')) .to be_truthy end @@ -3630,7 +3630,7 @@ describe Project do guest = create(:user) target_project.add_guest(guest) - expect(project.branch_allows_maintainer_push?(guest, 'awesome-feature-1')) + expect(project.branch_allows_collaboration?(guest, 'awesome-feature-1')) .to be_falsy end @@ -3640,31 +3640,31 @@ describe Project do target_branch: 'target-branch', source_project: project, source_branch: 'rejected-feature-1', - allow_maintainer_to_push: true) + allow_collaboration: true) - expect(project.branch_allows_maintainer_push?(user, 'rejected-feature-1')) + expect(project.branch_allows_collaboration?(user, 'rejected-feature-1')) .to be_falsy end it 'does not allow access if the user cannot merge the merge request' do create(:protected_branch, :masters_can_push, project: target_project, name: 'target-branch') - expect(project.branch_allows_maintainer_push?(user, 'awesome-feature-1')) + expect(project.branch_allows_collaboration?(user, 'awesome-feature-1')) .to be_falsy end it 'caches the result' do - control = ActiveRecord::QueryRecorder.new { project.branch_allows_maintainer_push?(user, 'awesome-feature-1') } + control = ActiveRecord::QueryRecorder.new { project.branch_allows_collaboration?(user, 'awesome-feature-1') } - expect { 3.times { project.branch_allows_maintainer_push?(user, 'awesome-feature-1') } } + expect { 3.times { project.branch_allows_collaboration?(user, 'awesome-feature-1') } } .not_to exceed_query_limit(control) end context 'when the requeststore is active', :request_store do it 'only queries per project across instances' do - control = ActiveRecord::QueryRecorder.new { project.branch_allows_maintainer_push?(user, 'awesome-feature-1') } + control = ActiveRecord::QueryRecorder.new { project.branch_allows_collaboration?(user, 'awesome-feature-1') } - expect { 2.times { described_class.find(project.id).branch_allows_maintainer_push?(user, 'awesome-feature-1') } } + expect { 2.times { described_class.find(project.id).branch_allows_collaboration?(user, 'awesome-feature-1') } } .not_to exceed_query_limit(control).with_threshold(2) end end diff --git a/spec/models/project_team_spec.rb b/spec/models/project_team_spec.rb index e07c522800a..9978f3e9566 100644 --- a/spec/models/project_team_spec.rb +++ b/spec/models/project_team_spec.rb @@ -179,14 +179,14 @@ describe ProjectTeam do end describe "#human_max_access" do - it 'returns Master role' do + it 'returns Maintainer role' do user = create(:user) group = create(:group) project = create(:project, namespace: group) group.add_master(user) - expect(project.team.human_max_access(user.id)).to eq 'Master' + expect(project.team.human_max_access(user.id)).to eq 'Maintainer' end it 'returns Owner role' do diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 0ccf55bd895..7c0a1cd967c 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -136,7 +136,10 @@ describe Repository do before do options = { message: 'test tag message\n', tagger: { name: 'John Smith', email: 'john@gmail.com' } } - repository.rugged.tags.create(annotated_tag_name, 'a48e4fc218069f68ef2e769dd8dfea3991362175', options) + + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + repository.rugged.tags.create(annotated_tag_name, 'a48e4fc218069f68ef2e769dd8dfea3991362175', options) + end double_first = double(committed_date: Time.now - 1.second) double_last = double(committed_date: Time.now) @@ -1048,6 +1051,13 @@ describe Repository do let(:target_project) { project } let(:target_repository) { target_project.repository } + around do |example| + # TODO Gitlab::Git::OperationService will be moved to gitaly-ruby and disappear from this repo + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + example.run + end + end + context 'when pre hooks were successful' do before do service = Gitlab::Git::HooksService.new @@ -1309,6 +1319,13 @@ describe Repository do end describe '#update_autocrlf_option' do + around do |example| + # TODO Gitlab::Git::OperationService will be moved to gitaly-ruby and disappear from this repo + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + example.run + end + end + describe 'when autocrlf is not already set to :input' do before do repository.raw_repository.autocrlf = true @@ -1802,7 +1819,9 @@ describe Repository do expect(repository.branch_count).to be_an(Integer) # NOTE: Until rugged goes away, make sure rugged and gitaly are in sync - rugged_count = repository.raw_repository.rugged.branches.count + rugged_count = Gitlab::GitalyClient::StorageSettings.allow_disk_access do + repository.raw_repository.rugged.branches.count + end expect(repository.branch_count).to eq(rugged_count) end @@ -1813,7 +1832,9 @@ describe Repository do expect(repository.tag_count).to be_an(Integer) # NOTE: Until rugged goes away, make sure rugged and gitaly are in sync - rugged_count = repository.raw_repository.rugged.tags.count + rugged_count = Gitlab::GitalyClient::StorageSettings.allow_disk_access do + repository.raw_repository.rugged.tags.count + end expect(repository.tag_count).to eq(rugged_count) end @@ -2073,7 +2094,10 @@ describe Repository do it "attempting to call keep_around on truncated ref does not fail" do repository.keep_around(sample_commit.id) ref = repository.send(:keep_around_ref_name, sample_commit.id) - path = File.join(repository.path, ref) + + path = Gitlab::GitalyClient::StorageSettings.allow_disk_access do + File.join(repository.path, ref) + end # Corrupt the reference File.truncate(path, 0) @@ -2088,6 +2112,13 @@ describe Repository do end describe '#update_ref' do + around do |example| + # TODO Gitlab::Git::OperationService will be moved to gitaly-ruby and disappear from this repo + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + example.run + end + end + it 'can create a ref' do Gitlab::Git::OperationService.new(nil, repository.raw_repository).send(:update_ref, 'refs/heads/foobar', 'refs/heads/master', Gitlab::Git::BLANK_SHA) @@ -2372,7 +2403,9 @@ describe Repository do end def create_remote_branch(remote_name, branch_name, target) - rugged = repository.rugged + rugged = Gitlab::GitalyClient::StorageSettings.allow_disk_access do + repository.rugged + end rugged.references.create("refs/remotes/#{remote_name}/#{branch_name}", target.id) end @@ -2410,6 +2443,32 @@ describe Repository do end end + describe '#archive_metadata' do + let(:ref) { 'master' } + let(:storage_path) { '/tmp' } + + let(:prefix) { [project.path, ref].join('-') } + let(:filename) { prefix + '.tar.gz' } + + subject(:result) { repository.archive_metadata(ref, storage_path, append_sha: false) } + + context 'with hashed storage disabled' do + let(:project) { create(:project, :repository, :legacy_storage) } + + it 'uses the project path to generate the filename' do + expect(result['ArchivePrefix']).to eq(prefix) + expect(File.basename(result['ArchivePath'])).to eq(filename) + end + end + + context 'with hashed storage enabled' do + it 'uses the project path to generate the filename' do + expect(result['ArchivePrefix']).to eq(prefix) + expect(File.basename(result['ArchivePath'])).to eq(filename) + end + end + end + describe 'commit cache' do set(:project) { create(:project, :repository) } diff --git a/spec/models/term_agreement_spec.rb b/spec/models/term_agreement_spec.rb index a59bf119692..950dfa09a6a 100644 --- a/spec/models/term_agreement_spec.rb +++ b/spec/models/term_agreement_spec.rb @@ -5,4 +5,13 @@ describe TermAgreement do it { is_expected.to validate_presence_of(:term) } it { is_expected.to validate_presence_of(:user) } end + + describe '.accepted' do + it 'only includes accepted terms' do + accepted = create(:term_agreement, :accepted) + create(:term_agreement, :declined) + + expect(described_class.accepted).to contain_exactly(accepted) + end + end end diff --git a/spec/policies/ci/build_policy_spec.rb b/spec/policies/ci/build_policy_spec.rb index 9ca156deaa0..eead55d33ca 100644 --- a/spec/policies/ci/build_policy_spec.rb +++ b/spec/policies/ci/build_policy_spec.rb @@ -101,7 +101,7 @@ describe Ci::BuildPolicy do it 'enables update_build if user is maintainer' do allow_any_instance_of(Project).to receive(:empty_repo?).and_return(false) - allow_any_instance_of(Project).to receive(:branch_allows_maintainer_push?).and_return(true) + allow_any_instance_of(Project).to receive(:branch_allows_collaboration?).and_return(true) expect(policy).to be_allowed :update_build expect(policy).to be_allowed :update_commit_status diff --git a/spec/policies/ci/pipeline_policy_spec.rb b/spec/policies/ci/pipeline_policy_spec.rb index a5e509cfa0f..bd32faf06ef 100644 --- a/spec/policies/ci/pipeline_policy_spec.rb +++ b/spec/policies/ci/pipeline_policy_spec.rb @@ -69,7 +69,7 @@ describe Ci::PipelinePolicy, :models do it 'enables update_pipeline if user is maintainer' do allow_any_instance_of(Project).to receive(:empty_repo?).and_return(false) - allow_any_instance_of(Project).to receive(:branch_allows_maintainer_push?).and_return(true) + allow_any_instance_of(Project).to receive(:branch_allows_collaboration?).and_return(true) expect(policy).to be_allowed :update_pipeline end diff --git a/spec/policies/project_policy_spec.rb b/spec/policies/project_policy_spec.rb index 6609f5f7afd..6ac151f92f3 100644 --- a/spec/policies/project_policy_spec.rb +++ b/spec/policies/project_policy_spec.rb @@ -400,7 +400,7 @@ describe ProjectPolicy do :merge_request, target_project: target_project, source_project: project, - allow_maintainer_to_push: true + allow_collaboration: true ) end let(:maintainer_abilities) do diff --git a/spec/requests/api/avatar_spec.rb b/spec/requests/api/avatar_spec.rb new file mode 100644 index 00000000000..26e0435a6d5 --- /dev/null +++ b/spec/requests/api/avatar_spec.rb @@ -0,0 +1,106 @@ +require 'spec_helper' + +describe API::Avatar do + let(:gravatar_service) { double('GravatarService') } + + describe 'GET /avatar' do + context 'avatar uploaded to GitLab' do + context 'user with matching public email address' do + let(:user) { create(:user, :with_avatar, email: 'public@example.com', public_email: 'public@example.com') } + + before do + user + end + + it 'returns the avatar url' do + get api('/avatar'), { email: 'public@example.com' } + + expect(response.status).to eq 200 + expect(json_response['avatar_url']).to eql("#{::Settings.gitlab.base_url}#{user.avatar.local_url}") + end + end + + context 'no user with matching public email address' do + before do + expect(GravatarService).to receive(:new).and_return(gravatar_service) + expect(gravatar_service).to( + receive(:execute) + .with('private@example.com', nil, 2, { username: nil }) + .and_return('https://gravatar')) + end + + it 'returns the avatar url from Gravatar' do + get api('/avatar'), { email: 'private@example.com' } + + expect(response.status).to eq 200 + expect(json_response['avatar_url']).to eq('https://gravatar') + end + end + end + + context 'avatar uploaded to Gravatar' do + context 'user with matching public email address' do + let(:user) { create(:user, email: 'public@example.com', public_email: 'public@example.com') } + + before do + user + + expect(GravatarService).to receive(:new).and_return(gravatar_service) + expect(gravatar_service).to( + receive(:execute) + .with('public@example.com', nil, 2, { username: user.username }) + .and_return('https://gravatar')) + end + + it 'returns the avatar url from Gravatar' do + get api('/avatar'), { email: 'public@example.com' } + + expect(response.status).to eq 200 + expect(json_response['avatar_url']).to eq('https://gravatar') + end + end + + context 'no user with matching public email address' do + before do + expect(GravatarService).to receive(:new).and_return(gravatar_service) + expect(gravatar_service).to( + receive(:execute) + .with('private@example.com', nil, 2, { username: nil }) + .and_return('https://gravatar')) + end + + it 'returns the avatar url from Gravatar' do + get api('/avatar'), { email: 'private@example.com' } + + expect(response.status).to eq 200 + expect(json_response['avatar_url']).to eq('https://gravatar') + end + end + + context 'public visibility level restricted' do + let(:user) { create(:user, :with_avatar, email: 'public@example.com', public_email: 'public@example.com') } + + before do + user + + stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::PUBLIC]) + end + + context 'when authenticated' do + it 'returns the avatar url' do + get api('/avatar', user), { email: 'public@example.com' } + + expect(response.status).to eq 200 + expect(json_response['avatar_url']).to eql("#{::Settings.gitlab.base_url}#{user.avatar.local_url}") + end + end + + context 'when unauthenticated' do + it_behaves_like '403 response' do + let(:request) { get api('/avatar'), { email: 'public@example.com' } } + end + end + end + end + end +end diff --git a/spec/requests/api/events_spec.rb b/spec/requests/api/events_spec.rb index 962c845f36d..e6a61fdcf39 100644 --- a/spec/requests/api/events_spec.rb +++ b/spec/requests/api/events_spec.rb @@ -176,7 +176,7 @@ describe API::Events do end it 'avoids N+1 queries' do - control_count = ActiveRecord::QueryRecorder.new do + control_count = ActiveRecord::QueryRecorder.new(skip_cached: false) do get api("/projects/#{private_project.id}/events", user), target_type: :merge_request end.count @@ -184,7 +184,7 @@ describe API::Events do expect do get api("/projects/#{private_project.id}/events", user), target_type: :merge_request - end.not_to exceed_query_limit(control_count) + end.not_to exceed_all_query_limit(control_count) expect(response).to have_gitlab_http_status(200) expect(response).to include_pagination_headers diff --git a/spec/requests/api/graphql/merge_request_query_spec.rb b/spec/requests/api/graphql/merge_request_query_spec.rb new file mode 100644 index 00000000000..12b1d5d18a2 --- /dev/null +++ b/spec/requests/api/graphql/merge_request_query_spec.rb @@ -0,0 +1,49 @@ +require 'spec_helper' + +describe 'getting merge request information' do + include GraphqlHelpers + + let(:project) { create(:project, :repository) } + let(:merge_request) { create(:merge_request, source_project: project) } + let(:current_user) { create(:user) } + + let(:query) do + attributes = { + 'fullPath' => merge_request.project.full_path, + 'iid' => merge_request.iid + } + graphql_query_for('mergeRequest', attributes) + end + + context 'when the user has access to the merge request' do + before do + project.add_developer(current_user) + post_graphql(query, current_user: current_user) + end + + it 'returns the merge request' do + expect(graphql_data['mergeRequest']).not_to be_nil + end + + # This is a field coming from the `MergeRequestPresenter` + it 'includes a web_url' do + expect(graphql_data['mergeRequest']['webUrl']).to be_present + end + + it_behaves_like 'a working graphql query' + end + + context 'when the user does not have access to the merge request' do + before do + post_graphql(query, current_user: current_user) + end + + it 'returns an empty field' do + post_graphql(query, current_user: current_user) + + expect(graphql_data['mergeRequest']).to be_nil + end + + it_behaves_like 'a working graphql query' + end +end diff --git a/spec/requests/api/graphql/project_query_spec.rb b/spec/requests/api/graphql/project_query_spec.rb new file mode 100644 index 00000000000..8196bcfa87c --- /dev/null +++ b/spec/requests/api/graphql/project_query_spec.rb @@ -0,0 +1,39 @@ +require 'spec_helper' + +describe 'getting project information' do + include GraphqlHelpers + + let(:project) { create(:project, :repository) } + let(:current_user) { create(:user) } + + let(:query) do + graphql_query_for('project', 'fullPath' => project.full_path) + end + + context 'when the user has access to the project' do + before do + project.add_developer(current_user) + post_graphql(query, current_user: current_user) + end + + it 'includes the project' do + expect(graphql_data['project']).not_to be_nil + end + + it_behaves_like 'a working graphql query' + end + + context 'when the user does not have access to the project' do + before do + post_graphql(query, current_user: current_user) + end + + it 'returns an empty field' do + post_graphql(query, current_user: current_user) + + expect(graphql_data['project']).to be_nil + end + + it_behaves_like 'a working graphql query' + end +end diff --git a/spec/requests/api/internal_spec.rb b/spec/requests/api/internal_spec.rb index 5dc3ddd4b36..bc32372d3a9 100644 --- a/spec/requests/api/internal_spec.rb +++ b/spec/requests/api/internal_spec.rb @@ -835,8 +835,7 @@ describe API::Internal do end def push(key, project, protocol = 'ssh', env: nil) - post( - api("/internal/allowed"), + params = { changes: 'd14d6c0abdd253381df51a723d58691b2ee1ab08 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/master', key_id: key.id, project: project.full_path, @@ -845,7 +844,19 @@ describe API::Internal do secret_token: secret_token, protocol: protocol, env: env - ) + } + + if Gitlab.rails5? + post( + api("/internal/allowed"), + params: params + ) + else + post( + api("/internal/allowed"), + params + ) + end end def archive(key, project) diff --git a/spec/requests/api/issues_spec.rb b/spec/requests/api/issues_spec.rb index 4181f4ebbbe..a15d60aafe0 100644 --- a/spec/requests/api/issues_spec.rb +++ b/spec/requests/api/issues_spec.rb @@ -630,15 +630,17 @@ describe API::Issues do end it 'avoids N+1 queries' do - control_count = ActiveRecord::QueryRecorder.new do + get api("/projects/#{project.id}/issues", user) + + control_count = ActiveRecord::QueryRecorder.new(skip_cached: false) do get api("/projects/#{project.id}/issues", user) end.count - create(:issue, author: user, project: project) + create_list(:issue, 3, project: project) expect do get api("/projects/#{project.id}/issues", user) - end.not_to exceed_query_limit(control_count) + end.not_to exceed_all_query_limit(control_count) end it 'returns 404 when project does not exist' do diff --git a/spec/requests/api/jobs_spec.rb b/spec/requests/api/jobs_spec.rb index 45082e644ca..50d6f4b4d99 100644 --- a/spec/requests/api/jobs_spec.rb +++ b/spec/requests/api/jobs_spec.rb @@ -177,6 +177,18 @@ describe API::Jobs do json_response.each { |job| expect(job['pipeline']['id']).to eq(pipeline.id) } end end + + it 'avoids N+1 queries' do + control_count = ActiveRecord::QueryRecorder.new(skip_cached: false) do + get api("/projects/#{project.id}/pipelines/#{pipeline.id}/jobs", api_user), query + end.count + + 3.times { create(:ci_build, :artifacts, pipeline: pipeline) } + + expect do + get api("/projects/#{project.id}/pipelines/#{pipeline.id}/jobs", api_user), query + end.not_to exceed_all_query_limit(control_count) + end end context 'unauthorized user' do diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index 605761867bf..d4ebfc3f782 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -386,12 +386,13 @@ describe API::MergeRequests do source_project: forked_project, target_project: project, source_branch: 'fixes', - allow_maintainer_to_push: true) + allow_collaboration: true) end - it 'includes the `allow_maintainer_to_push` field' do + it 'includes the `allow_collaboration` field' do get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user) + expect(json_response['allow_collaboration']).to be_truthy expect(json_response['allow_maintainer_to_push']).to be_truthy end end @@ -654,11 +655,12 @@ describe API::MergeRequests do expect(response).to have_gitlab_http_status(400) end - it 'allows setting `allow_maintainer_to_push`' do + it 'allows setting `allow_collaboration`' do post api("/projects/#{forked_project.id}/merge_requests", user2), - title: 'Test merge_request', source_branch: "feature_conflict", target_branch: "master", - author: user2, target_project_id: project.id, allow_maintainer_to_push: true + title: 'Test merge_request', source_branch: "feature_conflict", target_branch: "master", + author: user2, target_project_id: project.id, allow_collaboration: true expect(response).to have_gitlab_http_status(201) + expect(json_response['allow_collaboration']).to be_truthy expect(json_response['allow_maintainer_to_push']).to be_truthy end diff --git a/spec/requests/api/pipelines_spec.rb b/spec/requests/api/pipelines_spec.rb index 0736329f9fd..78ea77cb3bb 100644 --- a/spec/requests/api/pipelines_spec.rb +++ b/spec/requests/api/pipelines_spec.rb @@ -285,6 +285,15 @@ describe API::Pipelines do end describe 'POST /projects/:id/pipeline ' do + def expect_variables(variables, expected_variables) + variables.each_with_index do |variable, index| + expected_variable = expected_variables[index] + + expect(variable.key).to eq(expected_variable['key']) + expect(variable.value).to eq(expected_variable['value']) + end + end + context 'authorized user' do context 'with gitlab-ci.yml' do before do @@ -294,13 +303,62 @@ describe API::Pipelines do it 'creates and returns a new pipeline' do expect do post api("/projects/#{project.id}/pipeline", user), ref: project.default_branch - end.to change { Ci::Pipeline.count }.by(1) + end.to change { project.pipelines.count }.by(1) expect(response).to have_gitlab_http_status(201) expect(json_response).to be_a Hash expect(json_response['sha']).to eq project.commit.id end + context 'variables given' do + let(:variables) { [{ 'key' => 'UPLOAD_TO_S3', 'value' => 'true' }] } + + it 'creates and returns a new pipeline using the given variables' do + expect do + post api("/projects/#{project.id}/pipeline", user), ref: project.default_branch, variables: variables + end.to change { project.pipelines.count }.by(1) + expect_variables(project.pipelines.last.variables, variables) + + expect(response).to have_gitlab_http_status(201) + expect(json_response).to be_a Hash + expect(json_response['sha']).to eq project.commit.id + expect(json_response).not_to have_key('variables') + end + end + + describe 'using variables conditions' do + let(:variables) { [{ 'key' => 'STAGING', 'value' => 'true' }] } + + before do + config = YAML.dump(test: { script: 'test', only: { variables: ['$STAGING'] } }) + stub_ci_pipeline_yaml_file(config) + end + + it 'creates and returns a new pipeline using the given variables' do + expect do + post api("/projects/#{project.id}/pipeline", user), ref: project.default_branch, variables: variables + end.to change { project.pipelines.count }.by(1) + expect_variables(project.pipelines.last.variables, variables) + + expect(response).to have_gitlab_http_status(201) + expect(json_response).to be_a Hash + expect(json_response['sha']).to eq project.commit.id + expect(json_response).not_to have_key('variables') + end + + context 'condition unmatch' do + let(:variables) { [{ 'key' => 'STAGING', 'value' => 'false' }] } + + it "doesn't create a job" do + expect do + post api("/projects/#{project.id}/pipeline", user), ref: project.default_branch + end.not_to change { project.pipelines.count } + + expect(response).to have_gitlab_http_status(400) + end + end + end + it 'fails when using an invalid ref' do post api("/projects/#{project.id}/pipeline", user), ref: 'invalid_ref' diff --git a/spec/requests/api/repositories_spec.rb b/spec/requests/api/repositories_spec.rb index 9e6d69e3874..cd135dfc32a 100644 --- a/spec/requests/api/repositories_spec.rb +++ b/spec/requests/api/repositories_spec.rb @@ -220,11 +220,10 @@ describe API::Repositories do expect(response).to have_gitlab_http_status(200) - repo_name = project.repository.name.gsub("\.git", "") type, params = workhorse_send_data expect(type).to eq('git-archive') - expect(params['ArchivePath']).to match(/#{repo_name}\-[^\.]+\.tar.gz/) + expect(params['ArchivePath']).to match(/#{project.path}\-[^\.]+\.tar.gz/) end it 'returns the repository archive archive.zip' do @@ -232,11 +231,10 @@ describe API::Repositories do expect(response).to have_gitlab_http_status(200) - repo_name = project.repository.name.gsub("\.git", "") type, params = workhorse_send_data expect(type).to eq('git-archive') - expect(params['ArchivePath']).to match(/#{repo_name}\-[^\.]+\.zip/) + expect(params['ArchivePath']).to match(/#{project.path}\-[^\.]+\.zip/) end it 'returns the repository archive archive.tar.bz2' do @@ -244,11 +242,10 @@ describe API::Repositories do expect(response).to have_gitlab_http_status(200) - repo_name = project.repository.name.gsub("\.git", "") type, params = workhorse_send_data expect(type).to eq('git-archive') - expect(params['ArchivePath']).to match(/#{repo_name}\-[^\.]+\.tar.bz2/) + expect(params['ArchivePath']).to match(/#{project.path}\-[^\.]+\.tar.bz2/) end context 'when sha does not exist' do diff --git a/spec/requests/api/runner_spec.rb b/spec/requests/api/runner_spec.rb index 319ac389083..c981a10ac38 100644 --- a/spec/requests/api/runner_spec.rb +++ b/spec/requests/api/runner_spec.rb @@ -1101,6 +1101,7 @@ describe API::Runner, :clean_gitlab_redis_shared_state do expect(json_response['RemoteObject']).to have_key('GetURL') expect(json_response['RemoteObject']).to have_key('StoreURL') expect(json_response['RemoteObject']).to have_key('DeleteURL') + expect(json_response['RemoteObject']).to have_key('MultipartUpload') end end diff --git a/spec/requests/api/settings_spec.rb b/spec/requests/api/settings_spec.rb index aead8978dd4..57adc3ca7a6 100644 --- a/spec/requests/api/settings_spec.rb +++ b/spec/requests/api/settings_spec.rb @@ -35,7 +35,9 @@ describe API::Settings, 'Settings' do context "custom repository storage type set in the config" do before do - storages = { 'custom' => 'tmp/tests/custom_repositories' } + # Add a possible storage to the config + storages = Gitlab.config.repositories.storages + .merge({ 'custom' => 'tmp/tests/custom_repositories' }) allow(Gitlab.config.repositories).to receive(:storages).and_return(storages) end diff --git a/spec/requests/api/tags_spec.rb b/spec/requests/api/tags_spec.rb index e2b19ad59f9..969710d6613 100644 --- a/spec/requests/api/tags_spec.rb +++ b/spec/requests/api/tags_spec.rb @@ -287,7 +287,10 @@ describe API::Tags do context 'annotated tag' do it 'creates a new annotated tag' do # Identity must be set in .gitconfig to create annotated tag. - repo_path = project.repository.path_to_repo + repo_path = Gitlab::GitalyClient::StorageSettings.allow_disk_access do + project.repository.path_to_repo + end + system(*%W(#{Gitlab.config.git.bin_path} --git-dir=#{repo_path} config user.name #{user.name})) system(*%W(#{Gitlab.config.git.bin_path} --git-dir=#{repo_path} config user.email #{user.email})) diff --git a/spec/requests/lfs_http_spec.rb b/spec/requests/lfs_http_spec.rb index 79672fe1cc5..4d30b99262e 100644 --- a/spec/requests/lfs_http_spec.rb +++ b/spec/requests/lfs_http_spec.rb @@ -1021,6 +1021,7 @@ describe 'Git LFS API and storage' do expect(json_response['RemoteObject']).to have_key('GetURL') expect(json_response['RemoteObject']).to have_key('StoreURL') expect(json_response['RemoteObject']).to have_key('DeleteURL') + expect(json_response['RemoteObject']).not_to have_key('MultipartUpload') expect(json_response['LfsOid']).to eq(sample_oid) expect(json_response['LfsSize']).to eq(sample_size) end diff --git a/spec/routing/api_routing_spec.rb b/spec/routing/api_routing_spec.rb new file mode 100644 index 00000000000..5fde4bd885b --- /dev/null +++ b/spec/routing/api_routing_spec.rb @@ -0,0 +1,31 @@ +require 'spec_helper' + +describe 'api', 'routing' do + context 'when graphql is disabled' do + before do + stub_feature_flags(graphql: false) + end + + it 'does not route to the GraphqlController' do + expect(get('/api/graphql')).not_to route_to('graphql#execute') + end + + it 'does not expose graphiql' do + expect(get('/-/graphql-explorer')).not_to route_to('graphiql/rails/editors#show') + end + end + + context 'when graphql is disabled' do + before do + stub_feature_flags(graphql: true) + end + + it 'routes to the GraphqlController' do + expect(get('/api/graphql')).not_to route_to('graphql#execute') + end + + it 'exposes graphiql' do + expect(get('/-/graphql-explorer')).not_to route_to('graphiql/rails/editors#show') + end + end +end diff --git a/spec/serializers/pipeline_serializer_spec.rb b/spec/serializers/pipeline_serializer_spec.rb index b741308e2c5..e0e6eecb300 100644 --- a/spec/serializers/pipeline_serializer_spec.rb +++ b/spec/serializers/pipeline_serializer_spec.rb @@ -8,6 +8,10 @@ describe PipelineSerializer do described_class.new(current_user: user) end + before do + stub_feature_flags(ci_pipeline_persisted_stages: true) + end + subject { serializer.represent(resource) } describe '#represent' do @@ -99,7 +103,8 @@ describe PipelineSerializer do end end - context 'number of queries' do + describe 'number of queries when preloaded' do + subject { serializer.represent(resource, preload: true) } let(:resource) { Ci::Pipeline.all } before do @@ -107,7 +112,7 @@ describe PipelineSerializer do # gitaly calls in this block # Issue: https://gitlab.com/gitlab-org/gitlab-ce/issues/37772 Gitlab::GitalyClient.allow_n_plus_1_calls do - Ci::Pipeline::AVAILABLE_STATUSES.each do |status| + Ci::Pipeline::COMPLETED_STATUSES.each do |status| create_pipeline(status) end end @@ -120,7 +125,7 @@ describe PipelineSerializer do it 'verifies number of queries', :request_store do recorded = ActiveRecord::QueryRecorder.new { subject } - expect(recorded.count).to be_within(1).of(44) + expect(recorded.count).to be_within(2).of(27) expect(recorded.cached_count).to eq(0) end end @@ -139,7 +144,7 @@ describe PipelineSerializer do # pipeline. With the same ref this check is cached but if refs are # different then there is an extra query per ref # https://gitlab.com/gitlab-org/gitlab-ce/issues/46368 - expect(recorded.count).to be_within(1).of(51) + expect(recorded.count).to be_within(2).of(30) expect(recorded.cached_count).to eq(0) end end diff --git a/spec/services/ci/retry_pipeline_service_spec.rb b/spec/services/ci/retry_pipeline_service_spec.rb index a73bd7a0268..688d3b8c038 100644 --- a/spec/services/ci/retry_pipeline_service_spec.rb +++ b/spec/services/ci/retry_pipeline_service_spec.rb @@ -280,12 +280,12 @@ describe Ci::RetryPipelineService, '#execute' do source_project: forked_project, target_project: project, source_branch: 'fixes', - allow_maintainer_to_push: true) + allow_collaboration: true) create_build('rspec 1', :failed, 1) end it 'allows to retry failed pipeline' do - allow_any_instance_of(Project).to receive(:fetch_branch_allows_maintainer_push?).and_return(true) + allow_any_instance_of(Project).to receive(:fetch_branch_allows_collaboration?).and_return(true) allow_any_instance_of(Project).to receive(:empty_repo?).and_return(false) service.execute(pipeline) diff --git a/spec/services/lfs/unlock_file_service_spec.rb b/spec/services/lfs/unlock_file_service_spec.rb index 4bea112b9c6..948401d7bdc 100644 --- a/spec/services/lfs/unlock_file_service_spec.rb +++ b/spec/services/lfs/unlock_file_service_spec.rb @@ -80,12 +80,12 @@ describe Lfs::UnlockFileService do result = subject.execute expect(result[:status]).to eq(:error) - expect(result[:message]).to match(/You must have master access/) + expect(result[:message]).to match(/You must have maintainer access/) expect(result[:http_status]).to eq(403) end end - context 'by a master user' do + context 'by a maintainer user' do let(:current_user) { master } let(:params) do { id: lock.id, diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb index bd2e91f1f7a..443dcd92a8b 100644 --- a/spec/services/merge_requests/update_service_spec.rb +++ b/spec/services/merge_requests/update_service_spec.rb @@ -547,7 +547,7 @@ describe MergeRequests::UpdateService, :mailer do let(:closed_issuable) { create(:closed_merge_request, source_project: project) } end - context 'setting `allow_maintainer_to_push`' do + context 'setting `allow_collaboration`' do let(:target_project) { create(:project, :public) } let(:source_project) { fork_project(target_project) } let(:user) { create(:user) } @@ -562,23 +562,23 @@ describe MergeRequests::UpdateService, :mailer do allow(ProtectedBranch).to receive(:protected?).with(source_project, 'fixes') { false } end - it 'does not allow a maintainer of the target project to set `allow_maintainer_to_push`' do + it 'does not allow a maintainer of the target project to set `allow_collaboration`' do target_project.add_developer(user) - update_merge_request(allow_maintainer_to_push: true, title: 'Updated title') + update_merge_request(allow_collaboration: true, title: 'Updated title') expect(merge_request.title).to eq('Updated title') - expect(merge_request.allow_maintainer_to_push).to be_falsy + expect(merge_request.allow_collaboration).to be_falsy end it 'is allowed by a user that can push to the source and can update the merge request' do merge_request.update!(assignee: user) source_project.add_developer(user) - update_merge_request(allow_maintainer_to_push: true, title: 'Updated title') + update_merge_request(allow_collaboration: true, title: 'Updated title') expect(merge_request.title).to eq('Updated title') - expect(merge_request.allow_maintainer_to_push).to be_truthy + expect(merge_request.allow_collaboration).to be_truthy end end end diff --git a/spec/services/pages_service_spec.rb b/spec/services/pages_service_spec.rb deleted file mode 100644 index f8db6900a0a..00000000000 --- a/spec/services/pages_service_spec.rb +++ /dev/null @@ -1,53 +0,0 @@ -require 'spec_helper' - -describe PagesService do - let(:build) { create(:ci_build) } - let(:data) { Gitlab::DataBuilder::Build.build(build) } - let(:service) { described_class.new(data) } - - before do - allow(Gitlab.config.pages).to receive(:enabled).and_return(true) - end - - context 'execute asynchronously for pages job' do - before do - build.name = 'pages' - end - - context 'on success' do - before do - build.success - end - - it 'executes worker' do - expect(PagesWorker).to receive(:perform_async) - service.execute - end - end - - %w(pending running failed canceled).each do |status| - context "on #{status}" do - before do - build.status = status - end - - it 'does not execute worker' do - expect(PagesWorker).not_to receive(:perform_async) - service.execute - end - end - end - end - - context 'for other jobs' do - before do - build.name = 'other job' - build.success - end - - it 'does not execute worker' do - expect(PagesWorker).not_to receive(:perform_async) - service.execute - end - end -end diff --git a/spec/services/projects/autocomplete_service_spec.rb b/spec/services/projects/autocomplete_service_spec.rb index f7ff8b80bd7..6fd73a50511 100644 --- a/spec/services/projects/autocomplete_service_spec.rb +++ b/spec/services/projects/autocomplete_service_spec.rb @@ -115,5 +115,20 @@ describe Projects::AutocompleteService do expect(milestone_titles).to eq([group_milestone2.title, group_milestone1.title]) end + + context 'with nested groups', :nested_groups do + let(:subgroup) { create(:group, :public, parent: group) } + let!(:subgroup_milestone) { create(:milestone, group: subgroup) } + + before do + project.update(namespace: subgroup) + end + + it 'includes project milestones and all acestors milestones' do + expect(milestone_titles).to match_array( + [project_milestone.title, group_milestone2.title, group_milestone1.title, subgroup_milestone.title] + ) + end + end end end diff --git a/spec/services/projects/import_service_spec.rb b/spec/services/projects/import_service_spec.rb index 30c89ebd821..b3815045792 100644 --- a/spec/services/projects/import_service_spec.rb +++ b/spec/services/projects/import_service_spec.rb @@ -3,9 +3,17 @@ require 'spec_helper' describe Projects::ImportService do let!(:project) { create(:project) } let(:user) { project.creator } + let(:import_url) { 'http://www.gitlab.com/demo/repo.git' } + let(:oid_download_links) { { 'oid1' => "#{import_url}/gitlab-lfs/objects/oid1", 'oid2' => "#{import_url}/gitlab-lfs/objects/oid2" } } subject { described_class.new(project, user) } + before do + allow(project).to receive(:lfs_enabled?).and_return(true) + allow_any_instance_of(Projects::LfsPointers::LfsDownloadService).to receive(:execute) + allow_any_instance_of(Projects::LfsPointers::LfsImportService).to receive(:execute).and_return(oid_download_links) + end + describe '#async?' do it 'returns true for an asynchronous importer' do importer_class = double(:importer, async?: true) @@ -63,6 +71,15 @@ describe Projects::ImportService do expect(result[:status]).to eq :error expect(result[:message]).to eq "Error importing repository #{project.import_url} into #{project.full_path} - The repository could not be created." end + + context 'when repository creation succeeds' do + it 'does not download lfs files' do + expect_any_instance_of(Projects::LfsPointers::LfsImportService).not_to receive(:execute) + expect_any_instance_of(Projects::LfsPointers::LfsDownloadService).not_to receive(:execute) + + subject.execute + end + end end context 'with known url' do @@ -91,6 +108,15 @@ describe Projects::ImportService do expect(result[:status]).to eq :error end + + context 'when repository import scheduled' do + it 'does not download lfs objects' do + expect_any_instance_of(Projects::LfsPointers::LfsImportService).not_to receive(:execute) + expect_any_instance_of(Projects::LfsPointers::LfsDownloadService).not_to receive(:execute) + + subject.execute + end + end end context 'with a non Github repository' do @@ -99,9 +125,10 @@ describe Projects::ImportService do project.import_type = 'bitbucket' end - it 'succeeds if repository import is successfully' do + it 'succeeds if repository import is successfull' do expect_any_instance_of(Gitlab::Shell).to receive(:import_repository).and_return(true) expect_any_instance_of(Gitlab::BitbucketImport::Importer).to receive(:execute).and_return(true) + expect_any_instance_of(Projects::LfsPointers::LfsImportService).to receive(:execute).and_return({}) result = subject.execute @@ -116,6 +143,29 @@ describe Projects::ImportService do expect(result[:status]).to eq :error expect(result[:message]).to eq "Error importing repository #{project.import_url} into #{project.full_path} - Failed to import the repository" end + + context 'when repository import scheduled' do + before do + allow_any_instance_of(Gitlab::Shell).to receive(:import_repository).and_return(true) + allow(subject).to receive(:import_data) + end + + it 'downloads lfs objects if lfs_enabled is enabled for project' do + allow(project).to receive(:lfs_enabled?).and_return(true) + expect_any_instance_of(Projects::LfsPointers::LfsImportService).to receive(:execute).and_return(oid_download_links) + expect_any_instance_of(Projects::LfsPointers::LfsDownloadService).to receive(:execute).twice + + subject.execute + end + + it 'does not download lfs objects if lfs_enabled is not enabled for project' do + allow(project).to receive(:lfs_enabled?).and_return(false) + expect_any_instance_of(Projects::LfsPointers::LfsImportService).not_to receive(:execute) + expect_any_instance_of(Projects::LfsPointers::LfsDownloadService).not_to receive(:execute) + + subject.execute + end + end end end @@ -147,6 +197,26 @@ describe Projects::ImportService do expect(result[:status]).to eq :error end + + context 'when importer' do + it 'has a custom repository importer it does not download lfs objects' do + allow(Gitlab::GithubImport::ParallelImporter).to receive(:imports_repository?).and_return(true) + + expect_any_instance_of(Projects::LfsPointers::LfsImportService).not_to receive(:execute) + expect_any_instance_of(Projects::LfsPointers::LfsDownloadService).not_to receive(:execute) + + subject.execute + end + + it 'does not have a custom repository importer downloads lfs objects' do + allow(Gitlab::GithubImport::ParallelImporter).to receive(:imports_repository?).and_return(false) + + expect_any_instance_of(Projects::LfsPointers::LfsImportService).to receive(:execute).and_return(oid_download_links) + expect_any_instance_of(Projects::LfsPointers::LfsDownloadService).to receive(:execute) + + subject.execute + end + end end context 'with blocked import_URL' do diff --git a/spec/services/projects/lfs_pointers/lfs_download_link_list_service_spec.rb b/spec/services/projects/lfs_pointers/lfs_download_link_list_service_spec.rb new file mode 100644 index 00000000000..d7a2829d5f8 --- /dev/null +++ b/spec/services/projects/lfs_pointers/lfs_download_link_list_service_spec.rb @@ -0,0 +1,102 @@ +require 'spec_helper' + +describe Projects::LfsPointers::LfsDownloadLinkListService do + let(:import_url) { 'http://www.gitlab.com/demo/repo.git' } + let(:lfs_endpoint) { "#{import_url}/info/lfs/objects/batch" } + let!(:project) { create(:project, import_url: import_url) } + let(:new_oids) { { 'oid1' => 123, 'oid2' => 125 } } + let(:remote_uri) { URI.parse(lfs_endpoint) } + + let(:objects_response) do + body = new_oids.map do |oid, size| + { + 'oid' => oid, + 'size' => size, + 'actions' => { + 'download' => { 'href' => "#{import_url}/gitlab-lfs/objects/#{oid}" } + } + } + end + + Struct.new(:success?, :objects).new(true, body) + end + + let(:invalid_object_response) do + [ + 'oid' => 'whatever', + 'size' => 123 + ] + end + + subject { described_class.new(project, remote_uri: remote_uri) } + + before do + allow(project).to receive(:lfs_enabled?).and_return(true) + allow(Gitlab::HTTP).to receive(:post).and_return(objects_response) + end + + describe '#execute' do + it 'retrieves each download link of every non existent lfs object' do + subject.execute(new_oids).each do |oid, link| + expect(link).to eq "#{import_url}/gitlab-lfs/objects/#{oid}" + end + end + + context 'credentials' do + context 'when the download link and the lfs_endpoint have the same host' do + context 'when lfs_endpoint has credentials' do + let(:import_url) { 'http://user:password@www.gitlab.com/demo/repo.git' } + + it 'adds credentials to the download_link' do + result = subject.execute(new_oids) + + result.each do |oid, link| + expect(link.starts_with?('http://user:password@')).to be_truthy + end + end + end + + context 'when lfs_endpoint does not have any credentials' do + it 'does not add any credentials' do + result = subject.execute(new_oids) + + result.each do |oid, link| + expect(link.starts_with?('http://user:password@')).to be_falsey + end + end + end + end + + context 'when the download link and the lfs_endpoint have different hosts' do + let(:import_url_with_credentials) { 'http://user:password@www.otherdomain.com/demo/repo.git' } + let(:lfs_endpoint) { "#{import_url_with_credentials}/info/lfs/objects/batch" } + + it 'downloads without any credentials' do + result = subject.execute(new_oids) + + result.each do |oid, link| + expect(link.starts_with?('http://user:password@')).to be_falsey + end + end + end + end + end + + describe '#get_download_links' do + it 'raise errorif request fails' do + allow(Gitlab::HTTP).to receive(:post).and_return(Struct.new(:success?, :message).new(false, 'Failed request')) + + expect { subject.send(:get_download_links, new_oids) }.to raise_error(described_class::DownloadLinksError) + end + end + + describe '#parse_response_links' do + it 'does not add oid entry if href not found' do + expect(Rails.logger).to receive(:error).with("Link for Lfs Object with oid whatever not found or invalid.") + + result = subject.send(:parse_response_links, invalid_object_response) + + expect(result).to be_empty + end + end +end diff --git a/spec/services/projects/lfs_pointers/lfs_download_service_spec.rb b/spec/services/projects/lfs_pointers/lfs_download_service_spec.rb new file mode 100644 index 00000000000..6af5bfc7689 --- /dev/null +++ b/spec/services/projects/lfs_pointers/lfs_download_service_spec.rb @@ -0,0 +1,69 @@ +require 'spec_helper' + +describe Projects::LfsPointers::LfsDownloadService do + let(:project) { create(:project) } + let(:oid) { '9e548e25631dd9ce6b43afd6359ab76da2819d6a5b474e66118c7819e1d8b3e8' } + let(:download_link) { "http://gitlab.com/#{oid}" } + let(:lfs_content) do + <<~HEREDOC + whatever + HEREDOC + end + + subject { described_class.new(project) } + + before do + allow(project).to receive(:lfs_enabled?).and_return(true) + WebMock.stub_request(:get, download_link).to_return(body: lfs_content) + end + + describe '#execute' do + context 'when file download succeeds' do + it 'a new lfs object is created' do + expect { subject.execute(oid, download_link) }.to change { LfsObject.count }.from(0).to(1) + end + + it 'has the same oid' do + subject.execute(oid, download_link) + + expect(LfsObject.first.oid).to eq oid + end + + it 'stores the content' do + subject.execute(oid, download_link) + + expect(File.read(LfsObject.first.file.file.file)).to eq lfs_content + end + end + + context 'when file download fails' do + it 'no lfs object is created' do + expect { subject.execute(oid, download_link) }.to change { LfsObject.count } + end + end + + context 'when credentials present' do + let(:download_link_with_credentials) { "http://user:password@gitlab.com/#{oid}" } + + before do + WebMock.stub_request(:get, download_link).with(headers: { 'Authorization' => 'Basic dXNlcjpwYXNzd29yZA==' }).to_return(body: lfs_content) + end + + it 'the request adds authorization headers' do + subject.execute(oid, download_link_with_credentials) + end + end + + context 'when an lfs object with the same oid already exists' do + before do + create(:lfs_object, oid: 'oid') + end + + it 'does not download the file' do + expect(subject).not_to receive(:download_and_save_file) + + subject.execute('oid', download_link) + end + end + end +end diff --git a/spec/services/projects/lfs_pointers/lfs_import_service_spec.rb b/spec/services/projects/lfs_pointers/lfs_import_service_spec.rb new file mode 100644 index 00000000000..5a75fb38dec --- /dev/null +++ b/spec/services/projects/lfs_pointers/lfs_import_service_spec.rb @@ -0,0 +1,146 @@ +require 'spec_helper' + +describe Projects::LfsPointers::LfsImportService do + let(:import_url) { 'http://www.gitlab.com/demo/repo.git' } + let(:default_endpoint) { "#{import_url}/info/lfs/objects/batch"} + let(:group) { create(:group, lfs_enabled: true)} + let!(:project) { create(:project, namespace: group, import_url: import_url, lfs_enabled: true) } + let!(:lfs_objects_project) { create_list(:lfs_objects_project, 2, project: project) } + let!(:existing_lfs_objects) { LfsObject.pluck(:oid, :size).to_h } + let(:oids) { { 'oid1' => 123, 'oid2' => 125 } } + let(:oid_download_links) { { 'oid1' => "#{import_url}/gitlab-lfs/objects/oid1", 'oid2' => "#{import_url}/gitlab-lfs/objects/oid2" } } + let(:all_oids) { existing_lfs_objects.merge(oids) } + let(:remote_uri) { URI.parse(lfs_endpoint) } + + subject { described_class.new(project) } + + before do + allow(project.repository).to receive(:lfsconfig_for).and_return(nil) + allow(Gitlab.config.lfs).to receive(:enabled).and_return(true) + allow_any_instance_of(Projects::LfsPointers::LfsListService).to receive(:execute).and_return(all_oids) + end + + describe '#execute' do + context 'when no lfs pointer is linked' do + before do + allow_any_instance_of(Projects::LfsPointers::LfsLinkService).to receive(:execute).and_return([]) + allow_any_instance_of(Projects::LfsPointers::LfsDownloadLinkListService).to receive(:execute).and_return(oid_download_links) + expect(Projects::LfsPointers::LfsDownloadLinkListService).to receive(:new).with(project, remote_uri: URI.parse(default_endpoint)).and_call_original + end + + it 'retrieves all lfs pointers in the project repository' do + expect_any_instance_of(Projects::LfsPointers::LfsListService).to receive(:execute) + + subject.execute + end + + it 'links existent lfs objects to the project' do + expect_any_instance_of(Projects::LfsPointers::LfsLinkService).to receive(:execute) + + subject.execute + end + + it 'retrieves the download links of non existent objects' do + expect_any_instance_of(Projects::LfsPointers::LfsDownloadLinkListService).to receive(:execute).with(all_oids) + + subject.execute + end + end + + context 'when some lfs objects are linked' do + before do + allow_any_instance_of(Projects::LfsPointers::LfsLinkService).to receive(:execute).and_return(existing_lfs_objects.keys) + allow_any_instance_of(Projects::LfsPointers::LfsDownloadLinkListService).to receive(:execute).and_return(oid_download_links) + end + + it 'retrieves the download links of non existent objects' do + expect_any_instance_of(Projects::LfsPointers::LfsDownloadLinkListService).to receive(:execute).with(oids) + + subject.execute + end + end + + context 'when all lfs objects are linked' do + before do + allow_any_instance_of(Projects::LfsPointers::LfsLinkService).to receive(:execute).and_return(all_oids.keys) + allow_any_instance_of(Projects::LfsPointers::LfsDownloadLinkListService).to receive(:execute) + end + + it 'retrieves no download links' do + expect_any_instance_of(Projects::LfsPointers::LfsDownloadLinkListService).to receive(:execute).with({}).and_call_original + + expect(subject.execute).to be_empty + end + end + + context 'when lfsconfig file exists' do + before do + allow(project.repository).to receive(:lfsconfig_for).and_return("[lfs]\n\turl = #{lfs_endpoint}\n") + end + + context 'when url points to the same import url host' do + let(:lfs_endpoint) { "#{import_url}/different_endpoint" } + let(:service) { double } + + before do + allow(service).to receive(:execute) + end + it 'downloads lfs object using the new endpoint' do + expect(Projects::LfsPointers::LfsDownloadLinkListService).to receive(:new).with(project, remote_uri: remote_uri).and_return(service) + + subject.execute + end + + context 'when import url has credentials' do + let(:import_url) { 'http://user:password@www.gitlab.com/demo/repo.git'} + + it 'adds the credentials to the new endpoint' do + expect(Projects::LfsPointers::LfsDownloadLinkListService) + .to receive(:new).with(project, remote_uri: URI.parse("http://user:password@www.gitlab.com/demo/repo.git/different_endpoint")) + .and_return(service) + + subject.execute + end + + context 'when url has its own credentials' do + let(:lfs_endpoint) { "http://user1:password1@www.gitlab.com/demo/repo.git/different_endpoint" } + + it 'does not add the import url credentials' do + expect(Projects::LfsPointers::LfsDownloadLinkListService) + .to receive(:new).with(project, remote_uri: remote_uri) + .and_return(service) + + subject.execute + end + end + end + end + + context 'when url points to a third party service' do + let(:lfs_endpoint) { 'http://third_party_service.com/info/lfs/objects/' } + + it 'disables lfs from the project' do + expect(project.lfs_enabled?).to be_truthy + + subject.execute + + expect(project.lfs_enabled?).to be_falsey + end + + it 'does not download anything' do + expect_any_instance_of(Projects::LfsPointers::LfsListService).not_to receive(:execute) + + subject.execute + end + end + end + end + + describe '#default_endpoint_uri' do + let(:import_url) { 'http://www.gitlab.com/demo/repo' } + + it 'adds suffix .git if the url does not have it' do + expect(subject.send(:default_endpoint_uri).path).to match(/repo.git/) + end + end +end diff --git a/spec/services/projects/lfs_pointers/lfs_link_service_spec.rb b/spec/services/projects/lfs_pointers/lfs_link_service_spec.rb new file mode 100644 index 00000000000..b7b153655db --- /dev/null +++ b/spec/services/projects/lfs_pointers/lfs_link_service_spec.rb @@ -0,0 +1,33 @@ +require 'spec_helper' + +describe Projects::LfsPointers::LfsLinkService do + let!(:project) { create(:project, lfs_enabled: true) } + let!(:lfs_objects_project) { create_list(:lfs_objects_project, 2, project: project) } + let(:new_oids) { { 'oid1' => 123, 'oid2' => 125 } } + let(:all_oids) { LfsObject.pluck(:oid, :size).to_h.merge(new_oids) } + let(:new_lfs_object) { create(:lfs_object) } + let(:new_oid_list) { all_oids.merge(new_lfs_object.oid => new_lfs_object.size) } + + subject { described_class.new(project) } + + before do + allow(project).to receive(:lfs_enabled?).and_return(true) + end + + describe '#execute' do + it 'links existing lfs objects to the project' do + expect(project.all_lfs_objects.count).to eq 2 + + linked = subject.execute(new_oid_list.keys) + + expect(project.all_lfs_objects.count).to eq 3 + expect(linked.size).to eq 3 + end + + it 'returns linked oids' do + linked = lfs_objects_project.map(&:lfs_object).map(&:oid) << new_lfs_object.oid + + expect(subject.execute(new_oid_list.keys)).to eq linked + end + end +end diff --git a/spec/services/projects/transfer_service_spec.rb b/spec/services/projects/transfer_service_spec.rb index 3e6483d7e28..5100987c2fe 100644 --- a/spec/services/projects/transfer_service_spec.rb +++ b/spec/services/projects/transfer_service_spec.rb @@ -64,7 +64,7 @@ describe Projects::TransferService do it 'updates project full path in .git/config' do transfer_project(project, user, group) - expect(project.repository.rugged.config['gitlab.fullpath']).to eq "#{group.full_path}/#{project.path}" + expect(rugged_config['gitlab.fullpath']).to eq "#{group.full_path}/#{project.path}" end end @@ -84,7 +84,9 @@ describe Projects::TransferService do end def project_path(project) - project.repository.path_to_repo + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + project.repository.path_to_repo + end end def current_path @@ -101,7 +103,7 @@ describe Projects::TransferService do it 'rolls back project full path in .git/config' do attempt_project_transfer - expect(project.repository.rugged.config['gitlab.fullpath']).to eq project.full_path + expect(rugged_config['gitlab.fullpath']).to eq project.full_path end it "doesn't send move notifications" do @@ -264,4 +266,10 @@ describe Projects::TransferService do transfer_project(project, owner, group) end end + + def rugged_config + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + project.repository.rugged.config + end + end end diff --git a/spec/services/projects/update_service_spec.rb b/spec/services/projects/update_service_spec.rb index 3e6073b9861..ecf1ba05618 100644 --- a/spec/services/projects/update_service_spec.rb +++ b/spec/services/projects/update_service_spec.rb @@ -125,7 +125,7 @@ describe Projects::UpdateService do context 'when we update project but not enabling a wiki' do it 'does not try to create an empty wiki' do - FileUtils.rm_rf(project.wiki.repository.path) + Gitlab::Shell.new.rm_directory(project.repository_storage, project.wiki.path) result = update_project(project, user, { name: 'test1' }) @@ -146,7 +146,7 @@ describe Projects::UpdateService do context 'when enabling a wiki' do it 'creates a wiki' do project.project_feature.update(wiki_access_level: ProjectFeature::DISABLED) - FileUtils.rm_rf(project.wiki.repository.path) + Gitlab::Shell.new.rm_directory(project.repository_storage, project.wiki.path) result = update_project(project, user, project_feature_attributes: { wiki_access_level: ProjectFeature::ENABLED }) @@ -275,6 +275,10 @@ describe Projects::UpdateService do it { is_expected.to eq(false) } end + context 'when auto devops is nil' do + it { is_expected.to eq(false) } + end + context 'when auto devops is explicitly enabled' do before do project.create_auto_devops!(enabled: true) diff --git a/spec/services/test_hooks/project_service_spec.rb b/spec/services/test_hooks/project_service_spec.rb index 962b9f40c4f..19e1c5ff3b2 100644 --- a/spec/services/test_hooks/project_service_spec.rb +++ b/spec/services/test_hooks/project_service_spec.rb @@ -6,13 +6,19 @@ describe TestHooks::ProjectService do describe '#execute' do let(:project) { create(:project, :repository) } let(:hook) { create(:project_hook, project: project) } + let(:trigger) { 'not_implemented_events' } let(:service) { described_class.new(hook, current_user, trigger) } let(:sample_data) { { data: 'sample' } } let(:success_result) { { status: :success, http_status: 200, message: 'ok' } } - context 'hook with not implemented test' do - let(:trigger) { 'not_implemented_events' } + it 'allows to set a custom project' do + project = double + service.project = project + + expect(service.project).to eq(project) + end + context 'hook with not implemented test' do it 'returns error message' do expect(hook).not_to receive(:execute) expect(service.execute).to include({ status: :error, message: 'Testing not available for this hook' }) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index d3de2331244..e093444121a 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -133,6 +133,10 @@ RSpec.configure do |config| RequestStore.clear! end + config.after(:example) do + Fog.unmock! if Fog.mock? + end + config.before(:example, :mailer) do reset_delivered_emails! end diff --git a/spec/support/controllers/githubish_import_controller_shared_examples.rb b/spec/support/controllers/githubish_import_controller_shared_examples.rb index 368439aa5b0..1c1b68c12a2 100644 --- a/spec/support/controllers/githubish_import_controller_shared_examples.rb +++ b/spec/support/controllers/githubish_import_controller_shared_examples.rb @@ -118,14 +118,19 @@ shared_examples 'a GitHub-ish import controller: POST create' do expect(response).to have_gitlab_http_status(200) end - it 'returns 422 response when the project could not be imported' do + it 'returns 422 response with the base error when the project could not be imported' do + project = build(:project) + project.errors.add(:name, 'is invalid') + project.errors.add(:path, 'is old') + allow(Gitlab::LegacyGithubImport::ProjectCreator) .to receive(:new).with(provider_repo, provider_repo.name, user.namespace, user, access_params, type: provider) - .and_return(double(execute: build(:project))) + .and_return(double(execute: project)) post :create, format: :json expect(response).to have_gitlab_http_status(422) + expect(json_response['errors']).to eq('Name is invalid, Path is old') end context "when the repository owner is the provider user" do diff --git a/spec/support/helpers/cycle_analytics_helpers.rb b/spec/support/helpers/cycle_analytics_helpers.rb index 55359d36597..32d9807f06a 100644 --- a/spec/support/helpers/cycle_analytics_helpers.rb +++ b/spec/support/helpers/cycle_analytics_helpers.rb @@ -4,12 +4,12 @@ module CycleAnalyticsHelpers create_commit("Commit for ##{issue.iid}", issue.project, user, branch_name) end - def create_commit(message, project, user, branch_name, count: 1) + def create_commit(message, project, user, branch_name, count: 1, commit_time: nil, skip_push_handler: false) repository = project.repository - oldrev = repository.commit(branch_name).sha + oldrev = repository.commit(branch_name)&.sha || Gitlab::Git::BLANK_SHA if Timecop.frozen? && Gitlab::GitalyClient.feature_enabled?(:operation_user_commit_files) - mock_gitaly_multi_action_dates(repository.raw) + mock_gitaly_multi_action_dates(repository.raw, commit_time) end commit_shas = Array.new(count) do |index| @@ -19,6 +19,8 @@ module CycleAnalyticsHelpers commit_sha end + return if skip_push_handler + GitPushService.new(project, user, oldrev: oldrev, @@ -44,13 +46,11 @@ module CycleAnalyticsHelpers project.repository.add_branch(user, source_branch, 'master') end - sha = project.repository.create_file( - user, - generate(:branch), - 'content', - message: commit_message, - branch_name: source_branch) - project.repository.commit(sha) + # Cycle analytic specs often test with frozen times, which causes metrics to be + # pinned to the current time. For example, in the plan stage, we assume that an issue + # milestone has been created before any code has been written. We add a second + # to ensure that the plan time is positive. + create_commit(commit_message, project, user, source_branch, commit_time: Time.now + 1.second, skip_push_handler: true) opts = { title: 'Awesome merge_request', @@ -116,14 +116,18 @@ module CycleAnalyticsHelpers protected: false) end - def mock_gitaly_multi_action_dates(raw_repository) + def mock_gitaly_multi_action_dates(raw_repository, commit_time) allow(raw_repository).to receive(:multi_action).and_wrap_original do |m, *args| - new_date = Time.now + new_date = commit_time || Time.now branch_update = m.call(*args) if branch_update.newrev _, opts = args - commit = raw_repository.commit(branch_update.newrev).rugged_commit + + commit = Gitlab::GitalyClient::StorageSettings.allow_disk_access do + raw_repository.commit(branch_update.newrev).rugged_commit + end + branch_update.newrev = commit.amend( update_ref: "#{Gitlab::Git::BRANCH_REF_PREFIX}#{opts[:branch_name]}", author: commit.author.merge(time: new_date), diff --git a/spec/support/helpers/graphql_helpers.rb b/spec/support/helpers/graphql_helpers.rb new file mode 100644 index 00000000000..30ff9a1196a --- /dev/null +++ b/spec/support/helpers/graphql_helpers.rb @@ -0,0 +1,90 @@ +module GraphqlHelpers + # makes an underscored string look like a fieldname + # "merge_request" => "mergeRequest" + def self.fieldnamerize(underscored_field_name) + graphql_field_name = underscored_field_name.to_s.camelize + graphql_field_name[0] = graphql_field_name[0].downcase + + graphql_field_name + end + + # Run a loader's named resolver + def resolve(resolver_class, obj: nil, args: {}, ctx: {}) + resolver_class.new(object: obj, context: ctx).resolve(args) + end + + # Runs a block inside a BatchLoader::Executor wrapper + def batch(max_queries: nil, &blk) + wrapper = proc do + begin + BatchLoader::Executor.ensure_current + yield + ensure + BatchLoader::Executor.clear_current + end + end + + if max_queries + result = nil + expect { result = wrapper.call }.not_to exceed_query_limit(max_queries) + result + else + wrapper.call + end + end + + def graphql_query_for(name, attributes = {}, fields = nil) + fields ||= all_graphql_fields_for(name.classify) + attributes = attributes_to_graphql(attributes) + <<~QUERY + { + #{name}(#{attributes}) { + #{fields} + } + } + QUERY + end + + def all_graphql_fields_for(class_name) + type = GitlabSchema.types[class_name.to_s] + return "" unless type + + type.fields.map do |name, field| + if scalar?(field) + name + else + "#{name} { #{all_graphql_fields_for(field_type(field))} }" + end + end.join("\n") + end + + def attributes_to_graphql(attributes) + attributes.map do |name, value| + "#{GraphqlHelpers.fieldnamerize(name.to_s)}: \"#{value}\"" + end.join(", ") + end + + def post_graphql(query, current_user: nil) + post api('/', current_user, version: 'graphql'), query: query + end + + def graphql_data + json_response['data'] + end + + def graphql_errors + json_response['data'] + end + + def scalar?(field) + field_type(field).kind.scalar? + end + + def field_type(field) + if field.type.respond_to?(:of_type) + field.type.of_type + else + field.type + end + end +end diff --git a/spec/support/helpers/query_recorder.rb b/spec/support/helpers/query_recorder.rb index 28536bbef5e..7ce63375d34 100644 --- a/spec/support/helpers/query_recorder.rb +++ b/spec/support/helpers/query_recorder.rb @@ -1,10 +1,11 @@ module ActiveRecord class QueryRecorder - attr_reader :log, :cached + attr_reader :log, :skip_cached, :cached - def initialize(&block) + def initialize(skip_cached: true, &block) @log = [] @cached = [] + @skip_cached = skip_cached ActiveSupport::Notifications.subscribed(method(:callback), 'sql.active_record', &block) end @@ -16,7 +17,7 @@ module ActiveRecord def callback(name, start, finish, message_id, values) show_backtrace(values) if ENV['QUERY_RECORDER_DEBUG'] - if values[:name]&.include?("CACHE") + if values[:name]&.include?("CACHE") && skip_cached @cached << values[:sql] elsif !values[:name]&.include?("SCHEMA") @log << values[:sql] diff --git a/spec/support/helpers/stub_object_storage.rb b/spec/support/helpers/stub_object_storage.rb index 19d744b959a..bceaf8277ee 100644 --- a/spec/support/helpers/stub_object_storage.rb +++ b/spec/support/helpers/stub_object_storage.rb @@ -45,4 +45,16 @@ module StubObjectStorage remote_directory: 'uploads', **params) end + + def stub_object_storage_multipart_init(endpoint, upload_id = "upload_id") + stub_request(:post, %r{\A#{endpoint}tmp/uploads/[a-z0-9-]*\?uploads\z}) + .to_return status: 200, body: <<-EOS.strip_heredoc + <?xml version="1.0" encoding="UTF-8"?> + <InitiateMultipartUploadResult xmlns="http://s3.amazonaws.com/doc/2006-03-01/"> + <Bucket>example-bucket</Bucket> + <Key>example-object</Key> + <UploadId>#{upload_id}</UploadId> + </InitiateMultipartUploadResult> + EOS + end end diff --git a/spec/support/matchers/email_matchers.rb b/spec/support/matchers/email_matchers.rb deleted file mode 100644 index d9d59ec12ec..00000000000 --- a/spec/support/matchers/email_matchers.rb +++ /dev/null @@ -1,5 +0,0 @@ -RSpec::Matchers.define :have_html_escaped_body_text do |expected| - match do |actual| - expect(actual).to have_body_text(ERB::Util.html_escape(expected)) - end -end diff --git a/spec/support/matchers/exceed_query_limit.rb b/spec/support/matchers/exceed_query_limit.rb index 88d22a3ddd9..cd042401f3a 100644 --- a/spec/support/matchers/exceed_query_limit.rb +++ b/spec/support/matchers/exceed_query_limit.rb @@ -1,17 +1,4 @@ -RSpec::Matchers.define :exceed_query_limit do |expected| - supports_block_expectations - - match do |block| - @subject_block = block - actual_count > expected_count + threshold - end - - failure_message_when_negated do |actual| - threshold_message = threshold > 0 ? " (+#{@threshold})" : '' - counts = "#{expected_count}#{threshold_message}" - "Expected a maximum of #{counts} queries, got #{actual_count}:\n\n#{log_message}" - end - +module ExceedQueryLimitHelpers def with_threshold(threshold) @threshold = threshold self @@ -43,7 +30,7 @@ RSpec::Matchers.define :exceed_query_limit do |expected| end def recorder - @recorder ||= ActiveRecord::QueryRecorder.new(&@subject_block) + @recorder ||= ActiveRecord::QueryRecorder.new(skip_cached: skip_cached, &@subject_block) end def count_queries(queries) @@ -61,4 +48,52 @@ RSpec::Matchers.define :exceed_query_limit do |expected| @recorder.log_message end end + + def skip_cached + true + end + + def verify_count(&block) + @subject_block = block + actual_count > expected_count + threshold + end + + def failure_message + threshold_message = threshold > 0 ? " (+#{@threshold})" : '' + counts = "#{expected_count}#{threshold_message}" + "Expected a maximum of #{counts} queries, got #{actual_count}:\n\n#{log_message}" + end +end + +RSpec::Matchers.define :exceed_all_query_limit do |expected| + supports_block_expectations + + include ExceedQueryLimitHelpers + + match do |block| + verify_count(&block) + end + + failure_message_when_negated do |actual| + failure_message + end + + def skip_cached + false + end +end + +# Excludes cached methods from the query count +RSpec::Matchers.define :exceed_query_limit do |expected| + supports_block_expectations + + include ExceedQueryLimitHelpers + + match do |block| + verify_count(&block) + end + + failure_message_when_negated do |actual| + failure_message + end end diff --git a/spec/support/matchers/graphql_matchers.rb b/spec/support/matchers/graphql_matchers.rb new file mode 100644 index 00000000000..ba7a1c8cde0 --- /dev/null +++ b/spec/support/matchers/graphql_matchers.rb @@ -0,0 +1,40 @@ +RSpec::Matchers.define :require_graphql_authorizations do |*expected| + match do |field| + field_definition = field.metadata[:type_class] + expect(field_definition).to respond_to(:required_permissions) + expect(field_definition.required_permissions).to contain_exactly(*expected) + end +end + +RSpec::Matchers.define :have_graphql_fields do |*expected| + match do |kls| + field_names = expected.map { |name| GraphqlHelpers.fieldnamerize(name) } + expect(kls.fields.keys).to contain_exactly(*field_names) + end +end + +RSpec::Matchers.define :have_graphql_arguments do |*expected| + include GraphqlHelpers + + match do |field| + argument_names = expected.map { |name| GraphqlHelpers.fieldnamerize(name) } + expect(field.arguments.keys).to contain_exactly(*argument_names) + end +end + +RSpec::Matchers.define :have_graphql_type do |expected| + match do |field| + expect(field.type).to eq(expected.to_graphql) + end +end + +RSpec::Matchers.define :have_graphql_resolver do |expected| + match do |field| + case expected + when Method + expect(field.metadata[:type_class].resolve_proc).to eq(expected) + else + expect(field.metadata[:type_class].resolver).to eq(expected) + end + end +end diff --git a/spec/support/shared_examples/ci_trace_shared_examples.rb b/spec/support/shared_examples/ci_trace_shared_examples.rb index 21c6f3c829f..6dbe0f6f980 100644 --- a/spec/support/shared_examples/ci_trace_shared_examples.rb +++ b/spec/support/shared_examples/ci_trace_shared_examples.rb @@ -227,6 +227,42 @@ shared_examples_for 'common trace features' do end end end + + describe '#archive!' do + subject { trace.archive! } + + context 'when build status is success' do + let!(:build) { create(:ci_build, :success, :trace_live) } + + it 'does not have an archived trace yet' do + expect(build.job_artifacts_trace).to be_nil + end + + context 'when archives' do + it 'has an archived trace' do + subject + + build.reload + expect(build.job_artifacts_trace).to be_exist + end + + context 'when another process has already been archiving', :clean_gitlab_redis_shared_state do + before do + Gitlab::ExclusiveLease.new("trace:archive:#{trace.job.id}", timeout: 1.hour).try_obtain + end + + it 'blocks concurrent archiving' do + expect(Rails.logger).to receive(:error).with('Cannot obtain an exclusive lease. There must be another instance already in execution.') + + subject + + build.reload + expect(build.job_artifacts_trace).to be_nil + end + end + end + end + end end shared_examples_for 'trace with disabled live trace feature' do diff --git a/spec/support/shared_examples/file_finder.rb b/spec/support/shared_examples/file_finder.rb new file mode 100644 index 00000000000..ef144bdf61c --- /dev/null +++ b/spec/support/shared_examples/file_finder.rb @@ -0,0 +1,21 @@ +shared_examples 'file finder' do + let(:query) { 'files' } + let(:search_results) { subject.find(query) } + + it 'finds by name' do + filename, blob = search_results.find { |_, blob| blob.filename == expected_file_by_name } + expect(filename).to eq(expected_file_by_name) + expect(blob).to be_a(Gitlab::SearchResults::FoundBlob) + expect(blob.ref).to eq(subject.ref) + expect(blob.data).not_to be_empty + end + + it 'finds by content' do + filename, blob = search_results.find { |_, blob| blob.filename == expected_file_by_content } + + expect(filename).to eq(expected_file_by_content) + expect(blob).to be_a(Gitlab::SearchResults::FoundBlob) + expect(blob.ref).to eq(subject.ref) + expect(blob.data).not_to be_empty + end +end diff --git a/spec/support/shared_examples/models/atomic_internal_id_spec.rb b/spec/support/shared_examples/models/atomic_internal_id_spec.rb index 6a6e13418a9..7ab1041d17c 100644 --- a/spec/support/shared_examples/models/atomic_internal_id_spec.rb +++ b/spec/support/shared_examples/models/atomic_internal_id_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -shared_examples_for 'AtomicInternalId' do +shared_examples_for 'AtomicInternalId' do |validate_presence: true| describe '.has_internal_id' do describe 'Module inclusion' do subject { described_class } @@ -9,14 +9,31 @@ shared_examples_for 'AtomicInternalId' do end describe 'Validation' do - subject { instance } - before do - allow(InternalId).to receive(:generate_next).and_return(nil) + allow_any_instance_of(described_class).to receive(:"ensure_#{scope}_#{internal_id_attribute}!") + + instance.valid? end - it { is_expected.to validate_presence_of(internal_id_attribute) } - it { is_expected.to validate_numericality_of(internal_id_attribute) } + context 'when presence validation is required' do + before do + skip unless validate_presence + end + + it 'validates presence' do + expect(instance.errors[internal_id_attribute]).to include("can't be blank") + end + end + + context 'when presence validation is not required' do + before do + skip if validate_presence + end + + it 'does not validate presence' do + expect(instance.errors[internal_id_attribute]).to be_empty + end + end end describe 'Creating an instance' do diff --git a/spec/support/shared_examples/notify_shared_examples.rb b/spec/support/shared_examples/notify_shared_examples.rb index 43fdaddf545..d176d3fa425 100644 --- a/spec/support/shared_examples/notify_shared_examples.rb +++ b/spec/support/shared_examples/notify_shared_examples.rb @@ -212,7 +212,7 @@ shared_examples 'a note email' do end it 'contains the message from the note' do - is_expected.to have_html_escaped_body_text note.note + is_expected.to have_body_text note.note end it 'does not contain note author' do @@ -225,7 +225,7 @@ shared_examples 'a note email' do end it 'contains a link to note author' do - is_expected.to have_html_escaped_body_text note.author_name + is_expected.to have_body_text note.author_name end end end diff --git a/spec/support/shared_examples/requests/graphql_shared_examples.rb b/spec/support/shared_examples/requests/graphql_shared_examples.rb new file mode 100644 index 00000000000..9b2b74593a5 --- /dev/null +++ b/spec/support/shared_examples/requests/graphql_shared_examples.rb @@ -0,0 +1,11 @@ +require 'spec_helper' + +shared_examples 'a working graphql query' do + include GraphqlHelpers + + it 'is returns a successfull response', :aggregate_failures do + expect(response).to be_success + expect(graphql_errors['errors']).to be_nil + expect(json_response.keys).to include('data') + end +end diff --git a/spec/support/trace/trace_helpers.rb b/spec/support/trace/trace_helpers.rb new file mode 100644 index 00000000000..c7802bbcb94 --- /dev/null +++ b/spec/support/trace/trace_helpers.rb @@ -0,0 +1,27 @@ +module TraceHelpers + def create_legacy_trace(build, content) + File.open(legacy_trace_path(build), 'wb') { |stream| stream.write(content) } + end + + def create_legacy_trace_in_db(build, content) + build.update_column(:trace, content) + end + + def legacy_trace_path(build) + legacy_trace_dir = File.join(Settings.gitlab_ci.builds_path, + build.created_at.utc.strftime("%Y_%m"), + build.project_id.to_s) + + FileUtils.mkdir_p(legacy_trace_dir) + + File.join(legacy_trace_dir, "#{build.id}.log") + end + + def archived_trace_path(job_artifact) + disk_hash = Digest::SHA2.hexdigest(job_artifact.project_id.to_s) + creation_date = job_artifact.created_at.utc.strftime('%Y_%m_%d') + + File.join(Gitlab.config.artifacts.path, disk_hash[0..1], disk_hash[2..3], disk_hash, + creation_date, job_artifact.job_id.to_s, job_artifact.id.to_s, 'job.log') + end +end diff --git a/spec/tasks/gitlab/backup_rake_spec.rb b/spec/tasks/gitlab/backup_rake_spec.rb index c9252bebb2e..93a436cb2b5 100644 --- a/spec/tasks/gitlab/backup_rake_spec.rb +++ b/spec/tasks/gitlab/backup_rake_spec.rb @@ -101,7 +101,9 @@ describe 'gitlab:app namespace rake task' do before do stub_env('SKIP', 'db') - path = File.join(project.repository.path_to_repo, 'custom_hooks') + path = Gitlab::GitalyClient::StorageSettings.allow_disk_access do + File.join(project.repository.path_to_repo, 'custom_hooks') + end FileUtils.mkdir_p(path) FileUtils.touch(File.join(path, "dummy.txt")) end @@ -122,7 +124,10 @@ describe 'gitlab:app namespace rake task' do expect { run_rake_task('gitlab:backup:create') }.to output.to_stdout expect { run_rake_task('gitlab:backup:restore') }.to output.to_stdout - expect(Dir.entries(File.join(project.repository.path, 'custom_hooks'))).to include("dummy.txt") + repo_path = Gitlab::GitalyClient::StorageSettings.allow_disk_access do + project.repository.path + end + expect(Dir.entries(File.join(repo_path, 'custom_hooks'))).to include("dummy.txt") end end @@ -243,10 +248,12 @@ describe 'gitlab:app namespace rake task' do FileUtils.mkdir_p(b_storage_dir) # Even when overriding the storage, we have to move it there, so it exists - FileUtils.mv( - File.join(Settings.absolute(storages['default'].legacy_disk_path), project_b.repository.disk_path + '.git'), - Rails.root.join(storages['test_second_storage'].legacy_disk_path, project_b.repository.disk_path + '.git') - ) + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + FileUtils.mv( + File.join(Settings.absolute(storages['default'].legacy_disk_path), project_b.repository.disk_path + '.git'), + Rails.root.join(storages['test_second_storage'].legacy_disk_path, project_b.repository.disk_path + '.git') + ) + end expect { run_rake_task('gitlab:backup:create') }.to output.to_stdout diff --git a/spec/uploaders/object_storage_spec.rb b/spec/uploaders/object_storage_spec.rb index 2dd0925a8e6..4503288e410 100644 --- a/spec/uploaders/object_storage_spec.rb +++ b/spec/uploaders/object_storage_spec.rb @@ -355,14 +355,10 @@ describe ObjectStorage do end describe '.workhorse_authorize' do - subject { uploader_class.workhorse_authorize } + let(:has_length) { true } + let(:maximum_size) { nil } - before do - # ensure that we use regular Fog libraries - # other tests might call `Fog.mock!` and - # it will make tests to fail - Fog.unmock! - end + subject { uploader_class.workhorse_authorize(has_length: has_length, maximum_size: maximum_size) } shared_examples 'uses local storage' do it "returns temporary path" do @@ -371,10 +367,6 @@ describe ObjectStorage do expect(subject[:TempPath]).to start_with(uploader_class.root) expect(subject[:TempPath]).to include(described_class::TMP_UPLOAD_PATH) end - - it "does not return remote store" do - is_expected.not_to have_key('RemoteObject') - end end shared_examples 'uses remote storage' do @@ -383,7 +375,7 @@ describe ObjectStorage do expect(subject[:RemoteObject]).to have_key(:ID) expect(subject[:RemoteObject]).to include(Timeout: a_kind_of(Integer)) - expect(subject[:RemoteObject][:Timeout]).to be(ObjectStorage::DIRECT_UPLOAD_TIMEOUT) + expect(subject[:RemoteObject][:Timeout]).to be(ObjectStorage::DirectUpload::TIMEOUT) expect(subject[:RemoteObject]).to have_key(:GetURL) expect(subject[:RemoteObject]).to have_key(:DeleteURL) expect(subject[:RemoteObject]).to have_key(:StoreURL) @@ -391,9 +383,31 @@ describe ObjectStorage do expect(subject[:RemoteObject][:DeleteURL]).to include(described_class::TMP_UPLOAD_PATH) expect(subject[:RemoteObject][:StoreURL]).to include(described_class::TMP_UPLOAD_PATH) end + end - it "does not return local store" do - is_expected.not_to have_key('TempPath') + shared_examples 'uses remote storage with multipart uploads' do + it_behaves_like 'uses remote storage' do + it "returns multipart upload" do + is_expected.to have_key(:RemoteObject) + + expect(subject[:RemoteObject]).to have_key(:MultipartUpload) + expect(subject[:RemoteObject][:MultipartUpload]).to have_key(:PartSize) + expect(subject[:RemoteObject][:MultipartUpload]).to have_key(:PartURLs) + expect(subject[:RemoteObject][:MultipartUpload]).to have_key(:CompleteURL) + expect(subject[:RemoteObject][:MultipartUpload]).to have_key(:AbortURL) + expect(subject[:RemoteObject][:MultipartUpload][:PartURLs]).to all(include(described_class::TMP_UPLOAD_PATH)) + expect(subject[:RemoteObject][:MultipartUpload][:CompleteURL]).to include(described_class::TMP_UPLOAD_PATH) + expect(subject[:RemoteObject][:MultipartUpload][:AbortURL]).to include(described_class::TMP_UPLOAD_PATH) + end + end + end + + shared_examples 'uses remote storage without multipart uploads' do + it_behaves_like 'uses remote storage' do + it "does not return multipart upload" do + is_expected.to have_key(:RemoteObject) + expect(subject[:RemoteObject]).not_to have_key(:MultipartUpload) + end end end @@ -416,6 +430,8 @@ describe ObjectStorage do end context 'uses AWS' do + let(:storage_url) { "https://uploads.s3-eu-central-1.amazonaws.com/" } + before do expect(uploader_class).to receive(:object_store_credentials) do { provider: "AWS", @@ -425,18 +441,40 @@ describe ObjectStorage do end end - it_behaves_like 'uses remote storage' do - let(:storage_url) { "https://uploads.s3-eu-central-1.amazonaws.com/" } + context 'for known length' do + it_behaves_like 'uses remote storage without multipart uploads' do + it 'returns links for S3' do + expect(subject[:RemoteObject][:GetURL]).to start_with(storage_url) + expect(subject[:RemoteObject][:DeleteURL]).to start_with(storage_url) + expect(subject[:RemoteObject][:StoreURL]).to start_with(storage_url) + end + end + end - it 'returns links for S3' do - expect(subject[:RemoteObject][:GetURL]).to start_with(storage_url) - expect(subject[:RemoteObject][:DeleteURL]).to start_with(storage_url) - expect(subject[:RemoteObject][:StoreURL]).to start_with(storage_url) + context 'for unknown length' do + let(:has_length) { false } + let(:maximum_size) { 1.gigabyte } + + before do + stub_object_storage_multipart_init(storage_url) + end + + it_behaves_like 'uses remote storage with multipart uploads' do + it 'returns links for S3' do + expect(subject[:RemoteObject][:GetURL]).to start_with(storage_url) + expect(subject[:RemoteObject][:DeleteURL]).to start_with(storage_url) + expect(subject[:RemoteObject][:StoreURL]).to start_with(storage_url) + expect(subject[:RemoteObject][:MultipartUpload][:PartURLs]).to all(start_with(storage_url)) + expect(subject[:RemoteObject][:MultipartUpload][:CompleteURL]).to start_with(storage_url) + expect(subject[:RemoteObject][:MultipartUpload][:AbortURL]).to start_with(storage_url) + end end end end context 'uses Google' do + let(:storage_url) { "https://storage.googleapis.com/uploads/" } + before do expect(uploader_class).to receive(:object_store_credentials) do { provider: "Google", @@ -445,36 +483,71 @@ describe ObjectStorage do end end - it_behaves_like 'uses remote storage' do - let(:storage_url) { "https://storage.googleapis.com/uploads/" } + context 'for known length' do + it_behaves_like 'uses remote storage without multipart uploads' do + it 'returns links for Google Cloud' do + expect(subject[:RemoteObject][:GetURL]).to start_with(storage_url) + expect(subject[:RemoteObject][:DeleteURL]).to start_with(storage_url) + expect(subject[:RemoteObject][:StoreURL]).to start_with(storage_url) + end + end + end + + context 'for unknown length' do + let(:has_length) { false } + let(:maximum_size) { 1.gigabyte } - it 'returns links for Google Cloud' do - expect(subject[:RemoteObject][:GetURL]).to start_with(storage_url) - expect(subject[:RemoteObject][:DeleteURL]).to start_with(storage_url) - expect(subject[:RemoteObject][:StoreURL]).to start_with(storage_url) + it_behaves_like 'uses remote storage without multipart uploads' do + it 'returns links for Google Cloud' do + expect(subject[:RemoteObject][:GetURL]).to start_with(storage_url) + expect(subject[:RemoteObject][:DeleteURL]).to start_with(storage_url) + expect(subject[:RemoteObject][:StoreURL]).to start_with(storage_url) + end end end end context 'uses GDK/minio' do + let(:storage_url) { "http://minio:9000/uploads/" } + before do expect(uploader_class).to receive(:object_store_credentials) do { provider: "AWS", aws_access_key_id: "AWS_ACCESS_KEY_ID", aws_secret_access_key: "AWS_SECRET_ACCESS_KEY", - endpoint: 'http://127.0.0.1:9000', + endpoint: 'http://minio:9000', path_style: true, region: "gdk" } end end - it_behaves_like 'uses remote storage' do - let(:storage_url) { "http://127.0.0.1:9000/uploads/" } + context 'for known length' do + it_behaves_like 'uses remote storage without multipart uploads' do + it 'returns links for S3' do + expect(subject[:RemoteObject][:GetURL]).to start_with(storage_url) + expect(subject[:RemoteObject][:DeleteURL]).to start_with(storage_url) + expect(subject[:RemoteObject][:StoreURL]).to start_with(storage_url) + end + end + end + + context 'for unknown length' do + let(:has_length) { false } + let(:maximum_size) { 1.gigabyte } - it 'returns links for S3' do - expect(subject[:RemoteObject][:GetURL]).to start_with(storage_url) - expect(subject[:RemoteObject][:DeleteURL]).to start_with(storage_url) - expect(subject[:RemoteObject][:StoreURL]).to start_with(storage_url) + before do + stub_object_storage_multipart_init(storage_url) + end + + it_behaves_like 'uses remote storage with multipart uploads' do + it 'returns links for S3' do + expect(subject[:RemoteObject][:GetURL]).to start_with(storage_url) + expect(subject[:RemoteObject][:DeleteURL]).to start_with(storage_url) + expect(subject[:RemoteObject][:StoreURL]).to start_with(storage_url) + expect(subject[:RemoteObject][:MultipartUpload][:PartURLs]).to all(start_with(storage_url)) + expect(subject[:RemoteObject][:MultipartUpload][:CompleteURL]).to start_with(storage_url) + expect(subject[:RemoteObject][:MultipartUpload][:AbortURL]).to start_with(storage_url) + end end end end @@ -659,4 +732,26 @@ describe ObjectStorage do end end end + + describe '#retrieve_from_store!' do + [:group, :project, :user].each do |model| + context "for #{model}s" do + let(:models) { create_list(model, 3, :with_avatar).map(&:reload) } + let(:avatars) { models.map(&:avatar) } + + it 'batches fetching uploads from the database' do + # Ensure that these are all created and fully loaded before we start + # running queries for avatars + models + + expect { avatars }.not_to exceed_query_limit(1) + end + + it 'fetches a unique upload for each model' do + expect(avatars.map(&:url).uniq).to eq(avatars.map(&:url)) + expect(avatars.map(&:upload).uniq).to eq(avatars.map(&:upload)) + end + end + end + end end diff --git a/spec/uploaders/workers/object_storage/background_move_worker_spec.rb b/spec/uploaders/workers/object_storage/background_move_worker_spec.rb index b34f427fd8a..95813d15e52 100644 --- a/spec/uploaders/workers/object_storage/background_move_worker_spec.rb +++ b/spec/uploaders/workers/object_storage/background_move_worker_spec.rb @@ -125,8 +125,10 @@ describe ObjectStorage::BackgroundMoveWorker do it "migrates file to remote storage" do perform + project.reload + BatchLoader::Executor.clear_current - expect(project.reload.avatar.file_storage?).to be_falsey + expect(project.avatar).not_to be_file_storage end end @@ -137,7 +139,7 @@ describe ObjectStorage::BackgroundMoveWorker do it "migrates file to remote storage" do perform - expect(project.reload.avatar.file_storage?).to be_falsey + expect(project.reload.avatar).not_to be_file_storage end end end diff --git a/spec/workers/ci/archive_traces_cron_worker_spec.rb b/spec/workers/ci/archive_traces_cron_worker_spec.rb new file mode 100644 index 00000000000..9af51b7d4d8 --- /dev/null +++ b/spec/workers/ci/archive_traces_cron_worker_spec.rb @@ -0,0 +1,59 @@ +require 'spec_helper' + +describe Ci::ArchiveTracesCronWorker do + subject { described_class.new.perform } + + before do + stub_feature_flags(ci_enable_live_trace: true) + end + + shared_examples_for 'archives trace' do + it do + subject + + build.reload + expect(build.job_artifacts_trace).to be_exist + end + end + + shared_examples_for 'does not archive trace' do + it do + subject + + build.reload + expect(build.job_artifacts_trace).to be_nil + end + end + + context 'when a job was succeeded' do + let!(:build) { create(:ci_build, :success, :trace_live) } + + it_behaves_like 'archives trace' + + context 'when archive raised an exception' do + let!(:build) { create(:ci_build, :success, :trace_artifact, :trace_live) } + let!(:build2) { create(:ci_build, :success, :trace_live) } + + it 'archives valid targets' do + expect(Rails.logger).to receive(:error).with("Failed to archive stale live trace. id: #{build.id} message: Already archived") + + subject + + build2.reload + expect(build2.job_artifacts_trace).to be_exist + end + end + end + + context 'when a job was cancelled' do + let!(:build) { create(:ci_build, :canceled, :trace_live) } + + it_behaves_like 'archives trace' + end + + context 'when a job is running' do + let!(:build) { create(:ci_build, :running, :trace_live) } + + it_behaves_like 'does not archive trace' + end +end diff --git a/spec/workers/git_garbage_collect_worker_spec.rb b/spec/workers/git_garbage_collect_worker_spec.rb index 74539a7e493..f44b4edc305 100644 --- a/spec/workers/git_garbage_collect_worker_spec.rb +++ b/spec/workers/git_garbage_collect_worker_spec.rb @@ -104,7 +104,7 @@ describe GitGarbageCollectWorker do it_should_behave_like 'flushing ref caches', true end - context "with Gitaly turned off", :skip_gitaly_mock do + context "with Gitaly turned off", :disable_gitaly do it_should_behave_like 'flushing ref caches', false end diff --git a/spec/workers/gitlab/github_import/stage/import_lfs_objects_worker_spec.rb b/spec/workers/gitlab/github_import/stage/import_lfs_objects_worker_spec.rb new file mode 100644 index 00000000000..b19884d7991 --- /dev/null +++ b/spec/workers/gitlab/github_import/stage/import_lfs_objects_worker_spec.rb @@ -0,0 +1,28 @@ +require 'spec_helper' + +describe Gitlab::GithubImport::Stage::ImportLfsObjectsWorker do + let(:project) { create(:project) } + let(:worker) { described_class.new } + + describe '#import' do + it 'imports all the lfs objects' do + importer = double(:importer) + waiter = Gitlab::JobWaiter.new(2, '123') + + expect(Gitlab::GithubImport::Importer::LfsObjectsImporter) + .to receive(:new) + .with(project, nil) + .and_return(importer) + + expect(importer) + .to receive(:execute) + .and_return(waiter) + + expect(Gitlab::GithubImport::AdvanceStageWorker) + .to receive(:perform_async) + .with(project.id, { '123' => 2 }, :finish) + + worker.import(project) + end + end +end diff --git a/spec/workers/gitlab/github_import/stage/import_notes_worker_spec.rb b/spec/workers/gitlab/github_import/stage/import_notes_worker_spec.rb index 098d2d55386..94cff9e4e80 100644 --- a/spec/workers/gitlab/github_import/stage/import_notes_worker_spec.rb +++ b/spec/workers/gitlab/github_import/stage/import_notes_worker_spec.rb @@ -21,7 +21,7 @@ describe Gitlab::GithubImport::Stage::ImportNotesWorker do expect(Gitlab::GithubImport::AdvanceStageWorker) .to receive(:perform_async) - .with(project.id, { '123' => 2 }, :finish) + .with(project.id, { '123' => 2 }, :lfs_objects) worker.import(client, project) end diff --git a/spec/workers/repository_check/single_repository_worker_spec.rb b/spec/workers/repository_check/single_repository_worker_spec.rb index a021235aed6..22fc64c1536 100644 --- a/spec/workers/repository_check/single_repository_worker_spec.rb +++ b/spec/workers/repository_check/single_repository_worker_spec.rb @@ -88,7 +88,9 @@ describe RepositoryCheck::SingleRepositoryWorker do end def break_wiki(project) - break_repo(wiki_path(project)) + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + break_repo(wiki_path(project)) + end end def wiki_path(project) @@ -96,7 +98,9 @@ describe RepositoryCheck::SingleRepositoryWorker do end def break_project(project) - break_repo(project.repository.path_to_repo) + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + break_repo(project.repository.path_to_repo) + end end def break_repo(repo) diff --git a/spec/workers/stuck_ci_jobs_worker_spec.rb b/spec/workers/stuck_ci_jobs_worker_spec.rb index bdc64c6785b..2605c14334f 100644 --- a/spec/workers/stuck_ci_jobs_worker_spec.rb +++ b/spec/workers/stuck_ci_jobs_worker_spec.rb @@ -132,8 +132,10 @@ describe StuckCiJobsWorker do end it 'cancels exclusive lease after worker perform' do - expect(Gitlab::ExclusiveLease).to receive(:cancel).with(described_class::EXCLUSIVE_LEASE_KEY, exclusive_lease_uuid) worker.perform + + expect(Gitlab::ExclusiveLease.new(described_class::EXCLUSIVE_LEASE_KEY, timeout: 1.hour)) + .not_to be_exists end end end |