diff options
Diffstat (limited to 'spec/models')
108 files changed, 2675 insertions, 798 deletions
diff --git a/spec/models/ability_spec.rb b/spec/models/ability_spec.rb index e131661602e..bb8d476f257 100644 --- a/spec/models/ability_spec.rb +++ b/spec/models/ability_spec.rb @@ -425,9 +425,9 @@ RSpec.describe Ability do expect(keys).to include( :administrator, 'admin', - "/dp/condition/BasePolicy/admin/#{user_b.id}" + "/dp/condition/BasePolicy/admin/User:#{user_b.id}" ) - expect(keys).not_to include("/dp/condition/BasePolicy/admin/#{user_a.id}") + expect(keys).not_to include("/dp/condition/BasePolicy/admin/User:#{user_a.id}") end # regression spec for re-entrant admin condition checks diff --git a/spec/models/acts_as_taggable_on/tag_spec.rb b/spec/models/acts_as_taggable_on/tag_spec.rb new file mode 100644 index 00000000000..4b390bbd0bb --- /dev/null +++ b/spec/models/acts_as_taggable_on/tag_spec.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ActsAsTaggableOn::Tag do + it 'has the same connection as Ci::ApplicationRecord' do + query = 'select current_database()' + + expect(described_class.connection.execute(query).first).to eq(Ci::ApplicationRecord.connection.execute(query).first) + expect(described_class.retrieve_connection.execute(query).first).to eq(Ci::ApplicationRecord.retrieve_connection.execute(query).first) + end + + it 'has the same sticking as Ci::ApplicationRecord' do + expect(described_class.sticking).to eq(Ci::ApplicationRecord.sticking) + end +end diff --git a/spec/models/acts_as_taggable_on/tagging_spec.rb b/spec/models/acts_as_taggable_on/tagging_spec.rb new file mode 100644 index 00000000000..4520a0aaf70 --- /dev/null +++ b/spec/models/acts_as_taggable_on/tagging_spec.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ActsAsTaggableOn::Tagging do + it 'has the same connection as Ci::ApplicationRecord' do + query = 'select current_database()' + + expect(described_class.connection.execute(query).first).to eq(Ci::ApplicationRecord.connection.execute(query).first) + expect(described_class.retrieve_connection.execute(query).first).to eq(Ci::ApplicationRecord.retrieve_connection.execute(query).first) + end + + it 'has the same sticking as Ci::ApplicationRecord' do + expect(described_class.sticking).to eq(Ci::ApplicationRecord.sticking) + end +end diff --git a/spec/models/analytics/cycle_analytics/issue_stage_event_spec.rb b/spec/models/analytics/cycle_analytics/issue_stage_event_spec.rb index c0d5b9203b8..ac17271ff99 100644 --- a/spec/models/analytics/cycle_analytics/issue_stage_event_spec.rb +++ b/spec/models/analytics/cycle_analytics/issue_stage_event_spec.rb @@ -9,5 +9,12 @@ RSpec.describe Analytics::CycleAnalytics::IssueStageEvent do it { is_expected.to validate_presence_of(:project_id) } it { is_expected.to validate_presence_of(:start_event_timestamp) } - it_behaves_like 'StageEventModel' + it 'has state enum' do + expect(described_class.states).to eq(Issue.available_states) + end + + it_behaves_like 'StageEventModel' do + let_it_be(:stage_event_factory) { :cycle_analytics_issue_stage_event } + let_it_be(:issuable_factory) { :issue } + end end diff --git a/spec/models/analytics/cycle_analytics/merge_request_stage_event_spec.rb b/spec/models/analytics/cycle_analytics/merge_request_stage_event_spec.rb index 82a7e66d62a..bccc485d3f9 100644 --- a/spec/models/analytics/cycle_analytics/merge_request_stage_event_spec.rb +++ b/spec/models/analytics/cycle_analytics/merge_request_stage_event_spec.rb @@ -9,5 +9,12 @@ RSpec.describe Analytics::CycleAnalytics::MergeRequestStageEvent do it { is_expected.to validate_presence_of(:project_id) } it { is_expected.to validate_presence_of(:start_event_timestamp) } - it_behaves_like 'StageEventModel' + it 'has state enum' do + expect(described_class.states).to eq(MergeRequest.available_states) + end + + it_behaves_like 'StageEventModel' do + let_it_be(:stage_event_factory) { :cycle_analytics_merge_request_stage_event } + let_it_be(:issuable_factory) { :merge_request } + end end diff --git a/spec/models/blob_viewer/package_json_spec.rb b/spec/models/blob_viewer/package_json_spec.rb index 8a394a7334f..1dcba3bcb4f 100644 --- a/spec/models/blob_viewer/package_json_spec.rb +++ b/spec/models/blob_viewer/package_json_spec.rb @@ -27,11 +27,55 @@ RSpec.describe BlobViewer::PackageJson do end end - describe '#package_url' do - it 'returns the package URL' do - expect(subject).to receive(:prepare!) + context 'yarn' do + let(:data) do + <<-SPEC.strip_heredoc + { + "name": "module-name", + "version": "10.3.1", + "engines": { + "yarn": "^2.4.0" + } + } + SPEC + end + + let(:blob) { fake_blob(path: 'package.json', data: data) } + + subject { described_class.new(blob) } + + describe '#package_url' do + it 'returns the package URL', :aggregate_failures do + expect(subject).to receive(:prepare!) + + expect(subject.package_url).to eq("https://yarnpkg.com/package/#{subject.package_name}") + end + end - expect(subject.package_url).to eq("https://www.npmjs.com/package/#{subject.package_name}") + describe '#manager_url' do + it 'returns the manager URL', :aggregate_failures do + expect(subject).to receive(:prepare!) + + expect(subject.manager_url).to eq("https://yarnpkg.com/") + end + end + end + + context 'npm' do + describe '#package_url' do + it 'returns the package URL', :aggregate_failures do + expect(subject).to receive(:prepare!) + + expect(subject.package_url).to eq("https://www.npmjs.com/package/#{subject.package_name}") + end + end + + describe '#manager_url' do + it 'returns the manager URL', :aggregate_failures do + expect(subject).to receive(:prepare!) + + expect(subject.manager_url).to eq("https://www.npmjs.com/") + end end end diff --git a/spec/models/bulk_imports/entity_spec.rb b/spec/models/bulk_imports/entity_spec.rb index 278d7f4bc56..cc66572cd6f 100644 --- a/spec/models/bulk_imports/entity_spec.rb +++ b/spec/models/bulk_imports/entity_spec.rb @@ -243,4 +243,13 @@ RSpec.describe BulkImports::Entity, type: :model do end end end + + describe '#relation_download_url_path' do + it 'returns export relations url with download query string' do + entity = build(:bulk_import_entity) + + expect(entity.relation_download_url_path('test')) + .to eq("/groups/#{entity.encoded_source_full_path}/export_relations/download?relation=test") + end + end end diff --git a/spec/models/bulk_imports/file_transfer/project_config_spec.rb b/spec/models/bulk_imports/file_transfer/project_config_spec.rb index 3bd79333f0c..02151da583e 100644 --- a/spec/models/bulk_imports/file_transfer/project_config_spec.rb +++ b/spec/models/bulk_imports/file_transfer/project_config_spec.rb @@ -80,7 +80,7 @@ RSpec.describe BulkImports::FileTransfer::ProjectConfig do describe '#tree_relation_definition_for' do it 'returns relation definition' do - expected = { service_desk_setting: { except: [:outgoing_name, :file_template_project_id], include: [] } } + expected = { service_desk_setting: { except: [:outgoing_name, :file_template_project_id], include: [], only: %i[project_id issue_template_key project_key] } } expect(subject.tree_relation_definition_for('service_desk_setting')).to eq(expected) end diff --git a/spec/models/chat_name_spec.rb b/spec/models/chat_name_spec.rb index 9ed00003ac1..67e0f98d147 100644 --- a/spec/models/chat_name_spec.rb +++ b/spec/models/chat_name_spec.rb @@ -43,4 +43,12 @@ RSpec.describe ChatName do expect(subject.last_used_at).to eq(time) end end + + it_behaves_like 'it has loose foreign keys' do + let(:factory_name) { :chat_name } + + before do + Ci::PipelineChatData # ensure that the referenced model is loaded + end + end end diff --git a/spec/models/ci/bridge_spec.rb b/spec/models/ci/bridge_spec.rb index 8f1ae9c5f02..6fde55103f8 100644 --- a/spec/models/ci/bridge_spec.rb +++ b/spec/models/ci/bridge_spec.rb @@ -17,8 +17,6 @@ RSpec.describe Ci::Bridge do { trigger: { project: 'my/project', branch: 'master' } } end - it { is_expected.to respond_to(:runner_features) } - it 'has many sourced pipelines' do expect(bridge).to have_many(:sourced_pipelines) end diff --git a/spec/models/ci/build_metadata_spec.rb b/spec/models/ci/build_metadata_spec.rb index 069864fa765..b2ffb34da1d 100644 --- a/spec/models/ci/build_metadata_spec.rb +++ b/spec/models/ci/build_metadata_spec.rb @@ -121,4 +121,16 @@ RSpec.describe Ci::BuildMetadata do end end end + + describe 'set_cancel_gracefully' do + it 'sets cancel_gracefully' do + build.set_cancel_gracefully + + expect(build.cancel_gracefully?).to be true + end + + it 'returns false' do + expect(build.cancel_gracefully?).to be false + end + end end diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 2ebf75a1d8a..b7de8ca4337 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -35,7 +35,8 @@ RSpec.describe Ci::Build do it { is_expected.to respond_to(:has_trace?) } it { is_expected.to respond_to(:trace) } - it { is_expected.to respond_to(:runner_features) } + it { is_expected.to respond_to(:set_cancel_gracefully) } + it { is_expected.to respond_to(:cancel_gracefully?) } it { is_expected.to delegate_method(:merge_request?).to(:pipeline) } it { is_expected.to delegate_method(:merge_request_ref?).to(:pipeline) } @@ -214,6 +215,26 @@ RSpec.describe Ci::Build do end end + describe '.license_management_jobs' do + subject { described_class.license_management_jobs } + + let!(:management_build) { create(:ci_build, :success, name: :license_management) } + let!(:scanning_build) { create(:ci_build, :success, name: :license_scanning) } + let!(:another_build) { create(:ci_build, :success, name: :another_type) } + + it 'returns license_scanning jobs' do + is_expected.to include(scanning_build) + end + + it 'returns license_management jobs' do + is_expected.to include(management_build) + end + + it 'doesnt return filtered out jobs' do + is_expected.not_to include(another_build) + end + end + describe '.finished_before' do subject { described_class.finished_before(date) } @@ -350,7 +371,7 @@ RSpec.describe Ci::Build do it 'sticks the build if the status changed' do job = create(:ci_build, :pending) - expect(ApplicationRecord.sticking).to receive(:stick) + expect(described_class.sticking).to receive(:stick) .with(:build, job.id) job.update!(status: :running) @@ -1290,7 +1311,7 @@ RSpec.describe Ci::Build do end end - shared_examples_for 'state transition as a deployable' do + describe 'state transition as a deployable' do subject { build.send(event) } let!(:build) { create(:ci_build, :with_deployment, :start_review_app, project: project, pipeline: pipeline) } @@ -1332,6 +1353,22 @@ RSpec.describe Ci::Build do expect(deployment).to be_running end + + context 'when deployment is already running state' do + before do + build.deployment.success! + end + + it 'does not change deployment status and tracks an error' do + expect(Gitlab::ErrorTracking) + .to receive(:track_exception).with( + instance_of(Deployment::StatusSyncError), deployment_id: deployment.id, build_id: build.id) + + with_cross_database_modification_prevented do + expect { subject }.not_to change { deployment.reload.status } + end + end + end end context 'when transits to success' do @@ -1399,36 +1436,6 @@ RSpec.describe Ci::Build do end end - it_behaves_like 'state transition as a deployable' do - context 'when transits to running' do - let(:event) { :run! } - - context 'when deployment is already running state' do - before do - build.deployment.success! - end - - it 'does not change deployment status and tracks an error' do - expect(Gitlab::ErrorTracking) - .to receive(:track_exception).with( - instance_of(Deployment::StatusSyncError), deployment_id: deployment.id, build_id: build.id) - - with_cross_database_modification_prevented do - expect { subject }.not_to change { deployment.reload.status } - end - end - end - end - end - - context 'when update_deployment_after_transaction_commit feature flag is disabled' do - before do - stub_feature_flags(update_deployment_after_transaction_commit: false) - end - - it_behaves_like 'state transition as a deployable' - end - describe '#on_stop' do subject { build.on_stop } @@ -2759,7 +2766,10 @@ RSpec.describe Ci::Build do let(:job_dependency_var) { { key: 'job_dependency', value: 'value', public: true, masked: false } } before do - allow(build).to receive(:predefined_variables) { [build_pre_var] } + allow_next_instance_of(Gitlab::Ci::Variables::Builder) do |builder| + allow(builder).to receive(:predefined_variables) { [build_pre_var] } + end + allow(build).to receive(:yaml_variables) { [build_yaml_var] } allow(build).to receive(:persisted_variables) { [] } allow(build).to receive(:job_jwt_variables) { [job_jwt_var] } @@ -3411,75 +3421,122 @@ RSpec.describe Ci::Build do end describe '#scoped_variables' do - context 'when build has not been persisted yet' do - let(:build) do - described_class.new( - name: 'rspec', - stage: 'test', - ref: 'feature', - project: project, - pipeline: pipeline, - scheduling_type: :stage - ) - end + before do + pipeline.clear_memoization(:predefined_vars_in_builder_enabled) + end - let(:pipeline) { create(:ci_pipeline, project: project, ref: 'feature') } + it 'records a prometheus metric' do + histogram = double(:histogram) + expect(::Gitlab::Ci::Pipeline::Metrics).to receive(:pipeline_builder_scoped_variables_histogram) + .and_return(histogram) - it 'does not persist the build' do - expect(build).to be_valid - expect(build).not_to be_persisted + expect(histogram).to receive(:observe) + .with({}, a_kind_of(ActiveSupport::Duration)) - build.scoped_variables + build.scoped_variables + end - expect(build).not_to be_persisted - end + shared_examples 'calculates scoped_variables' do + context 'when build has not been persisted yet' do + let(:build) do + described_class.new( + name: 'rspec', + stage: 'test', + ref: 'feature', + project: project, + pipeline: pipeline, + scheduling_type: :stage + ) + end - it 'returns static predefined variables' do - keys = %w[CI_JOB_NAME - CI_COMMIT_SHA - CI_COMMIT_SHORT_SHA - CI_COMMIT_REF_NAME - CI_COMMIT_REF_SLUG - CI_JOB_STAGE] + let(:pipeline) { create(:ci_pipeline, project: project, ref: 'feature') } - variables = build.scoped_variables + it 'does not persist the build' do + expect(build).to be_valid + expect(build).not_to be_persisted - variables.map { |env| env[:key] }.tap do |names| - expect(names).to include(*keys) + build.scoped_variables + + expect(build).not_to be_persisted end - expect(variables) - .to include(key: 'CI_COMMIT_REF_NAME', value: 'feature', public: true, masked: false) + it 'returns static predefined variables' do + keys = %w[CI_JOB_NAME + CI_COMMIT_SHA + CI_COMMIT_SHORT_SHA + CI_COMMIT_REF_NAME + CI_COMMIT_REF_SLUG + CI_JOB_STAGE] + + variables = build.scoped_variables + + variables.map { |env| env[:key] }.tap do |names| + expect(names).to include(*keys) + end + + expect(variables) + .to include(key: 'CI_COMMIT_REF_NAME', value: 'feature', public: true, masked: false) + end + + it 'does not return prohibited variables' do + keys = %w[CI_JOB_ID + CI_JOB_URL + CI_JOB_TOKEN + CI_BUILD_ID + CI_BUILD_TOKEN + CI_REGISTRY_USER + CI_REGISTRY_PASSWORD + CI_REPOSITORY_URL + CI_ENVIRONMENT_URL + CI_DEPLOY_USER + CI_DEPLOY_PASSWORD] + + build.scoped_variables.map { |env| env[:key] }.tap do |names| + expect(names).not_to include(*keys) + end + end end - it 'does not return prohibited variables' do - keys = %w[CI_JOB_ID - CI_JOB_URL - CI_JOB_TOKEN - CI_BUILD_ID - CI_BUILD_TOKEN - CI_REGISTRY_USER - CI_REGISTRY_PASSWORD - CI_REPOSITORY_URL - CI_ENVIRONMENT_URL - CI_DEPLOY_USER - CI_DEPLOY_PASSWORD] + context 'with dependency variables' do + let!(:prepare) { create(:ci_build, name: 'prepare', pipeline: pipeline, stage_idx: 0) } + let!(:build) { create(:ci_build, pipeline: pipeline, stage_idx: 1, options: { dependencies: ['prepare'] }) } + + let!(:job_variable) { create(:ci_job_variable, :dotenv_source, job: prepare) } - build.scoped_variables.map { |env| env[:key] }.tap do |names| - expect(names).not_to include(*keys) + it 'inherits dependent variables' do + expect(build.scoped_variables.to_hash).to include(job_variable.key => job_variable.value) end end end - context 'with dependency variables' do - let!(:prepare) { create(:ci_build, name: 'prepare', pipeline: pipeline, stage_idx: 0) } - let!(:build) { create(:ci_build, pipeline: pipeline, stage_idx: 1, options: { dependencies: ['prepare'] }) } + it_behaves_like 'calculates scoped_variables' - let!(:job_variable) { create(:ci_job_variable, :dotenv_source, job: prepare) } + it 'delegates to the variable builders' do + expect_next_instance_of(Gitlab::Ci::Variables::Builder) do |builder| + expect(builder) + .to receive(:scoped_variables).with(build, hash_including(:environment, :dependencies)) + .and_call_original - it 'inherits dependent variables' do - expect(build.scoped_variables.to_hash).to include(job_variable.key => job_variable.value) + expect(builder).to receive(:predefined_variables).and_call_original end + + build.scoped_variables + end + + context 'when ci builder feature flag is disabled' do + before do + stub_feature_flags(ci_predefined_vars_in_builder: false) + end + + it 'does not delegate to the variable builders' do + expect_next_instance_of(Gitlab::Ci::Variables::Builder) do |builder| + expect(builder).not_to receive(:predefined_variables) + end + + build.scoped_variables + end + + it_behaves_like 'calculates scoped_variables' end end @@ -3569,6 +3626,27 @@ RSpec.describe Ci::Build do include_examples "secret CI variables" end + describe '#kubernetes_variables' do + let(:build) { create(:ci_build) } + let(:service) { double(execute: template) } + let(:template) { double(to_yaml: 'example-kubeconfig', valid?: template_valid) } + let(:template_valid) { true } + + subject { build.kubernetes_variables } + + before do + allow(Ci::GenerateKubeconfigService).to receive(:new).with(build).and_return(service) + end + + it { is_expected.to include(key: 'KUBECONFIG', value: 'example-kubeconfig', public: false, file: true) } + + context 'generated config is invalid' do + let(:template_valid) { false } + + it { is_expected.not_to include(key: 'KUBECONFIG', value: 'example-kubeconfig', public: false, file: true) } + end + end + describe '#deployment_variables' do let(:build) { create(:ci_build, environment: environment) } let(:environment) { 'production' } @@ -3728,7 +3806,7 @@ RSpec.describe Ci::Build do it 'ensures that it is not run in database transaction' do expect(job.pipeline.persistent_ref).to receive(:create) do - expect(Gitlab::Database.main).not_to be_inside_transaction + expect(ApplicationRecord).not_to be_inside_transaction end run_job_without_exception @@ -5326,4 +5404,23 @@ RSpec.describe Ci::Build do create(:ci_build) end end + + describe '#runner_features' do + subject do + build.save! + build.cancel_gracefully? + end + + let_it_be(:build) { create(:ci_build, pipeline: pipeline) } + + it 'cannot cancel gracefully' do + expect(subject).to be false + end + + it 'can cancel gracefully' do + build.set_cancel_gracefully + + expect(subject).to be true + end + end end diff --git a/spec/models/ci/job_artifact_spec.rb b/spec/models/ci/job_artifact_spec.rb index a94a1dd284a..d63f87e8943 100644 --- a/spec/models/ci/job_artifact_spec.rb +++ b/spec/models/ci/job_artifact_spec.rb @@ -351,6 +351,21 @@ RSpec.describe Ci::JobArtifact do end end + context 'when updating any field except the file' do + let(:artifact) { create(:ci_job_artifact, :unarchived_trace_artifact, file_store: 2) } + + before do + stub_artifacts_object_storage(direct_upload: true) + artifact.file.object_store = 1 + end + + it 'the `after_commit` hook does not update `file_store`' do + artifact.update!(expire_at: Time.current) + + expect(artifact.file_store).to be(2) + end + end + describe 'validates file format' do subject { artifact } @@ -507,6 +522,53 @@ RSpec.describe Ci::JobArtifact do end end + describe '#store_after_commit?' do + let(:file_type) { :archive } + let(:artifact) { build(:ci_job_artifact, file_type) } + + context 'when direct upload is enabled' do + before do + stub_artifacts_object_storage(direct_upload: true) + end + + context 'when the artifact is a trace' do + let(:file_type) { :trace } + + context 'when ci_store_trace_outside_transaction is enabled' do + it 'returns true' do + expect(artifact.store_after_commit?).to be_truthy + end + end + + context 'when ci_store_trace_outside_transaction is disabled' do + before do + stub_feature_flags(ci_store_trace_outside_transaction: false) + end + + it 'returns false' do + expect(artifact.store_after_commit?).to be_falsey + end + end + end + + context 'when the artifact is not a trace' do + it 'returns false' do + expect(artifact.store_after_commit?).to be_falsey + end + end + end + + context 'when direct upload is disabled' do + before do + stub_artifacts_object_storage(direct_upload: false) + end + + it 'returns false' do + expect(artifact.store_after_commit?).to be_falsey + end + end + end + describe 'file is being stored' do subject { create(:ci_job_artifact, :archive) } diff --git a/spec/models/ci/pipeline_schedule_spec.rb b/spec/models/ci/pipeline_schedule_spec.rb index c7e1fe91b1e..fee74f8f674 100644 --- a/spec/models/ci/pipeline_schedule_spec.rb +++ b/spec/models/ci/pipeline_schedule_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' RSpec.describe Ci::PipelineSchedule do - let_it_be(:project) { create_default(:project) } + let_it_be_with_reload(:project) { create_default(:project) } subject { build(:ci_pipeline_schedule) } diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 5f3aad0ab24..e573a6ef780 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -3361,7 +3361,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do shared_examples 'sending a notification' do it 'sends an email', :sidekiq_might_not_need_inline do - should_only_email(pipeline.user, kind: :bcc) + should_only_email(pipeline.user) end end @@ -4595,4 +4595,20 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end end end + + describe '#authorized_cluster_agents' do + let(:pipeline) { create(:ci_empty_pipeline, :created) } + let(:agent) { instance_double(Clusters::Agent) } + let(:authorization) { instance_double(Clusters::Agents::GroupAuthorization, agent: agent) } + let(:finder) { double(execute: [authorization]) } + + it 'retrieves agent records from the finder and caches the result' do + expect(Clusters::AgentAuthorizationsFinder).to receive(:new).once + .with(pipeline.project) + .and_return(finder) + + expect(pipeline.authorized_cluster_agents).to contain_exactly(agent) + expect(pipeline.authorized_cluster_agents).to contain_exactly(agent) # cached + end + end end diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index 826332268c5..2e79159cc60 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -5,6 +5,14 @@ require 'spec_helper' RSpec.describe Ci::Runner do it_behaves_like 'having unique enum values' + it_behaves_like 'it has loose foreign keys' do + let(:factory_name) { :ci_runner } + + before do + Clusters::Applications::Runner # ensure that the referenced model is loaded + end + end + describe 'groups association' do # Due to other assoctions such as projects this whole spec is allowed to # generate cross-database queries. So we have this temporary spec to @@ -44,7 +52,7 @@ RSpec.describe Ci::Runner do let(:runner) { create(:ci_runner, :group, groups: [group]) } it 'disallows assigning group if already assigned to a group' do - runner.groups << build(:group) + runner.runner_namespaces << build(:ci_runner_namespace) expect(runner).not_to be_valid expect(runner.errors.full_messages).to include('Runner needs to be assigned to exactly one group') @@ -397,7 +405,7 @@ RSpec.describe Ci::Runner do it 'sticks the runner to the primary and calls the original method' do runner = create(:ci_runner) - expect(ApplicationRecord.sticking).to receive(:stick) + expect(described_class.sticking).to receive(:stick) .with(:runner, runner.id) expect(Gitlab::Workhorse).to receive(:set_key_and_notify) @@ -618,7 +626,7 @@ RSpec.describe Ci::Runner do end describe '#status' do - let(:runner) { create(:ci_runner, :instance, contacted_at: 1.second.ago) } + let(:runner) { build(:ci_runner, :instance) } subject { runner.status } @@ -630,6 +638,45 @@ RSpec.describe Ci::Runner do it { is_expected.to eq(:not_connected) } end + context 'inactive but online' do + before do + runner.contacted_at = 1.second.ago + runner.active = false + end + + it { is_expected.to eq(:online) } + end + + context 'contacted 1s ago' do + before do + runner.contacted_at = 1.second.ago + end + + it { is_expected.to eq(:online) } + end + + context 'contacted long time ago' do + before do + runner.contacted_at = 1.year.ago + end + + it { is_expected.to eq(:offline) } + end + end + + describe '#deprecated_rest_status' do + let(:runner) { build(:ci_runner, :instance, contacted_at: 1.second.ago) } + + subject { runner.deprecated_rest_status } + + context 'never connected' do + before do + runner.contacted_at = nil + end + + it { is_expected.to eq(:not_connected) } + end + context 'contacted 1s ago' do before do runner.contacted_at = 1.second.ago diff --git a/spec/models/ci/trigger_spec.rb b/spec/models/ci/trigger_spec.rb index 4ba6c6e50f7..c254279a32f 100644 --- a/spec/models/ci/trigger_spec.rb +++ b/spec/models/ci/trigger_spec.rb @@ -57,4 +57,8 @@ RSpec.describe Ci::Trigger do it { is_expected.to eq(false) } end end + + it_behaves_like 'includes Limitable concern' do + subject { build(:ci_trigger, owner: project.owner, project: project) } + end end diff --git a/spec/models/clusters/applications/runner_spec.rb b/spec/models/clusters/applications/runner_spec.rb index 788430d53d3..806c60d5aff 100644 --- a/spec/models/clusters/applications/runner_spec.rb +++ b/spec/models/clusters/applications/runner_spec.rb @@ -112,7 +112,7 @@ RSpec.describe Clusters::Applications::Runner do subject expect(runner).to be_group_type - expect(runner.groups).to eq [group] + expect(runner.runner_namespaces.pluck(:namespace_id)).to match_array [group.id] end end @@ -162,12 +162,12 @@ RSpec.describe Clusters::Applications::Runner do it 'pauses associated runner' do active_runner = create(:ci_runner, contacted_at: 1.second.ago) - expect(active_runner.status).to eq(:online) + expect(active_runner.active).to be_truthy application_runner = create(:clusters_applications_runner, :scheduled, runner: active_runner) application_runner.prepare_uninstall - expect(active_runner.status).to eq(:paused) + expect(active_runner.active).to be_falsey end end diff --git a/spec/models/clusters/cluster_spec.rb b/spec/models/clusters/cluster_spec.rb index 9d305e31bad..d61bed80aaa 100644 --- a/spec/models/clusters/cluster_spec.rb +++ b/spec/models/clusters/cluster_spec.rb @@ -178,13 +178,13 @@ RSpec.describe Clusters::Cluster, :use_clean_rails_memory_store_caching do end end - describe '.with_application_prometheus' do - subject { described_class.with_application_prometheus } + describe '.with_integration_prometheus' do + subject { described_class.with_integration_prometheus } let!(:cluster) { create(:cluster) } context 'cluster has prometheus application' do - let!(:application) { create(:clusters_applications_prometheus, :installed, cluster: cluster) } + let!(:application) { create(:clusters_integrations_prometheus, cluster: cluster) } it { is_expected.to include(cluster) } end diff --git a/spec/models/commit_status_spec.rb b/spec/models/commit_status_spec.rb index 20afddd8470..59d14574c02 100644 --- a/spec/models/commit_status_spec.rb +++ b/spec/models/commit_status_spec.rb @@ -379,6 +379,22 @@ RSpec.describe CommitStatus do end end + describe '.retried_ordered' do + subject { described_class.retried_ordered.to_a } + + let!(:statuses) do + [create_status(name: 'aa', ref: 'bb', status: 'running', retried: true), + create_status(name: 'cc', ref: 'cc', status: 'pending', retried: true), + create_status(name: 'aa', ref: 'cc', status: 'success', retried: true), + create_status(name: 'cc', ref: 'bb', status: 'success'), + create_status(name: 'aa', ref: 'bb', status: 'success')] + end + + it 'returns retried statuses in order' do + is_expected.to eq(statuses.values_at(2, 0, 1)) + end + end + describe '.running_or_pending' do subject { described_class.running_or_pending.order(:id) } diff --git a/spec/models/concerns/bulk_insert_safe_spec.rb b/spec/models/concerns/bulk_insert_safe_spec.rb index 172986c142c..e6b197f34ca 100644 --- a/spec/models/concerns/bulk_insert_safe_spec.rb +++ b/spec/models/concerns/bulk_insert_safe_spec.rb @@ -5,42 +5,42 @@ require 'spec_helper' RSpec.describe BulkInsertSafe do before(:all) do ActiveRecord::Schema.define do - create_table :bulk_insert_parent_items, force: true do |t| + create_table :_test_bulk_insert_parent_items, force: true do |t| t.string :name, null: false end - create_table :bulk_insert_items, force: true do |t| + create_table :_test_bulk_insert_items, force: true do |t| t.string :name, null: true t.integer :enum_value, null: false t.text :encrypted_secret_value, null: false t.string :encrypted_secret_value_iv, null: false t.binary :sha_value, null: false, limit: 20 t.jsonb :jsonb_value, null: false - t.belongs_to :bulk_insert_parent_item, foreign_key: true, null: true + t.belongs_to :bulk_insert_parent_item, foreign_key: { to_table: :_test_bulk_insert_parent_items }, null: true t.timestamps null: true t.index :name, unique: true end - create_table :bulk_insert_items_with_composite_pk, id: false, force: true do |t| + create_table :_test_bulk_insert_items_with_composite_pk, id: false, force: true do |t| t.integer :id, null: true t.string :name, null: true end - execute("ALTER TABLE bulk_insert_items_with_composite_pk ADD PRIMARY KEY (id,name);") + execute("ALTER TABLE _test_bulk_insert_items_with_composite_pk ADD PRIMARY KEY (id,name);") end end after(:all) do ActiveRecord::Schema.define do - drop_table :bulk_insert_items, force: true - drop_table :bulk_insert_parent_items, force: true - drop_table :bulk_insert_items_with_composite_pk, force: true + drop_table :_test_bulk_insert_items, force: true + drop_table :_test_bulk_insert_parent_items, force: true + drop_table :_test_bulk_insert_items_with_composite_pk, force: true end end BulkInsertParentItem = Class.new(ActiveRecord::Base) do - self.table_name = :bulk_insert_parent_items + self.table_name = :_test_bulk_insert_parent_items self.inheritance_column = :_type_disabled def self.name @@ -54,7 +54,7 @@ RSpec.describe BulkInsertSafe do let_it_be(:bulk_insert_item_class) do Class.new(ActiveRecord::Base) do - self.table_name = 'bulk_insert_items' + self.table_name = '_test_bulk_insert_items' include BulkInsertSafe include ShaAttribute @@ -182,7 +182,7 @@ RSpec.describe BulkInsertSafe do context 'with returns option set' do let(:items) { bulk_insert_item_class.valid_list(1) } - subject(:bulk_insert) { bulk_insert_item_class.bulk_insert!(items, returns: returns) } + subject(:legacy_bulk_insert) { bulk_insert_item_class.bulk_insert!(items, returns: returns) } context 'when is set to :ids' do let(:returns) { :ids } @@ -247,7 +247,7 @@ RSpec.describe BulkInsertSafe do context 'when a model with composite primary key is inserted' do let_it_be(:bulk_insert_items_with_composite_pk_class) do Class.new(ActiveRecord::Base) do - self.table_name = 'bulk_insert_items_with_composite_pk' + self.table_name = '_test_bulk_insert_items_with_composite_pk' include BulkInsertSafe end diff --git a/spec/models/concerns/bulk_insertable_associations_spec.rb b/spec/models/concerns/bulk_insertable_associations_spec.rb index 25b13c8233d..9713f1ce9a4 100644 --- a/spec/models/concerns/bulk_insertable_associations_spec.rb +++ b/spec/models/concerns/bulk_insertable_associations_spec.rb @@ -6,42 +6,50 @@ RSpec.describe BulkInsertableAssociations do class BulkFoo < ApplicationRecord include BulkInsertSafe + self.table_name = '_test_bulk_foos' + validates :name, presence: true end class BulkBar < ApplicationRecord include BulkInsertSafe + + self.table_name = '_test_bulk_bars' end - SimpleBar = Class.new(ApplicationRecord) + SimpleBar = Class.new(ApplicationRecord) do + self.table_name = '_test_simple_bars' + end class BulkParent < ApplicationRecord include BulkInsertableAssociations - has_many :bulk_foos + self.table_name = '_test_bulk_parents' + + has_many :bulk_foos, class_name: 'BulkFoo' has_many :bulk_hunks, class_name: 'BulkFoo' - has_many :bulk_bars - has_many :simple_bars # not `BulkInsertSafe` + has_many :bulk_bars, class_name: 'BulkBar' + has_many :simple_bars, class_name: 'SimpleBar' # not `BulkInsertSafe` has_one :bulk_foo # not supported end before(:all) do ActiveRecord::Schema.define do - create_table :bulk_parents, force: true do |t| + create_table :_test_bulk_parents, force: true do |t| t.string :name, null: true end - create_table :bulk_foos, force: true do |t| + create_table :_test_bulk_foos, force: true do |t| t.string :name, null: true t.belongs_to :bulk_parent, null: false end - create_table :bulk_bars, force: true do |t| + create_table :_test_bulk_bars, force: true do |t| t.string :name, null: true t.belongs_to :bulk_parent, null: false end - create_table :simple_bars, force: true do |t| + create_table :_test_simple_bars, force: true do |t| t.string :name, null: true t.belongs_to :bulk_parent, null: false end @@ -50,10 +58,10 @@ RSpec.describe BulkInsertableAssociations do after(:all) do ActiveRecord::Schema.define do - drop_table :bulk_foos, force: true - drop_table :bulk_bars, force: true - drop_table :simple_bars, force: true - drop_table :bulk_parents, force: true + drop_table :_test_bulk_foos, force: true + drop_table :_test_bulk_bars, force: true + drop_table :_test_simple_bars, force: true + drop_table :_test_bulk_parents, force: true end end diff --git a/spec/models/concerns/cascading_namespace_setting_attribute_spec.rb b/spec/models/concerns/cascading_namespace_setting_attribute_spec.rb index e8f2b18e662..6be6e3f048f 100644 --- a/spec/models/concerns/cascading_namespace_setting_attribute_spec.rb +++ b/spec/models/concerns/cascading_namespace_setting_attribute_spec.rb @@ -136,6 +136,21 @@ RSpec.describe NamespaceSetting, 'CascadingNamespaceSettingAttribute' do .to raise_error(ActiveRecord::RecordInvalid, /Delayed project removal cannot be changed because it is locked by an ancestor/) end end + + context 'when parent locked the attribute then the application settings locks it' do + before do + subgroup_settings.update!(delayed_project_removal: true) + group_settings.update!(lock_delayed_project_removal: true, delayed_project_removal: false) + stub_application_setting(lock_delayed_project_removal: true, delayed_project_removal: true) + + subgroup_settings.clear_memoization(:delayed_project_removal) + subgroup_settings.clear_memoization(:delayed_project_removal_locked_ancestor) + end + + it 'returns the application setting value' do + expect(delayed_project_removal).to eq(true) + end + end end describe '#delayed_project_removal?' do diff --git a/spec/models/concerns/clusters/agents/authorization_config_scopes_spec.rb b/spec/models/concerns/clusters/agents/authorization_config_scopes_spec.rb new file mode 100644 index 00000000000..a4d1a33b3d5 --- /dev/null +++ b/spec/models/concerns/clusters/agents/authorization_config_scopes_spec.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Clusters::Agents::AuthorizationConfigScopes do + describe '.with_available_ci_access_fields' do + let(:project) { create(:project) } + + let!(:agent_authorization_0) { create(:agent_project_authorization, project: project) } + let!(:agent_authorization_1) { create(:agent_project_authorization, project: project, config: { access_as: {} }) } + let!(:agent_authorization_2) { create(:agent_project_authorization, project: project, config: { access_as: { agent: {} } }) } + let!(:impersonate_authorization) { create(:agent_project_authorization, project: project, config: { access_as: { impersonate: {} } }) } + let!(:ci_user_authorization) { create(:agent_project_authorization, project: project, config: { access_as: { ci_user: {} } }) } + let!(:ci_job_authorization) { create(:agent_project_authorization, project: project, config: { access_as: { ci_job: {} } }) } + let!(:unexpected_authorization) { create(:agent_project_authorization, project: project, config: { access_as: { unexpected: {} } }) } + + subject { Clusters::Agents::ProjectAuthorization.with_available_ci_access_fields(project) } + + it { is_expected.to contain_exactly(agent_authorization_0, agent_authorization_1, agent_authorization_2) } + end +end diff --git a/spec/models/concerns/database_reflection_spec.rb b/spec/models/concerns/database_reflection_spec.rb new file mode 100644 index 00000000000..4111f29ea8d --- /dev/null +++ b/spec/models/concerns/database_reflection_spec.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe DatabaseReflection do + describe '.reflect' do + it 'returns a Reflection instance' do + expect(User.database).to be_an_instance_of(Gitlab::Database::Reflection) + end + + it 'memoizes the result' do + instance1 = User.database + instance2 = User.database + + expect(instance1).to equal(instance2) + end + end +end diff --git a/spec/models/concerns/has_integrations_spec.rb b/spec/models/concerns/has_integrations_spec.rb deleted file mode 100644 index ea6b0e69209..00000000000 --- a/spec/models/concerns/has_integrations_spec.rb +++ /dev/null @@ -1,25 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe HasIntegrations do - let_it_be(:project_1) { create(:project) } - let_it_be(:project_2) { create(:project) } - let_it_be(:project_3) { create(:project) } - let_it_be(:project_4) { create(:project) } - let_it_be(:instance_integration) { create(:jira_integration, :instance) } - - before do - create(:jira_integration, project: project_1, inherit_from_id: instance_integration.id) - create(:jira_integration, project: project_2, inherit_from_id: nil) - create(:jira_integration, group: create(:group), project: nil, inherit_from_id: nil) - create(:jira_integration, project: project_3, inherit_from_id: nil) - create(:integrations_slack, project: project_4, inherit_from_id: nil) - end - - describe '.without_integration' do - it 'returns projects without integration' do - expect(Project.without_integration(instance_integration)).to contain_exactly(project_4) - end - end -end diff --git a/spec/models/concerns/legacy_bulk_insert_spec.rb b/spec/models/concerns/legacy_bulk_insert_spec.rb new file mode 100644 index 00000000000..0c6f84f391b --- /dev/null +++ b/spec/models/concerns/legacy_bulk_insert_spec.rb @@ -0,0 +1,103 @@ +# frozen_string_literal: true + +require 'spec_helper' + +# rubocop: disable Gitlab/BulkInsert +RSpec.describe LegacyBulkInsert do + let(:model) { ApplicationRecord } + + describe '#bulk_insert' do + before do + allow(model).to receive(:connection).and_return(dummy_connection) + allow(dummy_connection).to receive(:quote_column_name, &:itself) + allow(dummy_connection).to receive(:quote, &:itself) + allow(dummy_connection).to receive(:execute) + end + + let(:dummy_connection) { double(:connection) } + + let(:rows) do + [ + { a: 1, b: 2, c: 3 }, + { c: 6, a: 4, b: 5 } + ] + end + + it 'does nothing with empty rows' do + expect(dummy_connection).not_to receive(:execute) + + model.legacy_bulk_insert('test', []) + end + + it 'uses the ordering from the first row' do + expect(dummy_connection).to receive(:execute) do |sql| + expect(sql).to include('(1, 2, 3)') + expect(sql).to include('(4, 5, 6)') + end + + model.legacy_bulk_insert('test', rows) + end + + it 'quotes column names' do + expect(dummy_connection).to receive(:quote_column_name).with(:a) + expect(dummy_connection).to receive(:quote_column_name).with(:b) + expect(dummy_connection).to receive(:quote_column_name).with(:c) + + model.legacy_bulk_insert('test', rows) + end + + it 'quotes values' do + 1.upto(6) do |i| + expect(dummy_connection).to receive(:quote).with(i) + end + + model.legacy_bulk_insert('test', rows) + end + + it 'does not quote values of a column in the disable_quote option' do + [1, 2, 4, 5].each do |i| + expect(dummy_connection).to receive(:quote).with(i) + end + + model.legacy_bulk_insert('test', rows, disable_quote: :c) + end + + it 'does not quote values of columns in the disable_quote option' do + [2, 5].each do |i| + expect(dummy_connection).to receive(:quote).with(i) + end + + model.legacy_bulk_insert('test', rows, disable_quote: [:a, :c]) + end + + it 'handles non-UTF-8 data' do + expect { model.legacy_bulk_insert('test', [{ a: "\255" }]) }.not_to raise_error + end + + context 'when using PostgreSQL' do + it 'allows the returning of the IDs of the inserted rows' do + result = double(:result, values: [['10']]) + + expect(dummy_connection) + .to receive(:execute) + .with(/RETURNING id/) + .and_return(result) + + ids = model + .legacy_bulk_insert('test', [{ number: 10 }], return_ids: true) + + expect(ids).to eq([10]) + end + + it 'allows setting the upsert to do nothing' do + expect(dummy_connection) + .to receive(:execute) + .with(/ON CONFLICT DO NOTHING/) + + model + .legacy_bulk_insert('test', [{ number: 10 }], on_conflict: :do_nothing) + end + end + end +end +# rubocop: enable Gitlab/BulkInsert diff --git a/spec/models/concerns/loaded_in_group_list_spec.rb b/spec/models/concerns/loaded_in_group_list_spec.rb index c37943022ba..d38e842c666 100644 --- a/spec/models/concerns/loaded_in_group_list_spec.rb +++ b/spec/models/concerns/loaded_in_group_list_spec.rb @@ -3,49 +3,67 @@ require 'spec_helper' RSpec.describe LoadedInGroupList do - let(:parent) { create(:group) } + let_it_be(:parent) { create(:group) } + let_it_be(:group) { create(:group, parent: parent) } + let_it_be(:project) { create(:project, namespace: parent) } - subject(:found_group) { Group.with_selects_for_list.find_by(id: parent.id) } + let(:archived_parameter) { nil } - describe '.with_selects_for_list' do - it 'includes the preloaded counts for groups' do - create(:group, parent: parent) - create(:project, namespace: parent) - parent.add_developer(create(:user)) + before do + parent.add_developer(create(:user)) + end - found_group = Group.with_selects_for_list.find_by(id: parent.id) + subject(:found_group) { Group.with_selects_for_list(archived: archived_parameter).find_by(id: parent.id) } + describe '.with_selects_for_list' do + it 'includes the preloaded counts for groups' do expect(found_group.preloaded_project_count).to eq(1) expect(found_group.preloaded_subgroup_count).to eq(1) expect(found_group.preloaded_member_count).to eq(1) end + context 'with project namespaces' do + let_it_be(:group1) { create(:group, parent: parent) } + let_it_be(:group2) { create(:group, parent: parent) } + let_it_be(:project_namespace) { project.project_namespace } + + it 'does not include project_namespaces in the count of subgroups' do + expect(found_group.preloaded_subgroup_count).to eq(3) + expect(parent.subgroup_count).to eq(3) + end + end + context 'with archived projects' do - it 'counts including archived projects when `true` is passed' do - create(:project, namespace: parent, archived: true) - create(:project, namespace: parent) + let_it_be(:archived_project) { create(:project, namespace: parent, archived: true) } - found_group = Group.with_selects_for_list(archived: 'true').find_by(id: parent.id) + let(:archived_parameter) { true } + it 'counts including archived projects when `true` is passed' do expect(found_group.preloaded_project_count).to eq(2) end - it 'counts only archived projects when `only` is passed' do - create_list(:project, 2, namespace: parent, archived: true) - create(:project, namespace: parent) + context 'when not counting archived projects' do + let(:archived_parameter) { false } + + it 'counts projects without archived ones' do + expect(found_group.preloaded_project_count).to eq(1) + end + end + + context 'with archived only' do + let_it_be(:archived_project2) { create(:project, namespace: parent, archived: true) } - found_group = Group.with_selects_for_list(archived: 'only').find_by(id: parent.id) + let(:archived_parameter) { 'only' } - expect(found_group.preloaded_project_count).to eq(2) + it 'counts only archived projects when `only` is passed' do + expect(found_group.preloaded_project_count).to eq(2) + end end end end describe '#children_count' do it 'counts groups and projects' do - create(:group, parent: parent) - create(:project, namespace: parent) - expect(found_group.children_count).to eq(2) end end diff --git a/spec/models/concerns/loose_foreign_key_spec.rb b/spec/models/concerns/loose_foreign_key_spec.rb index ce5e33261a9..42da69eb75e 100644 --- a/spec/models/concerns/loose_foreign_key_spec.rb +++ b/spec/models/concerns/loose_foreign_key_spec.rb @@ -9,8 +9,8 @@ RSpec.describe LooseForeignKey do self.table_name = 'projects' - loose_foreign_key :issues, :project_id, on_delete: :async_delete, gitlab_schema: :gitlab_main - loose_foreign_key 'merge_requests', 'project_id', 'on_delete' => 'async_nullify', 'gitlab_schema' => :gitlab_main + loose_foreign_key :issues, :project_id, on_delete: :async_delete + loose_foreign_key 'merge_requests', 'project_id', 'on_delete' => 'async_nullify' end end @@ -28,7 +28,6 @@ RSpec.describe LooseForeignKey do expect(definition.to_table).to eq('merge_requests') expect(definition.column).to eq('project_id') expect(definition.on_delete).to eq(:async_nullify) - expect(definition.options[:gitlab_schema]).to eq(:gitlab_main) end context 'validation' do @@ -39,9 +38,9 @@ RSpec.describe LooseForeignKey do self.table_name = 'projects' - loose_foreign_key :issues, :project_id, on_delete: :async_delete, gitlab_schema: :gitlab_main - loose_foreign_key :merge_requests, :project_id, on_delete: :async_nullify, gitlab_schema: :gitlab_main - loose_foreign_key :merge_requests, :project_id, on_delete: :destroy, gitlab_schema: :gitlab_main + loose_foreign_key :issues, :project_id, on_delete: :async_delete + loose_foreign_key :merge_requests, :project_id, on_delete: :async_nullify + loose_foreign_key :merge_requests, :project_id, on_delete: :destroy end end @@ -50,28 +49,12 @@ RSpec.describe LooseForeignKey do end end - context 'gitlab_schema validation' do - let(:invalid_class) do - Class.new(ApplicationRecord) do - include LooseForeignKey - - self.table_name = 'projects' - - loose_foreign_key :merge_requests, :project_id, on_delete: :async_nullify, gitlab_schema: :unknown - end - end - - it 'raises error when invalid `gitlab_schema` option was given' do - expect { invalid_class }.to raise_error /Invalid gitlab_schema option given: unknown/ - end - end - context 'inheritance validation' do let(:inherited_project_class) do Class.new(Project) do include LooseForeignKey - loose_foreign_key :issues, :project_id, on_delete: :async_delete, gitlab_schema: :gitlab_main + loose_foreign_key :issues, :project_id, on_delete: :async_delete end end diff --git a/spec/models/concerns/noteable_spec.rb b/spec/models/concerns/noteable_spec.rb index 38766d8decd..81ae30b7116 100644 --- a/spec/models/concerns/noteable_spec.rb +++ b/spec/models/concerns/noteable_spec.rb @@ -77,6 +77,70 @@ RSpec.describe Noteable do end end + describe '#discussion_root_note_ids' do + let!(:label_event) { create(:resource_label_event, merge_request: subject) } + let!(:system_note) { create(:system_note, project: project, noteable: subject) } + let!(:milestone_event) { create(:resource_milestone_event, merge_request: subject) } + let!(:state_event) { create(:resource_state_event, merge_request: subject) } + + it 'returns ordered discussion_ids and synthetic note ids' do + discussions = subject.discussion_root_note_ids(notes_filter: UserPreference::NOTES_FILTERS[:all_notes]).map do |n| + { table_name: n.table_name, discussion_id: n.discussion_id, id: n.id } + end + + expect(discussions).to match([ + a_hash_including(table_name: 'notes', discussion_id: active_diff_note1.discussion_id), + a_hash_including(table_name: 'notes', discussion_id: active_diff_note3.discussion_id), + a_hash_including(table_name: 'notes', discussion_id: outdated_diff_note1.discussion_id), + a_hash_including(table_name: 'notes', discussion_id: discussion_note1.discussion_id), + a_hash_including(table_name: 'notes', discussion_id: commit_diff_note1.discussion_id), + a_hash_including(table_name: 'notes', discussion_id: commit_note1.discussion_id), + a_hash_including(table_name: 'notes', discussion_id: commit_note2.discussion_id), + a_hash_including(table_name: 'notes', discussion_id: commit_discussion_note1.discussion_id), + a_hash_including(table_name: 'notes', discussion_id: commit_discussion_note3.discussion_id), + a_hash_including(table_name: 'notes', discussion_id: note1.discussion_id), + a_hash_including(table_name: 'notes', discussion_id: note2.discussion_id), + a_hash_including(table_name: 'resource_label_events', id: label_event.id), + a_hash_including(table_name: 'notes', discussion_id: system_note.discussion_id), + a_hash_including(table_name: 'resource_milestone_events', id: milestone_event.id), + a_hash_including(table_name: 'resource_state_events', id: state_event.id) + ]) + end + + it 'filters by comments only' do + discussions = subject.discussion_root_note_ids(notes_filter: UserPreference::NOTES_FILTERS[:only_comments]).map do |n| + { table_name: n.table_name, discussion_id: n.discussion_id, id: n.id } + end + + expect(discussions).to match([ + a_hash_including(table_name: 'notes', discussion_id: active_diff_note1.discussion_id), + a_hash_including(table_name: 'notes', discussion_id: active_diff_note3.discussion_id), + a_hash_including(table_name: 'notes', discussion_id: outdated_diff_note1.discussion_id), + a_hash_including(table_name: 'notes', discussion_id: discussion_note1.discussion_id), + a_hash_including(table_name: 'notes', discussion_id: commit_diff_note1.discussion_id), + a_hash_including(table_name: 'notes', discussion_id: commit_note1.discussion_id), + a_hash_including(table_name: 'notes', discussion_id: commit_note2.discussion_id), + a_hash_including(table_name: 'notes', discussion_id: commit_discussion_note1.discussion_id), + a_hash_including(table_name: 'notes', discussion_id: commit_discussion_note3.discussion_id), + a_hash_including(table_name: 'notes', discussion_id: note1.discussion_id), + a_hash_including(table_name: 'notes', discussion_id: note2.discussion_id) + ]) + end + + it 'filters by system notes only' do + discussions = subject.discussion_root_note_ids(notes_filter: UserPreference::NOTES_FILTERS[:only_activity]).map do |n| + { table_name: n.table_name, discussion_id: n.discussion_id, id: n.id } + end + + expect(discussions).to match([ + a_hash_including(table_name: 'resource_label_events', id: label_event.id), + a_hash_including(table_name: 'notes', discussion_id: system_note.discussion_id), + a_hash_including(table_name: 'resource_milestone_events', id: milestone_event.id), + a_hash_including(table_name: 'resource_state_events', id: state_event.id) + ]) + end + end + describe '#grouped_diff_discussions' do let(:grouped_diff_discussions) { subject.grouped_diff_discussions } diff --git a/spec/models/concerns/prometheus_adapter_spec.rb b/spec/models/concerns/prometheus_adapter_spec.rb index 01c987a1d92..4158e8a0a4c 100644 --- a/spec/models/concerns/prometheus_adapter_spec.rb +++ b/spec/models/concerns/prometheus_adapter_spec.rb @@ -165,6 +165,14 @@ RSpec.describe PrometheusAdapter, :use_clean_rails_memory_store_caching do it { is_expected.to eq(success: false, result: %(#{status} - "QUERY FAILED!")) } end end + + context "when client raises Gitlab::PrometheusClient::ConnectionError" do + before do + stub_any_prometheus_request.to_raise(Gitlab::PrometheusClient::ConnectionError) + end + + it { is_expected.to include(success: false, result: kind_of(String)) } + end end describe '#build_query_args' do diff --git a/spec/models/concerns/reactive_caching_spec.rb b/spec/models/concerns/reactive_caching_spec.rb index 7e031bdd263..4f3b95e43cd 100644 --- a/spec/models/concerns/reactive_caching_spec.rb +++ b/spec/models/concerns/reactive_caching_spec.rb @@ -375,7 +375,7 @@ RSpec.describe ReactiveCaching, :use_clean_rails_memory_store_caching do end describe 'classes including this concern' do - it 'sets reactive_cache_work_type' do + it 'sets reactive_cache_work_type', :eager_load do classes = ObjectSpace.each_object(Class).select do |klass| klass < described_class && klass.name end diff --git a/spec/models/concerns/sha256_attribute_spec.rb b/spec/models/concerns/sha256_attribute_spec.rb index c247865d77f..02947325bf4 100644 --- a/spec/models/concerns/sha256_attribute_spec.rb +++ b/spec/models/concerns/sha256_attribute_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' RSpec.describe Sha256Attribute do - let(:model) { Class.new { include Sha256Attribute } } + let(:model) { Class.new(ApplicationRecord) { include Sha256Attribute } } before do columns = [ diff --git a/spec/models/concerns/sha_attribute_spec.rb b/spec/models/concerns/sha_attribute_spec.rb index 3846dd9c231..220eadfab92 100644 --- a/spec/models/concerns/sha_attribute_spec.rb +++ b/spec/models/concerns/sha_attribute_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' RSpec.describe ShaAttribute do - let(:model) { Class.new { include ShaAttribute } } + let(:model) { Class.new(ApplicationRecord) { include ShaAttribute } } before do columns = [ diff --git a/spec/models/concerns/where_composite_spec.rb b/spec/models/concerns/where_composite_spec.rb index 5e67f2f5b65..6abdd12aac5 100644 --- a/spec/models/concerns/where_composite_spec.rb +++ b/spec/models/concerns/where_composite_spec.rb @@ -4,7 +4,7 @@ require 'spec_helper' RSpec.describe WhereComposite do describe '.where_composite' do - let_it_be(:test_table_name) { "test_table_#{SecureRandom.hex(10)}" } + let_it_be(:test_table_name) { "_test_table_#{SecureRandom.hex(10)}" } let(:model) do tbl_name = test_table_name diff --git a/spec/models/concerns/x509_serial_number_attribute_spec.rb b/spec/models/concerns/x509_serial_number_attribute_spec.rb index 88550823748..723e2ad07b6 100644 --- a/spec/models/concerns/x509_serial_number_attribute_spec.rb +++ b/spec/models/concerns/x509_serial_number_attribute_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' RSpec.describe X509SerialNumberAttribute do - let(:model) { Class.new { include X509SerialNumberAttribute } } + let(:model) { Class.new(ApplicationRecord) { include X509SerialNumberAttribute } } before do columns = [ diff --git a/spec/models/custom_emoji_spec.rb b/spec/models/custom_emoji_spec.rb index 4a8b671bab7..01252a58681 100644 --- a/spec/models/custom_emoji_spec.rb +++ b/spec/models/custom_emoji_spec.rb @@ -14,7 +14,7 @@ RSpec.describe CustomEmoji do end describe 'exclusion of duplicated emoji' do - let(:emoji_name) { Gitlab::Emoji.emojis_names.sample } + let(:emoji_name) { TanukiEmoji.index.all.sample.name } let(:group) { create(:group, :private) } it 'disallows emoji names of built-in emoji' do diff --git a/spec/models/customer_relations/contact_spec.rb b/spec/models/customer_relations/contact_spec.rb index 298d5db3ab9..3a2d4e2d0ca 100644 --- a/spec/models/customer_relations/contact_spec.rb +++ b/spec/models/customer_relations/contact_spec.rb @@ -6,7 +6,8 @@ RSpec.describe CustomerRelations::Contact, type: :model do describe 'associations' do it { is_expected.to belong_to(:group) } it { is_expected.to belong_to(:organization).optional } - it { is_expected.to have_and_belong_to_many(:issues) } + it { is_expected.to have_many(:issue_contacts) } + it { is_expected.to have_many(:issues) } end describe 'validations' do diff --git a/spec/models/customer_relations/issue_contact_spec.rb b/spec/models/customer_relations/issue_contact_spec.rb new file mode 100644 index 00000000000..3747d159833 --- /dev/null +++ b/spec/models/customer_relations/issue_contact_spec.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe CustomerRelations::IssueContact do + let_it_be(:issue_contact, reload: true) { create(:issue_customer_relations_contact) } + + subject { issue_contact } + + it { expect(subject).to be_valid } + + describe 'associations' do + it { is_expected.to belong_to(:issue).required } + it { is_expected.to belong_to(:contact).required } + end + + describe 'factory' do + let(:built) { build(:issue_customer_relations_contact) } + let(:stubbed) { build_stubbed(:issue_customer_relations_contact) } + let(:created) { create(:issue_customer_relations_contact) } + + let(:group) { build(:group) } + let(:project) { build(:project, group: group) } + let(:issue) { build(:issue, project: project) } + let(:contact) { build(:contact, group: group) } + let(:for_issue) { build(:issue_customer_relations_contact, :for_issue, issue: issue) } + let(:for_contact) { build(:issue_customer_relations_contact, :for_contact, contact: contact) } + + it 'uses objects from the same group', :aggregate_failures do + expect(stubbed.contact.group).to eq(stubbed.issue.project.group) + expect(built.contact.group).to eq(built.issue.project.group) + expect(created.contact.group).to eq(created.issue.project.group) + end + + it 'builds using the same group', :aggregate_failures do + expect(for_issue.contact.group).to eq(group) + expect(for_contact.issue.project.group).to eq(group) + end + end + + describe 'validation' do + let(:built) { build(:issue_customer_relations_contact, issue: create(:issue), contact: create(:contact)) } + + it 'fails when the contact group does not match the issue group' do + expect(built).not_to be_valid + end + end +end diff --git a/spec/models/data_list_spec.rb b/spec/models/data_list_spec.rb new file mode 100644 index 00000000000..d2f15386808 --- /dev/null +++ b/spec/models/data_list_spec.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe DataList do + describe '#to_array' do + let(:jira_integration) { create(:jira_integration) } + let(:zentao_integration) { create(:zentao_integration) } + let(:cases) do + [ + [jira_integration, 'Integrations::JiraTrackerData', 'service_id'], + [zentao_integration, 'Integrations::ZentaoTrackerData', 'integration_id'] + ] + end + + def data_list(integration) + DataList.new([integration], integration.to_data_fields_hash, integration.data_fields.class).to_array + end + + it 'returns current data' do + cases.each do |integration, data_fields_class_name, foreign_key| + data_fields_klass, columns, values_items = data_list(integration) + + expect(data_fields_klass.to_s).to eq data_fields_class_name + expect(columns.last).to eq foreign_key + values = values_items.first + expect(values.last).to eq integration.id + end + end + end +end diff --git a/spec/models/dependency_proxy/manifest_spec.rb b/spec/models/dependency_proxy/manifest_spec.rb index e7f0889345a..59415096989 100644 --- a/spec/models/dependency_proxy/manifest_spec.rb +++ b/spec/models/dependency_proxy/manifest_spec.rb @@ -15,6 +15,17 @@ RSpec.describe DependencyProxy::Manifest, type: :model do it { is_expected.to validate_presence_of(:digest) } end + describe 'scopes' do + let_it_be(:manifest_one) { create(:dependency_proxy_manifest) } + let_it_be(:manifest_two) { create(:dependency_proxy_manifest) } + let_it_be(:manifests) { [manifest_one, manifest_two] } + let_it_be(:ids) { manifests.map(&:id) } + + it 'order_id_desc' do + expect(described_class.where(id: ids).order_id_desc.to_a).to eq [manifest_two, manifest_one] + end + end + describe 'file is being stored' do subject { create(:dependency_proxy_manifest) } @@ -31,18 +42,14 @@ RSpec.describe DependencyProxy::Manifest, type: :model do end end - describe '.find_or_initialize_by_file_name_or_digest' do + describe '.find_by_file_name_or_digest' do let_it_be(:file_name) { 'foo' } let_it_be(:digest) { 'bar' } - subject { DependencyProxy::Manifest.find_or_initialize_by_file_name_or_digest(file_name: file_name, digest: digest) } + subject { DependencyProxy::Manifest.find_by_file_name_or_digest(file_name: file_name, digest: digest) } context 'no manifest exists' do - it 'initializes a manifest' do - expect(DependencyProxy::Manifest).to receive(:new).with(file_name: file_name, digest: digest) - - subject - end + it { is_expected.to be_nil } end context 'manifest exists and matches file_name' do diff --git a/spec/models/deploy_key_spec.rb b/spec/models/deploy_key_spec.rb index fa78527e366..c22bad0e062 100644 --- a/spec/models/deploy_key_spec.rb +++ b/spec/models/deploy_key_spec.rb @@ -5,6 +5,17 @@ require 'spec_helper' RSpec.describe DeployKey, :mailer do describe "Associations" do it { is_expected.to have_many(:deploy_keys_projects) } + it do + is_expected.to have_many(:deploy_keys_projects_with_write_access) + .conditions(can_push: true) + .class_name('DeployKeysProject') + end + it do + is_expected.to have_many(:projects_with_write_access) + .class_name('Project') + .through(:deploy_keys_projects_with_write_access) + .source(:project) + end it { is_expected.to have_many(:projects) } it { is_expected.to have_many(:protected_branch_push_access_levels) } end diff --git a/spec/models/deployment_spec.rb b/spec/models/deployment_spec.rb index f9a05fbb06f..51e1e63da8d 100644 --- a/spec/models/deployment_spec.rb +++ b/spec/models/deployment_spec.rb @@ -385,6 +385,43 @@ RSpec.describe Deployment do end end + describe '.archivables_in' do + subject { described_class.archivables_in(project, limit: limit) } + + let_it_be(:project) { create(:project, :repository) } + let_it_be(:deployment_1) { create(:deployment, project: project) } + let_it_be(:deployment_2) { create(:deployment, project: project) } + let_it_be(:deployment_3) { create(:deployment, project: project) } + + let(:limit) { 100 } + + context 'when there are no archivable deployments in the project' do + it 'returns nothing' do + expect(subject).to be_empty + end + end + + context 'when there are archivable deployments in the project' do + before do + stub_const("::Deployment::ARCHIVABLE_OFFSET", 1) + end + + it 'returns all archivable deployments' do + expect(subject.count).to eq(2) + expect(subject).to contain_exactly(deployment_1, deployment_2) + end + + context 'with limit' do + let(:limit) { 1 } + + it 'takes the limit into account' do + expect(subject.count).to eq(1) + expect(subject.take).to be_in([deployment_1, deployment_2]) + end + end + end + end + describe 'scopes' do describe 'last_for_environment' do let(:production) { create(:environment) } @@ -456,6 +493,17 @@ RSpec.describe Deployment do end end + describe '.ordered' do + let!(:deployment1) { create(:deployment, status: :running) } + let!(:deployment2) { create(:deployment, status: :success, finished_at: Time.current) } + let!(:deployment3) { create(:deployment, status: :canceled, finished_at: 1.day.ago) } + let!(:deployment4) { create(:deployment, status: :success, finished_at: 2.days.ago) } + + it 'sorts by finished at' do + expect(described_class.ordered).to eq([deployment1, deployment2, deployment3, deployment4]) + end + end + describe 'visible' do subject { described_class.visible } @@ -763,6 +811,7 @@ RSpec.describe Deployment do it 'schedules workers when finishing a deploy' do expect(Deployments::UpdateEnvironmentWorker).to receive(:perform_async) expect(Deployments::LinkMergeRequestWorker).to receive(:perform_async) + expect(Deployments::ArchiveInProjectWorker).to receive(:perform_async) expect(Deployments::HooksWorker).to receive(:perform_async) expect(deploy.update_status('success')).to eq(true) @@ -840,6 +889,12 @@ RSpec.describe Deployment do context 'with created deployment' do let(:deployment_status) { :created } + context 'with created build' do + let(:build_status) { :created } + + it_behaves_like 'ignoring build' + end + context 'with running build' do let(:build_status) { :running } @@ -862,12 +917,16 @@ RSpec.describe Deployment do context 'with running deployment' do let(:deployment_status) { :running } + context 'with created build' do + let(:build_status) { :created } + + it_behaves_like 'ignoring build' + end + context 'with running build' do let(:build_status) { :running } - it_behaves_like 'gracefully handling error' do - let(:error_message) { %Q{Status cannot transition via \"run\"} } - end + it_behaves_like 'ignoring build' end context 'with finished build' do @@ -886,6 +945,12 @@ RSpec.describe Deployment do context 'with finished deployment' do let(:deployment_status) { :success } + context 'with created build' do + let(:build_status) { :created } + + it_behaves_like 'ignoring build' + end + context 'with running build' do let(:build_status) { :running } @@ -897,9 +962,13 @@ RSpec.describe Deployment do context 'with finished build' do let(:build_status) { :success } - it_behaves_like 'gracefully handling error' do - let(:error_message) { %Q{Status cannot transition via \"succeed\"} } - end + it_behaves_like 'ignoring build' + end + + context 'with failed build' do + let(:build_status) { :failed } + + it_behaves_like 'synchronizing deployment' end context 'with unrelated build' do diff --git a/spec/models/design_management/version_spec.rb b/spec/models/design_management/version_spec.rb index e004ad024bc..303bac61e1e 100644 --- a/spec/models/design_management/version_spec.rb +++ b/spec/models/design_management/version_spec.rb @@ -283,7 +283,7 @@ RSpec.describe DesignManagement::Version do it 'retrieves author from the Commit if author_id is nil and version has been persisted' do author = create(:user) version = create(:design_version, :committed, author: author) - author.destroy + author.destroy! version.reload commit = version.issue.project.design_repository.commit(version.sha) commit_user = create(:user, email: commit.author_email, name: commit.author_name) diff --git a/spec/models/email_spec.rb b/spec/models/email_spec.rb index 2b09ee5c190..59299a507e4 100644 --- a/spec/models/email_spec.rb +++ b/spec/models/email_spec.rb @@ -13,6 +13,15 @@ RSpec.describe Email do it_behaves_like 'an object with RFC3696 compliant email-formatted attributes', :email do subject { build(:email) } end + + context 'when the email conflicts with the primary email of a different user' do + let(:user) { create(:user) } + let(:email) { build(:email, email: user.email) } + + it 'is invalid' do + expect(email).to be_invalid + end + end end it 'normalize email value' do @@ -33,7 +42,7 @@ RSpec.describe Email do end describe 'scopes' do - let(:user) { create(:user) } + let(:user) { create(:user, :unconfirmed) } it 'scopes confirmed emails' do create(:email, :confirmed, user: user) diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index 08c639957d3..9d9862aa3d3 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -39,7 +39,7 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching do it 'ensures environment tier when a new object is created' do environment = build(:environment, name: 'gprd', tier: nil) - expect { environment.save }.to change { environment.tier }.from(nil).to('production') + expect { environment.save! }.to change { environment.tier }.from(nil).to('production') end it 'ensures environment tier when an existing object is updated' do @@ -418,7 +418,7 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching do context 'not in the same branch' do before do - deployment.update(sha: project.commit('feature').id) + deployment.update!(sha: project.commit('feature').id) end it 'returns false' do @@ -496,7 +496,7 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching do context 'when no other actions' do context 'environment is available' do before do - environment.update(state: :available) + environment.update!(state: :available) end it do @@ -508,7 +508,7 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching do context 'environment is already stopped' do before do - environment.update(state: :stopped) + environment.update!(state: :stopped) end it do @@ -1502,7 +1502,7 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching do deployment = create(:deployment, :success, environment: environment, project: project) deployment.create_ref - expect { environment.destroy }.to change { project.commit(deployment.ref_path) }.to(nil) + expect { environment.destroy! }.to change { project.commit(deployment.ref_path) }.to(nil) end end @@ -1517,7 +1517,7 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching do end it 'returns the environments count grouped by state with zero value' do - environment2.update(state: 'stopped') + environment2.update!(state: 'stopped') expect(project.environments.count_by_state).to eq({ stopped: 3, available: 0 }) end end @@ -1710,4 +1710,36 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching do subject end end + + describe '#should_link_to_merge_requests?' do + subject { environment.should_link_to_merge_requests? } + + context 'when environment is foldered' do + context 'when environment is production tier' do + let(:environment) { create(:environment, project: project, name: 'production/aws') } + + it { is_expected.to eq(true) } + end + + context 'when environment is development tier' do + let(:environment) { create(:environment, project: project, name: 'review/feature') } + + it { is_expected.to eq(false) } + end + end + + context 'when environment is unfoldered' do + context 'when environment is production tier' do + let(:environment) { create(:environment, project: project, name: 'production') } + + it { is_expected.to eq(true) } + end + + context 'when environment is development tier' do + let(:environment) { create(:environment, project: project, name: 'development') } + + it { is_expected.to eq(true) } + end + end + end end diff --git a/spec/models/error_tracking/error_event_spec.rb b/spec/models/error_tracking/error_event_spec.rb index 8e20eb25353..9cf5a405e74 100644 --- a/spec/models/error_tracking/error_event_spec.rb +++ b/spec/models/error_tracking/error_event_spec.rb @@ -11,7 +11,10 @@ RSpec.describe ErrorTracking::ErrorEvent, type: :model do describe 'validations' do it { is_expected.to validate_presence_of(:description) } + it { is_expected.to validate_length_of(:description).is_at_most(1024) } it { is_expected.to validate_presence_of(:occurred_at) } + it { is_expected.to validate_length_of(:level).is_at_most(255) } + it { is_expected.to validate_length_of(:environment).is_at_most(255) } end describe '#stacktrace' do @@ -37,6 +40,23 @@ RSpec.describe ErrorTracking::ErrorEvent, type: :model do expect(event.stacktrace).to be_kind_of(Array) expect(event.stacktrace.first).to eq(expected_entry) end + + context 'error context is missing' do + let(:event) { create(:error_tracking_error_event, :browser) } + + it 'generates a stacktrace without context' do + expected_entry = { + 'lineNo' => 6395, + 'context' => [], + 'filename' => 'webpack-internal:///./node_modules/vue/dist/vue.runtime.esm.js', + 'function' => 'hydrate', + 'colNo' => 0 + } + + expect(event.stacktrace).to be_kind_of(Array) + expect(event.stacktrace.first).to eq(expected_entry) + end + end end describe '#to_sentry_error_event' do diff --git a/spec/models/error_tracking/error_spec.rb b/spec/models/error_tracking/error_spec.rb index 9b8a81c6372..363cd197f3e 100644 --- a/spec/models/error_tracking/error_spec.rb +++ b/spec/models/error_tracking/error_spec.rb @@ -12,8 +12,12 @@ RSpec.describe ErrorTracking::Error, type: :model do describe 'validations' do it { is_expected.to validate_presence_of(:name) } + it { is_expected.to validate_length_of(:name).is_at_most(255) } it { is_expected.to validate_presence_of(:description) } + it { is_expected.to validate_length_of(:description).is_at_most(1024) } it { is_expected.to validate_presence_of(:actor) } + it { is_expected.to validate_length_of(:actor).is_at_most(255) } + it { is_expected.to validate_length_of(:platform).is_at_most(255) } end describe '.report_error' do diff --git a/spec/models/event_spec.rb b/spec/models/event_spec.rb index 41510b7aa1c..ee27eaf1d0b 100644 --- a/spec/models/event_spec.rb +++ b/spec/models/event_spec.rb @@ -32,7 +32,7 @@ RSpec.describe Event do describe 'after_create :set_last_repository_updated_at' do context 'with a push event' do it 'updates the project last_repository_updated_at' do - project.update(last_repository_updated_at: 1.year.ago) + project.update!(last_repository_updated_at: 1.year.ago) create_push_event(project, project.owner) @@ -44,7 +44,7 @@ RSpec.describe Event do context 'without a push event' do it 'does not update the project last_repository_updated_at' do - project.update(last_repository_updated_at: 1.year.ago) + project.update!(last_repository_updated_at: 1.year.ago) create(:closed_issue_event, project: project, author: project.owner) @@ -58,7 +58,7 @@ RSpec.describe Event do describe '#set_last_repository_updated_at' do it 'only updates once every Event::REPOSITORY_UPDATED_AT_INTERVAL minutes' do last_known_timestamp = (Event::REPOSITORY_UPDATED_AT_INTERVAL - 1.minute).ago - project.update(last_repository_updated_at: last_known_timestamp) + project.update!(last_repository_updated_at: last_known_timestamp) project.reload # a reload removes fractions of seconds expect do @@ -73,7 +73,7 @@ RSpec.describe Event do it 'passes event to UserInteractedProject.track' do expect(UserInteractedProject).to receive(:track).with(event) - event.save + event.save! end end end @@ -824,7 +824,7 @@ RSpec.describe Event do context 'when a project was updated less than 1 hour ago' do it 'does not update the project' do - project.update(last_activity_at: Time.current) + project.update!(last_activity_at: Time.current) expect(project).not_to receive(:update_column) .with(:last_activity_at, a_kind_of(Time)) @@ -835,7 +835,7 @@ RSpec.describe Event do context 'when a project was updated more than 1 hour ago' do it 'updates the project' do - project.update(last_activity_at: 1.year.ago) + project.update!(last_activity_at: 1.year.ago) create_push_event(project, project.owner) diff --git a/spec/models/fork_network_spec.rb b/spec/models/fork_network_spec.rb index c2ef1fdcb5f..f2ec0ccb4fd 100644 --- a/spec/models/fork_network_spec.rb +++ b/spec/models/fork_network_spec.rb @@ -8,7 +8,7 @@ RSpec.describe ForkNetwork do describe '#add_root_as_member' do it 'adds the root project as a member when creating a new root network' do project = create(:project) - fork_network = described_class.create(root_project: project) + fork_network = described_class.create!(root_project: project) expect(fork_network.projects).to include(project) end @@ -54,8 +54,8 @@ RSpec.describe ForkNetwork do first_fork = fork_project(first_project) second_fork = fork_project(second_project) - first_project.destroy - second_project.destroy + first_project.destroy! + second_project.destroy! expect(first_fork.fork_network).not_to be_nil expect(first_fork.fork_network.root_project).to be_nil diff --git a/spec/models/generic_commit_status_spec.rb b/spec/models/generic_commit_status_spec.rb index 6fe5a1407a9..9d70019734b 100644 --- a/spec/models/generic_commit_status_spec.rb +++ b/spec/models/generic_commit_status_spec.rb @@ -133,7 +133,7 @@ RSpec.describe GenericCommitStatus do before do generic_commit_status.context = nil generic_commit_status.stage = nil - generic_commit_status.save + generic_commit_status.save! end describe '#context' do diff --git a/spec/models/grafana_integration_spec.rb b/spec/models/grafana_integration_spec.rb index 79f102919ac..bb822187e0c 100644 --- a/spec/models/grafana_integration_spec.rb +++ b/spec/models/grafana_integration_spec.rb @@ -60,7 +60,7 @@ RSpec.describe GrafanaIntegration do context 'with grafana integration enabled' do it 'returns nil' do - grafana_integration.update(enabled: false) + grafana_integration.update!(enabled: false) expect(grafana_integration.client).to be(nil) end @@ -81,8 +81,8 @@ RSpec.describe GrafanaIntegration do end it 'prevents overriding token value with its encrypted or masked version', :aggregate_failures do - expect { grafana_integration.update(token: grafana_integration.encrypted_token) }.not_to change { grafana_integration.reload.send(:token) } - expect { grafana_integration.update(token: grafana_integration.masked_token) }.not_to change { grafana_integration.reload.send(:token) } + expect { grafana_integration.update!(token: grafana_integration.encrypted_token) }.not_to change { grafana_integration.reload.send(:token) } + expect { grafana_integration.update!(token: grafana_integration.masked_token) }.not_to change { grafana_integration.reload.send(:token) } end end end diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index e88abc21ef2..735aa4df2ba 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -37,6 +37,8 @@ RSpec.describe Group do it { is_expected.to have_many(:daily_build_group_report_results).class_name('Ci::DailyBuildGroupReportResult') } it { is_expected.to have_many(:group_callouts).class_name('Users::GroupCallout').with_foreign_key(:group_id) } it { is_expected.to have_many(:bulk_import_exports).class_name('BulkImports::Export') } + it { is_expected.to have_many(:contacts).class_name('CustomerRelations::Contact') } + it { is_expected.to have_many(:organizations).class_name('CustomerRelations::Organization') } describe '#members & #requesters' do let(:requester) { create(:user) } @@ -160,7 +162,7 @@ RSpec.describe Group do context 'when sub group is deleted' do it 'does not delete parent notification settings' do expect do - sub_group.destroy + sub_group.destroy! end.to change { NotificationSetting.count }.by(-1) end end @@ -404,7 +406,7 @@ RSpec.describe Group do subject do recorded_queries.record do - group.update(parent: new_parent) + group.update!(parent: new_parent) end end @@ -496,7 +498,7 @@ RSpec.describe Group do let!(:group) { create(:group, parent: parent_group) } before do - parent_group.update(parent: new_grandparent) + parent_group.update!(parent: new_grandparent) end it 'updates traversal_ids for all descendants' do @@ -563,6 +565,15 @@ RSpec.describe Group do it { expect(group.ancestors.to_sql).not_to include 'traversal_ids <@' } end end + + context 'when project namespace exists in the group' do + let!(:project) { create(:project, group: group) } + let!(:project_namespace) { project.project_namespace } + + it 'filters out project namespace' do + expect(group.descendants.find_by_id(project_namespace.id)).to be_nil + end + end end end @@ -571,8 +582,8 @@ RSpec.describe Group do let(:instance_integration) { build(:jira_integration, :instance) } before do - create(:jira_integration, group: group, project: nil) - create(:integrations_slack, group: another_group, project: nil) + create(:jira_integration, :group, group: group) + create(:integrations_slack, :group, group: another_group) end it 'returns groups without integration' do @@ -718,6 +729,22 @@ RSpec.describe Group do expect(group.group_members.developers.map(&:user)).to include(user) expect(group.group_members.guests.map(&:user)).not_to include(user) end + + context 'when `tasks_to_be_done` and `tasks_project_id` are passed' do + let!(:project) { create(:project, group: group) } + + before do + stub_experiments(invite_members_for_task: true) + group.add_users([create(:user)], :developer, tasks_to_be_done: %w(ci code), tasks_project_id: project.id) + end + + it 'creates a member_task with the correct attributes', :aggregate_failures do + member = group.group_members.last + + expect(member.tasks_to_be_done).to match_array([:ci, :code]) + expect(member.member_task.project).to eq(project) + end + end end describe '#avatar_type' do @@ -831,7 +858,7 @@ RSpec.describe Group do before do parent_group = create(:group) create(:group_member, :owner, group: parent_group) - group.update(parent: parent_group) + group.update!(parent: parent_group) end it { expect(group.last_owner?(@members[:owner])).to be_falsy } @@ -888,7 +915,7 @@ RSpec.describe Group do before do parent_group = create(:group) create(:group_member, :owner, group: parent_group) - group.update(parent: parent_group) + group.update!(parent: parent_group) end it { expect(group.member_last_blocked_owner?(member)).to be(false) } @@ -1936,7 +1963,7 @@ RSpec.describe Group do let(:environment) { 'foo%bar/test' } it 'matches literally for %' do - ci_variable.update(environment_scope: 'foo%bar/*') + ci_variable.update_attribute(:environment_scope, 'foo%bar/*') is_expected.to contain_exactly(ci_variable) end @@ -2077,7 +2104,7 @@ RSpec.describe Group do let(:ancestor_group) { create(:group) } before do - group.update(parent: ancestor_group) + group.update!(parent: ancestor_group) end it 'returns all ancestor group ids' do @@ -2594,7 +2621,7 @@ RSpec.describe Group do let_it_be(:project) { create(:project, group: group, service_desk_enabled: false) } before do - project.update(service_desk_enabled: false) + project.update!(service_desk_enabled: false) end it { is_expected.to eq(false) } @@ -2623,14 +2650,6 @@ RSpec.describe Group do end it_behaves_like 'returns namespaces with disabled email' - - context 'when feature flag :linear_group_ancestor_scopes is disabled' do - before do - stub_feature_flags(linear_group_ancestor_scopes: false) - end - - it_behaves_like 'returns namespaces with disabled email' - end end describe '.timelogs' do diff --git a/spec/models/hooks/project_hook_spec.rb b/spec/models/hooks/project_hook_spec.rb index d811f67d16b..f0ee9a613d8 100644 --- a/spec/models/hooks/project_hook_spec.rb +++ b/spec/models/hooks/project_hook_spec.rb @@ -32,8 +32,8 @@ RSpec.describe ProjectHook do end describe '#rate_limit' do - let_it_be(:hook) { create(:project_hook) } let_it_be(:plan_limits) { create(:plan_limits, :default_plan, web_hook_calls: 100) } + let_it_be(:hook) { create(:project_hook) } it 'returns the default limit' do expect(hook.rate_limit).to be(100) diff --git a/spec/models/identity_spec.rb b/spec/models/identity_spec.rb index c0efb2dff56..124c54a2028 100644 --- a/spec/models/identity_spec.rb +++ b/spec/models/identity_spec.rb @@ -118,19 +118,19 @@ RSpec.describe Identity do it 'if extern_uid changes' do expect(ldap_identity).not_to receive(:ensure_normalized_extern_uid) - ldap_identity.save + ldap_identity.save! end it 'if current_uid is nil' do expect(ldap_identity).to receive(:ensure_normalized_extern_uid) - ldap_identity.update(extern_uid: nil) + ldap_identity.update!(extern_uid: nil) expect(ldap_identity.extern_uid).to be_nil end it 'if extern_uid changed and not nil' do - ldap_identity.update(extern_uid: 'uid=john1,ou=PEOPLE,dc=example,dc=com') + ldap_identity.update!(extern_uid: 'uid=john1,ou=PEOPLE,dc=example,dc=com') expect(ldap_identity.extern_uid).to eq 'uid=john1,ou=people,dc=example,dc=com' end @@ -150,7 +150,7 @@ RSpec.describe Identity do expect(user.user_synced_attributes_metadata.provider).to eq 'ldapmain' - ldap_identity.destroy + ldap_identity.destroy! expect(user.reload.user_synced_attributes_metadata).to be_nil end @@ -162,7 +162,7 @@ RSpec.describe Identity do expect(user.user_synced_attributes_metadata.provider).to eq 'other' - ldap_identity.destroy + ldap_identity.destroy! expect(user.reload.user_synced_attributes_metadata.provider).to eq 'other' end diff --git a/spec/models/integration_spec.rb b/spec/models/integration_spec.rb index 1a83d948fcf..de47fb3839a 100644 --- a/spec/models/integration_spec.rb +++ b/spec/models/integration_spec.rb @@ -299,7 +299,7 @@ RSpec.describe Integration do end context 'when integration is a group-level integration' do - let(:group_integration) { create(:jira_integration, group: group, project: nil) } + let(:group_integration) { create(:jira_integration, :group, group: group) } it 'sets inherit_from_id from integration' do integration = described_class.build_from_integration(group_integration, project_id: project.id) @@ -458,7 +458,7 @@ RSpec.describe Integration do end context 'with an active group-level integration' do - let!(:group_integration) { create(:prometheus_integration, group: group, project: nil, api_url: 'https://prometheus.group.com/') } + let!(:group_integration) { create(:prometheus_integration, :group, group: group, api_url: 'https://prometheus.group.com/') } it 'creates an integration from the group-level integration' do described_class.create_from_active_default_integrations(project, :project_id) @@ -481,7 +481,7 @@ RSpec.describe Integration do end context 'with an active subgroup' do - let!(:subgroup_integration) { create(:prometheus_integration, group: subgroup, project: nil, api_url: 'https://prometheus.subgroup.com/') } + let!(:subgroup_integration) { create(:prometheus_integration, :group, group: subgroup, api_url: 'https://prometheus.subgroup.com/') } let!(:subgroup) { create(:group, parent: group) } let(:project) { create(:project, group: subgroup) } @@ -509,7 +509,7 @@ RSpec.describe Integration do end context 'having an integration inheriting settings' do - let!(:subgroup_integration) { create(:prometheus_integration, group: subgroup, project: nil, inherit_from_id: group_integration.id, api_url: 'https://prometheus.subgroup.com/') } + let!(:subgroup_integration) { create(:prometheus_integration, :group, group: subgroup, inherit_from_id: group_integration.id, api_url: 'https://prometheus.subgroup.com/') } it 'creates an integration from the group-level integration' do described_class.create_from_active_default_integrations(sub_subgroup, :group_id) @@ -552,11 +552,11 @@ RSpec.describe Integration do let_it_be(:subgroup2) { create(:group, parent: group) } let_it_be(:project1) { create(:project, group: subgroup1) } let_it_be(:project2) { create(:project, group: subgroup2) } - let_it_be(:group_integration) { create(:prometheus_integration, group: group, project: nil) } - let_it_be(:subgroup_integration1) { create(:prometheus_integration, group: subgroup1, project: nil, inherit_from_id: group_integration.id) } - let_it_be(:subgroup_integration2) { create(:prometheus_integration, group: subgroup2, project: nil) } - let_it_be(:project_integration1) { create(:prometheus_integration, group: nil, project: project1, inherit_from_id: group_integration.id) } - let_it_be(:project_integration2) { create(:prometheus_integration, group: nil, project: project2, inherit_from_id: subgroup_integration2.id) } + let_it_be(:group_integration) { create(:prometheus_integration, :group, group: group) } + let_it_be(:subgroup_integration1) { create(:prometheus_integration, :group, group: subgroup1, inherit_from_id: group_integration.id) } + let_it_be(:subgroup_integration2) { create(:prometheus_integration, :group, group: subgroup2) } + let_it_be(:project_integration1) { create(:prometheus_integration, project: project1, inherit_from_id: group_integration.id) } + let_it_be(:project_integration2) { create(:prometheus_integration, project: project2, inherit_from_id: subgroup_integration2.id) } it 'returns the groups and projects inheriting from integration ancestors', :aggregate_failures do expect(described_class.inherited_descendants_from_self_or_ancestors_from(group_integration)).to eq([subgroup_integration1, project_integration1]) diff --git a/spec/models/integrations/jira_spec.rb b/spec/models/integrations/jira_spec.rb index 0321b151633..1d81668f97d 100644 --- a/spec/models/integrations/jira_spec.rb +++ b/spec/models/integrations/jira_spec.rb @@ -495,6 +495,18 @@ RSpec.describe Integrations::Jira do end end + describe '#client' do + it 'uses the default GitLab::HTTP timeouts' do + timeouts = Gitlab::HTTP::DEFAULT_TIMEOUT_OPTIONS + stub_request(:get, 'http://jira.example.com/foo') + + expect(Gitlab::HTTP).to receive(:httparty_perform_request) + .with(Net::HTTP::Get, '/foo', hash_including(timeouts)).and_call_original + + jira_integration.client.get('/foo') + end + end + describe '#find_issue' do let(:issue_key) { 'JIRA-123' } let(:issue_url) { "#{url}/rest/api/2/issue/#{issue_key}" } @@ -503,7 +515,7 @@ RSpec.describe Integrations::Jira do stub_request(:get, issue_url).with(basic_auth: [username, password]) end - it 'call the Jira API to get the issue' do + it 'calls the Jira API to get the issue' do jira_integration.find_issue(issue_key) expect(WebMock).to have_requested(:get, issue_url) @@ -845,10 +857,14 @@ RSpec.describe Integrations::Jira do let_it_be(:user) { build_stubbed(:user) } let(:jira_issue) { ExternalIssue.new('JIRA-123', project) } + let(:success_message) { 'SUCCESS: Successfully posted to http://jira.example.com.' } + let(:favicon_path) { "http://localhost/assets/#{find_asset('favicon.png').digest_path}" } subject { jira_integration.create_cross_reference_note(jira_issue, resource, user) } - shared_examples 'creates a comment on Jira' do + shared_examples 'handles cross-references' do + let(:resource_name) { jira_integration.send(:noteable_name, resource) } + let(:resource_url) { jira_integration.send(:build_entity_url, resource_name, resource.to_param) } let(:issue_url) { "#{url}/rest/api/2/issue/JIRA-123" } let(:comment_url) { "#{issue_url}/comment" } let(:remote_link_url) { "#{issue_url}/remotelink" } @@ -860,12 +876,77 @@ RSpec.describe Integrations::Jira do stub_request(:post, remote_link_url).with(basic_auth: [username, password]) end - it 'creates a comment on Jira' do - subject + context 'when enabled' do + before do + allow(jira_integration).to receive(:can_cross_reference?) { true } + end - expect(WebMock).to have_requested(:post, comment_url).with( - body: /mentioned this issue in/ - ).once + it 'creates a comment and remote link' do + expect(subject).to eq(success_message) + expect(WebMock).to have_requested(:post, comment_url).with(body: comment_body).once + expect(WebMock).to have_requested(:post, remote_link_url).with( + body: hash_including( + GlobalID: 'GitLab', + relationship: 'mentioned on', + object: { + url: resource_url, + title: "#{resource.model_name.human} - #{resource.title}", + icon: { title: 'GitLab', url16x16: favicon_path }, + status: { resolved: false } + } + ) + ).once + end + + context 'when comment already exists' do + before do + allow(jira_integration).to receive(:comment_exists?) { true } + end + + it 'does not create a comment or remote link' do + expect(subject).to be_nil + expect(WebMock).not_to have_requested(:post, comment_url) + expect(WebMock).not_to have_requested(:post, remote_link_url) + end + end + + context 'when remote link already exists' do + let(:link) { double(object: { 'url' => resource_url }) } + + before do + allow(jira_integration).to receive(:find_remote_link).and_return(link) + end + + it 'updates the remote link but does not create a comment' do + expect(link).to receive(:save!) + expect(subject).to eq(success_message) + expect(WebMock).not_to have_requested(:post, comment_url) + end + end + end + + context 'when disabled' do + before do + allow(jira_integration).to receive(:can_cross_reference?) { false } + end + + it 'does not create a comment or remote link' do + expect(subject).to eq("Events for #{resource_name.pluralize.humanize(capitalize: false)} are disabled.") + expect(WebMock).not_to have_requested(:post, comment_url) + expect(WebMock).not_to have_requested(:post, remote_link_url) + end + end + + context 'with jira_use_first_ref_by_oid feature flag disabled' do + before do + stub_feature_flags(jira_use_first_ref_by_oid: false) + end + + it 'creates a comment and remote link on Jira' do + expect(subject).to eq(success_message) + expect(WebMock).to have_requested(:post, comment_url).with(body: comment_body).once + expect(WebMock).to have_requested(:post, remote_link_url).once + end end it 'tracks usage' do @@ -877,39 +958,38 @@ RSpec.describe Integrations::Jira do end end - context 'when resource is a commit' do - let(:resource) { project.commit('master') } - - context 'when disabled' do - before do - allow_next_instance_of(described_class) do |instance| - allow(instance).to receive(:commit_events) { false } - end - end - - it { is_expected.to eq('Events for commits are disabled.') } + context 'for commits' do + it_behaves_like 'handles cross-references' do + let(:resource) { project.commit('master') } + let(:comment_body) { /mentioned this issue in \[a commit\|.* on branch \[master\|/ } end + end - context 'when enabled' do - it_behaves_like 'creates a comment on Jira' + context 'for issues' do + it_behaves_like 'handles cross-references' do + let(:resource) { build_stubbed(:issue, project: project) } + let(:comment_body) { /mentioned this issue in \[a issue\|/ } end end - context 'when resource is a merge request' do - let(:resource) { build_stubbed(:merge_request, source_project: project) } - - context 'when disabled' do - before do - allow_next_instance_of(described_class) do |instance| - allow(instance).to receive(:merge_requests_events) { false } - end - end + context 'for merge requests' do + it_behaves_like 'handles cross-references' do + let(:resource) { build_stubbed(:merge_request, source_project: project) } + let(:comment_body) { /mentioned this issue in \[a merge request\|.* on branch \[master\|/ } + end + end - it { is_expected.to eq('Events for merge requests are disabled.') } + context 'for notes' do + it_behaves_like 'handles cross-references' do + let(:resource) { build_stubbed(:note, project: project) } + let(:comment_body) { /mentioned this issue in \[a note\|/ } end + end - context 'when enabled' do - it_behaves_like 'creates a comment on Jira' + context 'for snippets' do + it_behaves_like 'handles cross-references' do + let(:resource) { build_stubbed(:snippet, project: project) } + let(:comment_body) { /mentioned this issue in \[a snippet\|/ } end end end @@ -946,7 +1026,9 @@ RSpec.describe Integrations::Jira do expect(jira_integration).to receive(:log_error).with( 'Error sending message', client_url: 'http://jira.example.com', - error: error_message + 'exception.class' => anything, + 'exception.message' => error_message, + 'exception.backtrace' => anything ) expect(jira_integration.test(nil)).to eq(success: false, result: error_message) diff --git a/spec/models/integrations/pipelines_email_spec.rb b/spec/models/integrations/pipelines_email_spec.rb index afd9d71ebc4..d70f104b965 100644 --- a/spec/models/integrations/pipelines_email_spec.rb +++ b/spec/models/integrations/pipelines_email_spec.rb @@ -35,6 +35,42 @@ RSpec.describe Integrations::PipelinesEmail, :mailer do it { is_expected.not_to validate_presence_of(:recipients) } end + + describe 'validates number of recipients' do + before do + stub_const("#{described_class}::RECIPIENTS_LIMIT", 2) + end + + subject(:integration) { described_class.new(project: project, recipients: recipients, active: true) } + + context 'valid number of recipients' do + let(:recipients) { 'foo@bar.com, , ' } + + it 'does not count empty emails' do + is_expected.to be_valid + end + end + + context 'invalid number of recipients' do + let(:recipients) { 'foo@bar.com bar@foo.com bob@gitlab.com' } + + it { is_expected.not_to be_valid } + + it 'adds an error message' do + integration.valid? + + expect(integration.errors).to contain_exactly('Recipients can\'t exceed 2') + end + + context 'when integration is not active' do + before do + integration.active = false + end + + it { is_expected.to be_valid } + end + end + end end shared_examples 'sending email' do |branches_to_be_notified: nil| @@ -50,7 +86,7 @@ RSpec.describe Integrations::PipelinesEmail, :mailer do it 'sends email' do emails = receivers.map { |r| double(notification_email_or_default: r) } - should_only_email(*emails, kind: :bcc) + should_only_email(*emails) end end diff --git a/spec/models/integrations/shimo_spec.rb b/spec/models/integrations/shimo_spec.rb new file mode 100644 index 00000000000..25df8d2b249 --- /dev/null +++ b/spec/models/integrations/shimo_spec.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ::Integrations::Shimo do + describe '#fields' do + let(:shimo_integration) { create(:shimo_integration) } + + it 'returns custom fields' do + expect(shimo_integration.fields.pluck(:name)).to eq(%w[external_wiki_url]) + end + end + + describe '#create' do + let(:project) { create(:project, :repository) } + let(:external_wiki_url) { 'https://shimo.example.com/desktop' } + let(:params) { { active: true, project: project, external_wiki_url: external_wiki_url } } + + context 'with valid params' do + it 'creates the Shimo integration' do + shimo = described_class.create!(params) + + expect(shimo.valid?).to be true + expect(shimo.render?).to be true + expect(shimo.external_wiki_url).to eq(external_wiki_url) + end + end + + context 'with invalid params' do + it 'cannot create the Shimo integration without external_wiki_url' do + params['external_wiki_url'] = nil + expect { described_class.create!(params) }.to raise_error(ActiveRecord::RecordInvalid) + end + + it 'cannot create the Shimo integration with invalid external_wiki_url' do + params['external_wiki_url'] = 'Fake Invalid URL' + expect { described_class.create!(params) }.to raise_error(ActiveRecord::RecordInvalid) + end + end + end +end diff --git a/spec/models/integrations/zentao_spec.rb b/spec/models/integrations/zentao_spec.rb index a1503ecc092..2b0532c7930 100644 --- a/spec/models/integrations/zentao_spec.rb +++ b/spec/models/integrations/zentao_spec.rb @@ -50,4 +50,10 @@ RSpec.describe Integrations::Zentao do expect(zentao_integration.test).to eq(test_response) end end + + describe '#help' do + it 'renders prompt information' do + expect(zentao_integration.help).not_to be_empty + end + end end diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index 4319407706e..ba4429451d1 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -34,7 +34,8 @@ RSpec.describe Issue do it { is_expected.to have_many(:issue_email_participants) } it { is_expected.to have_many(:timelogs).autosave(true) } it { is_expected.to have_one(:incident_management_issuable_escalation_status) } - it { is_expected.to have_and_belong_to_many(:customer_relations_contacts) } + it { is_expected.to have_many(:issue_customer_relations_contacts) } + it { is_expected.to have_many(:customer_relations_contacts) } describe 'versions.most_recent' do it 'returns the most recent version' do @@ -305,7 +306,7 @@ RSpec.describe Issue do end describe '#reopen' do - let(:issue) { create(:issue, project: reusable_project, state: 'closed', closed_at: Time.current, closed_by: user) } + let_it_be_with_reload(:issue) { create(:issue, project: reusable_project, state: 'closed', closed_at: Time.current, closed_by: user) } it 'sets closed_at to nil when an issue is reopened' do expect { issue.reopen }.to change { issue.closed_at }.to(nil) @@ -315,6 +316,22 @@ RSpec.describe Issue do expect { issue.reopen }.to change { issue.closed_by }.from(user).to(nil) end + it 'clears moved_to_id for moved issues' do + moved_issue = create(:issue) + + issue.update!(moved_to_id: moved_issue.id) + + expect { issue.reopen }.to change { issue.moved_to_id }.from(moved_issue.id).to(nil) + end + + it 'clears duplicated_to_id for duplicated issues' do + duplicate_issue = create(:issue) + + issue.update!(duplicated_to_id: duplicate_issue.id) + + expect { issue.reopen }.to change { issue.duplicated_to_id }.from(duplicate_issue.id).to(nil) + end + it 'changes the state to opened' do expect { issue.reopen }.to change { issue.state_id }.from(described_class.available_states[:closed]).to(described_class.available_states[:opened]) end @@ -1218,7 +1235,7 @@ RSpec.describe Issue do end it 'returns public and hidden issues' do - expect(described_class.public_only).to eq([public_issue, hidden_issue]) + expect(described_class.public_only).to contain_exactly(public_issue, hidden_issue) end end end @@ -1247,7 +1264,7 @@ RSpec.describe Issue do end it 'returns public and hidden issues' do - expect(described_class.without_hidden).to eq([public_issue, hidden_issue]) + expect(described_class.without_hidden).to contain_exactly(public_issue, hidden_issue) end end end diff --git a/spec/models/jira_import_state_spec.rb b/spec/models/jira_import_state_spec.rb index e982b7353ba..a272d001429 100644 --- a/spec/models/jira_import_state_spec.rb +++ b/spec/models/jira_import_state_spec.rb @@ -175,7 +175,7 @@ RSpec.describe JiraImportState do let(:jira_import) { build(:jira_import_state, project: project)} it 'does not run the callback', :aggregate_failures do - expect { jira_import.save }.to change { JiraImportState.count }.by(1) + expect { jira_import.save! }.to change { JiraImportState.count }.by(1) expect(jira_import.reload.error_message).to be_nil end end @@ -184,7 +184,7 @@ RSpec.describe JiraImportState do let(:jira_import) { build(:jira_import_state, project: project, error_message: 'error')} it 'does not run the callback', :aggregate_failures do - expect { jira_import.save }.to change { JiraImportState.count }.by(1) + expect { jira_import.save! }.to change { JiraImportState.count }.by(1) expect(jira_import.reload.error_message).to eq('error') end end diff --git a/spec/models/key_spec.rb b/spec/models/key_spec.rb index 7468c1b9f0a..d41a1604211 100644 --- a/spec/models/key_spec.rb +++ b/spec/models/key_spec.rb @@ -85,9 +85,9 @@ RSpec.describe Key, :mailer do let_it_be(:expiring_soon_notified) { create(:key, expires_at: 4.days.from_now, user: user, before_expiry_notification_delivered_at: Time.current) } let_it_be(:future_expiry) { create(:key, expires_at: 1.month.from_now, user: user) } - describe '.expired_and_not_notified' do + describe '.expired_today_and_not_notified' do it 'returns keys that expire today and in the past' do - expect(described_class.expired_and_not_notified).to contain_exactly(expired_today_not_notified, expired_yesterday) + expect(described_class.expired_today_and_not_notified).to contain_exactly(expired_today_not_notified) end end diff --git a/spec/models/loose_foreign_keys/deleted_record_spec.rb b/spec/models/loose_foreign_keys/deleted_record_spec.rb new file mode 100644 index 00000000000..cd5068bdb52 --- /dev/null +++ b/spec/models/loose_foreign_keys/deleted_record_spec.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe LooseForeignKeys::DeletedRecord, type: :model do + let_it_be(:table) { 'public.projects' } + + let_it_be(:deleted_record_1) { described_class.create!(partition: 1, fully_qualified_table_name: table, primary_key_value: 5) } + let_it_be(:deleted_record_2) { described_class.create!(partition: 1, fully_qualified_table_name: table, primary_key_value: 1) } + let_it_be(:deleted_record_3) { described_class.create!(partition: 1, fully_qualified_table_name: 'public.other_table', primary_key_value: 3) } + let_it_be(:deleted_record_4) { described_class.create!(partition: 1, fully_qualified_table_name: table, primary_key_value: 1) } # duplicate + + describe '.load_batch_for_table' do + it 'loads records and orders them by creation date' do + records = described_class.load_batch_for_table(table, 10) + + expect(records).to eq([deleted_record_1, deleted_record_2, deleted_record_4]) + end + + it 'supports configurable batch size' do + records = described_class.load_batch_for_table(table, 2) + + expect(records).to eq([deleted_record_1, deleted_record_2]) + end + end + + describe '.mark_records_processed' do + it 'updates all records' do + described_class.mark_records_processed([deleted_record_1, deleted_record_2, deleted_record_4]) + + expect(described_class.status_pending.count).to eq(1) + expect(described_class.status_processed.count).to eq(3) + end + end +end diff --git a/spec/models/loose_foreign_keys/modification_tracker_spec.rb b/spec/models/loose_foreign_keys/modification_tracker_spec.rb new file mode 100644 index 00000000000..069ccf85141 --- /dev/null +++ b/spec/models/loose_foreign_keys/modification_tracker_spec.rb @@ -0,0 +1,93 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe LooseForeignKeys::ModificationTracker do + subject(:tracker) { described_class.new } + + describe '#over_limit?' do + it 'is true when deletion MAX_DELETES is exceeded' do + stub_const('LooseForeignKeys::ModificationTracker::MAX_DELETES', 5) + + tracker.add_deletions('issues', 10) + expect(tracker).to be_over_limit + end + + it 'is false when MAX_DELETES is not exceeded' do + tracker.add_deletions('issues', 3) + + expect(tracker).not_to be_over_limit + end + + it 'is true when deletion MAX_UPDATES is exceeded' do + stub_const('LooseForeignKeys::ModificationTracker::MAX_UPDATES', 5) + + tracker.add_updates('issues', 3) + tracker.add_updates('issues', 4) + + expect(tracker).to be_over_limit + end + + it 'is false when MAX_UPDATES is not exceeded' do + tracker.add_updates('projects', 3) + + expect(tracker).not_to be_over_limit + end + + it 'is true when max runtime is exceeded' do + monotonic_time_before = 1 # this will be the start time + monotonic_time_after = described_class::MAX_RUNTIME.to_i + 1 # this will be returned when over_limit? is called + + allow(Gitlab::Metrics::System).to receive(:monotonic_time).and_return(monotonic_time_before, monotonic_time_after) + + tracker + + expect(tracker).to be_over_limit + end + + it 'is false when max runtime is not exceeded' do + expect(tracker).not_to be_over_limit + end + end + + describe '#add_deletions' do + it 'increments a Prometheus counter' do + counter = Gitlab::Metrics.registry.get(:loose_foreign_key_deletions) + + subject.add_deletions(:users, 4) + + expect(counter.get(table: :users)).to eq(4) + end + end + + describe '#add_updates' do + it 'increments a Prometheus counter' do + counter = Gitlab::Metrics.registry.get(:loose_foreign_key_updates) + + subject.add_updates(:users, 4) + + expect(counter.get(table: :users)).to eq(4) + end + end + + describe '#stats' do + it 'exposes stats' do + freeze_time do + tracker + tracker.add_deletions('issues', 5) + tracker.add_deletions('issues', 2) + tracker.add_deletions('projects', 2) + + tracker.add_updates('projects', 3) + + expect(tracker.stats).to eq({ + over_limit: false, + delete_count_by_table: { 'issues' => 7, 'projects' => 2 }, + update_count_by_table: { 'projects' => 3 }, + delete_count: 9, + update_count: 3 + }) + end + end + end +end diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb index afe78adc547..abff1815f1a 100644 --- a/spec/models/member_spec.rb +++ b/spec/models/member_spec.rb @@ -9,6 +9,7 @@ RSpec.describe Member do describe 'Associations' do it { is_expected.to belong_to(:user) } + it { is_expected.to have_one(:member_task) } end describe 'Validation' do @@ -678,6 +679,19 @@ RSpec.describe Member do expect(member.invite_token).not_to be_nil expect_any_instance_of(Member).not_to receive(:after_accept_invite) end + + it 'schedules a TasksToBeDone::CreateWorker task' do + stub_experiments(invite_members_for_task: true) + + member_task = create(:member_task, member: member, project: member.project) + + expect(TasksToBeDone::CreateWorker) + .to receive(:perform_async) + .with(member_task.id, member.created_by_id, [user.id]) + .once + + member.accept_invite!(user) + end end describe '#decline_invite!' do diff --git a/spec/models/members/member_task_spec.rb b/spec/models/members/member_task_spec.rb new file mode 100644 index 00000000000..b06aa05c255 --- /dev/null +++ b/spec/models/members/member_task_spec.rb @@ -0,0 +1,124 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe MemberTask do + describe 'Associations' do + it { is_expected.to belong_to(:member) } + it { is_expected.to belong_to(:project) } + end + + describe 'Validations' do + it { is_expected.to validate_presence_of(:member) } + it { is_expected.to validate_presence_of(:project) } + it { is_expected.to validate_inclusion_of(:tasks).in_array(MemberTask::TASKS.values) } + + describe 'unique tasks validation' do + subject do + build(:member_task, tasks: [0, 0]) + end + + it 'expects the task values to be unique' do + expect(subject).to be_invalid + expect(subject.errors[:tasks]).to include('are not unique') + end + end + + describe 'project validations' do + let_it_be(:project) { create(:project) } + + subject do + build(:member_task, member: member, project: project, tasks_to_be_done: [:ci, :code]) + end + + context 'when the member source is a group' do + let_it_be(:member) { create(:group_member) } + + it "expects the project to be part of the member's group projects" do + expect(subject).to be_invalid + expect(subject.errors[:project]).to include('is not in the member group') + end + + context "when the project is part of the member's group projects" do + let_it_be(:project) { create(:project, namespace: member.source) } + + it { is_expected.to be_valid } + end + end + + context 'when the member source is a project' do + let_it_be(:member) { create(:project_member) } + + it "expects the project to be the member's project" do + expect(subject).to be_invalid + expect(subject.errors[:project]).to include('is not the member project') + end + + context "when the project is the member's project" do + let_it_be(:project) { member.source } + + it { is_expected.to be_valid } + end + end + end + end + + describe '.for_members' do + it 'returns the member_tasks for multiple members' do + member1 = create(:group_member) + member_task1 = create(:member_task, member: member1) + create(:member_task) + expect(described_class.for_members([member1])).to match_array([member_task1]) + end + end + + describe '#tasks_to_be_done' do + subject { member_task.tasks_to_be_done } + + let_it_be(:member_task) { build(:member_task) } + + before do + member_task[:tasks] = [0, 1] + end + + it 'returns an array of symbols for the corresponding integers' do + expect(subject).to match_array([:ci, :code]) + end + end + + describe '#tasks_to_be_done=' do + let_it_be(:member_task) { build(:member_task) } + + context 'when passing valid values' do + subject { member_task[:tasks] } + + before do + member_task.tasks_to_be_done = tasks + end + + context 'when passing tasks as strings' do + let_it_be(:tasks) { %w(ci code) } + + it 'sets an array of integers for the corresponding tasks' do + expect(subject).to match_array([0, 1]) + end + end + + context 'when passing a single task' do + let_it_be(:tasks) { :ci } + + it 'sets an array of integers for the corresponding tasks' do + expect(subject).to match_array([1]) + end + end + + context 'when passing a task twice' do + let_it_be(:tasks) { %w(ci ci) } + + it 'is set only once' do + expect(subject).to match_array([1]) + end + end + end + end +end diff --git a/spec/models/members/project_member_spec.rb b/spec/models/members/project_member_spec.rb index ca846cf9e8e..031caefbd43 100644 --- a/spec/models/members/project_member_spec.rb +++ b/spec/models/members/project_member_spec.rb @@ -256,59 +256,5 @@ RSpec.describe ProjectMember do it_behaves_like 'calls AuthorizedProjectUpdate::UserRefreshFromReplicaWorker with a delay to update project authorizations' end - - context 'when the feature flag `specialized_service_for_project_member_auth_refresh` is disabled' do - before do - stub_feature_flags(specialized_service_for_project_member_auth_refresh: false) - end - - shared_examples_for 'calls UserProjectAccessChangedService to recalculate authorizations' do - it 'calls UserProjectAccessChangedService' do - expect_next_instance_of(UserProjectAccessChangedService, user.id) do |service| - expect(service).to receive(:execute) - end - - action - end - end - - context 'on create' do - let(:action) { project.add_user(user, Gitlab::Access::GUEST) } - - it 'changes access level' do - expect { action }.to change { user.can?(:guest_access, project) }.from(false).to(true) - end - - it_behaves_like 'calls UserProjectAccessChangedService to recalculate authorizations' - end - - context 'on update' do - let(:action) { project.members.find_by(user: user).update!(access_level: Gitlab::Access::DEVELOPER) } - - before do - project.add_user(user, Gitlab::Access::GUEST) - end - - it 'changes access level' do - expect { action }.to change { user.can?(:developer_access, project) }.from(false).to(true) - end - - it_behaves_like 'calls UserProjectAccessChangedService to recalculate authorizations' - end - - context 'on destroy' do - let(:action) { project.members.find_by(user: user).destroy! } - - before do - project.add_user(user, Gitlab::Access::GUEST) - end - - it 'changes access level', :sidekiq_inline do - expect { action }.to change { user.can?(:guest_access, project) }.from(true).to(false) - end - - it_behaves_like 'calls UserProjectAccessChangedService to recalculate authorizations' - end - end end end diff --git a/spec/models/merge_request_assignee_spec.rb b/spec/models/merge_request_assignee_spec.rb index d287392bf7f..5bb8e7184a3 100644 --- a/spec/models/merge_request_assignee_spec.rb +++ b/spec/models/merge_request_assignee_spec.rb @@ -37,4 +37,8 @@ RSpec.describe MergeRequestAssignee do end end end + + it_behaves_like 'having unique enum values' + + it_behaves_like 'having reviewer state' end diff --git a/spec/models/merge_request_diff_commit_spec.rb b/spec/models/merge_request_diff_commit_spec.rb index adddec7ced8..25e5e40feb7 100644 --- a/spec/models/merge_request_diff_commit_spec.rb +++ b/spec/models/merge_request_diff_commit_spec.rb @@ -46,11 +46,7 @@ RSpec.describe MergeRequestDiffCommit do { "message": "Add submodule from gitlab.com\n\nSigned-off-by: Dmitriy Zaporozhets \u003cdmitriy.zaporozhets@gmail.com\u003e\n", "authored_date": "2014-02-27T10:01:38.000+01:00".to_time, - "author_name": "Dmitriy Zaporozhets", - "author_email": "dmitriy.zaporozhets@gmail.com", "committed_date": "2014-02-27T10:01:38.000+01:00".to_time, - "committer_name": "Dmitriy Zaporozhets", - "committer_email": "dmitriy.zaporozhets@gmail.com", "commit_author_id": an_instance_of(Integer), "committer_id": an_instance_of(Integer), "merge_request_diff_id": merge_request_diff_id, @@ -61,11 +57,7 @@ RSpec.describe MergeRequestDiffCommit do { "message": "Change some files\n\nSigned-off-by: Dmitriy Zaporozhets \u003cdmitriy.zaporozhets@gmail.com\u003e\n", "authored_date": "2014-02-27T09:57:31.000+01:00".to_time, - "author_name": "Dmitriy Zaporozhets", - "author_email": "dmitriy.zaporozhets@gmail.com", "committed_date": "2014-02-27T09:57:31.000+01:00".to_time, - "committer_name": "Dmitriy Zaporozhets", - "committer_email": "dmitriy.zaporozhets@gmail.com", "commit_author_id": an_instance_of(Integer), "committer_id": an_instance_of(Integer), "merge_request_diff_id": merge_request_diff_id, @@ -79,7 +71,7 @@ RSpec.describe MergeRequestDiffCommit do subject { described_class.create_bulk(merge_request_diff_id, commits) } it 'inserts the commits into the database en masse' do - expect(Gitlab::Database.main).to receive(:bulk_insert) + expect(ApplicationRecord).to receive(:legacy_bulk_insert) .with(described_class.table_name, rows) subject @@ -111,11 +103,7 @@ RSpec.describe MergeRequestDiffCommit do [{ "message": "Weird commit date\n", "authored_date": timestamp, - "author_name": "Alejandro RodrÃguez", - "author_email": "alejorro70@gmail.com", "committed_date": timestamp, - "committer_name": "Alejandro RodrÃguez", - "committer_email": "alejorro70@gmail.com", "commit_author_id": an_instance_of(Integer), "committer_id": an_instance_of(Integer), "merge_request_diff_id": merge_request_diff_id, @@ -126,7 +114,7 @@ RSpec.describe MergeRequestDiffCommit do end it 'uses a sanitized date' do - expect(Gitlab::Database.main).to receive(:bulk_insert) + expect(ApplicationRecord).to receive(:legacy_bulk_insert) .with(described_class.table_name, rows) subject diff --git a/spec/models/merge_request_diff_spec.rb b/spec/models/merge_request_diff_spec.rb index 5fff880c44e..afe7251f59a 100644 --- a/spec/models/merge_request_diff_spec.rb +++ b/spec/models/merge_request_diff_spec.rb @@ -240,8 +240,8 @@ RSpec.describe MergeRequestDiff do stub_external_diffs_setting(enabled: true) expect(diff).not_to receive(:save!) - expect(Gitlab::Database.main) - .to receive(:bulk_insert) + expect(ApplicationRecord) + .to receive(:legacy_bulk_insert) .with('merge_request_diff_files', anything) .and_raise(ActiveRecord::Rollback) @@ -1080,6 +1080,22 @@ RSpec.describe MergeRequestDiff do end end + describe '#commits' do + include ProjectForksHelper + + let_it_be(:target) { create(:project, :test_repo) } + let_it_be(:forked) { fork_project(target, nil, repository: true) } + let_it_be(:mr) { create(:merge_request, source_project: forked, target_project: target) } + + it 'returns a CommitCollection whose container points to the target project' do + expect(mr.merge_request_diff.commits.container).to eq(target) + end + + it 'returns a non-empty CommitCollection' do + expect(mr.merge_request_diff.commits.commits.size).to be > 0 + end + end + describe '.latest_diff_for_merge_requests' do let_it_be(:merge_request_1) { create(:merge_request_without_merge_request_diff) } let_it_be(:merge_request_1_diff_1) { create(:merge_request_diff, merge_request: merge_request_1, created_at: 3.days.ago) } diff --git a/spec/models/merge_request_reviewer_spec.rb b/spec/models/merge_request_reviewer_spec.rb index 76b44abca54..d69d60c94f0 100644 --- a/spec/models/merge_request_reviewer_spec.rb +++ b/spec/models/merge_request_reviewer_spec.rb @@ -7,6 +7,10 @@ RSpec.describe MergeRequestReviewer do subject { merge_request.merge_request_reviewers.build(reviewer: create(:user)) } + it_behaves_like 'having unique enum values' + + it_behaves_like 'having reviewer state' + describe 'associations' do it { is_expected.to belong_to(:merge_request).class_name('MergeRequest') } it { is_expected.to belong_to(:reviewer).class_name('User').inverse_of(:merge_request_reviewers) } diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index d871453e062..5618fb06157 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -1638,6 +1638,22 @@ RSpec.describe MergeRequest, factory_default: :keep do expect(request.default_merge_commit_message) .not_to match("By removing all code\n\n") end + + it 'uses template from target project' do + request = build(:merge_request, title: 'Fix everything') + subject.target_project.merge_commit_template = '%{title}' + + expect(request.default_merge_commit_message) + .to eq('Fix everything') + end + + it 'ignores template when include_description is true' do + request = build(:merge_request, title: 'Fix everything') + subject.target_project.merge_commit_template = '%{title}' + + expect(request.default_merge_commit_message(include_description: true)) + .to match("See merge request #{request.to_reference(full: true)}") + end end describe "#auto_merge_strategy" do @@ -2904,6 +2920,8 @@ RSpec.describe MergeRequest, factory_default: :keep do params = {} merge_jid = 'hash-123' + allow(MergeWorker).to receive(:with_status).and_return(MergeWorker) + expect(merge_request).to receive(:expire_etag_cache) expect(MergeWorker).to receive(:perform_async).with(merge_request.id, user_id, params) do merge_jid @@ -2922,6 +2940,10 @@ RSpec.describe MergeRequest, factory_default: :keep do subject(:execute) { merge_request.rebase_async(user_id) } + before do + allow(RebaseWorker).to receive(:with_status).and_return(RebaseWorker) + end + it 'atomically enqueues a RebaseWorker job and updates rebase_jid' do expect(RebaseWorker) .to receive(:perform_async) diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index c201d89947e..8f5860c799c 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -28,6 +28,41 @@ RSpec.describe Namespace do it { is_expected.to have_one :onboarding_progress } it { is_expected.to have_one :admin_note } it { is_expected.to have_many :pending_builds } + + describe '#children' do + let_it_be(:group) { create(:group) } + let_it_be(:subgroup) { create(:group, parent: group) } + let_it_be(:project_with_namespace) { create(:project, namespace: group) } + + it 'excludes project namespaces' do + expect(project_with_namespace.project_namespace.parent).to eq(group) + expect(group.children).to match_array([subgroup]) + end + end + end + + shared_examples 'validations called by different namespace types' do |method| + using RSpec::Parameterized::TableSyntax + + where(:namespace_type, :call_validation) do + :namespace | true + :group | true + :user_namespace | true + :project_namespace | false + end + + with_them do + it 'conditionally runs given validation' do + namespace = build(namespace_type) + if call_validation + expect(namespace).to receive(method) + else + expect(namespace).not_to receive(method) + end + + namespace.valid? + end + end end describe 'validations' do @@ -50,10 +85,10 @@ RSpec.describe Namespace do ref(:project_sti_name) | ref(:user_sti_name) | 'project namespace cannot be the parent of another namespace' ref(:project_sti_name) | ref(:group_sti_name) | 'project namespace cannot be the parent of another namespace' ref(:project_sti_name) | ref(:project_sti_name) | 'project namespace cannot be the parent of another namespace' - ref(:group_sti_name) | ref(:user_sti_name) | 'cannot not be used for user namespace' + ref(:group_sti_name) | ref(:user_sti_name) | 'cannot be used for user namespace' ref(:group_sti_name) | ref(:group_sti_name) | nil ref(:group_sti_name) | ref(:project_sti_name) | nil - ref(:user_sti_name) | ref(:user_sti_name) | 'cannot not be used for user namespace' + ref(:user_sti_name) | ref(:user_sti_name) | 'cannot be used for user namespace' ref(:user_sti_name) | ref(:group_sti_name) | 'user namespace cannot be the parent of another namespace' ref(:user_sti_name) | ref(:project_sti_name) | nil end @@ -102,14 +137,20 @@ RSpec.describe Namespace do end end - it 'does not allow too deep nesting' do - ancestors = (1..21).to_a - group = build(:group) + describe '#nesting_level_allowed' do + context 'for a group' do + it 'does not allow too deep nesting' do + ancestors = (1..21).to_a + group = build(:group) + + allow(group).to receive(:ancestors).and_return(ancestors) - allow(group).to receive(:ancestors).and_return(ancestors) + expect(group).not_to be_valid + expect(group.errors[:parent_id].first).to eq('has too deep level of nesting') + end + end - expect(group).not_to be_valid - expect(group.errors[:parent_id].first).to eq('has too deep level of nesting') + it_behaves_like 'validations called by different namespace types', :nesting_level_allowed end describe 'reserved path validation' do @@ -188,7 +229,7 @@ RSpec.describe Namespace do expect(namespace.path).to eq('j') - namespace.update(name: 'something new') + namespace.update!(name: 'something new') expect(namespace).to be_valid expect(namespace.name).to eq('something new') @@ -199,8 +240,10 @@ RSpec.describe Namespace do let(:namespace) { build(:project_namespace) } it 'allows to update path to single char' do - namespace = create(:project_namespace) - namespace.update(path: 'j') + project = create(:project) + namespace = project.project_namespace + + namespace.update!(path: 'j') expect(namespace).to be_valid end @@ -244,7 +287,7 @@ RSpec.describe Namespace do end end - context 'creating a default Namespace' do + context 'creating a Namespace with nil type' do let(:namespace_type) { nil } it 'is the correct type of namespace' do @@ -255,7 +298,7 @@ RSpec.describe Namespace do end context 'creating an unknown Namespace type' do - let(:namespace_type) { 'One' } + let(:namespace_type) { 'nonsense' } it 'creates a default Namespace' do expect(Namespace.find(namespace.id)).to be_a(Namespace) @@ -273,8 +316,8 @@ RSpec.describe Namespace do describe '.by_parent' do it 'includes correct namespaces' do - expect(described_class.by_parent(namespace1.id)).to eq([namespace1sub]) - expect(described_class.by_parent(namespace2.id)).to eq([namespace2sub]) + expect(described_class.by_parent(namespace1.id)).to match_array([namespace1sub]) + expect(described_class.by_parent(namespace2.id)).to match_array([namespace2sub]) expect(described_class.by_parent(nil)).to match_array([namespace, namespace1, namespace2]) end end @@ -302,9 +345,13 @@ RSpec.describe Namespace do describe '.without_project_namespaces' do let_it_be(:user_namespace) { create(:user_namespace) } - let_it_be(:project_namespace) { create(:project_namespace) } + let_it_be(:project) { create(:project) } + let_it_be(:project_namespace) { project.project_namespace } it 'excludes project namespaces' do + expect(project_namespace).not_to be_nil + expect(project_namespace.parent).not_to be_nil + expect(described_class.all).to include(project_namespace) expect(described_class.without_project_namespaces).to match_array([namespace, namespace1, namespace2, namespace1sub, namespace2sub, user_namespace, project_namespace.parent]) end end @@ -519,6 +566,25 @@ RSpec.describe Namespace do it 'returns namespaces with a matching route path regardless of the casing' do expect(described_class.search('PARENT-PATH/NEW-PATH', include_parents: true)).to eq([second_group]) end + + context 'with project namespaces' do + let_it_be(:project) { create(:project, namespace: parent_group, path: 'some-new-path') } + let_it_be(:project_namespace) { project.project_namespace } + + it 'does not return project namespace' do + search_result = described_class.search('path') + + expect(search_result).not_to include(project_namespace) + expect(search_result).to match_array([first_group, parent_group, second_group]) + end + + it 'does not return project namespace when including parents' do + search_result = described_class.search('path', include_parents: true) + + expect(search_result).not_to include(project_namespace) + expect(search_result).to match_array([first_group, parent_group, second_group]) + end + end end describe '.with_statistics' do @@ -528,26 +594,30 @@ RSpec.describe Namespace do create(:project, namespace: namespace, statistics: build(:project_statistics, - namespace: namespace, - repository_size: 101, - wiki_size: 505, - lfs_objects_size: 202, - build_artifacts_size: 303, - packages_size: 404, - snippets_size: 605)) + namespace: namespace, + repository_size: 101, + wiki_size: 505, + lfs_objects_size: 202, + build_artifacts_size: 303, + pipeline_artifacts_size: 707, + packages_size: 404, + snippets_size: 605, + uploads_size: 808)) end let(:project2) do create(:project, namespace: namespace, statistics: build(:project_statistics, - namespace: namespace, - repository_size: 10, - wiki_size: 50, - lfs_objects_size: 20, - build_artifacts_size: 30, - packages_size: 40, - snippets_size: 60)) + namespace: namespace, + repository_size: 10, + wiki_size: 50, + lfs_objects_size: 20, + build_artifacts_size: 30, + pipeline_artifacts_size: 70, + packages_size: 40, + snippets_size: 60, + uploads_size: 80)) end it "sums all project storage counters in the namespace" do @@ -555,13 +625,15 @@ RSpec.describe Namespace do project2 statistics = described_class.with_statistics.find(namespace.id) - expect(statistics.storage_size).to eq 2330 + expect(statistics.storage_size).to eq 3995 expect(statistics.repository_size).to eq 111 expect(statistics.wiki_size).to eq 555 expect(statistics.lfs_objects_size).to eq 222 expect(statistics.build_artifacts_size).to eq 333 + expect(statistics.pipeline_artifacts_size).to eq 777 expect(statistics.packages_size).to eq 444 expect(statistics.snippets_size).to eq 665 + expect(statistics.uploads_size).to eq 888 end it "correctly handles namespaces without projects" do @@ -572,8 +644,10 @@ RSpec.describe Namespace do expect(statistics.wiki_size).to eq 0 expect(statistics.lfs_objects_size).to eq 0 expect(statistics.build_artifacts_size).to eq 0 + expect(statistics.pipeline_artifacts_size).to eq 0 expect(statistics.packages_size).to eq 0 expect(statistics.snippets_size).to eq 0 + expect(statistics.uploads_size).to eq 0 end end @@ -673,7 +747,7 @@ RSpec.describe Namespace do end it "moves dir if path changed" do - namespace.update(path: namespace.full_path + '_new') + namespace.update!(path: namespace.full_path + '_new') expect(gitlab_shell.repository_exists?(project.repository_storage, "#{namespace.path}/#{project.path}.git")).to be_truthy end @@ -684,7 +758,7 @@ RSpec.describe Namespace do expect(namespace).to receive(:write_projects_repository_config).and_raise('foo') expect do - namespace.update(path: namespace.full_path + '_new') + namespace.update!(path: namespace.full_path + '_new') end.to raise_error('foo') end end @@ -701,7 +775,7 @@ RSpec.describe Namespace do end expect(Gitlab::ErrorTracking).to receive(:should_raise_for_dev?).and_return(false) # like prod - namespace.update(path: namespace.full_path + '_new') + namespace.update!(path: namespace.full_path + '_new') end end end @@ -931,7 +1005,7 @@ RSpec.describe Namespace do it "repository directory remains unchanged if path changed" do before_disk_path = project.disk_path - namespace.update(path: namespace.full_path + '_new') + namespace.update!(path: namespace.full_path + '_new') expect(before_disk_path).to eq(project.disk_path) expect(gitlab_shell.repository_exists?(project.repository_storage, "#{project.disk_path}.git")).to be_truthy @@ -946,7 +1020,7 @@ RSpec.describe Namespace do let!(:legacy_project_in_subgroup) { create(:project, :legacy_storage, :repository, namespace: subgroup, name: 'foo3') } it 'updates project full path in .git/config' do - parent.update(path: 'mygroup_new') + parent.update!(path: 'mygroup_new') expect(project_rugged(project_in_parent_group).config['gitlab.fullpath']).to eq "mygroup_new/#{project_in_parent_group.path}" expect(project_rugged(hashed_project_in_subgroup).config['gitlab.fullpath']).to eq "mygroup_new/mysubgroup/#{hashed_project_in_subgroup.path}" @@ -958,7 +1032,7 @@ RSpec.describe Namespace do repository_hashed_project_in_subgroup = hashed_project_in_subgroup.project_repository repository_legacy_project_in_subgroup = legacy_project_in_subgroup.project_repository - parent.update(path: 'mygroup_moved') + parent.update!(path: 'mygroup_moved') expect(repository_project_in_parent_group.reload.disk_path).to eq "mygroup_moved/#{project_in_parent_group.path}" expect(repository_hashed_project_in_subgroup.reload.disk_path).to eq hashed_project_in_subgroup.disk_path @@ -992,7 +1066,7 @@ RSpec.describe Namespace do it 'renames its dirs when deleted' do allow(GitlabShellWorker).to receive(:perform_in) - namespace.destroy + namespace.destroy! expect(File.exist?(deleted_path_in_dir)).to be(true) end @@ -1000,7 +1074,7 @@ RSpec.describe Namespace do it 'schedules the namespace for deletion' do expect(GitlabShellWorker).to receive(:perform_in).with(5.minutes, :rm_namespace, repository_storage, deleted_path) - namespace.destroy + namespace.destroy! end context 'in sub-groups' do @@ -1014,7 +1088,7 @@ RSpec.describe Namespace do it 'renames its dirs when deleted' do allow(GitlabShellWorker).to receive(:perform_in) - child.destroy + child.destroy! expect(File.exist?(deleted_path_in_dir)).to be(true) end @@ -1022,7 +1096,7 @@ RSpec.describe Namespace do it 'schedules the namespace for deletion' do expect(GitlabShellWorker).to receive(:perform_in).with(5.minutes, :rm_namespace, repository_storage, deleted_path) - child.destroy + child.destroy! end end end @@ -1035,7 +1109,7 @@ RSpec.describe Namespace do expect(File.exist?(path_in_dir)).to be(false) - namespace.destroy + namespace.destroy! expect(File.exist?(deleted_path_in_dir)).to be(false) end @@ -1293,6 +1367,7 @@ RSpec.describe Namespace do context 'refreshing project access on updating share_with_group_lock' do let(:group) { create(:group, share_with_group_lock: false) } let(:project) { create(:project, :private, group: group) } + let(:another_project) { create(:project, :private, group: group) } let_it_be(:shared_with_group_one) { create(:group) } let_it_be(:shared_with_group_two) { create(:group) } @@ -1305,6 +1380,7 @@ RSpec.describe Namespace do shared_with_group_one.add_developer(group_one_user) shared_with_group_two.add_developer(group_two_user) create(:project_group_link, group: shared_with_group_one, project: project) + create(:project_group_link, group: shared_with_group_one, project: another_project) create(:project_group_link, group: shared_with_group_two, project: project) end @@ -1312,6 +1388,9 @@ RSpec.describe Namespace do expect(AuthorizedProjectUpdate::ProjectRecalculateWorker) .to receive(:perform_async).with(project.id).once + expect(AuthorizedProjectUpdate::ProjectRecalculateWorker) + .to receive(:perform_async).with(another_project.id).once + execute_update end @@ -1344,11 +1423,23 @@ RSpec.describe Namespace do stub_feature_flags(specialized_worker_for_group_lock_update_auth_recalculation: false) end - it 'refreshes the permissions of the members of the old and new namespace' do + it 'updates authorizations leading to users from shared groups losing access', :sidekiq_inline do expect { execute_update } .to change { group_one_user.authorized_projects.include?(project) }.from(true).to(false) .and change { group_two_user.authorized_projects.include?(project) }.from(true).to(false) end + + it 'updates the authorizations in a non-blocking manner' do + expect(AuthorizedProjectsWorker).to( + receive(:bulk_perform_async) + .with([[group_one_user.id]])).once + + expect(AuthorizedProjectsWorker).to( + receive(:bulk_perform_async) + .with([[group_two_user.id]])).once + + execute_update + end end end @@ -1544,7 +1635,7 @@ RSpec.describe Namespace do it 'returns the path before last save' do group = create(:group) - group.update(parent: nil) + group.update!(parent: nil) expect(group.full_path_before_last_save).to eq(group.path_before_last_save) end @@ -1555,7 +1646,7 @@ RSpec.describe Namespace do group = create(:group, parent: nil) parent = create(:group) - group.update(parent: parent) + group.update!(parent: parent) expect(group.full_path_before_last_save).to eq("#{group.path_before_last_save}") end @@ -1566,7 +1657,7 @@ RSpec.describe Namespace do parent = create(:group) group = create(:group, parent: parent) - group.update(parent: nil) + group.update!(parent: nil) expect(group.full_path_before_last_save).to eq("#{parent.full_path}/#{group.path}") end @@ -1578,7 +1669,7 @@ RSpec.describe Namespace do group = create(:group, parent: parent) new_parent = create(:group) - group.update(parent: new_parent) + group.update!(parent: new_parent) expect(group.full_path_before_last_save).to eq("#{parent.full_path}/#{group.path}") end @@ -1845,87 +1936,95 @@ RSpec.describe Namespace do end context 'with a parent' do - context 'when parent has shared runners disabled' do - let(:parent) { create(:group, :shared_runners_disabled) } - let(:group) { build(:group, shared_runners_enabled: true, parent_id: parent.id) } - - it 'is invalid' do - expect(group).to be_invalid - expect(group.errors[:shared_runners_enabled]).to include('cannot be enabled because parent group has shared Runners disabled') + context 'when namespace is a group' do + context 'when parent has shared runners disabled' do + let(:parent) { create(:group, :shared_runners_disabled) } + let(:group) { build(:group, shared_runners_enabled: true, parent_id: parent.id) } + + it 'is invalid' do + expect(group).to be_invalid + expect(group.errors[:shared_runners_enabled]).to include('cannot be enabled because parent group has shared Runners disabled') + end end - end - context 'when parent has shared runners disabled but allows override' do - let(:parent) { create(:group, :shared_runners_disabled, :allow_descendants_override_disabled_shared_runners) } - let(:group) { build(:group, shared_runners_enabled: true, parent_id: parent.id) } + context 'when parent has shared runners disabled but allows override' do + let(:parent) { create(:group, :shared_runners_disabled, :allow_descendants_override_disabled_shared_runners) } + let(:group) { build(:group, shared_runners_enabled: true, parent_id: parent.id) } - it 'is valid' do - expect(group).to be_valid + it 'is valid' do + expect(group).to be_valid + end end - end - context 'when parent has shared runners enabled' do - let(:parent) { create(:group, shared_runners_enabled: true) } - let(:group) { build(:group, shared_runners_enabled: true, parent_id: parent.id) } + context 'when parent has shared runners enabled' do + let(:parent) { create(:group, shared_runners_enabled: true) } + let(:group) { build(:group, shared_runners_enabled: true, parent_id: parent.id) } - it 'is valid' do - expect(group).to be_valid + it 'is valid' do + expect(group).to be_valid + end end end end + + it_behaves_like 'validations called by different namespace types', :changing_shared_runners_enabled_is_allowed end describe 'validation #changing_allow_descendants_override_disabled_shared_runners_is_allowed' do - context 'without a parent' do - context 'with shared runners disabled' do - let(:namespace) { build(:namespace, :allow_descendants_override_disabled_shared_runners, :shared_runners_disabled) } + context 'when namespace is a group' do + context 'without a parent' do + context 'with shared runners disabled' do + let(:namespace) { build(:group, :allow_descendants_override_disabled_shared_runners, :shared_runners_disabled) } - it 'is valid' do - expect(namespace).to be_valid + it 'is valid' do + expect(namespace).to be_valid + end end - end - context 'with shared runners enabled' do - let(:namespace) { create(:namespace) } + context 'with shared runners enabled' do + let(:namespace) { create(:namespace) } - it 'is invalid' do - namespace.allow_descendants_override_disabled_shared_runners = true + it 'is invalid' do + namespace.allow_descendants_override_disabled_shared_runners = true - expect(namespace).to be_invalid - expect(namespace.errors[:allow_descendants_override_disabled_shared_runners]).to include('cannot be changed if shared runners are enabled') + expect(namespace).to be_invalid + expect(namespace.errors[:allow_descendants_override_disabled_shared_runners]).to include('cannot be changed if shared runners are enabled') + end end end - end - context 'with a parent' do - context 'when parent does not allow shared runners' do - let(:parent) { create(:group, :shared_runners_disabled) } - let(:group) { build(:group, :shared_runners_disabled, :allow_descendants_override_disabled_shared_runners, parent_id: parent.id) } + context 'with a parent' do + context 'when parent does not allow shared runners' do + let(:parent) { create(:group, :shared_runners_disabled) } + let(:group) { build(:group, :shared_runners_disabled, :allow_descendants_override_disabled_shared_runners, parent_id: parent.id) } - it 'is invalid' do - expect(group).to be_invalid - expect(group.errors[:allow_descendants_override_disabled_shared_runners]).to include('cannot be enabled because parent group does not allow it') + it 'is invalid' do + expect(group).to be_invalid + expect(group.errors[:allow_descendants_override_disabled_shared_runners]).to include('cannot be enabled because parent group does not allow it') + end end - end - context 'when parent allows shared runners and setting to true' do - let(:parent) { create(:group, shared_runners_enabled: true) } - let(:group) { build(:group, :shared_runners_disabled, :allow_descendants_override_disabled_shared_runners, parent_id: parent.id) } + context 'when parent allows shared runners and setting to true' do + let(:parent) { create(:group, shared_runners_enabled: true) } + let(:group) { build(:group, :shared_runners_disabled, :allow_descendants_override_disabled_shared_runners, parent_id: parent.id) } - it 'is valid' do - expect(group).to be_valid + it 'is valid' do + expect(group).to be_valid + end end - end - context 'when parent allows shared runners and setting to false' do - let(:parent) { create(:group, shared_runners_enabled: true) } - let(:group) { build(:group, :shared_runners_disabled, allow_descendants_override_disabled_shared_runners: false, parent_id: parent.id) } + context 'when parent allows shared runners and setting to false' do + let(:parent) { create(:group, shared_runners_enabled: true) } + let(:group) { build(:group, :shared_runners_disabled, allow_descendants_override_disabled_shared_runners: false, parent_id: parent.id) } - it 'is valid' do - expect(group).to be_valid + it 'is valid' do + expect(group).to be_valid + end end end end + + it_behaves_like 'validations called by different namespace types', :changing_allow_descendants_override_disabled_shared_runners_is_allowed end describe '#root?' do diff --git a/spec/models/namespaces/project_namespace_spec.rb b/spec/models/namespaces/project_namespace_spec.rb index f38e8aa85d0..4416c49f1bf 100644 --- a/spec/models/namespaces/project_namespace_spec.rb +++ b/spec/models/namespaces/project_namespace_spec.rb @@ -15,7 +15,7 @@ RSpec.describe Namespaces::ProjectNamespace, type: :model do # using delete rather than destroy due to `delete` skipping AR hooks/callbacks # so it's ensured to work at the DB level. Uses ON DELETE CASCADE on foreign key let_it_be(:project) { create(:project) } - let_it_be(:project_namespace) { create(:project_namespace, project: project) } + let_it_be(:project_namespace) { project.project_namespace } it 'also deletes the associated project' do project_namespace.delete diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index 0dd77967f25..9d9cca0678a 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -155,7 +155,7 @@ RSpec.describe Note do expect(note).to receive(:notify_after_destroy).and_call_original expect(note.noteable).to receive(:after_note_destroyed).with(note) - note.destroy + note.destroy! end it 'does not error if noteable is nil' do @@ -163,7 +163,7 @@ RSpec.describe Note do expect(note).to receive(:notify_after_destroy).and_call_original expect(note).to receive(:noteable).at_least(:once).and_return(nil) - expect { note.destroy }.not_to raise_error + expect { note.destroy! }.not_to raise_error end end end @@ -226,8 +226,8 @@ RSpec.describe Note do describe 'read' do before do - @p1.project_members.create(user: @u2, access_level: ProjectMember::GUEST) - @p2.project_members.create(user: @u3, access_level: ProjectMember::GUEST) + @p1.project_members.create!(user: @u2, access_level: ProjectMember::GUEST) + @p2.project_members.create!(user: @u3, access_level: ProjectMember::GUEST) end it { expect(Ability.allowed?(@u1, :read_note, @p1)).to be_falsey } @@ -237,8 +237,8 @@ RSpec.describe Note do describe 'write' do before do - @p1.project_members.create(user: @u2, access_level: ProjectMember::DEVELOPER) - @p2.project_members.create(user: @u3, access_level: ProjectMember::DEVELOPER) + @p1.project_members.create!(user: @u2, access_level: ProjectMember::DEVELOPER) + @p2.project_members.create!(user: @u3, access_level: ProjectMember::DEVELOPER) end it { expect(Ability.allowed?(@u1, :create_note, @p1)).to be_falsey } @@ -248,9 +248,9 @@ RSpec.describe Note do describe 'admin' do before do - @p1.project_members.create(user: @u1, access_level: ProjectMember::REPORTER) - @p1.project_members.create(user: @u2, access_level: ProjectMember::MAINTAINER) - @p2.project_members.create(user: @u3, access_level: ProjectMember::MAINTAINER) + @p1.project_members.create!(user: @u1, access_level: ProjectMember::REPORTER) + @p1.project_members.create!(user: @u2, access_level: ProjectMember::MAINTAINER) + @p2.project_members.create!(user: @u3, access_level: ProjectMember::MAINTAINER) end it { expect(Ability.allowed?(@u1, :admin_note, @p1)).to be_falsey } @@ -1468,7 +1468,7 @@ RSpec.describe Note do shared_examples 'assignee check' do context 'when the provided user is one of the assignees' do before do - note.noteable.update(assignees: [user, create(:user)]) + note.noteable.update!(assignees: [user, create(:user)]) end it 'returns true' do @@ -1480,7 +1480,7 @@ RSpec.describe Note do shared_examples 'author check' do context 'when the provided user is the author' do before do - note.noteable.update(author: user) + note.noteable.update!(author: user) end it 'returns true' do diff --git a/spec/models/notification_setting_spec.rb b/spec/models/notification_setting_spec.rb index 3f1684327e7..cc601fb30c2 100644 --- a/spec/models/notification_setting_spec.rb +++ b/spec/models/notification_setting_spec.rb @@ -36,7 +36,7 @@ RSpec.describe NotificationSetting do notification_setting.merge_merge_request = "t" notification_setting.close_merge_request = "nil" notification_setting.reopen_merge_request = "false" - notification_setting.save + notification_setting.save! end it "parses boolean before saving" do @@ -52,12 +52,12 @@ RSpec.describe NotificationSetting do context 'notification_email' do let_it_be(:user) { create(:user) } - subject { described_class.new(source_id: 1, source_type: 'Project', user_id: user.id) } + subject { build(:notification_setting, user_id: user.id) } it 'allows to change email to verified one' do email = create(:email, :confirmed, user: user) - subject.update(notification_email: email.email) + subject.notification_email = email.email expect(subject).to be_valid end @@ -65,13 +65,13 @@ RSpec.describe NotificationSetting do it 'does not allow to change email to not verified one' do email = create(:email, user: user) - subject.update(notification_email: email.email) + subject.notification_email = email.email expect(subject).to be_invalid end it 'allows to change email to empty one' do - subject.update(notification_email: '') + subject.notification_email = '' expect(subject).to be_valid end @@ -85,7 +85,7 @@ RSpec.describe NotificationSetting do 1.upto(4) do |i| setting = create(:notification_setting, user: user) - setting.project.update(pending_delete: true) if i.even? + setting.project.update!(pending_delete: true) if i.even? end end diff --git a/spec/models/operations/feature_flags/strategy_spec.rb b/spec/models/operations/feature_flags/strategy_spec.rb index 9289e3beab5..de1b9d2c855 100644 --- a/spec/models/operations/feature_flags/strategy_spec.rb +++ b/spec/models/operations/feature_flags/strategy_spec.rb @@ -20,8 +20,12 @@ RSpec.describe Operations::FeatureFlags::Strategy do end with_them do it 'skips parameters validation' do - strategy = described_class.create(feature_flag: feature_flag, - name: invalid_name, parameters: { bad: 'params' }) + strategy = build(:operations_strategy, + feature_flag: feature_flag, + name: invalid_name, + parameters: { bad: 'params' }) + + expect(strategy).to be_invalid expect(strategy.errors[:name]).to eq(['strategy name is invalid']) expect(strategy.errors[:parameters]).to be_empty @@ -36,19 +40,24 @@ RSpec.describe Operations::FeatureFlags::Strategy do end with_them do it 'must have valid parameters for the strategy' do - strategy = described_class.create(feature_flag: feature_flag, - name: 'gradualRolloutUserId', parameters: invalid_parameters) + strategy = build(:operations_strategy, + :gradual_rollout, + feature_flag: feature_flag, + parameters: invalid_parameters) + + expect(strategy).to be_invalid expect(strategy.errors[:parameters]).to eq(['parameters are invalid']) end end it 'allows the parameters in any order' do - strategy = described_class.create(feature_flag: feature_flag, - name: 'gradualRolloutUserId', - parameters: { percentage: '10', groupId: 'mygroup' }) + strategy = build(:operations_strategy, + :gradual_rollout, + feature_flag: feature_flag, + parameters: { percentage: '10', groupId: 'mygroup' }) - expect(strategy.errors[:parameters]).to be_empty + expect(strategy).to be_valid end describe 'percentage' do @@ -59,9 +68,12 @@ RSpec.describe Operations::FeatureFlags::Strategy do end with_them do it 'must be a string value between 0 and 100 inclusive and without a percentage sign' do - strategy = described_class.create(feature_flag: feature_flag, - name: 'gradualRolloutUserId', - parameters: { groupId: 'mygroup', percentage: invalid_value }) + strategy = build(:operations_strategy, + :gradual_rollout, + feature_flag: feature_flag, + parameters: { groupId: 'mygroup', percentage: invalid_value }) + + expect(strategy).to be_invalid expect(strategy.errors[:parameters]).to eq(['percentage must be a string between 0 and 100 inclusive']) end @@ -72,11 +84,12 @@ RSpec.describe Operations::FeatureFlags::Strategy do end with_them do it 'must be a string value between 0 and 100 inclusive and without a percentage sign' do - strategy = described_class.create(feature_flag: feature_flag, - name: 'gradualRolloutUserId', - parameters: { groupId: 'mygroup', percentage: valid_value }) + strategy = build(:operations_strategy, + :gradual_rollout, + feature_flag: feature_flag, + parameters: { groupId: 'mygroup', percentage: valid_value }) - expect(strategy.errors[:parameters]).to eq([]) + expect(strategy).to be_valid end end end @@ -88,9 +101,12 @@ RSpec.describe Operations::FeatureFlags::Strategy do end with_them do it 'must be a string value of up to 32 lowercase characters' do - strategy = described_class.create(feature_flag: feature_flag, - name: 'gradualRolloutUserId', - parameters: { groupId: invalid_value, percentage: '40' }) + strategy = build(:operations_strategy, + :gradual_rollout, + feature_flag: feature_flag, + parameters: { groupId: invalid_value, percentage: '40' }) + + expect(strategy).to be_invalid expect(strategy.errors[:parameters]).to eq(['groupId parameter is invalid']) end @@ -101,11 +117,12 @@ RSpec.describe Operations::FeatureFlags::Strategy do end with_them do it 'must be a string value of up to 32 lowercase characters' do - strategy = described_class.create(feature_flag: feature_flag, - name: 'gradualRolloutUserId', - parameters: { groupId: valid_value, percentage: '40' }) + strategy = build(:operations_strategy, + :gradual_rollout, + feature_flag: feature_flag, + parameters: { groupId: valid_value, percentage: '40' }) - expect(strategy.errors[:parameters]).to eq([]) + expect(strategy).to be_valid end end end @@ -123,9 +140,12 @@ RSpec.describe Operations::FeatureFlags::Strategy do ]) with_them do it 'must have valid parameters for the strategy' do - strategy = described_class.create(feature_flag: feature_flag, - name: 'flexibleRollout', - parameters: invalid_parameters) + strategy = build(:operations_strategy, + :flexible_rollout, + feature_flag: feature_flag, + parameters: invalid_parameters) + + expect(strategy).to be_invalid expect(strategy.errors[:parameters]).to eq(['parameters are invalid']) end @@ -137,11 +157,12 @@ RSpec.describe Operations::FeatureFlags::Strategy do [:groupId, 'mygroup'] ].permutation(3).each do |parameters| it "allows the parameters in the order #{parameters.map { |p| p.first }.join(', ')}" do - strategy = described_class.create(feature_flag: feature_flag, - name: 'flexibleRollout', - parameters: Hash[parameters]) + strategy = build(:operations_strategy, + :flexible_rollout, + feature_flag: feature_flag, + parameters: Hash[parameters]) - expect(strategy.errors[:parameters]).to be_empty + expect(strategy).to be_valid end end @@ -152,9 +173,12 @@ RSpec.describe Operations::FeatureFlags::Strategy do with_them do it 'must be a string value between 0 and 100 inclusive and without a percentage sign' do parameters = { stickiness: 'default', groupId: 'mygroup', rollout: invalid_value } - strategy = described_class.create(feature_flag: feature_flag, - name: 'flexibleRollout', - parameters: parameters) + strategy = build(:operations_strategy, + :flexible_rollout, + feature_flag: feature_flag, + parameters: parameters) + + expect(strategy).to be_invalid expect(strategy.errors[:parameters]).to eq([ 'rollout must be a string between 0 and 100 inclusive' @@ -166,11 +190,12 @@ RSpec.describe Operations::FeatureFlags::Strategy do with_them do it 'must be a string value between 0 and 100 inclusive and without a percentage sign' do parameters = { stickiness: 'default', groupId: 'mygroup', rollout: valid_value } - strategy = described_class.create(feature_flag: feature_flag, - name: 'flexibleRollout', - parameters: parameters) + strategy = build(:operations_strategy, + :flexible_rollout, + feature_flag: feature_flag, + parameters: parameters) - expect(strategy.errors[:parameters]).to eq([]) + expect(strategy).to be_valid end end end @@ -181,9 +206,12 @@ RSpec.describe Operations::FeatureFlags::Strategy do with_them do it 'must be a string value of up to 32 lowercase characters' do parameters = { stickiness: 'default', groupId: invalid_value, rollout: '40' } - strategy = described_class.create(feature_flag: feature_flag, - name: 'flexibleRollout', - parameters: parameters) + strategy = build(:operations_strategy, + :flexible_rollout, + feature_flag: feature_flag, + parameters: parameters) + + expect(strategy).to be_invalid expect(strategy.errors[:parameters]).to eq(['groupId parameter is invalid']) end @@ -193,11 +221,12 @@ RSpec.describe Operations::FeatureFlags::Strategy do with_them do it 'must be a string value of up to 32 lowercase characters' do parameters = { stickiness: 'default', groupId: valid_value, rollout: '40' } - strategy = described_class.create(feature_flag: feature_flag, - name: 'flexibleRollout', - parameters: parameters) + strategy = build(:operations_strategy, + :flexible_rollout, + feature_flag: feature_flag, + parameters: parameters) - expect(strategy.errors[:parameters]).to eq([]) + expect(strategy).to be_valid end end end @@ -207,9 +236,12 @@ RSpec.describe Operations::FeatureFlags::Strategy do with_them do it 'must be a string representing a supported stickiness setting' do parameters = { stickiness: invalid_value, groupId: 'mygroup', rollout: '40' } - strategy = described_class.create(feature_flag: feature_flag, - name: 'flexibleRollout', - parameters: parameters) + strategy = build(:operations_strategy, + :flexible_rollout, + feature_flag: feature_flag, + parameters: parameters) + + expect(strategy).to be_invalid expect(strategy.errors[:parameters]).to eq([ 'stickiness parameter must be default, userId, sessionId, or random' @@ -221,11 +253,12 @@ RSpec.describe Operations::FeatureFlags::Strategy do with_them do it 'must be a string representing a supported stickiness setting' do parameters = { stickiness: valid_value, groupId: 'mygroup', rollout: '40' } - strategy = described_class.create(feature_flag: feature_flag, - name: 'flexibleRollout', - parameters: parameters) + strategy = build(:operations_strategy, + :flexible_rollout, + feature_flag: feature_flag, + parameters: parameters) - expect(strategy.errors[:parameters]).to eq([]) + expect(strategy).to be_valid end end end @@ -237,8 +270,11 @@ RSpec.describe Operations::FeatureFlags::Strategy do end with_them do it 'must have valid parameters for the strategy' do - strategy = described_class.create(feature_flag: feature_flag, - name: 'userWithId', parameters: invalid_parameters) + strategy = build(:operations_strategy, + feature_flag: feature_flag, + name: 'userWithId', parameters: invalid_parameters) + + expect(strategy).to be_invalid expect(strategy.errors[:parameters]).to eq(['parameters are invalid']) end @@ -253,10 +289,12 @@ RSpec.describe Operations::FeatureFlags::Strategy do end with_them do it 'is valid with a string of comma separated values' do - strategy = described_class.create(feature_flag: feature_flag, - name: 'userWithId', parameters: { userIds: valid_value }) + strategy = build(:operations_strategy, + feature_flag: feature_flag, + name: 'userWithId', + parameters: { userIds: valid_value }) - expect(strategy.errors[:parameters]).to be_empty + expect(strategy).to be_valid end end @@ -267,8 +305,12 @@ RSpec.describe Operations::FeatureFlags::Strategy do end with_them do it 'is invalid' do - strategy = described_class.create(feature_flag: feature_flag, - name: 'userWithId', parameters: { userIds: invalid_value }) + strategy = build(:operations_strategy, + feature_flag: feature_flag, + name: 'userWithId', + parameters: { userIds: invalid_value }) + + expect(strategy).to be_invalid expect(strategy.errors[:parameters]).to include( 'userIds must be a string of unique comma separated values each 256 characters or less' @@ -284,43 +326,48 @@ RSpec.describe Operations::FeatureFlags::Strategy do end with_them do it 'must be empty' do - strategy = described_class.create(feature_flag: feature_flag, - name: 'default', - parameters: invalid_value) + strategy = build(:operations_strategy, :default, feature_flag: feature_flag, parameters: invalid_value) + + expect(strategy).to be_invalid expect(strategy.errors[:parameters]).to eq(['parameters are invalid']) end end it 'must be empty' do - strategy = described_class.create(feature_flag: feature_flag, - name: 'default', - parameters: {}) + strategy = build(:operations_strategy, :default, feature_flag: feature_flag) - expect(strategy.errors[:parameters]).to be_empty + expect(strategy).to be_valid end end context 'when the strategy name is gitlabUserList' do + let_it_be(:user_list) { create(:operations_feature_flag_user_list, project: project) } + where(:invalid_value) do [{ groupId: "default", percentage: "7" }, "", "nothing", 7, nil, [], 2.5, { userIds: 'user1' }] end with_them do - it 'must be empty' do - strategy = described_class.create(feature_flag: feature_flag, - name: 'gitlabUserList', - parameters: invalid_value) + it 'is invalid' do + strategy = build(:operations_strategy, + :gitlab_userlist, + user_list: user_list, + feature_flag: feature_flag, + parameters: invalid_value) + + expect(strategy).to be_invalid expect(strategy.errors[:parameters]).to eq(['parameters are invalid']) end end - it 'must be empty' do - strategy = described_class.create(feature_flag: feature_flag, - name: 'gitlabUserList', - parameters: {}) + it 'is valid' do + strategy = build(:operations_strategy, + :gitlab_userlist, + user_list: user_list, + feature_flag: feature_flag) - expect(strategy.errors[:parameters]).to be_empty + expect(strategy).to be_valid end end end @@ -329,18 +376,15 @@ RSpec.describe Operations::FeatureFlags::Strategy do context 'when name is gitlabUserList' do it 'is valid when associated with a user list' do user_list = create(:operations_feature_flag_user_list, project: project) - strategy = described_class.create(feature_flag: feature_flag, - name: 'gitlabUserList', - user_list: user_list, - parameters: {}) + strategy = build(:operations_strategy, :gitlab_userlist, feature_flag: feature_flag, user_list: user_list) - expect(strategy.errors[:user_list]).to be_empty + expect(strategy).to be_valid end it 'is invalid without a user list' do - strategy = described_class.create(feature_flag: feature_flag, - name: 'gitlabUserList', - parameters: {}) + strategy = build(:operations_strategy, :gitlab_userlist, feature_flag: feature_flag, user_list: nil) + + expect(strategy).to be_invalid expect(strategy.errors[:user_list]).to eq(["can't be blank"]) end @@ -348,10 +392,9 @@ RSpec.describe Operations::FeatureFlags::Strategy do it 'is invalid when associated with a user list from another project' do other_project = create(:project) user_list = create(:operations_feature_flag_user_list, project: other_project) - strategy = described_class.create(feature_flag: feature_flag, - name: 'gitlabUserList', - user_list: user_list, - parameters: {}) + strategy = build(:operations_strategy, :gitlab_userlist, feature_flag: feature_flag, user_list: user_list) + + expect(strategy).to be_invalid expect(strategy.errors[:user_list]).to eq(['must belong to the same project']) end @@ -360,84 +403,68 @@ RSpec.describe Operations::FeatureFlags::Strategy do context 'when name is default' do it 'is invalid when associated with a user list' do user_list = create(:operations_feature_flag_user_list, project: project) - strategy = described_class.create(feature_flag: feature_flag, - name: 'default', - user_list: user_list, - parameters: {}) + strategy = build(:operations_strategy, :default, feature_flag: feature_flag, user_list: user_list) + + expect(strategy).to be_invalid expect(strategy.errors[:user_list]).to eq(['must be blank']) end it 'is valid without a user list' do - strategy = described_class.create(feature_flag: feature_flag, - name: 'default', - parameters: {}) + strategy = build(:operations_strategy, :default, feature_flag: feature_flag) - expect(strategy.errors[:user_list]).to be_empty + expect(strategy).to be_valid end end context 'when name is userWithId' do it 'is invalid when associated with a user list' do user_list = create(:operations_feature_flag_user_list, project: project) - strategy = described_class.create(feature_flag: feature_flag, - name: 'userWithId', - user_list: user_list, - parameters: { userIds: 'user1' }) + strategy = build(:operations_strategy, :userwithid, feature_flag: feature_flag, user_list: user_list) + + expect(strategy).to be_invalid expect(strategy.errors[:user_list]).to eq(['must be blank']) end it 'is valid without a user list' do - strategy = described_class.create(feature_flag: feature_flag, - name: 'userWithId', - parameters: { userIds: 'user1' }) + strategy = build(:operations_strategy, :userwithid, feature_flag: feature_flag) - expect(strategy.errors[:user_list]).to be_empty + expect(strategy).to be_valid end end context 'when name is gradualRolloutUserId' do it 'is invalid when associated with a user list' do user_list = create(:operations_feature_flag_user_list, project: project) - strategy = described_class.create(feature_flag: feature_flag, - name: 'gradualRolloutUserId', - user_list: user_list, - parameters: { groupId: 'default', percentage: '10' }) + strategy = build(:operations_strategy, :gradual_rollout, feature_flag: feature_flag, user_list: user_list) + + expect(strategy).to be_invalid expect(strategy.errors[:user_list]).to eq(['must be blank']) end it 'is valid without a user list' do - strategy = described_class.create(feature_flag: feature_flag, - name: 'gradualRolloutUserId', - parameters: { groupId: 'default', percentage: '10' }) + strategy = build(:operations_strategy, :gradual_rollout, feature_flag: feature_flag) - expect(strategy.errors[:user_list]).to be_empty + expect(strategy).to be_valid end end context 'when name is flexibleRollout' do it 'is invalid when associated with a user list' do user_list = create(:operations_feature_flag_user_list, project: project) - strategy = described_class.create(feature_flag: feature_flag, - name: 'flexibleRollout', - user_list: user_list, - parameters: { groupId: 'default', - rollout: '10', - stickiness: 'default' }) + strategy = build(:operations_strategy, :flexible_rollout, feature_flag: feature_flag, user_list: user_list) + + expect(strategy).to be_invalid expect(strategy.errors[:user_list]).to eq(['must be blank']) end it 'is valid without a user list' do - strategy = described_class.create(feature_flag: feature_flag, - name: 'flexibleRollout', - parameters: { groupId: 'default', - rollout: '10', - stickiness: 'default' }) + strategy = build(:operations_strategy, :flexible_rollout, feature_flag: feature_flag) - expect(strategy.errors[:user_list]).to be_empty + expect(strategy).to be_valid end end end diff --git a/spec/models/operations/feature_flags/user_list_spec.rb b/spec/models/operations/feature_flags/user_list_spec.rb index 3a48d3389a3..b2dbebb2c0d 100644 --- a/spec/models/operations/feature_flags/user_list_spec.rb +++ b/spec/models/operations/feature_flags/user_list_spec.rb @@ -20,9 +20,9 @@ RSpec.describe Operations::FeatureFlags::UserList do end with_them do it 'is valid with a string of comma separated values' do - user_list = described_class.create(user_xids: valid_value) + user_list = build(:operations_feature_flag_user_list, user_xids: valid_value) - expect(user_list.errors[:user_xids]).to be_empty + expect(user_list).to be_valid end end @@ -31,9 +31,10 @@ RSpec.describe Operations::FeatureFlags::UserList do end with_them do it 'automatically casts values of other types' do - user_list = described_class.create(user_xids: typecast_value) + user_list = build(:operations_feature_flag_user_list, user_xids: typecast_value) + + expect(user_list).to be_valid - expect(user_list.errors[:user_xids]).to be_empty expect(user_list.user_xids).to eq(typecast_value.to_s) end end @@ -45,7 +46,9 @@ RSpec.describe Operations::FeatureFlags::UserList do end with_them do it 'is invalid' do - user_list = described_class.create(user_xids: invalid_value) + user_list = build(:operations_feature_flag_user_list, user_xids: invalid_value) + + expect(user_list).to be_invalid expect(user_list.errors[:user_xids]).to include( 'user_xids must be a string of unique comma separated values each 256 characters or less' @@ -70,20 +73,20 @@ RSpec.describe Operations::FeatureFlags::UserList do describe '#destroy' do it 'deletes the model if it is not associated with any feature flag strategies' do project = create(:project) - user_list = described_class.create(project: project, name: 'My User List', user_xids: 'user1,user2') + user_list = described_class.create!(project: project, name: 'My User List', user_xids: 'user1,user2') - user_list.destroy + user_list.destroy! expect(described_class.count).to eq(0) end it 'does not delete the model if it is associated with a feature flag strategy' do project = create(:project) - user_list = described_class.create(project: project, name: 'My User List', user_xids: 'user1,user2') + user_list = described_class.create!(project: project, name: 'My User List', user_xids: 'user1,user2') feature_flag = create(:operations_feature_flag, :new_version_flag, project: project) strategy = create(:operations_strategy, feature_flag: feature_flag, name: 'gitlabUserList', user_list: user_list) - user_list.destroy + user_list.destroy # rubocop:disable Rails/SaveBang expect(described_class.count).to eq(1) expect(::Operations::FeatureFlags::StrategyUserList.count).to eq(1) diff --git a/spec/models/packages/npm/metadatum_spec.rb b/spec/models/packages/npm/metadatum_spec.rb new file mode 100644 index 00000000000..ff8cce5310e --- /dev/null +++ b/spec/models/packages/npm/metadatum_spec.rb @@ -0,0 +1,50 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Packages::Npm::Metadatum, type: :model do + describe 'relationships' do + it { is_expected.to belong_to(:package).inverse_of(:npm_metadatum) } + end + + describe 'validations' do + describe 'package', :aggregate_failures do + it { is_expected.to validate_presence_of(:package) } + + it 'ensure npm package type' do + metadatum = build(:npm_metadatum) + + metadatum.package = build(:nuget_package) + + expect(metadatum).not_to be_valid + expect(metadatum.errors).to contain_exactly('Package type must be NPM') + end + end + + describe 'package_json', :aggregate_failures do + let(:valid_json) { { 'name' => 'foo', 'version' => 'v1.0', 'dist' => { 'tarball' => 'x', 'shasum' => 'x' } } } + + it { is_expected.to allow_value(valid_json).for(:package_json) } + it { is_expected.to allow_value(valid_json.merge('extra-field': { 'foo': 'bar' })).for(:package_json) } + it { is_expected.to allow_value(with_dist { |dist| dist.merge('extra-field': 'x') }).for(:package_json) } + + %w[name version dist].each do |field| + it { is_expected.not_to allow_value(valid_json.except(field)).for(:package_json) } + end + + %w[tarball shasum].each do |field| + it { is_expected.not_to allow_value(with_dist { |dist| dist.except(field) }).for(:package_json) } + end + + it { is_expected.not_to allow_value({}).for(:package_json) } + + it { is_expected.not_to allow_value(test: 'test' * 10000).for(:package_json) } + + def with_dist + valid_json.tap do |h| + h['dist'] = yield(h['dist']) + end + end + end + end +end diff --git a/spec/models/packages/package_file_spec.rb b/spec/models/packages/package_file_spec.rb index 450656e3e9c..8617793f41d 100644 --- a/spec/models/packages/package_file_spec.rb +++ b/spec/models/packages/package_file_spec.rb @@ -14,7 +14,6 @@ RSpec.describe Packages::PackageFile, type: :model do it { is_expected.to belong_to(:package) } it { is_expected.to have_one(:conan_file_metadatum) } it { is_expected.to have_many(:package_file_build_infos).inverse_of(:package_file) } - it { is_expected.to have_many(:pipelines).through(:package_file_build_infos) } it { is_expected.to have_one(:debian_file_metadatum).inverse_of(:package_file).class_name('Packages::Debian::FileMetadatum') } it { is_expected.to have_one(:helm_file_metadatum).inverse_of(:package_file).class_name('Packages::Helm::FileMetadatum') } end @@ -206,6 +205,28 @@ RSpec.describe Packages::PackageFile, type: :model do end end + describe '#pipelines' do + let_it_be_with_refind(:package_file) { create(:package_file) } + + subject { package_file.pipelines } + + context 'package_file without pipeline' do + it { is_expected.to be_empty } + end + + context 'package_file with pipeline' do + let_it_be(:pipeline) { create(:ci_pipeline) } + let_it_be(:pipeline2) { create(:ci_pipeline) } + + before do + package_file.package_file_build_infos.create!(pipeline: pipeline) + package_file.package_file_build_infos.create!(pipeline: pipeline2) + end + + it { is_expected.to contain_exactly(pipeline, pipeline2) } + end + end + describe '#update_file_store callback' do let_it_be(:package_file) { build(:package_file, :nuget, size: nil) } diff --git a/spec/models/packages/package_spec.rb b/spec/models/packages/package_spec.rb index 2573c01d686..6ee5219819c 100644 --- a/spec/models/packages/package_spec.rb +++ b/spec/models/packages/package_spec.rb @@ -14,13 +14,13 @@ RSpec.describe Packages::Package, type: :model do it { is_expected.to have_many(:dependency_links).inverse_of(:package) } it { is_expected.to have_many(:tags).inverse_of(:package) } it { is_expected.to have_many(:build_infos).inverse_of(:package) } - it { is_expected.to have_many(:pipelines).through(:build_infos) } it { is_expected.to have_one(:conan_metadatum).inverse_of(:package) } it { is_expected.to have_one(:maven_metadatum).inverse_of(:package) } it { is_expected.to have_one(:debian_publication).inverse_of(:package).class_name('Packages::Debian::Publication') } it { is_expected.to have_one(:debian_distribution).through(:debian_publication).source(:distribution).inverse_of(:packages).class_name('Packages::Debian::ProjectDistribution') } it { is_expected.to have_one(:nuget_metadatum).inverse_of(:package) } it { is_expected.to have_one(:rubygems_metadatum).inverse_of(:package) } + it { is_expected.to have_one(:npm_metadatum).inverse_of(:package) } end describe '.with_debian_codename' do @@ -999,6 +999,28 @@ RSpec.describe Packages::Package, type: :model do end end + describe '#pipelines' do + let_it_be_with_refind(:package) { create(:maven_package) } + + subject { package.pipelines } + + context 'package without pipeline' do + it { is_expected.to be_empty } + end + + context 'package with pipeline' do + let_it_be(:pipeline) { create(:ci_pipeline) } + let_it_be(:pipeline2) { create(:ci_pipeline) } + + before do + package.build_infos.create!(pipeline: pipeline) + package.build_infos.create!(pipeline: pipeline2) + end + + it { is_expected.to contain_exactly(pipeline, pipeline2) } + end + end + describe '#tag_names' do let_it_be(:package) { create(:nuget_package) } diff --git a/spec/models/pages_domain_spec.rb b/spec/models/pages_domain_spec.rb index 2b6ed9a9927..d476e18a72c 100644 --- a/spec/models/pages_domain_spec.rb +++ b/spec/models/pages_domain_spec.rb @@ -104,7 +104,7 @@ RSpec.describe PagesDomain do let(:domain) { build(:pages_domain) } it 'saves validity time' do - domain.save + domain.save! expect(domain.certificate_valid_not_before).to be_like_time(Time.zone.parse("2020-03-16 14:20:34 UTC")) expect(domain.certificate_valid_not_after).to be_like_time(Time.zone.parse("2220-01-28 14:20:34 UTC")) @@ -161,7 +161,7 @@ RSpec.describe PagesDomain do context 'when certificate is already saved' do it "doesn't add error to certificate" do - domain.save(validate: false) + domain.save!(validate: false) domain.valid? diff --git a/spec/models/preloaders/group_policy_preloader_spec.rb b/spec/models/preloaders/group_policy_preloader_spec.rb new file mode 100644 index 00000000000..f6e40d1f033 --- /dev/null +++ b/spec/models/preloaders/group_policy_preloader_spec.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Preloaders::GroupPolicyPreloader do + let_it_be(:user) { create(:user) } + let_it_be(:root_parent) { create(:group, :private, name: 'root-1', path: 'root-1') } + let_it_be(:guest_group) { create(:group, name: 'public guest', path: 'public-guest') } + let_it_be(:private_maintainer_group) { create(:group, :private, name: 'b private maintainer', path: 'b-private-maintainer', parent: root_parent) } + let_it_be(:private_developer_group) { create(:group, :private, project_creation_level: nil, name: 'c public developer', path: 'c-public-developer') } + let_it_be(:public_maintainer_group) { create(:group, :private, name: 'a public maintainer', path: 'a-public-maintainer') } + + let(:base_groups) { [guest_group, private_maintainer_group, private_developer_group, public_maintainer_group] } + + before_all do + guest_group.add_guest(user) + private_maintainer_group.add_maintainer(user) + private_developer_group.add_developer(user) + public_maintainer_group.add_maintainer(user) + end + + it 'avoids N+1 queries when authorizing a list of groups', :request_store do + preload_groups_for_policy(user) + control = ActiveRecord::QueryRecorder.new { authorize_all_groups(user) } + + new_group1 = create(:group, :private).tap { |group| group.add_maintainer(user) } + new_group2 = create(:group, :private, parent: private_maintainer_group) + + another_root = create(:group, :private, name: 'root-3', path: 'root-3') + new_group3 = create(:group, :private, parent: another_root).tap { |group| group.add_maintainer(user) } + + pristine_groups = Group.where(id: base_groups + [new_group1, new_group2, new_group3]) + + preload_groups_for_policy(user, pristine_groups) + expect { authorize_all_groups(user, pristine_groups) }.not_to exceed_query_limit(control) + end + + def authorize_all_groups(current_user, group_list = base_groups) + group_list.each { |group| current_user.can?(:read_group, group) } + end + + def preload_groups_for_policy(current_user, group_list = base_groups) + described_class.new(group_list, current_user).execute + end +end diff --git a/spec/models/preloaders/group_root_ancestor_preloader_spec.rb b/spec/models/preloaders/group_root_ancestor_preloader_spec.rb new file mode 100644 index 00000000000..0d622e84ef1 --- /dev/null +++ b/spec/models/preloaders/group_root_ancestor_preloader_spec.rb @@ -0,0 +1,63 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Preloaders::GroupRootAncestorPreloader do + let_it_be(:user) { create(:user) } + let_it_be(:root_parent1) { create(:group, :private, name: 'root-1', path: 'root-1') } + let_it_be(:root_parent2) { create(:group, :private, name: 'root-2', path: 'root-2') } + let_it_be(:guest_group) { create(:group, name: 'public guest', path: 'public-guest') } + let_it_be(:private_maintainer_group) { create(:group, :private, name: 'b private maintainer', path: 'b-private-maintainer', parent: root_parent1) } + let_it_be(:private_developer_group) { create(:group, :private, project_creation_level: nil, name: 'c public developer', path: 'c-public-developer') } + let_it_be(:public_maintainer_group) { create(:group, :private, name: 'a public maintainer', path: 'a-public-maintainer', parent: root_parent2) } + + let(:root_query_regex) { /\ASELECT.+FROM "namespaces" WHERE "namespaces"."id" = \d+/ } + let(:additional_preloads) { [] } + let(:groups) { [guest_group, private_maintainer_group, private_developer_group, public_maintainer_group] } + let(:pristine_groups) { Group.where(id: groups) } + + shared_examples 'executes N matching DB queries' do |expected_query_count, query_method = nil| + it 'executes the specified root_ancestor queries' do + expect do + pristine_groups.each do |group| + root_ancestor = group.root_ancestor + + root_ancestor.public_send(query_method) if query_method.present? + end + end.to make_queries_matching(root_query_regex, expected_query_count) + end + + it 'strong_memoizes the correct root_ancestor' do + pristine_groups.each do |group| + expected_parent_id = group.root_ancestor.id == group.id ? nil : group.root_ancestor.id + + expect(group.parent_id).to eq(expected_parent_id) + end + end + end + + context 'when the preloader is used' do + before do + preload_ancestors + end + + context 'when no additional preloads are provided' do + it_behaves_like 'executes N matching DB queries', 0 + end + + context 'when additional preloads are provided' do + let(:additional_preloads) { [:route] } + let(:root_query_regex) { /\ASELECT.+FROM "routes" WHERE "routes"."source_id" = \d+/ } + + it_behaves_like 'executes N matching DB queries', 0, :full_path + end + end + + context 'when the preloader is not used' do + it_behaves_like 'executes N matching DB queries', 2 + end + + def preload_ancestors + described_class.new(pristine_groups, additional_preloads).execute + end +end diff --git a/spec/models/preloaders/user_max_access_level_in_groups_preloader_spec.rb b/spec/models/preloaders/user_max_access_level_in_groups_preloader_spec.rb index 8144e1ad233..5fc7bfb1f62 100644 --- a/spec/models/preloaders/user_max_access_level_in_groups_preloader_spec.rb +++ b/spec/models/preloaders/user_max_access_level_in_groups_preloader_spec.rb @@ -13,32 +13,47 @@ RSpec.describe Preloaders::UserMaxAccessLevelInGroupsPreloader do shared_examples 'executes N max member permission queries to the DB' do it 'executes the specified max membership queries' do - queries = ActiveRecord::QueryRecorder.new do - groups.each { |group| user.can?(:read_group, group) } - end + expect { groups.each { |group| user.can?(:read_group, group) } }.to make_queries_matching(max_query_regex, expected_query_count) + end - max_queries = queries.log.grep(max_query_regex) + it 'caches the correct access_level for each group' do + groups.each do |group| + access_level_from_db = group.members_with_parents.where(user_id: user.id).group(:user_id).maximum(:access_level)[user.id] || Gitlab::Access::NO_ACCESS + cached_access_level = group.max_member_access_for_user(user) - expect(max_queries.count).to eq(expected_query_count) + expect(cached_access_level).to eq(access_level_from_db) + end end end context 'when the preloader is used', :request_store do - before do - described_class.new(groups, user).execute - end + context 'when user has indirect access to groups' do + let_it_be(:child_maintainer) { create(:group, :private, parent: group1).tap {|g| g.add_maintainer(user)} } + let_it_be(:child_indirect_access) { create(:group, :private, parent: group1) } - it_behaves_like 'executes N max member permission queries to the DB' do - # Will query all groups where the user is not already a member - let(:expected_query_count) { 1 } - end + let(:groups) { [group1, group2, group3, child_maintainer, child_indirect_access] } + + context 'when traversal_ids feature flag is disabled' do + it_behaves_like 'executes N max member permission queries to the DB' do + before do + stub_feature_flags(use_traversal_ids: false) + described_class.new(groups, user).execute + end + + # One query for group with no access and another one per group where the user is not a direct member + let(:expected_query_count) { 2 } + end + end - context 'when user has access but is not a direct member of the group' do - let(:groups) { [group1, group2, group3, create(:group, :private, parent: group1)] } + context 'when traversal_ids feature flag is enabled' do + it_behaves_like 'executes N max member permission queries to the DB' do + before do + stub_feature_flags(use_traversal_ids: true) + described_class.new(groups, user).execute + end - it_behaves_like 'executes N max member permission queries to the DB' do - # One query for group with no access and another one where the user is not a direct member - let(:expected_query_count) { 2 } + let(:expected_query_count) { 0 } + end end end end diff --git a/spec/models/project_authorization_spec.rb b/spec/models/project_authorization_spec.rb index c517fc8be55..58c0ff48b46 100644 --- a/spec/models/project_authorization_spec.rb +++ b/spec/models/project_authorization_spec.rb @@ -3,9 +3,10 @@ require 'spec_helper' RSpec.describe ProjectAuthorization do - let(:user) { create(:user) } - let(:project1) { create(:project) } - let(:project2) { create(:project) } + let_it_be(:user) { create(:user) } + let_it_be(:project1) { create(:project) } + let_it_be(:project2) { create(:project) } + let_it_be(:project3) { create(:project) } describe '.insert_authorizations' do it 'inserts the authorizations' do @@ -23,5 +24,19 @@ RSpec.describe ProjectAuthorization do expect(user.project_authorizations.count).to eq(2) end + + it 'skips duplicates and inserts the remaining rows without error' do + create(:project_authorization, user: user, project: project1, access_level: Gitlab::Access::MAINTAINER) + + rows = [ + [user.id, project1.id, Gitlab::Access::MAINTAINER], + [user.id, project2.id, Gitlab::Access::MAINTAINER], + [user.id, project3.id, Gitlab::Access::MAINTAINER] + ] + + described_class.insert_authorizations(rows) + + expect(user.project_authorizations.pluck(:user_id, :project_id, :access_level)).to match_array(rows) + end end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 2e5c5af4eb0..3a8768ff463 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -16,7 +16,7 @@ RSpec.describe Project, factory_default: :keep do describe 'associations' do it { is_expected.to belong_to(:group) } it { is_expected.to belong_to(:namespace) } - it { is_expected.to belong_to(:project_namespace).class_name('Namespaces::ProjectNamespace').with_foreign_key('project_namespace_id').inverse_of(:project) } + it { is_expected.to belong_to(:project_namespace).class_name('Namespaces::ProjectNamespace').with_foreign_key('project_namespace_id') } 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) } @@ -191,7 +191,7 @@ RSpec.describe Project, factory_default: :keep do # using delete rather than destroy due to `delete` skipping AR hooks/callbacks # so it's ensured to work at the DB level. Uses AFTER DELETE trigger. let_it_be(:project) { create(:project) } - let_it_be(:project_namespace) { create(:project_namespace, project: project) } + let_it_be(:project_namespace) { project.project_namespace } it 'also deletes the associated ProjectNamespace' do project.delete @@ -233,6 +233,58 @@ RSpec.describe Project, factory_default: :keep do expect(project.project_setting).to be_an_instance_of(ProjectSetting) expect(project.project_setting).to be_new_record end + + context 'with project namespaces' do + it 'automatically creates a project namespace' do + project = build(:project, path: 'hopefully-valid-path1') + project.save! + + expect(project).to be_persisted + expect(project.project_namespace).to be_persisted + expect(project.project_namespace).to be_in_sync_with_project(project) + end + + context 'with FF disabled' do + before do + stub_feature_flags(create_project_namespace_on_project_create: false) + end + + it 'does not create a project namespace' do + project = build(:project, path: 'hopefully-valid-path2') + project.save! + + expect(project).to be_persisted + expect(project.project_namespace).to be_nil + end + end + end + end + + context 'updating a project' do + context 'with project namespaces' do + it 'keeps project namespace in sync with project' do + project = create(:project) + project.update!(path: 'hopefully-valid-path1') + + expect(project).to be_persisted + expect(project.project_namespace).to be_persisted + expect(project.project_namespace).to be_in_sync_with_project(project) + end + + context 'with FF disabled' do + before do + stub_feature_flags(create_project_namespace_on_project_create: false) + end + + it 'does not create a project namespace when project is updated' do + project = create(:project) + project.update!(path: 'hopefully-valid-path1') + + expect(project).to be_persisted + expect(project.project_namespace).to be_nil + end + end + end end context 'updating cd_cd_settings' do @@ -294,6 +346,7 @@ RSpec.describe Project, factory_default: :keep do it { is_expected.to validate_presence_of(:namespace) } it { is_expected.to validate_presence_of(:repository_storage) } it { is_expected.to validate_numericality_of(:max_artifacts_size).only_integer.is_greater_than(0) } + it { is_expected.to validate_length_of(:suggestion_commit_message).is_at_most(255) } it 'validates build timeout constraints' do is_expected.to validate_numericality_of(:build_timeout) @@ -322,6 +375,18 @@ RSpec.describe Project, factory_default: :keep do create(:project) end + context 'validates project namespace creation' do + it 'does not create project namespace if project is not created' do + project = build(:project, path: 'tree') + + project.valid? + + expect(project).not_to be_valid + expect(project).to be_new_record + expect(project.project_namespace).to be_new_record + end + end + context 'repository storages inclusion' do let(:project2) { build(:project, repository_storage: 'missing') } @@ -424,8 +489,9 @@ RSpec.describe Project, factory_default: :keep do end include_context 'invalid urls' + include_context 'valid urls with CRLF' - it 'does not allow urls with CR or LF characters' do + it 'does not allow URLs with unencoded CR or LF characters' do project = build(:project) aggregate_failures do @@ -437,6 +503,19 @@ RSpec.describe Project, factory_default: :keep do end end end + + it 'allow URLs with CR or LF characters' do + project = build(:project) + + aggregate_failures do + valid_urls_with_CRLF.each do |url| + project.import_url = url + + expect(project).to be_valid + expect(project.errors).to be_empty + end + end + end end describe 'project pending deletion' do @@ -1714,13 +1793,19 @@ RSpec.describe Project, factory_default: :keep do allow(::Gitlab::ServiceDeskEmail).to receive(:config).and_return(config) end - it 'returns custom address when project_key is set' do - create(:service_desk_setting, project: project, project_key: 'key1') + context 'when project_key is set' do + it 'returns custom address including the project_key' do + create(:service_desk_setting, project: project, project_key: 'key1') - expect(subject).to eq("foo+#{project.full_path_slug}-key1@bar.com") + expect(subject).to eq("foo+#{project.full_path_slug}-key1@bar.com") + end end - it_behaves_like 'with incoming email address' + context 'when project_key is not set' do + it 'returns custom address including the project full path' do + expect(subject).to eq("foo+#{project.full_path_slug}-#{project.project_id}-issue-@bar.com") + end + end end end @@ -1780,6 +1865,20 @@ RSpec.describe Project, factory_default: :keep do end end + describe '.without_integration' do + it 'returns projects without the integration' do + project_1, project_2, project_3, project_4 = create_list(:project, 4) + instance_integration = create(:jira_integration, :instance) + create(:jira_integration, project: project_1, inherit_from_id: instance_integration.id) + create(:jira_integration, project: project_2, inherit_from_id: nil) + create(:jira_integration, group: create(:group), project: nil, inherit_from_id: nil) + create(:jira_integration, project: project_3, inherit_from_id: nil) + create(:integrations_slack, project: project_4, inherit_from_id: nil) + + expect(Project.without_integration(instance_integration)).to contain_exactly(project_4) + end + end + context 'repository storage by default' do let(:project) { build(:project) } diff --git a/spec/models/project_statistics_spec.rb b/spec/models/project_statistics_spec.rb index ead6238b2f4..5fbf1a9c502 100644 --- a/spec/models/project_statistics_spec.rb +++ b/spec/models/project_statistics_spec.rb @@ -325,12 +325,14 @@ RSpec.describe ProjectStatistics do lfs_objects_size: 3, snippets_size: 2, pipeline_artifacts_size: 3, + build_artifacts_size: 3, + packages_size: 6, uploads_size: 5 ) statistics.reload - expect(statistics.storage_size).to eq 19 + expect(statistics.storage_size).to eq 28 end it 'works during wiki_size backfill' do diff --git a/spec/models/project_team_spec.rb b/spec/models/project_team_spec.rb index 8eab50abd8c..a6a56180ce1 100644 --- a/spec/models/project_team_spec.rb +++ b/spec/models/project_team_spec.rb @@ -234,6 +234,20 @@ RSpec.describe ProjectTeam do expect(project.team.reporter?(user1)).to be(true) expect(project.team.reporter?(user2)).to be(true) end + + context 'when `tasks_to_be_done` and `tasks_project_id` are passed' do + before do + stub_experiments(invite_members_for_task: true) + project.team.add_users([user1], :developer, tasks_to_be_done: %w(ci code), tasks_project_id: project.id) + end + + it 'creates a member_task with the correct attributes', :aggregate_failures do + member = project.project_members.last + + expect(member.tasks_to_be_done).to match_array([:ci, :code]) + expect(member.member_task.project).to eq(project) + end + end end describe '#add_user' do diff --git a/spec/models/protectable_dropdown_spec.rb b/spec/models/protectable_dropdown_spec.rb index 918c3078405..ab3f455fe63 100644 --- a/spec/models/protectable_dropdown_spec.rb +++ b/spec/models/protectable_dropdown_spec.rb @@ -16,7 +16,7 @@ RSpec.describe ProtectableDropdown do describe '#protectable_ref_names' do context 'when project repository is not empty' do before do - project.protected_branches.create(name: 'master') + create(:protected_branch, project: project, name: 'master') end it { expect(subject.protectable_ref_names).to include('feature') } diff --git a/spec/models/redirect_route_spec.rb b/spec/models/redirect_route_spec.rb index c6e35923b89..2de662fd4b4 100644 --- a/spec/models/redirect_route_spec.rb +++ b/spec/models/redirect_route_spec.rb @@ -4,7 +4,7 @@ require 'spec_helper' RSpec.describe RedirectRoute do let(:group) { create(:group) } - let!(:redirect_route) { group.redirect_routes.create(path: 'gitlabb') } + let!(:redirect_route) { group.redirect_routes.create!(path: 'gitlabb') } describe 'relationships' do it { is_expected.to belong_to(:source) } @@ -17,10 +17,10 @@ RSpec.describe RedirectRoute do end describe '.matching_path_and_descendants' do - let!(:redirect2) { group.redirect_routes.create(path: 'gitlabb/test') } - let!(:redirect3) { group.redirect_routes.create(path: 'gitlabb/test/foo') } - let!(:redirect4) { group.redirect_routes.create(path: 'gitlabb/test/foo/bar') } - let!(:redirect5) { group.redirect_routes.create(path: 'gitlabb/test/baz') } + let!(:redirect2) { group.redirect_routes.create!(path: 'gitlabb/test') } + let!(:redirect3) { group.redirect_routes.create!(path: 'gitlabb/test/foo') } + let!(:redirect4) { group.redirect_routes.create!(path: 'gitlabb/test/foo/bar') } + let!(:redirect5) { group.redirect_routes.create!(path: 'gitlabb/test/baz') } context 'when the redirect route matches with same casing' do it 'returns correct routes' do diff --git a/spec/models/release_spec.rb b/spec/models/release_spec.rb index b88813b3328..125fec61d72 100644 --- a/spec/models/release_spec.rb +++ b/spec/models/release_spec.rb @@ -26,10 +26,10 @@ RSpec.describe Release do context 'when a release exists in the database without a name' do it 'does not require name' do existing_release_without_name = build(:release, project: project, author: user, name: nil) - existing_release_without_name.save(validate: false) + existing_release_without_name.save!(validate: false) existing_release_without_name.description = "change" - existing_release_without_name.save + existing_release_without_name.save! existing_release_without_name.reload expect(existing_release_without_name).to be_valid @@ -88,7 +88,7 @@ RSpec.describe Release do describe '.create' do it "fills released_at using created_at if it's not set" do - release = described_class.create(project: project, author: user) + release = create(:release, project: project, author: user, released_at: nil) expect(release.released_at).to eq(release.created_at) end @@ -96,14 +96,14 @@ RSpec.describe Release do it "does not change released_at if it's set explicitly" do released_at = Time.zone.parse('2018-10-20T18:00:00Z') - release = described_class.create(project: project, author: user, released_at: released_at) + release = create(:release, project: project, author: user, released_at: released_at) expect(release.released_at).to eq(released_at) end end describe '#update' do - subject { release.update(params) } + subject { release.update!(params) } context 'when links do not exist' do context 'when params are specified for creation' do @@ -182,7 +182,7 @@ RSpec.describe Release do it 'also deletes the associated evidence' do release_with_evidence - expect { release_with_evidence.destroy }.to change(Releases::Evidence, :count).by(-1) + expect { release_with_evidence.destroy! }.to change(Releases::Evidence, :count).by(-1) end end end @@ -190,7 +190,7 @@ RSpec.describe Release do describe '#name' do context 'name is nil' do before do - release.update(name: nil) + release.update!(name: nil) end it 'returns tag' do diff --git a/spec/models/remote_mirror_spec.rb b/spec/models/remote_mirror_spec.rb index 382359ccb17..9f1d1c84da3 100644 --- a/spec/models/remote_mirror_spec.rb +++ b/spec/models/remote_mirror_spec.rb @@ -289,7 +289,7 @@ RSpec.describe RemoteMirror, :mailer do context 'with remote mirroring disabled' do it 'returns nil' do - remote_mirror.update(enabled: false) + remote_mirror.update!(enabled: false) expect(remote_mirror.sync).to be_nil end @@ -354,7 +354,7 @@ RSpec.describe RemoteMirror, :mailer do let(:remote_mirror) { create(:project, :repository, :remote_mirror).remote_mirrors.first } it 'resets all the columns when URL changes' do - remote_mirror.update(last_error: Time.current, + remote_mirror.update!(last_error: Time.current, last_update_at: Time.current, last_successful_update_at: Time.current, update_status: 'started', @@ -378,7 +378,7 @@ RSpec.describe RemoteMirror, :mailer do end before do - remote_mirror.update(last_update_started_at: Time.current) + remote_mirror.update!(last_update_started_at: Time.current) end context 'when remote mirror does not have status failed' do @@ -393,7 +393,7 @@ RSpec.describe RemoteMirror, :mailer do context 'when remote mirror has status failed' do it 'returns false when last update started after the timestamp' do - remote_mirror.update(update_status: 'failed') + remote_mirror.update!(update_status: 'failed') expect(remote_mirror.updated_since?(timestamp)).to be false end @@ -409,7 +409,7 @@ RSpec.describe RemoteMirror, :mailer do updated_at: 25.hours.ago) project = mirror.project project.pending_delete = true - project.save + project.save! mirror.reload expect(mirror.sync).to be_nil diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 7bad907cf90..d50c60774b4 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -66,35 +66,58 @@ RSpec.describe Repository do it { is_expected.not_to include('v1.0.0') } end - describe 'tags_sorted_by' do + describe '#tags_sorted_by' do let(:tags_to_compare) { %w[v1.0.0 v1.1.0] } - let(:feature_flag) { true } - - before do - stub_feature_flags(tags_finder_gitaly: feature_flag) - end context 'name_desc' do subject { repository.tags_sorted_by('name_desc').map(&:name) & tags_to_compare } it { is_expected.to eq(['v1.1.0', 'v1.0.0']) } - - context 'when feature flag is disabled' do - let(:feature_flag) { false } - - it { is_expected.to eq(['v1.1.0', 'v1.0.0']) } - end end context 'name_asc' do - subject { repository.tags_sorted_by('name_asc').map(&:name) & tags_to_compare } + subject { repository.tags_sorted_by('name_asc', pagination_params).map(&:name) & tags_to_compare } + + let(:pagination_params) { nil } it { is_expected.to eq(['v1.0.0', 'v1.1.0']) } - context 'when feature flag is disabled' do - let(:feature_flag) { false } + context 'with pagination' do + context 'with limit' do + let(:pagination_params) { { limit: 1 } } + + it { is_expected.to eq(['v1.0.0']) } + end + + context 'with page token and limit' do + let(:pagination_params) { { page_token: 'refs/tags/v1.0.0', limit: 1 } } + + it { is_expected.to eq(['v1.1.0']) } + end - it { is_expected.to eq(['v1.0.0', 'v1.1.0']) } + context 'with page token only' do + let(:pagination_params) { { page_token: 'refs/tags/v1.0.0' } } + + it 'raises an ArgumentError' do + expect { subject }.to raise_error(ArgumentError) + end + end + + context 'with negative limit' do + let(:pagination_params) { { limit: -1 } } + + it 'returns all tags' do + is_expected.to eq(['v1.0.0', 'v1.1.0']) + end + end + + context 'with unknown token' do + let(:pagination_params) { { page_token: 'unknown' } } + + it 'raises an ArgumentError' do + expect { subject }.to raise_error(ArgumentError) + end + end end end @@ -113,24 +136,12 @@ RSpec.describe Repository do subject { repository.tags_sorted_by('updated_desc').map(&:name) & (tags_to_compare + [latest_tag]) } it { is_expected.to eq([latest_tag, 'v1.1.0', 'v1.0.0']) } - - context 'when feature flag is disabled' do - let(:feature_flag) { false } - - it { is_expected.to eq([latest_tag, 'v1.1.0', 'v1.0.0']) } - end end context 'asc' do subject { repository.tags_sorted_by('updated_asc').map(&:name) & (tags_to_compare + [latest_tag]) } it { is_expected.to eq(['v1.0.0', 'v1.1.0', latest_tag]) } - - context 'when feature flag is disabled' do - let(:feature_flag) { false } - - it { is_expected.to eq(['v1.0.0', 'v1.1.0', latest_tag]) } - end end context 'annotated tag pointing to a blob' do @@ -147,12 +158,6 @@ RSpec.describe Repository do it { is_expected.to eq(['v1.0.0', 'v1.1.0', annotated_tag_name]) } - context 'when feature flag is disabled' do - let(:feature_flag) { false } - - it { is_expected.to eq(['v1.0.0', 'v1.1.0', annotated_tag_name]) } - end - after do rugged_repo(repository).tags.delete(annotated_tag_name) end @@ -163,12 +168,6 @@ RSpec.describe Repository do subject { repository.tags_sorted_by('unknown_desc').map(&:name) & tags_to_compare } it { is_expected.to eq(['v1.0.0', 'v1.1.0']) } - - context 'when feature flag is disabled' do - let(:feature_flag) { false } - - it { is_expected.to eq(['v1.0.0', 'v1.1.0']) } - end end end diff --git a/spec/models/route_spec.rb b/spec/models/route_spec.rb index eb81db95cd3..b2fa9c24535 100644 --- a/spec/models/route_spec.rb +++ b/spec/models/route_spec.rb @@ -31,18 +31,18 @@ RSpec.describe Route do context 'after update' do it 'calls #create_redirect_for_old_path' do expect(route).to receive(:create_redirect_for_old_path) - route.update(path: 'foo') + route.update!(path: 'foo') end it 'calls #delete_conflicting_redirects' do expect(route).to receive(:delete_conflicting_redirects) - route.update(path: 'foo') + route.update!(path: 'foo') end end context 'after create' do it 'calls #delete_conflicting_redirects' do - route.destroy + route.destroy! new_route = described_class.new(source: group, path: group.path) expect(new_route).to receive(:delete_conflicting_redirects) new_route.save! @@ -81,7 +81,7 @@ RSpec.describe Route do context 'path update' do context 'when route name is set' do before do - route.update(path: 'bar') + route.update!(path: 'bar') end it 'updates children routes with new path' do @@ -111,7 +111,7 @@ RSpec.describe Route do let!(:conflicting_redirect3) { route.create_redirect('gitlab-org') } it 'deletes the conflicting redirects' do - route.update(path: 'bar') + route.update!(path: 'bar') expect(RedirectRoute.exists?(path: 'bar/test')).to be_falsey expect(RedirectRoute.exists?(path: 'bar/test/foo')).to be_falsey @@ -122,7 +122,7 @@ RSpec.describe Route do context 'name update' do it 'updates children routes with new path' do - route.update(name: 'bar') + route.update!(name: 'bar') expect(described_class.exists?(name: 'bar')).to be_truthy expect(described_class.exists?(name: 'bar / test')).to be_truthy @@ -134,7 +134,7 @@ RSpec.describe Route do # Note: using `update_columns` to skip all validation and callbacks route.update_columns(name: nil) - expect { route.update(name: 'bar') } + expect { route.update!(name: 'bar') } .to change { route.name }.from(nil).to('bar') end end diff --git a/spec/models/sentry_issue_spec.rb b/spec/models/sentry_issue_spec.rb index c24350d7067..09b23b6fd0d 100644 --- a/spec/models/sentry_issue_spec.rb +++ b/spec/models/sentry_issue_spec.rb @@ -53,7 +53,7 @@ RSpec.describe SentryIssue do create(:sentry_issue) project = sentry_issue.issue.project sentry_issue_3 = build(:sentry_issue, issue: create(:issue, project: project), sentry_issue_identifier: sentry_issue.sentry_issue_identifier) - sentry_issue_3.save(validate: false) + sentry_issue_3.save!(validate: false) result = described_class.for_project_and_identifier(project, sentry_issue.sentry_issue_identifier) diff --git a/spec/models/snippet_spec.rb b/spec/models/snippet_spec.rb index 4e20a83f18e..e24dd910c39 100644 --- a/spec/models/snippet_spec.rb +++ b/spec/models/snippet_spec.rb @@ -98,7 +98,7 @@ RSpec.describe Snippet do snippet = build(:snippet) expect(snippet.statistics).to be_nil - snippet.save + snippet.save! expect(snippet.statistics).to be_persisted end @@ -289,7 +289,7 @@ RSpec.describe Snippet do let(:access_level) { ProjectFeature::ENABLED } before do - project.project_feature.update(snippets_access_level: access_level) + project.project_feature.update!(snippets_access_level: access_level) end it 'includes snippets for projects with snippets enabled' do @@ -623,7 +623,7 @@ RSpec.describe Snippet do context 'when snippet_repository does not exist' do it 'creates a snippet_repository' do - snippet.snippet_repository.destroy + snippet.snippet_repository.destroy! snippet.reload expect do diff --git a/spec/models/suggestion_spec.rb b/spec/models/suggestion_spec.rb index 9a7624c253a..4f91908264f 100644 --- a/spec/models/suggestion_spec.rb +++ b/spec/models/suggestion_spec.rb @@ -154,6 +154,14 @@ RSpec.describe Suggestion do it { is_expected.to eq("This suggestion already matches its content.") } end + context 'when file is .ipynb' do + before do + allow(suggestion).to receive(:file_path).and_return("example.ipynb") + end + + it { is_expected.to eq(_("This file was modified for readability, and can't accept suggestions. Edit it directly.")) } + end + context 'when applicable' do it { is_expected.to be_nil } end diff --git a/spec/models/u2f_registration_spec.rb b/spec/models/u2f_registration_spec.rb index aba2f27d104..7a70cf69566 100644 --- a/spec/models/u2f_registration_spec.rb +++ b/spec/models/u2f_registration_spec.rb @@ -5,9 +5,11 @@ require 'spec_helper' RSpec.describe U2fRegistration do let_it_be(:user) { create(:user) } + let(:u2f_registration_name) { 'u2f_device' } + let(:u2f_registration) do device = U2F::FakeU2F.new(FFaker::BaconIpsum.characters(5)) - create(:u2f_registration, name: 'u2f_device', + create(:u2f_registration, name: u2f_registration_name, user: user, certificate: Base64.strict_encode64(device.cert_raw), key_handle: U2F.urlsafe_encode64(device.key_handle_raw), @@ -16,11 +18,27 @@ RSpec.describe U2fRegistration do describe 'callbacks' do describe '#create_webauthn_registration' do - it 'creates webauthn registration' do - u2f_registration.save! + shared_examples_for 'creates webauthn registration' do + it 'creates webauthn registration' do + u2f_registration.save! + + webauthn_registration = WebauthnRegistration.where(u2f_registration_id: u2f_registration.id) + expect(webauthn_registration).to exist + end + end + + it_behaves_like 'creates webauthn registration' + + context 'when the u2f_registration has a blank name' do + let(:u2f_registration_name) { '' } + + it_behaves_like 'creates webauthn registration' + end + + context 'when the u2f_registration has the name as `nil`' do + let(:u2f_registration_name) { nil } - webauthn_registration = WebauthnRegistration.where(u2f_registration_id: u2f_registration.id) - expect(webauthn_registration).to exist + it_behaves_like 'creates webauthn registration' end it 'logs error' do diff --git a/spec/models/upload_spec.rb b/spec/models/upload_spec.rb index 0ac684cd04c..cdf73b203af 100644 --- a/spec/models/upload_spec.rb +++ b/spec/models/upload_spec.rb @@ -19,7 +19,7 @@ RSpec.describe Upload do it 'schedules checksum calculation' do stub_const('UploadChecksumWorker', spy) - upload = described_class.create( + upload = described_class.create!( path: __FILE__, size: described_class::CHECKSUM_THRESHOLD + 1.kilobyte, model: build_stubbed(:user), @@ -42,7 +42,7 @@ RSpec.describe Upload do store: ObjectStorage::Store::LOCAL ) - expect { upload.save } + expect { upload.save! } .to change { upload.checksum }.from(nil) .to(a_string_matching(/\A\h{64}\z/)) end @@ -55,7 +55,7 @@ RSpec.describe Upload do it 'calls delete_file!' do is_expected.to receive(:delete_file!) - subject.destroy + subject.destroy! end end end @@ -82,6 +82,18 @@ RSpec.describe Upload do end end + describe '#relative_path' do + it "delegates to the uploader's relative_path method" do + uploader = spy('FakeUploader') + upload = described_class.new(path: '/tmp/secret/file.jpg', store: ObjectStorage::Store::LOCAL) + expect(upload).to receive(:uploader_class).and_return(uploader) + + upload.relative_path + + expect(uploader).to have_received(:relative_path).with(upload) + end + end + describe '#calculate_checksum!' do let(:upload) do described_class.new(path: __FILE__, diff --git a/spec/models/uploads/fog_spec.rb b/spec/models/uploads/fog_spec.rb index 899e6f2064c..1ffe7c6c43b 100644 --- a/spec/models/uploads/fog_spec.rb +++ b/spec/models/uploads/fog_spec.rb @@ -40,7 +40,9 @@ RSpec.describe Uploads::Fog do end describe '#delete_keys' do + let(:connection) { ::Fog::Storage.new(FileUploader.object_store_credentials) } let(:keys) { data_store.keys(relation) } + let(:paths) { relation.pluck(:path) } let!(:uploads) { create_list(:upload, 2, :with_file, :issuable_upload, model: project) } subject { data_store.delete_keys(keys) } @@ -50,17 +52,32 @@ RSpec.describe Uploads::Fog do end it 'deletes multiple data' do - paths = relation.pluck(:path) + paths.each do |path| + expect(connection.get_object('uploads', path)[:body]).not_to be_nil + end + + subject + + paths.each do |path| + expect { connection.get_object('uploads', path)[:body] }.to raise_error(Excon::Error::NotFound) + end + end - ::Fog::Storage.new(FileUploader.object_store_credentials).tap do |connection| + context 'when one of keys is missing' do + let(:keys) { ['unknown'] + super() } + + it 'deletes only existing keys' do paths.each do |path| expect(connection.get_object('uploads', path)[:body]).not_to be_nil end - end - subject + expect_next_instance_of(::Fog::Storage) do |storage| + allow(storage).to receive(:delete_object).and_call_original + expect(storage).to receive(:delete_object).with('uploads', keys.first).and_raise(::Google::Apis::ClientError, 'NotFound') + end + + subject - ::Fog::Storage.new(FileUploader.object_store_credentials).tap do |connection| paths.each do |path| expect { connection.get_object('uploads', path)[:body] }.to raise_error(Excon::Error::NotFound) end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 21c5aea514a..b5d4614d206 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -6,6 +6,7 @@ RSpec.describe User do include ProjectForksHelper include TermsHelper include ExclusiveLeaseHelpers + include LdapHelpers it_behaves_like 'having unique enum values' @@ -98,7 +99,7 @@ RSpec.describe User do it { is_expected.to have_many(:group_members) } it { is_expected.to have_many(:groups) } it { is_expected.to have_many(:keys).dependent(:destroy) } - it { is_expected.to have_many(:expired_and_unnotified_keys) } + it { is_expected.to have_many(:expired_today_and_unnotified_keys) } it { is_expected.to have_many(:deploy_keys).dependent(:nullify) } it { is_expected.to have_many(:group_deploy_keys) } it { is_expected.to have_many(:events).dependent(:delete_all) } @@ -1123,7 +1124,7 @@ RSpec.describe User do end describe 'after commit hook' do - describe '#update_emails_with_primary_email' do + describe 'when the primary email is updated' do before do @user = create(:user, email: 'primary@example.com').tap do |user| user.skip_reconfirmation! @@ -1132,13 +1133,7 @@ RSpec.describe User do @user.reload end - it 'gets called when email updated' do - expect(@user).to receive(:update_emails_with_primary_email) - - @user.update!(email: 'new_primary@example.com') - end - - it 'adds old primary to secondary emails when secondary is a new email' do + it 'keeps old primary to secondary emails when secondary is a new email' do @user.update!(email: 'new_primary@example.com') @user.reload @@ -1146,22 +1141,6 @@ RSpec.describe User do expect(@user.emails.pluck(:email)).to match_array([@secondary.email, 'primary@example.com']) end - it 'adds old primary to secondary emails if secondary is becoming a primary' do - @user.update!(email: @secondary.email) - @user.reload - - expect(@user.emails.count).to eq 1 - expect(@user.emails.first.email).to eq 'primary@example.com' - end - - it 'transfers old confirmation values into new secondary' do - @user.update!(email: @secondary.email) - @user.reload - - expect(@user.emails.count).to eq 1 - expect(@user.emails.first.confirmed_at).not_to eq nil - end - context 'when the first email was unconfirmed and the second email gets confirmed' do let(:user) { create(:user, :unconfirmed, email: 'should-be-unconfirmed@test.com') } @@ -1178,11 +1157,8 @@ RSpec.describe User do expect(user).to be_confirmed end - it 'keeps the unconfirmed email unconfirmed' do - email = user.emails.first - - expect(email.email).to eq('should-be-unconfirmed@test.com') - expect(email).not_to be_confirmed + it 'does not add unconfirmed email to secondary' do + expect(user.emails.map(&:email)).not_to include('should-be-unconfirmed@test.com') end it 'has only one email association' do @@ -1244,7 +1220,7 @@ RSpec.describe User do expect(user.email).to eq(confirmed_email) end - it 'moves the old email' do + it 'keeps the old email' do email = user.reload.emails.first expect(email.email).to eq(old_confirmed_email) @@ -1499,7 +1475,7 @@ RSpec.describe User do allow_any_instance_of(ApplicationSetting).to receive(:send_user_confirmation_email).and_return(true) end - let(:user) { create(:user, confirmed_at: nil, unconfirmed_email: 'test@gitlab.com') } + let(:user) { create(:user, :unconfirmed, unconfirmed_email: 'test@gitlab.com') } it 'returns unconfirmed' do expect(user.confirmed?).to be_falsey @@ -1509,6 +1485,22 @@ RSpec.describe User do user.confirm expect(user.confirmed?).to be_truthy end + + it 'adds the confirmed primary email to emails' do + expect(user.emails.confirmed.map(&:email)).not_to include(user.email) + + user.confirm + + expect(user.emails.confirmed.map(&:email)).to include(user.email) + end + end + + context 'if the user is created with confirmed_at set to a time' do + let!(:user) { create(:user, email: 'test@gitlab.com', confirmed_at: Time.now.utc) } + + it 'adds the confirmed primary email to emails upon creation' do + expect(user.emails.confirmed.map(&:email)).to include(user.email) + end end describe '#to_reference' do @@ -2216,7 +2208,7 @@ RSpec.describe User do end context 'primary email not confirmed' do - let(:user) { create(:user, confirmed_at: nil) } + let(:user) { create(:user, :unconfirmed) } let!(:email) { create(:email, :confirmed, user: user, email: 'foo@example.com') } it 'finds user respecting the confirmed flag' do @@ -2231,7 +2223,7 @@ RSpec.describe User do end it 'returns nil when user is not confirmed' do - user = create(:user, email: 'foo@example.com', confirmed_at: nil) + user = create(:user, :unconfirmed, email: 'foo@example.com') expect(described_class.find_by_any_email(user.email, confirmed: false)).to eq(user) expect(described_class.find_by_any_email(user.email, confirmed: true)).to be_nil @@ -4155,6 +4147,23 @@ RSpec.describe User do end end + describe '#remove_project_authorizations' do + let_it_be(:project1) { create(:project) } + let_it_be(:project2) { create(:project) } + let_it_be(:project3) { create(:project) } + let_it_be(:user) { create(:user) } + + it 'removes the project authorizations of the user, in specified projects' do + create(:project_authorization, user: user, project: project1) + create(:project_authorization, user: user, project: project2) + create(:project_authorization, user: user, project: project3) + + user.remove_project_authorizations([project1.id, project2.id]) + + expect(user.project_authorizations.pluck(:project_id)).to match_array([project3.id]) + end + end + describe '#access_level=' do let(:user) { build(:user) } @@ -5817,7 +5826,7 @@ RSpec.describe User do end describe '#active_for_authentication?' do - subject { user.active_for_authentication? } + subject(:active_for_authentication?) { user.active_for_authentication? } let(:user) { create(:user) } @@ -5827,6 +5836,14 @@ RSpec.describe User do end it { is_expected.to be false } + + it 'does not check if LDAP is allowed' do + stub_ldap_setting(enabled: true) + + expect(Gitlab::Auth::Ldap::Access).not_to receive(:allowed?) + + active_for_authentication? + end end context 'when user is a ghost user' do @@ -5837,6 +5854,28 @@ RSpec.describe User do it { is_expected.to be false } end + context 'when user is ldap_blocked' do + before do + user.ldap_block + end + + it 'rechecks if LDAP is allowed when LDAP is enabled' do + stub_ldap_setting(enabled: true) + + expect(Gitlab::Auth::Ldap::Access).to receive(:allowed?) + + active_for_authentication? + end + + it 'does not check if LDAP is allowed when LDAP is not enabled' do + stub_ldap_setting(enabled: false) + + expect(Gitlab::Auth::Ldap::Access).not_to receive(:allowed?) + + active_for_authentication? + end + end + context 'based on user type' do using RSpec::Parameterized::TableSyntax @@ -6011,7 +6050,7 @@ RSpec.describe User do subject { user.confirmation_required_on_sign_in? } context 'when user is confirmed' do - let(:user) { build_stubbed(:user) } + let(:user) { create(:user) } it 'is falsey' do expect(user.confirmed?).to be_truthy @@ -6203,4 +6242,31 @@ RSpec.describe User do expect(described_class.get_ids_by_username([user_name])).to match_array([user_id]) end end + + describe 'user_project' do + it 'returns users project matched by username and public visibility' do + user = create(:user) + public_project = create(:project, :public, path: user.username, namespace: user.namespace) + create(:project, namespace: user.namespace) + + expect(user.user_project).to eq(public_project) + end + end + + describe 'user_readme' do + it 'returns readme from user project' do + user = create(:user) + create(:project, :repository, :public, path: user.username, namespace: user.namespace) + + expect(user.user_readme.name).to eq('README.md') + expect(user.user_readme.data).to include('testme') + end + + it 'returns nil if project is private' do + user = create(:user) + create(:project, :repository, :private, path: user.username, namespace: user.namespace) + + expect(user.user_readme).to be(nil) + end + end end diff --git a/spec/models/users/credit_card_validation_spec.rb b/spec/models/users/credit_card_validation_spec.rb index d2b4f5ebd65..43edf7ed093 100644 --- a/spec/models/users/credit_card_validation_spec.rb +++ b/spec/models/users/credit_card_validation_spec.rb @@ -6,19 +6,25 @@ RSpec.describe Users::CreditCardValidation do it { is_expected.to belong_to(:user) } it { is_expected.to validate_length_of(:holder_name).is_at_most(26) } + it { is_expected.to validate_length_of(:network).is_at_most(32) } it { is_expected.to validate_numericality_of(:last_digits).is_less_than_or_equal_to(9999) } describe '.similar_records' do - let(:card_details) { subject.attributes.slice(:expiration_date, :last_digits, :holder_name) } + let(:card_details) do + subject.attributes.with_indifferent_access.slice(:expiration_date, :last_digits, :network, :holder_name) + end - subject(:credit_card_validation) { create(:credit_card_validation) } + subject!(:credit_card_validation) { create(:credit_card_validation, holder_name: 'Alice') } let!(:match1) { create(:credit_card_validation, card_details) } - let!(:other1) { create(:credit_card_validation, card_details.merge(last_digits: 9)) } - let!(:match2) { create(:credit_card_validation, card_details) } - let!(:other2) { create(:credit_card_validation, card_details.merge(holder_name: 'foo bar')) } + let!(:match2) { create(:credit_card_validation, card_details.merge(holder_name: 'Bob')) } + let!(:non_match1) { create(:credit_card_validation, card_details.merge(last_digits: 9)) } + let!(:non_match2) { create(:credit_card_validation, card_details.merge(network: 'unknown')) } + let!(:non_match3) do + create(:credit_card_validation, card_details.dup.tap { |h| h[:expiration_date] += 1.year }) + end - it 'returns records with matching credit card, ordered by credit_card_validated_at' do + it 'returns matches with the same last_digits, expiration and network, ordered by credit_card_validated_at' do expect(subject.similar_records).to eq([match2, match1, subject]) end end diff --git a/spec/models/users/in_product_marketing_email_spec.rb b/spec/models/users/in_product_marketing_email_spec.rb index a9ddd86677c..cf08cf7ceed 100644 --- a/spec/models/users/in_product_marketing_email_spec.rb +++ b/spec/models/users/in_product_marketing_email_spec.rb @@ -21,7 +21,8 @@ RSpec.describe Users::InProductMarketingEmail, type: :model do describe '.tracks' do it 'has an entry for every track' do - expect(Namespaces::InProductMarketingEmailsService::TRACKS.keys).to match_array(described_class.tracks.keys.map(&:to_sym)) + tracks = [Namespaces::InviteTeamEmailService::TRACK, Namespaces::InProductMarketingEmailsService::TRACKS.keys].flatten + expect(tracks).to match_array(described_class.tracks.keys.map(&:to_sym)) end end diff --git a/spec/models/users/merge_request_interaction_spec.rb b/spec/models/users/merge_request_interaction_spec.rb index d333577fa1a..12c7fa43a60 100644 --- a/spec/models/users/merge_request_interaction_spec.rb +++ b/spec/models/users/merge_request_interaction_spec.rb @@ -61,7 +61,7 @@ RSpec.describe ::Users::MergeRequestInteraction do merge_request.reviewers << user end - it { is_expected.to eq(Types::MergeRequestReviewStateEnum.values['UNREVIEWED'].value) } + it { is_expected.to eq(Types::MergeRequestReviewStateEnum.values['ATTENTION_REQUESTED'].value) } it 'implies not reviewed' do expect(interaction).not_to be_reviewed @@ -70,7 +70,8 @@ RSpec.describe ::Users::MergeRequestInteraction do context 'when the user has provided a review' do before do - merge_request.merge_request_reviewers.create!(reviewer: user, state: MergeRequestReviewer.states['reviewed']) + reviewer = merge_request.merge_request_reviewers.create!(reviewer: user) + reviewer.update!(state: MergeRequestReviewer.states['reviewed']) end it { is_expected.to eq(Types::MergeRequestReviewStateEnum.values['REVIEWED'].value) } diff --git a/spec/models/users_statistics_spec.rb b/spec/models/users_statistics_spec.rb index b4b7ddb7c63..8553d0bfdb0 100644 --- a/spec/models/users_statistics_spec.rb +++ b/spec/models/users_statistics_spec.rb @@ -34,11 +34,11 @@ RSpec.describe UsersStatistics do describe '.create_current_stats!' do before do - create_list(:user_highest_role, 4) + create_list(:user_highest_role, 1) create_list(:user_highest_role, 2, :guest) - create_list(:user_highest_role, 3, :reporter) - create_list(:user_highest_role, 4, :developer) - create_list(:user_highest_role, 3, :maintainer) + create_list(:user_highest_role, 2, :reporter) + create_list(:user_highest_role, 2, :developer) + create_list(:user_highest_role, 2, :maintainer) create_list(:user_highest_role, 2, :owner) create_list(:user, 2, :bot) create_list(:user, 1, :blocked) @@ -49,11 +49,11 @@ RSpec.describe UsersStatistics do context 'when successful' do it 'creates an entry with the current statistics values' do expect(described_class.create_current_stats!).to have_attributes( - without_groups_and_projects: 4, + without_groups_and_projects: 1, with_highest_role_guest: 2, - with_highest_role_reporter: 3, - with_highest_role_developer: 4, - with_highest_role_maintainer: 3, + with_highest_role_reporter: 2, + with_highest_role_developer: 2, + with_highest_role_maintainer: 2, with_highest_role_owner: 2, bots: 2, blocked: 1 diff --git a/spec/models/webauthn_registration_spec.rb b/spec/models/webauthn_registration_spec.rb new file mode 100644 index 00000000000..6813854bf6c --- /dev/null +++ b/spec/models/webauthn_registration_spec.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe WebauthnRegistration do + describe 'relations' do + it { is_expected.to belong_to(:user) } + end + + describe 'validations' do + it { is_expected.to validate_presence_of(:credential_xid) } + it { is_expected.to validate_presence_of(:public_key) } + it { is_expected.to validate_presence_of(:counter) } + it { is_expected.to validate_length_of(:name).is_at_least(0) } + it { is_expected.not_to allow_value(nil).for(:name) } + it do + is_expected.to validate_numericality_of(:counter) + .only_integer + .is_greater_than_or_equal_to(0) + .is_less_than_or_equal_to(4294967295) + end + end +end |