diff options
Diffstat (limited to 'spec/models')
20 files changed, 887 insertions, 139 deletions
diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb index 87b91286168..95ae7bd21ab 100644 --- a/spec/models/application_setting_spec.rb +++ b/spec/models/application_setting_spec.rb @@ -594,4 +594,24 @@ describe ApplicationSetting do end end end + + describe '#archive_builds_older_than' do + subject { setting.archive_builds_older_than } + + context 'when the archive_builds_in_seconds is set' do + before do + setting.archive_builds_in_seconds = 3600 + end + + it { is_expected.to be_within(1.minute).of(1.hour.ago) } + end + + context 'when the archive_builds_in_seconds is set' do + before do + setting.archive_builds_in_seconds = nil + end + + it { is_expected.to be_nil } + end + end end diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 65e06f27f35..2e65a6a2a0f 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -17,8 +17,8 @@ describe Ci::Build do it { is_expected.to belong_to(:runner) } it { is_expected.to belong_to(:trigger_request) } it { is_expected.to belong_to(:erased_by) } - it { is_expected.to have_many(:deployments) } it { is_expected.to have_many(:trace_sections)} + it { is_expected.to have_one(:deployment) } it { is_expected.to have_one(:runner_session)} it { is_expected.to validate_presence_of(:ref) } it { is_expected.to respond_to(:has_trace?) } @@ -216,14 +216,6 @@ describe Ci::Build do let(:build) { create(:ci_build, :created, :schedulable, project: project) } it { expect(subject).to be_truthy } - - context 'when feature flag is diabled' do - before do - stub_feature_flags(ci_enable_scheduled_build: false) - end - - it { expect(subject).to be_falsy } - end end context 'when build is not schedulable' do @@ -327,10 +319,6 @@ describe Ci::Build do describe '#enqueue_scheduled' do subject { build.enqueue_scheduled } - before do - stub_feature_flags(ci_enable_scheduled_build: true) - end - context 'when build is scheduled and the right time has not come yet' do let(:build) { create(:ci_build, :scheduled, pipeline: pipeline) } @@ -811,17 +799,100 @@ describe Ci::Build do end end + describe 'state transition as a deployable' do + let!(:build) { create(:ci_build, :start_review_app) } + let(:deployment) { build.deployment } + let(:environment) { deployment.environment } + + it 'has deployments record with created status' do + expect(deployment).to be_created + expect(environment.name).to eq('review/master') + end + + context 'when transits to running' do + before do + build.run! + end + + it 'transits deployment status to running' do + expect(deployment).to be_running + end + end + + context 'when transits to success' do + before do + allow(Deployments::SuccessWorker).to receive(:perform_async) + build.success! + end + + it 'transits deployment status to success' do + expect(deployment).to be_success + end + end + + context 'when transits to failed' do + before do + build.drop! + end + + it 'transits deployment status to failed' do + expect(deployment).to be_failed + end + end + + context 'when transits to skipped' do + before do + build.skip! + end + + it 'transits deployment status to canceled' do + expect(deployment).to be_canceled + end + end + + context 'when transits to canceled' do + before do + build.cancel! + end + + it 'transits deployment status to canceled' do + expect(deployment).to be_canceled + end + end + end + + describe '#on_stop' do + subject { build.on_stop } + + context 'when a job has a specification that it can be stopped from the other job' do + let(:build) { create(:ci_build, :start_review_app) } + + it 'returns the other job name' do + is_expected.to eq('stop_review_app') + end + end + + context 'when a job does not have environment information' do + let(:build) { create(:ci_build) } + + it 'returns nil' do + is_expected.to be_nil + end + end + end + describe 'deployment' do - describe '#last_deployment' do - subject { build.last_deployment } + describe '#has_deployment?' do + subject { build.has_deployment? } + + context 'when build has a deployment' do + let!(:deployment) { create(:deployment, deployable: build) } - context 'when multiple deployments are created' do - let!(:deployment1) { create(:deployment, deployable: build) } - let!(:deployment2) { create(:deployment, deployable: build) } + it { is_expected.to be_truthy } + end - it 'returns the latest one' do - is_expected.to eq(deployment2) - end + context 'when build does not have a deployment' do + it { is_expected.to be_falsy } end end @@ -830,14 +901,14 @@ describe Ci::Build do context 'when build succeeded' do let(:build) { create(:ci_build, :success) } - let!(:deployment) { create(:deployment, deployable: build) } + let!(:deployment) { create(:deployment, :success, deployable: build) } context 'current deployment is latest' do it { is_expected.to be_falsey } end context 'current deployment is not latest on environment' do - let!(:deployment2) { create(:deployment, environment: deployment.environment) } + let!(:deployment2) { create(:deployment, :success, environment: deployment.environment) } it { is_expected.to be_truthy } end @@ -1326,6 +1397,14 @@ describe Ci::Build do it { is_expected.not_to be_retryable } end + + context 'when build is degenerated' do + before do + build.degenerate! + end + + it { is_expected.not_to be_retryable } + end end end @@ -1408,6 +1487,14 @@ describe Ci::Build do expect(subject.retries_max).to eq 0 end end + + context 'when build is degenerated' do + subject { create(:ci_build, :degenerated) } + + it 'returns zero' do + expect(subject.retries_max).to eq 0 + end + end end end @@ -1523,11 +1610,11 @@ describe Ci::Build do end end - describe '#other_actions' do + describe '#other_manual_actions' do let(:build) { create(:ci_build, :manual, pipeline: pipeline) } let!(:other_build) { create(:ci_build, :manual, pipeline: pipeline, name: 'other action') } - subject { build.other_actions } + subject { build.other_manual_actions } before do project.add_developer(user) @@ -1558,6 +1645,48 @@ describe Ci::Build do end end + describe '#other_scheduled_actions' do + let(:build) { create(:ci_build, :scheduled, pipeline: pipeline) } + + subject { build.other_scheduled_actions } + + before do + project.add_developer(user) + end + + context "when other build's status is success" do + let!(:other_build) { create(:ci_build, :schedulable, :success, pipeline: pipeline, name: 'other action') } + + it 'returns other actions' do + is_expected.to contain_exactly(other_build) + end + end + + context "when other build's status is failed" do + let!(:other_build) { create(:ci_build, :schedulable, :failed, pipeline: pipeline, name: 'other action') } + + it 'returns other actions' do + is_expected.to contain_exactly(other_build) + end + end + + context "when other build's status is running" do + let!(:other_build) { create(:ci_build, :schedulable, :running, pipeline: pipeline, name: 'other action') } + + it 'does not return other actions' do + is_expected.to be_empty + end + end + + context "when other build's status is scheduled" do + let!(:other_build) { create(:ci_build, :scheduled, pipeline: pipeline, name: 'other action') } + + it 'does not return other actions' do + is_expected.to contain_exactly(other_build) + end + end + end + describe '#persisted_environment' do let!(:environment) do create(:environment, project: project, name: "foo-#{project.default_branch}") @@ -1629,6 +1758,12 @@ describe Ci::Build do it { is_expected.to be_playable } end + + context 'when build is a manual and degenerated' do + subject { build_stubbed(:ci_build, :manual, :degenerated, status: :manual) } + + it { is_expected.not_to be_playable } + end end context 'when build is scheduled' do @@ -3157,10 +3292,14 @@ describe Ci::Build do end describe '#deployment_status' do + before do + allow_any_instance_of(described_class).to receive(:create_deployment) + end + context 'when build is a last deployment' do let(:build) { create(:ci_build, :success, environment: 'production') } let(:environment) { create(:environment, name: 'production', project: build.project) } - let!(:deployment) { create(:deployment, environment: environment, project: environment.project, deployable: build) } + let!(:deployment) { create(:deployment, :success, environment: environment, project: environment.project, deployable: build) } it { expect(build.deployment_status).to eq(:last) } end @@ -3168,8 +3307,8 @@ describe Ci::Build do context 'when there is a newer build with deployment' do let(:build) { create(:ci_build, :success, environment: 'production') } let(:environment) { create(:environment, name: 'production', project: build.project) } - let!(:deployment) { create(:deployment, environment: environment, project: environment.project, deployable: build) } - let!(:last_deployment) { create(:deployment, environment: environment, project: environment.project) } + let!(:deployment) { create(:deployment, :success, environment: environment, project: environment.project, deployable: build) } + let!(:last_deployment) { create(:deployment, :success, environment: environment, project: environment.project) } it { expect(build.deployment_status).to eq(:out_of_date) } end @@ -3177,7 +3316,7 @@ describe Ci::Build do context 'when build with deployment has failed' do let(:build) { create(:ci_build, :failed, environment: 'production') } let(:environment) { create(:environment, name: 'production', project: build.project) } - let!(:deployment) { create(:deployment, environment: environment, project: environment.project, deployable: build) } + let!(:deployment) { create(:deployment, :success, environment: environment, project: environment.project, deployable: build) } it { expect(build.deployment_status).to eq(:failed) } end @@ -3185,16 +3324,59 @@ describe Ci::Build do context 'when build with deployment is running' do let(:build) { create(:ci_build, environment: 'production') } let(:environment) { create(:environment, name: 'production', project: build.project) } - let!(:deployment) { create(:deployment, environment: environment, project: environment.project, deployable: build) } + let!(:deployment) { create(:deployment, :success, environment: environment, project: environment.project, deployable: build) } it { expect(build.deployment_status).to eq(:creating) } end + end - context 'when build is successful but deployment is not ready yet' do - let(:build) { create(:ci_build, :success, environment: 'production') } - let(:environment) { create(:environment, name: 'production', project: build.project) } + describe '#degenerated?' do + context 'when build is degenerated' do + subject { create(:ci_build, :degenerated) } - it { expect(build.deployment_status).to eq(:creating) } + it { is_expected.to be_degenerated } + end + + context 'when build is valid' do + subject { create(:ci_build) } + + it { is_expected.not_to be_degenerated } + + context 'and becomes degenerated' do + before do + subject.degenerate! + end + + it { is_expected.to be_degenerated } + end + end + end + + describe '#archived?' do + context 'when build is degenerated' do + subject { create(:ci_build, :degenerated) } + + it { is_expected.to be_archived } + end + + context 'for old build' do + subject { create(:ci_build, created_at: 1.day.ago) } + + context 'when archive_builds_in is set' do + before do + stub_application_setting(archive_builds_in_seconds: 3600) + end + + it { is_expected.to be_archived } + end + + context 'when archive_builds_in is not set' do + before do + stub_application_setting(archive_builds_in_seconds: nil) + end + + it { is_expected.not_to be_archived } + end end end end diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 153244b2159..9e6146b8a44 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -1043,6 +1043,11 @@ describe Ci::Pipeline, :mailer do expect(described_class.newest_first.pluck(:status)) .to eq(%w[skipped failed success canceled]) end + + it 'searches limited backlog' do + expect(described_class.newest_first(limit: 1).pluck(:status)) + .to eq(%w[skipped]) + end end describe '.latest_status' do @@ -1148,6 +1153,19 @@ describe Ci::Pipeline, :mailer do end end + describe '.latest_successful_ids_per_project' do + let(:projects) { create_list(:project, 2) } + let!(:pipeline1) { create(:ci_pipeline, :success, project: projects[0]) } + let!(:pipeline2) { create(:ci_pipeline, :success, project: projects[0]) } + let!(:pipeline3) { create(:ci_pipeline, :failed, project: projects[0]) } + let!(:pipeline4) { create(:ci_pipeline, :success, project: projects[1]) } + + it 'returns expected pipeline ids' do + expect(described_class.latest_successful_ids_per_project) + .to contain_exactly(pipeline2, pipeline4) + end + end + describe '.internal_sources' do subject { described_class.internal_sources } diff --git a/spec/models/clusters/applications/prometheus_spec.rb b/spec/models/clusters/applications/prometheus_spec.rb index f9776acd4c8..48ba163b38c 100644 --- a/spec/models/clusters/applications/prometheus_spec.rb +++ b/spec/models/clusters/applications/prometheus_spec.rb @@ -109,14 +109,20 @@ describe Clusters::Applications::Prometheus do end context 'cluster has kubeclient' do + let(:cluster) { create(:cluster, :project, :provided_by_gcp) } let(:kubernetes_url) { subject.cluster.platform_kubernetes.api_url } let(:kube_client) { subject.cluster.kubeclient.core_client } - subject { create(:clusters_applications_prometheus) } + subject { create(:clusters_applications_prometheus, cluster: cluster) } before do subject.cluster.platform_kubernetes.namespace = 'a-namespace' - stub_kubeclient_discover(subject.cluster.platform_kubernetes.api_url) + stub_kubeclient_discover(cluster.platform_kubernetes.api_url) + + create(:cluster_kubernetes_namespace, + cluster: cluster, + cluster_project: cluster.cluster_project, + project: cluster.cluster_project.project) end it 'creates proxy prometheus rest client' do diff --git a/spec/models/clusters/cluster_spec.rb b/spec/models/clusters/cluster_spec.rb index c245e8df815..19b76ca8cfb 100644 --- a/spec/models/clusters/cluster_spec.rb +++ b/spec/models/clusters/cluster_spec.rb @@ -16,6 +16,7 @@ describe Clusters::Cluster do it { is_expected.to have_one(:application_runner) } it { is_expected.to have_many(:kubernetes_namespaces) } it { is_expected.to have_one(:kubernetes_namespace) } + it { is_expected.to have_one(:cluster_project) } it { is_expected.to delegate_method(:status).to(:provider) } it { is_expected.to delegate_method(:status_reason).to(:provider) } diff --git a/spec/models/clusters/kubernetes_namespace_spec.rb b/spec/models/clusters/kubernetes_namespace_spec.rb index dea58fa26c7..0dfeea5cd2f 100644 --- a/spec/models/clusters/kubernetes_namespace_spec.rb +++ b/spec/models/clusters/kubernetes_namespace_spec.rb @@ -10,23 +10,15 @@ RSpec.describe Clusters::KubernetesNamespace, type: :model do describe 'namespace uniqueness validation' do let(:cluster_project) { create(:cluster_project) } - - let(:kubernetes_namespace) do - build(:cluster_kubernetes_namespace, - cluster: cluster_project.cluster, - project: cluster_project.project, - cluster_project: cluster_project) - end + let(:kubernetes_namespace) { build(:cluster_kubernetes_namespace, namespace: 'my-namespace') } subject { kubernetes_namespace } context 'when cluster is using the namespace' do before do create(:cluster_kubernetes_namespace, - cluster: cluster_project.cluster, - project: cluster_project.project, - cluster_project: cluster_project, - namespace: kubernetes_namespace.namespace) + cluster: kubernetes_namespace.cluster, + namespace: 'my-namespace') end it { is_expected.not_to be_valid } @@ -37,48 +29,79 @@ RSpec.describe Clusters::KubernetesNamespace, type: :model do end end - describe '#set_namespace_and_service_account_to_default' do - let(:cluster) { platform.cluster } - let(:cluster_project) { create(:cluster_project, cluster: cluster) } - let(:kubernetes_namespace) do - create(:cluster_kubernetes_namespace, - cluster: cluster_project.cluster, - project: cluster_project.project, - cluster_project: cluster_project) - end + describe '#configure_predefined_variables' do + let(:kubernetes_namespace) { build(:cluster_kubernetes_namespace) } + let(:cluster) { kubernetes_namespace.cluster } + let(:platform) { kubernetes_namespace.platform_kubernetes } - describe 'namespace' do - let(:platform) { create(:cluster_platform_kubernetes, namespace: namespace) } + subject { kubernetes_namespace.configure_predefined_credentials } - subject { kubernetes_namespace.namespace } + describe 'namespace' do + before do + platform.update_column(:namespace, namespace) + end context 'when platform has a namespace assigned' do let(:namespace) { 'platform-namespace' } it 'should copy the namespace' do - is_expected.to eq('platform-namespace') + subject + + expect(kubernetes_namespace.namespace).to eq('platform-namespace') end end context 'when platform does not have namespace assigned' do + let(:project) { kubernetes_namespace.project } let(:namespace) { nil } + let(:project_slug) { "#{project.path}-#{project.id}" } - it 'should set default namespace' do - project_slug = "#{cluster_project.project.path}-#{cluster_project.project_id}" + it 'should fallback to project namespace' do + subject - is_expected.to eq(project_slug) + expect(kubernetes_namespace.namespace).to eq(project_slug) end end end describe 'service_account_name' do - let(:platform) { create(:cluster_platform_kubernetes) } - - subject { kubernetes_namespace.service_account_name } + let(:service_account_name) { "#{kubernetes_namespace.namespace}-service-account" } it 'should set a service account name based on namespace' do - is_expected.to eq("#{kubernetes_namespace.namespace}-service-account") + subject + + expect(kubernetes_namespace.service_account_name).to eq(service_account_name) end end end + + describe '#predefined_variables' do + let(:kubernetes_namespace) { create(:cluster_kubernetes_namespace, cluster: cluster, service_account_token: token) } + let(:cluster) { create(:cluster, :project, platform_kubernetes: platform) } + let(:platform) { create(:cluster_platform_kubernetes, api_url: api_url, ca_cert: ca_pem, token: token) } + + let(:api_url) { 'https://kube.domain.com' } + let(:ca_pem) { 'CA PEM DATA' } + let(:token) { 'token' } + + let(:kubeconfig) do + config_file = expand_fixture_path('config/kubeconfig.yml') + config = YAML.safe_load(File.read(config_file)) + config.dig('users', 0, 'user')['token'] = token + config.dig('contexts', 0, 'context')['namespace'] = kubernetes_namespace.namespace + config.dig('clusters', 0, 'cluster')['certificate-authority-data'] = + Base64.strict_encode64(ca_pem) + + YAML.dump(config) + end + + it 'sets the variables' do + expect(kubernetes_namespace.predefined_variables).to include( + { key: 'KUBE_SERVICE_ACCOUNT', value: kubernetes_namespace.service_account_name, public: true }, + { key: 'KUBE_NAMESPACE', value: kubernetes_namespace.namespace, public: true }, + { key: 'KUBE_TOKEN', value: kubernetes_namespace.service_account_token, public: false }, + { key: 'KUBECONFIG', value: kubeconfig, public: false, file: true } + ) + end + end end diff --git a/spec/models/clusters/platforms/kubernetes_spec.rb b/spec/models/clusters/platforms/kubernetes_spec.rb index e13eb554add..2bcccc8184a 100644 --- a/spec/models/clusters/platforms/kubernetes_spec.rb +++ b/spec/models/clusters/platforms/kubernetes_spec.rb @@ -124,9 +124,17 @@ describe Clusters::Platforms::Kubernetes, :use_clean_rails_memory_store_caching end describe '#kubeclient' do + let(:cluster) { create(:cluster, :project) } + let(:kubernetes) { build(:cluster_platform_kubernetes, :configured, namespace: 'a-namespace', cluster: cluster) } + subject { kubernetes.kubeclient } - let(:kubernetes) { build(:cluster_platform_kubernetes, :configured, namespace: 'a-namespace') } + before do + create(:cluster_kubernetes_namespace, + cluster: kubernetes.cluster, + cluster_project: kubernetes.cluster.cluster_project, + project: kubernetes.cluster.cluster_project.project) + end it { is_expected.to be_an_instance_of(Gitlab::Kubernetes::KubeClient) } end @@ -186,29 +194,14 @@ describe Clusters::Platforms::Kubernetes, :use_clean_rails_memory_store_caching describe '#predefined_variables' do let!(:cluster) { create(:cluster, :project, platform_kubernetes: kubernetes) } - let(:kubernetes) { create(:cluster_platform_kubernetes, api_url: api_url, ca_cert: ca_pem, token: token) } + let(:kubernetes) { create(:cluster_platform_kubernetes, api_url: api_url, ca_cert: ca_pem) } let(:api_url) { 'https://kube.domain.com' } let(:ca_pem) { 'CA PEM DATA' } - let(:token) { 'token' } - - let(:kubeconfig) do - config_file = expand_fixture_path('config/kubeconfig.yml') - config = YAML.load(File.read(config_file)) - config.dig('users', 0, 'user')['token'] = token - config.dig('contexts', 0, 'context')['namespace'] = namespace - config.dig('clusters', 0, 'cluster')['certificate-authority-data'] = - Base64.strict_encode64(ca_pem) - - YAML.dump(config) - end shared_examples 'setting variables' do it 'sets the variables' do - expect(kubernetes.predefined_variables).to include( + expect(kubernetes.predefined_variables(project: cluster.project)).to include( { key: 'KUBE_URL', value: api_url, public: true }, - { key: 'KUBE_TOKEN', value: token, public: false }, - { key: 'KUBE_NAMESPACE', value: namespace, public: true }, - { key: 'KUBECONFIG', value: kubeconfig, public: false, file: true }, { key: 'KUBE_CA_PEM', value: ca_pem, public: true }, { key: 'KUBE_CA_PEM_FILE', value: ca_pem, public: true, file: true } ) @@ -229,13 +222,6 @@ describe Clusters::Platforms::Kubernetes, :use_clean_rails_memory_store_caching let(:namespace) { kubernetes.actual_namespace } it_behaves_like 'setting variables' - - it 'sets the KUBE_NAMESPACE' do - kube_namespace = kubernetes.predefined_variables.find { |h| h[:key] == 'KUBE_NAMESPACE' } - - expect(kube_namespace).not_to be_nil - expect(kube_namespace[:value]).to match(/\A#{Gitlab::PathRegex::PATH_REGEX_STR}-\d+\z/) - end end end @@ -319,4 +305,27 @@ describe Clusters::Platforms::Kubernetes, :use_clean_rails_memory_store_caching it { is_expected.to include(pods: []) } end end + + describe '#update_kubernetes_namespace' do + let(:cluster) { create(:cluster, :provided_by_gcp) } + let(:platform) { cluster.platform } + + context 'when namespace is updated' do + it 'should call ConfigureWorker' do + expect(ClusterPlatformConfigureWorker).to receive(:perform_async).with(cluster.id).once + + platform.namespace = 'new-namespace' + platform.save + end + end + + context 'when namespace is not updated' do + it 'should not call ConfigureWorker' do + expect(ClusterPlatformConfigureWorker).not_to receive(:perform_async) + + platform.username = "new-username" + platform.save + end + end + end end diff --git a/spec/models/concerns/awardable_spec.rb b/spec/models/concerns/awardable_spec.rb index debc02fa51f..5713106418d 100644 --- a/spec/models/concerns/awardable_spec.rb +++ b/spec/models/concerns/awardable_spec.rb @@ -37,8 +37,8 @@ describe Awardable do create(:award_emoji, awardable: issue3, name: "star", user: award_emoji.user) create(:award_emoji, awardable: issue3, name: "star", user: award_emoji2.user) - expect(Issue.awarded(award_emoji.user)).to eq [issue, issue3] - expect(Issue.awarded(award_emoji2.user)).to eq [issue2, issue3] + expect(Issue.awarded(award_emoji.user)).to contain_exactly(issue, issue3) + expect(Issue.awarded(award_emoji2.user)).to contain_exactly(issue2, issue3) end end diff --git a/spec/models/concerns/deployable_spec.rb b/spec/models/concerns/deployable_spec.rb new file mode 100644 index 00000000000..ac79c75a55e --- /dev/null +++ b/spec/models/concerns/deployable_spec.rb @@ -0,0 +1,53 @@ +require 'rails_helper' + +describe Deployable do + describe '#create_deployment' do + let(:deployment) { job.deployment } + let(:environment) { deployment&.environment } + + before do + job.reload + end + + context 'when the deployable object will deploy to production' do + let!(:job) { create(:ci_build, :start_review_app) } + + it 'creates a deployment and environment record' do + expect(deployment.project).to eq(job.project) + expect(deployment.ref).to eq(job.ref) + expect(deployment.tag).to eq(job.tag) + expect(deployment.sha).to eq(job.sha) + expect(deployment.user).to eq(job.user) + expect(deployment.deployable).to eq(job) + expect(deployment.on_stop).to eq('stop_review_app') + expect(environment.name).to eq('review/master') + end + end + + context 'when the deployable object will stop an environment' do + let!(:job) { create(:ci_build, :stop_review_app) } + + it 'does not create a deployment record' do + expect(deployment).to be_nil + end + end + + context 'when the deployable object has already had a deployment' do + let!(:job) { create(:ci_build, :start_review_app, deployment: race_deployment) } + let!(:race_deployment) { create(:deployment, :success) } + + it 'does not create a new deployment' do + expect(deployment).to eq(race_deployment) + end + end + + context 'when the deployable object will not deploy' do + let!(:job) { create(:ci_build) } + + it 'does not create a deployment and environment record' do + expect(deployment).to be_nil + expect(environment).to be_nil + end + end + end +end diff --git a/spec/models/deployment_spec.rb b/spec/models/deployment_spec.rb index b8364e0cf88..06c1e9c8c6a 100644 --- a/spec/models/deployment_spec.rb +++ b/spec/models/deployment_spec.rb @@ -16,6 +16,22 @@ describe Deployment do it { is_expected.to validate_presence_of(:ref) } it { is_expected.to validate_presence_of(:sha) } + describe '#scheduled_actions' do + subject { deployment.scheduled_actions } + + let(:project) { create(:project, :repository) } + let(:pipeline) { create(:ci_pipeline, project: project) } + let(:build) { create(:ci_build, :success, pipeline: pipeline) } + let(:deployment) { create(:deployment, deployable: build) } + + it 'delegates to other_scheduled_actions' do + expect_any_instance_of(Ci::Build) + .to receive(:other_scheduled_actions) + + subject + end + end + describe 'modules' do it_behaves_like 'AtomicInternalId' do let(:internal_id_attribute) { :iid } @@ -26,16 +42,174 @@ describe Deployment do end end - describe 'after_create callbacks' do - let(:environment) { create(:environment) } - let(:store) { Gitlab::EtagCaching::Store.new } + describe '.success' do + subject { described_class.success } + + context 'when deployment status is success' do + let(:deployment) { create(:deployment, :success) } + + it { is_expected.to eq([deployment]) } + end + + context 'when deployment status is created' do + let(:deployment) { create(:deployment, :created) } + + it { is_expected.to be_empty } + end + + context 'when deployment status is running' do + let(:deployment) { create(:deployment, :running) } + + it { is_expected.to be_empty } + end + end + + describe 'state machine' do + context 'when deployment runs' do + let(:deployment) { create(:deployment) } + + before do + deployment.run! + end + + it 'starts running' do + Timecop.freeze do + expect(deployment).to be_running + expect(deployment.finished_at).to be_nil + end + end + end + + context 'when deployment succeeded' do + let(:deployment) { create(:deployment, :running) } + + it 'has correct status' do + Timecop.freeze do + deployment.succeed! + + expect(deployment).to be_success + expect(deployment.finished_at).to eq(Time.now) + end + end + + it 'executes Deployments::SuccessWorker asynchronously' do + expect(Deployments::SuccessWorker) + .to receive(:perform_async).with(deployment.id) + + deployment.succeed! + end + end + + context 'when deployment failed' do + let(:deployment) { create(:deployment, :running) } + + it 'has correct status' do + Timecop.freeze do + deployment.drop! + + expect(deployment).to be_failed + expect(deployment.finished_at).to eq(Time.now) + end + end + end + + context 'when deployment was canceled' do + let(:deployment) { create(:deployment, :running) } + + it 'has correct status' do + Timecop.freeze do + deployment.cancel! + + expect(deployment).to be_canceled + expect(deployment.finished_at).to eq(Time.now) + end + end + end + end + + describe '#success?' do + subject { deployment.success? } + + context 'when deployment status is success' do + let(:deployment) { create(:deployment, :success) } + + it { is_expected.to be_truthy } + end + + context 'when deployment status is failed' do + let(:deployment) { create(:deployment, :failed) } + + it { is_expected.to be_falsy } + end + end + + describe '#status_name' do + subject { deployment.status_name } + + context 'when deployment status is success' do + let(:deployment) { create(:deployment, :success) } + + it { is_expected.to eq(:success) } + end + + context 'when deployment status is failed' do + let(:deployment) { create(:deployment, :failed) } + + it { is_expected.to eq(:failed) } + end + end + + describe '#finished_at' do + subject { deployment.finished_at } - it 'invalidates the environment etag cache' do - old_value = store.get(environment.etag_cache_key) + context 'when deployment status is created' do + let(:deployment) { create(:deployment) } - create(:deployment, environment: environment) + it { is_expected.to be_nil } + end + + context 'when deployment status is success' do + let(:deployment) { create(:deployment, :success) } + + it { is_expected.to eq(deployment.read_attribute(:finished_at)) } + end - expect(store.get(environment.etag_cache_key)).not_to eq(old_value) + context 'when deployment status is success' do + let(:deployment) { create(:deployment, :success, finished_at: nil) } + + before do + deployment.update_column(:finished_at, nil) + end + + it { is_expected.to eq(deployment.read_attribute(:created_at)) } + end + + context 'when deployment status is running' do + let(:deployment) { create(:deployment, :running) } + + it { is_expected.to be_nil } + end + end + + describe '#deployed_at' do + subject { deployment.deployed_at } + + context 'when deployment status is created' do + let(:deployment) { create(:deployment) } + + it { is_expected.to be_nil } + end + + context 'when deployment status is success' do + let(:deployment) { create(:deployment, :success) } + + it { is_expected.to eq(deployment.read_attribute(:finished_at)) } + end + + context 'when deployment status is running' do + let(:deployment) { create(:deployment, :running) } + + it { is_expected.to be_nil } end end @@ -96,7 +270,7 @@ describe Deployment do end describe '#metrics' do - let(:deployment) { create(:deployment) } + let(:deployment) { create(:deployment, :success) } let(:prometheus_adapter) { double('prometheus_adapter', can_query?: true) } subject { deployment.metrics } @@ -125,7 +299,7 @@ describe Deployment do describe '#additional_metrics' do let(:project) { create(:project, :repository) } - let(:deployment) { create(:deployment, project: project) } + let(:deployment) { create(:deployment, :succeed, project: project) } subject { deployment.additional_metrics } diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index 1de95d881a7..e121369f6ac 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -95,7 +95,7 @@ describe Environment do context 'with a last deployment' do let!(:deployment) do - create(:deployment, environment: environment, sha: project.commit('master').id) + create(:deployment, :success, environment: environment, sha: project.commit('master').id) end context 'in the same branch' do @@ -136,8 +136,8 @@ describe Environment do describe '#first_deployment_for' do let(:project) { create(:project, :repository) } - let!(:deployment) { create(:deployment, environment: environment, ref: commit.parent.id) } - let!(:deployment1) { create(:deployment, environment: environment, ref: commit.id) } + let!(:deployment) { create(:deployment, :succeed, environment: environment, ref: commit.parent.id) } + let!(:deployment1) { create(:deployment, :succeed, environment: environment, ref: commit.id) } let(:head_commit) { project.commit } let(:commit) { project.commit.parent } @@ -181,7 +181,8 @@ describe Environment do let(:build) { create(:ci_build) } let!(:deployment) do - create(:deployment, environment: environment, + create(:deployment, :success, + environment: environment, deployable: build, on_stop: 'close_app') end @@ -249,7 +250,8 @@ describe Environment do let(:build) { create(:ci_build, pipeline: pipeline) } let!(:deployment) do - create(:deployment, environment: environment, + create(:deployment, :success, + environment: environment, deployable: build, on_stop: 'close_app') end @@ -304,7 +306,7 @@ describe Environment do context 'when last deployment to environment is the most recent one' do before do - create(:deployment, environment: environment, ref: 'feature') + create(:deployment, :success, environment: environment, ref: 'feature') end it { is_expected.to be true } @@ -312,8 +314,8 @@ describe Environment do context 'when last deployment to environment is not the most recent' do before do - create(:deployment, environment: environment, ref: 'feature') - create(:deployment, environment: environment, ref: 'master') + create(:deployment, :success, environment: environment, ref: 'feature') + create(:deployment, :success, environment: environment, ref: 'master') end it { is_expected.to be false } @@ -321,7 +323,7 @@ describe Environment do end describe '#actions_for' do - let(:deployment) { create(:deployment, environment: environment) } + let(:deployment) { create(:deployment, :success, environment: environment) } let(:pipeline) { deployment.deployable.pipeline } let!(:review_action) { create(:ci_build, :manual, name: 'review-apps', pipeline: pipeline, environment: 'review/$CI_COMMIT_REF_NAME' )} let!(:production_action) { create(:ci_build, :manual, name: 'production', pipeline: pipeline, environment: 'production' )} @@ -331,6 +333,70 @@ describe Environment do end end + describe '.deployments' do + subject { environment.deployments } + + context 'when there is a deployment record with created status' do + let(:deployment) { create(:deployment, :created, environment: environment) } + + it 'does not return the record' do + is_expected.to be_empty + end + end + + context 'when there is a deployment record with running status' do + let(:deployment) { create(:deployment, :running, environment: environment) } + + it 'does not return the record' do + is_expected.to be_empty + end + end + + context 'when there is a deployment record with success status' do + let(:deployment) { create(:deployment, :success, environment: environment) } + + it 'returns the record' do + is_expected.to eq([deployment]) + end + end + end + + describe '.last_deployment' do + subject { environment.last_deployment } + + before do + allow_any_instance_of(Deployment).to receive(:create_ref) + end + + context 'when there is an old deployment record' do + let!(:previous_deployment) { create(:deployment, :success, environment: environment) } + + context 'when there is a deployment record with created status' do + let!(:deployment) { create(:deployment, environment: environment) } + + it 'returns the previous deployment' do + is_expected.to eq(previous_deployment) + end + end + + context 'when there is a deployment record with running status' do + let!(:deployment) { create(:deployment, :running, environment: environment) } + + it 'returns the previous deployment' do + is_expected.to eq(previous_deployment) + end + end + + context 'when there is a deployment record with success status' do + let!(:deployment) { create(:deployment, :success, environment: environment) } + + it 'returns the latest successful deployment' do + is_expected.to eq(deployment) + end + end + end + end + describe '#has_terminals?' do subject { environment.has_terminals? } @@ -338,7 +404,7 @@ describe Environment do context 'with a deployment service' do shared_examples 'same behavior between KubernetesService and Platform::Kubernetes' do context 'and a deployment' do - let!(:deployment) { create(:deployment, environment: environment) } + let!(:deployment) { create(:deployment, :success, environment: environment) } it { is_expected.to be_truthy } end diff --git a/spec/models/environment_status_spec.rb b/spec/models/environment_status_spec.rb index e7805d52d75..52b98552184 100644 --- a/spec/models/environment_status_spec.rb +++ b/spec/models/environment_status_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe EnvironmentStatus do - let(:deployment) { create(:deployment, :review_app) } + let(:deployment) { create(:deployment, :succeed, :review_app) } let(:environment) { deployment.environment} let(:project) { deployment.project } let(:merge_request) { create(:merge_request, :deployed_review_app, deployment: deployment) } @@ -12,7 +12,7 @@ describe EnvironmentStatus do it { is_expected.to delegate_method(:id).to(:environment) } it { is_expected.to delegate_method(:name).to(:environment) } it { is_expected.to delegate_method(:project).to(:environment) } - it { is_expected.to delegate_method(:deployed_at).to(:deployment).as(:created_at) } + it { is_expected.to delegate_method(:deployed_at).to(:deployment) } it { is_expected.to delegate_method(:status).to(:deployment) } describe '#project' do diff --git a/spec/models/merge_request_diff_spec.rb b/spec/models/merge_request_diff_spec.rb index 90cce826b6c..47e8f04e728 100644 --- a/spec/models/merge_request_diff_spec.rb +++ b/spec/models/merge_request_diff_spec.rb @@ -52,9 +52,9 @@ describe MergeRequestDiff do context 'when it was not cleaned by the system' do it 'returns persisted diffs' do - expect(diff).to receive(:load_diffs) + expect(diff).to receive(:load_diffs).and_call_original - diff.diffs + diff.diffs.diff_files end end @@ -76,19 +76,19 @@ describe MergeRequestDiff do end it 'returns persisted diffs if cannot compare with diff refs' do - expect(diff).to receive(:load_diffs) + expect(diff).to receive(:load_diffs).and_call_original diff.update!(head_commit_sha: 'invalid-sha') - diff.diffs + diff.diffs.diff_files end it 'returns persisted diffs if diff refs does not exist' do - expect(diff).to receive(:load_diffs) + expect(diff).to receive(:load_diffs).and_call_original diff.update!(start_commit_sha: nil, base_commit_sha: nil) - diff.diffs + diff.diffs.diff_files end end end @@ -211,4 +211,25 @@ describe MergeRequestDiff do expect(diff_with_commits.commits_count).to eq(29) end end + + describe '#commits_by_shas' do + let(:commit_shas) { diff_with_commits.commit_shas } + + it 'returns empty if no SHAs were provided' do + expect(diff_with_commits.commits_by_shas([])).to be_empty + end + + it 'returns one SHA' do + commits = diff_with_commits.commits_by_shas([commit_shas.first, Gitlab::Git::BLANK_SHA]) + + expect(commits.count).to eq(1) + end + + it 'returns all matching SHAs' do + commits = diff_with_commits.commits_by_shas(commit_shas) + + expect(commits.count).to eq(commit_shas.count) + expect(commits.map(&:sha)).to match_array(commit_shas) + end + end end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 85a4ebac66c..3a54725c7ec 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -552,9 +552,9 @@ describe MergeRequest do it 'delegates to the MR diffs' do merge_request.save - expect(merge_request.merge_request_diff).to receive(:raw_diffs).with(hash_including(options)) + expect(merge_request.merge_request_diff).to receive(:raw_diffs).with(hash_including(options)).and_call_original - merge_request.diffs(options) + merge_request.diffs(options).diff_files end end @@ -1836,8 +1836,8 @@ describe MergeRequest do let(:environments) { create_list(:environment, 3, project: project) } before do - create(:deployment, environment: environments.first, ref: 'master', sha: project.commit('master').id) - create(:deployment, environment: environments.second, ref: 'feature', sha: project.commit('feature').id) + create(:deployment, :success, environment: environments.first, ref: 'master', sha: project.commit('master').id) + create(:deployment, :success, environment: environments.second, ref: 'feature', sha: project.commit('feature').id) end it 'selects deployed environments' do @@ -1857,7 +1857,7 @@ describe MergeRequest do let(:source_environment) { create(:environment, project: source_project) } before do - create(:deployment, environment: source_environment, ref: 'feature', sha: merge_request.diff_head_sha) + create(:deployment, :success, environment: source_environment, ref: 'feature', sha: merge_request.diff_head_sha) end it 'selects deployed environments' do @@ -1868,7 +1868,7 @@ describe MergeRequest do let(:target_environment) { create(:environment, project: project) } before do - create(:deployment, environment: target_environment, tag: true, sha: merge_request.diff_head_sha) + create(:deployment, :success, environment: target_environment, tag: true, sha: merge_request.diff_head_sha) end it 'selects deployed environments' do @@ -2611,6 +2611,32 @@ describe MergeRequest do end end + describe '#includes_any_commits?' do + it 'returns false' do + expect(subject.includes_any_commits?([Gitlab::Git::BLANK_SHA])).to be_falsey + end + + it 'returns true' do + expect(subject.includes_any_commits?([subject.merge_request_diff.head_commit_sha])).to be_truthy + end + + it 'returns true even when there is a non-existent comit' do + expect(subject.includes_any_commits?([Gitlab::Git::BLANK_SHA, subject.merge_request_diff.head_commit_sha])).to be_truthy + end + + context 'unpersisted merge request' do + let(:new_mr) { build(:merge_request) } + + it 'returns false' do + expect(new_mr.includes_any_commits?([Gitlab::Git::BLANK_SHA])).to be_falsey + end + + it 'returns true' do + expect(new_mr.includes_any_commits?([subject.merge_request_diff.head_commit_sha])).to be_truthy + end + end + end + describe '#can_allow_collaboration?' do let(:target_project) { create(:project, :public) } let(:source_project) { fork_project(target_project) } diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index 8913644a3ce..2db42fe802a 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -562,6 +562,17 @@ describe Namespace do it { expect(group.all_projects.to_a).to match_array([project2, project1]) } end + describe '#all_pipelines' do + let(:group) { create(:group) } + let(:child) { create(:group, parent: group) } + let!(:project1) { create(:project_empty_repo, namespace: group) } + let!(:project2) { create(:project_empty_repo, namespace: child) } + let!(:pipeline1) { create(:ci_empty_pipeline, project: project1) } + let!(:pipeline2) { create(:ci_empty_pipeline, project: project2) } + + it { expect(group.all_pipelines.to_a).to match_array([pipeline1, pipeline2]) } + end + describe '#share_with_group_lock with subgroups', :nested_groups do context 'when creating a subgroup' do let(:subgroup) { create(:group, parent: root_group )} diff --git a/spec/models/postgresql/replication_slot_spec.rb b/spec/models/postgresql/replication_slot_spec.rb index 919a7526803..e100af7ddc7 100644 --- a/spec/models/postgresql/replication_slot_spec.rb +++ b/spec/models/postgresql/replication_slot_spec.rb @@ -3,7 +3,27 @@ require 'spec_helper' describe Postgresql::ReplicationSlot, :postgresql do + describe '.in_use?' do + it 'returns true when replication slots are present' do + expect(described_class).to receive(:exists?).and_return(true) + expect(described_class.in_use?).to be_truthy + end + + it 'returns false when replication slots are not present' do + expect(described_class.in_use?).to be_falsey + end + + it 'returns false if the existence check is invalid' do + expect(described_class).to receive(:exists?).and_raise(ActiveRecord::StatementInvalid.new('PG::FeatureNotSupported')) + expect(described_class.in_use?).to be_falsey + end + end + describe '.lag_too_great?' do + before do + expect(described_class).to receive(:in_use?).and_return(true) + end + it 'returns true when replication lag is too great' do expect(described_class) .to receive(:pluck) diff --git a/spec/models/project_services/kubernetes_service_spec.rb b/spec/models/project_services/kubernetes_service_spec.rb index 68ab9fd08ec..9c27357ffaf 100644 --- a/spec/models/project_services/kubernetes_service_spec.rb +++ b/spec/models/project_services/kubernetes_service_spec.rb @@ -253,7 +253,7 @@ describe KubernetesService, :use_clean_rails_memory_store_caching do end end - describe '#predefined_variables' do + describe '#predefined_variable' do let(:kubeconfig) do config_file = expand_fixture_path('config/kubeconfig.yml') config = YAML.load(File.read(config_file)) @@ -274,7 +274,7 @@ describe KubernetesService, :use_clean_rails_memory_store_caching do shared_examples 'setting variables' do it 'sets the variables' do - expect(subject.predefined_variables).to include( + expect(subject.predefined_variables(project: project)).to include( { key: 'KUBE_URL', value: 'https://kube.domain.com', public: true }, { key: 'KUBE_TOKEN', value: 'token', public: false }, { key: 'KUBE_NAMESPACE', value: namespace, public: true }, @@ -301,7 +301,7 @@ describe KubernetesService, :use_clean_rails_memory_store_caching do it_behaves_like 'setting variables' it 'sets the KUBE_NAMESPACE' do - kube_namespace = subject.predefined_variables.find { |h| h[:key] == 'KUBE_NAMESPACE' } + kube_namespace = subject.predefined_variables(project: project).find { |h| h[:key] == 'KUBE_NAMESPACE' } expect(kube_namespace).not_to be_nil expect(kube_namespace[:value]).to match(/\A#{Gitlab::PathRegex::PATH_REGEX_STR}-\d+\z/) diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index be2aff73c0a..471f19f9b7c 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -8,6 +8,7 @@ describe Project do it { is_expected.to belong_to(:group) } it { is_expected.to belong_to(:namespace) } it { is_expected.to belong_to(:creator).class_name('User') } + it { is_expected.to belong_to(:pool_repository) } it { is_expected.to have_many(:users) } it { is_expected.to have_many(:services) } it { is_expected.to have_many(:events) } @@ -2405,12 +2406,24 @@ describe Project do it_behaves_like 'same behavior between KubernetesService and Platform::Kubernetes' end - context 'when user configured kubernetes from CI/CD > Clusters' do + context 'when user configured kubernetes from CI/CD > Clusters and KubernetesNamespace migration has not been executed' do let!(:cluster) { create(:cluster, :project, :provided_by_gcp) } let(:project) { cluster.project } it_behaves_like 'same behavior between KubernetesService and Platform::Kubernetes' end + + context 'when user configured kubernetes from CI/CD > Clusters and KubernetesNamespace migration has been executed' do + let!(:kubernetes_namespace) { create(:cluster_kubernetes_namespace) } + let!(:cluster) { kubernetes_namespace.cluster } + let(:project) { kubernetes_namespace.project } + + it 'should return token from kubernetes namespace' do + expect(project.deployment_variables).to include( + { key: 'KUBE_TOKEN', value: kubernetes_namespace.service_account_token, public: false } + ) + end + end end end @@ -3963,6 +3976,40 @@ describe Project do end end + describe '.deployments' do + subject { project.deployments } + + let(:project) { create(:project) } + + before do + allow_any_instance_of(Deployment).to receive(:create_ref) + end + + context 'when there is a deployment record with created status' do + let(:deployment) { create(:deployment, :created, project: project) } + + it 'does not return the record' do + is_expected.to be_empty + end + end + + context 'when there is a deployment record with running status' do + let(:deployment) { create(:deployment, :running, project: project) } + + it 'does not return the record' do + is_expected.to be_empty + end + end + + context 'when there is a deployment record with success status' do + let(:deployment) { create(:deployment, :success, project: project) } + + it 'returns the record' do + is_expected.to eq([deployment]) + end + end + end + describe '#snippets_visible?' do it 'returns true when a logged in user can read snippets' do project = create(:project, :public) diff --git a/spec/models/shard_spec.rb b/spec/models/shard_spec.rb new file mode 100644 index 00000000000..83104711b55 --- /dev/null +++ b/spec/models/shard_spec.rb @@ -0,0 +1,50 @@ +# frozen_string_literals: true +require 'spec_helper' + +describe Shard do + describe '.populate!' do + it 'creates shards based on the config file' do + expect(described_class.all).to be_empty + + stub_storage_settings(foo: {}, bar: {}, baz: {}) + + described_class.populate! + + expect(described_class.all.map(&:name)).to match_array(%w[default foo bar baz]) + end + end + + describe '.by_name' do + let(:default_shard) { described_class.find_by(name: 'default') } + + before do + described_class.populate! + end + + it 'returns an existing shard' do + expect(described_class.by_name('default')).to eq(default_shard) + end + + it 'creates a new shard' do + result = described_class.by_name('foo') + + expect(result).not_to eq(default_shard) + expect(result.name).to eq('foo') + end + + it 'retries if creation races' do + expect(described_class) + .to receive(:find_or_create_by) + .with(name: 'default') + .and_raise(ActiveRecord::RecordNotUnique, 'fail') + .once + + expect(described_class) + .to receive(:find_or_create_by) + .with(name: 'default') + .and_call_original + + expect(described_class.by_name('default')).to eq(default_shard) + end + end +end diff --git a/spec/models/user_preference_spec.rb b/spec/models/user_preference_spec.rb index 64d9d9a78b4..2898613545c 100644 --- a/spec/models/user_preference_spec.rb +++ b/spec/models/user_preference_spec.rb @@ -6,22 +6,43 @@ describe UserPreference do describe '#set_notes_filter' do let(:issuable) { build_stubbed(:issue) } let(:user_preference) { create(:user_preference) } - let(:only_comments) { described_class::NOTES_FILTERS[:only_comments] } - it 'returns updated discussion filter' do - filter_name = - user_preference.set_notes_filter(only_comments, issuable) + shared_examples 'setting system notes' do + it 'returns updated discussion filter' do + filter_name = + user_preference.set_notes_filter(filter, issuable) + + expect(filter_name).to eq(filter) + end + + it 'updates discussion filter for issuable class' do + user_preference.set_notes_filter(filter, issuable) + + expect(user_preference.reload.issue_notes_filter).to eq(filter) + end + end + + context 'when filter is set to all notes' do + let(:filter) { described_class::NOTES_FILTERS[:all_notes] } + + it_behaves_like 'setting system notes' + end + + context 'when filter is set to only comments' do + let(:filter) { described_class::NOTES_FILTERS[:only_comments] } - expect(filter_name).to eq(only_comments) + it_behaves_like 'setting system notes' end - it 'updates discussion filter for issuable class' do - user_preference.set_notes_filter(only_comments, issuable) + context 'when filter is set to only activity' do + let(:filter) { described_class::NOTES_FILTERS[:only_activity] } - expect(user_preference.reload.issue_notes_filter).to eq(only_comments) + it_behaves_like 'setting system notes' end context 'when notes_filter parameter is invalid' do + let(:only_comments) { described_class::NOTES_FILTERS[:only_comments] } + it 'returns the current notes filter' do user_preference.set_notes_filter(only_comments, issuable) |