diff options
Diffstat (limited to 'spec')
34 files changed, 1103 insertions, 272 deletions
diff --git a/spec/controllers/admin/integrations_controller_spec.rb b/spec/controllers/admin/integrations_controller_spec.rb index 4a5d5ede728..353f6f11e93 100644 --- a/spec/controllers/admin/integrations_controller_spec.rb +++ b/spec/controllers/admin/integrations_controller_spec.rb @@ -28,7 +28,7 @@ RSpec.describe Admin::IntegrationsController do before do allow(PropagateIntegrationWorker).to receive(:perform_async) - put :update, params: { id: integration.class.to_param, overwrite: true, service: { url: url } } + put :update, params: { id: integration.class.to_param, service: { url: url } } end context 'valid params' do @@ -40,7 +40,7 @@ RSpec.describe Admin::IntegrationsController do end it 'calls to PropagateIntegrationWorker' do - expect(PropagateIntegrationWorker).to have_received(:perform_async).with(integration.id, true) + expect(PropagateIntegrationWorker).to have_received(:perform_async).with(integration.id, false) end end diff --git a/spec/features/projects/blobs/blob_show_spec.rb b/spec/features/projects/blobs/blob_show_spec.rb index 3032d115a00..7c564d76f70 100644 --- a/spec/features/projects/blobs/blob_show_spec.rb +++ b/spec/features/projects/blobs/blob_show_spec.rb @@ -564,35 +564,76 @@ RSpec.describe 'File blob', :js do file_path: '.gitlab/dashboards/custom-dashboard.yml', file_content: file_content ).execute - - visit_blob('.gitlab/dashboards/custom-dashboard.yml') end - context 'valid dashboard file' do - let(:file_content) { File.read(Rails.root.join('config/prometheus/common_metrics.yml')) } + context 'with metrics_dashboard_exhaustive_validations feature flag off' do + before do + stub_feature_flags(metrics_dashboard_exhaustive_validations: false) + visit_blob('.gitlab/dashboards/custom-dashboard.yml') + end - it 'displays an auxiliary viewer' do - aggregate_failures do - # shows that dashboard yaml is valid - expect(page).to have_content('Metrics Dashboard YAML definition is valid.') + context 'valid dashboard file' do + let(:file_content) { File.read(Rails.root.join('config/prometheus/common_metrics.yml')) } + + it 'displays an auxiliary viewer' do + aggregate_failures do + # shows that dashboard yaml is valid + expect(page).to have_content('Metrics Dashboard YAML definition is valid.') + + # shows a learn more link + expect(page).to have_link('Learn more') + end + end + end + + context 'invalid dashboard file' do + let(:file_content) { "dashboard: 'invalid'" } - # shows a learn more link - expect(page).to have_link('Learn more') + it 'displays an auxiliary viewer' do + aggregate_failures do + # shows that dashboard yaml is invalid + expect(page).to have_content('Metrics Dashboard YAML definition is invalid:') + expect(page).to have_content("panel_groups: should be an array of panel_groups objects") + + # shows a learn more link + expect(page).to have_link('Learn more') + end end end end - context 'invalid dashboard file' do - let(:file_content) { "dashboard: 'invalid'" } + context 'with metrics_dashboard_exhaustive_validations feature flag on' do + before do + stub_feature_flags(metrics_dashboard_exhaustive_validations: true) + visit_blob('.gitlab/dashboards/custom-dashboard.yml') + end - it 'displays an auxiliary viewer' do - aggregate_failures do - # shows that dashboard yaml is invalid - expect(page).to have_content('Metrics Dashboard YAML definition is invalid:') - expect(page).to have_content("panel_groups: should be an array of panel_groups objects") + context 'valid dashboard file' do + let(:file_content) { File.read(Rails.root.join('config/prometheus/common_metrics.yml')) } - # shows a learn more link - expect(page).to have_link('Learn more') + it 'displays an auxiliary viewer' do + aggregate_failures do + # shows that dashboard yaml is valid + expect(page).to have_content('Metrics Dashboard YAML definition is valid.') + + # shows a learn more link + expect(page).to have_link('Learn more') + end + end + end + + context 'invalid dashboard file' do + let(:file_content) { "dashboard: 'invalid'" } + + it 'displays an auxiliary viewer' do + aggregate_failures do + # shows that dashboard yaml is invalid + expect(page).to have_content('Metrics Dashboard YAML definition is invalid:') + expect(page).to have_content("root is missing required keys: panel_groups") + + # shows a learn more link + expect(page).to have_link('Learn more') + end end end end diff --git a/spec/fixtures/lib/gitlab/metrics/dashboard/broken_yml_syntax.yml b/spec/fixtures/lib/gitlab/metrics/dashboard/broken_yml_syntax.yml new file mode 100644 index 00000000000..d551ad2dcdb --- /dev/null +++ b/spec/fixtures/lib/gitlab/metrics/dashboard/broken_yml_syntax.yml @@ -0,0 +1,13 @@ +dashboard: 'Test Dashboard' + panel_groups: + - group: Group B + panels: + - title: "Super Chart B" + type: "area-chart" + y_label: "y_label" + metrics: + - id: metric_b + query_range: 'query' + unit: unit + label: Legend Label +- group: Group A diff --git a/spec/frontend/jira_import/components/__snapshots__/jira_import_form_spec.js.snap b/spec/frontend/jira_import/components/__snapshots__/jira_import_form_spec.js.snap index 975c31bb59c..eede5426f42 100644 --- a/spec/frontend/jira_import/components/__snapshots__/jira_import_form_spec.js.snap +++ b/spec/frontend/jira_import/components/__snapshots__/jira_import_form_spec.js.snap @@ -114,7 +114,7 @@ exports[`JiraImportForm table body shows correct information in each cell 1`] = <!----> <div - class="gl-search-box-by-type m-2" + class="gl-search-box-by-type gl-m-3" > <svg class="gl-search-box-by-type-search-icon gl-icon s16" @@ -225,7 +225,7 @@ exports[`JiraImportForm table body shows correct information in each cell 1`] = <!----> <div - class="gl-search-box-by-type m-2" + class="gl-search-box-by-type gl-m-3" > <svg class="gl-search-box-by-type-search-icon gl-icon s16" diff --git a/spec/frontend/jira_import/components/jira_import_form_spec.js b/spec/frontend/jira_import/components/jira_import_form_spec.js index 7cc7b40f4c8..6ef28a71f48 100644 --- a/spec/frontend/jira_import/components/jira_import_form_spec.js +++ b/spec/frontend/jira_import/components/jira_import_form_spec.js @@ -10,6 +10,7 @@ import { imports, issuesPath, jiraProjects, + jiraUsersResponse, projectId, projectPath, userMappings as defaultUserMappings, @@ -38,7 +39,10 @@ describe('JiraImportForm', () => { const getHeader = name => getByRole(wrapper.element, 'columnheader', { name }); + const findLoadMoreUsersButton = () => wrapper.find('[data-testid="load-more-users-button"]'); + const mountComponent = ({ + hasMoreUsers = false, isSubmitting = false, loading = false, mutate = mutateSpy, @@ -55,6 +59,7 @@ describe('JiraImportForm', () => { projectPath, }, data: () => ({ + hasMoreUsers, isFetching: false, isSubmitting, searchTerm: '', @@ -300,6 +305,7 @@ describe('JiraImportForm', () => { variables: { input: { projectPath, + startAt: 0, }, }, }; @@ -318,4 +324,53 @@ describe('JiraImportForm', () => { }); }); }); + + describe('load more users button', () => { + describe('when all users have been loaded', () => { + it('is not shown', () => { + wrapper = mountComponent(); + + expect(findLoadMoreUsersButton().exists()).toBe(false); + }); + }); + + describe('when all users have not been loaded', () => { + it('is shown', () => { + wrapper = mountComponent({ hasMoreUsers: true }); + + expect(findLoadMoreUsersButton().exists()).toBe(true); + }); + }); + + describe('when clicked', () => { + beforeEach(() => { + mutateSpy = jest.fn(() => + Promise.resolve({ + data: { + jiraImportStart: { errors: [] }, + jiraImportUsers: { jiraUsers: jiraUsersResponse, errors: [] }, + }, + }), + ); + + wrapper = mountComponent({ hasMoreUsers: true }); + }); + + it('calls the GraphQL user mapping mutation', async () => { + const mutationArguments = { + mutation: getJiraUserMappingMutation, + variables: { + input: { + projectPath, + startAt: 0, + }, + }, + }; + + findLoadMoreUsersButton().vm.$emit('click'); + + expect(mutateSpy).toHaveBeenCalledWith(expect.objectContaining(mutationArguments)); + }); + }); + }); }); diff --git a/spec/frontend/jira_import/mock_data.js b/spec/frontend/jira_import/mock_data.js index 8ea40080f32..51dd939283e 100644 --- a/spec/frontend/jira_import/mock_data.js +++ b/spec/frontend/jira_import/mock_data.js @@ -1,5 +1,6 @@ import getJiraImportDetailsQuery from '~/jira_import/queries/get_jira_import_details.query.graphql'; import { IMPORT_STATE } from '~/jira_import/utils/jira_import_utils'; +import { userMappingsPageSize } from '~/jira_import/utils/constants'; export const fullPath = 'gitlab-org/gitlab-test'; @@ -87,6 +88,8 @@ export const jiraProjects = [ { text: 'Migrate to GitLab (MTG)', value: 'MTG' }, ]; +export const jiraUsersResponse = new Array(userMappingsPageSize); + export const imports = [ { jiraProjectKey: 'MTG', diff --git a/spec/frontend/monitoring/components/__snapshots__/dashboard_template_spec.js.snap b/spec/frontend/monitoring/components/__snapshots__/dashboard_template_spec.js.snap index 7ef956f8e05..80eacbe0a6a 100644 --- a/spec/frontend/monitoring/components/__snapshots__/dashboard_template_spec.js.snap +++ b/spec/frontend/monitoring/components/__snapshots__/dashboard_template_spec.js.snap @@ -52,7 +52,7 @@ exports[`Dashboard template matches the default snapshot 1`] = ` </gl-new-dropdown-header-stub> <gl-search-box-by-type-stub - class="m-2" + class="gl-m-3" clearbuttontitle="Clear" value="" /> diff --git a/spec/frontend/snippets/components/edit_spec.js b/spec/frontend/snippets/components/edit_spec.js index dff9bf088c0..25cef3a8045 100644 --- a/spec/frontend/snippets/components/edit_spec.js +++ b/spec/frontend/snippets/components/edit_spec.js @@ -183,7 +183,7 @@ describe('Snippet Edit app', () => { ${'foo'} | ${[]} | ${false} ${'foo'} | ${[TEST_ACTIONS.VALID]} | ${false} ${'foo'} | ${[TEST_ACTIONS.VALID, TEST_ACTIONS.NO_CONTENT]} | ${true} - ${'foo'} | ${[TEST_ACTIONS.VALID, TEST_ACTIONS.NO_PATH]} | ${true} + ${'foo'} | ${[TEST_ACTIONS.VALID, TEST_ACTIONS.NO_PATH]} | ${false} `( 'should handle submit disable (title=$title, actions=$actions, shouldDisable=$shouldDisable)', async ({ title, actions, shouldDisable }) => { diff --git a/spec/graphql/types/milestone_type_spec.rb b/spec/graphql/types/milestone_type_spec.rb index 2315c10433b..806495250ac 100644 --- a/spec/graphql/types/milestone_type_spec.rb +++ b/spec/graphql/types/milestone_type_spec.rb @@ -15,7 +15,7 @@ RSpec.describe GitlabSchema.types['Milestone'] do stats ] - expect(described_class).to have_graphql_fields(*expected_fields) + expect(described_class).to have_graphql_fields(*expected_fields).at_least end describe 'stats field' do diff --git a/spec/lib/gitlab/ci/trace_spec.rb b/spec/lib/gitlab/ci/trace_spec.rb index 0326fb2b061..171877dbaee 100644 --- a/spec/lib/gitlab/ci/trace_spec.rb +++ b/spec/lib/gitlab/ci/trace_spec.rb @@ -23,26 +23,14 @@ RSpec.describe Gitlab::Ci::Trace, :clean_gitlab_redis_shared_state do artifact1.file.migrate!(ObjectStorage::Store::REMOTE) end - context 'when write lock is not present' do - it 'raises an exception' do - expect { artifact2.job.trace.raw }.to raise_error(Errno::ENOENT) - end - end + it 'reloads the trace after is it migrated' do + stub_const('Gitlab::HttpIO::BUFFER_SIZE', test_data.length) - context 'when write lock is present', :clean_gitlab_redis_shared_state do - before do - Gitlab::ExclusiveLease.new("trace:write:lock:#{job.id}", timeout: 10.seconds).try_obtain + expect_next_instance_of(Gitlab::HttpIO) do |http_io| + expect(http_io).to receive(:get_chunk).and_return(test_data, "") end - it 'reloads the trace after is it migrated' do - stub_const('Gitlab::HttpIO::BUFFER_SIZE', test_data.length) - - expect_next_instance_of(Gitlab::HttpIO) do |http_io| - expect(http_io).to receive(:get_chunk).and_return(test_data, "") - end - - expect(artifact2.job.trace.raw).to eq(test_data) - end + expect(artifact2.job.trace.raw).to eq(test_data) end end diff --git a/spec/lib/gitlab/exclusive_lease_helpers_spec.rb b/spec/lib/gitlab/exclusive_lease_helpers_spec.rb index e4733d4fe5e..01e2fe8ce17 100644 --- a/spec/lib/gitlab/exclusive_lease_helpers_spec.rb +++ b/spec/lib/gitlab/exclusive_lease_helpers_spec.rb @@ -111,14 +111,4 @@ RSpec.describe Gitlab::ExclusiveLeaseHelpers, :clean_gitlab_redis_shared_state d end end end - - describe '#lock_taken?' do - it 'returns true when lock has been taken' do - expect(class_instance.lock_taken?(unique_key)).to be false - - class_instance.in_lock(unique_key) do - expect(class_instance.lock_taken?(unique_key)).to be true - end - end - end end diff --git a/spec/lib/gitlab/metrics/dashboard/validator/errors_spec.rb b/spec/lib/gitlab/metrics/dashboard/validator/errors_spec.rb index f0db1bd0d33..fdbba6c31b5 100644 --- a/spec/lib/gitlab/metrics/dashboard/validator/errors_spec.rb +++ b/spec/lib/gitlab/metrics/dashboard/validator/errors_spec.rb @@ -34,6 +34,17 @@ RSpec.describe Gitlab::Metrics::Dashboard::Validator::Errors do it { is_expected.to eq 'root is missing required keys: one' } end + + context 'when there is type mismatch' do + %w(null string boolean integer number array object).each do |expected_type| + context "on type: #{expected_type}" do + let(:type) { expected_type } + let(:details) { nil } + + it { is_expected.to eq "'property_name' at root is not of type: #{expected_type}" } + end + end + end end context 'for nested object' do @@ -52,8 +63,6 @@ RSpec.describe Gitlab::Metrics::Dashboard::Validator::Errors do let(:type) { expected_type } let(:details) { nil } - subject { described_class.new(error_hash).message } - it { is_expected.to eq "'property_name' at /nested_objects/0 is not of type: #{expected_type}" } end end diff --git a/spec/lib/gitlab/metrics/dashboard/validator_spec.rb b/spec/lib/gitlab/metrics/dashboard/validator_spec.rb index c4cda271408..eb67ea2b7da 100644 --- a/spec/lib/gitlab/metrics/dashboard/validator_spec.rb +++ b/spec/lib/gitlab/metrics/dashboard/validator_spec.rb @@ -143,4 +143,56 @@ RSpec.describe Gitlab::Metrics::Dashboard::Validator do end end end + + describe '#errors' do + context 'valid dashboard schema' do + it 'returns no errors' do + expect(described_class.errors(valid_dashboard)).to eq [] + end + + context 'with duplicate metric_ids' do + it 'returns errors' do + expect(described_class.errors(duplicate_id_dashboard)).to eq [Gitlab::Metrics::Dashboard::Validator::Errors::DuplicateMetricIds.new] + end + end + + context 'with dashboard_path and project' do + subject { described_class.errors(valid_dashboard, dashboard_path: 'test/path.yml', project: project) } + + context 'with no conflicting metric identifiers in db' do + it { is_expected.to eq [] } + end + + context 'with metric identifier present in current dashboard' do + before do + create(:prometheus_metric, + identifier: 'metric_a1', + dashboard_path: 'test/path.yml', + project: project + ) + end + + it { is_expected.to eq [] } + end + + context 'with metric identifier present in another dashboard' do + before do + create(:prometheus_metric, + identifier: 'metric_a1', + dashboard_path: 'some/other/dashboard/path.yml', + project: project + ) + end + + it { is_expected.to eq [Gitlab::Metrics::Dashboard::Validator::Errors::DuplicateMetricIds.new] } + end + end + end + + context 'invalid dashboard schema' do + it 'returns collection of validation errors' do + expect(described_class.errors(invalid_dashboard)).to all be_kind_of(Gitlab::Metrics::Dashboard::Validator::Errors::SchemaValidationError) + 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 index 057f0f32158..84dfc5186a8 100644 --- a/spec/models/blob_viewer/metrics_dashboard_yml_spec.rb +++ b/spec/models/blob_viewer/metrics_dashboard_yml_spec.rb @@ -9,119 +9,228 @@ RSpec.describe BlobViewer::MetricsDashboardYml do let_it_be(:project) { create(:project, :repository) } let(:blob) { fake_blob(path: '.gitlab/dashboards/custom-dashboard.yml', data: data) } let(:sha) { sample_commit.id } + let(:data) { fixture_file('lib/gitlab/metrics/dashboard/sample_dashboard.yml') } 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')) } + context 'with metrics_dashboard_exhaustive_validations feature flag on' do + before do + stub_feature_flags(metrics_dashboard_exhaustive_validations: true) + end + + context 'when the definition is valid' do + before do + allow(Gitlab::Metrics::Dashboard::Validator).to receive(:errors).and_return([]) + end + + describe '#valid?' do + it 'calls prepare! on the viewer' do + expect(viewer).to receive(:prepare!) + + viewer.valid? + end + + it 'processes dashboard yaml and 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(Gitlab::Metrics::Dashboard::Validator) + .to receive(:errors) + .with(yml, dashboard_path: '.gitlab/dashboards/custom-dashboard.yml', project: project) + .and_call_original + expect(viewer.valid?).to be true + end + end + + describe '#errors' do + it 'returns empty array' do + expect(viewer.errors).to eq [] + end + end + end + + context 'when definition is invalid' do + let(:error) { ::Gitlab::Metrics::Dashboard::Validator::Errors::SchemaValidationError.new } + let(:data) do + <<~YAML + dashboard: + YAML + end + + before do + allow(Gitlab::Metrics::Dashboard::Validator).to receive(:errors).and_return([error]) + end - describe '#valid?' do - it 'calls prepare! on the viewer' do - allow(PerformanceMonitoring::PrometheusDashboard).to receive(:from_json) + describe '#valid?' do + it 'returns false' do + expect(viewer.valid?).to be false + end + end - expect(viewer).to receive(:prepare!) + describe '#errors' do + it 'returns validation errors' do + expect(viewer.errors).to eq ["Dashboard failed schema validation"] + end + end + end - viewer.valid? + context 'when YAML syntax is invalid' do + let(:data) do + <<~YAML + dashboard: 'empty metrics' + panel_groups: + - group: 'Group Title' + YAML end - it 'returns true', :aggregate_failures do - yml = ::Gitlab::Config::Loader::Yaml.new(data).load_raw! + describe '#valid?' do + it 'returns false' do + expect(Gitlab::Metrics::Dashboard::Validator).not_to receive(:errors) + expect(viewer.valid?).to be false + end + end - expect_next_instance_of(::Gitlab::Config::Loader::Yaml, data) do |loader| - expect(loader).to receive(:load_raw!).and_call_original + describe '#errors' do + it 'returns validation errors' do + expect(viewer.errors).to all be_kind_of String 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) + 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 - expect(viewer.errors).to be nil + it 'is invalid' do + expect(Gitlab::Metrics::Dashboard::Validator).not_to receive(:errors) + expect(viewer.valid?).to be false + end + + it 'returns validation errors' do + expect(viewer.errors).to eq ['The parsed YAML is too big'] 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 + context 'with metrics_dashboard_exhaustive_validations feature flag off' do + before do + stub_feature_flags(metrics_dashboard_exhaustive_validations: false) end - describe '#valid?' do - it 'returns false' do - expect(PerformanceMonitoring::PrometheusDashboard) - .to receive(:from_json).and_raise(error) + context 'when the definition is valid' do + describe '#valid?' do + before do + allow(PerformanceMonitoring::PrometheusDashboard).to receive(:from_json) + end + + it 'calls prepare! on the viewer' do + expect(viewer).to receive(:prepare!) - expect(viewer.valid?).to be_falsey + viewer.valid? + end + + it 'processes dashboard yaml and 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 true + end + end + + describe '#errors' do + it 'returns empty array' do + expect(viewer.errors).to eq [] + end end end - describe '#errors' do - it 'returns validation errors' do - allow(PerformanceMonitoring::PrometheusDashboard) - .to receive(:from_json).and_raise(error) + 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.errors).to be error.model.errors + expect(viewer.valid?).to be false + end + end + + describe '#errors' do + it 'returns validation errors' do + allow(PerformanceMonitoring::PrometheusDashboard) + .to receive(:from_json).and_raise(error) + + expect(viewer.errors).to eq error.model.errors.messages.map { |messages| messages.join(': ') } + end end end - end - context 'when YAML syntax is invalid' do - let(:data) do - <<~YAML + 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 + YAML 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"] } + describe '#valid?' do + it 'returns false' do + expect(PerformanceMonitoring::PrometheusDashboard).not_to receive(:from_json) + expect(viewer.valid?).to be false + end + end - expect(viewer.errors).to be_kind_of ActiveModel::Errors - expect(viewer.errors.messages).to eql(yaml_wrapped_errors) + describe '#errors' do + it 'returns validation errors' do + expect(viewer.errors).to eq ["YAML syntax: (<unknown>): did not find expected key while parsing a block mapping at line 1 column 1"] + end end end - end - context 'when YAML loader raises error' do - let(:data) do - <<~YAML + 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 + YAML + end - it 'is invalid' do - expect(PerformanceMonitoring::PrometheusDashboard).not_to receive(:from_json) - expect(viewer.valid?).to be(false) - 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 'returns validation errors' do - yaml_wrapped_errors = { 'YAML syntax': ["The parsed YAML is too big"] } + it 'is invalid' do + expect(PerformanceMonitoring::PrometheusDashboard).not_to receive(:from_json) + expect(viewer.valid?).to be false + end - expect(viewer.errors).to be_kind_of(ActiveModel::Errors) - expect(viewer.errors.messages).to eq(yaml_wrapped_errors) + it 'returns validation errors' do + expect(viewer.errors).to eq ["YAML syntax: The parsed YAML is too big"] + end end end end diff --git a/spec/models/ci/job_artifact_spec.rb b/spec/models/ci/job_artifact_spec.rb index 91a669aa3f4..0518b2c7540 100644 --- a/spec/models/ci/job_artifact_spec.rb +++ b/spec/models/ci/job_artifact_spec.rb @@ -343,42 +343,6 @@ RSpec.describe Ci::JobArtifact do end end - describe '#each_blob' do - context 'when file format is gzip' do - context 'when gzip file contains one file' do - let(:artifact) { build(:ci_job_artifact, :junit) } - - it 'iterates blob once' do - expect { |b| artifact.each_blob(&b) }.to yield_control.once - end - end - - context 'when gzip file contains three files' do - let(:artifact) { build(:ci_job_artifact, :junit_with_three_testsuites) } - - it 'iterates blob three times' do - expect { |b| artifact.each_blob(&b) }.to yield_control.exactly(3).times - end - end - end - - context 'when file format is raw' do - let(:artifact) { build(:ci_job_artifact, :codequality, file_format: :raw) } - - it 'iterates blob once' do - expect { |b| artifact.each_blob(&b) }.to yield_control.once - end - end - - context 'when there are no adapters for the file format' do - let(:artifact) { build(:ci_job_artifact, :junit, file_format: :zip) } - - it 'raises an error' do - expect { |b| artifact.each_blob(&b) }.to raise_error(described_class::NotSupportedAdapterError) - end - end - end - describe 'expired?' do subject { artifact.expired? } diff --git a/spec/models/concerns/ci/artifactable_spec.rb b/spec/models/concerns/ci/artifactable_spec.rb index 13c2ff5efe5..f05189abdd2 100644 --- a/spec/models/concerns/ci/artifactable_spec.rb +++ b/spec/models/concerns/ci/artifactable_spec.rb @@ -18,4 +18,40 @@ RSpec.describe Ci::Artifactable do it { is_expected.to be_const_defined(:FILE_FORMAT_ADAPTERS) } end end + + describe '#each_blob' do + context 'when file format is gzip' do + context 'when gzip file contains one file' do + let(:artifact) { build(:ci_job_artifact, :junit) } + + it 'iterates blob once' do + expect { |b| artifact.each_blob(&b) }.to yield_control.once + end + end + + context 'when gzip file contains three files' do + let(:artifact) { build(:ci_job_artifact, :junit_with_three_testsuites) } + + it 'iterates blob three times' do + expect { |b| artifact.each_blob(&b) }.to yield_control.exactly(3).times + end + end + end + + context 'when file format is raw' do + let(:artifact) { build(:ci_job_artifact, :codequality, file_format: :raw) } + + it 'iterates blob once' do + expect { |b| artifact.each_blob(&b) }.to yield_control.once + end + end + + context 'when there are no adapters for the file format' do + let(:artifact) { build(:ci_job_artifact, :junit, file_format: :zip) } + + it 'raises an error' do + expect { |b| artifact.each_blob(&b) }.to raise_error(described_class::NotSupportedAdapterError) + end + end + end end diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index f685fe12e13..39ece3d1cb3 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -391,14 +391,14 @@ RSpec.describe Namespace do let(:uploads_dir) { FileUploader.root } let(:pages_dir) { File.join(TestEnv.pages_path) } - def expect_project_directories_at(namespace_path) + def expect_project_directories_at(namespace_path, with_pages: true) expected_repository_path = File.join(TestEnv.repos_path, namespace_path, 'the-project.git') expected_upload_path = File.join(uploads_dir, namespace_path, 'the-project') expected_pages_path = File.join(pages_dir, namespace_path, 'the-project') expect(File.directory?(expected_repository_path)).to be_truthy expect(File.directory?(expected_upload_path)).to be_truthy - expect(File.directory?(expected_pages_path)).to be_truthy + expect(File.directory?(expected_pages_path)).to be(with_pages) end before do @@ -412,7 +412,7 @@ RSpec.describe Namespace do FileUtils.remove_entry(File.join(TestEnv.repos_path, new_parent.full_path), true) FileUtils.remove_entry(File.join(TestEnv.repos_path, child.full_path), true) FileUtils.remove_entry(File.join(uploads_dir, project.full_path), true) - FileUtils.remove_entry(File.join(pages_dir, project.full_path), true) + FileUtils.remove_entry(pages_dir, true) end context 'renaming child' do @@ -426,10 +426,22 @@ RSpec.describe Namespace do end end - it 'correctly moves the repository, uploads and pages', :sidekiq_inline do - child.update!(path: 'renamed') + context 'when no projects have pages deployed' do + it 'moves the repository and uploads', :sidekiq_inline do + project.pages_metadatum.update!(deployed: false) + child.update!(path: 'renamed') + + expect_project_directories_at('parent/renamed', with_pages: false) + end + end + + context 'when the project has pages deployed' do + it 'correctly moves the repository, uploads and pages', :sidekiq_inline do + project.pages_metadatum.update!(deployed: true) + child.update!(path: 'renamed') - expect_project_directories_at('parent/renamed') + expect_project_directories_at('parent/renamed') + end end end @@ -444,10 +456,22 @@ RSpec.describe Namespace do end end - it 'correctly moves the repository, uploads and pages', :sidekiq_inline do - parent.update!(path: 'renamed') + context 'when no projects have pages deployed' do + it 'moves the repository and uploads', :sidekiq_inline do + project.pages_metadatum.update!(deployed: false) + parent.update!(path: 'renamed') + + expect_project_directories_at('renamed/child', with_pages: false) + end + end + + context 'when the project has pages deployed' do + it 'correctly moves the repository, uploads and pages', :sidekiq_inline do + project.pages_metadatum.update!(deployed: true) + parent.update!(path: 'renamed') - expect_project_directories_at('renamed/child') + expect_project_directories_at('renamed/child') + end end end @@ -462,10 +486,22 @@ RSpec.describe Namespace do end end - it 'correctly moves the repository, uploads and pages', :sidekiq_inline do - child.update!(parent: new_parent) + context 'when no projects have pages deployed' do + it 'moves the repository and uploads', :sidekiq_inline do + project.pages_metadatum.update!(deployed: false) + child.update!(parent: new_parent) + + expect_project_directories_at('new_parent/child', with_pages: false) + end + end + + context 'when the project has pages deployed' do + it 'correctly moves the repository, uploads and pages', :sidekiq_inline do + project.pages_metadatum.update!(deployed: true) + child.update!(parent: new_parent) - expect_project_directories_at('new_parent/child') + expect_project_directories_at('new_parent/child') + end end end @@ -480,10 +516,22 @@ RSpec.describe Namespace do end end - it 'correctly moves the repository, uploads and pages', :sidekiq_inline do - child.update!(parent: nil) + context 'when no projects have pages deployed' do + it 'moves the repository and uploads', :sidekiq_inline do + project.pages_metadatum.update!(deployed: false) + child.update!(parent: nil) + + expect_project_directories_at('child', with_pages: false) + end + end + + context 'when the project has pages deployed' do + it 'correctly moves the repository, uploads and pages', :sidekiq_inline do + project.pages_metadatum.update!(deployed: true) + child.update!(parent: nil) - expect_project_directories_at('child') + expect_project_directories_at('child') + end end end @@ -498,10 +546,22 @@ RSpec.describe Namespace do end end - it 'correctly moves the repository, uploads and pages', :sidekiq_inline do - parent.update!(parent: new_parent) + context 'when no projects have pages deployed' do + it 'moves the repository and uploads', :sidekiq_inline do + project.pages_metadatum.update!(deployed: false) + parent.update!(parent: new_parent) - expect_project_directories_at('new_parent/parent/child') + expect_project_directories_at('new_parent/parent/child', with_pages: false) + end + end + + context 'when the project has pages deployed' do + it 'correctly moves the repository, uploads and pages', :sidekiq_inline do + project.pages_metadatum.update!(deployed: true) + parent.update!(parent: new_parent) + + expect_project_directories_at('new_parent/parent/child') + end end end end @@ -1174,6 +1234,27 @@ RSpec.describe Namespace do end end + describe '#any_project_with_pages_deployed?' do + it 'returns true if any project nested under the group has pages deployed' do + parent_1 = create(:group) # Three projects, one with pages + child_1_1 = create(:group, parent: parent_1) # Two projects, one with pages + child_1_2 = create(:group, parent: parent_1) # One project, no pages + parent_2 = create(:group) # No projects + + create(:project, group: child_1_1).tap do |project| + project.pages_metadatum.update!(deployed: true) + end + + create(:project, group: child_1_1) + create(:project, group: child_1_2) + + expect(parent_1.any_project_with_pages_deployed?).to be(true) + expect(child_1_1.any_project_with_pages_deployed?).to be(true) + expect(child_1_2.any_project_with_pages_deployed?).to be(false) + expect(parent_2.any_project_with_pages_deployed?).to be(false) + end + end + describe '#has_parent?' do it 'returns true when the group has a parent' do group = create(:group, :nested) diff --git a/spec/models/performance_monitoring/prometheus_dashboard_spec.rb b/spec/models/performance_monitoring/prometheus_dashboard_spec.rb index 61174a7d0c5..634690d5d0b 100644 --- a/spec/models/performance_monitoring/prometheus_dashboard_spec.rb +++ b/spec/models/performance_monitoring/prometheus_dashboard_spec.rb @@ -219,20 +219,93 @@ RSpec.describe PerformanceMonitoring::PrometheusDashboard do 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 + let(:environment) { create(:environment, project: project) } + let(:path) { '.gitlab/dashboards/test.yml' } + let(:project) { create(:project, :repository, :custom_repo, files: { path => dashboard_schema.to_yaml }) } + + subject(:schema_validation_warnings) { described_class.new(dashboard_schema.merge(path: path, environment: environment)).schema_validation_warnings } + + before do + allow(Gitlab::Metrics::Dashboard::Finder).to receive(:find_raw).with(project, dashboard_path: path).and_call_original + end + + context 'metrics_dashboard_exhaustive_validations is on' do + before do + stub_feature_flags(metrics_dashboard_exhaustive_validations: true) + end + + context 'when schema is valid' do + let(:dashboard_schema) { YAML.safe_load(fixture_file('lib/gitlab/metrics/dashboard/sample_dashboard.yml')) } + + it 'returns empty array' do + expect(Gitlab::Metrics::Dashboard::Validator).to receive(:errors).with(dashboard_schema, dashboard_path: path, project: project).and_return([]) + + expect(schema_validation_warnings).to eq [] + end + end + + context 'when schema is invalid' do + let(:dashboard_schema) { YAML.safe_load(fixture_file('lib/gitlab/metrics/dashboard/dashboard_missing_panel_groups.yml')) } + + it 'returns array with errors messages' do + error = ::Gitlab::Metrics::Dashboard::Validator::Errors::SchemaValidationError.new + + expect(Gitlab::Metrics::Dashboard::Validator).to receive(:errors).with(dashboard_schema, dashboard_path: path, project: project).and_return([error]) + + expect(schema_validation_warnings).to eq [error.message] + end + end + + context 'when YAML has wrong syntax' do + let(:project) { create(:project, :repository, :custom_repo, files: { path => fixture_file('lib/gitlab/metrics/dashboard/broken_yml_syntax.yml') }) } + + subject(:schema_validation_warnings) { described_class.new(path: path, environment: environment).schema_validation_warnings } + + it 'returns array with errors messages' do + expect(Gitlab::Metrics::Dashboard::Validator).not_to receive(:errors) + + expect(schema_validation_warnings).to eq ['Invalid yaml'] + end 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') + context 'metrics_dashboard_exhaustive_validations is off' do + before do + stub_feature_flags(metrics_dashboard_exhaustive_validations: false) + end + + context 'when schema is valid' do + let(:dashboard_schema) { YAML.safe_load(fixture_file('lib/gitlab/metrics/dashboard/sample_dashboard.yml')) } + + it 'returns empty array' do + expect(described_class).to receive(:from_json).with(dashboard_schema) + + expect(schema_validation_warnings).to eq [] + end + end + + context 'when schema is invalid' do + let(:dashboard_schema) { YAML.safe_load(fixture_file('lib/gitlab/metrics/dashboard/dashboard_missing_panel_groups.yml')) } + + 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 - 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'] + context 'when YAML has wrong syntax' do + let(:project) { create(:project, :repository, :custom_repo, files: { path => fixture_file('lib/gitlab/metrics/dashboard/broken_yml_syntax.yml') }) } + + subject(:schema_validation_warnings) { described_class.new(path: path, environment: environment).schema_validation_warnings } + + it 'returns array with errors messages' do + expect(described_class).not_to receive(:from_json) + + expect(schema_validation_warnings).to eq ['Invalid yaml'] + end end end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index f9b819e22cd..9cd666e541f 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -1648,7 +1648,7 @@ RSpec.describe User do # add user to project project.add_maintainer(user) - # create invite to projet + # create invite to project create(:project_member, :developer, project: project, invite_token: '1234', invite_email: 'inviteduser1@example.com') # create request to join project diff --git a/spec/requests/api/conan_packages_spec.rb b/spec/requests/api/conan_packages_spec.rb index 5ad7fe5d579..6569ce91bb8 100644 --- a/spec/requests/api/conan_packages_spec.rb +++ b/spec/requests/api/conan_packages_spec.rb @@ -1,4 +1,5 @@ # frozen_string_literal: true + require 'spec_helper' RSpec.describe API::ConanPackages do diff --git a/spec/requests/api/generic_packages_spec.rb b/spec/requests/api/generic_packages_spec.rb new file mode 100644 index 00000000000..e08637629cc --- /dev/null +++ b/spec/requests/api/generic_packages_spec.rb @@ -0,0 +1,82 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe API::GenericPackages do + let_it_be(:personal_access_token) { create(:personal_access_token) } + let_it_be(:project) { create(:project) } + + describe 'GET /api/v4/projects/:id/packages/generic/ping' do + let(:user) { personal_access_token.user } + let(:auth_token) { personal_access_token.token } + + before do + project.add_developer(user) + end + + context 'packages feature is disabled' do + it 'responds with 404 Not Found' do + stub_packages_setting(enabled: false) + + ping(personal_access_token: auth_token) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'generic_packages feature flag is disabled' do + it 'responds with 404 Not Found' do + stub_feature_flags(generic_packages: false) + + ping(personal_access_token: auth_token) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'generic_packages feature flag is enabled' do + before do + stub_feature_flags(generic_packages: true) + end + + context 'authenticating using personal access token' do + it 'responds with 200 OK when valid personal access token is provided' do + ping(personal_access_token: auth_token) + + expect(response).to have_gitlab_http_status(:ok) + end + + it 'responds with 401 Unauthorized when invalid personal access token provided' do + ping(personal_access_token: 'invalid-token') + + expect(response).to have_gitlab_http_status(:unauthorized) + end + end + + context 'authenticating using job token' do + it 'responds with 200 OK when valid job token is provided' do + job_token = create(:ci_build, user: user).token + + ping(job_token: job_token) + + expect(response).to have_gitlab_http_status(:ok) + end + + it 'responds with 401 Unauthorized when invalid job token provided' do + ping(job_token: 'invalid-token') + + expect(response).to have_gitlab_http_status(:unauthorized) + end + end + end + + def ping(personal_access_token: nil, job_token: nil) + headers = { + Gitlab::Auth::AuthFinders::PRIVATE_TOKEN_HEADER => personal_access_token.presence, + Gitlab::Auth::AuthFinders::JOB_TOKEN_HEADER => job_token.presence + }.compact + + get api('/projects/%d/packages/generic/ping' % project.id), headers: headers + end + end +end diff --git a/spec/requests/api/graphql/metrics/dashboard_query_spec.rb b/spec/requests/api/graphql/metrics/dashboard_query_spec.rb index 456b0a5dea1..e01f59ee6a0 100644 --- a/spec/requests/api/graphql/metrics/dashboard_query_spec.rb +++ b/spec/requests/api/graphql/metrics/dashboard_query_spec.rb @@ -7,7 +7,7 @@ RSpec.describe 'Getting Metrics Dashboard' do let_it_be(:current_user) { create(:user) } let(:project) { create(:project) } - let!(:environment) { create(:environment, project: project) } + let(:environment) { create(:environment, project: project) } let(:query) do graphql_query_for( @@ -25,73 +25,156 @@ RSpec.describe 'Getting Metrics Dashboard' do ) end - context 'for anonymous user' do + context 'with metrics_dashboard_exhaustive_validations feature flag off' do before do - post_graphql(query, current_user: current_user) + stub_feature_flags(metrics_dashboard_exhaustive_validations: false) end - context 'requested dashboard is available' do - let(:path) { 'config/prometheus/common_metrics.yml' } + context 'for anonymous user' do + before do + post_graphql(query, current_user: current_user) + end + + context 'requested dashboard is available' do + let(:path) { 'config/prometheus/common_metrics.yml' } + + it_behaves_like 'a working graphql query' + + it 'returns nil' do + dashboard = graphql_data.dig('project', 'environments', 'nodes') + + expect(dashboard).to be_nil + end + end + end + + context 'for user with developer access' do + before do + project.add_developer(current_user) + post_graphql(query, current_user: current_user) + end + + context 'requested dashboard is available' do + let(:path) { 'config/prometheus/common_metrics.yml' } + + it_behaves_like 'a working graphql query' + + it 'returns metrics dashboard' do + dashboard = graphql_data.dig('project', 'environments', 'nodes', 0, 'metricsDashboard') + + expect(dashboard).to eql("path" => path, "schemaValidationWarnings" => nil) + end + + context 'invalid dashboard' do + let(:path) { '.gitlab/dashboards/metrics.yml' } + let(:project) { create(:project, :repository, :custom_repo, namespace: current_user.namespace, files: { path => "---\ndashboard: 'test'" }) } + + it 'returns metrics dashboard' do + dashboard = graphql_data.dig('project', 'environments', 'nodes', 0, 'metricsDashboard') + + expect(dashboard).to eql("path" => path, "schemaValidationWarnings" => ["panel_groups: should be an array of panel_groups objects"]) + end + end + + context 'empty dashboard' do + let(:path) { '.gitlab/dashboards/metrics.yml' } + let(:project) { create(:project, :repository, :custom_repo, namespace: current_user.namespace, files: { path => "" }) } + + it 'returns metrics dashboard' do + dashboard = graphql_data.dig('project', 'environments', 'nodes', 0, 'metricsDashboard') - it_behaves_like 'a working graphql query' + expect(dashboard).to eql("path" => path, "schemaValidationWarnings" => ["dashboard: can't be blank", "panel_groups: should be an array of panel_groups objects"]) + end + end + end + + context 'requested dashboard can not be found' do + let(:path) { 'config/prometheus/i_am_not_here.yml' } - it 'returns nil' do - dashboard = graphql_data.dig('project', 'environments', 'nodes') + it_behaves_like 'a working graphql query' - expect(dashboard).to be_nil + it 'returns nil' do + dashboard = graphql_data.dig('project', 'environments', 'nodes', 0, 'metricsDashboard') + + expect(dashboard).to be_nil + end end end end - context 'for user with developer access' do + context 'with metrics_dashboard_exhaustive_validations feature flag on' do before do - project.add_developer(current_user) - post_graphql(query, current_user: current_user) + stub_feature_flags(metrics_dashboard_exhaustive_validations: true) end - context 'requested dashboard is available' do - let(:path) { 'config/prometheus/common_metrics.yml' } + context 'for anonymous user' do + before do + post_graphql(query, current_user: current_user) + end + + context 'requested dashboard is available' do + let(:path) { 'config/prometheus/common_metrics.yml' } + + it_behaves_like 'a working graphql query' - it_behaves_like 'a working graphql query' + it 'returns nil' do + dashboard = graphql_data.dig('project', 'environments', 'nodes') - it 'returns metrics dashboard' do - dashboard = graphql_data.dig('project', 'environments', 'nodes')[0]['metricsDashboard'] + expect(dashboard).to be_nil + end + end + end - expect(dashboard).to eql("path" => path, "schemaValidationWarnings" => nil) + context 'for user with developer access' do + before do + project.add_developer(current_user) + post_graphql(query, current_user: current_user) end - context 'invalid dashboard' do - let(:path) { '.gitlab/dashboards/metrics.yml' } - let(:project) { create(:project, :repository, :custom_repo, namespace: current_user.namespace, files: { path => "---\ndashboard: 'test'" }) } + context 'requested dashboard is available' do + let(:path) { 'config/prometheus/common_metrics.yml' } + + it_behaves_like 'a working graphql query' it 'returns metrics dashboard' do dashboard = graphql_data.dig('project', 'environments', 'nodes', 0, 'metricsDashboard') - expect(dashboard).to eql("path" => path, "schemaValidationWarnings" => ["panel_groups: should be an array of panel_groups objects"]) + expect(dashboard).to eql("path" => path, "schemaValidationWarnings" => nil) end - end - context 'empty dashboard' do - let(:path) { '.gitlab/dashboards/metrics.yml' } - let(:project) { create(:project, :repository, :custom_repo, namespace: current_user.namespace, files: { path => "" }) } + context 'invalid dashboard' do + let(:path) { '.gitlab/dashboards/metrics.yml' } + let(:project) { create(:project, :repository, :custom_repo, namespace: current_user.namespace, files: { path => "---\ndashboard: 'test'" }) } - it 'returns metrics dashboard' do - dashboard = graphql_data.dig('project', 'environments', 'nodes', 0, 'metricsDashboard') + it 'returns metrics dashboard' do + dashboard = graphql_data.dig('project', 'environments', 'nodes', 0, 'metricsDashboard') - expect(dashboard).to eql("path" => path, "schemaValidationWarnings" => ["dashboard: can't be blank", "panel_groups: should be an array of panel_groups objects"]) + expect(dashboard).to eql("path" => path, "schemaValidationWarnings" => ["root is missing required keys: panel_groups"]) + end + end + + context 'empty dashboard' do + let(:path) { '.gitlab/dashboards/metrics.yml' } + let(:project) { create(:project, :repository, :custom_repo, namespace: current_user.namespace, files: { path => "" }) } + + it 'returns metrics dashboard' do + dashboard = graphql_data.dig('project', 'environments', 'nodes', 0, 'metricsDashboard') + + expect(dashboard).to eql("path" => path, "schemaValidationWarnings" => ["root is missing required keys: dashboard, panel_groups"]) + end end end - end - context 'requested dashboard can not be found' do - let(:path) { 'config/prometheus/i_am_not_here.yml' } + context 'requested dashboard can not be found' do + let(:path) { 'config/prometheus/i_am_not_here.yml' } - it_behaves_like 'a working graphql query' + it_behaves_like 'a working graphql query' - it 'returns nil' do - dashboard = graphql_data.dig('project', 'environments', 'nodes')[0]['metricsDashboard'] + it 'returns nil' do + dashboard = graphql_data.dig('project', 'environments', 'nodes', 0, 'metricsDashboard') - expect(dashboard).to be_nil + expect(dashboard).to be_nil + end end end end diff --git a/spec/requests/api/graphql/mutations/ci/pipeline_cancel_spec.rb b/spec/requests/api/graphql/mutations/ci/pipeline_cancel_spec.rb new file mode 100644 index 00000000000..5a4dc3ce187 --- /dev/null +++ b/spec/requests/api/graphql/mutations/ci/pipeline_cancel_spec.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'PipelineCancel' do + include GraphqlHelpers + + let_it_be(:user) { create(:user) } + let_it_be(:project) { create(:project) } + let_it_be(:pipeline) { create(:ci_pipeline, :running, project: project, user: user) } + + let(:mutation) { graphql_mutation(:pipeline_cancel, {}, 'errors') } + + let(:mutation_response) { graphql_mutation_response(:pipeline_cancel) } + + before_all do + project.add_maintainer(user) + end + + it 'reports the service-level error' do + service = double(execute: ServiceResponse.error(message: 'Error canceling pipeline')) + allow(::Ci::CancelUserPipelinesService).to receive(:new).and_return(service) + + post_graphql_mutation(mutation, current_user: create(:user)) + + expect(mutation_response).to include('errors' => ['Error canceling pipeline']) + end + + it 'does not change any pipelines not owned by the current user' do + build = create(:ci_build, :running, pipeline: pipeline) + + post_graphql_mutation(mutation, current_user: create(:user)) + + expect(build).not_to be_canceled + end + + it "cancels all of the current user's cancelable pipelines" do + build = create(:ci_build, :running, pipeline: pipeline) + + post_graphql_mutation(mutation, current_user: user) + + expect(response).to have_gitlab_http_status(:success) + expect(build.reload).to be_canceled + end +end diff --git a/spec/requests/api/graphql/mutations/ci/pipeline_destroy_spec.rb b/spec/requests/api/graphql/mutations/ci/pipeline_destroy_spec.rb new file mode 100644 index 00000000000..08959d354e2 --- /dev/null +++ b/spec/requests/api/graphql/mutations/ci/pipeline_destroy_spec.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'PipelineDestroy' do + include GraphqlHelpers + + let_it_be(:project) { create(:project) } + let_it_be(:user) { project.owner } + let_it_be(:pipeline) { create(:ci_pipeline, :success, project: project, user: user) } + + let(:mutation) do + variables = { + id: pipeline.to_global_id.to_s + } + graphql_mutation(:pipeline_destroy, variables, 'errors') + end + + it 'returns an error if the user is not allowed to destroy the pipeline' do + post_graphql_mutation(mutation, current_user: create(:user)) + + expect(graphql_errors).not_to be_empty + end + + it 'destroys a pipeline' do + post_graphql_mutation(mutation, current_user: user) + + expect(response).to have_gitlab_http_status(:success) + expect { pipeline.reload }.to raise_error(ActiveRecord::RecordNotFound) + end +end diff --git a/spec/requests/api/graphql/mutations/ci/pipeline_retry_spec.rb b/spec/requests/api/graphql/mutations/ci/pipeline_retry_spec.rb new file mode 100644 index 00000000000..f6acf29c321 --- /dev/null +++ b/spec/requests/api/graphql/mutations/ci/pipeline_retry_spec.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'PipelineRetry' do + include GraphqlHelpers + + let_it_be(:user) { create(:user) } + let_it_be(:project) { create(:project) } + let_it_be(:pipeline) { create(:ci_pipeline, project: project, user: user) } + + let(:mutation) do + variables = { + id: pipeline.to_global_id.to_s + } + graphql_mutation(:pipeline_retry, variables, + <<-QL + errors + pipeline { + id + } + QL + ) + end + + let(:mutation_response) { graphql_mutation_response(:pipeline_retry) } + + before_all do + project.add_maintainer(user) + end + + it 'returns an error if the user is not allowed to retry the pipeline' do + post_graphql_mutation(mutation, current_user: create(:user)) + + expect(graphql_errors).not_to be_empty + end + + it 'retries a pipeline' do + pipeline_id = ::Gitlab::GlobalId.build(pipeline, id: pipeline.id).to_s + post_graphql_mutation(mutation, current_user: user) + + expect(response).to have_gitlab_http_status(:success) + expect(mutation_response['pipeline']['id']).to eq(pipeline_id) + end +end diff --git a/spec/services/admin/propagate_integration_service_spec.rb b/spec/services/admin/propagate_integration_service_spec.rb index 2e879cf06d1..aded0f08ae8 100644 --- a/spec/services/admin/propagate_integration_service_spec.rb +++ b/spec/services/admin/propagate_integration_service_spec.rb @@ -43,7 +43,7 @@ RSpec.describe Admin::PropagateIntegrationService do ) end - let!(:another_inherited_integration) do + let!(:different_type_inherited_integration) do BambooService.create!( project: create(:project), inherit_from_id: instance_integration.id, @@ -59,7 +59,7 @@ RSpec.describe Admin::PropagateIntegrationService do shared_examples 'inherits settings from integration' do it 'updates the inherited integrations' do - described_class.propagate(integration: instance_integration, overwrite: overwrite) + described_class.propagate(instance_integration) expect(integration.reload.inherit_from_id).to eq(instance_integration.id) expect(integration.attributes.except(*excluded_attributes)) @@ -70,7 +70,7 @@ RSpec.describe Admin::PropagateIntegrationService do let(:excluded_attributes) { %w[id service_id created_at updated_at] } it 'updates the data fields from inherited integrations' do - described_class.propagate(integration: instance_integration, overwrite: overwrite) + described_class.propagate(instance_integration) expect(integration.reload.data_fields.attributes.except(*excluded_attributes)) .to eq(instance_integration.data_fields.attributes.except(*excluded_attributes)) @@ -80,7 +80,7 @@ RSpec.describe Admin::PropagateIntegrationService do shared_examples 'does not inherit settings from integration' do it 'does not update the not inherited integrations' do - described_class.propagate(integration: instance_integration, overwrite: overwrite) + described_class.propagate(instance_integration) expect(integration.reload.attributes.except(*excluded_attributes)) .not_to eq(instance_integration.attributes.except(*excluded_attributes)) @@ -88,8 +88,6 @@ RSpec.describe Admin::PropagateIntegrationService do end context 'update only inherited integrations' do - let(:overwrite) { false } - it_behaves_like 'inherits settings from integration' do let(:integration) { inherited_integration } end @@ -99,27 +97,7 @@ RSpec.describe Admin::PropagateIntegrationService do end it_behaves_like 'does not inherit settings from integration' do - let(:integration) { another_inherited_integration } - end - - it_behaves_like 'inherits settings from integration' do - let(:integration) { project.jira_service } - end - end - - context 'update all integrations' do - let(:overwrite) { true } - - it_behaves_like 'inherits settings from integration' do - let(:integration) { inherited_integration } - end - - it_behaves_like 'inherits settings from integration' do - let(:integration) { not_inherited_integration } - end - - it_behaves_like 'does not inherit settings from integration' do - let(:integration) { another_inherited_integration } + let(:integration) { different_type_inherited_integration } end it_behaves_like 'inherits settings from integration' do @@ -128,7 +106,7 @@ RSpec.describe Admin::PropagateIntegrationService do end it 'updates project#has_external_issue_tracker for issue tracker services' do - described_class.propagate(integration: instance_integration, overwrite: true) + described_class.propagate(instance_integration) expect(project.reload.has_external_issue_tracker).to eq(true) end @@ -141,7 +119,7 @@ RSpec.describe Admin::PropagateIntegrationService do external_wiki_url: 'http://external-wiki-url.com' ) - described_class.propagate(integration: instance_integration, overwrite: true) + described_class.propagate(instance_integration) expect(project.reload.has_external_wiki).to eq(true) end diff --git a/spec/services/ci/cancel_user_pipelines_service_spec.rb b/spec/services/ci/cancel_user_pipelines_service_spec.rb index 12117051b64..8491242dfd5 100644 --- a/spec/services/ci/cancel_user_pipelines_service_spec.rb +++ b/spec/services/ci/cancel_user_pipelines_service_spec.rb @@ -19,5 +19,17 @@ RSpec.describe Ci::CancelUserPipelinesService do expect(build.reload).to be_canceled end end + + context 'when an error ocurrs' do + it 'raises a service level error' do + service = double(execute: ServiceResponse.error(message: 'Error canceling pipeline')) + allow(::Ci::CancelUserPipelinesService).to receive(:new).and_return(service) + + result = subject + + expect(result).to be_a(ServiceResponse) + expect(result).to be_error + end + end end end diff --git a/spec/services/ci/generate_coverage_reports_service_spec.rb b/spec/services/ci/generate_coverage_reports_service_spec.rb index 76d446c1d2c..722b92ea3b6 100644 --- a/spec/services/ci/generate_coverage_reports_service_spec.rb +++ b/spec/services/ci/generate_coverage_reports_service_spec.rb @@ -25,7 +25,7 @@ RSpec.describe Ci::GenerateCoverageReportsService do end end - context 'when head pipeline has corrupted coverage reports' do + context 'when head pipeline does not have a coverage report artifact' do let!(:merge_request) { create(:merge_request, :with_coverage_reports, source_project: project) } let!(:service) { described_class.new(project, nil, id: merge_request.id) } let!(:head_pipeline) { merge_request.head_pipeline } diff --git a/spec/services/jira/requests/projects/list_service_spec.rb b/spec/services/jira/requests/projects/list_service_spec.rb index b4db77f8104..415dd42c795 100644 --- a/spec/services/jira/requests/projects/list_service_spec.rb +++ b/spec/services/jira/requests/projects/list_service_spec.rb @@ -69,7 +69,7 @@ RSpec.describe Jira::Requests::Projects::ListService do expect(client).to receive(:get).and_return([{ 'key' => 'pr1', 'name' => 'First Project' }, { 'key' => 'pr2', 'name' => 'Second Project' }]) end - it 'returns a paylod with Jira projets' do + it 'returns a paylod with Jira projects' do payload = subject.payload expect(subject.success?).to be_truthy @@ -80,7 +80,7 @@ RSpec.describe Jira::Requests::Projects::ListService do context 'when filtering projects by name' do let(:params) { { query: 'first' } } - it 'returns a paylod with Jira projets' do + it 'returns a paylod with Jira procjets' do payload = subject.payload expect(subject.success?).to be_truthy diff --git a/spec/services/projects/after_rename_service_spec.rb b/spec/services/projects/after_rename_service_spec.rb index 11e3604c38a..89971a3edfb 100644 --- a/spec/services/projects/after_rename_service_spec.rb +++ b/spec/services/projects/after_rename_service_spec.rb @@ -75,10 +75,25 @@ RSpec.describe Projects::AfterRenameService do end end - it 'schedules a move of the pages directory' do - expect(PagesTransferWorker).to receive(:perform_async).with('rename_project', anything) + context 'when the project has pages deployed' do + it 'schedules a move of the pages directory' do + allow(project).to receive(:pages_deployed?).and_return(true) - service_execute + expect(PagesTransferWorker).to receive(:perform_async).with('rename_project', anything) + + service_execute + end + end + + context 'when the project does not have pages deployed' do + it 'does nothing with the pages directory' do + allow(project).to receive(:pages_deployed?).and_return(false) + + expect(PagesTransferWorker).not_to receive(:perform_async) + expect(Gitlab::PagesTransfer).not_to receive(:new) + + service_execute + end end end @@ -180,10 +195,25 @@ RSpec.describe Projects::AfterRenameService do end end - it 'schedules a move of the pages directory' do - expect(PagesTransferWorker).to receive(:perform_async).with('rename_project', anything) + context 'when the project has pages deployed' do + it 'schedules a move of the pages directory' do + allow(project).to receive(:pages_deployed?).and_return(true) - service_execute + expect(PagesTransferWorker).to receive(:perform_async).with('rename_project', anything) + + service_execute + end + end + + context 'when the project does not have pages deployed' do + it 'does nothing with the pages directory' do + allow(project).to receive(:pages_deployed?).and_return(false) + + expect(PagesTransferWorker).not_to receive(:perform_async) + expect(Gitlab::PagesTransfer).not_to receive(:new) + + service_execute + end end end diff --git a/spec/services/projects/transfer_service_spec.rb b/spec/services/projects/transfer_service_spec.rb index 3362b333c6e..01af999f117 100644 --- a/spec/services/projects/transfer_service_spec.rb +++ b/spec/services/projects/transfer_service_spec.rb @@ -5,8 +5,8 @@ require 'spec_helper' RSpec.describe Projects::TransferService do include GitHelpers - let(:user) { create(:user) } - let(:group) { create(:group) } + let_it_be(:user) { create(:user) } + let_it_be(:group) { create(:group) } let(:project) { create(:project, :repository, :legacy_storage, namespace: user.namespace) } subject(:execute_transfer) { described_class.new(project, user).execute(group) } @@ -489,6 +489,43 @@ RSpec.describe Projects::TransferService do end end + context 'moving pages' do + let_it_be(:project) { create(:project, namespace: user.namespace) } + + before do + group.add_owner(user) + end + + it 'schedules a job when pages are deployed' do + project.mark_pages_as_deployed + + expect(PagesTransferWorker).to receive(:perform_async) + .with("move_project", [project.path, user.namespace.full_path, group.full_path]) + + execute_transfer + end + + it 'does not schedule a job when no pages are deployed' do + expect(PagesTransferWorker).not_to receive(:perform_async) + + execute_transfer + end + + context 'when async_pages_move_project_transfer is disabled' do + before do + stub_feature_flags(async_pages_move_project_transfer: false) + end + + it 'moves pages inline' do + expect_next_instance_of(Gitlab::PagesTransfer) do |transfer| + expect(transfer).to receive(:move_project).with(project.path, user.namespace.full_path, group.full_path) + end + + execute_transfer + end + end + end + def rugged_config rugged_repo(project.repository).config end diff --git a/spec/support/shared_examples/models/relative_positioning_shared_examples.rb b/spec/support/shared_examples/models/relative_positioning_shared_examples.rb index 18f4932907a..ebb91a2fbad 100644 --- a/spec/support/shared_examples/models/relative_positioning_shared_examples.rb +++ b/spec/support/shared_examples/models/relative_positioning_shared_examples.rb @@ -70,6 +70,37 @@ RSpec.shared_examples 'a class that supports relative positioning' do expect(items.sort_by(&:relative_position)).to eq(items) end + it 'manages to move nulls to the end even if there is not enough space' do + run = run_at_end(20).to_a + bunch_a = create_items_with_positions(run[0..18]) + bunch_b = create_items_with_positions([run.last]) + + nils = create_items_with_positions([nil] * 4) + described_class.move_nulls_to_end(nils) + + items = [*bunch_a, *bunch_b, *nils] + items.each(&:reset) + + expect(items.map(&:relative_position)).to all(be_valid_position) + expect(items.reverse.sort_by(&:relative_position)).to eq(items) + end + + it 'manages to move nulls to the end, stacking if we cannot create enough space' do + run = run_at_end(40).to_a + bunch = create_items_with_positions(run.select(&:even?)) + + nils = create_items_with_positions([nil] * 20) + described_class.move_nulls_to_end(nils) + + items = [*bunch, *nils] + items.each(&:reset) + + expect(items.map(&:relative_position)).to all(be_valid_position) + expect(bunch.reverse.sort_by(&:relative_position)).to eq(bunch) + expect(nils.reverse.sort_by(&:relative_position)).not_to eq(nils) + expect(bunch.map(&:relative_position)).to all(be < nils.map(&:relative_position).min) + end + it 'does not have an N+1 issue' do create_items_with_positions(10..12) @@ -130,6 +161,37 @@ RSpec.shared_examples 'a class that supports relative positioning' do expect(described_class.move_nulls_to_start([item1])).to be(0) expect(item1.reload.relative_position).to be(1) end + + it 'manages to move nulls to the start even if there is not enough space' do + run = run_at_start(20).to_a + bunch_a = create_items_with_positions([run.first]) + bunch_b = create_items_with_positions(run[2..]) + + nils = create_items_with_positions([nil, nil, nil, nil]) + described_class.move_nulls_to_start(nils) + + items = [*nils, *bunch_a, *bunch_b] + items.each(&:reset) + + expect(items.map(&:relative_position)).to all(be_valid_position) + expect(items.reverse.sort_by(&:relative_position)).to eq(items) + end + + it 'manages to move nulls to the end, stacking if we cannot create enough space' do + run = run_at_start(40).to_a + bunch = create_items_with_positions(run.select(&:even?)) + + nils = create_items_with_positions([nil].cycle.take(20)) + described_class.move_nulls_to_start(nils) + + items = [*nils, *bunch] + items.each(&:reset) + + expect(items.map(&:relative_position)).to all(be_valid_position) + expect(bunch.reverse.sort_by(&:relative_position)).to eq(bunch) + expect(nils.reverse.sort_by(&:relative_position)).not_to eq(nils) + expect(bunch.map(&:relative_position)).to all(be > nils.map(&:relative_position).max) + end end describe '#max_relative_position' do diff --git a/spec/support/shared_examples/services/wiki_pages/update_service_shared_examples.rb b/spec/support/shared_examples/services/wiki_pages/update_service_shared_examples.rb index 0191a6dfbc9..fd10dd4367e 100644 --- a/spec/support/shared_examples/services/wiki_pages/update_service_shared_examples.rb +++ b/spec/support/shared_examples/services/wiki_pages/update_service_shared_examples.rb @@ -19,8 +19,10 @@ RSpec.shared_examples 'WikiPages::UpdateService#execute' do |container_type| subject(:service) { described_class.new(container: container, current_user: user, params: opts) } it 'updates the wiki page' do - updated_page = service.execute(page) + response = service.execute(page) + updated_page = response.payload[:page] + expect(response).to be_success expect(updated_page).to be_valid expect(updated_page.message).to eq(opts[:message]) expect(updated_page.content).to eq(opts[:content]) @@ -81,7 +83,11 @@ RSpec.shared_examples 'WikiPages::UpdateService#execute' do |container_type| end it 'reports the error' do - expect(service.execute(page)).to be_invalid + response = service.execute(page) + page = response.payload[:page] + + expect(response).to be_error + expect(page).to be_invalid .and have_attributes(errors: be_present) end end diff --git a/spec/workers/propagate_integration_worker_spec.rb b/spec/workers/propagate_integration_worker_spec.rb index 3fe76f14750..b8c7f2bebe7 100644 --- a/spec/workers/propagate_integration_worker_spec.rb +++ b/spec/workers/propagate_integration_worker_spec.rb @@ -17,8 +17,13 @@ RSpec.describe PropagateIntegrationWorker do end it 'calls the propagate service with the integration' do - expect(Admin::PropagateIntegrationService).to receive(:propagate) - .with(integration: integration, overwrite: true) + expect(Admin::PropagateIntegrationService).to receive(:propagate).with(integration) + + subject.perform(integration.id) + end + + it 'ignores overwrite parameter from previous version' do + expect(Admin::PropagateIntegrationService).to receive(:propagate).with(integration) subject.perform(integration.id, true) end |