diff options
| author | GitLab Bot <gitlab-bot@gitlab.com> | 2020-06-18 11:18:50 +0000 |
|---|---|---|
| committer | GitLab Bot <gitlab-bot@gitlab.com> | 2020-06-18 11:18:50 +0000 |
| commit | 8c7f4e9d5f36cff46365a7f8c4b9c21578c1e781 (patch) | |
| tree | a77e7fe7a93de11213032ed4ab1f33a3db51b738 /spec/models | |
| parent | 00b35af3db1abfe813a778f643dad221aad51fca (diff) | |
| download | gitlab-ce-8c7f4e9d5f36cff46365a7f8c4b9c21578c1e781.tar.gz | |
Add latest changes from gitlab-org/gitlab@13-1-stable-ee
Diffstat (limited to 'spec/models')
105 files changed, 3293 insertions, 1319 deletions
diff --git a/spec/models/active_session_spec.rb b/spec/models/active_session_spec.rb index 6a97d91b3ca..24b47be3c69 100644 --- a/spec/models/active_session_spec.rb +++ b/spec/models/active_session_spec.rb @@ -16,7 +16,7 @@ RSpec.describe ActiveSession, :clean_gitlab_redis_shared_state do double(:request, { user_agent: 'Mozilla/5.0 (iPhone; CPU iPhone OS 8_1_3 like Mac OS X) AppleWebKit/600.1.4 ' \ '(KHTML, like Gecko) Mobile/12B466 [FBDV/iPhone7,2]', - ip: '127.0.0.1', + remote_ip: '127.0.0.1', session: session }) end diff --git a/spec/models/alert_management/alert_assignee_spec.rb b/spec/models/alert_management/alert_assignee_spec.rb new file mode 100644 index 00000000000..c51a5d543ab --- /dev/null +++ b/spec/models/alert_management/alert_assignee_spec.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe AlertManagement::AlertAssignee do + describe 'associations' do + it { is_expected.to belong_to(:alert) } + it { is_expected.to belong_to(:assignee) } + end + + describe 'validations' do + let(:alert) { create(:alert_management_alert) } + let(:user) { create(:user) } + + subject { alert.alert_assignees.build(assignee: user) } + + it { is_expected.to validate_presence_of(:alert) } + it { is_expected.to validate_presence_of(:assignee) } + it { is_expected.to validate_uniqueness_of(:assignee).scoped_to(:alert_id) } + end +end diff --git a/spec/models/alert_management/alert_spec.rb b/spec/models/alert_management/alert_spec.rb index 1da0c6d4071..27b8bb48073 100644 --- a/spec/models/alert_management/alert_spec.rb +++ b/spec/models/alert_management/alert_spec.rb @@ -6,6 +6,10 @@ describe AlertManagement::Alert do describe 'associations' do it { is_expected.to belong_to(:project) } it { is_expected.to belong_to(:issue) } + it { is_expected.to have_many(:assignees).through(:alert_assignees) } + it { is_expected.to have_many(:notes) } + it { is_expected.to have_many(:ordered_notes) } + it { is_expected.to have_many(:user_mentions) } end describe 'validations' do @@ -241,6 +245,12 @@ describe AlertManagement::Alert do end end + describe '#to_reference' do + let(:alert) { build(:alert_management_alert) } + + it { expect(alert.to_reference).to eq('') } + end + describe '#trigger' do subject { alert.trigger } @@ -317,4 +327,14 @@ describe AlertManagement::Alert do expect { subject }.to change { alert.reload.ended_at }.to nil end end + + describe '#register_new_event!' do + subject { alert.register_new_event! } + + let(:alert) { create(:alert_management_alert) } + + it 'increments the events count by 1' do + expect { subject }.to change { alert.events }.by(1) + end + end end diff --git a/spec/models/alert_management/alert_user_mention_spec.rb b/spec/models/alert_management/alert_user_mention_spec.rb new file mode 100644 index 00000000000..cce090a2231 --- /dev/null +++ b/spec/models/alert_management/alert_user_mention_spec.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe AlertManagement::AlertUserMention do + describe 'associations' do + it { is_expected.to belong_to(:alert_management_alert) } + it { is_expected.to belong_to(:note) } + end + + it_behaves_like 'has user mentions' +end diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb index 64308af38f9..96bf19439a1 100644 --- a/spec/models/application_setting_spec.rb +++ b/spec/models/application_setting_spec.rb @@ -105,6 +105,14 @@ describe ApplicationSetting do it { is_expected.not_to allow_value(false).for(:hashed_storage_enabled) } + it { is_expected.not_to allow_value(101).for(:repository_storages_weighted_default) } + it { is_expected.not_to allow_value(-1).for(:repository_storages_weighted_default) } + it { is_expected.to allow_value(100).for(:repository_storages_weighted_default) } + it { is_expected.to allow_value(0).for(:repository_storages_weighted_default) } + it { is_expected.to allow_value(50).for(:repository_storages_weighted_default) } + it { is_expected.to allow_value(nil).for(:repository_storages_weighted_default) } + it { is_expected.not_to allow_value({ default: 100, shouldntexist: 50 }).for(:repository_storages_weighted) } + context 'grafana_url validations' do before do subject.instance_variable_set(:@parsed_grafana_url, nil) @@ -152,6 +160,30 @@ describe ApplicationSetting do end end + describe 'spam_check_endpoint' do + context 'when spam_check_endpoint is enabled' do + before do + setting.spam_check_endpoint_enabled = true + end + + it { is_expected.to allow_value('https://example.org/spam_check').for(:spam_check_endpoint_url) } + it { is_expected.not_to allow_value('nonsense').for(:spam_check_endpoint_url) } + it { is_expected.not_to allow_value(nil).for(:spam_check_endpoint_url) } + it { is_expected.not_to allow_value('').for(:spam_check_endpoint_url) } + end + + context 'when spam_check_endpoint is NOT enabled' do + before do + setting.spam_check_endpoint_enabled = false + end + + it { is_expected.to allow_value('https://example.org/spam_check').for(:spam_check_endpoint_url) } + it { is_expected.not_to allow_value('nonsense').for(:spam_check_endpoint_url) } + it { is_expected.to allow_value(nil).for(:spam_check_endpoint_url) } + it { is_expected.to allow_value('').for(:spam_check_endpoint_url) } + end + end + context 'when snowplow is enabled' do before do setting.snowplow_enabled = true @@ -251,6 +283,14 @@ describe ApplicationSetting do .is_greater_than(0) end + it { is_expected.to validate_presence_of(:max_import_size) } + + it do + is_expected.to validate_numericality_of(:max_import_size) + .only_integer + .is_greater_than_or_equal_to(0) + end + it do is_expected.to validate_numericality_of(:local_markdown_version) .only_integer @@ -762,4 +802,17 @@ describe ApplicationSetting do end it_behaves_like 'application settings examples' + + describe 'repository_storages_weighted_attributes' do + it 'returns the keys for repository_storages_weighted' do + expect(subject.class.repository_storages_weighted_attributes).to eq([:repository_storages_weighted_default]) + end + end + + it 'does not allow to set weight for non existing storage' do + setting.repository_storages_weighted = { invalid_storage: 100 } + + expect(setting).not_to be_valid + expect(setting.errors.messages[:repository_storages_weighted]).to match_array(["can't include: invalid_storage"]) + end end diff --git a/spec/models/blob_viewer/go_mod_spec.rb b/spec/models/blob_viewer/go_mod_spec.rb new file mode 100644 index 00000000000..ba6038533ea --- /dev/null +++ b/spec/models/blob_viewer/go_mod_spec.rb @@ -0,0 +1,63 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe BlobViewer::GoMod do + include FakeBlobHelpers + + let(:project) { build_stubbed(:project) } + let(:data) do + <<-SPEC.strip_heredoc + module #{Settings.build_gitlab_go_url}/#{project.full_path} + SPEC + end + let(:blob) { fake_blob(path: 'go.mod', data: data) } + + subject { described_class.new(blob) } + + describe '#package_name' do + it 'returns the package name' do + expect(subject.package_name).to eq("#{Settings.build_gitlab_go_url}/#{project.full_path}") + end + end + + describe '#package_url' do + it 'returns the package URL' do + expect(subject.package_url).to eq("#{Gitlab.config.gitlab.protocol}://#{Settings.build_gitlab_go_url}/#{project.full_path}/") + end + + context 'when the homepage has an invalid URL' do + let(:data) do + <<-SPEC.strip_heredoc + module javascript:alert() + SPEC + end + + it 'returns nil' do + expect(subject.package_url).to be_nil + end + end + end + + describe '#package_type' do + it 'returns "package"' do + expect(subject.package_type).to eq('go') + end + end + + context 'when the module name does not start with the instance URL' do + let(:data) do + <<-SPEC.strip_heredoc + module example.com/foo/bar + SPEC + end + + subject { described_class.new(blob) } + + describe '#package_url' do + it 'returns the pkg.go.dev URL' do + expect(subject.package_url).to eq("https://pkg.go.dev/example.com/foo/bar") + end + end + end +end diff --git a/spec/models/blob_viewer/metrics_dashboard_yml_spec.rb b/spec/models/blob_viewer/metrics_dashboard_yml_spec.rb new file mode 100644 index 00000000000..f5b8586975d --- /dev/null +++ b/spec/models/blob_viewer/metrics_dashboard_yml_spec.rb @@ -0,0 +1,127 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe BlobViewer::MetricsDashboardYml do + include FakeBlobHelpers + include RepoHelpers + + let_it_be(:project) { create(:project, :repository) } + let(:blob) { fake_blob(path: '.gitlab/dashboards/custom-dashboard.yml', data: data) } + let(:sha) { sample_commit.id } + + subject(:viewer) { described_class.new(blob) } + + context 'when the definition is valid' do + let(:data) { File.read(Rails.root.join('config/prometheus/common_metrics.yml')) } + + describe '#valid?' do + it 'calls prepare! on the viewer' do + allow(PerformanceMonitoring::PrometheusDashboard).to receive(:from_json) + + expect(viewer).to receive(:prepare!) + + viewer.valid? + end + + it 'returns true', :aggregate_failures do + yml = ::Gitlab::Config::Loader::Yaml.new(data).load_raw! + + expect_next_instance_of(::Gitlab::Config::Loader::Yaml, data) do |loader| + expect(loader).to receive(:load_raw!).and_call_original + end + expect(PerformanceMonitoring::PrometheusDashboard) + .to receive(:from_json) + .with(yml) + .and_call_original + expect(viewer.valid?).to be_truthy + end + end + + describe '#errors' do + it 'returns nil' do + allow(PerformanceMonitoring::PrometheusDashboard).to receive(:from_json) + + expect(viewer.errors).to be nil + end + end + end + + context 'when definition is invalid' do + let(:error) { ActiveModel::ValidationError.new(PerformanceMonitoring::PrometheusDashboard.new.tap(&:validate)) } + let(:data) do + <<~YAML + dashboard: + YAML + end + + describe '#valid?' do + it 'returns false' do + expect(PerformanceMonitoring::PrometheusDashboard) + .to receive(:from_json).and_raise(error) + + expect(viewer.valid?).to be_falsey + end + end + + describe '#errors' do + it 'returns validation errors' do + allow(PerformanceMonitoring::PrometheusDashboard) + .to receive(:from_json).and_raise(error) + + expect(viewer.errors).to be error.model.errors + end + end + end + + context 'when YAML syntax is invalid' do + let(:data) do + <<~YAML + dashboard: 'empty metrics' + panel_groups: + - group: 'Group Title' + YAML + end + + describe '#valid?' do + it 'returns false' do + expect(PerformanceMonitoring::PrometheusDashboard).not_to receive(:from_json) + expect(viewer.valid?).to be_falsey + end + end + + describe '#errors' do + it 'returns validation errors' do + yaml_wrapped_errors = { 'YAML syntax': ["(<unknown>): did not find expected key while parsing a block mapping at line 1 column 1"] } + + expect(viewer.errors).to be_kind_of ActiveModel::Errors + expect(viewer.errors.messages).to eql(yaml_wrapped_errors) + end + end + end + + context 'when YAML loader raises error' do + let(:data) do + <<~YAML + large yaml file + YAML + end + + before do + allow(::Gitlab::Config::Loader::Yaml).to receive(:new) + .and_raise(::Gitlab::Config::Loader::Yaml::DataTooLargeError, 'The parsed YAML is too big') + end + + it 'is invalid' do + expect(PerformanceMonitoring::PrometheusDashboard).not_to receive(:from_json) + expect(viewer.valid?).to be(false) + end + + it 'returns validation errors' do + yaml_wrapped_errors = { 'YAML syntax': ["The parsed YAML is too big"] } + + expect(viewer.errors).to be_kind_of(ActiveModel::Errors) + expect(viewer.errors.messages).to eq(yaml_wrapped_errors) + end + end +end diff --git a/spec/models/broadcast_message_spec.rb b/spec/models/broadcast_message_spec.rb index 127faa5e8e2..8032f913d86 100644 --- a/spec/models/broadcast_message_spec.rb +++ b/spec/models/broadcast_message_spec.rb @@ -66,7 +66,7 @@ describe BroadcastMessage do end it 'expires the value if a broadcast message has ended', :request_store do - message = create(:broadcast_message, broadcast_type: broadcast_type, ends_at: Time.now.utc + 1.day) + message = create(:broadcast_message, broadcast_type: broadcast_type, ends_at: Time.current.utc + 1.day) expect(subject.call).to match_array([message]) expect(described_class.cache).to receive(:expire).and_call_original @@ -87,8 +87,8 @@ describe BroadcastMessage do future = create( :broadcast_message, - starts_at: Time.now + 10.minutes, - ends_at: Time.now + 20.minutes, + starts_at: Time.current + 10.minutes, + ends_at: Time.current + 20.minutes, broadcast_type: broadcast_type ) diff --git a/spec/models/ci/bridge_spec.rb b/spec/models/ci/bridge_spec.rb index 34f89d9cdae..385261e0ee9 100644 --- a/spec/models/ci/bridge_spec.rb +++ b/spec/models/ci/bridge_spec.rb @@ -21,6 +21,11 @@ describe Ci::Bridge do expect(bridge).to have_many(:sourced_pipelines) end + it 'has one downstream pipeline' do + expect(bridge).to have_one(:sourced_pipeline) + expect(bridge).to have_one(:downstream_pipeline) + end + describe '#tags' do it 'only has a bridge tag' do expect(bridge.tags).to eq [:bridge] diff --git a/spec/models/ci/build_report_result_spec.rb b/spec/models/ci/build_report_result_spec.rb new file mode 100644 index 00000000000..078b0d100a1 --- /dev/null +++ b/spec/models/ci/build_report_result_spec.rb @@ -0,0 +1,74 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Ci::BuildReportResult do + let(:build_report_result) { build(:ci_build_report_result, :with_junit_success) } + + describe 'associations' do + it { is_expected.to belong_to(:build) } + it { is_expected.to belong_to(:project) } + end + + describe 'validations' do + it { is_expected.to validate_presence_of(:project) } + it { is_expected.to validate_presence_of(:build) } + + context 'when attributes are valid' do + it 'returns no errors' do + expect(build_report_result).to be_valid + end + end + + context 'when data is invalid' do + it 'returns errors' do + build_report_result.data = { invalid: 'data' } + + expect(build_report_result).to be_invalid + expect(build_report_result.errors.full_messages).to eq(["Data must be a valid json schema"]) + end + end + end + + describe '#tests_name' do + it 'returns the suite name' do + expect(build_report_result.tests_name).to eq("rspec") + end + end + + describe '#tests_duration' do + it 'returns the suite duration' do + expect(build_report_result.tests_duration).to eq(0.42) + end + end + + describe '#tests_success' do + it 'returns the success count' do + expect(build_report_result.tests_success).to eq(2) + end + end + + describe '#tests_failed' do + it 'returns the failed count' do + expect(build_report_result.tests_failed).to eq(0) + end + end + + describe '#tests_errored' do + it 'returns the errored count' do + expect(build_report_result.tests_errored).to eq(0) + end + end + + describe '#tests_skipped' do + it 'returns the skipped count' do + expect(build_report_result.tests_skipped).to eq(0) + end + end + + describe '#tests_total' do + it 'returns the total count' do + expect(build_report_result.tests_total).to eq(2) + end + end +end diff --git a/spec/models/ci/build_runner_session_spec.rb b/spec/models/ci/build_runner_session_spec.rb index cdf56f24cd7..3e520407884 100644 --- a/spec/models/ci/build_runner_session_spec.rb +++ b/spec/models/ci/build_runner_session_spec.rb @@ -63,4 +63,64 @@ describe Ci::BuildRunnerSession, model: true do end end end + + describe '#service_specification' do + let(:service) { 'foo'} + let(:port) { 80 } + let(:path) { 'path' } + let(:subprotocols) { nil } + let(:specification) { subject.service_specification(service: service, port: port, path: path, subprotocols: subprotocols) } + + it 'returns service proxy url' do + expect(specification[:url]).to eq "https://localhost/proxy/#{service}/#{port}/#{path}" + end + + it 'returns default service proxy websocket subprotocol' do + expect(specification[:subprotocols]).to eq %w[terminal.gitlab.com] + end + + it 'returns empty hash if no url' do + subject.url = '' + + expect(specification).to be_empty + end + + context 'when port is not present' do + let(:port) { nil } + + it 'uses the default port name' do + expect(specification[:url]).to eq "https://localhost/proxy/#{service}/default_port/#{path}" + end + end + + context 'when the service is not present' do + let(:service) { '' } + + it 'uses the service name "build" as default' do + expect(specification[:url]).to eq "https://localhost/proxy/build/#{port}/#{path}" + end + end + + context 'when url is present' do + it 'returns ca_pem nil if empty certificate' do + subject.certificate = '' + + expect(specification[:ca_pem]).to be_nil + end + + it 'adds Authorization header if authorization is present' do + subject.authorization = 'foobar' + + expect(specification[:headers]).to include(Authorization: ['foobar']) + end + end + + context 'when subprotocol is present' do + let(:subprotocols) { 'foobar' } + + it 'returns the new subprotocol' do + expect(specification[:subprotocols]).to eq [subprotocols] + end + end + end end diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 6605866d9c0..6fdd8463329 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -24,6 +24,7 @@ describe Ci::Build do it { is_expected.to have_many(:needs) } it { is_expected.to have_many(:sourced_pipelines) } it { is_expected.to have_many(:job_variables) } + it { is_expected.to have_many(:report_results) } it { is_expected.to have_one(:deployment) } it { is_expected.to have_one(:runner_session) } @@ -626,7 +627,7 @@ describe Ci::Build do context 'is expired' do before do - build.update(artifacts_expire_at: Time.now - 7.days) + build.update(artifacts_expire_at: Time.current - 7.days) end it { is_expected.to be_truthy } @@ -634,7 +635,7 @@ describe Ci::Build do context 'is not expired' do before do - build.update(artifacts_expire_at: Time.now + 7.days) + build.update(artifacts_expire_at: Time.current + 7.days) end it { is_expected.to be_falsey } @@ -661,13 +662,13 @@ describe Ci::Build do it { is_expected.to be_nil } context 'when artifacts_expire_at is specified' do - let(:expire_at) { Time.now + 7.days } + let(:expire_at) { Time.current + 7.days } before do build.artifacts_expire_at = expire_at end - it { is_expected.to be_within(5).of(expire_at - Time.now) } + it { is_expected.to be_within(5).of(expire_at - Time.current) } end end @@ -874,6 +875,22 @@ describe Ci::Build do end end + describe '#has_test_reports?' do + subject { build.has_test_reports? } + + context 'when build has a test report' do + let(:build) { create(:ci_build, :test_reports) } + + it { is_expected.to be_truthy } + end + + context 'when build does not have a test report' do + let(:build) { create(:ci_build) } + + it { is_expected.to be_falsey } + end + end + describe '#has_old_trace?' do subject { build.has_old_trace? } @@ -1795,7 +1812,7 @@ describe Ci::Build do end describe '#keep_artifacts!' do - let(:build) { create(:ci_build, artifacts_expire_at: Time.now + 7.days) } + let(:build) { create(:ci_build, artifacts_expire_at: Time.current + 7.days) } subject { build.keep_artifacts! } @@ -2285,7 +2302,7 @@ describe Ci::Build do let(:predefined_variables) do [ { key: 'CI_PIPELINE_ID', value: pipeline.id.to_s, public: true, masked: false }, - { key: 'CI_PIPELINE_URL', value: project.web_url + "/pipelines/#{pipeline.id}", public: true, masked: false }, + { key: 'CI_PIPELINE_URL', value: project.web_url + "/-/pipelines/#{pipeline.id}", public: true, masked: false }, { key: 'CI_JOB_ID', value: build.id.to_s, public: true, masked: false }, { key: 'CI_JOB_URL', value: project.web_url + "/-/jobs/#{build.id}", public: true, masked: false }, { key: 'CI_JOB_TOKEN', value: 'my-token', public: false, masked: true }, @@ -2390,13 +2407,13 @@ describe Ci::Build do allow(build).to receive(:job_jwt_variables) { [job_jwt_var] } allow(build).to receive(:dependency_variables) { [job_dependency_var] } - allow_any_instance_of(Project) + allow(build.project) .to receive(:predefined_variables) { [project_pre_var] } project.variables.create!(key: 'secret', value: 'value') - allow_any_instance_of(Ci::Pipeline) - .to receive(:predefined_variables) { [pipeline_pre_var] } + allow(build.pipeline) + .to receive(:predefined_variables).and_return([pipeline_pre_var]) end it 'returns variables in order depending on resource hierarchy' do @@ -2512,6 +2529,17 @@ describe Ci::Build do end end end + + context 'with the :modified_path_ci_variables feature flag disabled' do + before do + stub_feature_flags(modified_path_ci_variables: false) + end + + it 'does not set CI_MERGE_REQUEST_CHANGED_PAGES_* variables' do + expect(subject.find { |var| var[:key] == 'CI_MERGE_REQUEST_CHANGED_PAGE_PATHS' }).to be_nil + expect(subject.find { |var| var[:key] == 'CI_MERGE_REQUEST_CHANGED_PAGE_URLS' }).to be_nil + end + end end context 'when build has user' do @@ -3095,24 +3123,8 @@ describe Ci::Build do let!(:job_variable) { create(:ci_job_variable, :dotenv_source, job: prepare) } - context 'FF ci_dependency_variables is enabled' do - before do - stub_feature_flags(ci_dependency_variables: true) - end - - it 'inherits dependent variables' do - expect(build.scoped_variables.to_hash).to include(job_variable.key => job_variable.value) - end - end - - context 'FF ci_dependency_variables is disabled' do - before do - stub_feature_flags(ci_dependency_variables: false) - end - - it 'does not inherit dependent variables' do - expect(build.scoped_variables.to_hash).not_to include(job_variable.key => job_variable.value) - end + it 'inherits dependent variables' do + expect(build.scoped_variables.to_hash).to include(job_variable.key => job_variable.value) end end end @@ -3601,7 +3613,7 @@ describe Ci::Build do .to receive(:execute) .with(subject) .and_raise(Gitlab::Access::AccessDeniedError) - allow(Rails.logger).to receive(:error) + allow(Gitlab::AppLogger).to receive(:error) end it 'handles raised exception' do @@ -3611,7 +3623,7 @@ describe Ci::Build do it 'logs the error' do subject.drop! - expect(Rails.logger) + expect(Gitlab::AppLogger) .to have_received(:error) .with(a_string_matching("Unable to auto-retry job #{subject.id}")) end @@ -4040,10 +4052,11 @@ describe Ci::Build do expect(terraform_reports.plans).to match( a_hash_including( - 'tfplan.json' => a_hash_including( + build.id.to_s => a_hash_including( 'create' => 0, 'update' => 1, - 'delete' => 0 + 'delete' => 0, + 'job_name' => build.options.dig(:artifacts, :name).to_s ) ) ) @@ -4203,7 +4216,7 @@ describe Ci::Build do subject { build.supported_runner?(runner_features) } - context 'when feature is required by build' do + context 'when `upload_multiple_artifacts` feature is required by build' do before do expect(build).to receive(:runner_required_feature_names) do [:upload_multiple_artifacts] @@ -4227,7 +4240,7 @@ describe Ci::Build do end end - context 'when refspecs feature is required by build' do + context 'when `refspecs` feature is required by build' do before do allow(build).to receive(:merge_request_ref?) { true } end @@ -4244,6 +4257,26 @@ describe Ci::Build do it { is_expected.to be_falsey } end end + + context 'when `release_steps` feature is required by build' do + before do + expect(build).to receive(:runner_required_feature_names) do + [:release_steps] + end + end + + context 'when runner provides given feature' do + let(:runner_features) { { release_steps: true } } + + it { is_expected.to be_truthy } + end + + context 'when runner does not provide given feature' do + let(:runner_features) { {} } + + it { is_expected.to be_falsey } + end + end end describe '#deployment_status' do diff --git a/spec/models/ci/build_trace_chunk_spec.rb b/spec/models/ci/build_trace_chunk_spec.rb index f08f05a09bf..85873847fca 100644 --- a/spec/models/ci/build_trace_chunk_spec.rb +++ b/spec/models/ci/build_trace_chunk_spec.rb @@ -33,7 +33,7 @@ describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do expect(subjects.count).to be > 0 expect { subjects.first.destroy }.to raise_error('`destroy` and `destroy_all` are forbidden. Please use `fast_destroy_all`') - expect { subjects.destroy_all }.to raise_error('`destroy` and `destroy_all` are forbidden. Please use `fast_destroy_all`') # rubocop: disable DestroyAll + expect { subjects.destroy_all }.to raise_error('`destroy` and `destroy_all` are forbidden. Please use `fast_destroy_all`') # rubocop: disable Cop/DestroyAll expect(subjects.count).to be > 0 expect(external_data_counter).to be > 0 diff --git a/spec/models/ci/daily_build_group_report_result_spec.rb b/spec/models/ci/daily_build_group_report_result_spec.rb index d4c305c649a..f2ce1b5775f 100644 --- a/spec/models/ci/daily_build_group_report_result_spec.rb +++ b/spec/models/ci/daily_build_group_report_result_spec.rb @@ -3,6 +3,30 @@ require 'spec_helper' describe Ci::DailyBuildGroupReportResult do + let(:daily_build_group_report_result) { build(:ci_daily_build_group_report_result)} + + describe 'associations' do + it { is_expected.to belong_to(:last_pipeline) } + it { is_expected.to belong_to(:project) } + end + + describe 'validations' do + context 'when attributes are valid' do + it 'returns no errors' do + expect(daily_build_group_report_result).to be_valid + end + end + + context 'when data is invalid' do + it 'returns errors' do + daily_build_group_report_result.data = { invalid: 'data' } + + expect(daily_build_group_report_result).to be_invalid + expect(daily_build_group_report_result.errors.full_messages).to eq(["Data must be a valid json schema"]) + end + end + end + describe '.upsert_reports' do let!(:rspec_coverage) do create( diff --git a/spec/models/ci/instance_variable_spec.rb b/spec/models/ci/instance_variable_spec.rb index ff8676e1424..4d69b7ac2f8 100644 --- a/spec/models/ci/instance_variable_spec.rb +++ b/spec/models/ci/instance_variable_spec.rb @@ -9,6 +9,26 @@ describe Ci::InstanceVariable do it { is_expected.to include_module(Ci::Maskable) } it { is_expected.to validate_uniqueness_of(:key).with_message(/\(\w+\) has already been taken/) } + it { is_expected.to validate_length_of(:encrypted_value).is_at_most(1024).with_message(/Variables over 700 characters risk exceeding the limit/) } + + it_behaves_like 'includes Limitable concern' do + subject { build(:ci_instance_variable) } + end + + context 'with instance level variable feature flag disabled' do + let(:plan_limits) { create(:plan_limits, :default_plan) } + + before do + stub_feature_flags(ci_instance_level_variables_limit: false) + plan_limits.update(described_class.limit_name => 1) + create(:ci_instance_variable) + end + + it 'can create new models exceeding the plan limits', :aggregate_failures do + expect { subject.save }.to change { described_class.count } + expect(subject.errors[:base]).to be_empty + end + end describe '.unprotected' do subject { described_class.unprotected } @@ -39,7 +59,7 @@ describe Ci::InstanceVariable do it { expect(described_class.all_cached).to contain_exactly(protected_variable, unprotected_variable) } it 'memoizes the result' do - expect(described_class).to receive(:store_cache).with(:ci_instance_variable_data).once.and_call_original + expect(described_class).to receive(:unscoped).once.and_call_original 2.times do expect(described_class.all_cached).to contain_exactly(protected_variable, unprotected_variable) @@ -65,15 +85,6 @@ describe Ci::InstanceVariable do expect(described_class.all_cached).to contain_exactly(protected_variable, unprotected_variable, variable) end - - it 'resets the cache when the shared key is missing' do - expect(Rails.cache).to receive(:read).with(:ci_instance_variable_changed_at).twice.and_return(nil) - expect(described_class).to receive(:store_cache).with(:ci_instance_variable_data).thrice.and_call_original - - 3.times do - expect(described_class.all_cached).to contain_exactly(protected_variable, unprotected_variable) - end - end end describe '.unprotected_cached', :use_clean_rails_memory_store_caching do @@ -83,7 +94,7 @@ describe Ci::InstanceVariable do it { expect(described_class.unprotected_cached).to contain_exactly(unprotected_variable) } it 'memoizes the result' do - expect(described_class).to receive(:store_cache).with(:ci_instance_variable_data).once.and_call_original + expect(described_class).to receive(:unscoped).once.and_call_original 2.times do expect(described_class.unprotected_cached).to contain_exactly(unprotected_variable) diff --git a/spec/models/ci/job_artifact_spec.rb b/spec/models/ci/job_artifact_spec.rb index 4cdc74d7a41..17e00533ac3 100644 --- a/spec/models/ci/job_artifact_spec.rb +++ b/spec/models/ci/job_artifact_spec.rb @@ -23,6 +23,14 @@ describe Ci::JobArtifact do subject { build(:ci_job_artifact, :archive, size: 107464) } end + describe '.not_expired' do + it 'returns artifacts that have not expired' do + _expired_artifact = create(:ci_job_artifact, :expired) + + expect(described_class.not_expired).to contain_exactly(artifact) + end + end + describe '.with_reports' do let!(:artifact) { create(:ci_job_artifact, :archive) } @@ -118,6 +126,17 @@ describe Ci::JobArtifact do end end + describe '.downloadable' do + subject { described_class.downloadable } + + it 'filters for downloadable artifacts' do + downloadable_artifact = create(:ci_job_artifact, :codequality) + _not_downloadable_artifact = create(:ci_job_artifact, :trace) + + expect(subject).to contain_exactly(downloadable_artifact) + end + end + describe '.archived_trace_exists_for?' do subject { described_class.archived_trace_exists_for?(job_id) } @@ -357,19 +376,75 @@ describe Ci::JobArtifact do end end + describe 'expired?' do + subject { artifact.expired? } + + context 'when expire_at is nil' do + let(:artifact) { build(:ci_job_artifact, expire_at: nil) } + + it 'returns false' do + is_expected.to be_falsy + end + end + + context 'when expire_at is in the past' do + let(:artifact) { build(:ci_job_artifact, expire_at: Date.yesterday) } + + it 'returns true' do + is_expected.to be_truthy + end + end + + context 'when expire_at is in the future' do + let(:artifact) { build(:ci_job_artifact, expire_at: Date.tomorrow) } + + it 'returns false' do + is_expected.to be_falsey + end + end + end + + describe '#expiring?' do + subject { artifact.expiring? } + + context 'when expire_at is nil' do + let(:artifact) { build(:ci_job_artifact, expire_at: nil) } + + it 'returns false' do + is_expected.to be_falsy + end + end + + context 'when expire_at is in the past' do + let(:artifact) { build(:ci_job_artifact, expire_at: Date.yesterday) } + + it 'returns false' do + is_expected.to be_falsy + end + end + + context 'when expire_at is in the future' do + let(:artifact) { build(:ci_job_artifact, expire_at: Date.tomorrow) } + + it 'returns true' do + is_expected.to be_truthy + end + end + end + describe '#expire_in' do subject { artifact.expire_in } it { is_expected.to be_nil } context 'when expire_at is specified' do - let(:expire_at) { Time.now + 7.days } + let(:expire_at) { Time.current + 7.days } before do artifact.expire_at = expire_at end - it { is_expected.to be_within(5).of(expire_at - Time.now) } + it { is_expected.to be_within(5).of(expire_at - Time.current) } end end diff --git a/spec/models/ci/pipeline_schedule_spec.rb b/spec/models/ci/pipeline_schedule_spec.rb index 9a10c7629b2..4ba70552f01 100644 --- a/spec/models/ci/pipeline_schedule_spec.rb +++ b/spec/models/ci/pipeline_schedule_spec.rb @@ -118,7 +118,7 @@ describe Ci::PipelineSchedule do let(:pipeline_schedule) { create(:ci_pipeline_schedule, :every_minute) } it "updates next_run_at to the sidekiq worker's execution time" do - Timecop.freeze(Time.parse("2019-06-01 12:18:00+0000")) do + Timecop.freeze(Time.zone.parse("2019-06-01 12:18:00+0000")) do expect(pipeline_schedule.next_run_at).to eq(cron_worker_next_run_at) end end diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 4f53b6b4418..782a4206c36 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -26,6 +26,8 @@ describe Ci::Pipeline, :mailer do it { is_expected.to have_many(:trigger_requests) } it { is_expected.to have_many(:variables) } it { is_expected.to have_many(:builds) } + it { is_expected.to have_many(:bridges) } + it { is_expected.to have_many(:job_artifacts).through(:builds) } it { is_expected.to have_many(:auto_canceled_pipelines) } it { is_expected.to have_many(:auto_canceled_jobs) } it { is_expected.to have_many(:sourced_pipelines) } @@ -51,6 +53,27 @@ describe Ci::Pipeline, :mailer do expect(Project.reflect_on_association(:all_pipelines).has_inverse?).to eq(:project) expect(Project.reflect_on_association(:ci_pipelines).has_inverse?).to eq(:project) end + + describe '#latest_builds' do + it 'has a one to many relationship with its latest builds' do + _old_build = create(:ci_build, :retried, pipeline: pipeline) + latest_build = create(:ci_build, :expired, pipeline: pipeline) + + expect(pipeline.latest_builds).to contain_exactly(latest_build) + end + end + + describe '#downloadable_artifacts' do + let(:build) { create(:ci_build, pipeline: pipeline) } + + it 'returns downloadable artifacts that have not expired' do + downloadable_artifact = create(:ci_job_artifact, :codequality, job: build) + _expired_artifact = create(:ci_job_artifact, :junit, :expired, job: build) + _undownloadable_artifact = create(:ci_job_artifact, :trace, job: build) + + expect(pipeline.downloadable_artifacts).to contain_exactly(downloadable_artifact) + end + end end describe '#set_status' do @@ -99,6 +122,17 @@ describe Ci::Pipeline, :mailer do end end + describe '.for_iid' do + subject { described_class.for_iid(iid) } + + let(:iid) { '1234' } + let!(:pipeline) { create(:ci_pipeline, iid: '1234') } + + it 'returns the pipeline' do + is_expected.to contain_exactly(pipeline) + end + end + describe '.for_sha' do subject { described_class.for_sha(sha) } @@ -1007,19 +1041,6 @@ describe Ci::Pipeline, :mailer do subject { pipeline.ordered_stages } - context 'when using legacy stages' do - before do - stub_feature_flags( - ci_pipeline_persisted_stages: false, - ci_atomic_processing: false - ) - end - - it 'returns legacy stages in valid order' do - expect(subject.map(&:name)).to eq %w[build test] - end - end - context 'when using atomic processing' do before do stub_feature_flags( @@ -1051,7 +1072,6 @@ describe Ci::Pipeline, :mailer do context 'when using persisted stages' do before do stub_feature_flags( - ci_pipeline_persisted_stages: true, ci_atomic_processing: false ) end @@ -1079,7 +1099,7 @@ describe Ci::Pipeline, :mailer do end describe 'state machine' do - let(:current) { Time.now.change(usec: 0) } + let(:current) { Time.current.change(usec: 0) } let(:build) { create_build('build1', queued_at: 0) } let(:build_b) { create_build('build2', queued_at: 0) } let(:build_c) { create_build('build3', queued_at: 0) } @@ -2633,38 +2653,34 @@ describe Ci::Pipeline, :mailer do end end - shared_examples 'enqueues the notification worker' do - it 'enqueues PipelineUpdateCiRefStatusWorker' do - expect(PipelineUpdateCiRefStatusWorker).to receive(:perform_async).with(pipeline.id) - expect(PipelineNotificationWorker).not_to receive(:perform_async).with(pipeline.id) + context 'with success pipeline' do + it_behaves_like 'sending a notification' do + before do + perform_enqueued_jobs do + pipeline.succeed + end + end + end + + it 'enqueues PipelineNotificationWorker' do + expect(PipelineNotificationWorker) + .to receive(:perform_async).with(pipeline.id, ref_status: :success) pipeline.succeed end - context 'when ci_pipeline_fixed_notifications is disabled' do + context 'when pipeline is not the latest' do before do - stub_feature_flags(ci_pipeline_fixed_notifications: false) + create(:ci_pipeline, :success, project: project, ci_ref: pipeline.ci_ref) end - it 'enqueues PipelineNotificationWorker' do - expect(PipelineUpdateCiRefStatusWorker).not_to receive(:perform_async).with(pipeline.id) - expect(PipelineNotificationWorker).to receive(:perform_async).with(pipeline.id) + it 'does not pass ref_status' do + expect(PipelineNotificationWorker) + .to receive(:perform_async).with(pipeline.id, ref_status: nil) - pipeline.succeed - end - end - end - - context 'with success pipeline' do - it_behaves_like 'sending a notification' do - before do - perform_enqueued_jobs do - pipeline.succeed - end + pipeline.succeed! end end - - it_behaves_like 'enqueues the notification worker' end context 'with failed pipeline' do @@ -2679,7 +2695,12 @@ describe Ci::Pipeline, :mailer do end end - it_behaves_like 'enqueues the notification worker' + it 'enqueues PipelineNotificationWorker' do + expect(PipelineNotificationWorker) + .to receive(:perform_async).with(pipeline.id, ref_status: :failed) + + pipeline.drop + end end context 'with skipped pipeline' do @@ -2703,6 +2724,69 @@ describe Ci::Pipeline, :mailer do end end + describe 'updates ci_ref when pipeline finished' do + context 'when ci_ref exists' do + let!(:pipeline) { create(:ci_pipeline, :running) } + + it 'updates the ci_ref' do + expect(pipeline.ci_ref) + .to receive(:update_status_by!).with(pipeline).and_call_original + + pipeline.succeed! + end + end + + context 'when ci_ref does not exist' do + let!(:pipeline) { create(:ci_pipeline, :running, ci_ref_presence: false) } + + it 'does not raise an exception' do + expect { pipeline.succeed! }.not_to raise_error + end + end + end + + describe '#ensure_ci_ref!' do + subject { pipeline.ensure_ci_ref! } + + shared_examples_for 'protected by feature flag' do + context 'when feature flag is disabled' do + before do + stub_feature_flags(ci_pipeline_fixed_notifications: false) + end + + it 'does not do anything' do + expect(Ci::Ref).not_to receive(:ensure_for) + + subject + end + end + end + + context 'when ci_ref does not exist yet' do + let!(:pipeline) { create(:ci_pipeline, ci_ref_presence: false) } + + it_behaves_like 'protected by feature flag' + + it 'creates a new ci_ref and assigns it' do + expect { subject }.to change { Ci::Ref.count }.by(1) + + expect(pipeline.ci_ref).to be_present + end + end + + context 'when ci_ref already exists' do + let!(:pipeline) { create(:ci_pipeline) } + + it_behaves_like 'protected by feature flag' + + it 'fetches a new ci_ref and assigns it' do + expect { subject }.not_to change { Ci::Ref.count } + + expect(pipeline.ci_ref).to be_present + end + end + end + describe '#find_job_with_archive_artifacts' do let!(:old_job) { create(:ci_build, name: 'rspec', retried: true, pipeline: pipeline) } let!(:job_without_artifacts) { create(:ci_build, name: 'rspec', pipeline: pipeline) } @@ -2741,6 +2825,30 @@ describe Ci::Pipeline, :mailer do end end + describe '#latest_report_builds' do + it 'returns build with test artifacts' do + test_build = create(:ci_build, :test_reports, pipeline: pipeline, project: project) + coverage_build = create(:ci_build, :coverage_reports, pipeline: pipeline, project: project) + create(:ci_build, :artifacts, pipeline: pipeline, project: project) + + expect(pipeline.latest_report_builds).to contain_exactly(test_build, coverage_build) + end + + it 'filters builds by scope' do + test_build = create(:ci_build, :test_reports, pipeline: pipeline, project: project) + create(:ci_build, :coverage_reports, pipeline: pipeline, project: project) + + expect(pipeline.latest_report_builds(Ci::JobArtifact.test_reports)).to contain_exactly(test_build) + end + + it 'only returns not retried builds' do + test_build = create(:ci_build, :test_reports, pipeline: pipeline, project: project) + create(:ci_build, :test_reports, :retried, pipeline: pipeline, project: project) + + expect(pipeline.latest_report_builds).to contain_exactly(test_build) + end + end + describe '#has_reports?' do subject { pipeline.has_reports?(Ci::JobArtifact.test_reports) } diff --git a/spec/models/ci/ref_spec.rb b/spec/models/ci/ref_spec.rb index aa3b8cdbc3e..3d75cb63141 100644 --- a/spec/models/ci/ref_spec.rb +++ b/spec/models/ci/ref_spec.rb @@ -4,8 +4,155 @@ require 'spec_helper' describe Ci::Ref do it { is_expected.to belong_to(:project) } - it { is_expected.to belong_to(:last_updated_by_pipeline) } - it { is_expected.to validate_inclusion_of(:status).in_array(%w[success failed fixed]) } - it { is_expected.to validate_presence_of(:last_updated_by_pipeline) } + describe '.ensure_for' do + let_it_be(:project) { create(:project, :repository) } + + subject { described_class.ensure_for(pipeline) } + + shared_examples_for 'ensures ci_ref' do + context 'when ci_ref already exists' do + let(:options) { {} } + + it 'returns an existing ci_ref' do + expect { subject }.not_to change { described_class.count } + + expect(subject).to eq(Ci::Ref.find_by(project_id: project.id, ref_path: expected_ref_path)) + end + end + + context 'when ci_ref does not exist yet' do + let(:options) { { ci_ref_presence: false } } + + it 'creates a new ci_ref' do + expect { subject }.to change { described_class.count }.by(1) + + expect(subject).to eq(Ci::Ref.find_by(project_id: project.id, ref_path: expected_ref_path)) + end + end + end + + context 'when pipeline is a branch pipeline' do + let!(:pipeline) { create(:ci_pipeline, ref: 'master', project: project, **options) } + let(:expected_ref_path) { 'refs/heads/master' } + + it_behaves_like 'ensures ci_ref' + end + + context 'when pipeline is a tag pipeline' do + let!(:pipeline) { create(:ci_pipeline, ref: 'v1.1.0', tag: true, project: project, **options) } + let(:expected_ref_path) { 'refs/tags/v1.1.0' } + + it_behaves_like 'ensures ci_ref' + end + + context 'when pipeline is a detached merge request pipeline' do + let(:merge_request) do + create(:merge_request, target_project: project, target_branch: 'master', + source_project: project, source_branch: 'feature') + end + + let!(:pipeline) do + create(:ci_pipeline, :detached_merge_request_pipeline, merge_request: merge_request, project: project, **options) + end + + let(:expected_ref_path) { 'refs/heads/feature' } + + it_behaves_like 'ensures ci_ref' + end + end + + describe '#update_status_by!' do + subject { ci_ref.update_status_by!(pipeline) } + + let!(:ci_ref) { create(:ci_ref) } + + shared_examples_for 'no-op' do + it 'does nothing and returns nil' do + expect { subject }.not_to change { ci_ref.status_name } + + is_expected.to be_nil + end + end + + context 'when pipeline status is success or failed' do + using RSpec::Parameterized::TableSyntax + + where(:pipeline_status, :current_ref_status, :expected_ref_status) do + :success | :unknown | :success + :success | :success | :success + :success | :failed | :fixed + :success | :fixed | :success + :success | :broken | :fixed + :success | :still_failing | :fixed + :failed | :unknown | :failed + :failed | :success | :broken + :failed | :failed | :still_failing + :failed | :fixed | :broken + :failed | :broken | :still_failing + :failed | :still_failing | :still_failing + end + + with_them do + let(:ci_ref) { create(:ci_ref, status: described_class.state_machines[:status].states[current_ref_status].value) } + let(:pipeline) { create(:ci_pipeline, status: pipeline_status, ci_ref: ci_ref) } + + it 'transitions the status via state machine' do + expect(subject).to eq(expected_ref_status) + expect(ci_ref.status_name).to eq(expected_ref_status) + end + end + end + + context 'when pipeline status is success' do + let(:pipeline) { create(:ci_pipeline, :success, ci_ref: ci_ref) } + + it 'updates the status' do + expect { subject }.to change { ci_ref.status_name }.from(:unknown).to(:success) + + is_expected.to eq(:success) + end + end + + context 'when pipeline status is canceled' do + let(:pipeline) { create(:ci_pipeline, status: :canceled, ci_ref: ci_ref) } + + it { is_expected.to eq(:unknown) } + end + + context 'when pipeline status is skipped' do + let(:pipeline) { create(:ci_pipeline, status: :skipped, ci_ref: ci_ref) } + + it_behaves_like 'no-op' + end + + context 'when pipeline status is not complete' do + let(:pipeline) { create(:ci_pipeline, :running, ci_ref: ci_ref) } + + it_behaves_like 'no-op' + end + + context 'when feature flag is disabled' do + let(:pipeline) { create(:ci_pipeline, :success, ci_ref: ci_ref) } + + before do + stub_feature_flags(ci_pipeline_fixed_notifications: false) + end + + it_behaves_like 'no-op' + end + + context 'when pipeline is not the latest pipeline' do + let!(:pipeline) { create(:ci_pipeline, :success, ci_ref: ci_ref) } + let!(:latest_pipeline) { create(:ci_pipeline, :success, ci_ref: ci_ref) } + + it_behaves_like 'no-op' + end + + context 'when pipeline does not belong to the ci_ref' do + let(:pipeline) { create(:ci_pipeline, :success, ci_ref: create(:ci_ref)) } + + it_behaves_like 'no-op' + end + end end diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index 8b6a4fa6ade..296240b1602 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -263,7 +263,7 @@ describe Ci::Runner do subject { described_class.online } before do - @runner1 = create(:ci_runner, :instance, contacted_at: 1.hour.ago) + @runner1 = create(:ci_runner, :instance, contacted_at: 2.hours.ago) @runner2 = create(:ci_runner, :instance, contacted_at: 1.second.ago) end @@ -344,7 +344,7 @@ describe Ci::Runner do subject { described_class.offline } before do - @runner1 = create(:ci_runner, :instance, contacted_at: 1.hour.ago) + @runner1 = create(:ci_runner, :instance, contacted_at: 2.hours.ago) @runner2 = create(:ci_runner, :instance, contacted_at: 1.second.ago) end @@ -598,14 +598,14 @@ describe Ci::Runner do end end - describe '#update_cached_info' do + describe '#heartbeat' do let(:runner) { create(:ci_runner, :project) } - subject { runner.update_cached_info(architecture: '18-bit') } + subject { runner.heartbeat(architecture: '18-bit') } context 'when database was updated recently' do before do - runner.contacted_at = Time.now + runner.contacted_at = Time.current end it 'updates cache' do diff --git a/spec/models/clusters/applications/elastic_stack_spec.rb b/spec/models/clusters/applications/elastic_stack_spec.rb index 02ada219e32..50042a4e29a 100644 --- a/spec/models/clusters/applications/elastic_stack_spec.rb +++ b/spec/models/clusters/applications/elastic_stack_spec.rb @@ -175,6 +175,7 @@ describe Clusters::Applications::ElasticStack do expect(faraday_connection.headers["Authorization"]).to eq(kube_client.headers[:Authorization]) expect(faraday_connection.ssl.cert_store).to be_instance_of(OpenSSL::X509::Store) expect(faraday_connection.ssl.verify).to eq(1) + expect(faraday_connection.options.timeout).to be_nil end context 'when cluster is not reachable' do @@ -186,6 +187,15 @@ describe Clusters::Applications::ElasticStack do expect(subject.elasticsearch_client).to be_nil end end + + context 'when timeout is provided' do + it 'sets timeout in elasticsearch_client' do + client = subject.elasticsearch_client(timeout: 123) + faraday_connection = client.transport.connections.first.connection + + expect(faraday_connection.options.timeout).to eq(123) + end + end end end end diff --git a/spec/models/clusters/applications/prometheus_spec.rb b/spec/models/clusters/applications/prometheus_spec.rb index ce341e67c14..1ed9e207b6b 100644 --- a/spec/models/clusters/applications/prometheus_spec.rb +++ b/spec/models/clusters/applications/prometheus_spec.rb @@ -47,7 +47,7 @@ describe Clusters::Applications::Prometheus do it 'sets last_update_started_at to now' do Timecop.freeze do - expect { subject.make_updating }.to change { subject.reload.last_update_started_at }.to be_within(1.second).of(Time.now) + expect { subject.make_updating }.to change { subject.reload.last_update_started_at }.to be_within(1.second).of(Time.current) end end end @@ -347,14 +347,14 @@ describe Clusters::Applications::Prometheus do describe '#updated_since?' do let(:cluster) { create(:cluster) } let(:prometheus_app) { build(:clusters_applications_prometheus, cluster: cluster) } - let(:timestamp) { Time.now - 5.minutes } + let(:timestamp) { Time.current - 5.minutes } around do |example| Timecop.freeze { example.run } end before do - prometheus_app.last_update_started_at = Time.now + prometheus_app.last_update_started_at = Time.current end context 'when app does not have status failed' do @@ -363,7 +363,7 @@ describe Clusters::Applications::Prometheus do end it 'returns false when last update started before the timestamp' do - expect(prometheus_app.updated_since?(Time.now + 5.minutes)).to be false + expect(prometheus_app.updated_since?(Time.current + 5.minutes)).to be false end end diff --git a/spec/models/clusters/cluster_spec.rb b/spec/models/clusters/cluster_spec.rb index 521ed98f637..4dd74976028 100644 --- a/spec/models/clusters/cluster_spec.rb +++ b/spec/models/clusters/cluster_spec.rb @@ -28,6 +28,8 @@ describe Clusters::Cluster, :use_clean_rails_memory_store_caching do it { is_expected.to have_one(:cluster_project) } it { is_expected.to have_many(:deployment_clusters) } it { is_expected.to have_many(:metrics_dashboard_annotations) } + it { is_expected.to have_many(:successful_deployments) } + it { is_expected.to have_many(:environments).through(:deployments) } it { is_expected.to delegate_method(:status).to(:provider) } it { is_expected.to delegate_method(:status_reason).to(:provider) } @@ -172,6 +174,108 @@ describe Clusters::Cluster, :use_clean_rails_memory_store_caching do end end + describe '.with_application_prometheus' do + subject { described_class.with_application_prometheus } + + let!(:cluster) { create(:cluster) } + + context 'cluster has prometheus application' do + let!(:application) { create(:clusters_applications_prometheus, :installed, cluster: cluster) } + + it { is_expected.to include(cluster) } + end + + context 'cluster does not have prometheus application' do + let(:cluster) { create(:cluster) } + + it { is_expected.not_to include(cluster) } + end + end + + describe '.with_enabled_modsecurity' do + subject { described_class.with_enabled_modsecurity } + + let_it_be(:cluster) { create(:cluster) } + + context 'cluster has ingress application with enabled modsecurity' do + let!(:application) { create(:clusters_applications_ingress, :installed, :modsecurity_logging, cluster: cluster) } + + it { is_expected.to include(cluster) } + end + + context 'cluster has ingress application with disabled modsecurity' do + let!(:application) { create(:clusters_applications_ingress, :installed, :modsecurity_disabled, cluster: cluster) } + + it { is_expected.not_to include(cluster) } + end + + context 'cluster does not have ingress application' do + it { is_expected.not_to include(cluster) } + end + end + + describe '.with_available_elasticstack' do + subject { described_class.with_available_elasticstack } + + let_it_be(:cluster) { create(:cluster) } + + context 'cluster has ElasticStack application' do + let!(:application) { create(:clusters_applications_elastic_stack, :installed, cluster: cluster) } + + it { is_expected.to include(cluster) } + end + + context 'cluster does not have ElasticStack application' do + it { is_expected.not_to include(cluster) } + end + end + + describe '.distinct_with_deployed_environments' do + subject { described_class.distinct_with_deployed_environments } + + let_it_be(:cluster) { create(:cluster) } + + context 'cluster has multiple successful deployment with environment' do + let!(:environment) { create(:environment) } + let!(:deployment) { create(:deployment, :success, cluster: cluster, environment: environment) } + let!(:deployment_2) { create(:deployment, :success, cluster: cluster, environment: environment) } + + it { is_expected.to include(cluster) } + + it 'lists only distinct environments' do + expect(subject.first.environments.count).to eq(1) + end + end + + context 'cluster has only failed deployment with environment' do + let!(:environment) { create(:environment) } + let!(:deployment) { create(:deployment, :failed, cluster: cluster, environment: environment) } + + it { is_expected.not_to include(cluster) } + end + + context 'cluster does not have any deployment' do + it { is_expected.not_to include(cluster) } + end + end + + describe '.with_project_alert_service_data' do + subject { described_class.with_project_alert_service_data(project_id) } + + let!(:cluster) { create(:cluster, :project) } + let!(:project_id) { cluster.first_project.id } + + context 'project has alert service data' do + let!(:alerts_service) { create(:alerts_service, project: cluster.clusterable) } + + it { is_expected.to include(cluster) } + end + + context 'project has no alert service data' do + it { is_expected.not_to include(cluster) } + end + end + describe '.for_project_namespace' do subject { described_class.for_project_namespace(namespace_id) } @@ -573,19 +677,12 @@ describe Clusters::Cluster, :use_clean_rails_memory_store_caching do end context 'when applications are created' do - let!(:helm) { create(:clusters_applications_helm, cluster: cluster) } - let!(:ingress) { create(:clusters_applications_ingress, cluster: cluster) } - let!(:cert_manager) { create(:clusters_applications_cert_manager, cluster: cluster) } - let!(:crossplane) { create(:clusters_applications_crossplane, cluster: cluster) } - let!(:prometheus) { create(:clusters_applications_prometheus, cluster: cluster) } - let!(:runner) { create(:clusters_applications_runner, cluster: cluster) } - let!(:jupyter) { create(:clusters_applications_jupyter, cluster: cluster) } - let!(:knative) { create(:clusters_applications_knative, cluster: cluster) } - let!(:elastic_stack) { create(:clusters_applications_elastic_stack, cluster: cluster) } - let!(:fluentd) { create(:clusters_applications_fluentd, cluster: cluster) } + let(:cluster) { create(:cluster, :with_all_applications) } - it 'returns a list of created applications' do - is_expected.to contain_exactly(helm, ingress, cert_manager, crossplane, prometheus, runner, jupyter, knative, elastic_stack, fluentd) + it 'returns a list of created applications', :aggregate_failures do + is_expected.to have_attributes(size: described_class::APPLICATIONS.size) + is_expected.to all(be_kind_of(::Clusters::Concerns::ApplicationCore)) + is_expected.to all(be_persisted) end end end @@ -611,33 +708,13 @@ describe Clusters::Cluster, :use_clean_rails_memory_store_caching do end context 'when application is persisted' do - let!(:helm) { create(:clusters_applications_helm, cluster: cluster) } - let!(:ingress) { create(:clusters_applications_ingress, cluster: cluster) } - let!(:cert_manager) { create(:clusters_applications_cert_manager, cluster: cluster) } - let!(:crossplane) { create(:clusters_applications_crossplane, cluster: cluster) } - let!(:prometheus) { create(:clusters_applications_prometheus, cluster: cluster) } - let!(:runner) { create(:clusters_applications_runner, cluster: cluster) } - let!(:jupyter) { create(:clusters_applications_jupyter, cluster: cluster) } - let!(:knative) { create(:clusters_applications_knative, cluster: cluster) } - let!(:elastic_stack) { create(:clusters_applications_elastic_stack, cluster: cluster) } - let!(:fluentd) { create(:clusters_applications_fluentd, cluster: cluster) } + let(:cluster) { create(:cluster, :with_all_applications) } it 'returns the persisted application', :aggregate_failures do - { - Clusters::Applications::Helm => helm, - Clusters::Applications::Ingress => ingress, - Clusters::Applications::CertManager => cert_manager, - Clusters::Applications::Crossplane => crossplane, - Clusters::Applications::Prometheus => prometheus, - Clusters::Applications::Runner => runner, - Clusters::Applications::Jupyter => jupyter, - Clusters::Applications::Knative => knative, - Clusters::Applications::ElasticStack => elastic_stack, - Clusters::Applications::Fluentd => fluentd - }.each do |application_class, expected_object| + described_class::APPLICATIONS.each_value do |application_class| application = cluster.find_or_build_application(application_class) - expect(application).to eq(expected_object) + expect(application).to be_kind_of(::Clusters::Concerns::ApplicationCore) expect(application).to be_persisted end end @@ -1049,7 +1126,7 @@ describe Clusters::Cluster, :use_clean_rails_memory_store_caching do .and_raise(SocketError) end - it { is_expected.to eq(connection_status: :unreachable, nodes: []) } + it { is_expected.to eq(connection_status: :unreachable, nodes: nil) } end context 'cluster cannot be authenticated to' do @@ -1058,7 +1135,7 @@ describe Clusters::Cluster, :use_clean_rails_memory_store_caching do .and_raise(OpenSSL::X509::CertificateError.new("Certificate error")) end - it { is_expected.to eq(connection_status: :authentication_failure, nodes: []) } + it { is_expected.to eq(connection_status: :authentication_failure, nodes: nil) } end describe 'Kubeclient::HttpError' do @@ -1070,18 +1147,18 @@ describe Clusters::Cluster, :use_clean_rails_memory_store_caching do .and_raise(Kubeclient::HttpError.new(error_code, error_message, nil)) end - it { is_expected.to eq(connection_status: :authentication_failure, nodes: []) } + it { is_expected.to eq(connection_status: :authentication_failure, nodes: nil) } context 'generic timeout' do let(:error_message) { 'Timed out connecting to server'} - it { is_expected.to eq(connection_status: :unreachable, nodes: []) } + it { is_expected.to eq(connection_status: :unreachable, nodes: nil) } end context 'gateway timeout' do let(:error_message) { '504 Gateway Timeout for GET https://kubernetes.example.com/api/v1'} - it { is_expected.to eq(connection_status: :unreachable, nodes: []) } + it { is_expected.to eq(connection_status: :unreachable, nodes: nil) } end end @@ -1091,7 +1168,7 @@ describe Clusters::Cluster, :use_clean_rails_memory_store_caching do .and_raise(StandardError) end - it { is_expected.to eq(connection_status: :unknown_failure, nodes: []) } + it { is_expected.to eq(connection_status: :unknown_failure, nodes: nil) } it 'notifies Sentry' do expect(Gitlab::ErrorTracking).to receive(:track_exception) diff --git a/spec/models/commit_status_spec.rb b/spec/models/commit_status_spec.rb index 05d3329215a..85fc503a1ca 100644 --- a/spec/models/commit_status_spec.rb +++ b/spec/models/commit_status_spec.rb @@ -96,6 +96,21 @@ describe CommitStatus do it { is_expected.to be_truthy } end + + it "processed state is always persisted" do + commit_status.update!(retried: false, status: :pending) + + # another process does mark object as processed + CommitStatus.find(commit_status.id).update_column(:processed, true) + + # subsequent status transitions on the same instance + # always saves processed=false to DB even though + # the current value did not change + commit_status.update!(retried: false, status: :running) + + # we look at a persisted state in DB + expect(CommitStatus.find(commit_status.id).processed).to eq(false) + end end end @@ -235,7 +250,7 @@ describe CommitStatus do context 'if the building process has started' do before do - commit_status.started_at = Time.now - 1.minute + commit_status.started_at = Time.current - 1.minute commit_status.finished_at = nil end @@ -708,7 +723,7 @@ describe CommitStatus do end describe '#enqueue' do - let!(:current_time) { Time.new(2018, 4, 5, 14, 0, 0) } + let!(:current_time) { Time.zone.local(2018, 4, 5, 14, 0, 0) } before do allow(Time).to receive(:now).and_return(current_time) diff --git a/spec/models/concerns/bulk_insert_safe_spec.rb b/spec/models/concerns/bulk_insert_safe_spec.rb index 5d65d614ac5..07d6cee487f 100644 --- a/spec/models/concerns/bulk_insert_safe_spec.rb +++ b/spec/models/concerns/bulk_insert_safe_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe BulkInsertSafe do - class BulkInsertItem < ApplicationRecord + class BulkInsertItem < ActiveRecord::Base include BulkInsertSafe include ShaAttribute @@ -74,6 +74,8 @@ describe BulkInsertSafe do ActiveRecord::Schema.define do drop_table :bulk_insert_items, force: true end + + BulkInsertItem.reset_column_information end describe BulkInsertItem do diff --git a/spec/models/concerns/cacheable_attributes_spec.rb b/spec/models/concerns/cacheable_attributes_spec.rb index 56e0d044247..6694b2aba22 100644 --- a/spec/models/concerns/cacheable_attributes_spec.rb +++ b/spec/models/concerns/cacheable_attributes_spec.rb @@ -135,7 +135,7 @@ describe CacheableAttributes do end it 'returns an uncached record and logs a warning' do - expect(Rails.logger).to receive(:warn).with("Cached record for TestClass couldn't be loaded, falling back to uncached record: Redis::BaseError") + expect(Gitlab::AppLogger).to receive(:warn).with("Cached record for TestClass couldn't be loaded, falling back to uncached record: Redis::BaseError") expect(MinimalTestClass.current).to eq(:last) end @@ -147,7 +147,7 @@ describe CacheableAttributes do end it 'returns an uncached record and logs a warning' do - expect(Rails.logger).not_to receive(:warn) + expect(Gitlab::AppLogger).not_to receive(:warn) expect { MinimalTestClass.current }.to raise_error(Redis::BaseError) end diff --git a/spec/models/concerns/each_batch_spec.rb b/spec/models/concerns/each_batch_spec.rb index 294fde4f8e6..ee3d9aea505 100644 --- a/spec/models/concerns/each_batch_spec.rb +++ b/spec/models/concerns/each_batch_spec.rb @@ -44,7 +44,7 @@ describe EachBatch do end it 'allows updating of the yielded relations' do - time = Time.now + time = Time.current model.each_batch do |relation| relation.update_all(updated_at: time) diff --git a/spec/models/concerns/featurable_spec.rb b/spec/models/concerns/featurable_spec.rb new file mode 100644 index 00000000000..89720e3652c --- /dev/null +++ b/spec/models/concerns/featurable_spec.rb @@ -0,0 +1,184 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Featurable do + let_it_be(:user) { create(:user) } + let(:project) { create(:project) } + let(:feature_class) { subject.class } + let(:features) { feature_class::FEATURES } + + subject { project.project_feature } + + describe '.quoted_access_level_column' do + it 'returns the table name and quoted column name for a feature' do + expected = '"project_features"."issues_access_level"' + + expect(feature_class.quoted_access_level_column(:issues)).to eq(expected) + end + end + + describe '.access_level_attribute' do + it { expect(feature_class.access_level_attribute(:wiki)).to eq :wiki_access_level } + + it 'raises error for unspecified feature' do + expect { feature_class.access_level_attribute(:unknown) } + .to raise_error(ArgumentError, /invalid feature: unknown/) + end + end + + describe '.set_available_features' do + let!(:klass) do + Class.new do + include Featurable + set_available_features %i(feature1 feature2) + + def feature1_access_level + Featurable::DISABLED + end + + def feature2_access_level + Featurable::ENABLED + end + end + end + let!(:instance) { klass.new } + + it { expect(klass.available_features).to eq [:feature1, :feature2] } + it { expect(instance.feature1_enabled?).to be_falsey } + it { expect(instance.feature2_enabled?).to be_truthy } + end + + describe '.available_features' do + it { expect(feature_class.available_features).to include(*features) } + end + + describe '#access_level' do + it 'returns access level' do + expect(subject.access_level(:wiki)).to eq(subject.wiki_access_level) + end + end + + describe '#feature_available?' do + let(:features) { %w(issues wiki builds merge_requests snippets repository pages metrics_dashboard) } + + context 'when features are disabled' do + it "returns false" do + update_all_project_features(project, features, ProjectFeature::DISABLED) + + features.each do |feature| + expect(project.feature_available?(feature.to_sym, user)).to eq(false), "#{feature} failed" + end + end + end + + context 'when features are enabled only for team members' do + it "returns false when user is not a team member" do + update_all_project_features(project, features, ProjectFeature::PRIVATE) + + features.each do |feature| + expect(project.feature_available?(feature.to_sym, user)).to eq(false), "#{feature} failed" + end + end + + it "returns true when user is a team member" do + project.add_developer(user) + + update_all_project_features(project, features, ProjectFeature::PRIVATE) + + features.each do |feature| + expect(project.feature_available?(feature.to_sym, user)).to eq(true), "#{feature} failed" + end + end + + it "returns true when user is a member of project group" do + group = create(:group) + project = create(:project, namespace: group) + group.add_developer(user) + + update_all_project_features(project, features, ProjectFeature::PRIVATE) + + features.each do |feature| + expect(project.feature_available?(feature.to_sym, user)).to eq(true), "#{feature} failed" + end + end + + context 'when admin mode is enabled', :enable_admin_mode do + it "returns true if user is an admin" do + user.update_attribute(:admin, true) + + update_all_project_features(project, features, ProjectFeature::PRIVATE) + + features.each do |feature| + expect(project.feature_available?(feature.to_sym, user)).to eq(true), "#{feature} failed" + end + end + end + + context 'when admin mode is disabled' do + it "returns false when user is an admin" do + user.update_attribute(:admin, true) + + update_all_project_features(project, features, ProjectFeature::PRIVATE) + + features.each do |feature| + expect(project.feature_available?(feature.to_sym, user)).to eq(false), "#{feature} failed" + end + end + end + end + + context 'when feature is enabled for everyone' do + it "returns true" do + expect(project.feature_available?(:issues, user)).to eq(true) + end + end + + context 'when feature is disabled by a feature flag' do + it 'returns false' do + stub_feature_flags(issues: false) + + expect(project.feature_available?(:issues, user)).to eq(false) + end + end + + context 'when feature is enabled by a feature flag' do + it 'returns true' do + stub_feature_flags(issues: true) + + expect(project.feature_available?(:issues, user)).to eq(true) + end + end + end + + describe '#*_enabled?' do + let(:features) { %w(wiki builds merge_requests) } + + it "returns false when feature is disabled" do + update_all_project_features(project, features, ProjectFeature::DISABLED) + + features.each do |feature| + expect(project.public_send("#{feature}_enabled?")).to eq(false), "#{feature} failed" + end + end + + it "returns true when feature is enabled only for team members" do + update_all_project_features(project, features, ProjectFeature::PRIVATE) + + features.each do |feature| + expect(project.public_send("#{feature}_enabled?")).to eq(true), "#{feature} failed" + end + end + + it "returns true when feature is enabled for everyone" do + features.each do |feature| + expect(project.public_send("#{feature}_enabled?")).to eq(true), "#{feature} failed" + end + end + end + + def update_all_project_features(project, features, value) + project_feature_attributes = features.map { |f| ["#{f}_access_level", value] }.to_h + project.project_feature.update(project_feature_attributes) + end +end diff --git a/spec/models/concerns/issuable_spec.rb b/spec/models/concerns/issuable_spec.rb index 24908785320..74ee7a87b7b 100644 --- a/spec/models/concerns/issuable_spec.rb +++ b/spec/models/concerns/issuable_spec.rb @@ -102,6 +102,22 @@ describe Issuable do end end + describe '.any_label' do + let_it_be(:issue_with_label) { create(:labeled_issue, labels: [create(:label)]) } + let_it_be(:issue_with_multiple_labels) { create(:labeled_issue, labels: [create(:label), create(:label)]) } + let_it_be(:issue_without_label) { create(:issue) } + + it 'returns an issuable with at least one label' do + expect(issuable_class.any_label).to match_array([issue_with_label, issue_with_multiple_labels]) + end + + context 'for custom sorting' do + it 'returns an issuable with at least one label' do + expect(issuable_class.any_label('created_at')).to eq([issue_with_label, issue_with_multiple_labels]) + end + end + end + describe ".search" do let!(:searchable_issue) { create(:issue, title: "Searchable awesome issue") } let!(:searchable_issue2) { create(:issue, title: 'Aw') } @@ -422,7 +438,7 @@ describe Issuable do context 'total_time_spent is updated' do before do - issue.spend_time(duration: 2, user_id: user.id, spent_at: Time.now) + issue.spend_time(duration: 2, user_id: user.id, spent_at: Time.current) issue.save expect(Gitlab::HookData::IssuableBuilder) .to receive(:new).with(issue).and_return(builder) @@ -572,8 +588,8 @@ describe Issuable do second_priority = create(:label, project: project, priority: 2) no_priority = create(:label, project: project) - first_milestone = create(:milestone, project: project, due_date: Time.now) - second_milestone = create(:milestone, project: project, due_date: Time.now + 1.month) + first_milestone = create(:milestone, project: project, due_date: Time.current) + second_milestone = create(:milestone, project: project, due_date: Time.current + 1.month) third_milestone = create(:milestone, project: project) # The issues here are ordered by label priority, to ensure that we don't diff --git a/spec/models/concerns/limitable_spec.rb b/spec/models/concerns/limitable_spec.rb new file mode 100644 index 00000000000..ca0a257be7a --- /dev/null +++ b/spec/models/concerns/limitable_spec.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Limitable do + let(:minimal_test_class) do + Class.new do + include ActiveModel::Model + + def self.name + 'TestClass' + end + + include Limitable + end + end + + before do + stub_const("MinimalTestClass", minimal_test_class) + end + + it { expect(MinimalTestClass.limit_name).to eq('test_classes') } + + context 'with scoped limit' do + before do + MinimalTestClass.limit_scope = :project + end + + it { expect(MinimalTestClass.limit_scope).to eq(:project) } + + it 'triggers scoped validations' do + instance = MinimalTestClass.new + + expect(instance).to receive(:validate_scoped_plan_limit_not_exceeded) + + instance.valid?(:create) + end + end + + context 'with global limit' do + before do + MinimalTestClass.limit_scope = Limitable::GLOBAL_SCOPE + end + + it { expect(MinimalTestClass.limit_scope).to eq(Limitable::GLOBAL_SCOPE) } + + it 'triggers scoped validations' do + instance = MinimalTestClass.new + + expect(instance).to receive(:validate_global_plan_limit_not_exceeded) + + instance.valid?(:create) + end + end +end diff --git a/spec/models/concerns/milestoneish_spec.rb b/spec/models/concerns/milestoneish_spec.rb index 81f173cd23a..8c43a12aa15 100644 --- a/spec/models/concerns/milestoneish_spec.rb +++ b/spec/models/concerns/milestoneish_spec.rb @@ -290,13 +290,13 @@ describe Milestone, 'Milestoneish' do end it 'shows 0 if start_date is a future' do - milestone = build_stubbed(:milestone, start_date: Time.now + 2.days) + milestone = build_stubbed(:milestone, start_date: Time.current + 2.days) expect(milestone.elapsed_days).to eq(0) end it 'shows correct amount of days' do - milestone = build_stubbed(:milestone, start_date: Time.now - 2.days) + milestone = build_stubbed(:milestone, start_date: Time.current - 2.days) expect(milestone.elapsed_days).to eq(2) end diff --git a/spec/models/concerns/resolvable_discussion_spec.rb b/spec/models/concerns/resolvable_discussion_spec.rb index 9ea01ca9002..95553fb13a6 100644 --- a/spec/models/concerns/resolvable_discussion_spec.rb +++ b/spec/models/concerns/resolvable_discussion_spec.rb @@ -6,10 +6,10 @@ describe Discussion, ResolvableDiscussion do subject { described_class.new([first_note, second_note, third_note]) } let(:first_note) { create(:discussion_note_on_merge_request) } - let(:merge_request) { first_note.noteable } + let(:noteable) { first_note.noteable } let(:project) { first_note.project } - let(:second_note) { create(:discussion_note_on_merge_request, noteable: merge_request, project: project, in_reply_to: first_note) } - let(:third_note) { create(:discussion_note_on_merge_request, noteable: merge_request, project: project) } + let(:second_note) { create(:discussion_note_on_merge_request, noteable: noteable, project: project, in_reply_to: first_note) } + let(:third_note) { create(:discussion_note_on_merge_request, noteable: noteable, project: project) } describe "#resolvable?" do context "when potentially resolvable" do @@ -198,12 +198,26 @@ describe Discussion, ResolvableDiscussion do it "returns true" do expect(subject.can_resolve?(current_user)).to be true end + + context "when the noteable has no author" do + it "returns true" do + expect(noteable).to receive(:author).and_return(nil) + expect(subject.can_resolve?(current_user)).to be true + end + end end context "when the signed in user is a random user" do it "returns false" do expect(subject.can_resolve?(current_user)).to be false end + + context "when the noteable has no author" do + it "returns false" do + expect(noteable).to receive(:author).and_return(nil) + expect(subject.can_resolve?(current_user)).to be false + end + end end end end @@ -536,7 +550,7 @@ describe Discussion, ResolvableDiscussion do describe "#last_resolved_note" do let(:current_user) { create(:user) } - let(:time) { Time.now.utc } + let(:time) { Time.current.utc } before do Timecop.freeze(time - 1.second) do diff --git a/spec/models/concerns/sortable_spec.rb b/spec/models/concerns/sortable_spec.rb index 18ac4d19938..a1fe5c0928d 100644 --- a/spec/models/concerns/sortable_spec.rb +++ b/spec/models/concerns/sortable_spec.rb @@ -91,7 +91,7 @@ describe Sortable do Group.all.order_by(order).map(&:name) end - let!(:ref_time) { Time.parse('2018-05-01 00:00:00') } + let!(:ref_time) { Time.zone.parse('2018-05-01 00:00:00') } let!(:group1) { create(:group, name: 'aa', id: 1, created_at: ref_time - 15.seconds, updated_at: ref_time) } let!(:group2) { create(:group, name: 'AAA', id: 2, created_at: ref_time - 10.seconds, updated_at: ref_time - 5.seconds) } let!(:group3) { create(:group, name: 'BB', id: 3, created_at: ref_time - 5.seconds, updated_at: ref_time - 10.seconds) } diff --git a/spec/models/container_expiration_policy_spec.rb b/spec/models/container_expiration_policy_spec.rb index c22362ed5d4..588685b04bf 100644 --- a/spec/models/container_expiration_policy_spec.rb +++ b/spec/models/container_expiration_policy_spec.rb @@ -37,6 +37,37 @@ RSpec.describe ContainerExpirationPolicy, type: :model do it { is_expected.to allow_value(nil).for(:keep_n) } it { is_expected.not_to allow_value('foo').for(:keep_n) } end + + context 'with a set of regexps' do + valid_regexps = %w[master .* v.+ v10.1.* (?:v.+|master|release)] + invalid_regexps = ['[', '(?:v.+|master|release'] + + valid_regexps.each do |valid_regexp| + it { is_expected.to allow_value(valid_regexp).for(:name_regex) } + it { is_expected.to allow_value(valid_regexp).for(:name_regex_keep) } + end + + invalid_regexps.each do |invalid_regexp| + it { is_expected.not_to allow_value(invalid_regexp).for(:name_regex) } + it { is_expected.not_to allow_value(invalid_regexp).for(:name_regex_keep) } + end + + context 'with a disabled container expiration policy' do + let_it_be(:container_expiration_policy) { create(:container_expiration_policy, :disabled) } + + subject { container_expiration_policy } + + valid_regexps.each do |valid_regexp| + it { is_expected.to allow_value(valid_regexp).for(:name_regex) } + it { is_expected.to allow_value(valid_regexp).for(:name_regex_keep) } + end + + invalid_regexps.each do |invalid_regexp| + it { is_expected.to allow_value(invalid_regexp).for(:name_regex) } + it { is_expected.to allow_value(invalid_regexp).for(:name_regex_keep) } + end + end + end end describe '.preloaded' do @@ -72,4 +103,14 @@ RSpec.describe ContainerExpirationPolicy, type: :model do end end end + + describe '#disable!' do + let_it_be(:container_expiration_policy) { create(:container_expiration_policy) } + + subject { container_expiration_policy.disable! } + + it 'disables the container expiration policy' do + expect { subject }.to change { container_expiration_policy.reload.enabled }.from(true).to(false) + end + end end diff --git a/spec/models/container_repository_spec.rb b/spec/models/container_repository_spec.rb index 1eecefe5d4a..4f23a905e93 100644 --- a/spec/models/container_repository_spec.rb +++ b/spec/models/container_repository_spec.rb @@ -81,6 +81,12 @@ describe ContainerRepository do end end + describe '#tags_count' do + it 'returns the count of tags' do + expect(repository.tags_count).to eq(1) + end + end + describe '#has_tags?' do it 'has tags' do expect(repository).to have_tags diff --git a/spec/models/deployment_spec.rb b/spec/models/deployment_spec.rb index 6eb006a5d67..ac2a4c9877d 100644 --- a/spec/models/deployment_spec.rb +++ b/spec/models/deployment_spec.rb @@ -114,7 +114,7 @@ describe Deployment do deployment.succeed! expect(deployment).to be_success - expect(deployment.finished_at).to be_like_time(Time.now) + expect(deployment.finished_at).to be_like_time(Time.current) end end @@ -141,7 +141,7 @@ describe Deployment do deployment.drop! expect(deployment).to be_failed - expect(deployment.finished_at).to be_like_time(Time.now) + expect(deployment.finished_at).to be_like_time(Time.current) end end @@ -161,7 +161,7 @@ describe Deployment do deployment.cancel! expect(deployment).to be_canceled - expect(deployment.finished_at).to be_like_time(Time.now) + expect(deployment.finished_at).to be_like_time(Time.current) end end @@ -587,7 +587,7 @@ describe Deployment do Timecop.freeze do deploy.update_status('success') - expect(deploy.read_attribute(:finished_at)).to eq(Time.now) + expect(deploy.read_attribute(:finished_at)).to eq(Time.current) end end end diff --git a/spec/models/design_management/design_spec.rb b/spec/models/design_management/design_spec.rb index 95782c1f674..bc1f54f057e 100644 --- a/spec/models/design_management/design_spec.rb +++ b/spec/models/design_management/design_spec.rb @@ -27,6 +27,7 @@ describe DesignManagement::Design do it { is_expected.to validate_presence_of(:project) } it { is_expected.to validate_presence_of(:issue) } it { is_expected.to validate_presence_of(:filename) } + it { is_expected.to validate_length_of(:filename).is_at_most(255) } it { is_expected.to validate_uniqueness_of(:filename).scoped_to(:issue_id) } it "validates that the extension is an image" do @@ -460,38 +461,6 @@ describe DesignManagement::Design do it 'uses the simple format' do expect(reference).to eq "#1[homescreen.jpg]" end - - context 'when the filename contains spaces, hyphens, periods, single-quotes, underscores and colons' do - let(:filename) { %q{a complex filename: containing - _ : etc., but still 'simple'.gif} } - - it 'uses the simple format' do - expect(reference).to eq "#1[#{filename}]" - end - end - - context 'when the filename contains HTML angle brackets' do - let(:filename) { 'a <em>great</em> filename.jpg' } - - it 'uses Base64 encoding' do - expect(reference).to eq "#1[base64:#{Base64.strict_encode64(filename)}]" - end - end - - context 'when the filename contains quotation marks' do - let(:filename) { %q{a "great" filename.jpg} } - - it 'uses enclosing quotes, with backslash encoding' do - expect(reference).to eq %q{#1["a \"great\" filename.jpg"]} - end - end - - context 'when the filename contains square brackets' do - let(:filename) { %q{a [great] filename.jpg} } - - it 'uses enclosing quotes' do - expect(reference).to eq %q{#1["a [great] filename.jpg"]} - end - end end context 'when full is true' do @@ -525,31 +494,55 @@ describe DesignManagement::Design do end describe 'reference_pattern' do - let(:match) { described_class.reference_pattern.match(ref) } - let(:ref) { design.to_reference } - let(:design) { build(:design, filename: filename) } + it 'is nil' do + expect(described_class.reference_pattern).to be_nil + end + end - context 'simple_file_name' do - let(:filename) { 'simple-file-name.jpg' } + describe 'link_reference_pattern' do + it 'is not nil' do + expect(described_class.link_reference_pattern).not_to be_nil + end + + it 'does not match the designs tab' do + expect(described_class.link_reference_pattern).not_to match(url_for_designs(issue)) + end - it 'matches :simple_file_name' do - expect(match[:simple_file_name]).to eq(filename) + where(:ext) do + (described_class::SAFE_IMAGE_EXT + described_class::DANGEROUS_IMAGE_EXT).flat_map do |ext| + [[ext], [ext.upcase]] end end - context 'quoted_file_name' do - let(:filename) { 'simple "file" name.jpg' } + with_them do + let(:filename) { "my-file.#{ext}" } + let(:design) { build(:design, filename: filename) } + let(:url) { url_for_design(design) } + let(:captures) { described_class.link_reference_pattern.match(url)&.named_captures } - it 'matches :simple_file_name' do - expect(match[:escaped_filename].gsub(/\\"/, '"')).to eq(filename) + it 'matches the URL' do + expect(captures).to include( + 'url_filename' => filename, + 'issue' => design.issue.iid.to_s, + 'namespace' => design.project.namespace.to_param, + 'project' => design.project.name + ) end - end - context 'Base64 name' do - let(:filename) { '<>.png' } + context 'the file needs to be encoded' do + let(:filename) { "my file.#{ext}" } - it 'matches base_64_encoded_name' do - expect(Base64.decode64(match[:base_64_encoded_name])).to eq(filename) + it 'extracts the encoded filename' do + expect(captures).to include('url_filename' => 'my%20file.' + ext) + end + end + + context 'the file is all upper case' do + let(:filename) { "file.#{ext}".upcase } + + it 'extracts the encoded filename' do + expect(captures).to include('url_filename' => filename) + end end end end diff --git a/spec/models/diff_note_spec.rb b/spec/models/diff_note_spec.rb index 65f06a5b270..8bfe2ac7a6c 100644 --- a/spec/models/diff_note_spec.rb +++ b/spec/models/diff_note_spec.rb @@ -238,12 +238,32 @@ describe DiffNote do end context 'when the discussion was created in the diff' do - it 'returns correct diff file' do - diff_file = subject.diff_file + context 'when file_identifier_hash is disabled' do + before do + stub_feature_flags(file_identifier_hash: false) + end - expect(diff_file.old_path).to eq(position.old_path) - expect(diff_file.new_path).to eq(position.new_path) - expect(diff_file.diff_refs).to eq(position.diff_refs) + it 'returns correct diff file' do + diff_file = subject.diff_file + + expect(diff_file.old_path).to eq(position.old_path) + expect(diff_file.new_path).to eq(position.new_path) + expect(diff_file.diff_refs).to eq(position.diff_refs) + end + end + + context 'when file_identifier_hash is enabled' do + before do + stub_feature_flags(file_identifier_hash: true) + end + + it 'returns correct diff file' do + diff_file = subject.diff_file + + expect(diff_file.old_path).to eq(position.old_path) + expect(diff_file.new_path).to eq(position.new_path) + expect(diff_file.diff_refs).to eq(position.diff_refs) + end end end diff --git a/spec/models/draft_note_spec.rb b/spec/models/draft_note_spec.rb new file mode 100644 index 00000000000..b880d3c5b97 --- /dev/null +++ b/spec/models/draft_note_spec.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe DraftNote do + include RepoHelpers + + let(:project) { create(:project, :repository) } + let(:merge_request) { create(:merge_request_with_diffs, target_project: project, source_project: project) } + + describe 'validations' do + it_behaves_like 'a valid diff positionable note', :draft_note + end + + describe 'delegations' do + it { is_expected.to delegate_method(:file_path).to(:diff_file).allow_nil } + it { is_expected.to delegate_method(:file_hash).to(:diff_file).allow_nil } + it { is_expected.to delegate_method(:file_identifier_hash).to(:diff_file).allow_nil } + end + + describe '#diff_file' do + let(:draft_note) { build(:draft_note, merge_request: merge_request) } + + context 'when diff_file exists' do + it "returns an unfolded diff_file" do + diff_file = instance_double(Gitlab::Diff::File) + expect(draft_note.original_position).to receive(:diff_file).with(project.repository).and_return(diff_file) + expect(diff_file).to receive(:unfold_diff_lines).with(draft_note.original_position) + + expect(draft_note.diff_file).to be diff_file + end + end + + context 'when diff_file does not exist' do + it 'returns nil' do + expect(draft_note.original_position).to receive(:diff_file).with(project.repository).and_return(nil) + + expect(draft_note.diff_file).to be_nil + end + end + end +end diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index c0b2a4ae984..b93da518b68 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -1061,7 +1061,7 @@ describe Environment, :use_clean_rails_memory_store_caching do end context 'when time window arguments are provided' do - let(:metric_params) { [1552642245.067, Time.now] } + let(:metric_params) { [1552642245.067, Time.current] } it 'queries with the expected parameters' do expect(environment.prometheus_adapter) diff --git a/spec/models/event_spec.rb b/spec/models/event_spec.rb index ac89f8fe9e1..14066b1e9d2 100644 --- a/spec/models/event_spec.rb +++ b/spec/models/event_spec.rb @@ -13,6 +13,7 @@ describe Event do it { is_expected.to respond_to(:author_email) } it { is_expected.to respond_to(:issue_title) } it { is_expected.to respond_to(:merge_request_title) } + it { is_expected.to respond_to(:design_title) } end describe 'Callbacks' do @@ -37,7 +38,7 @@ describe Event do project.reload - expect(project.last_repository_updated_at).to be_within(1.minute).of(Time.now) + expect(project.last_repository_updated_at).to be_within(1.minute).of(Time.current) end end @@ -67,19 +68,32 @@ describe Event do end end - describe 'after_create :track_user_interacted_projects' do + describe 'after_create UserInteractedProject.track' do let(:event) { build(:push_event, project: project, author: project.owner) } it 'passes event to UserInteractedProject.track' do - expect(UserInteractedProject).to receive(:available?).and_return(true) expect(UserInteractedProject).to receive(:track).with(event) event.save end + end + end - it 'does not call UserInteractedProject.track if its not yet available' do - expect(UserInteractedProject).to receive(:available?).and_return(false) - expect(UserInteractedProject).not_to receive(:track) - event.save + describe 'validations' do + describe 'action' do + context 'for a design' do + where(:action, :valid) do + valid = described_class::DESIGN_ACTIONS.map(&:to_s).to_set + + described_class.actions.keys.map do |action| + [action, valid.include?(action)] + end + end + + with_them do + let(:event) { build(:design_event, action: action) } + + specify { expect(event.valid?).to eq(valid) } + end end end end @@ -552,7 +566,7 @@ describe Event do end end - context 'design event' do + context 'design note event' do include DesignManagementTestHelpers let(:target) { note_on_design } @@ -577,6 +591,32 @@ describe Event do include_examples 'visible to assignee and author', true end end + + context 'design event' do + include DesignManagementTestHelpers + + let(:target) { design } + + before do + enable_design_management + end + + include_examples 'visibility examples' do + let(:visibility) { visible_to_all } + end + + include_examples 'visible to assignee and author', true + + context 'the event refers to a design on a confidential issue' do + let(:design) { create(:design, issue: confidential_issue, project: project) } + + include_examples 'visibility examples' do + let(:visibility) { visible_to_none_except(:member, :admin) } + end + + include_examples 'visible to assignee and author', true + end + end end describe 'wiki_page predicate scopes' do @@ -587,10 +627,31 @@ describe Event do create(:wiki_page_event), create(:closed_issue_event), create(:event, :created), - create(:wiki_page_event) + create(:design_event, :destroyed), + create(:wiki_page_event), + create(:design_event) ] end + describe '.for_design' do + it 'only includes design events' do + design_events = events.select(&:design?) + + expect(described_class.for_design) + .to be_present + .and match_array(design_events) + end + end + + describe '.not_design' do + it 'does not contain the design events' do + non_design_events = events.reject(&:design?) + + expect(events).not_to match_array(non_design_events) + expect(described_class.not_design).to match_array(non_design_events) + end + end + describe '.for_wiki_page' do it 'only contains the wiki page events' do wiki_events = events.select(&:wiki_page?) @@ -618,26 +679,76 @@ describe Event do end end - describe '#wiki_page and #wiki_page?' do + describe 'categorization' do let_it_be(:project) { create(:project, :repository) } + let_it_be(:all_valid_events) do + # mapping from factory name to whether we need to supply the project + valid_target_factories = { + issue: true, + note_on_issue: true, + user: false, + merge_request: true, + note_on_merge_request: true, + project_snippet: true, + personal_snippet: false, + note_on_project_snippet: true, + note_on_personal_snippet: false, + wiki_page_meta: true, + milestone: true, + project: false, + design: true, + note_on_design: true, + note_on_commit: true + } + valid_target_factories.map do |kind, needs_project| + extra_data = needs_project ? { project: project } : {} + target = kind == :project ? nil : build(kind, **extra_data) + [kind, build(:event, :created, project: project, target: target)] + end.to_h + end + + it 'passes a sanity check', :aggregate_failures do + expect(all_valid_events.values).to all(be_valid) + end - context 'for a wiki page event' do - let(:wiki_page) do - create(:wiki_page, project: project) + describe '#wiki_page and #wiki_page?' do + context 'for a wiki page event' do + let(:wiki_page) do + create(:wiki_page, project: project) + end + + subject(:event) { create(:wiki_page_event, project: project, wiki_page: wiki_page) } + + it { is_expected.to have_attributes(wiki_page?: be_truthy, wiki_page: wiki_page) } end - subject(:event) { create(:wiki_page_event, project: project, wiki_page: wiki_page) } + context 'for any other event' do + it 'has no wiki_page and is not a wiki_page', :aggregate_failures do + all_valid_events.each do |k, event| + next if k == :wiki_page_meta - it { is_expected.to have_attributes(wiki_page?: be_truthy, wiki_page: wiki_page) } + expect(event).to have_attributes(wiki_page: be_nil, wiki_page?: be_falsy) + end + end + end end - [:issue, :user, :merge_request, :snippet, :milestone, nil].each do |kind| - context "for a #{kind} event" do - it 'is nil' do - target = create(kind) if kind - event = create(:event, project: project, target: target) + describe '#design and #design?' do + context 'for a design event' do + let(:design) { build(:design, project: project) } + + subject(:event) { build(:design_event, target: design, project: project) } + + it { is_expected.to have_attributes(design?: be_truthy, design: design) } + end + + context 'for any other event' do + it 'has no design and is not a design', :aggregate_failures do + all_valid_events.each do |k, event| + next if k == :design - expect(event).to have_attributes(wiki_page: be_nil, wiki_page?: be_falsy) + expect(event).to have_attributes(design: be_nil, design?: be_falsy) + end end end end @@ -665,7 +776,7 @@ 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.now) + project.update(last_activity_at: Time.current) expect(project).not_to receive(:update_column) .with(:last_activity_at, a_kind_of(Time)) @@ -682,7 +793,7 @@ describe Event do project.reload - expect(project.last_activity_at).to be_within(1.minute).of(Time.now) + expect(project.last_activity_at).to be_within(1.minute).of(Time.current) end end end @@ -765,6 +876,19 @@ describe Event do end end + describe '#action_name' do + it 'handles all valid design events' do + created, updated, destroyed, archived = %i[created updated destroyed archived].map do |trait| + build(:design_event, trait).action_name + end + + expect(created).to eq('uploaded') + expect(updated).to eq('revised') + expect(destroyed).to eq('deleted') + expect(archived).to eq('archived') + end + end + def create_push_event(project, user) event = create(:push_event, project: project, author: user) diff --git a/spec/models/fork_network_member_spec.rb b/spec/models/fork_network_member_spec.rb index eab758248de..d7a0dd5be65 100644 --- a/spec/models/fork_network_member_spec.rb +++ b/spec/models/fork_network_member_spec.rb @@ -13,7 +13,7 @@ describe ForkNetworkMember do let(:fork_network) { fork_network_member.fork_network } it 'removes the fork network if it was the last member' do - fork_network.fork_network_members.destroy_all # rubocop: disable DestroyAll + fork_network.fork_network_members.destroy_all # rubocop: disable Cop/DestroyAll expect(ForkNetwork.count).to eq(0) end diff --git a/spec/models/global_milestone_spec.rb b/spec/models/global_milestone_spec.rb deleted file mode 100644 index 34dbdfec60d..00000000000 --- a/spec/models/global_milestone_spec.rb +++ /dev/null @@ -1,208 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -describe GlobalMilestone do - let(:user) { create(:user) } - let(:user2) { create(:user) } - let(:group) { create(:group) } - let(:project1) { create(:project, group: group) } - let(:project2) { create(:project, path: 'gitlab-ci', group: group) } - let(:project3) { create(:project, path: 'cookbook-gitlab', group: group) } - - describe '.build_collection' do - let(:milestone1_due_date) { 2.weeks.from_now.to_date } - - let!(:milestone1_project1) do - create( - :milestone, - title: "Milestone v1.2", - project: project1, - due_date: milestone1_due_date - ) - end - - let!(:milestone1_project2) do - create( - :milestone, - title: "Milestone v1.2", - project: project2, - due_date: milestone1_due_date - ) - end - - let!(:milestone1_project3) do - create( - :milestone, - title: "Milestone v1.2", - project: project3, - due_date: milestone1_due_date - ) - end - - let!(:milestone2_project1) do - create( - :milestone, - title: "VD-123", - project: project1, - due_date: nil - ) - end - - let!(:milestone2_project2) do - create( - :milestone, - title: "VD-123", - project: project2, - due_date: nil - ) - end - - let!(:milestone2_project3) do - create( - :milestone, - title: "VD-123", - project: project3, - due_date: nil - ) - end - - let!(:projects) do - [ - project1, - project2, - project3 - ] - end - - let!(:global_milestones) { described_class.build_collection(projects, {}) } - - context 'when building a collection of milestones' do - it 'has all project milestones' do - expect(global_milestones.count).to eq(6) - end - - it 'has all project milestones titles' do - expect(global_milestones.map(&:title)).to match_array(['Milestone v1.2', 'Milestone v1.2', 'Milestone v1.2', 'VD-123', 'VD-123', 'VD-123']) - end - - it 'has all project milestones' do - expect(global_milestones.size).to eq(6) - end - - it 'sorts collection by due date' do - expect(global_milestones.map(&:due_date)).to eq [milestone1_due_date, milestone1_due_date, milestone1_due_date, nil, nil, nil] - end - - it 'filters milestones by search_title when params[:search_title] is present' do - global_milestones = described_class.build_collection(projects, { search_title: 'v1.2' }) - - expect(global_milestones.map(&:title)).to match_array(['Milestone v1.2', 'Milestone v1.2', 'Milestone v1.2']) - end - end - - context 'when adding new milestones' do - it 'does not add more queries' do - control_count = ActiveRecord::QueryRecorder.new do - described_class.build_collection(projects, {}) - end.count - - create_list(:milestone, 3, project: project3) - - expect do - described_class.build_collection(projects, {}) - end.not_to exceed_all_query_limit(control_count) - end - end - end - - describe '.states_count' do - context 'when the projects have milestones' do - before do - create(:closed_milestone, title: 'Active Group Milestone', project: project3) - create(:active_milestone, title: 'Active Group Milestone', project: project1) - create(:active_milestone, title: 'Active Group Milestone', project: project2) - create(:closed_milestone, title: 'Closed Group Milestone', project: project1) - create(:closed_milestone, title: 'Closed Group Milestone', project: project2) - create(:closed_milestone, title: 'Closed Group Milestone', project: project3) - create(:closed_milestone, title: 'Closed Group Milestone 4', group: group) - end - - it 'returns the quantity of global milestones and group milestones in each possible state' do - expected_count = { opened: 2, closed: 5, all: 7 } - - count = described_class.states_count(Project.all, group) - - expect(count).to eq(expected_count) - end - - it 'returns the quantity of global milestones in each possible state' do - expected_count = { opened: 2, closed: 4, all: 6 } - - count = described_class.states_count(Project.all) - - expect(count).to eq(expected_count) - end - end - - context 'when the projects do not have milestones' do - before do - project1 - end - - it 'returns 0 as the quantity of global milestones in each state' do - expected_count = { opened: 0, closed: 0, all: 0 } - - count = described_class.states_count(Project.all) - - expect(count).to eq(expected_count) - end - end - end - - describe '#initialize' do - let(:milestone1_project1) { create(:milestone, title: "Milestone v1.2", project: project1) } - - subject(:global_milestone) { described_class.new(milestone1_project1) } - - it 'has exactly one group milestone' do - expect(global_milestone.title).to eq('Milestone v1.2') - end - - it 'has all project milestones with the same title' do - expect(global_milestone.milestone).to eq(milestone1_project1) - end - end - - describe '#safe_title' do - let(:milestone) { create(:milestone, title: "git / test", project: project1) } - - it 'strips out slashes and spaces' do - global_milestone = described_class.new(milestone) - - expect(global_milestone.safe_title).to eq('git-test') - end - end - - describe '#state' do - context 'when at least one milestone is active' do - it 'returns active' do - title = 'Active Group Milestone' - - global_milestone = described_class.new(create(:active_milestone, title: title)) - - expect(global_milestone.state).to eq('active') - end - end - - context 'when all milestones are closed' do - it 'returns closed' do - title = 'Closed Group Milestone' - - global_milestone = described_class.new(create(:closed_milestone, title: title)) - - expect(global_milestone.state).to eq('closed') - end - end - end -end diff --git a/spec/models/group_deploy_key_spec.rb b/spec/models/group_deploy_key_spec.rb new file mode 100644 index 00000000000..3ba56c7e504 --- /dev/null +++ b/spec/models/group_deploy_key_spec.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe GroupDeployKey do + it { is_expected.to validate_presence_of(:user) } + + it 'is of type DeployKey' do + expect(build(:group_deploy_key).type).to eq('DeployKey') + end +end diff --git a/spec/models/group_group_link_spec.rb b/spec/models/group_group_link_spec.rb index 1fbd399e82b..54e622b2f22 100644 --- a/spec/models/group_group_link_spec.rb +++ b/spec/models/group_group_link_spec.rb @@ -29,6 +29,32 @@ describe GroupGroupLink do ]) end end + + describe '.public_or_visible_to_user' do + let!(:user_with_access) { create :user } + let!(:user_without_access) { create :user } + let!(:shared_with_group) { create :group, :private } + let!(:shared_group) { create :group } + let!(:private_group_group_link) { create(:group_group_link, shared_group: shared_group, shared_with_group: shared_with_group) } + + before do + shared_group.add_owner(user_with_access) + shared_group.add_owner(user_without_access) + shared_with_group.add_developer(user_with_access) + end + + context 'when user can access shared group' do + it 'returns the private group' do + expect(described_class.public_or_visible_to_user(shared_group, user_with_access)).to include(private_group_group_link) + end + end + + context 'when user does not have access to shared group' do + it 'does not return private group' do + expect(described_class.public_or_visible_to_user(shared_group, user_without_access)).not_to include(private_group_group_link) + end + end + end end describe 'validation' do diff --git a/spec/models/group_import_state_spec.rb b/spec/models/group_import_state_spec.rb new file mode 100644 index 00000000000..9d9cb1e8391 --- /dev/null +++ b/spec/models/group_import_state_spec.rb @@ -0,0 +1,72 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe GroupImportState do + describe 'validations' do + let_it_be(:group) { create(:group) } + + it { is_expected.to validate_presence_of(:group) } + it { is_expected.to validate_presence_of(:status) } + + it 'can be created without a jid' do + import_state = build(:group_import_state, :created, group: group, jid: nil) + + expect(import_state).to be_valid + end + + it 'cannot be started without a jid' do + import_state = build(:group_import_state, :started, group: group, jid: nil) + + expect(import_state).not_to be_valid + expect(import_state.errors[:jid]).to include "can't be blank" + end + + it 'cannot be finished without a jid' do + import_state = build(:group_import_state, :finished, group: group, jid: nil) + + expect(import_state).not_to be_valid + expect(import_state.errors[:jid]).to include "can't be blank" + end + + it 'can fail without a jid' do + import_state = build(:group_import_state, :failed, group: group, jid: nil) + + expect(import_state).to be_valid + end + end + + describe '#in_progress?' do + context "when the import is 'created'" do + it "returns true" do + group_import_state = build(:group_import_state, :created) + + expect(group_import_state.in_progress?).to eq true + end + end + + context "when the import is 'started'" do + it "returns true" do + group_import_state = build(:group_import_state, :started) + + expect(group_import_state.in_progress?).to eq true + end + end + + context "when the import is 'finished'" do + it "returns false" do + group_import_state = build(:group_import_state, :finished) + + expect(group_import_state.in_progress?).to eq false + end + end + + context "when the import is 'failed'" do + it "returns false" do + group_import_state = build(:group_import_state, :failed) + + expect(group_import_state.in_progress?).to eq false + end + end + end +end diff --git a/spec/models/group_milestone_spec.rb b/spec/models/group_milestone_spec.rb deleted file mode 100644 index 01856870fe0..00000000000 --- a/spec/models/group_milestone_spec.rb +++ /dev/null @@ -1,57 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -describe GroupMilestone do - let(:group) { create(:group) } - let(:project) { create(:project, group: group) } - let(:project_milestone) do - create(:milestone, title: "Milestone v1.2", project: project) - end - - describe '.build' do - it 'returns milestone with group assigned' do - milestone = described_class.build( - group, - [project], - project_milestone.title - ) - - expect(milestone.group).to eq group - end - end - - describe '.build_collection' do - let(:group) { create(:group) } - let(:project1) { create(:project, group: group) } - let(:project2) { create(:project, path: 'gitlab-ci', group: group) } - let(:project3) { create(:project, path: 'cookbook-gitlab', group: group) } - - let!(:projects) do - [ - project1, - project2, - project3 - ] - end - - it 'returns array of milestones, each with group assigned' do - milestones = described_class.build_collection(group, [project], {}) - expect(milestones).to all(have_attributes(group: group)) - end - - context 'when adding new milestones' do - it 'does not add more queries' do - control_count = ActiveRecord::QueryRecorder.new do - described_class.build_collection(group, projects, {}) - end.count - - create(:milestone, title: 'This title', project: project1) - - expect do - described_class.build_collection(group, projects, {}) - end.not_to exceed_all_query_limit(control_count) - end - end - end -end diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index d6e76258491..93cb6d83489 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -662,6 +662,19 @@ describe Group do expect(group.members_with_parents).to include(developer) expect(group.members_with_parents).to include(maintainer) end + + context 'group sharing' do + let!(:shared_group) { create(:group) } + + before do + create(:group_group_link, shared_group: shared_group, shared_with_group: group) + end + + it 'returns shared with group members' do + expect(shared_group.members_with_parents).to( + include(developer)) + end + end end describe '#members_from_self_and_ancestors_with_effective_access_level' do @@ -800,6 +813,22 @@ describe Group do expect(group.user_ids_for_project_authorizations) .to include(maintainer.id, developer.id) end + + context 'group sharing' do + let_it_be(:group) { create(:group) } + let_it_be(:group_user) { create(:user) } + let_it_be(:shared_group) { create(:group) } + + before do + group.add_developer(group_user) + create(:group_group_link, shared_group: shared_group, shared_with_group: group) + end + + it 'returns the user IDs for shared with group members' do + expect(shared_group.user_ids_for_project_authorizations).to( + include(group_user.id)) + end + end end describe '#update_two_factor_requirement' do diff --git a/spec/models/hooks/system_hook_spec.rb b/spec/models/hooks/system_hook_spec.rb index 94f1b0cba2e..2e836c19e3c 100644 --- a/spec/models/hooks/system_hook_spec.rb +++ b/spec/models/hooks/system_hook_spec.rb @@ -75,7 +75,7 @@ describe SystemHook do it "project member destroy hook" do project.add_maintainer(user) - project.project_members.destroy_all # rubocop: disable DestroyAll + project.project_members.destroy_all # rubocop: disable Cop/DestroyAll expect(WebMock).to have_requested(:post, system_hook.url).with( body: /user_remove_from_team/, @@ -121,7 +121,7 @@ describe SystemHook do it 'group member destroy hook' do group.add_maintainer(user) - group.group_members.destroy_all # rubocop: disable DestroyAll + group.group_members.destroy_all # rubocop: disable Cop/DestroyAll expect(WebMock).to have_requested(:post, system_hook.url).with( body: /user_remove_from_group/, diff --git a/spec/models/instance_configuration_spec.rb b/spec/models/instance_configuration_spec.rb index 3e0181b8846..747e9dc2faa 100644 --- a/spec/models/instance_configuration_spec.rb +++ b/spec/models/instance_configuration_spec.rb @@ -110,7 +110,7 @@ describe InstanceConfiguration do end it 'expires after EXPIRATION_TIME' do - allow(Time).to receive(:now).and_return(Time.now + described_class::EXPIRATION_TIME) + allow(Time).to receive(:now).and_return(Time.current + described_class::EXPIRATION_TIME) Rails.cache.cleanup expect(Rails.cache.read(described_class::CACHE_KEY)).to eq(nil) diff --git a/spec/models/integration_spec.rb b/spec/models/integration_spec.rb new file mode 100644 index 00000000000..3042fd15a7b --- /dev/null +++ b/spec/models/integration_spec.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Integration do + let(:project_1) { create(:project) } + let(:project_2) { create(:project) } + let(:instance_integration) { create(:jira_service, :instance) } + + before do + create(:jira_service, project: project_1, inherit_from_id: instance_integration.id) + create(:jira_service, project: project_2, inherit_from_id: nil) + create(:slack_service, project: project_1, inherit_from_id: nil) + end + + describe '#with_custom_integration_for' do + it 'returns projects with custom integrations' do + expect(Project.with_custom_integration_for(instance_integration)).to contain_exactly(project_2) + end + end +end diff --git a/spec/models/internal_id_spec.rb b/spec/models/internal_id_spec.rb index 33d03bfc0f5..0dfb59cf43a 100644 --- a/spec/models/internal_id_spec.rb +++ b/spec/models/internal_id_spec.rb @@ -88,33 +88,6 @@ describe InternalId do expect(normalized).to eq((0..seq.size - 1).to_a) end - - context 'with an insufficient schema version' do - before do - described_class.reset_column_information - # Project factory will also call the current_version - expect(ActiveRecord::Migrator).to receive(:current_version).at_least(:once).and_return(InternalId::REQUIRED_SCHEMA_VERSION - 1) - end - - let(:init) { double('block') } - - it 'calculates next internal ids on the fly' do - val = rand(1..100) - - expect(init).to receive(:call).with(issue).and_return(val) - expect(subject).to eq(val + 1) - end - - it 'always attempts to generate internal IDs in production mode' do - stub_rails_env('production') - - val = rand(1..100) - generator = double(generate: val) - expect(InternalId::InternalIdGenerator).to receive(:new).and_return(generator) - - expect(subject).to eq(val) - end - end end describe '.reset' do @@ -152,20 +125,6 @@ describe InternalId do described_class.generate_next(issue, scope, usage, init) end end - - context 'with an insufficient schema version' do - let(:value) { 2 } - - before do - described_class.reset_column_information - # Project factory will also call the current_version - expect(ActiveRecord::Migrator).to receive(:current_version).at_least(:once).and_return(InternalId::REQUIRED_SCHEMA_VERSION - 1) - end - - it 'does not reset any of the iids' do - expect(subject).to be_falsey - end - end end describe '.track_greatest' do diff --git a/spec/models/issue/metrics_spec.rb b/spec/models/issue/metrics_spec.rb index 0d0628277a6..dc22d26e2f9 100644 --- a/spec/models/issue/metrics_spec.rb +++ b/spec/models/issue/metrics_spec.rb @@ -23,7 +23,7 @@ describe Issue::Metrics do describe '.with_first_mention_not_earlier_than' do subject(:scope) { described_class.with_first_mention_not_earlier_than(timestamp) } - let(:timestamp) { DateTime.now } + let(:timestamp) { DateTime.current } it 'returns metrics without mentioning in commit or with mentioning after given timestamp' do issue1 = create(:issue) @@ -37,7 +37,7 @@ describe Issue::Metrics do describe "when recording the default set of issue metrics on issue save" do context "milestones" do it "records the first time an issue is associated with a milestone" do - time = Time.now + time = Time.current Timecop.freeze(time) { subject.update(milestone: create(:milestone, project: project)) } metrics = subject.metrics @@ -46,7 +46,7 @@ describe Issue::Metrics do end it "does not record the second time an issue is associated with a milestone" do - time = Time.now + time = Time.current Timecop.freeze(time) { subject.update(milestone: create(:milestone, project: project)) } Timecop.freeze(time + 2.hours) { subject.update(milestone: nil) } Timecop.freeze(time + 6.hours) { subject.update(milestone: create(:milestone, project: project)) } @@ -60,7 +60,7 @@ describe Issue::Metrics do context "list labels" do it "records the first time an issue is associated with a list label" do list_label = create(:list).label - time = Time.now + time = Time.current Timecop.freeze(time) { subject.update(label_ids: [list_label.id]) } metrics = subject.metrics @@ -69,7 +69,7 @@ describe Issue::Metrics do end it "does not record the second time an issue is associated with a list label" do - time = Time.now + time = Time.current first_list_label = create(:list).label Timecop.freeze(time) { subject.update(label_ids: [first_list_label.id]) } second_list_label = create(:list).label diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index dd5ff3dbdde..291cccd72db 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -21,6 +21,9 @@ describe Issue do it { is_expected.to have_one(:alert_management_alert) } it { is_expected.to have_many(:resource_milestone_events) } it { is_expected.to have_many(:resource_state_events) } + it { is_expected.to have_and_belong_to_many(:prometheus_alert_events) } + it { is_expected.to have_and_belong_to_many(:self_managed_prometheus_alert_events) } + it { is_expected.to have_many(:prometheus_alerts) } describe 'versions.most_recent' do it 'returns the most recent version' do @@ -176,7 +179,7 @@ describe Issue do describe '#close' do subject(:issue) { create(:issue, state: 'opened') } - it 'sets closed_at to Time.now when an issue is closed' do + it 'sets closed_at to Time.current when an issue is closed' do expect { issue.close }.to change { issue.closed_at }.from(nil) end @@ -186,11 +189,40 @@ describe Issue do expect { issue.close }.to change { issue.state_id }.from(open_state).to(closed_state) end + + context 'when there is an associated Alert Management Alert' do + context 'when alert can be resolved' do + let!(:alert) { create(:alert_management_alert, project: issue.project, issue: issue) } + + it 'resolves an alert' do + expect { issue.close }.to change { alert.reload.resolved? }.to(true) + end + end + + context 'when alert cannot be resolved' do + let!(:alert) { create(:alert_management_alert, :with_validation_errors, project: issue.project, issue: issue) } + + before do + allow(Gitlab::AppLogger).to receive(:warn).and_call_original + end + + it 'writes a warning into the log' do + issue.close + + expect(Gitlab::AppLogger).to have_received(:warn).with( + message: 'Cannot resolve an associated Alert Management alert', + issue_id: issue.id, + alert_id: alert.id, + alert_errors: { hosts: ['hosts array is over 255 chars'] } + ) + end + end + end end describe '#reopen' do let(:user) { create(:user) } - let(:issue) { create(:issue, state: 'closed', closed_at: Time.now, closed_by: user) } + let(:issue) { create(:issue, state: 'closed', closed_at: Time.current, closed_by: user) } it 'sets closed_at to nil when an issue is reopend' do expect { issue.reopen }.to change { issue.closed_at }.to(nil) @@ -994,7 +1026,7 @@ describe Issue do it_behaves_like 'versioned description' describe "#previous_updated_at" do - let_it_be(:updated_at) { Time.new(2012, 01, 06) } + let_it_be(:updated_at) { Time.zone.local(2012, 01, 06) } let_it_be(:issue) { create(:issue, updated_at: updated_at) } it 'returns updated_at value if updated_at did not change at all' do @@ -1010,15 +1042,15 @@ describe Issue do end it 'returns updated_at value if previous updated_at value is not present' do - allow(issue).to receive(:previous_changes).and_return({ 'updated_at' => [nil, Time.new(2013, 02, 06)] }) + allow(issue).to receive(:previous_changes).and_return({ 'updated_at' => [nil, Time.zone.local(2013, 02, 06)] }) expect(issue.previous_updated_at).to eq(updated_at) end it 'returns previous updated_at when present' do - allow(issue).to receive(:previous_changes).and_return({ 'updated_at' => [Time.new(2013, 02, 06), Time.new(2013, 03, 06)] }) + allow(issue).to receive(:previous_changes).and_return({ 'updated_at' => [Time.zone.local(2013, 02, 06), Time.zone.local(2013, 03, 06)] }) - expect(issue.previous_updated_at).to eq(Time.new(2013, 02, 06)) + expect(issue.previous_updated_at).to eq(Time.zone.local(2013, 02, 06)) end end @@ -1085,4 +1117,15 @@ describe Issue do expect(subject).not_to include(labeled_issue) end end + + describe 'banzai_render_context' do + let(:project) { build(:project_empty_repo) } + let(:issue) { build :issue, project: project } + + subject(:context) { issue.banzai_render_context(:title) } + + it 'sets the label_url_method in the context' do + expect(context[:label_url_method]).to eq(:project_issues_url) + end + end end diff --git a/spec/models/iteration_spec.rb b/spec/models/iteration_spec.rb index e5b7b746639..ae14adf9106 100644 --- a/spec/models/iteration_spec.rb +++ b/spec/models/iteration_spec.rb @@ -6,10 +6,6 @@ describe Iteration do let_it_be(:project) { create(:project) } let_it_be(:group) { create(:group) } - it_behaves_like 'a timebox', :iteration do - let(:timebox_table_name) { described_class.table_name.to_sym } - end - describe "#iid" do it "is properly scoped on project and group" do iteration1 = create(:iteration, project: project) @@ -62,7 +58,7 @@ describe Iteration do end context 'when end_date is in range' do - let(:start_date) { Time.now } + let(:start_date) { Time.current } let(:due_date) { 6.days.from_now } it 'is not valid' do @@ -94,7 +90,7 @@ describe Iteration do describe '#future_date' do context 'when dates are in the future' do - let(:start_date) { Time.now } + let(:start_date) { Time.current } let(:due_date) { 1.week.from_now } it { is_expected.to be_valid } @@ -111,7 +107,7 @@ describe Iteration do end context 'when due_date is in the past' do - let(:start_date) { Time.now } + let(:start_date) { Time.current } let(:due_date) { 1.week.ago } it 'is not valid' do @@ -122,7 +118,7 @@ describe Iteration do context 'when start_date is over 500 years in the future' do let(:start_date) { 501.years.from_now } - let(:due_date) { Time.now } + let(:due_date) { Time.current } it 'is not valid' do expect(subject).not_to be_valid @@ -131,7 +127,7 @@ describe Iteration do end context 'when due_date is over 500 years in the future' do - let(:start_date) { Time.now } + let(:start_date) { Time.current } let(:due_date) { 501.years.from_now } it 'is not valid' do @@ -143,7 +139,7 @@ describe Iteration do end describe '.within_timeframe' do - let_it_be(:now) { Time.now } + let_it_be(:now) { Time.current } let_it_be(:project) { create(:project, :empty_repo) } let_it_be(:iteration_1) { create(:iteration, project: project, start_date: now, due_date: 1.day.from_now) } let_it_be(:iteration_2) { create(:iteration, project: project, start_date: 2.days.from_now, due_date: 3.days.from_now) } diff --git a/spec/models/jira_import_state_spec.rb b/spec/models/jira_import_state_spec.rb index 99f9e035205..d2535636c63 100644 --- a/spec/models/jira_import_state_spec.rb +++ b/spec/models/jira_import_state_spec.rb @@ -124,7 +124,7 @@ describe JiraImportState do jira_import.schedule expect(jira_import.jid).to eq('some-job-id') - expect(jira_import.scheduled_at).to be_within(1.second).of(Time.now) + expect(jira_import.scheduled_at).to be_within(1.second).of(Time.current) end end @@ -163,4 +163,39 @@ describe JiraImportState do end end end + + context 'ensure error_message size on save' do + let_it_be(:project) { create(:project) } + + before do + stub_const('JiraImportState::ERROR_MESSAGE_SIZE', 10) + end + + context 'when jira import has no error_message' 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.reload.error_message).to be_nil + end + end + + context 'when jira import error_message does not exceed the limit' 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.reload.error_message).to eq('error') + end + end + + context 'when error_message exceeds limit' do + let(:jira_import) { build(:jira_import_state, project: project, error_message: 'error message longer than the limit')} + + it 'truncates error_message to the limit', :aggregate_failures do + expect { jira_import.save! }.to change { JiraImportState.count }.by(1) + expect(jira_import.reload.error_message.size).to eq 10 + end + end + end end diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb index a8d864ad3f0..7c40bb24b56 100644 --- a/spec/models/member_spec.rb +++ b/spec/models/member_spec.rb @@ -494,7 +494,7 @@ describe Member do end describe '#accept_request' do - let(:member) { create(:project_member, requested_at: Time.now.utc) } + let(:member) { create(:project_member, requested_at: Time.current.utc) } it { expect(member.accept_request).to be_truthy } @@ -518,14 +518,14 @@ describe Member do end describe '#request?' do - subject { create(:project_member, requested_at: Time.now.utc) } + subject { create(:project_member, requested_at: Time.current.utc) } it { is_expected.to be_request } end describe '#pending?' do let(:invited_member) { create(:project_member, invite_email: "user@example.com", user: nil) } - let(:requester) { create(:project_member, requested_at: Time.now.utc) } + let(:requester) { create(:project_member, requested_at: Time.current.utc) } it { expect(invited_member).to be_invite } it { expect(requester).to be_pending } diff --git a/spec/models/members/group_member_spec.rb b/spec/models/members/group_member_spec.rb index 9b5cce1aebf..fdb71b7ec7d 100644 --- a/spec/models/members/group_member_spec.rb +++ b/spec/models/members/group_member_spec.rb @@ -4,7 +4,7 @@ require 'spec_helper' describe GroupMember do context 'scopes' do - describe '.count_users_by_group_id' do + shared_examples '.count_users_by_group_id' do it 'counts users by group ID' do user_1 = create(:user) user_2 = create(:user) @@ -20,6 +20,36 @@ describe GroupMember do end end + describe '.count_users_by_group_id with optimized_count_users_by_group_id feature flag on' do + before do + stub_feature_flags(optimized_count_users_by_group_id: true) + end + + it_behaves_like '.count_users_by_group_id' + + it 'does not JOIN users' do + scope = described_class.all + expect(scope).not_to receive(:joins).with(:user) + + scope.count_users_by_group_id + end + end + + describe '.count_users_by_group_id with optimized_count_users_by_group_id feature flag off' do + before do + stub_feature_flags(optimized_count_users_by_group_id: false) + end + + it_behaves_like '.count_users_by_group_id' + + it 'does JOIN users' do + scope = described_class.all + expect(scope).to receive(:joins).with(:user).and_call_original + + scope.count_users_by_group_id + end + end + describe '.of_ldap_type' do it 'returns ldap type users' do group_member = create(:group_member, :ldap) diff --git a/spec/models/members/project_member_spec.rb b/spec/models/members/project_member_spec.rb index 79c39b81196..fdb9457b211 100644 --- a/spec/models/members/project_member_spec.rb +++ b/spec/models/members/project_member_spec.rb @@ -44,14 +44,14 @@ describe ProjectMember do let(:maintainer) { create(:project_member, project: project) } it "creates an expired event when left due to expiry" do - expired = create(:project_member, project: project, expires_at: Time.now - 6.days) + expired = create(:project_member, project: project, expires_at: Time.current - 6.days) expired.destroy - expect(Event.recent.first.action).to eq(Event::EXPIRED) + expect(Event.recent.first).to be_expired_action end it "creates a left event when left due to leave" do maintainer.destroy - expect(Event.recent.first.action).to eq(Event::LEFT) + expect(Event.recent.first).to be_left_action end end diff --git a/spec/models/merge_request_diff_commit_spec.rb b/spec/models/merge_request_diff_commit_spec.rb index 8b51c6fae08..62430b08c5c 100644 --- a/spec/models/merge_request_diff_commit_spec.rb +++ b/spec/models/merge_request_diff_commit_spec.rb @@ -73,7 +73,7 @@ describe MergeRequestDiffCommit do # This commit's date is "Sun Aug 17 07:12:55 292278994 +0000" [project.commit('ba3343bc4fa403a8dfbfcab7fc1a8c29ee34bd69')] end - let(:timestamp) { Time.at((1 << 31) - 1) } + let(:timestamp) { Time.zone.at((1 << 31) - 1) } let(:rows) do [{ "message": "Weird commit date\n", diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index fc4590f7b22..c70ddac5da6 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -22,6 +22,8 @@ describe MergeRequest do it { is_expected.to belong_to(:iteration) } it { is_expected.to have_many(:resource_milestone_events) } it { is_expected.to have_many(:resource_state_events) } + it { is_expected.to have_many(:draft_notes) } + it { is_expected.to have_many(:reviews).inverse_of(:merge_request) } context 'for forks' do let!(:project) { create(:project) } @@ -721,11 +723,15 @@ describe MergeRequest do end describe '#note_positions_for_paths' do + let(:user) { create(:user) } let(:merge_request) { create(:merge_request, :with_diffs) } let(:project) { merge_request.project } let!(:diff_note) do create(:diff_note_on_merge_request, project: project, noteable: merge_request) end + let!(:draft_note) do + create(:draft_note_on_text_diff, author: user, merge_request: merge_request) + end let(:file_paths) { merge_request.diffs.diff_files.map(&:file_path) } @@ -758,6 +764,26 @@ describe MergeRequest do expect(subject.to_a).to be_empty end end + + context 'when user is given' do + subject do + merge_request.note_positions_for_paths(file_paths, user) + end + + it 'returns notes and draft notes positions' do + expect(subject).to match_array([draft_note.position, diff_note.position]) + end + end + + context 'when user is not given' do + subject do + merge_request.note_positions_for_paths(file_paths) + end + + it 'returns notes positions' do + expect(subject).to match_array([diff_note.position]) + end + end end describe '#discussions_diffs' do @@ -1625,14 +1651,6 @@ describe MergeRequest do let(:merge_request) { create(:merge_request, :with_accessibility_reports, source_project: project) } it { is_expected.to be_truthy } - - context 'when feature flag is disabled' do - before do - stub_feature_flags(accessibility_report_view: false) - end - - it { is_expected.to be_falsey } - end end context 'when head pipeline does not have accessibility reports' do @@ -2018,7 +2036,7 @@ describe MergeRequest do describe '#can_be_reverted?' do context 'when there is no merge_commit for the MR' do before do - subject.metrics.update!(merged_at: Time.now.utc) + subject.metrics.update!(merged_at: Time.current.utc) end it 'returns false' do @@ -2157,7 +2175,34 @@ describe MergeRequest do end end - context 'when merging note is persisted, but no metrics or merge event exists' do + context 'when state event tracking is disabled' do + before do + stub_feature_flags(track_resource_state_change_events: false) + end + + context 'when merging note is persisted, but no metrics or merge event exists' do + let(:user) { create(:user) } + let(:merge_request) { create(:merge_request, :merged) } + + before do + merge_request.metrics.destroy! + + SystemNoteService.change_status(merge_request, + merge_request.target_project, + user, + merge_request.state, nil) + end + + it 'returns merging note creation date' do + expect(merge_request.reload.metrics).to be_nil + expect(merge_request.merge_event).to be_nil + expect(merge_request.notes.count).to eq(1) + expect(merge_request.merged_at).to eq(merge_request.notes.first.created_at) + end + end + end + + context 'when state event tracking is enabled' do let(:user) { create(:user) } let(:merge_request) { create(:merge_request, :merged) } @@ -2170,11 +2215,8 @@ describe MergeRequest do merge_request.state, nil) end - it 'returns merging note creation date' do - expect(merge_request.reload.metrics).to be_nil - expect(merge_request.merge_event).to be_nil - expect(merge_request.notes.count).to eq(1) - expect(merge_request.merged_at).to eq(merge_request.notes.first.created_at) + it 'does not create a system note' do + expect(merge_request.notes).to be_empty end end end @@ -2327,24 +2369,10 @@ describe MergeRequest do end context 'when async is true' do - context 'and async_merge_request_check_mergeability feature flag is enabled' do - it 'executes MergeabilityCheckService asynchronously' do - expect(mergeability_service).to receive(:async_execute) - - subject.check_mergeability(async: true) - end - end - - context 'and async_merge_request_check_mergeability feature flag is disabled' do - before do - stub_feature_flags(async_merge_request_check_mergeability: false) - end - - it 'executes MergeabilityCheckService' do - expect(mergeability_service).to receive(:execute) + it 'executes MergeabilityCheckService asynchronously' do + expect(mergeability_service).to receive(:async_execute) - subject.check_mergeability(async: true) - end + subject.check_mergeability(async: true) end end end @@ -2489,12 +2517,13 @@ describe MergeRequest do end describe '#mergeable_ci_state?' do - let(:project) { create(:project, only_allow_merge_if_pipeline_succeeds: true) } let(:pipeline) { create(:ci_empty_pipeline) } - subject { build(:merge_request, target_project: project) } - context 'when it is only allowed to merge when build is green' do + let(:project) { create(:project, only_allow_merge_if_pipeline_succeeds: true) } + + subject { build(:merge_request, target_project: project) } + context 'and a failed pipeline is associated' do before do pipeline.update(status: 'failed', sha: subject.diff_head_sha) @@ -2516,7 +2545,7 @@ describe MergeRequest do context 'and a skipped pipeline is associated' do before do pipeline.update(status: 'skipped', sha: subject.diff_head_sha) - allow(subject).to receive(:head_pipeline) { pipeline } + allow(subject).to receive(:head_pipeline).and_return(pipeline) end it { expect(subject.mergeable_ci_state?).to be_falsey } @@ -2524,7 +2553,48 @@ describe MergeRequest do context 'when no pipeline is associated' do before do - allow(subject).to receive(:head_pipeline) { nil } + allow(subject).to receive(:head_pipeline).and_return(nil) + end + + it { expect(subject.mergeable_ci_state?).to be_falsey } + end + end + + context 'when it is only allowed to merge when build is green or skipped' do + let(:project) { create(:project, only_allow_merge_if_pipeline_succeeds: true, allow_merge_on_skipped_pipeline: true) } + + subject { build(:merge_request, target_project: project) } + + context 'and a failed pipeline is associated' do + before do + pipeline.update!(status: 'failed', sha: subject.diff_head_sha) + allow(subject).to receive(:head_pipeline).and_return(pipeline) + end + + it { expect(subject.mergeable_ci_state?).to be_falsey } + end + + context 'and a successful pipeline is associated' do + before do + pipeline.update!(status: 'success', sha: subject.diff_head_sha) + allow(subject).to receive(:head_pipeline).and_return(pipeline) + end + + it { expect(subject.mergeable_ci_state?).to be_truthy } + end + + context 'and a skipped pipeline is associated' do + before do + pipeline.update!(status: 'skipped', sha: subject.diff_head_sha) + allow(subject).to receive(:head_pipeline).and_return(pipeline) + end + + it { expect(subject.mergeable_ci_state?).to be_truthy } + end + + context 'when no pipeline is associated' do + before do + allow(subject).to receive(:head_pipeline).and_return(nil) end it { expect(subject.mergeable_ci_state?).to be_falsey } @@ -2532,7 +2602,9 @@ describe MergeRequest do end context 'when merges are not restricted to green builds' do - subject { build(:merge_request, target_project: create(:project, only_allow_merge_if_pipeline_succeeds: false)) } + let(:project) { create(:project, only_allow_merge_if_pipeline_succeeds: false) } + + subject { build(:merge_request, target_project: project) } context 'and a failed pipeline is associated' do before do @@ -2550,6 +2622,23 @@ describe MergeRequest do it { expect(subject.mergeable_ci_state?).to be_truthy } end + + context 'and a skipped pipeline is associated' do + before do + pipeline.update!(status: 'skipped', sha: subject.diff_head_sha) + allow(subject).to receive(:head_pipeline).and_return(pipeline) + end + + it { expect(subject.mergeable_ci_state?).to be_truthy } + end + + context 'when no pipeline is associated' do + before do + allow(subject).to receive(:head_pipeline).and_return(nil) + end + + it { expect(subject.mergeable_ci_state?).to be_truthy } + end end end @@ -2581,7 +2670,7 @@ describe MergeRequest do context 'with no discussions' do before do - merge_request.notes.destroy_all # rubocop: disable DestroyAll + merge_request.notes.destroy_all # rubocop: disable Cop/DestroyAll end it 'returns true' do @@ -3876,4 +3965,15 @@ describe MergeRequest do expect(count).to eq(0) end end + + describe 'banzai_render_context' do + let(:project) { build(:project_empty_repo) } + let(:merge_request) { build :merge_request, target_project: project, source_project: project } + + subject(:context) { merge_request.banzai_render_context(:title) } + + it 'sets the label_url_method in the context' do + expect(context[:label_url_method]).to eq(:project_merge_requests_url) + end + end end diff --git a/spec/models/metrics/dashboard/annotation_spec.rb b/spec/models/metrics/dashboard/annotation_spec.rb index f7fd7ded7e6..3cba31ffdfe 100644 --- a/spec/models/metrics/dashboard/annotation_spec.rb +++ b/spec/models/metrics/dashboard/annotation_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' describe Metrics::Dashboard::Annotation do + using RSpec::Parameterized::TableSyntax + describe 'associations' do it { is_expected.to belong_to(:environment).inverse_of(:metrics_dashboard_annotations) } it { is_expected.to belong_to(:cluster).class_name('Clusters::Cluster').inverse_of(:metrics_dashboard_annotations) } @@ -28,6 +30,26 @@ describe Metrics::Dashboard::Annotation do end end + context 'ending_at_after_starting_at' do + where(:starting_at, :ending_at, :valid?, :message) do + 2.days.ago.beginning_of_day | 1.day.ago.beginning_of_day | true | nil + 1.day.ago.beginning_of_day | nil | true | nil + 1.day.ago.beginning_of_day | 1.day.ago.beginning_of_day | true | nil + 1.day.ago.beginning_of_day | 2.days.ago.beginning_of_day | false | /Ending at can't be before starting_at time/ + nil | 2.days.ago.beginning_of_day | false | /Starting at can't be blank/ # validation is covered by other method, be we need to assure, that ending_at_after_starting_at will not break with nil as starting_at + nil | nil | false | /Starting at can't be blank/ # validation is covered by other method, be we need to assure, that ending_at_after_starting_at will not break with nil as starting_at + end + + with_them do + subject(:annotation) { build(:metrics_dashboard_annotation, starting_at: starting_at, ending_at: ending_at) } + + it do + expect(annotation.valid?).to be(valid?) + expect(annotation.errors.full_messages).to include(message) if message + end + end + end + context 'environments annotation' do subject { build(:metrics_dashboard_annotation) } @@ -75,5 +97,16 @@ describe Metrics::Dashboard::Annotation do expect(described_class.for_dashboard('other_dashboard.yml')).to match_array [other_dashboard_annotation] end end + + describe '#ending_before' do + it 'returns annotations only for appointed dashboard' do + Timecop.freeze do + twelve_minutes_old_annotation = create(:metrics_dashboard_annotation, starting_at: 15.minutes.ago, ending_at: 12.minutes.ago) + create(:metrics_dashboard_annotation, starting_at: 15.minutes.ago, ending_at: 11.minutes.ago) + + expect(described_class.ending_before(11.minutes.ago)).to match_array [fifteen_minutes_old_annotation, twelve_minutes_old_annotation] + end + end + end end end diff --git a/spec/models/milestone_spec.rb b/spec/models/milestone_spec.rb index e51108947a7..33f84da27f6 100644 --- a/spec/models/milestone_spec.rb +++ b/spec/models/milestone_spec.rb @@ -81,7 +81,7 @@ describe Milestone do end it_behaves_like 'within_timeframe scope' do - let_it_be(:now) { Time.now } + let_it_be(:now) { Time.current } let_it_be(:project) { create(:project, :empty_repo) } let_it_be(:resource_1) { create(:milestone, project: project, start_date: now - 1.day, due_date: now + 1.day) } let_it_be(:resource_2) { create(:milestone, project: project, start_date: now + 2.days, due_date: now + 3.days) } @@ -130,7 +130,7 @@ describe Milestone do describe '#upcoming?' do it 'returns true when start_date is in the future' do - milestone = build(:milestone, start_date: Time.now + 1.month) + milestone = build(:milestone, start_date: Time.current + 1.month) expect(milestone.upcoming?).to be_truthy end @@ -225,70 +225,88 @@ describe Milestone do end end - describe '#for_projects_and_groups' do - let(:project) { create(:project) } - let(:project_other) { create(:project) } - let(:group) { create(:group) } - let(:group_other) { create(:group) } + shared_examples '#for_projects_and_groups' do + describe '#for_projects_and_groups' do + let_it_be(:project) { create(:project) } + let_it_be(:project_other) { create(:project) } + let_it_be(:group) { create(:group) } + let_it_be(:group_other) { create(:group) } + + before(:all) do + create(:milestone, project: project) + create(:milestone, project: project_other) + create(:milestone, group: group) + create(:milestone, group: group_other) + end - before do - create(:milestone, project: project) - create(:milestone, project: project_other) - create(:milestone, group: group) - create(:milestone, group: group_other) - end + subject { described_class.for_projects_and_groups(projects, groups) } + + shared_examples 'filters by projects and groups' do + it 'returns milestones filtered by project' do + milestones = described_class.for_projects_and_groups(projects, []) - subject { described_class.for_projects_and_groups(projects, groups) } + expect(milestones.count).to eq(1) + expect(milestones.first.project_id).to eq(project.id) + end + + it 'returns milestones filtered by group' do + milestones = described_class.for_projects_and_groups([], groups) + + expect(milestones.count).to eq(1) + expect(milestones.first.group_id).to eq(group.id) + end - shared_examples 'filters by projects and groups' do - it 'returns milestones filtered by project' do - milestones = described_class.for_projects_and_groups(projects, []) + it 'returns milestones filtered by both project and group' do + milestones = described_class.for_projects_and_groups(projects, groups) - expect(milestones.count).to eq(1) - expect(milestones.first.project_id).to eq(project.id) + expect(milestones.count).to eq(2) + expect(milestones).to contain_exactly(project.milestones.first, group.milestones.first) + end end - it 'returns milestones filtered by group' do - milestones = described_class.for_projects_and_groups([], groups) + context 'ids as params' do + let(:projects) { [project.id] } + let(:groups) { [group.id] } - expect(milestones.count).to eq(1) - expect(milestones.first.group_id).to eq(group.id) + it_behaves_like 'filters by projects and groups' end - it 'returns milestones filtered by both project and group' do - milestones = described_class.for_projects_and_groups(projects, groups) + context 'relations as params' do + let(:projects) { Project.where(id: project.id).select(:id) } + let(:groups) { Group.where(id: group.id).select(:id) } - expect(milestones.count).to eq(2) - expect(milestones).to contain_exactly(project.milestones.first, group.milestones.first) + it_behaves_like 'filters by projects and groups' end - end - context 'ids as params' do - let(:projects) { [project.id] } - let(:groups) { [group.id] } + context 'objects as params' do + let(:projects) { [project] } + let(:groups) { [group] } - it_behaves_like 'filters by projects and groups' - end + it_behaves_like 'filters by projects and groups' + end - context 'relations as params' do - let(:projects) { Project.where(id: project.id).select(:id) } - let(:groups) { Group.where(id: group.id).select(:id) } + it 'returns no records if projects and groups are nil' do + milestones = described_class.for_projects_and_groups(nil, nil) - it_behaves_like 'filters by projects and groups' + expect(milestones).to be_empty + end end + end - context 'objects as params' do - let(:projects) { [project] } - let(:groups) { [group] } - - it_behaves_like 'filters by projects and groups' + context 'when `optimized_timebox_queries` feature flag is enabled' do + before do + stub_feature_flags(optimized_timebox_queries: true) end - it 'returns no records if projects and groups are nil' do - milestones = described_class.for_projects_and_groups(nil, nil) + it_behaves_like '#for_projects_and_groups' + end - expect(milestones).to be_empty + context 'when `optimized_timebox_queries` feature flag is disabled' do + before do + stub_feature_flags(optimized_timebox_queries: false) end + + it_behaves_like '#for_projects_and_groups' end describe '.upcoming_ids' do @@ -297,30 +315,30 @@ describe Milestone do let(:group_3) { create(:group) } let(:groups) { [group_1, group_2, group_3] } - let!(:past_milestone_group_1) { create(:milestone, group: group_1, due_date: Time.now - 1.day) } - let!(:current_milestone_group_1) { create(:milestone, group: group_1, due_date: Time.now + 1.day) } - let!(:future_milestone_group_1) { create(:milestone, group: group_1, due_date: Time.now + 2.days) } + let!(:past_milestone_group_1) { create(:milestone, group: group_1, due_date: Time.current - 1.day) } + let!(:current_milestone_group_1) { create(:milestone, group: group_1, due_date: Time.current + 1.day) } + let!(:future_milestone_group_1) { create(:milestone, group: group_1, due_date: Time.current + 2.days) } - let!(:past_milestone_group_2) { create(:milestone, group: group_2, due_date: Time.now - 1.day) } - let!(:closed_milestone_group_2) { create(:milestone, :closed, group: group_2, due_date: Time.now + 1.day) } - let!(:current_milestone_group_2) { create(:milestone, group: group_2, due_date: Time.now + 2.days) } + let!(:past_milestone_group_2) { create(:milestone, group: group_2, due_date: Time.current - 1.day) } + let!(:closed_milestone_group_2) { create(:milestone, :closed, group: group_2, due_date: Time.current + 1.day) } + let!(:current_milestone_group_2) { create(:milestone, group: group_2, due_date: Time.current + 2.days) } - let!(:past_milestone_group_3) { create(:milestone, group: group_3, due_date: Time.now - 1.day) } + let!(:past_milestone_group_3) { create(:milestone, group: group_3, due_date: Time.current - 1.day) } let(:project_1) { create(:project) } let(:project_2) { create(:project) } let(:project_3) { create(:project) } let(:projects) { [project_1, project_2, project_3] } - let!(:past_milestone_project_1) { create(:milestone, project: project_1, due_date: Time.now - 1.day) } - let!(:current_milestone_project_1) { create(:milestone, project: project_1, due_date: Time.now + 1.day) } - let!(:future_milestone_project_1) { create(:milestone, project: project_1, due_date: Time.now + 2.days) } + let!(:past_milestone_project_1) { create(:milestone, project: project_1, due_date: Time.current - 1.day) } + let!(:current_milestone_project_1) { create(:milestone, project: project_1, due_date: Time.current + 1.day) } + let!(:future_milestone_project_1) { create(:milestone, project: project_1, due_date: Time.current + 2.days) } - let!(:past_milestone_project_2) { create(:milestone, project: project_2, due_date: Time.now - 1.day) } - let!(:closed_milestone_project_2) { create(:milestone, :closed, project: project_2, due_date: Time.now + 1.day) } - let!(:current_milestone_project_2) { create(:milestone, project: project_2, due_date: Time.now + 2.days) } + let!(:past_milestone_project_2) { create(:milestone, project: project_2, due_date: Time.current - 1.day) } + let!(:closed_milestone_project_2) { create(:milestone, :closed, project: project_2, due_date: Time.current + 1.day) } + let!(:current_milestone_project_2) { create(:milestone, project: project_2, due_date: Time.current + 2.days) } - let!(:past_milestone_project_3) { create(:milestone, project: project_3, due_date: Time.now - 1.day) } + let!(:past_milestone_project_3) { create(:milestone, project: project_3, due_date: Time.current - 1.day) } let(:milestone_ids) { described_class.upcoming_ids(projects, groups).map(&:id) } @@ -498,4 +516,23 @@ describe Milestone do end end end + + describe '#subgroup_milestone' do + context 'parent is subgroup' do + it 'returns true' do + group = create(:group) + subgroup = create(:group, :private, parent: group) + + expect(build(:milestone, group: subgroup).subgroup_milestone?).to eq(true) + end + end + + context 'parent is not subgroup' do + it 'returns false' do + group = create(:group) + + expect(build(:milestone, group: group).subgroup_milestone?).to eq(false) + end + end + end end diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index 6dd295ca915..af3fdcfaa2e 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -11,6 +11,7 @@ describe Note do it { is_expected.to belong_to(:author).class_name('User') } it { is_expected.to have_many(:todos) } + it { is_expected.to belong_to(:review).inverse_of(:notes) } end describe 'modules' do @@ -285,7 +286,7 @@ describe Note do end describe "edited?" do - let(:note) { build(:note, updated_by_id: nil, created_at: Time.now, updated_at: Time.now + 5.hours) } + let(:note) { build(:note, updated_by_id: nil, created_at: Time.current, updated_at: Time.current + 5.hours) } context "with updated_by" do it "returns true" do @@ -304,11 +305,22 @@ describe Note do describe '#confidential?' do context 'when note is not confidential' do - it 'is true when a noteable is confidential' do - issue = create(:issue, :confidential) - note = build(:note, noteable: issue, project: issue.project) + context 'when include_noteable is set to true' do + it 'is true when a noteable is confidential ' do + issue = create(:issue, :confidential) + note = build(:note, noteable: issue, project: issue.project) - expect(note.confidential?).to be_truthy + expect(note.confidential?(include_noteable: true)).to be_truthy + end + end + + context 'when include_noteable is not set to true' do + it 'is false when a noteable is confidential ' do + issue = create(:issue, :confidential) + note = build(:note, noteable: issue, project: issue.project) + + expect(note.confidential?).to be_falsey + end end it 'is false when a noteable is not confidential' do @@ -318,7 +330,7 @@ describe Note do expect(note.confidential?).to be_falsy end - it "is falsey when noteable can't be confidential" do + it "is false when noteable can't be confidential" do commit_note = build(:note_on_commit) expect(commit_note.confidential?).to be_falsy @@ -817,6 +829,10 @@ describe Note do it 'returns commit for a commit note' do expect(build(:note_on_commit).noteable_ability_name).to eq('commit') end + + it 'returns alert_management_alert for an alert note' do + expect(build(:note_on_alert).noteable_ability_name).to eq('alert_management_alert') + end end describe '#cache_markdown_field' do @@ -1352,4 +1368,28 @@ describe Note do end end end + + describe 'banzai_render_context' do + let(:project) { build(:project_empty_repo) } + + context 'when noteable is a merge request' do + let(:noteable) { build :merge_request, target_project: project, source_project: project } + + subject(:context) { noteable.banzai_render_context(:title) } + + it 'sets the label_url_method in the context' do + expect(context[:label_url_method]).to eq(:project_merge_requests_url) + end + end + + context 'when noteable is an issue' do + let(:noteable) { build :issue, project: project } + + subject(:context) { noteable.banzai_render_context(:title) } + + it 'sets the label_url_method in the context' do + expect(context[:label_url_method]).to eq(:project_issues_url) + end + end + end end diff --git a/spec/models/pages_domain_spec.rb b/spec/models/pages_domain_spec.rb index 54747ddf525..fc7694530d0 100644 --- a/spec/models/pages_domain_spec.rb +++ b/spec/models/pages_domain_spec.rb @@ -97,8 +97,8 @@ describe PagesDomain do it 'saves validity time' do domain.save - expect(domain.certificate_valid_not_before).to be_like_time(Time.parse("2020-03-16 14:20:34 UTC")) - expect(domain.certificate_valid_not_after).to be_like_time(Time.parse("2220-01-28 14:20:34 UTC")) + 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")) end end @@ -366,7 +366,7 @@ describe PagesDomain do let_it_be(:domain) { create(:pages_domain) } where(:attribute, :old_value, :new_value, :update_expected) do - now = Time.now + now = Time.current future = now + 1.day :project | nil | :project1 | true @@ -548,7 +548,7 @@ describe PagesDomain do it 'does not clear failure on unrelated updates' do expect do - domain.update!(verified_at: Time.now) + domain.update!(verified_at: Time.current) end.not_to change { domain.auto_ssl_failed }.from(true) end end diff --git a/spec/models/performance_monitoring/prometheus_dashboard_spec.rb b/spec/models/performance_monitoring/prometheus_dashboard_spec.rb index e6fc03a0fb6..ef7298c3d8c 100644 --- a/spec/models/performance_monitoring/prometheus_dashboard_spec.rb +++ b/spec/models/performance_monitoring/prometheus_dashboard_spec.rb @@ -38,24 +38,141 @@ describe PerformanceMonitoring::PrometheusDashboard do end describe 'validations' do - context 'when dashboard is missing' do - before do - json_content['dashboard'] = nil + shared_examples 'validation failed' do |errors_messages| + it 'raises error with corresponding messages', :aggregate_failures do + expect { subject }.to raise_error do |error| + expect(error).to be_kind_of(ActiveModel::ValidationError) + expect(error.model.errors.messages).to eq(errors_messages) + end end + end + + context 'dashboard content is missing' do + let(:json_content) { nil } + + it_behaves_like 'validation failed', panel_groups: ["can't be blank"], dashboard: ["can't be blank"] + end + + context 'dashboard content is NOT a hash' do + let(:json_content) { YAML.safe_load("'test'") } + + it_behaves_like 'validation failed', panel_groups: ["can't be blank"], dashboard: ["can't be blank"] + end + + context 'content is an array' do + let(:json_content) { [{ "dashboard" => "Dashboard Title" }] } + + it_behaves_like 'validation failed', panel_groups: ["can't be blank"], dashboard: ["can't be blank"] + end - subject { described_class.from_json(json_content) } + context 'dashboard definition is missing panels_groups and dashboard keys' do + let(:json_content) do + { + "dashboard" => nil + } + end + + it_behaves_like 'validation failed', panel_groups: ["can't be blank"], dashboard: ["can't be blank"] + end + + context 'group definition is missing panels and group keys' do + let(:json_content) do + { + "dashboard" => "Dashboard Title", + "templating" => { + "variables" => { + "variable1" => %w(value1 value2 value3) + } + }, + "panel_groups" => [{ "group" => nil }] + } + end + + it_behaves_like 'validation failed', panels: ["can't be blank"], group: ["can't be blank"] + end + + context 'panel definition is missing metrics and title keys' do + let(:json_content) do + { + "dashboard" => "Dashboard Title", + "templating" => { + "variables" => { + "variable1" => %w(value1 value2 value3) + } + }, + "panel_groups" => [{ + "group" => "Group Title", + "panels" => [{ + "type" => "area-chart", + "y_label" => "Y-Axis" + }] + }] + } + end - it { expect { subject }.to raise_error(ActiveModel::ValidationError) } + it_behaves_like 'validation failed', metrics: ["can't be blank"], title: ["can't be blank"] end - context 'when panel groups are missing' do - before do - json_content['panel_groups'] = [] + context 'metrics definition is missing unit, query and query_range keys' do + let(:json_content) do + { + "dashboard" => "Dashboard Title", + "templating" => { + "variables" => { + "variable1" => %w(value1 value2 value3) + } + }, + "panel_groups" => [{ + "group" => "Group Title", + "panels" => [{ + "type" => "area-chart", + "title" => "Chart Title", + "y_label" => "Y-Axis", + "metrics" => [{ + "id" => "metric_of_ages", + "label" => "Metric of Ages", + "query_range" => nil + }] + }] + }] + } end - subject { described_class.from_json(json_content) } + it_behaves_like 'validation failed', unit: ["can't be blank"], query_range: ["can't be blank"], query: ["can't be blank"] + end + + # for each parent entry validation first is done to its children, + # whole execution is stopped on first encountered error + # which is the one that is reported + context 'multiple offences on different levels' do + let(:json_content) do + { + "dashboard" => nil, + "panel_groups" => [{ + "group" => nil, + "panels" => [{ + "type" => "area-chart", + "title" => nil, + "y_label" => "Y-Axis", + "metrics" => [{ + "id" => "metric_of_ages", + "label" => "Metric of Ages", + "query_range" => 'query' + }, { + "id" => "metric_of_ages", + "unit" => "count", + "label" => "Metric of Ages", + "query_range" => nil + }] + }] + }, { + "group" => 'group', + "panels" => nil + }] + } + end - it { expect { subject }.to raise_error(ActiveModel::ValidationError) } + it_behaves_like 'validation failed', unit: ["can't be blank"] end end end @@ -73,20 +190,51 @@ describe PerformanceMonitoring::PrometheusDashboard do dashboard_instance = described_class.find_for(project: project, user: user, path: path, options: { environment: environment }) expect(dashboard_instance).to be_instance_of described_class - expect(dashboard_instance.environment).to be environment - expect(dashboard_instance.path).to be path + expect(dashboard_instance.environment).to eq environment + expect(dashboard_instance.path).to eq path end end context 'dashboard has NOT been found' do it 'returns nil' do - allow(Gitlab::Metrics::Dashboard::Finder).to receive(:find).and_return(status: :error) + allow(Gitlab::Metrics::Dashboard::Finder).to receive(:find).and_return(http_status: :not_found) dashboard_instance = described_class.find_for(project: project, user: user, path: path, options: { environment: environment }) expect(dashboard_instance).to be_nil end end + + context 'dashboard has invalid schema', :aggregate_failures do + it 'still returns dashboard object' do + expect(Gitlab::Metrics::Dashboard::Finder).to receive(:find).and_return(http_status: :unprocessable_entity) + + dashboard_instance = described_class.find_for(project: project, user: user, path: path, options: { environment: environment }) + + expect(dashboard_instance).to be_instance_of described_class + expect(dashboard_instance.environment).to eq environment + expect(dashboard_instance.path).to eq path + end + end + end + + describe '#schema_validation_warnings' do + context 'when schema is valid' do + it 'returns nil' do + expect(described_class).to receive(:from_json) + expect(described_class.new.schema_validation_warnings).to be_nil + end + end + + context 'when schema is invalid' do + it 'returns array with errors messages' do + instance = described_class.new + instance.errors.add(:test, 'test error') + + expect(described_class).to receive(:from_json).and_raise(ActiveModel::ValidationError.new(instance)) + expect(described_class.new.schema_validation_warnings).to eq ['test: test error'] + end + end end describe '#to_yaml' do diff --git a/spec/models/performance_monitoring/prometheus_metric_spec.rb b/spec/models/performance_monitoring/prometheus_metric_spec.rb index 08288e5d993..83f687aa90e 100644 --- a/spec/models/performance_monitoring/prometheus_metric_spec.rb +++ b/spec/models/performance_monitoring/prometheus_metric_spec.rb @@ -24,6 +24,14 @@ describe PerformanceMonitoring::PrometheusMetric do end describe 'validations' do + context 'json_content is not a hash' do + let(:json_content) { nil } + + subject { described_class.from_json(json_content) } + + it { expect { subject }.to raise_error(ActiveModel::ValidationError) } + end + context 'when unit is missing' do before do json_content['unit'] = nil diff --git a/spec/models/performance_monitoring/prometheus_panel_group_spec.rb b/spec/models/performance_monitoring/prometheus_panel_group_spec.rb index 2447bb5df94..ecf7e13a9a3 100644 --- a/spec/models/performance_monitoring/prometheus_panel_group_spec.rb +++ b/spec/models/performance_monitoring/prometheus_panel_group_spec.rb @@ -30,9 +30,17 @@ describe PerformanceMonitoring::PrometheusPanelGroup do end describe 'validations' do + context 'json_content is not a hash' do + let(:json_content) { nil } + + subject { described_class.from_json(json_content) } + + it { expect { subject }.to raise_error(ActiveModel::ValidationError) } + end + context 'when group is missing' do before do - json_content['group'] = nil + json_content.delete('group') end subject { described_class.from_json(json_content) } diff --git a/spec/models/performance_monitoring/prometheus_panel_spec.rb b/spec/models/performance_monitoring/prometheus_panel_spec.rb index f5e04ec91e2..127b9e8183a 100644 --- a/spec/models/performance_monitoring/prometheus_panel_spec.rb +++ b/spec/models/performance_monitoring/prometheus_panel_spec.rb @@ -42,6 +42,14 @@ describe PerformanceMonitoring::PrometheusPanel do end describe 'validations' do + context 'json_content is not a hash' do + let(:json_content) { nil } + + subject { described_class.from_json(json_content) } + + it { expect { subject }.to raise_error(ActiveModel::ValidationError) } + end + context 'when title is missing' do before do json_content['title'] = nil @@ -54,7 +62,7 @@ describe PerformanceMonitoring::PrometheusPanel do context 'when metrics are missing' do before do - json_content['metrics'] = [] + json_content.delete('metrics') end subject { described_class.from_json(json_content) } diff --git a/spec/models/personal_access_token_spec.rb b/spec/models/personal_access_token_spec.rb index 596b11613b3..a3f5eb38511 100644 --- a/spec/models/personal_access_token_spec.rb +++ b/spec/models/personal_access_token_spec.rb @@ -178,6 +178,15 @@ describe PersonalAccessToken do end end end + + describe '.without_impersonation' do + let_it_be(:impersonation_token) { create(:personal_access_token, :impersonation) } + let_it_be(:personal_access_token) { create(:personal_access_token) } + + it 'returns only non-impersonation tokens' do + expect(described_class.without_impersonation).to contain_exactly(personal_access_token) + end + end end describe '.simple_sorts' do diff --git a/spec/models/project_ci_cd_setting_spec.rb b/spec/models/project_ci_cd_setting_spec.rb index 86115a61aa7..ecca371ce4e 100644 --- a/spec/models/project_ci_cd_setting_spec.rb +++ b/spec/models/project_ci_cd_setting_spec.rb @@ -3,25 +3,6 @@ require 'spec_helper' describe ProjectCiCdSetting do - describe '.available?' do - before do - described_class.reset_column_information - end - - it 'returns true' do - expect(described_class).to be_available - end - - it 'memoizes the schema version' do - expect(ActiveRecord::Migrator) - .to receive(:current_version) - .and_call_original - .once - - 2.times { described_class.available? } - end - end - describe 'validations' do it 'validates default_git_depth is between 0 and 1000 or nil' do expect(subject).to validate_numericality_of(:default_git_depth) diff --git a/spec/models/project_feature_spec.rb b/spec/models/project_feature_spec.rb index e072cc21b38..e33ea75bc5d 100644 --- a/spec/models/project_feature_spec.rb +++ b/spec/models/project_feature_spec.rb @@ -18,106 +18,6 @@ describe ProjectFeature do end end - describe '.quoted_access_level_column' do - it 'returns the table name and quoted column name for a feature' do - expected = '"project_features"."issues_access_level"' - - expect(described_class.quoted_access_level_column(:issues)).to eq(expected) - end - end - - describe '#feature_available?' do - let(:features) { %w(issues wiki builds merge_requests snippets repository pages metrics_dashboard) } - - context 'when features are disabled' do - it "returns false" do - update_all_project_features(project, features, ProjectFeature::DISABLED) - - features.each do |feature| - expect(project.feature_available?(feature.to_sym, user)).to eq(false), "#{feature} failed" - end - end - end - - context 'when features are enabled only for team members' do - it "returns false when user is not a team member" do - update_all_project_features(project, features, ProjectFeature::PRIVATE) - - features.each do |feature| - expect(project.feature_available?(feature.to_sym, user)).to eq(false), "#{feature} failed" - end - end - - it "returns true when user is a team member" do - project.add_developer(user) - - update_all_project_features(project, features, ProjectFeature::PRIVATE) - - features.each do |feature| - expect(project.feature_available?(feature.to_sym, user)).to eq(true), "#{feature} failed" - end - end - - it "returns true when user is a member of project group" do - group = create(:group) - project = create(:project, namespace: group) - group.add_developer(user) - - update_all_project_features(project, features, ProjectFeature::PRIVATE) - - features.each do |feature| - expect(project.feature_available?(feature.to_sym, user)).to eq(true), "#{feature} failed" - end - end - - context 'when admin mode is enabled', :enable_admin_mode do - it "returns true if user is an admin" do - user.update_attribute(:admin, true) - - update_all_project_features(project, features, ProjectFeature::PRIVATE) - - features.each do |feature| - expect(project.feature_available?(feature.to_sym, user)).to eq(true), "#{feature} failed" - end - end - end - - context 'when admin mode is disabled' do - it "returns false when user is an admin" do - user.update_attribute(:admin, true) - - update_all_project_features(project, features, ProjectFeature::PRIVATE) - - features.each do |feature| - expect(project.feature_available?(feature.to_sym, user)).to eq(false), "#{feature} failed" - end - end - end - end - - context 'when feature is enabled for everyone' do - it "returns true" do - expect(project.feature_available?(:issues, user)).to eq(true) - end - end - - context 'when feature is disabled by a feature flag' do - it 'returns false' do - stub_feature_flags(issues: false) - - expect(project.feature_available?(:issues, user)).to eq(false) - end - end - - context 'when feature is enabled by a feature flag' do - it 'returns true' do - stub_feature_flags(issues: true) - - expect(project.feature_available?(:issues, user)).to eq(true) - end - end - end - context 'repository related features' do before do project.project_feature.update( @@ -153,32 +53,6 @@ describe ProjectFeature do end end - describe '#*_enabled?' do - let(:features) { %w(wiki builds merge_requests) } - - it "returns false when feature is disabled" do - update_all_project_features(project, features, ProjectFeature::DISABLED) - - features.each do |feature| - expect(project.public_send("#{feature}_enabled?")).to eq(false), "#{feature} failed" - end - end - - it "returns true when feature is enabled only for team members" do - update_all_project_features(project, features, ProjectFeature::PRIVATE) - - features.each do |feature| - expect(project.public_send("#{feature}_enabled?")).to eq(true), "#{feature} failed" - end - end - - it "returns true when feature is enabled for everyone" do - features.each do |feature| - expect(project.public_send("#{feature}_enabled?")).to eq(true), "#{feature} failed" - end - end - end - describe 'default pages access level' do subject { project_feature.pages_access_level } @@ -313,9 +187,4 @@ describe ProjectFeature do expect(described_class.required_minimum_access_level_for_private_project(:issues)).to eq(Gitlab::Access::GUEST) end end - - def update_all_project_features(project, features, value) - project_feature_attributes = features.map { |f| ["#{f}_access_level", value] }.to_h - project.project_feature.update(project_feature_attributes) - end end diff --git a/spec/models/project_group_link_spec.rb b/spec/models/project_group_link_spec.rb index 9c51180b55b..8ef29e8a876 100644 --- a/spec/models/project_group_link_spec.rb +++ b/spec/models/project_group_link_spec.rb @@ -49,22 +49,6 @@ describe ProjectGroupLink do end end - describe "destroying a record", :delete do - it "refreshes group users' authorized projects" do - project = create(:project, :private) - group = create(:group) - reporter = create(:user) - group_users = group.users - - group.add_reporter(reporter) - project.project_group_links.create(group: group) - group_users.each { |user| expect(user.authorized_projects).to include(project) } - - project.project_group_links.destroy_all # rubocop: disable DestroyAll - group_users.each { |user| expect(user.authorized_projects).not_to include(project) } - end - end - describe 'search by group name' do let_it_be(:project_group_link) { create(:project_group_link) } let_it_be(:group) { project_group_link.group } diff --git a/spec/models/project_import_state_spec.rb b/spec/models/project_import_state_spec.rb index cb34d898a6e..f3b83c036b5 100644 --- a/spec/models/project_import_state_spec.rb +++ b/spec/models/project_import_state_spec.rb @@ -57,6 +57,30 @@ describe ProjectImportState, type: :model do end end + describe '#mark_as_failed' do + let(:error_message) { 'some message' } + + it 'logs error when update column fails' do + allow(import_state).to receive(:update_column).and_raise(ActiveRecord::ActiveRecordError) + + expect_next_instance_of(Gitlab::Import::Logger) do |logger| + expect(logger).to receive(:error).with( + error: 'ActiveRecord::ActiveRecordError', + message: 'Error setting import status to failed', + original_error: error_message + ) + end + + import_state.mark_as_failed(error_message) + end + + it 'updates last_error with error message' do + import_state.mark_as_failed(error_message) + + expect(import_state.last_error).to eq(error_message) + end + end + describe '#human_status_name' do context 'when import_state exists' do it 'returns the humanized status name' do diff --git a/spec/models/project_metrics_setting_spec.rb b/spec/models/project_metrics_setting_spec.rb index 7df01625ba1..adfbbbc3a45 100644 --- a/spec/models/project_metrics_setting_spec.rb +++ b/spec/models/project_metrics_setting_spec.rb @@ -44,12 +44,20 @@ describe ProjectMetricsSetting do it { is_expected.to be_valid } end - context 'external_dashboard_url is blank' do - before do - subject.external_dashboard_url = '' + context 'dashboard_timezone' do + it { is_expected.to define_enum_for(:dashboard_timezone).with_values({ local: 0, utc: 1 }) } + + it 'defaults to local' do + expect(subject.dashboard_timezone).to eq('local') end + end + end - it { is_expected.to be_invalid } + describe '#dashboard_timezone=' do + it 'downcases string' do + subject.dashboard_timezone = 'UTC' + + expect(subject.dashboard_timezone).to eq('utc') end end end diff --git a/spec/models/project_repository_storage_move_spec.rb b/spec/models/project_repository_storage_move_spec.rb index 146fc13bee0..83711085c92 100644 --- a/spec/models/project_repository_storage_move_spec.rb +++ b/spec/models/project_repository_storage_move_spec.rb @@ -30,25 +30,36 @@ RSpec.describe ProjectRepositoryStorageMove, type: :model do expect(subject.errors[:destination_storage_name].first).to match(/is not included in the list/) end end + + context 'project repository read-only' do + subject { build(:project_repository_storage_move, project: project) } + + let(:project) { build(:project, repository_read_only: true) } + + it "does not allow the project to be read-only on create" do + expect(subject).not_to be_valid + expect(subject.errors[:project].first).to match(/is read only/) + end + end end describe 'state transitions' do - using RSpec::Parameterized::TableSyntax + let(:project) { create(:project) } + + before do + stub_storage_settings('test_second_storage' => { 'path' => 'tmp/tests/extra_storage' }) + end context 'when in the default state' do subject(:storage_move) { create(:project_repository_storage_move, project: project, destination_storage_name: 'test_second_storage') } - let(:project) { create(:project) } - - before do - stub_storage_settings('test_second_storage' => { 'path' => 'tmp/tests/extra_storage' }) - end - context 'and transits to scheduled' do it 'triggers ProjectUpdateRepositoryStorageWorker' do expect(ProjectUpdateRepositoryStorageWorker).to receive(:perform_async).with(project.id, 'test_second_storage', storage_move.id) storage_move.schedule! + + expect(project).to be_repository_read_only end end @@ -59,5 +70,26 @@ RSpec.describe ProjectRepositoryStorageMove, type: :model do end end end + + context 'when started' do + subject(:storage_move) { create(:project_repository_storage_move, :started, project: project, destination_storage_name: 'test_second_storage') } + + context 'and transits to finished' do + it 'sets the repository storage and marks the project as writable' do + storage_move.finish! + + expect(project.repository_storage).to eq('test_second_storage') + expect(project).not_to be_repository_read_only + end + end + + context 'and transits to failed' do + it 'marks the project as writable' do + storage_move.do_fail! + + expect(project).not_to be_repository_read_only + end + end + end end end diff --git a/spec/models/project_services/chat_message/alert_message_spec.rb b/spec/models/project_services/chat_message/alert_message_spec.rb new file mode 100644 index 00000000000..a1dd332c005 --- /dev/null +++ b/spec/models/project_services/chat_message/alert_message_spec.rb @@ -0,0 +1,57 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe ChatMessage::AlertMessage do + subject { described_class.new(args) } + + let_it_be(:start_time) { Time.current } + let(:alert) { create(:alert_management_alert, started_at: start_time) } + + let(:args) do + { + project_name: 'project_name', + project_url: 'http://example.com' + }.merge(Gitlab::DataBuilder::Alert.build(alert)) + end + + describe '#message' do + it 'returns the correct message' do + expect(subject.message).to eq("Alert firing in #{args[:project_name]}") + end + end + + describe '#attachments' do + it 'returns an array of one' do + expect(subject.attachments).to be_a(Array) + expect(subject.attachments.size).to eq(1) + end + + it 'contains the correct attributes' do + attachments_item = subject.attachments.first + expect(attachments_item).to have_key(:title) + expect(attachments_item).to have_key(:title_link) + expect(attachments_item).to have_key(:color) + expect(attachments_item).to have_key(:fields) + end + + it 'returns the correct color' do + expect(subject.attachments.first[:color]).to eq("#C95823") + end + + it 'returns the correct attachment fields' do + attachments_item = subject.attachments.first + fields = attachments_item[:fields].map { |h| h[:title] } + + expect(fields).to match_array(['Severity', 'Events', 'Status', 'Start time']) + end + + it 'returns the correctly formatted time' do + time_item = subject.attachments.first[:fields].detect { |h| h[:title] == 'Start time' } + + expected_time = start_time.strftime("%B #{start_time.day.ordinalize}, %Y %l:%M%p %Z") + + expect(time_item[:value]).to eq(expected_time) + end + end +end diff --git a/spec/models/project_services/chat_message/merge_message_spec.rb b/spec/models/project_services/chat_message/merge_message_spec.rb index 150ee6f7472..6063ef4ecb3 100644 --- a/spec/models/project_services/chat_message/merge_message_spec.rb +++ b/spec/models/project_services/chat_message/merge_message_spec.rb @@ -52,7 +52,7 @@ describe ChatMessage::MergeMessage do context 'open' do it 'returns a message regarding opening of merge requests' do expect(subject.pretext).to eq( - 'Test User (test.user) opened <http://somewhere.com/-/merge_requests/100|!100 *Merge Request title*> in <http://somewhere.com|project_name>') + 'Test User (test.user) opened merge request <http://somewhere.com/-/merge_requests/100|!100 *Merge Request title*> in <http://somewhere.com|project_name>') expect(subject.attachments).to be_empty end end @@ -63,7 +63,7 @@ describe ChatMessage::MergeMessage do end it 'returns a message regarding closing of merge requests' do expect(subject.pretext).to eq( - 'Test User (test.user) closed <http://somewhere.com/-/merge_requests/100|!100 *Merge Request title*> in <http://somewhere.com|project_name>') + 'Test User (test.user) closed merge request <http://somewhere.com/-/merge_requests/100|!100 *Merge Request title*> in <http://somewhere.com|project_name>') expect(subject.attachments).to be_empty end end @@ -77,7 +77,7 @@ describe ChatMessage::MergeMessage do context 'open' do it 'returns a message regarding opening of merge requests' do expect(subject.pretext).to eq( - 'Test User (test.user) opened [!100 *Merge Request title*](http://somewhere.com/-/merge_requests/100) in [project_name](http://somewhere.com)') + 'Test User (test.user) opened merge request [!100 *Merge Request title*](http://somewhere.com/-/merge_requests/100) in [project_name](http://somewhere.com)') expect(subject.attachments).to be_empty expect(subject.activity).to eq({ title: 'Merge Request opened by Test User (test.user)', @@ -95,7 +95,7 @@ describe ChatMessage::MergeMessage do it 'returns a message regarding closing of merge requests' do expect(subject.pretext).to eq( - 'Test User (test.user) closed [!100 *Merge Request title*](http://somewhere.com/-/merge_requests/100) in [project_name](http://somewhere.com)') + 'Test User (test.user) closed merge request [!100 *Merge Request title*](http://somewhere.com/-/merge_requests/100) in [project_name](http://somewhere.com)') expect(subject.attachments).to be_empty expect(subject.activity).to eq({ title: 'Merge Request closed by Test User (test.user)', diff --git a/spec/models/project_services/chat_message/pipeline_message_spec.rb b/spec/models/project_services/chat_message/pipeline_message_spec.rb index 7c3e48f572a..a7171577063 100644 --- a/spec/models/project_services/chat_message/pipeline_message_spec.rb +++ b/spec/models/project_services/chat_message/pipeline_message_spec.rb @@ -61,8 +61,8 @@ describe ChatMessage::PipelineMessage do it "returns the pipeline summary in the activity's title" do expect(subject.activity[:title]).to eq( - "Pipeline [#123](http://example.gitlab.com/pipelines/123)" \ - " of branch [develop](http://example.gitlab.com/commits/develop)" \ + "Pipeline [#123](http://example.gitlab.com/-/pipelines/123)" \ + " of branch [develop](http://example.gitlab.com/-/commits/develop)" \ " by The Hacker (hacker) has passed" ) end @@ -74,8 +74,8 @@ describe ChatMessage::PipelineMessage do it "returns the summary with a 'failed' status" do expect(subject.activity[:title]).to eq( - "Pipeline [#123](http://example.gitlab.com/pipelines/123)" \ - " of branch [develop](http://example.gitlab.com/commits/develop)" \ + "Pipeline [#123](http://example.gitlab.com/-/pipelines/123)" \ + " of branch [develop](http://example.gitlab.com/-/commits/develop)" \ " by The Hacker (hacker) has failed" ) end @@ -88,8 +88,8 @@ describe ChatMessage::PipelineMessage do it "returns the summary with a 'passed with warnings' status" do expect(subject.activity[:title]).to eq( - "Pipeline [#123](http://example.gitlab.com/pipelines/123)" \ - " of branch [develop](http://example.gitlab.com/commits/develop)" \ + "Pipeline [#123](http://example.gitlab.com/-/pipelines/123)" \ + " of branch [develop](http://example.gitlab.com/-/commits/develop)" \ " by The Hacker (hacker) has passed with warnings" ) end @@ -102,8 +102,8 @@ describe ChatMessage::PipelineMessage do it "returns the summary with 'API' as the username" do expect(subject.activity[:title]).to eq( - "Pipeline [#123](http://example.gitlab.com/pipelines/123)" \ - " of branch [develop](http://example.gitlab.com/commits/develop)" \ + "Pipeline [#123](http://example.gitlab.com/-/pipelines/123)" \ + " of branch [develop](http://example.gitlab.com/-/commits/develop)" \ " by API has passed" ) end @@ -134,8 +134,8 @@ describe ChatMessage::PipelineMessage do it "returns the pipeline summary as the attachment's fallback property" do expect(subject.attachments.first[:fallback]).to eq( "<http://example.gitlab.com|project_name>:" \ - " Pipeline <http://example.gitlab.com/pipelines/123|#123>" \ - " of branch <http://example.gitlab.com/commits/develop|develop>" \ + " Pipeline <http://example.gitlab.com/-/pipelines/123|#123>" \ + " of branch <http://example.gitlab.com/-/commits/develop|develop>" \ " by The Hacker (hacker) has passed in 02:00:10" ) end @@ -199,7 +199,7 @@ describe ChatMessage::PipelineMessage do end it "returns the pipeline URL as the attachment's title_link property" do - expect(subject.attachments.first[:title_link]).to eq("http://example.gitlab.com/pipelines/123") + expect(subject.attachments.first[:title_link]).to eq("http://example.gitlab.com/-/pipelines/123") end it "returns two attachment fields" do @@ -209,7 +209,7 @@ describe ChatMessage::PipelineMessage do it "returns the commit message as the attachment's second field property" do expect(subject.attachments.first[:fields][0]).to eq({ title: "Branch", - value: "<http://example.gitlab.com/commits/develop|develop>", + value: "<http://example.gitlab.com/-/commits/develop|develop>", short: true }) end @@ -237,7 +237,7 @@ describe ChatMessage::PipelineMessage do it "returns the stage name and link to the 'Failed jobs' tab on the pipeline's page as the attachment's third field property" do expect(subject.attachments.first[:fields][2]).to eq({ title: "Failed stage", - value: "<http://example.gitlab.com/pipelines/123/failures|test>", + value: "<http://example.gitlab.com/-/pipelines/123/failures|test>", short: true }) end @@ -261,7 +261,7 @@ describe ChatMessage::PipelineMessage do it "returns the stage names and links to the 'Failed jobs' tab on the pipeline's page as the attachment's third field property" do expect(subject.attachments.first[:fields][2]).to eq({ title: "Failed stages", - value: "<http://example.gitlab.com/pipelines/123/failures|stage-2>, <http://example.gitlab.com/pipelines/123/failures|stage-1>, <http://example.gitlab.com/pipelines/123/failures|stage-3>", + value: "<http://example.gitlab.com/-/pipelines/123/failures|stage-2>, <http://example.gitlab.com/-/pipelines/123/failures|stage-1>, <http://example.gitlab.com/-/pipelines/123/failures|stage-3>", short: true }) end @@ -271,7 +271,7 @@ describe ChatMessage::PipelineMessage do "<http://example.gitlab.com/-/jobs/#{i}|job-#{i}>" end - expected_jobs << "and <http://example.gitlab.com/pipelines/123/failures|15 more>" + expected_jobs << "and <http://example.gitlab.com/-/pipelines/123/failures|15 more>" expect(subject.attachments.first[:fields][3]).to eq({ title: "Failed jobs", @@ -369,8 +369,8 @@ describe ChatMessage::PipelineMessage do it 'returns the pipeline summary as the attachments in markdown format' do expect(subject.attachments).to eq( "[project_name](http://example.gitlab.com):" \ - " Pipeline [#123](http://example.gitlab.com/pipelines/123)" \ - " of branch [develop](http://example.gitlab.com/commits/develop)" \ + " Pipeline [#123](http://example.gitlab.com/-/pipelines/123)" \ + " of branch [develop](http://example.gitlab.com/-/commits/develop)" \ " by The Hacker (hacker) has passed in 02:00:10" ) end diff --git a/spec/models/project_services/emails_on_push_service_spec.rb b/spec/models/project_services/emails_on_push_service_spec.rb index ce1952b503f..44db95afc57 100644 --- a/spec/models/project_services/emails_on_push_service_spec.rb +++ b/spec/models/project_services/emails_on_push_service_spec.rb @@ -21,23 +21,25 @@ describe EmailsOnPushService do end end - context 'when properties is missing branches_to_be_notified' do - subject { described_class.new(properties: {}) } + describe '.new' do + context 'when properties is missing branches_to_be_notified' do + subject { described_class.new(properties: {}) } - it 'sets the default value to all' do - expect(subject.branches_to_be_notified).to eq('all') + it 'sets the default value to all' do + expect(subject.branches_to_be_notified).to eq('all') + end end - end - context 'when branches_to_be_notified is already set' do - subject { described_class.new(properties: { branches_to_be_notified: 'protected' }) } + context 'when branches_to_be_notified is already set' do + subject { described_class.new(properties: { branches_to_be_notified: 'protected' }) } - it 'does not overwrite it with the default value' do - expect(subject.branches_to_be_notified).to eq('protected') + it 'does not overwrite it with the default value' do + expect(subject.branches_to_be_notified).to eq('protected') + end end end - context 'project emails' do + describe '#execute' do let(:push_data) { { object_kind: 'push' } } let(:project) { create(:project, :repository) } let(:service) { create(:emails_on_push_service, project: project) } diff --git a/spec/models/project_services/hipchat_service_spec.rb b/spec/models/project_services/hipchat_service_spec.rb index ae6e93cfe3a..c25edf81352 100644 --- a/spec/models/project_services/hipchat_service_spec.rb +++ b/spec/models/project_services/hipchat_service_spec.rb @@ -327,8 +327,8 @@ describe HipchatService do user_name = data[:user][:name] expect(message).to eq("<a href=\"#{project_url}\">#{project_name}</a>: " \ - "Pipeline <a href=\"#{project_url}/pipelines/#{pipeline.id}\">##{pipeline.id}</a> " \ - "of <a href=\"#{project_url}/commits/#{ref}\">#{ref}</a> #{ref_type} " \ + "Pipeline <a href=\"#{project_url}/-/pipelines/#{pipeline.id}\">##{pipeline.id}</a> " \ + "of <a href=\"#{project_url}/-/commits/#{ref}\">#{ref}</a> #{ref_type} " \ "by #{user_name} failed in #{duration} second(s)") end end diff --git a/spec/models/project_services/jira_service_spec.rb b/spec/models/project_services/jira_service_spec.rb index a0d36f0a238..20e85f0fd4b 100644 --- a/spec/models/project_services/jira_service_spec.rb +++ b/spec/models/project_services/jira_service_spec.rb @@ -688,22 +688,18 @@ describe JiraService do context 'when the test fails' do it 'returns result with the error' do test_url = 'http://jira.example.com/rest/api/2/serverInfo' + error_message = 'Some specific failure.' WebMock.stub_request(:get, test_url).with(basic_auth: [username, password]) - .to_raise(JIRA::HTTPError.new(double(message: 'Some specific failure.'))) + .to_raise(JIRA::HTTPError.new(double(message: error_message))) expect(jira_service).to receive(:log_error).with( - "Error sending message", - hash_including( - client_url: url, - error: hash_including( - exception_class: 'JIRA::HTTPError', - exception_message: 'Some specific failure.' - ) - ) + 'Error sending message', + client_url: 'http://jira.example.com', + error: error_message ) - expect(jira_service.test(nil)).to eq(success: false, result: 'Some specific failure.') + expect(jira_service.test(nil)).to eq(success: false, result: error_message) end end end diff --git a/spec/models/project_services/pipelines_email_service_spec.rb b/spec/models/project_services/pipelines_email_service_spec.rb index f29414c80c9..de1edf2099a 100644 --- a/spec/models/project_services/pipelines_email_service_spec.rb +++ b/spec/models/project_services/pipelines_email_service_spec.rb @@ -37,22 +37,6 @@ describe PipelinesEmailService, :mailer do end end - describe '#test_data' do - let(:build) { create(:ci_build) } - let(:project) { build.project } - let(:user) { create(:user) } - - before do - project.add_developer(user) - end - - it 'builds test data' do - data = subject.test_data(project, user) - - expect(data[:object_kind]).to eq('pipeline') - end - end - shared_examples 'sending email' do |branches_to_be_notified: nil| before do subject.recipients = recipients diff --git a/spec/models/project_services/prometheus_service_spec.rb b/spec/models/project_services/prometheus_service_spec.rb index a85dbe3a7df..db3cbe23ad3 100644 --- a/spec/models/project_services/prometheus_service_spec.rb +++ b/spec/models/project_services/prometheus_service_spec.rb @@ -252,6 +252,26 @@ describe PrometheusService, :use_clean_rails_memory_store_caching do end end end + + context 'behind IAP' do + let(:manual_configuration) { true } + + before do + # dummy private key generated only for this test to pass openssl validation + service.google_iap_service_account_json = '{"type":"service_account","private_key":"-----BEGIN RSA PRIVATE KEY-----\nMIIBOAIBAAJAU85LgUY5o6j6j/07GMLCNUcWJOBA1buZnNgKELayA6mSsHrIv31J\nY8kS+9WzGPQninea7DcM4hHA7smMgQD1BwIDAQABAkAqKxMy6PL3tn7dFL43p0ex\nJyOtSmlVIiAZG1t1LXhE/uoLpYi5DnbYqGgu0oih+7nzLY/dXpNpXUmiRMOUEKmB\nAiEAoTi2rBXbrLSi2C+H7M/nTOjMQQDuZ8Wr4uWpKcjYJTMCIQCFEskL565oFl/7\nRRQVH+cARrAsAAoJSbrOBAvYZ0PI3QIgIEFwis10vgEF86rOzxppdIG/G+JL0IdD\n9IluZuXAGPECIGUo7qSaLr75o2VEEgwtAFH5aptIPFjrL5LFCKwtdB4RAiAYZgFV\nHCMmaooAw/eELuMoMWNYmujZ7VaAnOewGDW0uw==\n-----END RSA PRIVATE KEY-----\n"}' + service.google_iap_audience_client_id = "IAP_CLIENT_ID.apps.googleusercontent.com" + + stub_request(:post, "https://oauth2.googleapis.com/token").to_return(status: 200, body: '{"id_token": "FOO"}', headers: { 'Content-Type': 'application/json; charset=UTF-8' }) + + stub_feature_flags(prometheus_service_iap_auth: true) + end + + it 'includes the authorization header' do + expect(service.prometheus_client).not_to be_nil + expect(service.prometheus_client.send(:options)).to have_key(:headers) + expect(service.prometheus_client.send(:options)[:headers]).to eq(authorization: "Bearer FOO") + end + end end describe '#prometheus_available?' do @@ -457,9 +477,34 @@ describe PrometheusService, :use_clean_rails_memory_store_caching do } ] end + let(:feature_flagged_fields) do + [ + { + type: 'text', + name: 'google_iap_audience_client_id', + title: 'Google IAP Audience Client ID', + placeholder: s_('PrometheusService|Client ID of the IAP secured resource (looks like IAP_CLIENT_ID.apps.googleusercontent.com)'), + autocomplete: 'off', + required: false + }, + { + type: 'textarea', + name: 'google_iap_service_account_json', + title: 'Google IAP Service Account JSON', + placeholder: s_('PrometheusService|Contents of the credentials.json file of your service account, like: { "type": "service_account", "project_id": ... }'), + required: false + } + ] + end it 'returns fields' do + stub_feature_flags(prometheus_service_iap_auth: false) expect(service.fields).to eq(expected_fields) end + + it 'returns fields with feature flag on' do + stub_feature_flags(prometheus_service_iap_auth: true) + expect(service.fields).to eq(expected_fields + feature_flagged_fields) + end end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 5f8b51c250d..9ec306d297e 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -79,6 +79,7 @@ describe Project do it { is_expected.to have_many(:ci_refs) } it { is_expected.to have_many(:builds) } it { is_expected.to have_many(:build_trace_section_names)} + it { is_expected.to have_many(:build_report_results) } it { is_expected.to have_many(:runner_projects) } it { is_expected.to have_many(:runners) } it { is_expected.to have_many(:variables) } @@ -117,6 +118,7 @@ describe Project do it { is_expected.to have_many(:jira_imports) } it { is_expected.to have_many(:metrics_users_starred_dashboards).inverse_of(:project) } it { is_expected.to have_many(:repository_storage_moves) } + it { is_expected.to have_many(:reviews).inverse_of(:project) } it_behaves_like 'model with repository' do let_it_be(:container) { create(:project, :repository, path: 'somewhere') } @@ -181,9 +183,9 @@ describe Project do expect(project.pages_metadatum).to be_persisted end - it 'automatically creates a project setting row' do + it 'automatically builds a project setting row' do expect(project.project_setting).to be_an_instance_of(ProjectSetting) - expect(project.project_setting).to be_persisted + expect(project.project_setting).to be_new_record end end @@ -770,7 +772,7 @@ describe Project do describe 'last_activity_date' do it 'returns the creation date of the project\'s last event if present' do - new_event = create(:event, :closed, project: project, created_at: Time.now) + new_event = create(:event, :closed, project: project, created_at: Time.current) project.reload expect(project.last_activity_at.to_i).to eq(new_event.created_at.to_i) @@ -2836,48 +2838,6 @@ describe Project do end end - describe '#change_repository_storage' do - let(:project) { create(:project, :repository) } - let(:read_only_project) { create(:project, :repository, repository_read_only: true) } - - before do - stub_storage_settings('test_second_storage' => { 'path' => 'tmp/tests/extra_storage' }) - end - - it 'schedules the transfer of the repository to the new storage and locks the project' do - expect(ProjectUpdateRepositoryStorageWorker).to receive(:perform_async).with(project.id, 'test_second_storage', anything) - - project.change_repository_storage('test_second_storage') - project.save! - - expect(project).to be_repository_read_only - expect(project.repository_storage_moves.last).to have_attributes( - source_storage_name: "default", - destination_storage_name: "test_second_storage" - ) - end - - it "doesn't schedule the transfer if the repository is already read-only" do - expect(ProjectUpdateRepositoryStorageWorker).not_to receive(:perform_async) - - read_only_project.change_repository_storage('test_second_storage') - read_only_project.save! - end - - it "doesn't lock or schedule the transfer if the storage hasn't changed" do - expect(ProjectUpdateRepositoryStorageWorker).not_to receive(:perform_async) - - project.change_repository_storage(project.repository_storage) - project.save! - - expect(project).not_to be_repository_read_only - end - - it 'throws an error if an invalid repository storage is provided' do - expect { project.change_repository_storage('unknown') }.to raise_error(ArgumentError) - end - end - describe '#pushes_since_gc' do let(:project) { create(:project) } @@ -3620,7 +3580,7 @@ describe Project do expect(project).not_to receive(:visibility_level_allowed_as_fork).and_call_original expect(project).not_to receive(:visibility_level_allowed_by_group).and_call_original - project.update(updated_at: Time.now) + project.update(updated_at: Time.current) end end @@ -3766,7 +3726,7 @@ describe Project do context 'when feature is private' do let(:project) { create(:project, :public, :merge_requests_private) } - context 'when user does not has access to the feature' do + context 'when user does not have access to the feature' do it 'does not return projects with the project feature private' do is_expected.not_to include(project) end @@ -4302,8 +4262,7 @@ describe Project do describe '#auto_devops_enabled?' do before do - allow(Feature).to receive(:enabled?).and_call_original - Feature.get(:force_autodevops_on_by_default).enable_percentage_of_actors(0) + Feature.enable_percentage_of_actors(:force_autodevops_on_by_default, 0) end let_it_be(:project, reload: true) { create(:project) } @@ -4505,8 +4464,7 @@ describe Project do let_it_be(:project, reload: true) { create(:project) } before do - allow(Feature).to receive(:enabled?).and_call_original - Feature.get(:force_autodevops_on_by_default).enable_percentage_of_actors(0) + Feature.enable_percentage_of_actors(:force_autodevops_on_by_default, 0) end context 'when explicitly disabled' do @@ -4552,7 +4510,7 @@ describe Project do before do create(:project_auto_devops, project: project, enabled: false) - Feature.get(:force_autodevops_on_by_default).enable_percentage_of_actors(100) + Feature.enable_percentage_of_actors(:force_autodevops_on_by_default, 100) end it 'does not have auto devops implicitly disabled' do @@ -4854,6 +4812,32 @@ describe Project do end end + describe '#execute_services' do + let(:service) { create(:slack_service, push_events: true, merge_requests_events: false, active: true) } + + it 'executes services with the specified scope' do + data = 'any data' + + expect(SlackService).to receive(:allocate).and_wrap_original do |method| + method.call.tap do |instance| + expect(instance).to receive(:async_execute).with(data).once + end + end + + service.project.execute_services(data, :push_hooks) + end + + it 'does not execute services that don\'t match the specified scope' do + expect(SlackService).not_to receive(:allocate).and_wrap_original do |method| + method.call.tap do |instance| + expect(instance).not_to receive(:async_execute) + end + end + + service.project.execute_services(anything, :merge_request_hooks) + end + end + describe '#has_active_hooks?' do let_it_be(:project) { create(:project) } @@ -5241,25 +5225,21 @@ describe Project do end end - describe "#find_or_initialize_services" do - subject { build(:project) } - + describe '#find_or_initialize_services' do it 'returns only enabled services' do - allow(Service).to receive(:available_services_names).and_return(%w(prometheus pushover)) - allow(subject).to receive(:disabled_services).and_return(%w(prometheus)) + allow(Service).to receive(:available_services_names).and_return(%w[prometheus pushover teamcity]) + allow(subject).to receive(:disabled_services).and_return(%w[prometheus]) services = subject.find_or_initialize_services - expect(services.count).to eq 1 - expect(services).to include(PushoverService) + expect(services.count).to eq(2) + expect(services.map(&:title)).to eq(['JetBrains TeamCity CI', 'Pushover']) end end - describe "#find_or_initialize_service" do - subject { build(:project) } - + describe '#find_or_initialize_service' do it 'avoids N+1 database queries' do - allow(Service).to receive(:available_services_names).and_return(%w(prometheus pushover)) + allow(Service).to receive(:available_services_names).and_return(%w[prometheus pushover]) control_count = ActiveRecord::QueryRecorder.new { subject.find_or_initialize_service('prometheus') }.count @@ -5268,11 +5248,51 @@ describe Project do expect { subject.find_or_initialize_service('prometheus') }.not_to exceed_query_limit(control_count) end - it 'returns nil if service is disabled' do - allow(subject).to receive(:disabled_services).and_return(%w(prometheus)) + it 'returns nil if integration is disabled' do + allow(subject).to receive(:disabled_services).and_return(%w[prometheus]) expect(subject.find_or_initialize_service('prometheus')).to be_nil end + + context 'with an existing integration' do + subject { create(:project) } + + before do + create(:prometheus_service, project: subject, api_url: 'https://prometheus.project.com/') + end + + it 'retrieves the integration' do + expect(subject.find_or_initialize_service('prometheus').api_url).to eq('https://prometheus.project.com/') + end + end + + context 'with an instance-level and template integrations' do + before do + create(:prometheus_service, :instance, api_url: 'https://prometheus.instance.com/') + create(:prometheus_service, :template, api_url: 'https://prometheus.template.com/') + end + + it 'builds the service from the instance if exists' do + expect(subject.find_or_initialize_service('prometheus').api_url).to eq('https://prometheus.instance.com/') + end + end + + context 'with an instance-level and template integrations' do + before do + create(:prometheus_service, :template, api_url: 'https://prometheus.template.com/') + end + + it 'builds the service from the template if instance does not exists' do + expect(subject.find_or_initialize_service('prometheus').api_url).to eq('https://prometheus.template.com/') + end + end + + context 'without an exisiting integration, nor instance-level or template' do + it 'builds the service if instance or template does not exists' do + expect(subject.find_or_initialize_service('prometheus')).to be_a(PrometheusService) + expect(subject.find_or_initialize_service('prometheus').api_url).to be_nil + end + end end describe '.for_group' do @@ -6005,119 +6025,6 @@ describe Project do end end - describe '#validate_jira_import_settings!' do - include JiraServiceHelper - - let_it_be(:project, reload: true) { create(:project) } - - shared_examples 'raise Jira import error' do |message| - it 'returns error' do - expect { subject }.to raise_error(Projects::ImportService::Error, message) - end - end - - shared_examples 'jira configuration base checks' do - context 'when feature flag is disabled' do - before do - stub_feature_flags(jira_issue_import: false) - end - - it_behaves_like 'raise Jira import error', 'Jira import feature is disabled.' - end - - context 'when feature flag is enabled' do - before do - stub_feature_flags(jira_issue_import: true) - end - - context 'when Jira service was not setup' do - it_behaves_like 'raise Jira import error', 'Jira integration not configured.' - end - - context 'when Jira service exists' do - let!(:jira_service) { create(:jira_service, project: project, active: true) } - - context 'when Jira connection is not valid' do - before do - WebMock.stub_request(:get, 'https://jira.example.com/rest/api/2/serverInfo') - .to_raise(JIRA::HTTPError.new(double(message: 'Some failure.'))) - end - - it_behaves_like 'raise Jira import error', 'Unable to connect to the Jira instance. Please check your Jira integration configuration.' - end - end - end - end - - before do - stub_jira_service_test - end - - context 'without user param' do - subject { project.validate_jira_import_settings! } - - it_behaves_like 'jira configuration base checks' - - context 'when jira connection is valid' do - let!(:jira_service) { create(:jira_service, project: project, active: true) } - - it 'does not return any error' do - expect { subject }.not_to raise_error - end - end - end - - context 'with user param provided' do - let_it_be(:user) { create(:user) } - - subject { project.validate_jira_import_settings!(user: user) } - - context 'when user has permission to run import' do - before do - project.add_maintainer(user) - end - - it_behaves_like 'jira configuration base checks' - end - - context 'when feature flag is enabled' do - before do - stub_feature_flags(jira_issue_import: true) - end - - context 'when user does not have permissions to run the import' do - before do - create(:jira_service, project: project, active: true) - - project.add_developer(user) - end - - it_behaves_like 'raise Jira import error', 'You do not have permissions to run the import.' - end - - context 'when user has permission to run import' do - before do - project.add_maintainer(user) - end - - let!(:jira_service) { create(:jira_service, project: project, active: true) } - - context 'when issues feature is disabled' do - let_it_be(:project, reload: true) { create(:project, :issues_disabled) } - - it_behaves_like 'raise Jira import error', 'Cannot import because issues are not available in this project.' - end - - context 'when everything is ok' do - it 'does not return any error' do - expect { subject }.not_to raise_error - end - end - end - end - end - end - describe '#design_management_enabled?' do let(:project) { build(:project) } @@ -6157,6 +6064,14 @@ describe Project do it { is_expected.not_to include(user) } end + describe "#metrics_setting" do + let(:project) { build(:project) } + + it 'creates setting if it does not exist' do + expect(project.metrics_setting).to be_an_instance_of(ProjectMetricsSetting) + end + end + def finish_job(export_job) export_job.start export_job.finish diff --git a/spec/models/project_team_spec.rb b/spec/models/project_team_spec.rb index d62fa58739a..24652a1d706 100644 --- a/spec/models/project_team_spec.rb +++ b/spec/models/project_team_spec.rb @@ -96,11 +96,9 @@ describe ProjectTeam do it 'returns invited members of a group' do group_member = create(:group_member) - - project.project_group_links.create!( - group: group_member.group, - group_access: Gitlab::Access::GUEST - ) + create(:project_group_link, group: group_member.group, + project: project, + group_access: Gitlab::Access::GUEST) expect(project.team.members) .to contain_exactly(group_member.user, project.owner) @@ -108,11 +106,9 @@ describe ProjectTeam do it 'returns invited members of a group of a specified level' do group_member = create(:group_member) - - project.project_group_links.create!( - group: group_member.group, - group_access: Gitlab::Access::REPORTER - ) + create(:project_group_link, group: group_member.group, + project: project, + group_access: Gitlab::Access::REPORTER) expect(project.team.guests).to be_empty expect(project.team.reporters).to contain_exactly(group_member.user) diff --git a/spec/models/project_wiki_spec.rb b/spec/models/project_wiki_spec.rb index a4181e3be9a..aff2b248642 100644 --- a/spec/models/project_wiki_spec.rb +++ b/spec/models/project_wiki_spec.rb @@ -27,8 +27,8 @@ describe ProjectWiki do subject.create_page('Test Page', 'This is content') wiki_container.reload - expect(wiki_container.last_activity_at).to be_within(1.minute).of(Time.now) - expect(wiki_container.last_repository_updated_at).to be_within(1.minute).of(Time.now) + expect(wiki_container.last_activity_at).to be_within(1.minute).of(Time.current) + expect(wiki_container.last_repository_updated_at).to be_within(1.minute).of(Time.current) end end end diff --git a/spec/models/prometheus_alert_event_spec.rb b/spec/models/prometheus_alert_event_spec.rb index 040113643dd..85e57cb08c3 100644 --- a/spec/models/prometheus_alert_event_spec.rb +++ b/spec/models/prometheus_alert_event_spec.rb @@ -49,7 +49,7 @@ describe PrometheusAlertEvent do describe 'transaction' do describe 'fire' do - let(:started_at) { Time.now } + let(:started_at) { Time.current } context 'when status is none' do subject { build(:prometheus_alert_event, :none) } @@ -75,7 +75,7 @@ describe PrometheusAlertEvent do end describe 'resolve' do - let(:ended_at) { Time.now } + let(:ended_at) { Time.current } context 'when firing' do subject { build(:prometheus_alert_event) } diff --git a/spec/models/push_event_spec.rb b/spec/models/push_event_spec.rb index 8682e1c797b..5c1802669c1 100644 --- a/spec/models/push_event_spec.rb +++ b/spec/models/push_event_spec.rb @@ -118,8 +118,8 @@ describe PushEvent do end describe '.sti_name' do - it 'returns Event::PUSHED' do - expect(described_class.sti_name).to eq(Event::PUSHED) + it 'returns the integer representation of the :pushed event action' do + expect(described_class.sti_name).to eq(Event.actions[:pushed]) end end @@ -299,7 +299,7 @@ describe PushEvent do describe '#validate_push_action' do it 'adds an error when the action is not PUSHED' do - event.action = Event::CREATED + event.action = :created event.validate_push_action expect(event.errors.count).to eq(1) diff --git a/spec/models/release_spec.rb b/spec/models/release_spec.rb index d72fd137f3f..716e7dc786e 100644 --- a/spec/models/release_spec.rb +++ b/spec/models/release_spec.rb @@ -94,14 +94,6 @@ RSpec.describe Release do describe 'evidence' do let(:release_with_evidence) { create(:release, :with_evidence, project: project) } - describe '#create_evidence!' do - context 'when a release is created' do - it 'creates one Evidence object too' do - expect { release_with_evidence }.to change(Releases::Evidence, :count).by(1) - end - end - end - context 'when a release is deleted' do it 'also deletes the associated evidence' do release_with_evidence diff --git a/spec/models/releases/evidence_spec.rb b/spec/models/releases/evidence_spec.rb index d38d2021117..927e2e9bbe6 100644 --- a/spec/models/releases/evidence_spec.rb +++ b/spec/models/releases/evidence_spec.rb @@ -5,83 +5,21 @@ require 'spec_helper' describe Releases::Evidence do let_it_be(:project) { create(:project) } let(:release) { create(:release, project: project) } - let(:schema_file) { 'evidences/evidence' } - let(:summary_json) { described_class.last.summary.to_json } describe 'associations' do it { is_expected.to belong_to(:release) } end - describe 'summary_sha' do - it 'returns nil if summary is nil' do - expect(build(:evidence, summary: nil).summary_sha).to be_nil - end - end - - describe '#generate_summary_and_sha' do - before do - described_class.create!(release: release) - end - - context 'when a release name is not provided' do - let(:release) { create(:release, project: project, name: nil) } - - it 'creates a valid JSON object' do - expect(release.name).to eq(release.tag) - expect(summary_json).to match_schema(schema_file) - end - end - - context 'when a release is associated to a milestone' do - let(:milestone) { create(:milestone, project: project) } - let(:release) { create(:release, project: project, milestones: [milestone]) } - - context 'when a milestone has no issue associated with it' do - it 'creates a valid JSON object' do - expect(milestone.issues).to be_empty - expect(summary_json).to match_schema(schema_file) - end - end - - context 'when a milestone has no description' do - let(:milestone) { create(:milestone, project: project, description: nil) } - - it 'creates a valid JSON object' do - expect(milestone.description).to be_nil - expect(summary_json).to match_schema(schema_file) - end - end - - context 'when a milestone has no due_date' do - let(:milestone) { create(:milestone, project: project, due_date: nil) } - - it 'creates a valid JSON object' do - expect(milestone.due_date).to be_nil - expect(summary_json).to match_schema(schema_file) - end - end - - context 'when a milestone has an issue' do - context 'when the issue has no description' do - let(:issue) { create(:issue, project: project, description: nil, state: 'closed') } - - before do - milestone.issues << issue - end + it 'filters out issues from summary json' do + milestone = create(:milestone, project: project, due_date: nil) + issue = create(:issue, project: project, description: nil, state: 'closed') + milestone.issues << issue + release.milestones << milestone - it 'creates a valid JSON object' do - expect(milestone.issues.first.description).to be_nil - expect(summary_json).to match_schema(schema_file) - end - end - end - end + ::Releases::CreateEvidenceService.new(release).execute + evidence = release.evidences.last - context 'when a release is not associated to any milestone' do - it 'creates a valid JSON object' do - expect(release.milestones).to be_empty - expect(summary_json).to match_schema(schema_file) - end - end + expect(evidence.read_attribute(:summary)["release"]["milestones"].first["issues"].first["title"]).to be_present + expect(evidence.summary["release"]["milestones"].first["issues"]).to be_nil end end diff --git a/spec/models/remote_mirror_spec.rb b/spec/models/remote_mirror_spec.rb index a87cdcf9344..6d163a16e63 100644 --- a/spec/models/remote_mirror_spec.rb +++ b/spec/models/remote_mirror_spec.rb @@ -306,7 +306,7 @@ describe RemoteMirror, :mailer do context 'when it did not update in the last minute' do it 'schedules a RepositoryUpdateRemoteMirrorWorker to run now' do - expect(RepositoryUpdateRemoteMirrorWorker).to receive(:perform_async).with(remote_mirror.id, Time.now) + expect(RepositoryUpdateRemoteMirrorWorker).to receive(:perform_async).with(remote_mirror.id, Time.current) remote_mirror.sync end @@ -314,9 +314,9 @@ describe RemoteMirror, :mailer do context 'when it did update in the last minute' do it 'schedules a RepositoryUpdateRemoteMirrorWorker to run in the next minute' do - remote_mirror.last_update_started_at = Time.now - 30.seconds + remote_mirror.last_update_started_at = Time.current - 30.seconds - expect(RepositoryUpdateRemoteMirrorWorker).to receive(:perform_in).with(RemoteMirror::PROTECTED_BACKOFF_DELAY, remote_mirror.id, Time.now) + expect(RepositoryUpdateRemoteMirrorWorker).to receive(:perform_in).with(RemoteMirror::PROTECTED_BACKOFF_DELAY, remote_mirror.id, Time.current) remote_mirror.sync end @@ -330,7 +330,7 @@ describe RemoteMirror, :mailer do context 'when it did not update in the last 5 minutes' do it 'schedules a RepositoryUpdateRemoteMirrorWorker to run now' do - expect(RepositoryUpdateRemoteMirrorWorker).to receive(:perform_async).with(remote_mirror.id, Time.now) + expect(RepositoryUpdateRemoteMirrorWorker).to receive(:perform_async).with(remote_mirror.id, Time.current) remote_mirror.sync end @@ -338,9 +338,9 @@ describe RemoteMirror, :mailer do context 'when it did update within the last 5 minutes' do it 'schedules a RepositoryUpdateRemoteMirrorWorker to run in the next 5 minutes' do - remote_mirror.last_update_started_at = Time.now - 30.seconds + remote_mirror.last_update_started_at = Time.current - 30.seconds - expect(RepositoryUpdateRemoteMirrorWorker).to receive(:perform_in).with(RemoteMirror::UNPROTECTED_BACKOFF_DELAY, remote_mirror.id, Time.now) + expect(RepositoryUpdateRemoteMirrorWorker).to receive(:perform_in).with(RemoteMirror::UNPROTECTED_BACKOFF_DELAY, remote_mirror.id, Time.current) remote_mirror.sync end @@ -377,9 +377,9 @@ 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.now, - last_update_at: Time.now, - last_successful_update_at: Time.now, + remote_mirror.update(last_error: Time.current, + last_update_at: Time.current, + last_successful_update_at: Time.current, update_status: 'started', error_notification_sent: true) @@ -394,14 +394,14 @@ describe RemoteMirror, :mailer do describe '#updated_since?' do let(:remote_mirror) { create(:project, :repository, :remote_mirror).remote_mirrors.first } - let(:timestamp) { Time.now - 5.minutes } + let(:timestamp) { Time.current - 5.minutes } around do |example| Timecop.freeze { example.run } end before do - remote_mirror.update(last_update_started_at: Time.now) + remote_mirror.update(last_update_started_at: Time.current) end context 'when remote mirror does not have status failed' do @@ -410,7 +410,7 @@ describe RemoteMirror, :mailer do end it 'returns false when last update started before the timestamp' do - expect(remote_mirror.updated_since?(Time.now + 5.minutes)).to be false + expect(remote_mirror.updated_since?(Time.current + 5.minutes)).to be false end end diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index be626dd6e32..c698b40a4c0 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -88,8 +88,8 @@ describe Repository do subject { repository.tags_sorted_by('updated_desc').map(&:name) } before do - double_first = double(committed_date: Time.now) - double_last = double(committed_date: Time.now - 1.second) + double_first = double(committed_date: Time.current) + double_last = double(committed_date: Time.current - 1.second) allow(tag_a).to receive(:dereferenced_target).and_return(double_first) allow(tag_b).to receive(:dereferenced_target).and_return(double_last) @@ -103,8 +103,8 @@ describe Repository do subject { repository.tags_sorted_by('updated_asc').map(&:name) } before do - double_first = double(committed_date: Time.now - 1.second) - double_last = double(committed_date: Time.now) + double_first = double(committed_date: Time.current - 1.second) + double_last = double(committed_date: Time.current) allow(tag_a).to receive(:dereferenced_target).and_return(double_last) allow(tag_b).to receive(:dereferenced_target).and_return(double_first) @@ -125,8 +125,8 @@ describe Repository do rugged_repo(repository).tags.create(annotated_tag_name, 'a48e4fc218069f68ef2e769dd8dfea3991362175', options) - double_first = double(committed_date: Time.now - 1.second) - double_last = double(committed_date: Time.now) + double_first = double(committed_date: Time.current - 1.second) + double_last = double(committed_date: Time.current) allow(tag_a).to receive(:dereferenced_target).and_return(double_last) allow(tag_b).to receive(:dereferenced_target).and_return(double_first) @@ -227,7 +227,7 @@ describe Repository do tree_builder = Rugged::Tree::Builder.new(rugged) tree_builder.insert({ oid: blob_id, name: "hello\x80world", filemode: 0100644 }) tree_id = tree_builder.write - user = { email: "jcai@gitlab.com", time: Time.now, name: "John Cai" } + user = { email: "jcai@gitlab.com", time: Time.current.to_time, name: "John Cai" } Rugged::Commit.create(rugged, message: 'some commit message', parents: [rugged.head.target.oid], tree: tree_id, committer: user, author: user) end @@ -974,7 +974,7 @@ describe Repository do end it 'returns nil' do - expect(Rails.logger).to receive(:info).with("Remove remote job failed to create for #{project.id} with remote name joe.") + expect(Gitlab::AppLogger).to receive(:info).with("Remove remote job failed to create for #{project.id} with remote name joe.") expect(repository.async_remove_remote('joe')).to be_nil end @@ -1446,17 +1446,13 @@ describe Repository do let(:empty_repository) { create(:project_empty_repo).repository } it 'returns empty array for an empty repository' do - # rubocop:disable Style/WordArray - expect(empty_repository.blobs_at(['master', 'foobar'])).to eq([]) - # rubocop:enable Style/WordArray + expect(empty_repository.blobs_at(%w[master foobar])).to eq([]) end it 'returns blob array for a non-empty repository' do repository.create_file(User.last, 'foobar', 'CONTENT', message: 'message', branch_name: 'master') - # rubocop:disable Style/WordArray - blobs = repository.blobs_at([['master', 'foobar']]) - # rubocop:enable Style/WordArray + blobs = repository.blobs_at([%w[master foobar]]) expect(blobs.first.name).to eq('foobar') expect(blobs.size).to eq(1) diff --git a/spec/models/resource_label_event_spec.rb b/spec/models/resource_label_event_spec.rb index a1a2150f461..6a235d3aa17 100644 --- a/spec/models/resource_label_event_spec.rb +++ b/spec/models/resource_label_event_spec.rb @@ -96,4 +96,48 @@ RSpec.describe ResourceLabelEvent, type: :model do expect(subject.outdated_markdown?).to be false end end + + describe '.visible_to_user?' do + let_it_be(:user) { create(:user) } + let_it_be(:issue_project) { create(:project) } + let_it_be(:issue) { create(:issue, project: issue_project) } + + subject { described_class.visible_to_user?(user, issue.resource_label_events.inc_relations) } + + it 'returns events with labels accessible by user' do + label = create(:label, project: issue_project) + event = create_event(label) + issue_project.add_guest(user) + + expect(subject).to eq [event] + end + + it 'filters events with public project labels if issues and MRs are private' do + project = create(:project, :public, :issues_private, :merge_requests_private) + label = create(:label, project: project) + create_event(label) + + expect(subject).to be_empty + end + + it 'filters events with project labels not accessible by user' do + project = create(:project, :private) + label = create(:label, project: project) + create_event(label) + + expect(subject).to be_empty + end + + it 'filters events with group labels not accessible by user' do + group = create(:group, :private) + label = create(:group_label, group: group) + create_event(label) + + expect(subject).to be_empty + end + + def create_event(label) + create(:resource_label_event, issue: issue, label: label) + end + end end diff --git a/spec/models/resource_milestone_event_spec.rb b/spec/models/resource_milestone_event_spec.rb index 3f8d8b4c1df..66686ec77d0 100644 --- a/spec/models/resource_milestone_event_spec.rb +++ b/spec/models/resource_milestone_event_spec.rb @@ -95,4 +95,34 @@ describe ResourceMilestoneEvent, type: :model do end end end + + describe '#milestone_parent' do + let_it_be(:project) { create(:project) } + let_it_be(:group) { create(:group) } + + let(:milestone) { create(:milestone, project: project) } + let(:event) { create(:resource_milestone_event, milestone: milestone) } + + context 'when milestone parent is project' do + it 'returns the expected parent' do + expect(event.milestone_parent).to eq(project) + end + end + + context 'when milestone parent is group' do + let(:milestone) { create(:milestone, group: group) } + + it 'returns the expected parent' do + expect(event.milestone_parent).to eq(group) + end + end + + context 'when milestone is nil' do + let(:event) { create(:resource_milestone_event, milestone: nil) } + + it 'returns nil' do + expect(event.milestone_parent).to be_nil + end + end + end end diff --git a/spec/models/review_spec.rb b/spec/models/review_spec.rb new file mode 100644 index 00000000000..9dd8b90feee --- /dev/null +++ b/spec/models/review_spec.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Review do + describe 'associations' do + it { is_expected.to belong_to(:author).class_name('User').with_foreign_key(:author_id).inverse_of(:reviews) } + it { is_expected.to belong_to(:merge_request).inverse_of(:reviews).touch(false) } + it { is_expected.to belong_to(:project).inverse_of(:reviews) } + + it { is_expected.to have_many(:notes).order(:id).inverse_of(:review) } + end + + describe 'modules' do + it { is_expected.to include_module(Participable) } + it { is_expected.to include_module(Mentionable) } + end + + describe '#all_references' do + it 'returns an extractor with the correct referenced users' do + user1 = create(:user, username: "foo") + user2 = create(:user, username: "bar") + review = create(:review) + project = review.project + author = review.author + + create(:note, review: review, project: project, author: author, note: "cc @foo @non_existent") + create(:note, review: review, project: project, author: author, note: "cc @bar") + + expect(review.all_references(author).users).to match_array([user1, user2]) + end + end + + describe '#participants' do + it 'includes the review author' do + project = create(:project, :public) + merge_request = create(:merge_request, source_project: project) + review = create(:review, project: project, merge_request: merge_request) + create(:note, review: review, noteable: merge_request, project: project, author: review.author) + + expect(review.participants).to include(review.author) + end + end +end diff --git a/spec/models/service_spec.rb b/spec/models/service_spec.rb index 106f8def42d..8698a6cf3d3 100644 --- a/spec/models/service_spec.rb +++ b/spec/models/service_spec.rb @@ -114,6 +114,20 @@ describe Service do expect(described_class.confidential_note_hooks.count).to eq 0 end end + + describe '.alert_hooks' do + it 'includes services where alert_events is true' do + create(:service, active: true, alert_events: true) + + expect(described_class.alert_hooks.count).to eq 1 + end + + it 'excludes services where alert_events is false' do + create(:service, active: true, alert_events: false) + + expect(described_class.alert_hooks.count).to eq 0 + end + end end describe "Test Button" do @@ -264,20 +278,32 @@ describe Service do end end - describe '.build_from_template' do - context 'when template is invalid' do - it 'sets service template to inactive when template is invalid' do - template = build(:prometheus_service, template: true, active: true, properties: {}) - template.save(validate: false) + describe '.build_from_integration' do + context 'when integration is invalid' do + let(:integration) do + build(:prometheus_service, :template, active: true, properties: {}) + .tap { |integration| integration.save(validate: false) } + end - service = described_class.build_from_template(project.id, template) + it 'sets service to inactive' do + service = described_class.build_from_integration(project.id, integration) expect(service).to be_valid expect(service.active).to be false end end - describe 'build issue tracker from a template' do + context 'when integration is an instance' do + let(:integration) { create(:jira_service, :instance) } + + it 'sets inherit_from_id from integration' do + service = described_class.build_from_integration(project.id, integration) + + expect(service.inherit_from_id).to eq(integration.id) + end + end + + describe 'build issue tracker from an integration' do let(:title) { 'custom title' } let(:description) { 'custom description' } let(:url) { 'http://jira.example.com' } @@ -291,9 +317,9 @@ describe Service do } end - shared_examples 'service creation from a template' do + shared_examples 'service creation from an integration' do it 'creates a correct service' do - service = described_class.build_from_template(project.id, template) + service = described_class.build_from_integration(project.id, integration) expect(service).to be_active expect(service.title).to eq(title) @@ -302,36 +328,38 @@ describe Service do expect(service.api_url).to eq(api_url) expect(service.username).to eq(username) expect(service.password).to eq(password) + expect(service.template).to eq(false) + expect(service.instance).to eq(false) end end # this will be removed as part of https://gitlab.com/gitlab-org/gitlab/issues/29404 context 'when data are stored in properties' do let(:properties) { data_params.merge(title: title, description: description) } - let!(:template) do + let!(:integration) do create(:jira_service, :without_properties_callback, template: true, properties: properties.merge(additional: 'something')) end - it_behaves_like 'service creation from a template' + it_behaves_like 'service creation from an integration' end context 'when data are stored in separated fields' do - let(:template) do + let(:integration) do create(:jira_service, :template, data_params.merge(properties: {}, title: title, description: description)) end - it_behaves_like 'service creation from a template' + it_behaves_like 'service creation from an integration' end context 'when data are stored in both properties and separated fields' do let(:properties) { data_params.merge(title: title, description: description) } - let(:template) do + let(:integration) do create(:jira_service, :without_properties_callback, active: true, template: true, properties: properties).tap do |service| create(:jira_tracker_data, data_params.merge(service: service)) end end - it_behaves_like 'service creation from a template' + it_behaves_like 'service creation from an integration' end end end diff --git a/spec/models/snippet_input_action_collection_spec.rb b/spec/models/snippet_input_action_collection_spec.rb new file mode 100644 index 00000000000..ef18ab5a810 --- /dev/null +++ b/spec/models/snippet_input_action_collection_spec.rb @@ -0,0 +1,47 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe SnippetInputActionCollection do + let(:action_name) { 'create' } + let(:action) { { action: action_name, file_path: 'foo', content: 'bar', previous_path: 'foobar' } } + let(:data) { [action, action] } + + it { is_expected.to delegate_method(:empty?).to(:actions) } + it { is_expected.to delegate_method(:any?).to(:actions) } + it { is_expected.to delegate_method(:[]).to(:actions) } + + describe '#to_commit_actions' do + subject { described_class.new(data).to_commit_actions} + + it 'translates all actions to commit actions' do + transformed_action = action.merge(action: action_name.to_sym) + + expect(subject).to eq [transformed_action, transformed_action] + end + end + + describe '#valid?' do + subject { described_class.new(data).valid?} + + it 'returns true' do + expect(subject).to be true + end + + context 'when any of the actions is invalid' do + let(:data) { [action, { action: 'foo' }, action]} + + it 'returns false' do + expect(subject).to be false + end + end + end + + context 'when allowed_actions param is passed' do + it 'builds SnippetInputAction with that param' do + expect(SnippetInputAction).to receive(:new).with(hash_including(allowed_actions: :create)) + + described_class.new([action], allowed_actions: :create) + end + end +end diff --git a/spec/models/snippet_input_action_spec.rb b/spec/models/snippet_input_action_spec.rb new file mode 100644 index 00000000000..5e379a48171 --- /dev/null +++ b/spec/models/snippet_input_action_spec.rb @@ -0,0 +1,89 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe SnippetInputAction do + describe 'validations' do + using RSpec::Parameterized::TableSyntax + + where(:action, :file_path, :content, :previous_path, :allowed_actions, :is_valid, :invalid_field) do + :create | 'foobar' | 'foobar' | 'foobar' | nil | true | nil + :move | 'foobar' | 'foobar' | 'foobar' | nil | true | nil + :delete | 'foobar' | 'foobar' | 'foobar' | nil | true | nil + :update | 'foobar' | 'foobar' | 'foobar' | nil | true | nil + :foo | 'foobar' | 'foobar' | 'foobar' | nil | false | :action + 'create' | 'foobar' | 'foobar' | 'foobar' | nil | true | nil + 'move' | 'foobar' | 'foobar' | 'foobar' | nil | true | nil + 'delete' | 'foobar' | 'foobar' | 'foobar' | nil | true | nil + 'update' | 'foobar' | 'foobar' | 'foobar' | nil | true | nil + 'foo' | 'foobar' | 'foobar' | 'foobar' | nil | false | :action + nil | 'foobar' | 'foobar' | 'foobar' | nil | false | :action + '' | 'foobar' | 'foobar' | 'foobar' | nil | false | :action + :move | 'foobar' | 'foobar' | nil | nil | false | :previous_path + :move | 'foobar' | 'foobar' | '' | nil | false | :previous_path + :create | 'foobar' | nil | 'foobar' | nil | false | :content + :create | 'foobar' | '' | 'foobar' | nil | false | :content + :create | nil | 'foobar' | 'foobar' | nil | false | :file_path + :create | '' | 'foobar' | 'foobar' | nil | false | :file_path + :update | 'foobar' | nil | 'foobar' | nil | false | :content + :update | 'foobar' | '' | 'foobar' | nil | false | :content + :update | 'other' | 'foobar' | 'foobar' | nil | false | :file_path + :update | 'foobar' | 'foobar' | nil | nil | true | nil + :update | 'foobar' | 'foobar' | '' | nil | true | nil + :update | 'foobar' | 'foobar' | '' | :update | true | nil + :update | 'foobar' | 'foobar' | '' | 'update' | true | nil + :update | 'foobar' | 'foobar' | '' | [:update] | true | nil + :update | 'foobar' | 'foobar' | '' | [:update, :create] | true | nil + :update | 'foobar' | 'foobar' | '' | :create | false | :action + :update | 'foobar' | 'foobar' | '' | 'create' | false | :action + :update | 'foobar' | 'foobar' | '' | [:create] | false | :action + :foo | 'foobar' | 'foobar' | '' | :foo | false | :action + end + + with_them do + subject { described_class.new(action: action, file_path: file_path, content: content, previous_path: previous_path, allowed_actions: allowed_actions) } + + specify do + expect(subject.valid?).to be is_valid + + unless is_valid + expect(subject.errors).to include(invalid_field) + end + end + end + end + + describe '#to_commit_action' do + let(:action) { 'create' } + let(:file_path) { 'foo' } + let(:content) { 'bar' } + let(:previous_path) { 'previous_path' } + let(:options) { { action: action, file_path: file_path, content: content, previous_path: previous_path } } + let(:expected_options) { options.merge(action: action.to_sym) } + + subject { described_class.new(options).to_commit_action } + + it 'transforms attributes to commit action' do + expect(subject).to eq(expected_options) + end + + context 'action is update' do + let(:action) { 'update' } + + context 'when previous_path is present' do + it 'returns the existing previous_path' do + expect(subject).to eq(expected_options) + end + end + + context 'when previous_path is not present' do + let(:previous_path) { nil } + let(:expected_options) { options.merge(action: action.to_sym, previous_path: file_path) } + + it 'assigns the file_path to the previous_path' do + expect(subject).to eq(expected_options) + end + end + end + end +end diff --git a/spec/models/todo_spec.rb b/spec/models/todo_spec.rb index e125f58399e..bda89fc01f3 100644 --- a/spec/models/todo_spec.rb +++ b/spec/models/todo_spec.rb @@ -100,6 +100,20 @@ describe Todo do end end + describe '#for_alert?' do + it 'returns true when target is a Alert' do + subject.target_type = 'AlertManagement::Alert' + + expect(subject.for_alert?).to eq(true) + end + + it 'returns false when target is not a Alert' do + subject.target_type = 'Issue' + + expect(subject.for_alert?).to eq(false) + end + end + describe '#target' do context 'for commits' do let(:project) { create(:project, :repository) } @@ -393,10 +407,10 @@ describe Todo do end end - describe '.update_state' do + describe '.batch_update' do it 'updates the state of todos' do todo = create(:todo, :pending) - ids = described_class.update_state(:done) + ids = described_class.batch_update(state: :done) todo.reload @@ -407,16 +421,16 @@ describe Todo do it 'does not update todos that already have the given state' do create(:todo, :pending) - expect(described_class.update_state(:pending)).to be_empty + expect(described_class.batch_update(state: :pending)).to be_empty end it 'updates updated_at' do create(:todo, :pending) Timecop.freeze(1.day.from_now) do - expected_update_date = Time.now.utc + expected_update_date = Time.current.utc - ids = described_class.update_state(:done) + ids = described_class.batch_update(state: :done) expect(Todo.where(id: ids).map(&:updated_at)).to all(be_like_time(expected_update_date)) end diff --git a/spec/models/user_interacted_project_spec.rb b/spec/models/user_interacted_project_spec.rb index e2c485343ae..83c66bf1969 100644 --- a/spec/models/user_interacted_project_spec.rb +++ b/spec/models/user_interacted_project_spec.rb @@ -8,7 +8,7 @@ describe UserInteractedProject do let(:event) { build(:event) } - Event::ACTIONS.each do |action| + Event.actions.each_key do |action| context "for all actions (event types)" do let(:event) { build(:event, action: action) } @@ -44,21 +44,6 @@ describe UserInteractedProject do end end - describe '.available?' do - before do - described_class.instance_variable_set('@available_flag', nil) - end - - it 'checks schema version and properly caches positive result' do - expect(ActiveRecord::Migrator).to receive(:current_version).and_return(described_class::REQUIRED_SCHEMA_VERSION - 1 - rand(1000)) - expect(described_class.available?).to be_falsey - expect(ActiveRecord::Migrator).to receive(:current_version).and_return(described_class::REQUIRED_SCHEMA_VERSION + rand(1000)) - expect(described_class.available?).to be_truthy - expect(ActiveRecord::Migrator).not_to receive(:current_version) - expect(described_class.available?).to be_truthy # cached response - end - end - it { is_expected.to validate_presence_of(:project_id) } it { is_expected.to validate_presence_of(:user_id) } end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index e82e8c1a21d..dd4b174a38f 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -23,8 +23,41 @@ describe User do describe 'delegations' do it { is_expected.to delegate_method(:path).to(:namespace).with_prefix } + it { is_expected.to delegate_method(:notes_filter_for).to(:user_preference) } + it { is_expected.to delegate_method(:set_notes_filter).to(:user_preference) } + + it { is_expected.to delegate_method(:first_day_of_week).to(:user_preference) } + it { is_expected.to delegate_method(:first_day_of_week=).to(:user_preference).with_arguments(:args) } + + it { is_expected.to delegate_method(:timezone).to(:user_preference) } + it { is_expected.to delegate_method(:timezone=).to(:user_preference).with_arguments(:args) } + + it { is_expected.to delegate_method(:time_display_relative).to(:user_preference) } + it { is_expected.to delegate_method(:time_display_relative=).to(:user_preference).with_arguments(:args) } + + it { is_expected.to delegate_method(:time_format_in_24h).to(:user_preference) } + it { is_expected.to delegate_method(:time_format_in_24h=).to(:user_preference).with_arguments(:args) } + + it { is_expected.to delegate_method(:show_whitespace_in_diffs).to(:user_preference) } + it { is_expected.to delegate_method(:show_whitespace_in_diffs=).to(:user_preference).with_arguments(:args) } + it { is_expected.to delegate_method(:tab_width).to(:user_preference) } - it { is_expected.to delegate_method(:tab_width=).to(:user_preference).with_arguments(5) } + it { is_expected.to delegate_method(:tab_width=).to(:user_preference).with_arguments(:args) } + + it { is_expected.to delegate_method(:sourcegraph_enabled).to(:user_preference) } + it { is_expected.to delegate_method(:sourcegraph_enabled=).to(:user_preference).with_arguments(:args) } + + it { is_expected.to delegate_method(:setup_for_company).to(:user_preference) } + it { is_expected.to delegate_method(:setup_for_company=).to(:user_preference).with_arguments(:args) } + + it { is_expected.to delegate_method(:render_whitespace_in_code).to(:user_preference) } + it { is_expected.to delegate_method(:render_whitespace_in_code=).to(:user_preference).with_arguments(:args) } + + it { is_expected.to delegate_method(:experience_level).to(:user_preference) } + it { is_expected.to delegate_method(:experience_level=).to(:user_preference).with_arguments(:args) } + + it { is_expected.to delegate_method(:job_title).to(:user_detail).allow_nil } + it { is_expected.to delegate_method(:job_title=).to(:user_detail).with_arguments(:args).allow_nil } end describe 'associations' do @@ -56,6 +89,7 @@ describe User do it { is_expected.to have_many(:custom_attributes).class_name('UserCustomAttribute') } it { is_expected.to have_many(:releases).dependent(:nullify) } it { is_expected.to have_many(:metrics_users_starred_dashboards).inverse_of(:user) } + it { is_expected.to have_many(:reviews).inverse_of(:author) } describe "#bio" do it 'syncs bio with `user_details.bio` on create' do @@ -816,6 +850,7 @@ describe User do let_it_be(:expired_token) { create(:personal_access_token, user: user1, expires_at: 2.days.ago) } let_it_be(:revoked_token) { create(:personal_access_token, user: user1, revoked: true) } + let_it_be(:impersonation_token) { create(:personal_access_token, :impersonation, user: user1, expires_at: 2.days.from_now) } let_it_be(:valid_token_and_notified) { create(:personal_access_token, user: user2, expires_at: 2.days.from_now, expire_notification_delivered: true) } let_it_be(:valid_token1) { create(:personal_access_token, user: user2, expires_at: 2.days.from_now) } let_it_be(:valid_token2) { create(:personal_access_token, user: user2, expires_at: 2.days.from_now) } @@ -967,7 +1002,7 @@ describe User do end it 'has only one email association' do - expect(user.emails.size).to be(1) + expect(user.emails.size).to eq(1) end end end @@ -1381,7 +1416,7 @@ describe User do end it 'is true when sent less than one minute ago' do - user = build_stubbed(:user, reset_password_sent_at: Time.now) + user = build_stubbed(:user, reset_password_sent_at: Time.current) expect(user.recently_sent_password_reset?).to eq true end @@ -2157,7 +2192,7 @@ describe User do describe '#all_emails' do let(:user) { create(:user) } - let!(:email_confirmed) { create :email, user: user, confirmed_at: Time.now } + let!(:email_confirmed) { create :email, user: user, confirmed_at: Time.current } let!(:email_unconfirmed) { create :email, user: user } context 'when `include_private_email` is true' do @@ -2186,7 +2221,7 @@ describe User do let(:user) { create(:user) } it 'returns only confirmed emails' do - email_confirmed = create :email, user: user, confirmed_at: Time.now + email_confirmed = create :email, user: user, confirmed_at: Time.current create :email, user: user expect(user.verified_emails).to contain_exactly( @@ -2226,7 +2261,7 @@ describe User do let(:user) { create(:user) } it 'returns true when the email is verified/confirmed' do - email_confirmed = create :email, user: user, confirmed_at: Time.now + email_confirmed = create :email, user: user, confirmed_at: Time.current create :email, user: user user.reload @@ -2350,26 +2385,6 @@ describe User do end end - describe '#ultraauth_user?' do - it 'is true if provider is ultraauth' do - user = create(:omniauth_user, provider: 'ultraauth') - - expect(user.ultraauth_user?).to be_truthy - end - - it 'is false with othe provider' do - user = create(:omniauth_user, provider: 'not-ultraauth') - - expect(user.ultraauth_user?).to be_falsey - end - - it 'is false if no extern_uid is provided' do - user = create(:omniauth_user, extern_uid: nil) - - expect(user.ldap_user?).to be_falsey - end - end - describe '#full_website_url' do let(:user) { create(:user) } @@ -2863,10 +2878,10 @@ describe User do it "includes projects shared with user's group" do user = create(:user) project = create(:project, :private) - group = create(:group) - - group.add_reporter(user) - project.project_group_links.create(group: group) + group = create(:group) do |group| + group.add_reporter(user) + end + create(:project_group_link, group: group, project: project) expect(user.authorized_projects).to include(project) end @@ -3645,12 +3660,6 @@ describe User do expect(user.allow_password_authentication_for_web?).to be_falsey end - - it 'returns false for ultraauth user' do - user = create(:omniauth_user, provider: 'ultraauth') - - expect(user.allow_password_authentication_for_web?).to be_falsey - end end describe '#allow_password_authentication_for_git?' do @@ -3673,12 +3682,6 @@ describe User do expect(user.allow_password_authentication_for_git?).to be_falsey end - - it 'returns false for ultraauth user' do - user = create(:omniauth_user, provider: 'ultraauth') - - expect(user.allow_password_authentication_for_git?).to be_falsey - end end describe '#assigned_open_merge_requests_count' do diff --git a/spec/models/web_ide_terminal_spec.rb b/spec/models/web_ide_terminal_spec.rb new file mode 100644 index 00000000000..4103a26c75a --- /dev/null +++ b/spec/models/web_ide_terminal_spec.rb @@ -0,0 +1,82 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe WebIdeTerminal do + let(:build) { create(:ci_build) } + + subject { described_class.new(build) } + + it 'returns the show_path of the build' do + expect(subject.show_path).to end_with("/ide_terminals/#{build.id}") + end + + it 'returns the retry_path of the build' do + expect(subject.retry_path).to end_with("/ide_terminals/#{build.id}/retry") + end + + it 'returns the cancel_path of the build' do + expect(subject.cancel_path).to end_with("/ide_terminals/#{build.id}/cancel") + end + + it 'returns the terminal_path of the build' do + expect(subject.terminal_path).to end_with("/jobs/#{build.id}/terminal.ws") + end + + it 'returns the proxy_websocket_path of the build' do + expect(subject.proxy_websocket_path).to end_with("/jobs/#{build.id}/proxy.ws") + end + + describe 'services' do + let(:services_with_aliases) do + { + services: [{ name: 'postgres', alias: 'postgres' }, + { name: 'docker:stable-dind', alias: 'docker' }] + } + end + + before do + allow(build).to receive(:options).and_return(config) + end + + context 'when image does not have an alias' do + let(:config) do + { image: 'ruby:2.7' }.merge(services_with_aliases) + end + + it 'returns services aliases' do + expect(subject.services).to eq %w(postgres docker) + end + end + + context 'when both image and services have aliases' do + let(:config) do + { image: { name: 'ruby:2.7', alias: 'ruby' } }.merge(services_with_aliases) + end + + it 'returns all aliases' do + expect(subject.services).to eq %w(postgres docker ruby) + end + end + + context 'when image and services does not have any alias' do + let(:config) do + { image: 'ruby:2.7', services: ['postgres'] } + end + + it 'returns an empty array' do + expect(subject.services).to be_empty + end + end + + context 'when no image nor services' do + let(:config) do + { script: %w(echo) } + end + + it 'returns an empty array' do + expect(subject.services).to be_empty + end + end + end +end diff --git a/spec/models/wiki_directory_spec.rb b/spec/models/wiki_directory_spec.rb index 5fbcccf897e..4cac90786eb 100644 --- a/spec/models/wiki_directory_spec.rb +++ b/spec/models/wiki_directory_spec.rb @@ -40,7 +40,7 @@ RSpec.describe WikiDirectory do it 'returns the relative path to the partial to be used' do directory = build(:wiki_directory) - expect(directory.to_partial_path).to eq('projects/wikis/wiki_directory') + expect(directory.to_partial_path).to eq('../shared/wikis/wiki_directory') end end end diff --git a/spec/models/wiki_page_spec.rb b/spec/models/wiki_page_spec.rb index 201dc85daf8..8f2da8ff9a1 100644 --- a/spec/models/wiki_page_spec.rb +++ b/spec/models/wiki_page_spec.rb @@ -651,6 +651,7 @@ describe WikiPage do let(:untitled_page) { described_class.new(wiki) } let(:directory_page) { create(:wiki_page, title: 'parent directory/child page') } + let(:page_with_special_characters) { create(:wiki_page, title: 'test+page') } where(:page, :title, :changed) do :untitled_page | nil | false @@ -658,6 +659,8 @@ describe WikiPage do :new_page | nil | true :new_page | 'test page' | true + :new_page | 'test-page' | true + :new_page | 'test+page' | true :new_page | 'new title' | true :existing_page | nil | false @@ -665,6 +668,7 @@ describe WikiPage do :existing_page | 'test-page' | false :existing_page | '/test page' | false :existing_page | '/test-page' | false + :existing_page | 'test+page' | true :existing_page | ' test page ' | true :existing_page | 'new title' | true :existing_page | 'new-title' | true @@ -681,6 +685,11 @@ describe WikiPage do :directory_page | 'parent-directory / child-page' | true :directory_page | 'other directory/child page' | true :directory_page | 'other-directory/child page' | true + + :page_with_special_characters | nil | false + :page_with_special_characters | 'test+page' | false + :page_with_special_characters | 'test-page' | true + :page_with_special_characters | 'test page' | true end with_them do @@ -772,7 +781,7 @@ describe WikiPage do describe '#to_partial_path' do it 'returns the relative path to the partial to be used' do - expect(subject.to_partial_path).to eq('projects/wikis/wiki_page') + expect(subject.to_partial_path).to eq('../shared/wikis/wiki_page') end end |