diff options
author | Shinya Maeda <shinya@gitlab.com> | 2018-06-02 05:15:53 +0300 |
---|---|---|
committer | Shinya Maeda <shinya@gitlab.com> | 2018-06-02 05:15:53 +0300 |
commit | b02b2602c1bcfd60b760fbfd1aca936475d78474 (patch) | |
tree | 09d906b8d6fafcae31994cf0c2af6af2af6970fd /spec/models | |
parent | c89e57842ebf7f395363bcddaeff76bc7b3f7890 (diff) | |
parent | fe0ebf76c49e2512b211c5d43152275c536f7e3a (diff) |
Merge branch 'master' into per-project-pipeline-iid
Diffstat (limited to 'spec/models')
-rw-r--r-- | spec/models/ci/build_spec.rb | 10 | ||||
-rw-r--r-- | spec/models/ci/pipeline_spec.rb | 2 | ||||
-rw-r--r-- | spec/models/ci/runner_spec.rb | 199 | ||||
-rw-r--r-- | spec/models/clusters/applications/jupyter_spec.rb | 59 | ||||
-rw-r--r-- | spec/models/clusters/cluster_spec.rb | 3 | ||||
-rw-r--r-- | spec/models/concerns/cacheable_attributes_spec.rb | 133 | ||||
-rw-r--r-- | spec/models/concerns/has_variable_spec.rb | 4 | ||||
-rw-r--r-- | spec/models/concerns/issuable_spec.rb | 5 | ||||
-rw-r--r-- | spec/models/concerns/reactive_caching_spec.rb | 1 | ||||
-rw-r--r-- | spec/models/project_spec.rb | 14 | ||||
-rw-r--r-- | spec/models/remote_mirror_spec.rb | 2 | ||||
-rw-r--r-- | spec/models/timelog_spec.rb | 3 | ||||
-rw-r--r-- | spec/models/user_spec.rb | 40 |
13 files changed, 303 insertions, 172 deletions
diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index c89655f4058..66c9708b4cf 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -148,10 +148,9 @@ describe Ci::Build do end context 'when there are runners' do - let(:runner) { create(:ci_runner) } + let(:runner) { create(:ci_runner, :project, projects: [build.project]) } before do - build.project.runners << runner runner.update_attributes(contacted_at: 1.second.ago) end @@ -1388,12 +1387,7 @@ describe Ci::Build do it { is_expected.to be_truthy } context "and there are specific runner" do - let(:runner) { create(:ci_runner, contacted_at: 1.second.ago) } - - before do - build.project.runners << runner - runner.save - end + let!(:runner) { create(:ci_runner, :project, projects: [build.project], contacted_at: 1.second.ago) } it { is_expected.to be_falsey } end diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 2b9c232743d..24692ebb9a3 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -1600,7 +1600,7 @@ describe Ci::Pipeline, :mailer do context 'when pipeline is not stuck' do before do - create(:ci_runner, :shared, :online) + create(:ci_runner, :instance, :online) end it 'is not stuck' do diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index 0fbc934f669..0f072aa1719 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -21,60 +21,58 @@ describe Ci::Runner do end end - context 'either_projects_or_group' do + context '#exactly_one_group' do let(:group) { create(:group) } + let(:runner) { create(:ci_runner, :group, groups: [group]) } - it 'disallows assigning to a group if already assigned to a group' do - runner = create(:ci_runner, groups: [group]) - + it 'disallows assigning group if already assigned to a group' do runner.groups << build(:group) expect(runner).not_to be_valid - expect(runner.errors.full_messages).to eq ['Runner can only be assigned to one group'] + expect(runner.errors.full_messages).to include('Runner needs to be assigned to exactly one group') end + end - it 'disallows assigning to a group if already assigned to a project' do - project = create(:project) - runner = create(:ci_runner, projects: [project]) + context 'runner_type validations' do + set(:group) { create(:group) } + set(:project) { create(:project) } + let(:group_runner) { create(:ci_runner, :group, groups: [group]) } + let(:project_runner) { create(:ci_runner, :project, projects: [project]) } + let(:instance_runner) { create(:ci_runner, :instance) } - runner.groups << build(:group) + it 'disallows assigning group to project_type runner' do + project_runner.groups << build(:group) - expect(runner).not_to be_valid - expect(runner.errors.full_messages).to eq ['Runner can only be assigned either to projects or to a group'] + expect(project_runner).not_to be_valid + expect(project_runner.errors.full_messages).to include('Runner cannot have groups assigned') end - it 'disallows assigning to a project if already assigned to a group' do - runner = create(:ci_runner, groups: [group]) - - runner.projects << build(:project) + it 'disallows assigning group to instance_type runner' do + instance_runner.groups << build(:group) - expect(runner).not_to be_valid - expect(runner.errors.full_messages).to eq ['Runner can only be assigned either to projects or to a group'] + expect(instance_runner).not_to be_valid + expect(instance_runner.errors.full_messages).to include('Runner cannot have groups assigned') end - it 'allows assigning to a group if not assigned to a group nor a project' do - runner = create(:ci_runner) - - runner.groups << build(:group) + it 'disallows assigning project to group_type runner' do + group_runner.projects << build(:project) - expect(runner).to be_valid + expect(group_runner).not_to be_valid + expect(group_runner.errors.full_messages).to include('Runner cannot have projects assigned') end - it 'allows assigning to a project if not assigned to a group nor a project' do - runner = create(:ci_runner) - - runner.projects << build(:project) + it 'disallows assigning project to instance_type runner' do + instance_runner.projects << build(:project) - expect(runner).to be_valid + expect(instance_runner).not_to be_valid + expect(instance_runner.errors.full_messages).to include('Runner cannot have projects assigned') end - it 'allows assigning to a project if already assigned to a project' do - project = create(:project) - runner = create(:ci_runner, projects: [project]) - - runner.projects << build(:project) + it 'should fail to save a group assigned to a project runner even if the runner is already saved' do + group_runner - expect(runner).to be_valid + expect { create(:group, runners: [project_runner]) } + .to raise_error(ActiveRecord::RecordInvalid) end end end @@ -110,17 +108,12 @@ describe Ci::Runner do describe '.shared' do let(:group) { create(:group) } let(:project) { create(:project) } + let!(:group_runner) { create(:ci_runner, :group, groups: [group]) } + let!(:project_runner) { create(:ci_runner, :project, projects: [project]) } + let!(:shared_runner) { create(:ci_runner, :instance) } - it 'returns the shared group runner' do - runner = create(:ci_runner, :shared, groups: [group]) - - expect(described_class.shared).to eq [runner] - end - - it 'returns the shared project runner' do - runner = create(:ci_runner, :shared, projects: [project]) - - expect(described_class.shared).to eq [runner] + it 'returns only shared runners' do + expect(described_class.shared).to contain_exactly(shared_runner) end end @@ -128,11 +121,11 @@ describe Ci::Runner do it 'returns the specific project runner' do # own specific_project = create(:project) - specific_runner = create(:ci_runner, :specific, projects: [specific_project]) + specific_runner = create(:ci_runner, :project, projects: [specific_project]) # other other_project = create(:project) - create(:ci_runner, :specific, projects: [other_project]) + create(:ci_runner, :project, projects: [other_project]) expect(described_class.belonging_to_project(specific_project.id)).to eq [specific_runner] end @@ -141,17 +134,17 @@ describe Ci::Runner do describe '.belonging_to_parent_group_of_project' do let(:project) { create(:project, group: group) } let(:group) { create(:group) } - let(:runner) { create(:ci_runner, :specific, groups: [group]) } + let(:runner) { create(:ci_runner, :group, groups: [group]) } let!(:unrelated_group) { create(:group) } let!(:unrelated_project) { create(:project, group: unrelated_group) } - let!(:unrelated_runner) { create(:ci_runner, :specific, groups: [unrelated_group]) } + let!(:unrelated_runner) { create(:ci_runner, :group, groups: [unrelated_group]) } it 'returns the specific group runner' do expect(described_class.belonging_to_parent_group_of_project(project.id)).to contain_exactly(runner) end context 'with a parent group with a runner', :nested_groups do - let(:runner) { create(:ci_runner, :specific, groups: [parent_group]) } + let(:runner) { create(:ci_runner, :group, groups: [parent_group]) } let(:project) { create(:project, group: group) } let(:group) { create(:group, parent: parent_group) } let(:parent_group) { create(:group) } @@ -167,13 +160,13 @@ describe Ci::Runner do # group specific group = create(:group) project = create(:project, group: group) - group_runner = create(:ci_runner, :specific, groups: [group]) + group_runner = create(:ci_runner, :group, groups: [group]) # project specific - project_runner = create(:ci_runner, :specific, projects: [project]) + project_runner = create(:ci_runner, :project, projects: [project]) # globally shared - shared_runner = create(:ci_runner, :shared) + shared_runner = create(:ci_runner, :instance) expect(described_class.owned_or_shared(project.id)).to contain_exactly( group_runner, project_runner, shared_runner @@ -183,31 +176,32 @@ describe Ci::Runner do describe '#display_name' do it 'returns the description if it has a value' do - runner = FactoryBot.build(:ci_runner, description: 'Linux/Ruby-1.9.3-p448') + runner = build(:ci_runner, description: 'Linux/Ruby-1.9.3-p448') expect(runner.display_name).to eq 'Linux/Ruby-1.9.3-p448' end it 'returns the token if it does not have a description' do - runner = FactoryBot.create(:ci_runner) + runner = create(:ci_runner) expect(runner.display_name).to eq runner.description end it 'returns the token if the description is an empty string' do - runner = FactoryBot.build(:ci_runner, description: '', token: 'token') + runner = build(:ci_runner, description: '', token: 'token') expect(runner.display_name).to eq runner.token end end describe '#assign_to' do - let!(:project) { FactoryBot.create(:project) } + let(:project) { create(:project) } subject { runner.assign_to(project) } context 'with shared_runner' do - let!(:runner) { FactoryBot.create(:ci_runner, :shared) } + let(:runner) { create(:ci_runner, :instance) } it 'transitions shared runner to project runner and assigns project' do - subject + expect(subject).to be_truthy + expect(runner).to be_specific expect(runner).to be_project_type expect(runner.projects).to eq([project]) @@ -216,7 +210,8 @@ describe Ci::Runner do end context 'with group runner' do - let!(:runner) { FactoryBot.create(:ci_runner, runner_type: :group_type) } + let(:group) { create(:group) } + let(:runner) { create(:ci_runner, :group, groups: [group]) } it 'raises an error' do expect { subject } @@ -229,15 +224,15 @@ describe Ci::Runner do subject { described_class.online } before do - @runner1 = FactoryBot.create(:ci_runner, :shared, contacted_at: 1.year.ago) - @runner2 = FactoryBot.create(:ci_runner, :shared, contacted_at: 1.second.ago) + @runner1 = create(:ci_runner, :instance, contacted_at: 1.year.ago) + @runner2 = create(:ci_runner, :instance, contacted_at: 1.second.ago) end it { is_expected.to eq([@runner2])} end describe '#online?' do - let(:runner) { FactoryBot.create(:ci_runner, :shared) } + let(:runner) { create(:ci_runner, :instance) } subject { runner.online? } @@ -307,21 +302,20 @@ describe Ci::Runner do end describe '#can_pick?' do - let(:pipeline) { create(:ci_pipeline) } + set(:pipeline) { create(:ci_pipeline) } let(:build) { create(:ci_build, pipeline: pipeline) } - let(:runner) { create(:ci_runner, tag_list: tag_list, run_untagged: run_untagged) } + let(:runner_project) { build.project } + let(:runner) { create(:ci_runner, :project, projects: [runner_project], tag_list: tag_list, run_untagged: run_untagged) } let(:tag_list) { [] } let(:run_untagged) { true } subject { runner.can_pick?(build) } - before do - build.project.runners << runner - end - context 'a different runner' do + let(:other_project) { create(:project) } + let(:other_runner) { create(:ci_runner, :project, projects: [other_project], tag_list: tag_list, run_untagged: run_untagged) } + it 'cannot handle builds' do - other_runner = create(:ci_runner) expect(other_runner.can_pick?(build)).to be_falsey end end @@ -375,18 +369,14 @@ describe Ci::Runner do end context 'when runner is shared' do - let(:runner) { create(:ci_runner, :shared) } - - before do - build.project.runners = [] - end + let(:runner) { create(:ci_runner, :instance) } it 'can handle builds' do expect(runner.can_pick?(build)).to be_truthy end context 'when runner is locked' do - let(:runner) { create(:ci_runner, :shared, locked: true) } + let(:runner) { create(:ci_runner, :instance, locked: true) } it 'can handle builds' do expect(runner.can_pick?(build)).to be_truthy @@ -401,10 +391,8 @@ describe Ci::Runner do end end - context 'when runner is not assigned to a project' do - before do - build.project.runners = [] - end + context 'when runner is assigned to another project' do + let(:runner_project) { create(:project) } it 'cannot handle builds' do expect(runner.can_pick?(build)).to be_falsey @@ -412,10 +400,8 @@ describe Ci::Runner do end context 'when runner is assigned to a group' do - before do - build.project.runners = [] - runner.groups << create(:group, projects: [build.project]) - end + let(:group) { create(:group, projects: [build.project]) } + let(:runner) { create(:ci_runner, :group, tag_list: tag_list, run_untagged: run_untagged, groups: [group]) } it 'can handle builds' do expect(runner.can_pick?(build)).to be_truthy @@ -469,7 +455,7 @@ describe Ci::Runner do end describe '#status' do - let(:runner) { FactoryBot.create(:ci_runner, :shared, contacted_at: 1.second.ago) } + let(:runner) { create(:ci_runner, :instance, contacted_at: 1.second.ago) } subject { runner.status } @@ -626,12 +612,13 @@ describe Ci::Runner do end describe '.assignable_for' do - let!(:unlocked_project_runner) { create(:ci_runner, runner_type: :project_type, projects: [project]) } - let!(:locked_project_runner) { create(:ci_runner, runner_type: :project_type, locked: true, projects: [project]) } - let!(:group_runner) { create(:ci_runner, runner_type: :group_type) } - let!(:instance_runner) { create(:ci_runner, :shared) } let(:project) { create(:project) } + let(:group) { create(:group) } let(:another_project) { create(:project) } + let!(:unlocked_project_runner) { create(:ci_runner, :project, projects: [project]) } + let!(:locked_project_runner) { create(:ci_runner, :project, locked: true, projects: [project]) } + let!(:group_runner) { create(:ci_runner, :group, groups: [group]) } + let!(:instance_runner) { create(:ci_runner, :instance) } context 'with already assigned project' do subject { described_class.assignable_for(project) } @@ -651,19 +638,16 @@ describe Ci::Runner do describe "belongs_to_one_project?" do it "returns false if there are two projects runner assigned to" do - runner = FactoryBot.create(:ci_runner) - project = FactoryBot.create(:project) - project1 = FactoryBot.create(:project) - project.runners << runner - project1.runners << runner + project1 = create(:project) + project2 = create(:project) + runner = create(:ci_runner, :project, projects: [project1, project2]) expect(runner.belongs_to_one_project?).to be_falsey end it "returns true" do - runner = FactoryBot.create(:ci_runner) - project = FactoryBot.create(:project) - project.runners << runner + project = create(:project) + runner = create(:ci_runner, :project, projects: [project]) expect(runner.belongs_to_one_project?).to be_truthy end @@ -713,21 +697,21 @@ describe Ci::Runner do subject { runner.assigned_to_group? } context 'when project runner' do - let(:runner) { create(:ci_runner, description: 'Project runner', projects: [project]) } + let(:runner) { create(:ci_runner, :project, description: 'Project runner', projects: [project]) } let(:project) { create(:project) } it { is_expected.to be_falsey } end context 'when shared runner' do - let(:runner) { create(:ci_runner, :shared, description: 'Shared runner') } + let(:runner) { create(:ci_runner, :instance, description: 'Shared runner') } it { is_expected.to be_falsey } end context 'when group runner' do let(:group) { create(:group) } - let(:runner) { create(:ci_runner, description: 'Group runner', groups: [group]) } + let(:runner) { create(:ci_runner, :group, description: 'Group runner', groups: [group]) } it { is_expected.to be_truthy } end @@ -737,18 +721,18 @@ describe Ci::Runner do subject { runner.assigned_to_project? } context 'when group runner' do - let(:runner) { create(:ci_runner, description: 'Group runner', groups: [group]) } + let(:runner) { create(:ci_runner, :group, description: 'Group runner', groups: [group]) } let(:group) { create(:group) } it { is_expected.to be_falsey } end context 'when shared runner' do - let(:runner) { create(:ci_runner, :shared, description: 'Shared runner') } + let(:runner) { create(:ci_runner, :instance, description: 'Shared runner') } it { is_expected.to be_falsey } end context 'when project runner' do - let(:runner) { create(:ci_runner, description: 'Group runner', projects: [project]) } + let(:runner) { create(:ci_runner, :project, description: 'Project runner', projects: [project]) } let(:project) { create(:project) } it { is_expected.to be_truthy } @@ -780,4 +764,17 @@ describe Ci::Runner do end end end + + describe 'project runner without projects is destroyable' do + subject { create(:ci_runner, :project, :without_projects) } + + it 'does not have projects' do + expect(subject.runner_projects).to be_empty + end + + it 'can be destroyed' do + subject + expect { subject.destroy }.to change { described_class.count }.by(-1) + end + end end diff --git a/spec/models/clusters/applications/jupyter_spec.rb b/spec/models/clusters/applications/jupyter_spec.rb new file mode 100644 index 00000000000..ca48a1d8072 --- /dev/null +++ b/spec/models/clusters/applications/jupyter_spec.rb @@ -0,0 +1,59 @@ +require 'rails_helper' + +describe Clusters::Applications::Jupyter do + include_examples 'cluster application core specs', :clusters_applications_jupyter + + it { is_expected.to belong_to(:oauth_application) } + + describe '#set_initial_status' do + before do + jupyter.set_initial_status + end + + context 'when ingress is not installed' do + let(:cluster) { create(:cluster, :provided_by_gcp) } + let(:jupyter) { create(:clusters_applications_jupyter, cluster: cluster) } + + it { expect(jupyter).to be_not_installable } + end + + context 'when ingress is installed and external_ip is assigned' do + let(:ingress) { create(:clusters_applications_ingress, :installed, external_ip: '127.0.0.1') } + let(:jupyter) { create(:clusters_applications_jupyter, cluster: ingress.cluster) } + + it { expect(jupyter).to be_installable } + end + end + + describe '#install_command' do + let!(:ingress) { create(:clusters_applications_ingress, :installed, external_ip: '127.0.0.1') } + let!(:jupyter) { create(:clusters_applications_jupyter, cluster: ingress.cluster) } + + subject { jupyter.install_command } + + it { is_expected.to be_an_instance_of(Gitlab::Kubernetes::Helm::InstallCommand) } + + it 'should be initialized with 4 arguments' do + expect(subject.name).to eq('jupyter') + expect(subject.chart).to eq('jupyter/jupyterhub') + expect(subject.repository).to eq('https://jupyterhub.github.io/helm-chart/') + expect(subject.values).to eq(jupyter.values) + end + end + + describe '#values' do + let(:jupyter) { create(:clusters_applications_jupyter) } + + subject { jupyter.values } + + it 'should include valid values' do + is_expected.to include('ingress') + is_expected.to include('hub') + is_expected.to include('rbac') + is_expected.to include('proxy') + is_expected.to include('auth') + is_expected.to include("clientId: #{jupyter.oauth_application.uid}") + is_expected.to include("callbackUrl: #{jupyter.callback_url}") + end + end +end diff --git a/spec/models/clusters/cluster_spec.rb b/spec/models/clusters/cluster_spec.rb index b942554d67b..6f66515b45f 100644 --- a/spec/models/clusters/cluster_spec.rb +++ b/spec/models/clusters/cluster_spec.rb @@ -234,9 +234,10 @@ describe Clusters::Cluster do let!(:ingress) { create(:clusters_applications_ingress, cluster: cluster) } let!(:prometheus) { create(:clusters_applications_prometheus, cluster: cluster) } let!(:runner) { create(:clusters_applications_runner, cluster: cluster) } + let!(:jupyter) { create(:clusters_applications_jupyter, cluster: cluster) } it 'returns a list of created applications' do - is_expected.to contain_exactly(helm, ingress, prometheus, runner) + is_expected.to contain_exactly(helm, ingress, prometheus, runner, jupyter) end end end diff --git a/spec/models/concerns/cacheable_attributes_spec.rb b/spec/models/concerns/cacheable_attributes_spec.rb index 49e4b23ebc7..c6331c5ec15 100644 --- a/spec/models/concerns/cacheable_attributes_spec.rb +++ b/spec/models/concerns/cacheable_attributes_spec.rb @@ -22,7 +22,7 @@ describe CacheableAttributes do attr_accessor :attributes - def initialize(attrs = {}) + def initialize(attrs = {}, *) @attributes = attrs end end @@ -52,7 +52,7 @@ describe CacheableAttributes do describe '.cache_key' do it 'excludes cache attributes' do - expect(minimal_test_class.cache_key).to eq("TestClass:#{Gitlab::VERSION}:#{Gitlab.migrations_hash}:json") + expect(minimal_test_class.cache_key).to eq("TestClass:#{Gitlab::VERSION}:#{Gitlab.migrations_hash}:#{Rails.version}") end end @@ -75,49 +75,117 @@ describe CacheableAttributes do context 'without any attributes given' do it 'intializes a new object with the defaults' do - expect(minimal_test_class.build_from_defaults).not_to be_persisted + expect(minimal_test_class.build_from_defaults.attributes).to eq(minimal_test_class.defaults) end end - context 'without attributes given' do + context 'with attributes given' do it 'intializes a new object with the given attributes merged into the defaults' do expect(minimal_test_class.build_from_defaults(foo: 'd').attributes[:foo]).to eq('d') end end + + describe 'edge cases on concrete implementations' do + describe '.build_from_defaults' do + context 'without any attributes given' do + it 'intializes all attributes even if they are nil' do + record = ApplicationSetting.build_from_defaults + + expect(record).not_to be_persisted + expect(record.sign_in_text).to be_nil + end + end + end + end end describe '.current', :use_clean_rails_memory_store_caching do context 'redis unavailable' do - it 'returns an uncached record' do + before do allow(minimal_test_class).to receive(:last).and_return(:last) - expect(Rails.cache).to receive(:read).and_raise(Redis::BaseError) + expect(Rails.cache).to receive(:read).with(minimal_test_class.cache_key).and_raise(Redis::BaseError) + end + + context 'in production environment' do + before do + expect(Rails.env).to receive(:production?).and_return(true) + end + + it 'returns an uncached record and logs a warning' do + expect(Rails.logger).to receive(:warn).with("Cached record for TestClass couldn't be loaded, falling back to uncached record: Redis::BaseError") - expect(minimal_test_class.current).to eq(:last) + expect(minimal_test_class.current).to eq(:last) + end + end + + context 'in other environments' do + before do + expect(Rails.env).to receive(:production?).and_return(false) + end + + it 'returns an uncached record and logs a warning' do + expect(Rails.logger).not_to receive(:warn) + + expect { minimal_test_class.current }.to raise_error(Redis::BaseError) + end end end context 'when a record is not yet present' do it 'does not cache nil object' do # when missing settings a nil object is returned, but not cached - allow(minimal_test_class).to receive(:last).twice.and_return(nil) + allow(ApplicationSetting).to receive(:current_without_cache).twice.and_return(nil) - expect(minimal_test_class.current).to be_nil - expect(Rails.cache.exist?(minimal_test_class.cache_key)).to be(false) + expect(ApplicationSetting.current).to be_nil + expect(Rails.cache.exist?(ApplicationSetting.cache_key)).to be(false) end - it 'cache non-nil object' do - # when the settings are set the method returns a valid object - allow(minimal_test_class).to receive(:last).and_call_original + it 'caches non-nil object' do + create(:application_setting) - expect(minimal_test_class.current).to eq(minimal_test_class.last) - expect(Rails.cache.exist?(minimal_test_class.cache_key)).to be(true) + expect(ApplicationSetting.current).to eq(ApplicationSetting.last) + expect(Rails.cache.exist?(ApplicationSetting.cache_key)).to be(true) # subsequent calls retrieve the record from the cache - last_record = minimal_test_class.last - expect(minimal_test_class).not_to receive(:last) - expect(minimal_test_class.current.attributes).to eq(last_record.attributes) + last_record = ApplicationSetting.last + expect(ApplicationSetting).not_to receive(:current_without_cache) + expect(ApplicationSetting.current.attributes).to eq(last_record.attributes) end end + + describe 'edge cases' do + describe 'caching behavior', :use_clean_rails_memory_store_caching do + it 'retrieves upload fields properly' do + ar_record = create(:appearance, :with_logo) + ar_record.cache! + + cache_record = Appearance.current + + expect(cache_record).to be_persisted + expect(cache_record.logo).to be_an(AttachmentUploader) + expect(cache_record.logo.url).to end_with('/dk.png') + end + + it 'retrieves markdown fields properly' do + ar_record = create(:appearance, description: '**Hello**') + ar_record.cache! + + cache_record = Appearance.current + + expect(cache_record.description).to eq('**Hello**') + expect(cache_record.description_html).to eq('<p dir="auto"><strong>Hello</strong></p>') + end + end + end + + it 'uses RequestStore in addition to Rails.cache', :request_store do + # Warm up the cache + create(:application_setting).cache! + + expect(Rails.cache).to receive(:read).with(ApplicationSetting.cache_key).once.and_call_original + + 2.times { ApplicationSetting.current } + end end describe '.cached', :use_clean_rails_memory_store_caching do @@ -127,27 +195,36 @@ describe CacheableAttributes do end end - context 'when cached settings do not include the latest defaults' do + context 'when cached is warm' do before do - Rails.cache.write(minimal_test_class.cache_key, { bar: 'b', baz: 'c' }.to_json) - minimal_test_class.define_singleton_method(:defaults) do - { foo: 'a', bar: 'b', baz: 'c' } - end + # Warm up the cache + create(:appearance).cache! end - it 'includes attributes from defaults' do - expect(minimal_test_class.cached.attributes[:foo]).to eq(minimal_test_class.defaults[:foo]) + it 'retrieves the record from cache' do + expect(ActiveRecord::QueryRecorder.new { Appearance.cached }.count).to eq(0) + expect(Appearance.cached).to eq(Appearance.current_without_cache) end end end describe '#cache!', :use_clean_rails_memory_store_caching do - let(:appearance_record) { create(:appearance) } + let(:record) { create(:appearance) } it 'caches the attributes' do - appearance_record.cache! + record.cache! - expect(Rails.cache.read(Appearance.cache_key)).to eq(appearance_record.attributes.to_json) + expect(Rails.cache.read(Appearance.cache_key)).to eq(record) + end + + describe 'edge cases' do + let(:record) { create(:appearance) } + + it 'caches the attributes' do + record.cache! + + expect(Rails.cache.read(Appearance.cache_key)).to eq(record) + end end end end diff --git a/spec/models/concerns/has_variable_spec.rb b/spec/models/concerns/has_variable_spec.rb index f87869a2fdc..3fbe86c5b56 100644 --- a/spec/models/concerns/has_variable_spec.rb +++ b/spec/models/concerns/has_variable_spec.rb @@ -45,8 +45,10 @@ describe HasVariable do end it 'fails to decrypt if iv is incorrect' do - subject.encrypted_value_iv = SecureRandom.hex + # attr_encrypted expects the IV to be 16 bytes and base64-encoded + subject.encrypted_value_iv = [SecureRandom.hex(8)].pack('m') subject.instance_variable_set(:@value, nil) + expect { subject.value } .to raise_error(OpenSSL::Cipher::CipherError, 'bad decrypt') end diff --git a/spec/models/concerns/issuable_spec.rb b/spec/models/concerns/issuable_spec.rb index bd6bf5b0712..1cfd526834c 100644 --- a/spec/models/concerns/issuable_spec.rb +++ b/spec/models/concerns/issuable_spec.rb @@ -12,6 +12,7 @@ describe Issuable do it { is_expected.to belong_to(:author) } it { is_expected.to have_many(:notes).dependent(:destroy) } it { is_expected.to have_many(:todos).dependent(:destroy) } + it { is_expected.to have_many(:labels) } context 'Notes' do let!(:note) { create(:note, noteable: issue, project: issue.project) } @@ -274,8 +275,8 @@ describe Issuable do it 'skips coercion for not Integer values' do expect { issue.time_estimate = nil }.to change { issue.time_estimate }.to(nil) - expect { issue.time_estimate = 'invalid time' }.not_to raise_error(StandardError) - expect { issue.time_estimate = 22.33 }.not_to raise_error(StandardError) + expect { issue.time_estimate = 'invalid time' }.not_to raise_error + expect { issue.time_estimate = 22.33 }.not_to raise_error end end diff --git a/spec/models/concerns/reactive_caching_spec.rb b/spec/models/concerns/reactive_caching_spec.rb index 4570dbb1d8e..f2a3df50c1a 100644 --- a/spec/models/concerns/reactive_caching_spec.rb +++ b/spec/models/concerns/reactive_caching_spec.rb @@ -94,6 +94,7 @@ describe ReactiveCaching, :use_clean_rails_memory_store_caching do end it { expect(instance.result).to be_nil } + it { expect(reactive_cache_alive?(instance)).to be_falsy } end describe '#exclusively_update_reactive_cache!' do diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index af2240f4f89..9a76452a808 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -1177,8 +1177,8 @@ describe Project do describe '#any_runners?' do context 'shared runners' do let(:project) { create(:project, shared_runners_enabled: shared_runners_enabled) } - let(:specific_runner) { create(:ci_runner) } - let(:shared_runner) { create(:ci_runner, :shared) } + let(:specific_runner) { create(:ci_runner, :project, projects: [project]) } + let(:shared_runner) { create(:ci_runner, :instance) } context 'for shared runners disabled' do let(:shared_runners_enabled) { false } @@ -1188,7 +1188,7 @@ describe Project do end it 'has a specific runner' do - project.runners << specific_runner + specific_runner expect(project.any_runners?).to be_truthy end @@ -1200,13 +1200,13 @@ describe Project do end it 'checks the presence of specific runner' do - project.runners << specific_runner + specific_runner expect(project.any_runners? { |runner| runner == specific_runner }).to be_truthy end it 'returns false if match cannot be found' do - project.runners << specific_runner + specific_runner expect(project.any_runners? { false }).to be_falsey end @@ -1238,7 +1238,7 @@ describe Project do context 'group runners' do let(:project) { create(:project, group_runners_enabled: group_runners_enabled) } let(:group) { create(:group, projects: [project]) } - let(:group_runner) { create(:ci_runner, groups: [group]) } + let(:group_runner) { create(:ci_runner, :group, groups: [group]) } context 'for group runners disabled' do let(:group_runners_enabled) { false } @@ -1279,7 +1279,7 @@ describe Project do end describe '#shared_runners' do - let!(:runner) { create(:ci_runner, :shared) } + let!(:runner) { create(:ci_runner, :instance) } subject { project.shared_runners } diff --git a/spec/models/remote_mirror_spec.rb b/spec/models/remote_mirror_spec.rb index a80800c6c92..1d94abe4195 100644 --- a/spec/models/remote_mirror_spec.rb +++ b/spec/models/remote_mirror_spec.rb @@ -12,8 +12,8 @@ describe RemoteMirror do context 'with an invalid URL' do it 'should not be valid' do remote_mirror = build(:remote_mirror, url: 'ftp://invalid.invalid') + expect(remote_mirror).not_to be_valid - expect(remote_mirror.errors[:url].size).to eq(2) end end end diff --git a/spec/models/timelog_spec.rb b/spec/models/timelog_spec.rb index 6e30798356c..a0c93c531ea 100644 --- a/spec/models/timelog_spec.rb +++ b/spec/models/timelog_spec.rb @@ -5,6 +5,9 @@ RSpec.describe Timelog do let(:issue) { create(:issue) } let(:merge_request) { create(:merge_request) } + it { is_expected.to belong_to(:issue).touch(true) } + it { is_expected.to belong_to(:merge_request).touch(true) } + it { is_expected.to be_valid } it { is_expected.to validate_presence_of(:time_spent) } diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 16b409844fa..09dfeae6377 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -1858,13 +1858,10 @@ describe User do describe '#ci_owned_runners' do let(:user) { create(:user) } - let(:runner_1) { create(:ci_runner) } - let(:runner_2) { create(:ci_runner) } + let!(:project) { create(:project) } + let(:runner) { create(:ci_runner, :project, projects: [project]) } context 'without any projects nor groups' do - let!(:project) { create(:project, runners: [runner_1]) } - let!(:group) { create(:group) } - it 'does not load' do expect(user.ci_owned_runners).to be_empty end @@ -1872,38 +1869,40 @@ describe User do context 'with personal projects runners' do let(:namespace) { create(:namespace, owner: user) } - let!(:project) { create(:project, namespace: namespace, runners: [runner_1]) } + let!(:project) { create(:project, namespace: namespace) } it 'loads' do - expect(user.ci_owned_runners).to contain_exactly(runner_1) + expect(user.ci_owned_runners).to contain_exactly(runner) end end context 'with personal group runner' do - let!(:project) { create(:project, runners: [runner_1]) } + let!(:project) { create(:project) } + let(:group_runner) { create(:ci_runner, :group, groups: [group]) } let!(:group) do - create(:group, runners: [runner_2]).tap do |group| + create(:group).tap do |group| group.add_owner(user) end end it 'loads' do - expect(user.ci_owned_runners).to contain_exactly(runner_2) + expect(user.ci_owned_runners).to contain_exactly(group_runner) end end context 'with personal project and group runner' do let(:namespace) { create(:namespace, owner: user) } - let!(:project) { create(:project, namespace: namespace, runners: [runner_1]) } + let!(:project) { create(:project, namespace: namespace) } + let!(:group_runner) { create(:ci_runner, :group, groups: [group]) } let!(:group) do - create(:group, runners: [runner_2]).tap do |group| + create(:group).tap do |group| group.add_owner(user) end end it 'loads' do - expect(user.ci_owned_runners).to contain_exactly(runner_1, runner_2) + expect(user.ci_owned_runners).to contain_exactly(runner, group_runner) end end @@ -1914,7 +1913,7 @@ describe User do end it 'loads' do - expect(user.ci_owned_runners).to contain_exactly(runner_1) + expect(user.ci_owned_runners).to contain_exactly(runner) end end @@ -1931,7 +1930,7 @@ describe User do context 'with groups projects runners' do let(:group) { create(:group) } - let!(:project) { create(:project, group: group, runners: [runner_1]) } + let!(:project) { create(:project, group: group) } def add_user(access) group.add_user(user, access) @@ -1941,11 +1940,8 @@ describe User do end context 'with groups runners' do - let!(:group) do - create(:group, runners: [runner_1]).tap do |group| - group.add_owner(user) - end - end + let!(:runner) { create(:ci_runner, :group, groups: [group]) } + let!(:group) { create(:group) } def add_user(access) group.add_user(user, access) @@ -1955,7 +1951,7 @@ describe User do end context 'with other projects runners' do - let!(:project) { create(:project, runners: [runner_1]) } + let!(:project) { create(:project) } def add_user(access) project.add_role(user, access) @@ -1968,7 +1964,7 @@ describe User do let(:group) { create(:group) } let(:another_user) { create(:user) } let(:subgroup) { create(:group, parent: group) } - let!(:project) { create(:project, group: subgroup, runners: [runner_1]) } + let!(:project) { create(:project, group: subgroup) } def add_user(access) group.add_user(user, access) |