diff options
Diffstat (limited to 'spec')
29 files changed, 473 insertions, 163 deletions
diff --git a/spec/controllers/graphql_controller_spec.rb b/spec/controllers/graphql_controller_spec.rb index c19a752b07b..9937bdf4061 100644 --- a/spec/controllers/graphql_controller_spec.rb +++ b/spec/controllers/graphql_controller_spec.rb @@ -7,6 +7,27 @@ describe GraphqlController do stub_feature_flags(graphql: true) end + describe 'ArgumentError' do + let(:user) { create(:user) } + let(:message) { 'green ideas sleep furiously' } + + before do + sign_in(user) + end + + it 'handles argument errors' do + allow(subject).to receive(:execute) do + raise Gitlab::Graphql::Errors::ArgumentError, message + end + + post :execute + + expect(json_response).to include( + 'errors' => include(a_hash_including('message' => message)) + ) + end + end + describe 'POST #execute' do context 'when user is logged in' do let(:user) { create(:user) } diff --git a/spec/controllers/projects/error_tracking_controller_spec.rb b/spec/controllers/projects/error_tracking_controller_spec.rb index 844c61f1ace..d11ef24ef96 100644 --- a/spec/controllers/projects/error_tracking_controller_spec.rb +++ b/spec/controllers/projects/error_tracking_controller_spec.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -require 'rails_helper' +require 'spec_helper' describe Projects::ErrorTrackingController do set(:project) { create(:project) } diff --git a/spec/fast_spec_helper.rb b/spec/fast_spec_helper.rb index 91ef7653822..f5a487b4d57 100644 --- a/spec/fast_spec_helper.rb +++ b/spec/fast_spec_helper.rb @@ -4,6 +4,7 @@ ENV['GITLAB_ENV'] = 'test' ENV['IN_MEMORY_APPLICATION_SETTINGS'] = 'true' require 'active_support/dependencies' +require_relative '../config/initializers/0_inject_enterprise_edition_module' require_relative '../config/settings' require_relative 'support/rspec' require 'active_support/all' diff --git a/spec/features/admin/admin_settings_spec.rb b/spec/features/admin/admin_settings_spec.rb index a074bbcff0a..c77605f3869 100644 --- a/spec/features/admin/admin_settings_spec.rb +++ b/spec/features/admin/admin_settings_spec.rb @@ -356,16 +356,18 @@ describe 'Admin updates settings' do end it 'Change Help page' do + new_support_url = 'http://example.com/help' + page.within('.as-help-page') do fill_in 'Help page text', with: 'Example text' check 'Hide marketing-related entries from help' - fill_in 'Support page URL', with: 'http://example.com/help' + fill_in 'Support page URL', with: new_support_url click_button 'Save changes' end expect(current_settings.help_page_text).to eq "Example text" expect(current_settings.help_page_hide_commercial_content).to be_truthy - expect(current_settings.help_page_support_url).to eq "http://example.com/help" + expect(current_settings.help_page_support_url).to eq new_support_url expect(page).to have_content "Application settings saved successfully" end @@ -415,6 +417,34 @@ describe 'Admin updates settings' do end end + context 'Nav bar' do + it 'Shows default help links in nav' do + default_support_url = 'https://about.gitlab.com/getting-help/' + + visit root_dashboard_path + + find('.header-help-dropdown-toggle').click + + page.within '.header-help' do + expect(page).to have_link(text: 'Help', href: help_path) + expect(page).to have_link(text: 'Support', href: default_support_url) + end + end + + it 'Shows custom support url in nav when set' do + new_support_url = 'http://example.com/help' + stub_application_setting(help_page_support_url: new_support_url) + + visit root_dashboard_path + + find('.header-help-dropdown-toggle').click + + page.within '.header-help' do + expect(page).to have_link(text: 'Support', href: new_support_url) + end + end + end + def check_all_events page.check('Active') page.check('Push') diff --git a/spec/features/display_system_header_and_footer_bar_spec.rb b/spec/features/display_system_header_and_footer_bar_spec.rb index af9d9a5834f..e32da1a02bc 100644 --- a/spec/features/display_system_header_and_footer_bar_spec.rb +++ b/spec/features/display_system_header_and_footer_bar_spec.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -require 'rails_helper' +require 'spec_helper' describe 'Display system header and footer bar' do let(:header_message) { "Foo" } diff --git a/spec/features/projects/clusters/applications_spec.rb b/spec/features/projects/clusters/applications_spec.rb index d0182270d82..de97c8a8bc0 100644 --- a/spec/features/projects/clusters/applications_spec.rb +++ b/spec/features/projects/clusters/applications_spec.rb @@ -126,7 +126,7 @@ describe 'Clusters Applications', :js do it 'shows status transition' do page.within('.js-cluster-application-row-knative') do expect(domainname_form_value).to eq('domain.example.org') - expect(page).to have_css('.js-cluster-application-install-button', exact_text: 'Installed') + expect(page).to have_css('.js-cluster-application-uninstall-button', exact_text: 'Uninstall') end expect(page).to have_content('Knative was successfully installed on your Kubernetes cluster') diff --git a/spec/finders/issues_finder_spec.rb b/spec/finders/issues_finder_spec.rb index 879ff01f294..bcde730c40b 100644 --- a/spec/finders/issues_finder_spec.rb +++ b/spec/finders/issues_finder_spec.rb @@ -113,13 +113,13 @@ describe IssuesFinder do let(:params) { { milestone_title: 'Any' } } it 'returns issues with any assigned milestone' do - expect(issues).to contain_exactly(issue1) + expect(issues).to contain_exactly(issue1, issue2, issue3, issue4) end it 'returns issues with any assigned milestone (deprecated)' do params[:milestone_title] = Milestone::Any.title - expect(issues).to contain_exactly(issue1) + expect(issues).to contain_exactly(issue1, issue2, issue3, issue4) end end diff --git a/spec/javascripts/boards/components/boards_selector_spec.js b/spec/javascripts/boards/components/boards_selector_spec.js index 504bc51778c..473cc0612ea 100644 --- a/spec/javascripts/boards/components/boards_selector_spec.js +++ b/spec/javascripts/boards/components/boards_selector_spec.js @@ -76,7 +76,8 @@ describe('BoardsSelector', () => { document.querySelector('.js-boards-selector'), ); - vm.$el.querySelector('.js-dropdown-toggle').click(); + // Emits gl-dropdown show event to simulate the dropdown is opened at initialization time + vm.$children[0].$emit('show'); Promise.all([allBoardsResponse, recentBoardsResponse]) .then(() => vm.$nextTick()) diff --git a/spec/javascripts/pdf/page_spec.js b/spec/javascripts/pdf/page_spec.js index 6dea570266b..efeb65acf87 100644 --- a/spec/javascripts/pdf/page_spec.js +++ b/spec/javascripts/pdf/page_spec.js @@ -17,7 +17,7 @@ describe('Page component', () => { pdfjsLib.GlobalWorkerOptions.workerSrc = workerSrc; pdfjsLib .getDocument(testPDF) - .then(pdf => pdf.getPage(1)) + .promise.then(pdf => pdf.getPage(1)) .then(page => { testPage = page; }) @@ -31,7 +31,8 @@ describe('Page component', () => { it('renders the page when mounting', done => { const promise = Promise.resolve(); - spyOn(testPage, 'render').and.callFake(() => promise); + spyOn(testPage, 'render').and.returnValue({ promise }); + vm = mountComponent(Component, { page: testPage, number: 1, diff --git a/spec/lib/gitlab/ci/yaml_processor_spec.rb b/spec/lib/gitlab/ci/yaml_processor_spec.rb index fc9d4f97bda..d3676ee03bf 100644 --- a/spec/lib/gitlab/ci/yaml_processor_spec.rb +++ b/spec/lib/gitlab/ci/yaml_processor_spec.rb @@ -1085,6 +1085,31 @@ module Gitlab it { expect { subject }.to raise_error(Gitlab::Ci::YamlProcessor::ValidationError, 'test1 job: dependency deploy is not defined in prior stages') } end + + context 'when a job depends on another job that references a not-yet defined stage' do + let(:config) do + { + "stages" => [ + "version" + ], + "version" => { + "stage" => "version", + "dependencies" => ["release:components:versioning"], + "script" => ["./versioning/versioning"] + }, + ".release_go" => { + "stage" => "build", + "script" => ["cd versioning"] + }, + "release:components:versioning" => { + "stage" => "build", + "script" => ["cd versioning"] + } + } + end + + it { expect { subject }.to raise_error(Gitlab::Ci::YamlProcessor::ValidationError, /is not defined in prior stages/) } + end end describe "Hidden jobs" do diff --git a/spec/lib/gitlab/danger/teammate_spec.rb b/spec/lib/gitlab/danger/teammate_spec.rb index 6a6cf1429c8..171f2344e82 100644 --- a/spec/lib/gitlab/danger/teammate_spec.rb +++ b/spec/lib/gitlab/danger/teammate_spec.rb @@ -40,6 +40,14 @@ describe Gitlab::Danger::Teammate do it '#maintainer? returns false' do expect(subject.maintainer?(project, :test, labels)).to be_falsey end + + context 'when hyperlink is mangled in the role' do + let(:role) { '<a href="#">Test Automation Engineer</a>, Create' } + + it '#reviewer? returns true' do + expect(subject.reviewer?(project, :test, labels)).to be_truthy + end + end end context 'when role is Test Automation Engineer, Manage' do diff --git a/spec/lib/gitlab/gitaly_client_spec.rb b/spec/lib/gitlab/gitaly_client_spec.rb index e1d24ae8977..e9fb6c0125c 100644 --- a/spec/lib/gitlab/gitaly_client_spec.rb +++ b/spec/lib/gitlab/gitaly_client_spec.rb @@ -17,6 +17,16 @@ describe Gitlab::GitalyClient do }) end + describe '.filesystem_id_from_disk' do + it 'catches errors' do + [Errno::ENOENT, Errno::EACCES, JSON::ParserError].each do |error| + allow(File).to receive(:read).with(described_class.storage_metadata_file_path('default')).and_raise(error) + + expect(described_class.filesystem_id_from_disk('default')).to be_nil + end + end + end + describe '.stub_class' do it 'returns the gRPC health check stub' do expect(described_class.stub_class(:health_check)).to eq(::Grpc::Health::V1::Health::Stub) diff --git a/spec/lib/gitlab/sidekiq_logging/structured_logger_spec.rb b/spec/lib/gitlab/sidekiq_logging/structured_logger_spec.rb index 7bc4599e20f..98286eb432d 100644 --- a/spec/lib/gitlab/sidekiq_logging/structured_logger_spec.rb +++ b/spec/lib/gitlab/sidekiq_logging/structured_logger_spec.rb @@ -2,7 +2,10 @@ require 'spec_helper' describe Gitlab::SidekiqLogging::StructuredLogger do describe '#call' do - let(:timestamp) { Time.new('2018-01-01 12:00:00').utc } + let(:timestamp) { Time.iso8601('2018-01-01T12:00:00Z') } + let(:created_at) { timestamp } + let(:scheduling_latency_s) { 0.0 } + let(:job) do { "class" => "TestWorker", @@ -11,19 +14,21 @@ describe Gitlab::SidekiqLogging::StructuredLogger do "queue" => "cronjob:test_queue", "queue_namespace" => "cronjob", "jid" => "da883554ee4fe414012f5f42", - "created_at" => timestamp.to_f, - "enqueued_at" => timestamp.to_f, + "created_at" => created_at.to_f, + "enqueued_at" => created_at.to_f, "correlation_id" => 'cid' } end + let(:logger) { double } let(:start_payload) do job.merge( 'message' => 'TestWorker JID-da883554ee4fe414012f5f42: start', 'job_status' => 'start', 'pid' => Process.pid, - 'created_at' => timestamp.iso8601(3), - 'enqueued_at' => timestamp.iso8601(3) + 'created_at' => created_at.iso8601(3), + 'enqueued_at' => created_at.iso8601(3), + 'scheduling_latency_s' => scheduling_latency_s ) end let(:end_payload) do @@ -118,6 +123,35 @@ describe Gitlab::SidekiqLogging::StructuredLogger do subject.call(job, 'test_queue') { } end end + + it 'logs without created_at and enqueued_at fields' do + Timecop.freeze(timestamp) do + excluded_fields = %w(created_at enqueued_at args scheduling_latency_s) + + expect(logger).to receive(:info).with(start_payload.except(*excluded_fields)).ordered + expect(logger).to receive(:info).with(end_payload.except(*excluded_fields)).ordered + expect(subject).to receive(:log_job_start).and_call_original + expect(subject).to receive(:log_job_done).and_call_original + + subject.call(job.except("created_at", "enqueued_at"), 'test_queue') { } + end + end + end + + context 'with latency' do + let(:created_at) { Time.iso8601('2018-01-01T10:00:00Z') } + let(:scheduling_latency_s) { 7200.0 } + + it 'logs with scheduling latency' do + Timecop.freeze(timestamp) do + expect(logger).to receive(:info).with(start_payload.except('args')).ordered + expect(logger).to receive(:info).with(end_payload.except('args')).ordered + expect(subject).to receive(:log_job_start).and_call_original + expect(subject).to receive(:log_job_done).and_call_original + + subject.call(job, 'test_queue') { } + end + end end end end diff --git a/spec/lib/gitlab/sidekiq_middleware/memory_killer_spec.rb b/spec/lib/gitlab/sidekiq_middleware/memory_killer_spec.rb index b451844f06c..1de9a644610 100644 --- a/spec/lib/gitlab/sidekiq_middleware/memory_killer_spec.rb +++ b/spec/lib/gitlab/sidekiq_middleware/memory_killer_spec.rb @@ -4,7 +4,7 @@ describe Gitlab::SidekiqMiddleware::MemoryKiller do subject { described_class.new } let(:pid) { 999 } - let(:worker) { double(:worker, class: 'TestWorker') } + let(:worker) { double(:worker, class: ProjectCacheWorker) } let(:job) { { 'jid' => 123 } } let(:queue) { 'test_queue' } @@ -46,7 +46,10 @@ describe Gitlab::SidekiqMiddleware::MemoryKiller do expect(Process).to receive(:kill).with('SIGKILL', pid).ordered expect(Sidekiq.logger) - .to receive(:warn).with(class: 'TestWorker', message: anything, pid: pid, signal: anything).at_least(:once) + .to receive(:warn).with(class: 'ProjectCacheWorker', + message: anything, + pid: pid, + signal: anything).at_least(:once) run end diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb index bd87bbd8d68..db80b85360f 100644 --- a/spec/models/application_setting_spec.rb +++ b/spec/models/application_setting_spec.rb @@ -45,7 +45,7 @@ describe ApplicationSetting do it { is_expected.to allow_value(['xn--itlab-j1a.com']).for(:outbound_local_requests_whitelist) } it { is_expected.not_to allow_value(['<h1></h1>']).for(:outbound_local_requests_whitelist) } it { is_expected.to allow_value(['gitlab.com']).for(:outbound_local_requests_whitelist) } - it { is_expected.to allow_value(nil).for(:outbound_local_requests_whitelist) } + it { is_expected.not_to allow_value(nil).for(:outbound_local_requests_whitelist) } it { is_expected.to allow_value([]).for(:outbound_local_requests_whitelist) } context "when user accepted let's encrypt terms of service" do diff --git a/spec/models/clusters/applications/cert_manager_spec.rb b/spec/models/clusters/applications/cert_manager_spec.rb index a77e06ea829..e956a2355db 100644 --- a/spec/models/clusters/applications/cert_manager_spec.rb +++ b/spec/models/clusters/applications/cert_manager_spec.rb @@ -48,7 +48,7 @@ describe Clusters::Applications::CertManager do expect(subject.version).to eq('v0.5.2') expect(subject).to be_rbac expect(subject.files).to eq(cert_manager.files.merge(cluster_issuer_file)) - expect(subject.postinstall).to eq(['/usr/bin/kubectl create -f /data/helm/certmanager/config/cluster_issuer.yaml']) + expect(subject.postinstall).to eq(['kubectl create -f /data/helm/certmanager/config/cluster_issuer.yaml']) end context 'for a specific user' do diff --git a/spec/models/clusters/applications/knative_spec.rb b/spec/models/clusters/applications/knative_spec.rb index 7f4819cbb9a..342ed907854 100644 --- a/spec/models/clusters/applications/knative_spec.rb +++ b/spec/models/clusters/applications/knative_spec.rb @@ -39,7 +39,7 @@ describe Clusters::Applications::Knative do describe '#can_uninstall?' do subject { knative.can_uninstall? } - it { is_expected.to be_falsey } + it { is_expected.to be_truthy } end describe '#schedule_status_update with external_ip' do @@ -129,6 +129,46 @@ describe Clusters::Applications::Knative do it_behaves_like 'a command' end + describe '#uninstall_command' do + subject { knative.uninstall_command } + + it { is_expected.to be_an_instance_of(Gitlab::Kubernetes::Helm::DeleteCommand) } + + it "removes knative deployed services before uninstallation" do + 2.times do |i| + cluster_project = create(:cluster_project, cluster: knative.cluster) + + create(:cluster_kubernetes_namespace, + cluster: cluster_project.cluster, + cluster_project: cluster_project, + project: cluster_project.project, + namespace: "namespace_#{i}") + end + + remove_namespaced_services_script = [ + "kubectl delete ksvc --all -n #{knative.cluster.kubernetes_namespaces.first.namespace}", + "kubectl delete ksvc --all -n #{knative.cluster.kubernetes_namespaces.second.namespace}" + ] + + expect(subject.predelete).to match_array(remove_namespaced_services_script) + end + + it "initializes command with all necessary postdelete script" do + api_resources = YAML.safe_load(File.read(Rails.root.join(Clusters::Applications::Knative::API_RESOURCES_PATH))) + + remove_knative_istio_leftovers_script = [ + "kubectl delete --ignore-not-found ns knative-serving", + "kubectl delete --ignore-not-found ns knative-build" + ] + + full_delete_commands_size = api_resources.size + remove_knative_istio_leftovers_script.size + + expect(subject.postdelete).to include(*remove_knative_istio_leftovers_script) + expect(subject.postdelete.size).to eq(full_delete_commands_size) + expect(subject.postdelete[2]).to eq("kubectl delete --ignore-not-found crd #{api_resources[0]}") + end + end + describe '#files' do let(:application) { knative } let(:values) { subject[:'values.yaml'] } diff --git a/spec/models/clusters/cluster_spec.rb b/spec/models/clusters/cluster_spec.rb index d9d25a1f7aa..96212d0c864 100644 --- a/spec/models/clusters/cluster_spec.rb +++ b/spec/models/clusters/cluster_spec.rb @@ -121,26 +121,6 @@ describe Clusters::Cluster, :use_clean_rails_memory_store_caching do end end - describe '.missing_kubernetes_namespace' do - let!(:cluster) { create(:cluster, :provided_by_gcp, :project) } - let(:project) { cluster.project } - let(:kubernetes_namespaces) { project.kubernetes_namespaces } - - subject do - described_class.joins(:projects).where(projects: { id: project.id }).missing_kubernetes_namespace(kubernetes_namespaces) - end - - it { is_expected.to contain_exactly(cluster) } - - context 'kubernetes namespace exists' do - before do - create(:cluster_kubernetes_namespace, project: project, cluster: cluster) - end - - it { is_expected.to be_empty } - end - end - describe 'validations' do subject { cluster.valid? } @@ -423,31 +403,6 @@ describe Clusters::Cluster, :use_clean_rails_memory_store_caching do end end - describe '#all_projects' do - let(:project) { create(:project) } - let(:cluster) { create(:cluster, projects: [project]) } - - subject { cluster.all_projects } - - context 'project cluster' do - it 'returns project' do - is_expected.to eq([project]) - end - end - - context 'group cluster' do - let(:cluster) { create(:cluster, :group) } - let(:group) { cluster.group } - let(:project) { create(:project, group: group) } - let(:subgroup) { create(:group, parent: group) } - let(:subproject) { create(:project, group: subgroup) } - - it 'returns all projects for group' do - is_expected.to contain_exactly(project, subproject) - end - end - end - describe '#first_project' do subject { cluster.first_project } diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index 03003e3dd7d..bfd0e5f0558 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -913,6 +913,22 @@ describe Note do end end + describe '#special_role=' do + let(:role) { Note::SpecialRole::FIRST_TIME_CONTRIBUTOR } + + it 'assigns role' do + subject.special_role = role + + expect(subject.special_role).to eq(role) + end + + it 'does not assign unknown role' do + expect { subject.special_role = :bogus }.to raise_error(/Role is undefined/) + + expect(subject.special_role).to be_nil + end + end + describe '#parent' do it 'returns project for project notes' do project = create(:project) diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 24dfd0c5605..32fb0c0ff73 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -173,24 +173,6 @@ describe Project do it { is_expected.to include_module(Sortable) } end - describe '.missing_kubernetes_namespace' do - let!(:project) { create(:project) } - let!(:cluster) { create(:cluster, :provided_by_user, :group) } - let(:kubernetes_namespaces) { project.kubernetes_namespaces } - - subject { described_class.missing_kubernetes_namespace(kubernetes_namespaces) } - - it { is_expected.to contain_exactly(project) } - - context 'kubernetes namespace exists' do - before do - create(:cluster_kubernetes_namespace, project: project, cluster: cluster) - end - - it { is_expected.to be_empty } - end - end - describe 'validation' do let!(:project) { create(:project) } diff --git a/spec/requests/api/issues/get_project_issues_spec.rb b/spec/requests/api/issues/get_project_issues_spec.rb index f7ca6fd1e0a..f11d8259d4a 100644 --- a/spec/requests/api/issues/get_project_issues_spec.rb +++ b/spec/requests/api/issues/get_project_issues_spec.rb @@ -389,7 +389,7 @@ describe API::Issues do it 'returns an array of issues with any milestone' do get api("#{base_url}/issues", user), params: { milestone: any_milestone_title } - expect_paginated_array_response([issue.id, closed_issue.id]) + expect_paginated_array_response([issue.id, confidential_issue.id, closed_issue.id]) end context 'without sort params' do diff --git a/spec/rubocop/cop/inject_enterprise_edition_module_spec.rb b/spec/rubocop/cop/inject_enterprise_edition_module_spec.rb index 0ff777388e5..27df42c0aee 100644 --- a/spec/rubocop/cop/inject_enterprise_edition_module_spec.rb +++ b/spec/rubocop/cop/inject_enterprise_edition_module_spec.rb @@ -10,91 +10,91 @@ describe RuboCop::Cop::InjectEnterpriseEditionModule do subject(:cop) { described_class.new } - it 'flags the use of `prepend EE` in the middle of a file' do + it 'flags the use of `prepend_if_ee EE` in the middle of a file' do expect_offense(<<~SOURCE) class Foo - prepend EE::Foo - ^^^^^^^^^^^^^^^ Injecting EE modules must be done on the last line of this file, outside of any class or module definitions + prepend_if_ee 'EE::Foo' + ^^^^^^^^^^^^^^^^^^^^^^^ Injecting EE modules must be done on the last line of this file, outside of any class or module definitions end SOURCE end - it 'does not flag the use of `prepend EEFoo` in the middle of a file' do + it 'does not flag the use of `prepend_if_ee EEFoo` in the middle of a file' do expect_no_offenses(<<~SOURCE) class Foo - prepend EEFoo + prepend_if_ee 'EEFoo' end SOURCE end - it 'flags the use of `prepend EE::Foo::Bar` in the middle of a file' do + it 'flags the use of `prepend_if_ee EE::Foo::Bar` in the middle of a file' do expect_offense(<<~SOURCE) class Foo - prepend EE::Foo::Bar - ^^^^^^^^^^^^^^^^^^^^ Injecting EE modules must be done on the last line of this file, outside of any class or module definitions + prepend_if_ee 'EE::Foo::Bar' + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Injecting EE modules must be done on the last line of this file, outside of any class or module definitions end SOURCE end - it 'flags the use of `prepend(EE::Foo::Bar)` in the middle of a file' do + it 'flags the use of `prepend_if_ee(EE::Foo::Bar)` in the middle of a file' do expect_offense(<<~SOURCE) class Foo - prepend(EE::Foo::Bar) - ^^^^^^^^^^^^^^^^^^^^^ Injecting EE modules must be done on the last line of this file, outside of any class or module definitions + prepend_if_ee('EE::Foo::Bar') + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Injecting EE modules must be done on the last line of this file, outside of any class or module definitions end SOURCE end - it 'flags the use of `prepend EE::Foo::Bar::Baz` in the middle of a file' do + it 'flags the use of `prepend_if_ee EE::Foo::Bar::Baz` in the middle of a file' do expect_offense(<<~SOURCE) class Foo - prepend EE::Foo::Bar::Baz - ^^^^^^^^^^^^^^^^^^^^^^^^^ Injecting EE modules must be done on the last line of this file, outside of any class or module definitions + prepend_if_ee 'EE::Foo::Bar::Baz' + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Injecting EE modules must be done on the last line of this file, outside of any class or module definitions end SOURCE end - it 'flags the use of `prepend ::EE` in the middle of a file' do + it 'flags the use of `prepend_if_ee ::EE` in the middle of a file' do expect_offense(<<~SOURCE) class Foo - prepend ::EE::Foo - ^^^^^^^^^^^^^^^^^ Injecting EE modules must be done on the last line of this file, outside of any class or module definitions + prepend_if_ee '::EE::Foo' + ^^^^^^^^^^^^^^^^^^^^^^^^^ Injecting EE modules must be done on the last line of this file, outside of any class or module definitions end SOURCE end - it 'flags the use of `include EE` in the middle of a file' do + it 'flags the use of `include_if_ee EE` in the middle of a file' do expect_offense(<<~SOURCE) class Foo - include EE::Foo - ^^^^^^^^^^^^^^^ Injecting EE modules must be done on the last line of this file, outside of any class or module definitions + include_if_ee 'EE::Foo' + ^^^^^^^^^^^^^^^^^^^^^^^ Injecting EE modules must be done on the last line of this file, outside of any class or module definitions end SOURCE end - it 'flags the use of `include ::EE` in the middle of a file' do + it 'flags the use of `include_if_ee ::EE` in the middle of a file' do expect_offense(<<~SOURCE) class Foo - include ::EE::Foo - ^^^^^^^^^^^^^^^^^ Injecting EE modules must be done on the last line of this file, outside of any class or module definitions + include_if_ee '::EE::Foo' + ^^^^^^^^^^^^^^^^^^^^^^^^^ Injecting EE modules must be done on the last line of this file, outside of any class or module definitions end SOURCE end - it 'flags the use of `extend EE` in the middle of a file' do + it 'flags the use of `extend_if_ee EE` in the middle of a file' do expect_offense(<<~SOURCE) class Foo - extend EE::Foo - ^^^^^^^^^^^^^^ Injecting EE modules must be done on the last line of this file, outside of any class or module definitions + extend_if_ee 'EE::Foo' + ^^^^^^^^^^^^^^^^^^^^^^ Injecting EE modules must be done on the last line of this file, outside of any class or module definitions end SOURCE end - it 'flags the use of `extend ::EE` in the middle of a file' do + it 'flags the use of `extend_if_ee ::EE` in the middle of a file' do expect_offense(<<~SOURCE) class Foo - extend ::EE::Foo - ^^^^^^^^^^^^^^^^ Injecting EE modules must be done on the last line of this file, outside of any class or module definitions + extend_if_ee '::EE::Foo' + ^^^^^^^^^^^^^^^^^^^^^^^^ Injecting EE modules must be done on the last line of this file, outside of any class or module definitions end SOURCE end @@ -102,7 +102,7 @@ describe RuboCop::Cop::InjectEnterpriseEditionModule do it 'does not flag prepending of regular modules' do expect_no_offenses(<<~SOURCE) class Foo - prepend Foo + prepend_if_ee 'Foo' end SOURCE end @@ -110,7 +110,7 @@ describe RuboCop::Cop::InjectEnterpriseEditionModule do it 'does not flag including of regular modules' do expect_no_offenses(<<~SOURCE) class Foo - include Foo + include_if_ee 'Foo' end SOURCE end @@ -118,51 +118,111 @@ describe RuboCop::Cop::InjectEnterpriseEditionModule do it 'does not flag extending using regular modules' do expect_no_offenses(<<~SOURCE) class Foo - extend Foo + extend_if_ee 'Foo' end SOURCE end - it 'does not flag the use of `prepend EE` on the last line' do + it 'does not flag the use of `prepend_if_ee EE` on the last line' do expect_no_offenses(<<~SOURCE) class Foo end - Foo.prepend(EE::Foo) + Foo.prepend_if_ee('EE::Foo') SOURCE end - it 'does not flag the use of `include EE` on the last line' do + it 'does not flag the use of `include_if_ee EE` on the last line' do expect_no_offenses(<<~SOURCE) class Foo end - Foo.include(EE::Foo) + Foo.include_if_ee('EE::Foo') SOURCE end - it 'does not flag the use of `extend EE` on the last line' do + it 'does not flag the use of `extend_if_ee EE` on the last line' do expect_no_offenses(<<~SOURCE) class Foo end - Foo.extend(EE::Foo) + Foo.extend_if_ee('EE::Foo') SOURCE end it 'autocorrects offenses by just disabling the Cop' do source = <<~SOURCE class Foo - prepend EE::Foo - include Bar + prepend_if_ee 'EE::Foo' + include_if_ee 'Bar' end SOURCE expect(autocorrect_source(source)).to eq(<<~SOURCE) class Foo - prepend EE::Foo # rubocop: disable Cop/InjectEnterpriseEditionModule - include Bar + prepend_if_ee 'EE::Foo' # rubocop: disable Cop/InjectEnterpriseEditionModule + include_if_ee 'Bar' + end + SOURCE + end + + it 'disallows the use of prepend to inject an EE module' do + expect_offense(<<~SOURCE) + class Foo + end + + Foo.prepend(EE::Foo) + ^^^^^^^^^^^^^^^^^^^^ EE modules must be injected using `include_if_ee`, `extend_if_ee`, or `prepend_if_ee` + SOURCE + end + + it 'disallows the use of extend to inject an EE module' do + expect_offense(<<~SOURCE) + class Foo end + + Foo.extend(EE::Foo) + ^^^^^^^^^^^^^^^^^^^ EE modules must be injected using `include_if_ee`, `extend_if_ee`, or `prepend_if_ee` + SOURCE + end + + it 'disallows the use of include to inject an EE module' do + expect_offense(<<~SOURCE) + class Foo + end + + Foo.include(EE::Foo) + ^^^^^^^^^^^^^^^^^^^^ EE modules must be injected using `include_if_ee`, `extend_if_ee`, or `prepend_if_ee` + SOURCE + end + + it 'disallows the use of prepend_if_ee without a String' do + expect_offense(<<~SOURCE) + class Foo + end + + Foo.prepend_if_ee(EE::Foo) + ^^^^^^^ EE modules to inject must be specified as a String + SOURCE + end + + it 'disallows the use of include_if_ee without a String' do + expect_offense(<<~SOURCE) + class Foo + end + + Foo.include_if_ee(EE::Foo) + ^^^^^^^ EE modules to inject must be specified as a String + SOURCE + end + + it 'disallows the use of extend_if_ee without a String' do + expect_offense(<<~SOURCE) + class Foo + end + + Foo.extend_if_ee(EE::Foo) + ^^^^^^^ EE modules to inject must be specified as a String SOURCE end end diff --git a/spec/services/application_settings/update_service_spec.rb b/spec/services/application_settings/update_service_spec.rb index 33cd1f37ff6..adb5219d691 100644 --- a/spec/services/application_settings/update_service_spec.rb +++ b/spec/services/application_settings/update_service_spec.rb @@ -62,6 +62,54 @@ describe ApplicationSettings::UpdateService do end end + describe 'updating outbound_local_requests_whitelist' do + context 'when params is blank' do + let(:params) { {} } + + it 'does not add to whitelist' do + expect { subject.execute }.not_to change { + application_settings.outbound_local_requests_whitelist + } + end + end + + context 'when param add_to_outbound_local_requests_whitelist contains values' do + before do + application_settings.outbound_local_requests_whitelist = ['127.0.0.1'] + end + + let(:params) { { add_to_outbound_local_requests_whitelist: ['example.com', ''] } } + + it 'adds to whitelist' do + expect { subject.execute }.to change { + application_settings.outbound_local_requests_whitelist + } + + expect(application_settings.outbound_local_requests_whitelist).to contain_exactly( + '127.0.0.1', 'example.com' + ) + end + end + + context 'when param outbound_local_requests_whitelist_raw is passed' do + before do + application_settings.outbound_local_requests_whitelist = ['127.0.0.1'] + end + + let(:params) { { outbound_local_requests_whitelist_raw: 'example.com;gitlab.com' } } + + it 'overwrites the existing whitelist' do + expect { subject.execute }.to change { + application_settings.outbound_local_requests_whitelist + } + + expect(application_settings.outbound_local_requests_whitelist).to contain_exactly( + 'example.com', 'gitlab.com' + ) + end + end + end + describe 'performance bar settings' do using RSpec::Parameterized::TableSyntax diff --git a/spec/services/metrics/dashboard/default_embed_service_spec.rb b/spec/services/metrics/dashboard/default_embed_service_spec.rb index 5b24b9b2a14..803b9a93be7 100644 --- a/spec/services/metrics/dashboard/default_embed_service_spec.rb +++ b/spec/services/metrics/dashboard/default_embed_service_spec.rb @@ -14,14 +14,16 @@ describe Metrics::Dashboard::DefaultEmbedService, :use_clean_rails_memory_store_ end describe '#get_dashboard' do - let(:service_params) { [project, user, { environment: environment, dashboard_path: nil }] } + let(:service_params) { [project, user, { environment: environment }] } let(:service_call) { described_class.new(*service_params).get_dashboard } it_behaves_like 'valid embedded dashboard service response' it_behaves_like 'raises error for users with insufficient permissions' it 'caches the unprocessed dashboard for subsequent calls' do - expect(YAML).to receive(:safe_load).once.and_call_original + system_service = Metrics::Dashboard::SystemDashboardService + + expect(system_service).to receive(:new).once.and_call_original described_class.new(*service_params).get_dashboard described_class.new(*service_params).get_dashboard diff --git a/spec/services/self_monitoring/project/create_service_spec.rb b/spec/services/self_monitoring/project/create_service_spec.rb index d11e27c6d52..a1e7aaf45f2 100644 --- a/spec/services/self_monitoring/project/create_service_spec.rb +++ b/spec/services/self_monitoring/project/create_service_spec.rb @@ -90,19 +90,17 @@ describe SelfMonitoring::Project::CreateService do ) end - # This should pass when https://gitlab.com/gitlab-org/gitlab-ce/issues/44496 - # is complete and the prometheus listen address is added to the whitelist. - # context 'when local requests from hooks and services are not allowed' do - # before do - # allow(ApplicationSetting) - # .to receive(:current) - # .and_return( - # ApplicationSetting.build_from_defaults(allow_local_requests_from_hooks_and_services: false) - # ) - # end - - # it_behaves_like 'has prometheus service', 'http://localhost:9090' - # end + context 'when local requests from hooks and services are not allowed' do + before do + allow(ApplicationSetting) + .to receive(:current) + .and_return( + ApplicationSetting.build_from_defaults(allow_local_requests_from_hooks_and_services: false) + ) + end + + it_behaves_like 'has prometheus service', 'http://localhost:9090' + end context 'with non default prometheus address' do before do diff --git a/spec/support/helpers/features/notes_helpers.rb b/spec/support/helpers/features/notes_helpers.rb index a2d8d71b541..8c27f81930d 100644 --- a/spec/support/helpers/features/notes_helpers.rb +++ b/spec/support/helpers/features/notes_helpers.rb @@ -23,6 +23,14 @@ module Spec end end + def edit_note(note_text_to_edit, new_note_text) + page.within('#notes-list li.note', text: note_text_to_edit) do + find('.js-note-edit').click + fill_in('note[note]', with: new_note_text) + find('.js-comment-button').click + end + end + def preview_note(text) page.within('.js-main-target-form') do filled_text = fill_in('note[note]', with: text) diff --git a/spec/support/helpers/graphql_helpers.rb b/spec/support/helpers/graphql_helpers.rb index ae1b859ae3f..d86371d70b9 100644 --- a/spec/support/helpers/graphql_helpers.rb +++ b/spec/support/helpers/graphql_helpers.rb @@ -3,6 +3,8 @@ module GraphqlHelpers MutationDefinition = Struct.new(:query, :variables) + NoData = Class.new(StandardError) + # makes an underscored string look like a fieldname # "merge_request" => "mergeRequest" def self.fieldnamerize(underscored_field_name) @@ -158,8 +160,9 @@ module GraphqlHelpers post_graphql(mutation.query, current_user: current_user, variables: mutation.variables) end + # Raises an error if no data is found def graphql_data - json_response['data'] + json_response['data'] || (raise NoData, graphql_errors) end def graphql_errors @@ -173,8 +176,9 @@ module GraphqlHelpers end end + # Raises an error if no response is found def graphql_mutation_response(mutation_name) - graphql_data[GraphqlHelpers.fieldnamerize(mutation_name)] + graphql_data.fetch(GraphqlHelpers.fieldnamerize(mutation_name)) end def nested_fields?(field) diff --git a/spec/support/shared_examples/application_setting_examples.rb b/spec/support/shared_examples/application_setting_examples.rb index 2c600785ad3..a43d2a75082 100644 --- a/spec/support/shared_examples/application_setting_examples.rb +++ b/spec/support/shared_examples/application_setting_examples.rb @@ -40,6 +40,11 @@ RSpec.shared_examples 'string of domains' do |attribute| setting.method("#{attribute}_raw=").call("example;34543:garbage:fdh5654;") expect(setting.method(attribute).call).to contain_exactly('example', '34543:garbage:fdh5654') end + + it 'does not raise error with nil' do + setting.method("#{attribute}_raw=").call(nil) + expect(setting.method(attribute).call).to eq([]) + end end RSpec.shared_examples 'application settings examples' do @@ -58,6 +63,19 @@ RSpec.shared_examples 'application settings examples' do context 'outbound_local_requests_whitelist' do it_behaves_like 'string of domains', :outbound_local_requests_whitelist + + it 'clears outbound_local_requests_whitelist_arrays memoization' do + setting.outbound_local_requests_whitelist_raw = 'example.com' + + expect(setting.outbound_local_requests_whitelist_arrays).to contain_exactly( + [], ['example.com'] + ) + + setting.outbound_local_requests_whitelist_raw = 'gitlab.com' + expect(setting.outbound_local_requests_whitelist_arrays).to contain_exactly( + [], ['gitlab.com'] + ) + end end context 'outbound_local_requests_whitelist_arrays' do @@ -73,7 +91,60 @@ RSpec.shared_examples 'application settings examples' do ] domain_whitelist = ['www.example.com', 'example.com', 'subdomain.example.com'] - expect(setting.outbound_local_requests_whitelist_arrays).to contain_exactly(ip_whitelist, domain_whitelist) + expect(setting.outbound_local_requests_whitelist_arrays).to contain_exactly( + ip_whitelist, domain_whitelist + ) + end + end + + context 'add_to_outbound_local_requests_whitelist' do + it 'adds entry to outbound_local_requests_whitelist' do + setting.outbound_local_requests_whitelist = ['example.com'] + + setting.add_to_outbound_local_requests_whitelist( + ['example.com', '127.0.0.1', 'gitlab.com'] + ) + + expect(setting.outbound_local_requests_whitelist).to contain_exactly( + 'example.com', + '127.0.0.1', + 'gitlab.com' + ) + end + + it 'clears outbound_local_requests_whitelist_arrays memoization' do + setting.outbound_local_requests_whitelist = ['example.com'] + + expect(setting.outbound_local_requests_whitelist_arrays).to contain_exactly( + [], + ['example.com'] + ) + + setting.add_to_outbound_local_requests_whitelist( + ['example.com', 'gitlab.com'] + ) + + expect(setting.outbound_local_requests_whitelist_arrays).to contain_exactly( + [], + ['example.com', 'gitlab.com'] + ) + end + + it 'does not raise error with nil' do + setting.outbound_local_requests_whitelist = nil + + setting.add_to_outbound_local_requests_whitelist(['gitlab.com']) + + expect(setting.outbound_local_requests_whitelist).to contain_exactly('gitlab.com') + expect(setting.outbound_local_requests_whitelist_arrays).to contain_exactly( + [], ['gitlab.com'] + ) + end + + it 'does not raise error with nil' do + setting.outbound_local_requests_whitelist = nil + + expect(setting.outbound_local_requests_whitelist_arrays).to contain_exactly([], []) end end diff --git a/spec/validators/qualified_domain_array_validator_spec.rb b/spec/validators/qualified_domain_array_validator_spec.rb index a96b00bfd1d..6beb4c67f6f 100644 --- a/spec/validators/qualified_domain_array_validator_spec.rb +++ b/spec/validators/qualified_domain_array_validator_spec.rb @@ -19,20 +19,19 @@ describe QualifiedDomainArrayValidator do subject { validator.validate(record) } - shared_examples 'cannot be blank' do - it 'returns error when attribute is blank' do - record.domain_array = [] + shared_examples 'can be nil' do + it 'allows when attribute is nil' do + record.domain_array = nil subject - expect(record.errors).to be_present - expect(record.errors.first[1]).to eq 'entries cannot be blank' + expect(record.errors).to be_empty end end - shared_examples 'can be nil' do - it 'allows when attribute is nil' do - record.domain_array = nil + shared_examples 'can be blank' do + it 'allows when attribute is blank' do + record.domain_array = [] subject @@ -43,7 +42,7 @@ describe QualifiedDomainArrayValidator do describe 'validations' do let(:validator) { described_class.new(attributes: [:domain_array]) } - it_behaves_like 'cannot be blank' + it_behaves_like 'can be blank' it 'returns error when attribute is nil' do record.domain_array = nil @@ -51,6 +50,7 @@ describe QualifiedDomainArrayValidator do subject expect(record.errors).to be_present + expect(record.errors.first[1]).to eq('entries cannot be nil') end it 'allows when domain is valid' do @@ -91,21 +91,13 @@ describe QualifiedDomainArrayValidator do let(:validator) { described_class.new(attributes: [:domain_array], allow_nil: true) } it_behaves_like 'can be nil' - - it_behaves_like 'cannot be blank' + it_behaves_like 'can be blank' end context 'when allow_blank is set to true' do let(:validator) { described_class.new(attributes: [:domain_array], allow_blank: true) } it_behaves_like 'can be nil' - - it 'allows when attribute is blank' do - record.domain_array = [] - - subject - - expect(record.errors).to be_empty - end + it_behaves_like 'can be blank' end end |