From cacc3815006ab7d3828ebe8903f95154b27a6e21 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Tue, 14 Feb 2023 12:07:42 +0000 Subject: Add latest changes from gitlab-org/gitlab@master --- spec/controllers/groups_controller_spec.rb | 1 - spec/features/projects/navbar_spec.rb | 14 +- spec/helpers/namespaces_helper_spec.rb | 112 ---------------- spec/lib/api/ci/helpers/runner_spec.rb | 32 +++-- .../menus/packages_registries_menu_spec.rb | 22 +++- spec/mailers/emails/service_desk_spec.rb | 87 ++++++------- spec/models/ci/runner_machine_spec.rb | 142 +++++++++++++++++++++ spec/models/user_spec.rb | 15 --- spec/requests/api/ci/runner/jobs_put_spec.rb | 5 +- .../api/ci/runner/jobs_request_post_spec.rb | 57 +++++++-- .../api/ci/runner/runners_verify_post_spec.rb | 63 +++++++-- spec/requests/api/releases_spec.rb | 69 +++++++--- .../pages/destroy_deployments_service_spec.rb | 22 ++-- spec/services/releases/create_service_spec.rb | 71 +++++++++-- spec/services/releases/update_service_spec.rb | 77 ++++++++--- spec/support/helpers/navbar_structure_helper.rb | 8 ++ .../mailers/emails/service_desk_shared_context.rb | 40 ++++++ .../delete_source_branch_worker_spec.rb | 109 ++++------------ 18 files changed, 587 insertions(+), 359 deletions(-) create mode 100644 spec/support/shared_contexts/mailers/emails/service_desk_shared_context.rb (limited to 'spec') diff --git a/spec/controllers/groups_controller_spec.rb b/spec/controllers/groups_controller_spec.rb index fa46a4aab2d..9184cd2263e 100644 --- a/spec/controllers/groups_controller_spec.rb +++ b/spec/controllers/groups_controller_spec.rb @@ -611,7 +611,6 @@ RSpec.describe GroupsController, factory_default: :keep, feature_category: :code expect(response.body).to have_content('Open Merged Closed All') expect(response.body).not_to have_content('Open 0 Merged 0 Closed 0 All 0') - expect(assigns(:can_bulk_update)).to be_falsey end end end diff --git a/spec/features/projects/navbar_spec.rb b/spec/features/projects/navbar_spec.rb index 6090d132e3a..01457f77121 100644 --- a/spec/features/projects/navbar_spec.rb +++ b/spec/features/projects/navbar_spec.rb @@ -18,7 +18,7 @@ RSpec.describe 'Project navbar', :with_license, feature_category: :projects do stub_feature_flags(show_pages_in_deployments_menu: false) stub_config(registry: { enabled: false }) - stub_feature_flags(harbor_registry_integration: false) + stub_feature_flags(harbor_registry_integration: false, ml_experiment_tracking: false) insert_package_nav(_('Deployments')) insert_infrastructure_registry_nav insert_infrastructure_google_cloud_nav @@ -97,4 +97,16 @@ RSpec.describe 'Project navbar', :with_license, feature_category: :projects do it_behaves_like 'verified navigation bar' end + + context 'when models experiments is available' do + before do + stub_feature_flags(ml_experiment_tracking: true) + + insert_model_experiments_nav(_('Infrastructure Registry')) + + visit project_path(project) + end + + it_behaves_like 'verified navigation bar' + end end diff --git a/spec/helpers/namespaces_helper_spec.rb b/spec/helpers/namespaces_helper_spec.rb index f7500709d0e..3e6780d6831 100644 --- a/spec/helpers/namespaces_helper_spec.rb +++ b/spec/helpers/namespaces_helper_spec.rb @@ -45,118 +45,6 @@ RSpec.describe NamespacesHelper do user_group.add_owner(user) end - describe '#namespaces_options' do - context 'when admin mode is enabled', :enable_admin_mode do - it 'returns groups without being a member for admin' do - allow(helper).to receive(:current_user).and_return(admin) - - options = helper.namespaces_options(user_group.id, display_path: true, extra_group: user_group.id) - - expect(options).to include(admin_group.name) - expect(options).to include(user_group.name) - end - end - - context 'when admin mode is disabled' do - it 'returns only allowed namespaces for admin' do - allow(helper).to receive(:current_user).and_return(admin) - - options = helper.namespaces_options(user_group.id, display_path: true, extra_group: user_group.id) - - expect(options).to include(admin_group.name) - expect(options).not_to include(user_group.name) - end - end - - it 'returns only allowed namespaces for user' do - allow(helper).to receive(:current_user).and_return(user) - - options = helper.namespaces_options - - expect(options).not_to include(admin_group.name) - expect(options).to include(user_group.name) - expect(options).to include(user.name) - end - - it 'avoids duplicate groups when extra_group is used' do - allow(helper).to receive(:current_user).and_return(admin) - - options = helper.namespaces_options(user_group.id, display_path: true, extra_group: build(:group, name: admin_group.name)) - - expect(options.scan("data-name=\"#{admin_group.name}\"").count).to eq(1) - expect(options).to include(admin_group.name) - end - - context 'when admin mode is disabled' do - it 'selects existing group' do - allow(helper).to receive(:current_user).and_return(admin) - user_group.add_owner(admin) - - options = helper.namespaces_options(:extra_group, display_path: true, extra_group: user_group) - - expect(options).to include("selected=\"selected\" value=\"#{user_group.id}\"") - expect(options).to include(admin_group.name) - end - end - - it 'selects the new group by default' do - # Ensure we don't select a group with the same name - create(:group, name: 'new-group', path: 'another-path') - - allow(helper).to receive(:current_user).and_return(user) - - options = helper.namespaces_options(:extra_group, display_path: true, extra_group: build(:group, name: 'new-group', path: 'new-group')) - - expect(options).to include(user_group.name) - expect(options).not_to include(admin_group.name) - expect(options).to include("selected=\"selected\" value=\"-1\"") - end - - it 'falls back to current user selection' do - allow(helper).to receive(:current_user).and_return(user) - - options = helper.namespaces_options(:extra_group, display_path: true, extra_group: build(:group, name: admin_group.name)) - - expect(options).to include(user_group.name) - expect(options).not_to include(admin_group.name) - expect(options).to include("selected=\"selected\" value=\"#{user.namespace.id}\"") - end - - it 'returns only groups if groups_only option is true' do - allow(helper).to receive(:current_user).and_return(user) - - options = helper.namespaces_options(nil, groups_only: true) - - expect(options).not_to include(user.name) - expect(options).to include(user_group.name) - end - - context 'when nested groups are available' do - it 'includes groups nested in groups the user can administer' do - allow(helper).to receive(:current_user).and_return(user) - child_group = create(:group, :private, parent: user_group) - - options = helper.namespaces_options - - expect(options).to include(child_group.name) - end - - it 'orders the groups correctly' do - allow(helper).to receive(:current_user).and_return(user) - child_group = create(:group, :private, parent: user_group) - other_child = create(:group, :private, parent: user_group) - sub_child = create(:group, :private, parent: child_group) - - expect(helper).to receive(:options_for_group) - .with([user_group, child_group, sub_child, other_child], anything) - .and_call_original - allow(helper).to receive(:options_for_group).and_call_original - - helper.namespaces_options - end - end - end - describe '#cascading_namespace_settings_popover_data' do attribute = :delayed_project_removal diff --git a/spec/lib/api/ci/helpers/runner_spec.rb b/spec/lib/api/ci/helpers/runner_spec.rb index 7f711ba1ca0..8264db8344d 100644 --- a/spec/lib/api/ci/helpers/runner_spec.rb +++ b/spec/lib/api/ci/helpers/runner_spec.rb @@ -78,12 +78,6 @@ RSpec.describe API::Ci::Helpers::Runner do stub_feature_flags(create_runner_machine: true) end - it 'does not return runner machine if no system_id specified' do - allow(helper).to receive(:params).and_return(token: runner.token) - - is_expected.to be_nil - end - context 'when runner machine already exists' do before do allow(helper).to receive(:params).and_return(token: runner.token, system_id: runner_machine.system_xid) @@ -96,15 +90,27 @@ RSpec.describe API::Ci::Helpers::Runner do end end - it 'creates a new runner machine if one could be not be found', :aggregate_failures do - allow(helper).to receive(:params).and_return(token: runner.token, system_id: 'new_system_id') + context 'when runner machine cannot be found' do + it 'creates a new runner machine', :aggregate_failures do + allow(helper).to receive(:params).and_return(token: runner.token, system_id: 'new_system_id') + + expect { current_runner_machine }.to change { Ci::RunnerMachine.count }.by(1) + + expect(current_runner_machine).not_to be_nil + expect(current_runner_machine.system_xid).to eq('new_system_id') + expect(current_runner_machine.contacted_at).to eq(Time.current) + expect(current_runner_machine.runner).to eq(runner) + end + + it 'creates a new runner machine if system_id is not specified', :aggregate_failures do + allow(helper).to receive(:params).and_return(token: runner.token) - expect { current_runner_machine }.to change { Ci::RunnerMachine.count }.by(1) + expect { current_runner_machine }.to change { Ci::RunnerMachine.count }.by(1) - expect(current_runner_machine).not_to be_nil - expect(current_runner_machine.system_xid).to eq('new_system_id') - expect(current_runner_machine.contacted_at).to eq(Time.current) - expect(current_runner_machine.runner).to eq(runner) + expect(current_runner_machine).not_to be_nil + expect(current_runner_machine.system_xid).to eq(::API::Ci::Helpers::Runner::LEGACY_SYSTEM_XID) + expect(current_runner_machine.runner).to eq(runner) + end end end diff --git a/spec/lib/sidebars/projects/menus/packages_registries_menu_spec.rb b/spec/lib/sidebars/projects/menus/packages_registries_menu_spec.rb index b03269c424a..22847ab3c8f 100644 --- a/spec/lib/sidebars/projects/menus/packages_registries_menu_spec.rb +++ b/spec/lib/sidebars/projects/menus/packages_registries_menu_spec.rb @@ -35,7 +35,7 @@ RSpec.describe Sidebars::Projects::Menus::PackagesRegistriesMenu do before do stub_container_registry_config(enabled: registry_enabled) stub_config(packages: { enabled: packages_enabled }) - stub_feature_flags(harbor_registry_integration: false) + stub_feature_flags(harbor_registry_integration: false, ml_experiment_tracking: false) end context 'when Packages Registry is visible' do @@ -176,5 +176,25 @@ RSpec.describe Sidebars::Projects::Menus::PackagesRegistriesMenu do end end end + + describe 'Model experiments' do + let(:item_id) { :model_experiments } + + context 'when :ml_experiment_tracking is enabled' do + it 'shows the menu item' do + stub_feature_flags(ml_experiment_tracking: true) + + is_expected.not_to be_nil + end + end + + context 'when :ml_experiment_tracking is disabled' do + it 'does not show the menu item' do + stub_feature_flags(ml_experiment_tracking: false) + + is_expected.to be_nil + end + end + end end end diff --git a/spec/mailers/emails/service_desk_spec.rb b/spec/mailers/emails/service_desk_spec.rb index 1f55aabc535..25afa8b48ce 100644 --- a/spec/mailers/emails/service_desk_spec.rb +++ b/spec/mailers/emails/service_desk_spec.rb @@ -9,11 +9,13 @@ RSpec.describe Emails::ServiceDesk do include EmailHelpers include_context 'gitlab email notification' + include_context 'with service desk mailer' let_it_be(:user) { create(:user) } let_it_be(:project) { create(:project) } let_it_be(:issue) { create(:issue, project: project) } let_it_be(:email) { 'someone@gitlab.com' } + let_it_be(:expected_unsubscribe_url) { unsubscribe_sent_notification_url('b7721fc7e8419911a8bea145236a0519') } let(:template) { double(content: template_content) } @@ -21,43 +23,6 @@ RSpec.describe Emails::ServiceDesk do issue.issue_email_participants.create!(email: email) end - before do - stub_const('ServiceEmailClass', Class.new(ApplicationMailer)) - - ServiceEmailClass.class_eval do - include GitlabRoutingHelper - include EmailsHelper - include Emails::ServiceDesk - - helper GitlabRoutingHelper - helper EmailsHelper - - # this method is implemented in Notify class, we don't need to test it - def reply_key - 'test-key' - end - - # this method is implemented in Notify class, we don't need to test it - def sender(author_id, params = {}) - author_id - end - - # this method is implemented in Notify class - # - # We do not need to test the Notify method, it is already tested in notify_spec - def mail_new_thread(issue, options) - # we need to rewrite this in order to look up templates in the correct directory - self.class.mailer_name = 'notify' - - # this is needed for default layout - @unsubscribe_url = 'http://unsubscribe.example.com' - - mail(options) - end - alias_method :mail_answer_thread, :mail_new_thread - end - end - shared_examples 'handle template content' do |template_key, attachments_count| before do expect(Gitlab::Template::ServiceDeskTemplate).to receive(:find) @@ -134,13 +99,31 @@ RSpec.describe Emails::ServiceDesk do it_behaves_like 'handle template content', 'thank_you' end - context 'with an issue id and issue path placeholders' do - let(:template_content) { 'thank you, **your new issue:** %{ISSUE_ID}, path: %{ISSUE_PATH}' } - let(:expected_body) { "thank you, your new issue: ##{issue.iid}, path: #{project.full_path}##{issue.iid}" } + context 'with an issue id, issue path and unsubscribe url placeholders' do + let(:template_content) do + 'thank you, **your new issue:** %{ISSUE_ID}, path: %{ISSUE_PATH}' \ + '[Unsubscribe](%{UNSUBSCRIBE_URL})' + end + + let(:expected_body) do + "

thank you, your new issue: ##{issue.iid}, path: #{project.full_path}##{issue.iid}" \ + "Unsubscribe

" + end it_behaves_like 'handle template content', 'thank_you' end + context 'with header and footer placeholders' do + let(:template_content) do + '%{SYSTEM_HEADER}' \ + 'thank you, **your new issue** has been created.' \ + '%{SYSTEM_FOOTER}' + end + + it_behaves_like 'appearance header and footer enabled' + it_behaves_like 'appearance header and footer not enabled' + end + context 'with an issue id placeholder with whitespace' do let(:template_content) { 'thank you, **your new issue:** %{ ISSUE_ID}' } let(:expected_body) { "thank you, your new issue: ##{issue.iid}" } @@ -174,13 +157,31 @@ RSpec.describe Emails::ServiceDesk do it_behaves_like 'handle template content', 'new_note' end - context 'with an issue id, issue path and note placeholders' do - let(:template_content) { 'thank you, **new note on issue:** %{ISSUE_ID}, path: %{ISSUE_PATH}: %{NOTE_TEXT}' } - let(:expected_body) { "thank you, new note on issue: ##{issue.iid}, path: #{project.full_path}##{issue.iid}: #{note.note}" } + context 'with an issue id, issue path, note and unsubscribe url placeholders' do + let(:template_content) do + 'thank you, **new note on issue:** %{ISSUE_ID}, path: %{ISSUE_PATH}: %{NOTE_TEXT}' \ + '[Unsubscribe](%{UNSUBSCRIBE_URL})' + end + + let(:expected_body) do + "

thank you, new note on issue: ##{issue.iid}, path: #{project.full_path}##{issue.iid}: #{note.note}" \ + "Unsubscribe

" + end it_behaves_like 'handle template content', 'new_note' end + context 'with header and footer placeholders' do + let(:template_content) do + '%{SYSTEM_HEADER}' \ + 'thank you, **your new issue** has been created.' \ + '%{SYSTEM_FOOTER}' + end + + it_behaves_like 'appearance header and footer enabled' + it_behaves_like 'appearance header and footer not enabled' + end + context 'with an issue id placeholder with whitespace' do let(:template_content) { 'thank you, **new note on issue:** %{ ISSUE_ID}: %{ NOTE_TEXT }' } let(:expected_body) { "thank you, new note on issue: ##{issue.iid}: #{note.note}" } diff --git a/spec/models/ci/runner_machine_spec.rb b/spec/models/ci/runner_machine_spec.rb index 79d383ad182..d0979d8a485 100644 --- a/spec/models/ci/runner_machine_spec.rb +++ b/spec/models/ci/runner_machine_spec.rb @@ -52,4 +52,146 @@ RSpec.describe Ci::RunnerMachine, feature_category: :runner_fleet, type: :model is_expected.to match_array([runner_machine1.id, runner_machine2.id]) end end + + describe '#heartbeat', :freeze_time do + let(:runner_machine) { create(:ci_runner_machine) } + let(:executor) { 'shell' } + let(:version) { '15.0.1' } + let(:values) do + { + ip_address: '8.8.8.8', + architecture: '18-bit', + config: { gpus: "all" }, + executor: executor, + version: version + } + end + + subject(:heartbeat) do + runner_machine.heartbeat(values) + end + + context 'when database was updated recently' do + before do + runner_machine.contacted_at = Time.current + end + + it 'schedules version update' do + expect(Ci::Runners::ProcessRunnerVersionUpdateWorker).to receive(:perform_async).with(version).once + + heartbeat + + expect(runner_machine.runner_version).to be_nil + end + + it 'updates cache' do + expect_redis_update + + heartbeat + end + + context 'with only ip_address specified' do + let(:values) do + { ip_address: '1.1.1.1' } + end + + it 'updates only ip_address' do + attrs = Gitlab::Json.dump(ip_address: '1.1.1.1', contacted_at: Time.current) + + Gitlab::Redis::Cache.with do |redis| + redis_key = runner_machine.send(:cache_attribute_key) + expect(redis).to receive(:set).with(redis_key, attrs, any_args) + end + + heartbeat + end + end + end + + context 'when database was not updated recently' do + before do + runner_machine.contacted_at = 2.hours.ago + + allow(Ci::Runners::ProcessRunnerVersionUpdateWorker).to receive(:perform_async).with(version).once + end + + context 'with invalid runner_machine' do + before do + runner_machine.runner = nil + end + + it 'still updates redis cache and database' do + expect(runner_machine).to be_invalid + + expect_redis_update + does_db_update + + expect(Ci::Runners::ProcessRunnerVersionUpdateWorker).to have_received(:perform_async) + .with(version).once + end + end + + context 'with unchanged runner_machine version' do + let(:runner_machine) { create(:ci_runner_machine, version: version) } + + it 'does not schedule ci_runner_versions update' do + heartbeat + + expect(Ci::Runners::ProcessRunnerVersionUpdateWorker).not_to have_received(:perform_async) + end + end + + it 'updates redis cache and database' do + expect_redis_update + does_db_update + + expect(Ci::Runners::ProcessRunnerVersionUpdateWorker).to have_received(:perform_async) + .with(version).once + end + + Ci::Runner::EXECUTOR_NAME_TO_TYPES.each_key do |executor| + context "with #{executor} executor" do + let(:executor) { executor } + + it 'updates with expected executor type' do + expect_redis_update + + heartbeat + + expect(runner_machine.reload.read_attribute(:executor_type)).to eq(expected_executor_type) + end + + def expected_executor_type + executor.gsub(/[+-]/, '_') + end + end + end + + context "with an unknown executor type" do + let(:executor) { 'some-unknown-type' } + + it 'updates with unknown executor type' do + expect_redis_update + + heartbeat + + expect(runner_machine.reload.read_attribute(:executor_type)).to eq('unknown') + end + end + end + + def expect_redis_update + Gitlab::Redis::Cache.with do |redis| + redis_key = runner_machine.send(:cache_attribute_key) + expect(redis).to receive(:set).with(redis_key, anything, any_args).and_call_original + end + end + + def does_db_update + expect { heartbeat }.to change { runner_machine.reload.read_attribute(:contacted_at) } + .and change { runner_machine.reload.read_attribute(:architecture) } + .and change { runner_machine.reload.read_attribute(:config) } + .and change { runner_machine.reload.read_attribute(:executor_type) } + end + end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 7b5b8acdb66..3fb867f8ee8 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -2404,21 +2404,6 @@ RSpec.describe User, feature_category: :user_profile do it_behaves_like 'manageable groups examples' end end - - describe '#manageable_groups_with_routes' do - it 'eager loads routes from manageable groups' do - control_count = - ActiveRecord::QueryRecorder.new(skip_cached: false) do - user.manageable_groups_with_routes.map(&:route) - end.count - - create(:group, parent: subgroup) - - expect do - user.manageable_groups_with_routes.map(&:route) - end.not_to exceed_all_query_limit(control_count) - end - end end end diff --git a/spec/requests/api/ci/runner/jobs_put_spec.rb b/spec/requests/api/ci/runner/jobs_put_spec.rb index 22817922b1b..ef3b38e3fc4 100644 --- a/spec/requests/api/ci/runner/jobs_put_spec.rb +++ b/spec/requests/api/ci/runner/jobs_put_spec.rb @@ -21,11 +21,13 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state, feature_catego let_it_be(:project) { create(:project, namespace: group, shared_runners_enabled: false) } let_it_be(:pipeline) { create(:ci_pipeline, project: project, ref: 'master') } let_it_be(:runner) { create(:ci_runner, :project, projects: [project]) } + let_it_be(:runner_machine) { create(:ci_runner_machine, runner: runner) } let_it_be(:user) { create(:user) } describe 'PUT /api/v4/jobs/:id' do let_it_be_with_reload(:job) do - create(:ci_build, :pending, :trace_live, pipeline: pipeline, project: project, user: user, runner_id: runner.id) + create(:ci_build, :pending, :trace_live, pipeline: pipeline, project: project, user: user, + runner_id: runner.id, runner_machine: runner_machine) end before do @@ -38,6 +40,7 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state, feature_catego it 'updates runner info' do expect { update_job(state: 'success') }.to change { runner.reload.contacted_at } + .and change { runner_machine.reload.contacted_at } end context 'when status is given' do diff --git a/spec/requests/api/ci/runner/jobs_request_post_spec.rb b/spec/requests/api/ci/runner/jobs_request_post_spec.rb index 618adeb7db0..6e721d40560 100644 --- a/spec/requests/api/ci/runner/jobs_request_post_spec.rb +++ b/spec/requests/api/ci/runner/jobs_request_post_spec.rb @@ -122,25 +122,56 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state, feature_catego context 'when system_id parameter is specified' do subject(:request) { request_job(**args) } - context 'when ci_runner_machines with same system_xid does not exist' do - let(:args) { { system_id: 's_some_system_id' } } + context 'with create_runner_machine FF enabled' do + before do + stub_feature_flags(create_runner_machine: true) + end + + context 'when ci_runner_machines with same system_xid does not exist' do + let(:args) { { system_id: 's_some_system_id' } } + + it 'creates respective ci_runner_machines record', :freeze_time do + expect { request }.to change { runner.runner_machines.reload.count }.from(0).to(1) + + machine = runner.runner_machines.last + expect(machine.system_xid).to eq args[:system_id] + expect(machine.runner).to eq runner + expect(machine.contacted_at).to eq Time.current + end + end + + context 'when ci_runner_machines with same system_xid already exists', :freeze_time do + let(:args) { { system_id: 's_existing_system_id' } } + let!(:runner_machine) do + create(:ci_runner_machine, runner: runner, system_xid: args[:system_id], contacted_at: 1.hour.ago) + end - it 'creates respective ci_runner_machines record', :freeze_time do - expect { request }.to change { runner.runner_machines.reload.count }.from(0).to(1) + it 'does not create new ci_runner_machines record' do + expect { request }.not_to change { Ci::RunnerMachine.count } + end + + it 'updates the contacted_at field' do + request - machine = runner.runner_machines.last - expect(machine.system_xid).to eq args[:system_id] - expect(machine.runner).to eq runner - expect(machine.contacted_at).to eq Time.current + expect(runner_machine.reload.contacted_at).to eq Time.current + end end end - context 'when ci_runner_machines with same system_xid already exists' do - let(:args) { { system_id: 's_existing_system_id' } } - let!(:runner_machine) { create(:ci_runner_machine, runner: runner, system_xid: args[:system_id]) } + context 'with create_runner_machine FF disabled' do + before do + stub_feature_flags(create_runner_machine: false) + end - it 'does not create new ci_runner_machines record' do - expect { request }.not_to change { Ci::RunnerMachine.count } + context 'when ci_runner_machines with same system_xid does not exist' do + let(:args) { { system_id: 's_some_system_id' } } + + it 'does not create respective ci_runner_machines record', :freeze_time, :aggregate_failures do + expect { request }.not_to change { runner.runner_machines.reload.count } + + expect(response).to have_gitlab_http_status(:created) + expect(runner.runner_machines).to be_empty + end end end end diff --git a/spec/requests/api/ci/runner/runners_verify_post_spec.rb b/spec/requests/api/ci/runner/runners_verify_post_spec.rb index 22a954cc444..3853e7ed6c9 100644 --- a/spec/requests/api/ci/runner/runners_verify_post_spec.rb +++ b/spec/requests/api/ci/runner/runners_verify_post_spec.rb @@ -19,6 +19,9 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state, feature_catego describe '/api/v4/runners' do describe 'POST /api/v4/runners/verify' do let(:runner) { create(:ci_runner) } + let(:params) {} + + subject(:verify) { post api('/runners/verify'), params: params } context 'when no token is provided' do it 'returns 400 error' do @@ -29,46 +32,84 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state, feature_catego end context 'when invalid token is provided' do + let(:params) { { token: 'invalid-token' } } + it 'returns 403 error' do - post api('/runners/verify'), params: { token: 'invalid-token' } + verify expect(response).to have_gitlab_http_status(:forbidden) end end context 'when valid token is provided' do - subject { post api('/runners/verify'), params: { token: runner.token } } + let(:params) { { token: runner.token } } - it 'verifies Runner credentials' do - subject + context 'with create_runner_machine FF enabled' do + before do + stub_feature_flags(create_runner_machine: true) + end - expect(response).to have_gitlab_http_status(:ok) + it 'verifies Runner credentials' do + verify + + expect(response).to have_gitlab_http_status(:ok) + end + + it_behaves_like 'storing arguments in the application context for the API' do + let(:expected_params) { { client_id: "runner/#{runner.id}" } } + end + + context 'when system_id is provided' do + let(:params) { { token: runner.token, system_id: 's_some_system_id' } } + + it 'creates a runner_machine' do + expect { verify }.to change { Ci::RunnerMachine.count }.by(1) + end + end end - it_behaves_like 'storing arguments in the application context for the API' do - let(:expected_params) { { client_id: "runner/#{runner.id}" } } + context 'with create_runner_machine FF disabled' do + before do + stub_feature_flags(create_runner_machine: false) + end + + it 'verifies Runner credentials' do + verify + + expect(response).to have_gitlab_http_status(:ok) + end + + context 'when system_id is provided' do + let(:params) { { token: runner.token, system_id: 's_some_system_id' } } + + it 'does not create a runner_machine', :aggregate_failures do + expect { verify }.not_to change { Ci::RunnerMachine.count } + + expect(response).to have_gitlab_http_status(:ok) + end + end end end context 'when non-expired token is provided' do - subject { post api('/runners/verify'), params: { token: runner.token } } + let(:params) { { token: runner.token } } it 'verifies Runner credentials' do runner["token_expires_at"] = 10.days.from_now runner.save! - subject + verify expect(response).to have_gitlab_http_status(:ok) end end context 'when expired token is provided' do - subject { post api('/runners/verify'), params: { token: runner.token } } + let(:params) { { token: runner.token } } it 'does not verify Runner credentials' do runner["token_expires_at"] = 10.days.ago runner.save! - subject + verify expect(response).to have_gitlab_http_status(:forbidden) end diff --git a/spec/requests/api/releases_spec.rb b/spec/requests/api/releases_spec.rb index e209ad2b2d5..c3f99872cef 100644 --- a/spec/requests/api/releases_spec.rb +++ b/spec/requests/api/releases_spec.rb @@ -1215,11 +1215,23 @@ RSpec.describe API::Releases, feature_category: :release_orchestration do end context 'with a project milestone' do - let(:milestone_params) { { milestones: [milestone.title] } } + shared_examples 'adds milestone' do + it 'adds the milestone' do + expect(response).to have_gitlab_http_status(:created) + expect(returned_milestones).to match_array(['v1.0']) + end + end - it 'adds the milestone' do - expect(response).to have_gitlab_http_status(:created) - expect(returned_milestones).to match_array(['v1.0']) + context 'by title' do + let(:milestone_params) { { milestones: [milestone.title] } } + + it_behaves_like 'adds milestone' + end + + context 'by id' do + let(:milestone_params) { { milestone_ids: [milestone.id] } } + + it_behaves_like 'adds milestone' end end @@ -1408,18 +1420,14 @@ RSpec.describe API::Releases, feature_category: :release_orchestration do context 'when a milestone is passed in' do let(:milestone) { create(:milestone, project: project, title: 'v1.0') } - let(:milestone_title) { milestone.title } - let(:params) { { milestones: [milestone_title] } } + let!(:milestone2) { create(:milestone, project: project, title: 'v2.0') } before do release.milestones << milestone end - context 'a different milestone' do - let(:milestone_title) { 'v2.0' } - let!(:milestone2) { create(:milestone, project: project, title: milestone_title) } - - it 'replaces the milestone' do + shared_examples 'updates milestone' do + it 'updates the milestone' do subject expect(response).to have_gitlab_http_status(:ok) @@ -1427,8 +1435,20 @@ RSpec.describe API::Releases, feature_category: :release_orchestration do end end + context 'by title' do + let(:params) { { milestones: [milestone2.title] } } + + it_behaves_like 'updates milestone' + end + + context 'by id' do + let(:params) { { milestone_ids: [milestone2.id] } } + + it_behaves_like 'updates milestone' + end + context 'an identical milestone' do - let(:milestone_title) { 'v1.0' } + let(:params) { { milestones: [milestone.title] } } it 'does not change the milestone' do subject @@ -1439,7 +1459,7 @@ RSpec.describe API::Releases, feature_category: :release_orchestration do end context 'an empty milestone' do - let(:milestone_title) { nil } + let(:params) { { milestones: [] } } it 'removes the milestone' do subject @@ -1476,13 +1496,26 @@ RSpec.describe API::Releases, feature_category: :release_orchestration do context 'with all new' do let!(:milestone2) { create(:milestone, project: project, title: 'milestone2') } let!(:milestone3) { create(:milestone, project: project, title: 'milestone3') } - let(:params) { { milestones: [milestone2.title, milestone3.title] } } - it 'replaces the milestones' do - subject + shared_examples 'update milestones' do + it 'replaces the milestones' do + subject - expect(response).to have_gitlab_http_status(:ok) - expect(returned_milestones).to match_array(%w(milestone2 milestone3)) + expect(response).to have_gitlab_http_status(:ok) + expect(returned_milestones).to match_array(%w(milestone2 milestone3)) + end + end + + context 'by title' do + let(:params) { { milestones: [milestone2.title, milestone3.title] } } + + it_behaves_like 'update milestones' + end + + context 'by id' do + let(:params) { { milestone_ids: [milestone2.id, milestone3.id] } } + + it_behaves_like 'update milestones' end end end diff --git a/spec/services/pages/destroy_deployments_service_spec.rb b/spec/services/pages/destroy_deployments_service_spec.rb index 0f8e8b6573e..0ca8cbbb681 100644 --- a/spec/services/pages/destroy_deployments_service_spec.rb +++ b/spec/services/pages/destroy_deployments_service_spec.rb @@ -2,28 +2,26 @@ require 'spec_helper' -RSpec.describe Pages::DestroyDeploymentsService do - let(:project) { create(:project) } +RSpec.describe Pages::DestroyDeploymentsService, feature_category: :pages do + let_it_be(:project) { create(:project) } let!(:old_deployments) { create_list(:pages_deployment, 2, project: project) } let!(:last_deployment) { create(:pages_deployment, project: project) } let!(:newer_deployment) { create(:pages_deployment, project: project) } let!(:deployment_from_another_project) { create(:pages_deployment) } it 'destroys all deployments of the project' do - expect do - described_class.new(project).execute - end.to change { PagesDeployment.count }.by(-4) + expect { described_class.new(project).execute } + .to change { PagesDeployment.count }.by(-4) - expect(deployment_from_another_project.reload).to be + expect(deployment_from_another_project.reload).to be_persisted end it 'destroy only deployments older than last deployment if it is provided' do - expect do - described_class.new(project, last_deployment.id).execute - end.to change { PagesDeployment.count }.by(-2) + expect { described_class.new(project, last_deployment.id).execute } + .to change { PagesDeployment.count }.by(-2) - expect(last_deployment.reload).to be - expect(newer_deployment.reload).to be - expect(deployment_from_another_project.reload).to be + expect(last_deployment.reload).to be_persisted + expect(newer_deployment.reload).to be_persisted + expect(deployment_from_another_project.reload).to be_persisted end end diff --git a/spec/services/releases/create_service_spec.rb b/spec/services/releases/create_service_spec.rb index 5f49eed3e77..9768ceb12e8 100644 --- a/spec/services/releases/create_service_spec.rb +++ b/spec/services/releases/create_service_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Releases::CreateService do +RSpec.describe Releases::CreateService, feature_category: :continuous_integration do let(:project) { create(:project, :repository) } let(:user) { create(:user) } let(:tag_name) { project.repository.tag_names.first } @@ -132,6 +132,15 @@ RSpec.describe Releases::CreateService do expect(result[:status]).to eq(:error) expect(result[:message]).to eq("Milestone(s) not found: #{inexistent_milestone_tag}") end + + it 'raises an error saying the milestone id is inexistent' do + inexistent_milestone_id = non_existing_record_id + service = described_class.new(project, user, params.merge!({ milestone_ids: [inexistent_milestone_id] })) + result = service.execute + + expect(result[:status]).to eq(:error) + expect(result[:message]).to eq("Milestone id(s) not found: #{inexistent_milestone_id}") + end end context 'when existing milestone is passed in' do @@ -140,15 +149,27 @@ RSpec.describe Releases::CreateService do let(:params_with_milestone) { params.merge!({ milestones: [title] }) } let(:service) { described_class.new(milestone.project, user, params_with_milestone) } - it 'creates a release and ties this milestone to it' do - result = service.execute + shared_examples 'creates release' do + it 'creates a release and ties this milestone to it' do + result = service.execute - expect(project.releases.count).to eq(1) - expect(result[:status]).to eq(:success) + expect(project.releases.count).to eq(1) + expect(result[:status]).to eq(:success) + + release = project.releases.last + + expect(release.milestones).to match_array([milestone]) + end + end - release = project.releases.last + context 'by title' do + it_behaves_like 'creates release' + end + + context 'by ids' do + let(:params_with_milestone) { params.merge!({ milestone_ids: [milestone.id] }) } - expect(release.milestones).to match_array([milestone]) + it_behaves_like 'creates release' end context 'when another release was previously created with that same milestone linked' do @@ -164,18 +185,31 @@ RSpec.describe Releases::CreateService do end end - context 'when multiple existing milestone titles are passed in' do + context 'when multiple existing milestones are passed in' do let(:title_1) { 'v1.0' } let(:title_2) { 'v1.0-rc' } let!(:milestone_1) { create(:milestone, :active, project: project, title: title_1) } let!(:milestone_2) { create(:milestone, :active, project: project, title: title_2) } - let!(:params_with_milestones) { params.merge!({ milestones: [title_1, title_2] }) } - it 'creates a release and ties it to these milestones' do - described_class.new(project, user, params_with_milestones).execute - release = project.releases.last + shared_examples 'creates multiple releases' do + it 'creates a release and ties it to these milestones' do + described_class.new(project, user, params_with_milestones).execute + release = project.releases.last + + expect(release.milestones.map(&:title)).to include(title_1, title_2) + end + end + + context 'by title' do + let!(:params_with_milestones) { params.merge!({ milestones: [title_1, title_2] }) } + + it_behaves_like 'creates multiple releases' + end + + context 'by ids' do + let!(:params_with_milestones) { params.merge!({ milestone_ids: [milestone_1.id, milestone_2.id] }) } - expect(release.milestones.map(&:title)).to include(title_1, title_2) + it_behaves_like 'creates multiple releases' end end @@ -198,6 +232,17 @@ RSpec.describe Releases::CreateService do service.execute end.not_to change(Release, :count) end + + context 'with milestones as ids' do + let!(:params_with_milestones) { params.merge!({ milestone_ids: [milestone.id, non_existing_record_id] }) } + + it 'raises an error' do + result = service.execute + + expect(result[:status]).to eq(:error) + expect(result[:message]).to eq("Milestone id(s) not found: #{non_existing_record_id}") + end + end end context 'no milestone association behavior' do diff --git a/spec/services/releases/update_service_spec.rb b/spec/services/releases/update_service_spec.rb index 7461470a844..6bddea48251 100644 --- a/spec/services/releases/update_service_spec.rb +++ b/spec/services/releases/update_service_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Releases::UpdateService do +RSpec.describe Releases::UpdateService, feature_category: :continuous_integration do let(:project) { create(:project, :repository) } let(:user) { create(:user) } let(:new_name) { 'A new name' } @@ -60,18 +60,22 @@ RSpec.describe Releases::UpdateService do release.milestones << milestone end - context 'a different milestone' do - let(:new_title) { 'v2.0' } - + shared_examples 'updates milestones' do it 'updates the related milestone accordingly' do - result = service.execute release.reload + result = service.execute expect(release.milestones.first.title).to eq(new_title) expect(result[:milestones_updated]).to be_truthy end end + context 'a different milestone' do + let(:new_title) { 'v2.0' } + + it_behaves_like 'updates milestones' + end + context 'an identical milestone' do let(:new_title) { 'v1.0' } @@ -79,11 +83,17 @@ RSpec.describe Releases::UpdateService do expect { service.execute }.to raise_error(ActiveRecord::RecordInvalid) end end + + context 'by ids' do + let(:new_title) { 'v2.0' } + let(:params_with_milestone) { params.merge!({ milestone_ids: [new_milestone.id] }) } + + it_behaves_like 'updates milestones' + end end context "when an 'empty' milestone is passed in" do let(:milestone) { create(:milestone, project: project, title: 'v1.0') } - let(:params_with_empty_milestone) { params.merge!({ milestones: [] }) } before do release.milestones << milestone @@ -91,12 +101,26 @@ RSpec.describe Releases::UpdateService do service.params = params_with_empty_milestone end - it 'removes the old milestone and does not associate any new milestone' do - result = service.execute - release.reload + shared_examples 'removes milestones' do + it 'removes the old milestone and does not associate any new milestone' do + result = service.execute + release.reload + + expect(release.milestones).not_to be_present + expect(result[:milestones_updated]).to be_truthy + end + end - expect(release.milestones).not_to be_present - expect(result[:milestones_updated]).to be_truthy + context 'by title' do + let(:params_with_empty_milestone) { params.merge!({ milestones: [] }) } + + it_behaves_like 'removes milestones' + end + + context 'by id' do + let(:params_with_empty_milestone) { params.merge!({ milestone_ids: [] }) } + + it_behaves_like 'removes milestones' end end @@ -104,22 +128,35 @@ RSpec.describe Releases::UpdateService do let(:new_title_1) { 'v2.0' } let(:new_title_2) { 'v2.0-rc' } let(:milestone) { create(:milestone, project: project, title: 'v1.0') } - let(:params_with_milestones) { params.merge!({ milestones: [new_title_1, new_title_2] }) } let(:service) { described_class.new(project, user, params_with_milestones) } + let!(:new_milestone_1) { create(:milestone, project: project, title: new_title_1) } + let!(:new_milestone_2) { create(:milestone, project: project, title: new_title_2) } before do - create(:milestone, project: project, title: new_title_1) - create(:milestone, project: project, title: new_title_2) release.milestones << milestone end - it 'removes the old milestone and update the release with the new ones' do - result = service.execute - release.reload + shared_examples 'updates multiple milestones' do + it 'removes the old milestone and update the release with the new ones' do + result = service.execute + release.reload + + milestone_titles = release.milestones.map(&:title) + expect(milestone_titles).to match_array([new_title_1, new_title_2]) + expect(result[:milestones_updated]).to be_truthy + end + end + + context 'by title' do + let(:params_with_milestones) { params.merge!({ milestones: [new_title_1, new_title_2] }) } + + it_behaves_like 'updates multiple milestones' + end + + context 'by id' do + let(:params_with_milestones) { params.merge!({ milestone_ids: [new_milestone_1.id, new_milestone_2.id] }) } - milestone_titles = release.milestones.map(&:title) - expect(milestone_titles).to match_array([new_title_1, new_title_2]) - expect(result[:milestones_updated]).to be_truthy + it_behaves_like 'updates multiple milestones' end end end diff --git a/spec/support/helpers/navbar_structure_helper.rb b/spec/support/helpers/navbar_structure_helper.rb index 48c6e590e1b..d9384094657 100644 --- a/spec/support/helpers/navbar_structure_helper.rb +++ b/spec/support/helpers/navbar_structure_helper.rb @@ -85,6 +85,14 @@ module NavbarStructureHelper ) end + def insert_model_experiments_nav(within) + insert_after_sub_nav_item( + within, + within: _('Packages and registries'), + new_sub_nav_item_name: _('Model Experiments') + ) + end + def insert_observability_nav insert_after_nav_item( _('Kubernetes'), diff --git a/spec/support/shared_contexts/mailers/emails/service_desk_shared_context.rb b/spec/support/shared_contexts/mailers/emails/service_desk_shared_context.rb new file mode 100644 index 00000000000..4aa4d500f5c --- /dev/null +++ b/spec/support/shared_contexts/mailers/emails/service_desk_shared_context.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +RSpec.shared_context 'with service desk mailer' do + before do + stub_const('ServiceEmailClass', Class.new(ApplicationMailer)) + + ServiceEmailClass.class_eval do + include GitlabRoutingHelper + include EmailsHelper + include Emails::ServiceDesk + + helper GitlabRoutingHelper + helper EmailsHelper + + # this method is implemented in Notify class, we don't need to test it + def reply_key + 'b7721fc7e8419911a8bea145236a0519' + end + + # this method is implemented in Notify class, we don't need to test it + def sender(author_id, params = {}) + author_id + end + + # this method is implemented in Notify class + # + # We do not need to test the Notify method, it is already tested in notify_spec + def mail_new_thread(issue, options) + # we need to rewrite this in order to look up templates in the correct directory + self.class.mailer_name = 'notify' + + # this is needed for default layout + @unsubscribe_url = 'http://unsubscribe.example.com' + + mail(options) + end + alias_method :mail_answer_thread, :mail_new_thread + end + end +end diff --git a/spec/workers/merge_requests/delete_source_branch_worker_spec.rb b/spec/workers/merge_requests/delete_source_branch_worker_spec.rb index a7e4ffad259..e17ad02e272 100644 --- a/spec/workers/merge_requests/delete_source_branch_worker_spec.rb +++ b/spec/workers/merge_requests/delete_source_branch_worker_spec.rb @@ -13,114 +13,53 @@ RSpec.describe MergeRequests::DeleteSourceBranchWorker do before do allow_next_instance_of(::Projects::DeleteBranchWorker) do |instance| allow(instance).to receive(:perform).with(merge_request.source_project.id, user.id, - merge_request.source_branch) + merge_request.source_branch) end end - context 'when the add_delete_branch_worker feature flag is enabled' do - context 'with a non-existing merge request' do - it 'does nothing' do - expect(::Projects::DeleteBranchWorker).not_to receive(:new) - - worker.perform(non_existing_record_id, sha, user.id) - end - end + context 'with a non-existing merge request' do + it 'does nothing' do + expect(::Projects::DeleteBranchWorker).not_to receive(:new) - context 'with a non-existing user' do - it 'does nothing' do - expect(::Projects::DeleteBranchWorker).not_to receive(:new) - - worker.perform(merge_request.id, sha, non_existing_record_id) - end + worker.perform(non_existing_record_id, sha, user.id) end + end - context 'with existing user and merge request' do - it 'creates a new delete branch worker async' do - expect_next_instance_of(::Projects::DeleteBranchWorker) do |instance| - expect(instance).to receive(:perform).with(merge_request.source_project.id, user.id, - merge_request.source_branch) - end - - worker.perform(merge_request.id, sha, user.id) - end - - context 'source branch sha does not match' do - it 'does nothing' do - expect(::Projects::DeleteBranchWorker).not_to receive(:new) - - worker.perform(merge_request.id, 'new-source-branch-sha', user.id) - end - end - end + context 'with a non-existing user' do + it 'does nothing' do + expect(::Projects::DeleteBranchWorker).not_to receive(:new) - it_behaves_like 'an idempotent worker' do - let(:job_args) { [merge_request.id, sha, user.id] } + worker.perform(merge_request.id, sha, non_existing_record_id) end end - context 'when the add_delete_branch_worker feature flag is disabled' do - before do - stub_feature_flags(add_delete_branch_worker: false) - end - - context 'with a non-existing merge request' do - it 'does nothing' do - expect(::Branches::DeleteService).not_to receive(:new) - expect(::MergeRequests::RetargetChainService).not_to receive(:new) - - worker.perform(non_existing_record_id, sha, user.id) + context 'with existing user and merge request' do + it 'calls delete branch worker' do + expect_next_instance_of(::Projects::DeleteBranchWorker) do |instance| + expect(instance).to receive(:perform).with(merge_request.source_project.id, user.id, + merge_request.source_branch) end + + worker.perform(merge_request.id, sha, user.id) end - context 'with a non-existing user' do + context 'source branch sha does not match' do it 'does nothing' do - expect(::Branches::DeleteService).not_to receive(:new) - expect(::MergeRequests::RetargetChainService).not_to receive(:new) + expect(::Projects::DeleteBranchWorker).not_to receive(:new) - worker.perform(merge_request.id, sha, non_existing_record_id) + worker.perform(merge_request.id, 'new-source-branch-sha', user.id) end end - context 'with existing user and merge request' do - it 'calls service to delete source branch' do - expect_next_instance_of(::Branches::DeleteService) do |instance| - expect(instance).to receive(:execute).with(merge_request.source_branch) - end + context 'when delete worker raises an error' do + it 'still retargets the merge request' do + expect(::Projects::DeleteBranchWorker).to receive(:new).and_raise(StandardError) - worker.perform(merge_request.id, sha, user.id) - end - - it 'calls service to try retarget merge requests' do expect_next_instance_of(::MergeRequests::RetargetChainService) do |instance| expect(instance).to receive(:execute).with(merge_request) end - worker.perform(merge_request.id, sha, user.id) - end - - context 'source branch sha does not match' do - it 'does nothing' do - expect(::Branches::DeleteService).not_to receive(:new) - expect(::MergeRequests::RetargetChainService).not_to receive(:new) - - worker.perform(merge_request.id, 'new-source-branch-sha', user.id) - end - end - - context 'when delete service returns an error' do - let(:service_result) { ServiceResponse.error(message: 'placeholder') } - - it 'still retargets the merge request' do - expect_next_instance_of(::Branches::DeleteService) do |instance| - expect(instance).to receive(:execute).with(merge_request.source_branch).and_return(service_result) - end - - expect_next_instance_of(::MergeRequests::RetargetChainService) do |instance| - expect(instance).to receive(:execute).with(merge_request) - end - - worker.perform(merge_request.id, sha, user.id) - end + expect { worker.perform(merge_request.id, sha, user.id) }.to raise_error(StandardError) end end -- cgit v1.2.3