diff options
Diffstat (limited to 'spec')
23 files changed, 321 insertions, 113 deletions
diff --git a/spec/controllers/projects/pipelines_controller_spec.rb b/spec/controllers/projects/pipelines_controller_spec.rb index 3ddfdd478a3..a929eaeba3f 100644 --- a/spec/controllers/projects/pipelines_controller_spec.rb +++ b/spec/controllers/projects/pipelines_controller_spec.rb @@ -634,7 +634,8 @@ describe Projects::PipelinesController do it 'does not persist a pipeline' do expect { post_request }.not_to change { project.ci_pipelines.count } - expect(response).to have_gitlab_http_status(:ok) + expect(response).to have_gitlab_http_status(:bad_request) + expect(response).to render_template('new') end end diff --git a/spec/frontend/ci_variable_list/components/ci_variable_modal_spec.js b/spec/frontend/ci_variable_list/components/ci_variable_modal_spec.js index 8db6626fca3..058aca16ea0 100644 --- a/spec/frontend/ci_variable_list/components/ci_variable_modal_spec.js +++ b/spec/frontend/ci_variable_list/components/ci_variable_modal_spec.js @@ -1,9 +1,10 @@ import Vuex from 'vuex'; import { createLocalVue, shallowMount } from '@vue/test-utils'; -import { GlModal } from '@gitlab/ui'; +import { GlButton } from '@gitlab/ui'; import CiVariableModal from '~/ci_variable_list/components/ci_variable_modal.vue'; import createStore from '~/ci_variable_list/store'; import mockData from '../services/mock_data'; +import ModalStub from '../stubs'; const localVue = createLocalVue(); localVue.use(Vuex); @@ -15,12 +16,23 @@ describe('Ci variable modal', () => { const createComponent = () => { store = createStore(); wrapper = shallowMount(CiVariableModal, { + stubs: { + GlModal: ModalStub, + }, localVue, store, }); }; - const findModal = () => wrapper.find(GlModal); + const findModal = () => wrapper.find(ModalStub); + const addOrUpdateButton = index => + findModal() + .findAll(GlButton) + .at(index); + const deleteVariableButton = () => + findModal() + .findAll(GlButton) + .at(1); beforeEach(() => { createComponent(); @@ -32,7 +44,7 @@ describe('Ci variable modal', () => { }); it('button is disabled when no key/value pair are present', () => { - expect(findModal().props('actionPrimary').attributes.disabled).toBeTruthy(); + expect(addOrUpdateButton(1).attributes('disabled')).toBeTruthy(); }); describe('Adding a new variable', () => { @@ -42,11 +54,11 @@ describe('Ci variable modal', () => { }); it('button is enabled when key/value pair are present', () => { - expect(findModal().props('actionPrimary').attributes.disabled).toBeFalsy(); + expect(addOrUpdateButton(1).attributes('disabled')).toBeFalsy(); }); it('Add variable button dispatches addVariable action', () => { - findModal().vm.$emit('ok'); + addOrUpdateButton(1).vm.$emit('click'); expect(store.dispatch).toHaveBeenCalledWith('addVariable'); }); @@ -63,11 +75,11 @@ describe('Ci variable modal', () => { }); it('button text is Update variable when updating', () => { - expect(wrapper.vm.modalActionText).toBe('Update variable'); + expect(addOrUpdateButton(2).text()).toBe('Update variable'); }); it('Update variable button dispatches updateVariable with correct variable', () => { - findModal().vm.$emit('ok'); + addOrUpdateButton(2).vm.$emit('click'); expect(store.dispatch).toHaveBeenCalledWith( 'updateVariable', store.state.variableBeingEdited, @@ -80,7 +92,7 @@ describe('Ci variable modal', () => { }); it('dispatches deleteVariable with correct variable to delete', () => { - findModal().vm.$emit('secondary'); + deleteVariableButton().vm.$emit('click'); expect(store.dispatch).toHaveBeenCalledWith('deleteVariable', mockData.mockVariables[0]); }); }); diff --git a/spec/frontend/ci_variable_list/stubs.js b/spec/frontend/ci_variable_list/stubs.js new file mode 100644 index 00000000000..5769d6190f6 --- /dev/null +++ b/spec/frontend/ci_variable_list/stubs.js @@ -0,0 +1,14 @@ +const ModalStub = { + name: 'glmodal-stub', + template: ` + <div> + <slot></slot> + <slot name="modal-footer"></slot> + </div> + `, + methods: { + hide: jest.fn(), + }, +}; + +export default ModalStub; diff --git a/spec/lib/gitlab/background_migration/backfill_project_repositories_spec.rb b/spec/lib/gitlab/background_migration/backfill_project_repositories_spec.rb index d601b0f064d..510a0074554 100644 --- a/spec/lib/gitlab/background_migration/backfill_project_repositories_spec.rb +++ b/spec/lib/gitlab/background_migration/backfill_project_repositories_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' # rubocop:disable RSpec/FactoriesInMigrationSpecs -describe Gitlab::BackgroundMigration::BackfillProjectRepositories, schema: :latest do +describe Gitlab::BackgroundMigration::BackfillProjectRepositories do let(:group) { create(:group, name: 'foo', path: 'foo') } describe described_class::ShardFinder do diff --git a/spec/lib/gitlab/background_migration/legacy_upload_mover_spec.rb b/spec/lib/gitlab/background_migration/legacy_upload_mover_spec.rb index ff82a6580df..850ef48d44a 100644 --- a/spec/lib/gitlab/background_migration/legacy_upload_mover_spec.rb +++ b/spec/lib/gitlab/background_migration/legacy_upload_mover_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' # rubocop: disable RSpec/FactoriesInMigrationSpecs -describe Gitlab::BackgroundMigration::LegacyUploadMover, schema: :latest do +describe Gitlab::BackgroundMigration::LegacyUploadMover do let(:test_dir) { FileUploader.options['storage_path'] } let(:filename) { 'image.png' } diff --git a/spec/lib/gitlab/background_migration/legacy_uploads_migrator_spec.rb b/spec/lib/gitlab/background_migration/legacy_uploads_migrator_spec.rb index 0c0ce3acf0e..85187d039c1 100644 --- a/spec/lib/gitlab/background_migration/legacy_uploads_migrator_spec.rb +++ b/spec/lib/gitlab/background_migration/legacy_uploads_migrator_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' # rubocop: disable RSpec/FactoriesInMigrationSpecs -describe Gitlab::BackgroundMigration::LegacyUploadsMigrator, schema: :latest do +describe Gitlab::BackgroundMigration::LegacyUploadsMigrator do let(:test_dir) { FileUploader.options['storage_path'] } let!(:hashed_project) { create(:project) } diff --git a/spec/lib/gitlab/background_migration/merge_request_assignees_migration_progress_check_spec.rb b/spec/lib/gitlab/background_migration/merge_request_assignees_migration_progress_check_spec.rb index 0d8fc3e7c53..eecd290e3ca 100644 --- a/spec/lib/gitlab/background_migration/merge_request_assignees_migration_progress_check_spec.rb +++ b/spec/lib/gitlab/background_migration/merge_request_assignees_migration_progress_check_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe Gitlab::BackgroundMigration::MergeRequestAssigneesMigrationProgressCheck, schema: :latest do +describe Gitlab::BackgroundMigration::MergeRequestAssigneesMigrationProgressCheck do context 'rescheduling' do context 'when there are ongoing and no dead jobs' do it 'reschedules check' do diff --git a/spec/lib/gitlab/background_migration_spec.rb b/spec/lib/gitlab/background_migration_spec.rb index cff6a42d242..71959f54b38 100644 --- a/spec/lib/gitlab/background_migration_spec.rb +++ b/spec/lib/gitlab/background_migration_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe Gitlab::BackgroundMigration, schema: :latest do +describe Gitlab::BackgroundMigration do describe '.queue' do it 'returns background migration worker queue' do expect(described_class.queue) diff --git a/spec/lib/gitlab/bitbucket_import/importer_spec.rb b/spec/lib/gitlab/bitbucket_import/importer_spec.rb index b0d07c6e0b0..b95175efc0c 100644 --- a/spec/lib/gitlab/bitbucket_import/importer_spec.rb +++ b/spec/lib/gitlab/bitbucket_import/importer_spec.rb @@ -80,8 +80,7 @@ describe Gitlab::BitbucketImport::Importer do end let(:importer) { described_class.new(project) } - let(:gitlab_shell) { double } - + let(:sample) { RepoHelpers.sample_compare } let(:issues_statuses_sample_data) do { count: sample_issues_statuses.count, @@ -89,12 +88,6 @@ describe Gitlab::BitbucketImport::Importer do } end - let(:sample) { RepoHelpers.sample_compare } - - before do - allow(importer).to receive(:gitlab_shell) { gitlab_shell } - end - subject { described_class.new(project) } describe '#import_pull_requests' do @@ -316,7 +309,7 @@ describe Gitlab::BitbucketImport::Importer do describe 'wiki import' do it 'is skipped when the wiki exists' do expect(project.wiki).to receive(:repository_exists?) { true } - expect(importer.gitlab_shell).not_to receive(:import_wiki_repository) + expect(project.wiki.repository).not_to receive(:import_repository) importer.execute @@ -325,7 +318,7 @@ describe Gitlab::BitbucketImport::Importer do it 'imports to the project disk_path' do expect(project.wiki).to receive(:repository_exists?) { false } - expect(importer.gitlab_shell).to receive(:import_wiki_repository) + expect(project.wiki.repository).to receive(:import_repository) importer.execute diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index a2326728679..b706cad612a 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -2138,6 +2138,33 @@ describe Gitlab::Git::Repository, :seed_helper do end end + describe '#import_repository' do + let_it_be(:project) { create(:project) } + + let(:repository) { project.repository } + let(:url) { 'http://invalid.invalid' } + + it 'raises an error if a relative path is provided' do + expect { repository.import_repository('/foo') }.to raise_error(ArgumentError, /disk path/) + end + + it 'raises an error if an absolute path is provided' do + expect { repository.import_repository('./foo') }.to raise_error(ArgumentError, /disk path/) + end + + it 'delegates to Gitaly' do + expect_next_instance_of(Gitlab::GitalyClient::RepositoryService) do |svc| + expect(svc).to receive(:import_repository).with(url).and_return(nil) + end + + repository.import_repository(url) + end + + it_behaves_like 'wrapping gRPC errors', Gitlab::GitalyClient::RepositoryService, :import_repository do + subject { repository.import_repository('http://invalid.invalid') } + end + end + describe '#replicate' do let(:new_repository) do Gitlab::Git::Repository.new('test_second_storage', TEST_REPO_PATH, '', 'group/project') diff --git a/spec/lib/gitlab/github_import/importer/repository_importer_spec.rb b/spec/lib/gitlab/github_import/importer/repository_importer_spec.rb index c65b28fafbf..e26ac7bf81e 100644 --- a/spec/lib/gitlab/github_import/importer/repository_importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer/repository_importer_spec.rb @@ -11,10 +11,15 @@ describe Gitlab::GithubImport::Importer::RepositoryImporter do double( :wiki, disk_path: 'foo.wiki', - full_path: 'group/foo.wiki' + full_path: 'group/foo.wiki', + repository: wiki_repository ) end + let(:wiki_repository) do + double(:wiki_repository) + end + let(:project) do double( :project, @@ -221,17 +226,19 @@ describe Gitlab::GithubImport::Importer::RepositoryImporter do describe '#import_wiki_repository' do it 'imports the wiki repository' do - expect(importer.gitlab_shell) + expect(wiki_repository) .to receive(:import_repository) - .with('foo', 'foo.wiki', 'foo.wiki.git', 'group/foo.wiki') + .with(importer.wiki_url) + .and_return(true) expect(importer.import_wiki_repository).to eq(true) end it 'marks the import as failed and creates an empty repo if an error was raised' do - expect(importer.gitlab_shell) + expect(wiki_repository) .to receive(:import_repository) - .and_raise(Gitlab::Shell::Error) + .with(importer.wiki_url) + .and_raise(Gitlab::Git::CommandError) expect(importer) .to receive(:fail_import) diff --git a/spec/lib/gitlab/legacy_github_import/importer_spec.rb b/spec/lib/gitlab/legacy_github_import/importer_spec.rb index 23df970957a..af0bffa91a5 100644 --- a/spec/lib/gitlab/legacy_github_import/importer_spec.rb +++ b/spec/lib/gitlab/legacy_github_import/importer_spec.rb @@ -45,7 +45,7 @@ describe Gitlab::LegacyGithubImport::Importer do allow(Rails).to receive(:cache).and_return(ActiveSupport::Cache::MemoryStore.new) allow_any_instance_of(Octokit::Client).to receive(:rate_limit!).and_raise(Octokit::NotFound) - allow_any_instance_of(Gitlab::Shell).to receive(:import_repository).and_raise(Gitlab::Shell::Error) + allow(project.wiki.repository).to receive(:import_repository).and_raise(Gitlab::Git::CommandError) allow_any_instance_of(Octokit::Client).to receive(:user).and_return(octocat) allow_any_instance_of(Octokit::Client).to receive(:labels).and_return([label1, label2]) @@ -169,7 +169,7 @@ describe Gitlab::LegacyGithubImport::Importer do errors: [ { type: :label, url: "#{api_root}/repos/octocat/Hello-World/labels/bug", errors: "Validation failed: Title can't be blank, Title is invalid" }, { type: :issue, url: "#{api_root}/repos/octocat/Hello-World/issues/1348", errors: "Validation failed: Title can't be blank" }, - { type: :wiki, errors: "Gitlab::Shell::Error" } + { type: :wiki, errors: "Gitlab::Git::CommandError" } ] } diff --git a/spec/lib/gitlab/shell_spec.rb b/spec/lib/gitlab/shell_spec.rb index 3ea9f71d3a6..e4c33863ac2 100644 --- a/spec/lib/gitlab/shell_spec.rb +++ b/spec/lib/gitlab/shell_spec.rb @@ -9,7 +9,6 @@ describe Gitlab::Shell do let(:gitlab_shell) { described_class.new } it { is_expected.to respond_to :remove_repository } - it { is_expected.to respond_to :fork_repository } describe '.url_to_repo' do let(:full_path) { 'diaspora/disaspora-rails' } @@ -95,52 +94,6 @@ describe Gitlab::Shell do expect(TestEnv.storage_dir_exists?(project2.repository_storage, "#{project2.disk_path}.git")).to be(true) end end - - describe '#fork_repository' do - let(:target_project) { create(:project) } - - subject do - gitlab_shell.fork_repository(project, target_project) - end - - it 'returns true when the command succeeds' do - expect_any_instance_of(Gitlab::GitalyClient::RepositoryService).to receive(:fork_repository) - .with(repository.raw_repository) { :gitaly_response_object } - - is_expected.to be_truthy - end - - it 'return false when the command fails' do - expect_any_instance_of(Gitlab::GitalyClient::RepositoryService).to receive(:fork_repository) - .with(repository.raw_repository) { raise GRPC::BadStatus, 'bla' } - - is_expected.to be_falsy - end - end - - describe '#import_repository' do - let(:import_url) { 'https://gitlab.com/gitlab-org/gitlab-foss.git' } - - context 'with gitaly' do - it 'returns true when the command succeeds' do - expect_any_instance_of(Gitlab::GitalyClient::RepositoryService).to receive(:import_repository).with(import_url) - - result = gitlab_shell.import_repository(project.repository_storage, project.disk_path, import_url, project.full_path) - - expect(result).to be_truthy - end - - it 'raises an exception when the command fails' do - expect_any_instance_of(Gitlab::GitalyClient::RepositoryService).to receive(:import_repository) - .with(import_url) { raise GRPC::BadStatus, 'bla' } - expect_any_instance_of(Gitlab::Shell::GitalyGitlabProjects).to receive(:output) { 'error'} - - expect do - gitlab_shell.import_repository(project.repository_storage, project.disk_path, import_url, project.full_path) - end.to raise_error(Gitlab::Shell::Error, "error") - end - end - end end describe 'namespace actions' do diff --git a/spec/lib/gitlab/sidekiq_cluster/cli_spec.rb b/spec/lib/gitlab/sidekiq_cluster/cli_spec.rb index 8b31b8ade68..5bda8ff8c72 100644 --- a/spec/lib/gitlab/sidekiq_cluster/cli_spec.rb +++ b/spec/lib/gitlab/sidekiq_cluster/cli_spec.rb @@ -36,11 +36,13 @@ describe Gitlab::SidekiqCluster::CLI do end it 'allows the special * selector' do + worker_queues = %w(foo bar baz) + + expect(Gitlab::SidekiqConfig::CliMethods) + .to receive(:worker_queues).and_return(worker_queues) + expect(Gitlab::SidekiqCluster) - .to receive(:start).with( - [Gitlab::SidekiqConfig::CliMethods.worker_queues], - default_options - ) + .to receive(:start).with([worker_queues], default_options) cli.run(%w(*)) end @@ -157,15 +159,17 @@ describe Gitlab::SidekiqCluster::CLI do .with([['chat_notification'], ['project_export']], default_options) .and_return([]) - cli.run(%w(--experimental-queue-selector feature_category=chatops&urgency=high resource_boundary=memory&feature_category=importers)) + cli.run(%w(--experimental-queue-selector feature_category=chatops&has_external_dependencies=true resource_boundary=memory&feature_category=importers)) end it 'allows the special * selector' do + worker_queues = %w(foo bar baz) + + expect(Gitlab::SidekiqConfig::CliMethods) + .to receive(:worker_queues).and_return(worker_queues) + expect(Gitlab::SidekiqCluster) - .to receive(:start).with( - [Gitlab::SidekiqConfig::CliMethods.worker_queues], - default_options - ) + .to receive(:start).with([worker_queues], default_options) cli.run(%w(--experimental-queue-selector *)) end diff --git a/spec/lib/quality/test_level_spec.rb b/spec/lib/quality/test_level_spec.rb index 34163c6cfbc..6042ab24787 100644 --- a/spec/lib/quality/test_level_spec.rb +++ b/spec/lib/quality/test_level_spec.rb @@ -28,7 +28,14 @@ RSpec.describe Quality::TestLevel do context 'when level is migration' do it 'returns a pattern' do expect(subject.pattern(:migration)) - .to eq("spec/{migrations,lib/gitlab/background_migration,lib/ee/gitlab/background_migration}{,/**/}*_spec.rb") + .to eq("spec/{migrations}{,/**/}*_spec.rb") + end + end + + context 'when level is background_migration' do + it 'returns a pattern' do + expect(subject.pattern(:background_migration)) + .to eq("spec/{lib/gitlab/background_migration,lib/ee/gitlab/background_migration}{,/**/}*_spec.rb") end end @@ -89,7 +96,14 @@ RSpec.describe Quality::TestLevel do context 'when level is migration' do it 'returns a regexp' do expect(subject.regexp(:migration)) - .to eq(%r{spec/(migrations|lib/gitlab/background_migration|lib/ee/gitlab/background_migration)}) + .to eq(%r{spec/(migrations)}) + end + end + + context 'when level is background_migration' do + it 'returns a regexp' do + expect(subject.regexp(:background_migration)) + .to eq(%r{spec/(lib/gitlab/background_migration|lib/ee/gitlab/background_migration)}) end end @@ -160,4 +174,26 @@ RSpec.describe Quality::TestLevel do %r{Test level for spec/unknown/foo_spec.rb couldn't be set. Please rename the file properly or change the test level detection regexes in .+/lib/quality/test_level.rb.}) end end + + describe '#background_migration?' do + it 'returns false for a unit test' do + expect(subject.background_migration?('spec/models/abuse_report_spec.rb')).to be(false) + end + + it 'returns true for a migration test' do + expect(subject.background_migration?('spec/migrations/add_default_and_free_plans_spec.rb')).to be(false) + end + + it 'returns true for a background migration test' do + expect(subject.background_migration?('spec/lib/gitlab/background_migration/archive_legacy_traces_spec.rb')).to be(true) + end + + it 'returns true for a geo migration test' do + expect(described_class.new('ee/').background_migration?('ee/spec/migrations/geo/migrate_ci_job_artifacts_to_separate_registry_spec.rb')).to be(false) + end + + it 'returns true for a EE-namespaced background migration test' do + expect(described_class.new('ee/').background_migration?('ee/spec/lib/ee/gitlab/background_migration/prune_orphaned_geo_events_spec.rb')).to be(true) + end + end end diff --git a/spec/models/clusters/applications/prometheus_spec.rb b/spec/models/clusters/applications/prometheus_spec.rb index 04e4d261b1c..ecb87910d2d 100644 --- a/spec/models/clusters/applications/prometheus_spec.rb +++ b/spec/models/clusters/applications/prometheus_spec.rb @@ -330,4 +330,46 @@ describe Clusters::Applications::Prometheus do it { is_expected.to be_falsy } end end + + describe 'alert manager token' do + subject { create(:clusters_applications_prometheus) } + + context 'when not set' do + it 'is empty by default' do + expect(subject.alert_manager_token).to be_nil + expect(subject.encrypted_alert_manager_token).to be_nil + expect(subject.encrypted_alert_manager_token_iv).to be_nil + end + + describe '#generate_alert_manager_token!' do + it 'generates a token' do + subject.generate_alert_manager_token! + + expect(subject.alert_manager_token).to match(/\A\h{32}\z/) + end + end + end + + context 'when set' do + let(:token) { SecureRandom.hex } + + before do + subject.update!(alert_manager_token: token) + end + + it 'reads the token' do + expect(subject.alert_manager_token).to eq(token) + expect(subject.encrypted_alert_manager_token).not_to be_nil + expect(subject.encrypted_alert_manager_token_iv).not_to be_nil + end + + describe '#generate_alert_manager_token!' do + it 'does not re-generate the token' do + subject.generate_alert_manager_token! + + expect(subject.alert_manager_token).to eq(token) + end + end + end + end end diff --git a/spec/policies/note_policy_spec.rb b/spec/policies/note_policy_spec.rb index e480fc2a642..94e6a86025c 100644 --- a/spec/policies/note_policy_spec.rb +++ b/spec/policies/note_policy_spec.rb @@ -35,6 +35,18 @@ describe NotePolicy do end end + context 'when the noteable is a deleted commit' do + let(:commit) { nil } + let(:note) { create(:note_on_commit, commit_id: '12345678', author: user, project: project) } + + it 'allows to read' do + expect(policy).to be_allowed(:read_note) + expect(policy).to be_disallowed(:admin_note) + expect(policy).to be_disallowed(:resolve_note) + expect(policy).to be_disallowed(:award_emoji) + end + end + context 'when the noteable is a commit' do let(:commit) { project.repository.head_commit } let(:note) { create(:note_on_commit, commit_id: commit.id, author: user, project: project) } diff --git a/spec/services/metrics/dashboard/update_dashboard_service_spec.rb b/spec/services/metrics/dashboard/update_dashboard_service_spec.rb index 66622524e9c..227041344d7 100644 --- a/spec/services/metrics/dashboard/update_dashboard_service_spec.rb +++ b/spec/services/metrics/dashboard/update_dashboard_service_spec.rb @@ -27,7 +27,14 @@ describe Metrics::Dashboard::UpdateDashboardService, :use_clean_rails_memory_sto end context 'user does not have push right to repository' do - it_behaves_like 'misconfigured dashboard service response', :forbidden, "You are not allowed to push into this branch. Create another branch or open a merge request." + it 'returns an appropriate message and status code', :aggregate_failures do + result = service_call + + expect(result.keys).to contain_exactly(:message, :http_status, :status, :last_step) + expect(result[:status]).to eq(:error) + expect(result[:http_status]).to eq(:forbidden) + expect(result[:message]).to eq("You are not allowed to push into this branch. Create another branch or open a merge request.") + end end context 'with rights to push to the repository' do @@ -39,13 +46,27 @@ describe Metrics::Dashboard::UpdateDashboardService, :use_clean_rails_memory_sto context 'with a yml extension' do let(:file_name) { 'config/prometheus/../database.yml' } - it_behaves_like 'misconfigured dashboard service response', :bad_request, "A file with this name doesn't exist" + it 'returns an appropriate message and status code', :aggregate_failures do + result = service_call + + expect(result.keys).to contain_exactly(:message, :http_status, :status, :last_step) + expect(result[:status]).to eq(:error) + expect(result[:http_status]).to eq(:bad_request) + expect(result[:message]).to eq("A file with this name doesn't exist") + end end context 'without a yml extension' do let(:file_name) { '../../..../etc/passwd' } - it_behaves_like 'misconfigured dashboard service response', :bad_request, "The file name should have a .yml extension" + it 'returns an appropriate message and status code', :aggregate_failures do + result = service_call + + expect(result.keys).to contain_exactly(:message, :http_status, :status, :last_step) + expect(result[:status]).to eq(:error) + expect(result[:http_status]).to eq(:bad_request) + expect(result[:message]).to eq("The file name should have a .yml extension") + end end end @@ -60,7 +81,14 @@ describe Metrics::Dashboard::UpdateDashboardService, :use_clean_rails_memory_sto project.repository.add_branch(user, branch, 'master') end - it_behaves_like 'misconfigured dashboard service response', :bad_request, "There was an error updating the dashboard, branch named: existing_branch already exists." + it 'returns an appropriate message and status code', :aggregate_failures do + result = service_call + + expect(result.keys).to contain_exactly(:message, :http_status, :status, :last_step) + expect(result[:status]).to eq(:error) + expect(result[:http_status]).to eq(:bad_request) + expect(result[:message]).to eq("There was an error updating the dashboard, branch named: existing_branch already exists.") + end end context 'Files::UpdateService success' do diff --git a/spec/services/projects/import_service_spec.rb b/spec/services/projects/import_service_spec.rb index d9f9ede8ecd..1e9ac40128a 100644 --- a/spec/services/projects/import_service_spec.rb +++ b/spec/services/projects/import_service_spec.rb @@ -122,7 +122,7 @@ describe Projects::ImportService do end it 'succeeds if repository import is successful' do - expect_any_instance_of(Gitlab::Shell).to receive(:import_repository).and_return(true) + expect(project.repository).to receive(:import_repository).and_return(true) expect_any_instance_of(Gitlab::BitbucketImport::Importer).to receive(:execute).and_return(true) expect_any_instance_of(Projects::LfsPointers::LfsImportService).to receive(:execute).and_return(status: :success) @@ -132,7 +132,9 @@ describe Projects::ImportService do end it 'fails if repository import fails' do - expect_any_instance_of(Gitlab::Shell).to receive(:import_repository).and_raise(Gitlab::Shell::Error.new('Failed to import the repository /a/b/c')) + expect(project.repository) + .to receive(:import_repository) + .and_raise(Gitlab::Git::CommandError, 'Failed to import the repository /a/b/c') result = subject.execute @@ -144,7 +146,7 @@ describe Projects::ImportService do it 'logs the error' do error_message = 'error message' - expect_any_instance_of(Gitlab::Shell).to receive(:import_repository).and_return(true) + expect(project.repository).to receive(:import_repository).and_return(true) expect_any_instance_of(Gitlab::BitbucketImport::Importer).to receive(:execute).and_return(true) expect_any_instance_of(Projects::LfsPointers::LfsImportService).to receive(:execute).and_return(status: :error, message: error_message) expect(Gitlab::AppLogger).to receive(:error).with("The Lfs import process failed. #{error_message}") @@ -155,7 +157,7 @@ describe Projects::ImportService do context 'when repository import scheduled' do before do - allow_any_instance_of(Gitlab::Shell).to receive(:import_repository).and_return(true) + expect(project.repository).to receive(:import_repository).and_return(true) allow(subject).to receive(:import_data) end diff --git a/spec/services/search_service_spec.rb b/spec/services/search_service_spec.rb index 48065bf596a..10dafaebe85 100644 --- a/spec/services/search_service_spec.rb +++ b/spec/services/search_service_spec.rb @@ -10,13 +10,16 @@ describe SearchService do let!(:group_member) { create(:group_member, group: accessible_group, user: user) } let!(:accessible_project) { create(:project, :private, name: 'accessible_project') } - let!(:inaccessible_project) { create(:project, :private, name: 'inaccessible_project') } let(:note) { create(:note_on_issue, project: accessible_project) } + let!(:inaccessible_project) { create(:project, :private, name: 'inaccessible_project') } + let(:snippet) { create(:snippet, author: user) } let(:group_project) { create(:project, group: accessible_group, name: 'group_project') } let(:public_project) { create(:project, :public, name: 'public_project') } + subject(:search_service) { described_class.new(user, search: search, scope: scope, page: 1) } + before do accessible_project.add_maintainer(user) end @@ -293,5 +296,70 @@ describe SearchService do expect(search_objects.first).to eq public_project end end + + context 'redacting search results' do + shared_examples 'it redacts incorrect results' do + before do + allow(Ability).to receive(:allowed?).and_return(allowed) + end + + context 'when allowed' do + let(:allowed) { true } + + it 'does nothing' do + expect(results).not_to be_empty + expect(results).to all(be_an(model_class)) + end + end + + context 'when disallowed' do + let(:allowed) { false } + + it 'does nothing' do + expect(results).to be_empty + end + end + end + + context 'issues' do + let(:issue) { create(:issue, project: accessible_project) } + let(:scope) { 'issues' } + let(:model_class) { Issue } + let(:ability) { :read_issue } + let(:search) { issue.title } + let(:results) { subject.search_objects } + + it_behaves_like 'it redacts incorrect results' + end + + context 'notes' do + let(:note) { create(:note_on_commit, project: accessible_project) } + let(:scope) { 'notes' } + let(:model_class) { Note } + let(:ability) { :read_note } + let(:search) { note.note } + let(:results) do + described_class.new( + user, + project_id: accessible_project.id, + scope: scope, + search: note.note + ).search_objects + end + + it_behaves_like 'it redacts incorrect results' + end + + context 'merge_requests' do + let(:scope) { 'merge_requests' } + let(:model_class) { MergeRequest } + let(:ability) { :read_merge_request } + let(:merge_request) { create(:merge_request, source_project: accessible_project, author: user) } + let(:search) { merge_request.title } + let(:results) { subject.search_objects } + + it_behaves_like 'it redacts incorrect results' + end + end end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 3f6503a60a9..30524e4bbae 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -81,6 +81,11 @@ RSpec.configure do |config| metadata[:migration] = true if metadata[:level] == :migration end + # Do not overwrite schema if it's already set + unless metadata.key?(:schema) + metadata[:schema] = :latest if quality_level.background_migration?(location) + end + # Do not overwrite type if it's already set unless metadata.key?(:type) match = location.match(%r{/spec/([^/]+)/}) diff --git a/spec/uploaders/object_storage_spec.rb b/spec/uploaders/object_storage_spec.rb index 2f2ed28891a..06116bd4737 100644 --- a/spec/uploaders/object_storage_spec.rb +++ b/spec/uploaders/object_storage_spec.rb @@ -272,7 +272,7 @@ describe ObjectStorage do end it "to raise an error" do - expect { subject }.to raise_error(/Object Storage is not enabled/) + expect { subject }.to raise_error(/Object Storage is not enabled for JobArtifactUploader/) end end diff --git a/spec/workers/repository_fork_worker_spec.rb b/spec/workers/repository_fork_worker_spec.rb index c0fb0a40025..7209c40646f 100644 --- a/spec/workers/repository_fork_worker_spec.rb +++ b/spec/workers/repository_fork_worker_spec.rb @@ -13,7 +13,6 @@ describe RepositoryForkWorker do describe "#perform" do let(:project) { create(:project, :public, :repository) } - let(:shell) { Gitlab::Shell.new } let(:forked_project) { create(:project, :repository, :import_scheduled) } before do @@ -21,12 +20,17 @@ describe RepositoryForkWorker do end shared_examples 'RepositoryForkWorker performing' do - before do - allow(subject).to receive(:gitlab_shell).and_return(shell) - end - - def expect_fork_repository - expect(shell).to receive(:fork_repository).with(project, forked_project) + def expect_fork_repository(success:) + allow(::Gitlab::GitalyClient::RepositoryService).to receive(:new).and_call_original + expect_next_instance_of(::Gitlab::GitalyClient::RepositoryService, forked_project.repository.raw) do |svc| + exp = expect(svc).to receive(:fork_repository).with(project.repository.raw) + + if success + exp.and_return(true) + else + exp.and_raise(GRPC::BadStatus, 'Fork failed in tests') + end + end end describe 'when a worker was reset without cleanup' do @@ -35,20 +39,20 @@ describe RepositoryForkWorker do it 'creates a new repository from a fork' do allow(subject).to receive(:jid).and_return(jid) - expect_fork_repository.and_return(true) + expect_fork_repository(success: true) perform! end end it "creates a new repository from a fork" do - expect_fork_repository.and_return(true) + expect_fork_repository(success: true) perform! end it 'protects the default branch' do - expect_fork_repository.and_return(true) + expect_fork_repository(success: true) perform! @@ -56,7 +60,7 @@ describe RepositoryForkWorker do end it 'flushes various caches' do - expect_fork_repository.and_return(true) + expect_fork_repository(success: true) # Works around https://github.com/rspec/rspec-mocks/issues/910 expect(Project).to receive(:find).with(forked_project.id).and_return(forked_project) @@ -75,13 +79,13 @@ describe RepositoryForkWorker do it 'handles bad fork' do error_message = "Unable to fork project #{forked_project.id} for repository #{project.disk_path} -> #{forked_project.disk_path}: Failed to create fork repository" - expect_fork_repository.and_return(false) + expect_fork_repository(success: false) expect { perform! }.to raise_error(StandardError, error_message) end it 'calls Projects::LfsPointers::LfsLinkService#execute with OIDs of source project LFS objects' do - expect_fork_repository.and_return(true) + expect_fork_repository(success: true) expect_next_instance_of(Projects::LfsPointers::LfsLinkService) do |service| expect(service).to receive(:execute).with(project.all_lfs_objects_oids) end @@ -92,7 +96,7 @@ describe RepositoryForkWorker do it "handles LFS objects link failure" do error_message = "Unable to fork project #{forked_project.id} for repository #{project.disk_path} -> #{forked_project.disk_path}: Source project has too many LFS objects" - expect_fork_repository.and_return(true) + expect_fork_repository(success: true) expect_next_instance_of(Projects::LfsPointers::LfsLinkService) do |service| expect(service).to receive(:execute).and_raise(Projects::LfsPointers::LfsLinkService::TooManyOidsError) end |