From 9b124bc39e0902fffd4285d077c70edbfbce3b36 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Wed, 25 Aug 2021 00:11:06 +0000 Subject: Add latest changes from gitlab-org/gitlab@master --- spec/factories/compares.rb | 22 +++++++ spec/lib/gitlab/url_builder_spec.rb | 21 +++++++ .../set_default_job_token_scope_true_spec.rb | 33 +++++++++++ spec/models/project_ci_cd_setting_spec.rb | 6 -- spec/requests/api/generic_packages_spec.rb | 2 +- spec/requests/api/go_proxy_spec.rb | 2 +- spec/requests/api/maven_packages_spec.rb | 2 +- spec/requests/api/pypi_packages_spec.rb | 2 +- spec/requests/api/releases_spec.rb | 2 +- spec/requests/api/repositories_spec.rb | 6 ++ spec/requests/api/rubygem_packages_spec.rb | 2 +- .../api/terraform/modules/v1/packages_spec.rb | 2 +- spec/requests/git_http_spec.rb | 8 +++ .../create_pipeline_service_spec.rb | 67 +++++++++++++-------- .../requests/api/npm_packages_shared_context.rb | 2 +- .../create_pipeline_worker_spec.rb | 69 ++++++++++++++++++++++ 16 files changed, 210 insertions(+), 38 deletions(-) create mode 100644 spec/factories/compares.rb create mode 100644 spec/migrations/set_default_job_token_scope_true_spec.rb create mode 100644 spec/workers/ci/external_pull_requests/create_pipeline_worker_spec.rb (limited to 'spec') diff --git a/spec/factories/compares.rb b/spec/factories/compares.rb new file mode 100644 index 00000000000..4dd94b93049 --- /dev/null +++ b/spec/factories/compares.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +FactoryBot.define do + factory :compare do + skip_create # No persistence + + start_project { association(:project, :repository) } + target_project { start_project } + + start_ref { 'master' } + target_ref { 'feature' } + + base_sha { nil } + straight { false } + + initialize_with do + CompareService + .new(start_project, start_ref) + .execute(target_project, target_ref, base_sha: base_sha, straight: straight) + end + end +end diff --git a/spec/lib/gitlab/url_builder_spec.rb b/spec/lib/gitlab/url_builder_spec.rb index b359eb422d7..8e372ba795b 100644 --- a/spec/lib/gitlab/url_builder_spec.rb +++ b/spec/lib/gitlab/url_builder_spec.rb @@ -69,6 +69,27 @@ RSpec.describe Gitlab::UrlBuilder do end end + context 'when passing a compare' do + # NOTE: The Compare requires an actual repository, which isn't available + # with the `build_stubbed` strategy used by the table tests above + let_it_be(:compare) { create(:compare) } + let_it_be(:project) { compare.project } + + it 'returns the full URL' do + expect(subject.build(compare)).to eq("#{Gitlab.config.gitlab.url}/#{project.full_path}/-/compare/#{compare.base_commit_sha}...#{compare.head_commit_sha}") + end + + it 'returns only the path if only_path is given' do + expect(subject.build(compare, only_path: true)).to eq("/#{project.full_path}/-/compare/#{compare.base_commit_sha}...#{compare.head_commit_sha}") + end + + it 'returns an empty string for missing project' do + expect(compare).to receive(:project).and_return(nil) + + expect(subject.build(compare)).to eq('') + end + end + context 'when passing a commit without a project' do let(:commit) { build_stubbed(:commit) } diff --git a/spec/migrations/set_default_job_token_scope_true_spec.rb b/spec/migrations/set_default_job_token_scope_true_spec.rb new file mode 100644 index 00000000000..e7c77357318 --- /dev/null +++ b/spec/migrations/set_default_job_token_scope_true_spec.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +require 'spec_helper' +require_migration! + +RSpec.describe SetDefaultJobTokenScopeTrue, schema: 20210819153805 do + let(:ci_cd_settings) { table(:project_ci_cd_settings) } + let(:namespaces) { table(:namespaces) } + let(:projects) { table(:projects) } + + let(:namespace) { namespaces.create!(name: 'test', path: 'path', type: 'Group') } + let(:project) { projects.create!(namespace_id: namespace.id) } + + describe '#up' do + it 'sets the job_token_scope_enabled default to true' do + described_class.new.up + + settings = ci_cd_settings.create!(project_id: project.id) + + expect(settings.job_token_scope_enabled).to be_truthy + end + end + + describe '#down' do + it 'sets the job_token_scope_enabled default to false' do + described_class.new.down + + settings = ci_cd_settings.create!(project_id: project.id) + + expect(settings.job_token_scope_enabled).to be_falsey + end + end +end diff --git a/spec/models/project_ci_cd_setting_spec.rb b/spec/models/project_ci_cd_setting_spec.rb index caab182cda8..406485d8cc8 100644 --- a/spec/models/project_ci_cd_setting_spec.rb +++ b/spec/models/project_ci_cd_setting_spec.rb @@ -21,12 +21,6 @@ RSpec.describe ProjectCiCdSetting do end end - describe '#job_token_scope_enabled' do - it 'is false by default' do - expect(described_class.new.job_token_scope_enabled).to be_falsey - end - end - describe '#default_git_depth' do let(:default_value) { described_class::DEFAULT_GIT_DEPTH } diff --git a/spec/requests/api/generic_packages_spec.rb b/spec/requests/api/generic_packages_spec.rb index d615247aeb0..7e439a22e4b 100644 --- a/spec/requests/api/generic_packages_spec.rb +++ b/spec/requests/api/generic_packages_spec.rb @@ -18,7 +18,7 @@ RSpec.describe API::GenericPackages do let_it_be(:project_deploy_token_wo) { create(:project_deploy_token, deploy_token: deploy_token_wo, project: project) } let(:user) { personal_access_token.user } - let(:ci_build) { create(:ci_build, :running, user: user) } + let(:ci_build) { create(:ci_build, :running, user: user, project: project) } let(:snowplow_standard_context_params) { { user: user, project: project, namespace: project.namespace } } def auth_header diff --git a/spec/requests/api/go_proxy_spec.rb b/spec/requests/api/go_proxy_spec.rb index e678b6cf1c8..0143340de11 100644 --- a/spec/requests/api/go_proxy_spec.rb +++ b/spec/requests/api/go_proxy_spec.rb @@ -11,7 +11,7 @@ RSpec.describe API::GoProxy do let_it_be(:base) { "#{Settings.build_gitlab_go_url}/#{project.full_path}" } let_it_be(:oauth) { create :oauth_access_token, scopes: 'api', resource_owner: user } - let_it_be(:job) { create :ci_build, user: user, status: :running } + let_it_be(:job) { create :ci_build, user: user, status: :running, project: project } let_it_be(:pa_token) { create :personal_access_token, user: user } let_it_be(:modules) do diff --git a/spec/requests/api/maven_packages_spec.rb b/spec/requests/api/maven_packages_spec.rb index c3fd02dad51..07111dd1d62 100644 --- a/spec/requests/api/maven_packages_spec.rb +++ b/spec/requests/api/maven_packages_spec.rb @@ -15,7 +15,7 @@ RSpec.describe API::MavenPackages do let_it_be(:package_file) { package.package_files.with_file_name_like('%.xml').first } let_it_be(:jar_file) { package.package_files.with_file_name_like('%.jar').first } let_it_be(:personal_access_token) { create(:personal_access_token, user: user) } - let_it_be(:job, reload: true) { create(:ci_build, user: user, status: :running) } + let_it_be(:job, reload: true) { create(:ci_build, user: user, status: :running, project: project) } let_it_be(:deploy_token) { create(:deploy_token, read_package_registry: true, write_package_registry: true) } let_it_be(:project_deploy_token) { create(:project_deploy_token, deploy_token: deploy_token, project: project) } let_it_be(:deploy_token_for_group) { create(:deploy_token, :group, read_package_registry: true, write_package_registry: true) } diff --git a/spec/requests/api/pypi_packages_spec.rb b/spec/requests/api/pypi_packages_spec.rb index 8df2460a2b6..c17d0600aca 100644 --- a/spec/requests/api/pypi_packages_spec.rb +++ b/spec/requests/api/pypi_packages_spec.rb @@ -13,7 +13,7 @@ RSpec.describe API::PypiPackages do let_it_be(:personal_access_token) { create(:personal_access_token, user: user) } let_it_be(:deploy_token) { create(:deploy_token, read_package_registry: true, write_package_registry: true) } let_it_be(:project_deploy_token) { create(:project_deploy_token, deploy_token: deploy_token, project: project) } - let_it_be(:job) { create(:ci_build, :running, user: user) } + let_it_be(:job) { create(:ci_build, :running, user: user, project: project) } let(:headers) { {} } diff --git a/spec/requests/api/releases_spec.rb b/spec/requests/api/releases_spec.rb index 87b08587904..90b03a480a8 100644 --- a/spec/requests/api/releases_spec.rb +++ b/spec/requests/api/releases_spec.rb @@ -839,7 +839,7 @@ RSpec.describe API::Releases do context 'when a valid token is provided' do it 'creates the release for a running job' do - job.update!(status: :running) + job.update!(status: :running, project: project) post api("/projects/#{project.id}/releases"), params: params.merge(job_token: job.token) expect(response).to have_gitlab_http_status(:created) diff --git a/spec/requests/api/repositories_spec.rb b/spec/requests/api/repositories_spec.rb index d3262b8056b..1b90d6fa47e 100644 --- a/spec/requests/api/repositories_spec.rb +++ b/spec/requests/api/repositories_spec.rb @@ -354,6 +354,7 @@ RSpec.describe API::Repositories do expect(response).to have_gitlab_http_status(:ok) expect(json_response['commits']).to be_present expect(json_response['diffs']).to be_present + expect(json_response['web_url']).to be_present end it "compares branches with explicit merge-base mode" do @@ -365,6 +366,7 @@ RSpec.describe API::Repositories do expect(response).to have_gitlab_http_status(:ok) expect(json_response['commits']).to be_present expect(json_response['diffs']).to be_present + expect(json_response['web_url']).to be_present end it "compares branches with explicit straight mode" do @@ -376,6 +378,7 @@ RSpec.describe API::Repositories do expect(response).to have_gitlab_http_status(:ok) expect(json_response['commits']).to be_present expect(json_response['diffs']).to be_present + expect(json_response['web_url']).to be_present end it "compares tags" do @@ -384,6 +387,7 @@ RSpec.describe API::Repositories do expect(response).to have_gitlab_http_status(:ok) expect(json_response['commits']).to be_present expect(json_response['diffs']).to be_present + expect(json_response['web_url']).to be_present end it "compares commits" do @@ -393,6 +397,7 @@ RSpec.describe API::Repositories do expect(json_response['commits']).to be_empty expect(json_response['diffs']).to be_empty expect(json_response['compare_same_ref']).to be_falsey + expect(json_response['web_url']).to be_present end it "compares commits in reverse order" do @@ -401,6 +406,7 @@ RSpec.describe API::Repositories do expect(response).to have_gitlab_http_status(:ok) expect(json_response['commits']).to be_present expect(json_response['diffs']).to be_present + expect(json_response['web_url']).to be_present end it "compare commits between different projects with non-forked relation" do diff --git a/spec/requests/api/rubygem_packages_spec.rb b/spec/requests/api/rubygem_packages_spec.rb index afa7adad80c..9b104520b52 100644 --- a/spec/requests/api/rubygem_packages_spec.rb +++ b/spec/requests/api/rubygem_packages_spec.rb @@ -10,7 +10,7 @@ RSpec.describe API::RubygemPackages do let_it_be_with_reload(:project) { create(:project) } let_it_be(:personal_access_token) { create(:personal_access_token) } let_it_be(:user) { personal_access_token.user } - let_it_be(:job) { create(:ci_build, :running, user: user) } + let_it_be(:job) { create(:ci_build, :running, user: user, project: project) } let_it_be(:deploy_token) { create(:deploy_token, read_package_registry: true, write_package_registry: true) } let_it_be(:project_deploy_token) { create(:project_deploy_token, deploy_token: deploy_token, project: project) } let_it_be(:headers) { {} } diff --git a/spec/requests/api/terraform/modules/v1/packages_spec.rb b/spec/requests/api/terraform/modules/v1/packages_spec.rb index 6803c09b8c2..b04f5ad9a94 100644 --- a/spec/requests/api/terraform/modules/v1/packages_spec.rb +++ b/spec/requests/api/terraform/modules/v1/packages_spec.rb @@ -12,7 +12,7 @@ RSpec.describe API::Terraform::Modules::V1::Packages do let_it_be(:package) { create(:terraform_module_package, project: project) } let_it_be(:personal_access_token) { create(:personal_access_token) } let_it_be(:user) { personal_access_token.user } - let_it_be(:job) { create(:ci_build, :running, user: user) } + let_it_be(:job) { create(:ci_build, :running, user: user, project: project) } let_it_be(:deploy_token) { create(:deploy_token, read_package_registry: true, write_package_registry: true) } let_it_be(:project_deploy_token) { create(:project_deploy_token, deploy_token: deploy_token, project: project) } diff --git a/spec/requests/git_http_spec.rb b/spec/requests/git_http_spec.rb index e4a0c034b20..a16f5abf608 100644 --- a/spec/requests/git_http_spec.rb +++ b/spec/requests/git_http_spec.rb @@ -882,6 +882,10 @@ RSpec.describe 'Git HTTP requests' do before do build.update!(user: user) project.add_reporter(user) + create(:ci_job_token_project_scope_link, + source_project: project, + target_project: other_project, + added_by: user) end shared_examples 'can download code only' do @@ -1447,6 +1451,10 @@ RSpec.describe 'Git HTTP requests' do before do build.update!(project: project) # can't associate it on factory create + create(:ci_job_token_project_scope_link, + source_project: project, + target_project: other_project, + added_by: user) end context 'when build created by system is authenticated' do diff --git a/spec/services/ci/external_pull_requests/create_pipeline_service_spec.rb b/spec/services/ci/external_pull_requests/create_pipeline_service_spec.rb index 2b310443b37..04d75630295 100644 --- a/spec/services/ci/external_pull_requests/create_pipeline_service_spec.rb +++ b/spec/services/ci/external_pull_requests/create_pipeline_service_spec.rb @@ -12,7 +12,7 @@ RSpec.describe Ci::ExternalPullRequests::CreatePipelineService do project.add_maintainer(user) end - subject(:response) { described_class.new(project, user).execute(pull_request) } + subject(:execute) { described_class.new(project, user).execute(pull_request) } context 'when pull request is open' do before do @@ -21,26 +21,43 @@ RSpec.describe Ci::ExternalPullRequests::CreatePipelineService do context 'when source sha is the head of the source branch' do let(:source_branch) { project.repository.branches.last } - let(:create_pipeline_service) { instance_double(Ci::CreatePipelineService) } before do pull_request.update!(source_branch: source_branch.name, source_sha: source_branch.target) end - it 'creates a pipeline for external pull request', :aggregate_failures do - pipeline = response.payload - - expect(response).to be_success - expect(pipeline).to be_valid - expect(pipeline).to be_persisted - expect(pipeline).to be_external_pull_request_event - expect(pipeline).to eq(project.ci_pipelines.last) - expect(pipeline.external_pull_request).to eq(pull_request) - expect(pipeline.user).to eq(user) - expect(pipeline.status).to eq('created') - expect(pipeline.ref).to eq(pull_request.source_branch) - expect(pipeline.sha).to eq(pull_request.source_sha) - expect(pipeline.source_sha).to eq(pull_request.source_sha) + context 'when the FF ci_create_external_pr_pipeline_async is disabled' do + before do + stub_feature_flags(ci_create_external_pr_pipeline_async: false) + end + + it 'creates a pipeline for external pull request', :aggregate_failures do + pipeline = execute.payload + + expect(execute).to be_success + expect(pipeline).to be_valid + expect(pipeline).to be_persisted + expect(pipeline).to be_external_pull_request_event + expect(pipeline).to eq(project.ci_pipelines.last) + expect(pipeline.external_pull_request).to eq(pull_request) + expect(pipeline.user).to eq(user) + expect(pipeline.status).to eq('created') + expect(pipeline.ref).to eq(pull_request.source_branch) + expect(pipeline.sha).to eq(pull_request.source_sha) + expect(pipeline.source_sha).to eq(pull_request.source_sha) + end + end + + it 'enqueues Ci::ExternalPullRequests::CreatePipelineWorker' do + expect { execute } + .to change { ::Ci::ExternalPullRequests::CreatePipelineWorker.jobs.count } + .by(1) + + args = ::Ci::ExternalPullRequests::CreatePipelineWorker.jobs.last['args'] + + expect(args[0]).to eq(project.id) + expect(args[1]).to eq(user.id) + expect(args[2]).to eq(pull_request.id) end end @@ -53,11 +70,12 @@ RSpec.describe Ci::ExternalPullRequests::CreatePipelineService do end it 'does nothing', :aggregate_failures do - expect(Ci::CreatePipelineService).not_to receive(:new) + expect { execute } + .not_to change { ::Ci::ExternalPullRequests::CreatePipelineWorker.jobs.count } - expect(response).to be_error - expect(response.message).to eq('The source sha is not the head of the source branch') - expect(response.payload).to be_nil + expect(execute).to be_error + expect(execute.message).to eq('The source sha is not the head of the source branch') + expect(execute.payload).to be_nil end end end @@ -68,11 +86,12 @@ RSpec.describe Ci::ExternalPullRequests::CreatePipelineService do end it 'does nothing', :aggregate_failures do - expect(Ci::CreatePipelineService).not_to receive(:new) + expect { execute } + .not_to change { ::Ci::ExternalPullRequests::CreatePipelineWorker.jobs.count } - expect(response).to be_error - expect(response.message).to eq('The pull request is not opened') - expect(response.payload).to be_nil + expect(execute).to be_error + expect(execute.message).to eq('The pull request is not opened') + expect(execute.payload).to be_nil end end end diff --git a/spec/support/shared_contexts/requests/api/npm_packages_shared_context.rb b/spec/support/shared_contexts/requests/api/npm_packages_shared_context.rb index 815108be447..c737091df48 100644 --- a/spec/support/shared_contexts/requests/api/npm_packages_shared_context.rb +++ b/spec/support/shared_contexts/requests/api/npm_packages_shared_context.rb @@ -11,7 +11,7 @@ RSpec.shared_context 'npm api setup' do let_it_be(:package, reload: true) { create(:npm_package, project: project, name: "@#{group.path}/scoped_package") } let_it_be(:token) { create(:oauth_access_token, scopes: 'api', resource_owner: user) } let_it_be(:personal_access_token) { create(:personal_access_token, user: user) } - let_it_be(:job, reload: true) { create(:ci_build, user: user, status: :running) } + let_it_be(:job, reload: true) { create(:ci_build, user: user, status: :running, project: project) } let_it_be(:deploy_token) { create(:deploy_token, read_package_registry: true, write_package_registry: true) } let_it_be(:project_deploy_token) { create(:project_deploy_token, deploy_token: deploy_token, project: project) } diff --git a/spec/workers/ci/external_pull_requests/create_pipeline_worker_spec.rb b/spec/workers/ci/external_pull_requests/create_pipeline_worker_spec.rb new file mode 100644 index 00000000000..116a0e4d035 --- /dev/null +++ b/spec/workers/ci/external_pull_requests/create_pipeline_worker_spec.rb @@ -0,0 +1,69 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ci::ExternalPullRequests::CreatePipelineWorker do + let_it_be(:project) { create(:project, :auto_devops, :repository) } + let_it_be(:user) { project.owner } + let_it_be(:external_pull_request) do + branch = project.repository.branches.last + create(:external_pull_request, project: project, source_branch: branch.name, source_sha: branch.target) + end + + let(:worker) { described_class.new } + + describe '#perform' do + let(:project_id) { project.id } + let(:user_id) { user.id } + let(:external_pull_request_id) { external_pull_request.id } + + subject(:perform) { worker.perform(project_id, user_id, external_pull_request_id) } + + it 'creates the pipeline' do + pipeline = perform.payload + + expect(pipeline).to be_valid + expect(pipeline).to be_persisted + expect(pipeline).to be_external_pull_request_event + expect(pipeline.project).to eq(project) + expect(pipeline.user).to eq(user) + expect(pipeline.external_pull_request).to eq(external_pull_request) + expect(pipeline.status).to eq('created') + expect(pipeline.ref).to eq(external_pull_request.source_branch) + expect(pipeline.sha).to eq(external_pull_request.source_sha) + expect(pipeline.source_sha).to eq(external_pull_request.source_sha) + expect(pipeline.target_sha).to eq(external_pull_request.target_sha) + end + + shared_examples_for 'not calling service' do + it 'does not call the service' do + expect(Ci::CreatePipelineService).not_to receive(:new) + perform + end + end + + context 'when the project not found' do + let(:project_id) { non_existing_record_id } + + it_behaves_like 'not calling service' + end + + context 'when the user not found' do + let(:user_id) { non_existing_record_id } + + it_behaves_like 'not calling service' + end + + context 'when the pull request not found' do + let(:external_pull_request_id) { non_existing_record_id } + + it_behaves_like 'not calling service' + end + + context 'when the pull request does not belong to the project' do + let(:external_pull_request_id) { create(:external_pull_request).id } + + it_behaves_like 'not calling service' + end + end +end -- cgit v1.2.3