diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2019-10-03 03:05:59 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2019-10-03 03:05:59 +0300 |
commit | 427b23c12718bea233931431e7d9307881a960c0 (patch) | |
tree | 5e15672783c950a5e68dd89517d7888e652e01a7 /spec | |
parent | 6d60f910762c1a92a07a4afaf1b26962f75ee4b6 (diff) |
Add latest changes from gitlab-org/gitlab@master
Diffstat (limited to 'spec')
23 files changed, 334 insertions, 96 deletions
diff --git a/spec/com_spec_helper.rb b/spec/com_spec_helper.rb new file mode 100644 index 00000000000..f71ccde9509 --- /dev/null +++ b/spec/com_spec_helper.rb @@ -0,0 +1,3 @@ +# frozen_string_literal: true + +Settings.gitlab[:url] = "https://test.gitlab.com" diff --git a/spec/factories/uploads.rb b/spec/factories/uploads.rb index bfe02c6010b..ef464d3d6e0 100644 --- a/spec/factories/uploads.rb +++ b/spec/factories/uploads.rb @@ -2,7 +2,7 @@ FactoryBot.define do factory :upload do - model { build(:project) } + model { create(:project) } size { 100.kilobytes } uploader { "AvatarUploader" } mount_point { :avatar } @@ -11,23 +11,27 @@ FactoryBot.define do # we should build a mount agnostic upload by default transient do - filename { 'myfile.jpg' } + filename { 'avatar.jpg' } end - # this needs to comply with RecordsUpload::Concern#upload_path - path { File.join("uploads/-/system", model.class.underscore, mount_point.to_s, 'avatar.jpg') } + path do + uploader_instance = Object.const_get(uploader.to_s).new(model, mount_point) + File.join(uploader_instance.store_dir, filename) + end trait :personal_snippet_upload do - uploader { "PersonalFileUploader" } + model { create(:personal_snippet) } path { File.join(secret, filename) } - model { build(:personal_snippet) } + uploader { "PersonalFileUploader" } secret { SecureRandom.hex } + mount_point { nil } end trait :issuable_upload do uploader { "FileUploader" } path { File.join(secret, filename) } secret { SecureRandom.hex } + mount_point { nil } end trait :with_file do @@ -42,22 +46,23 @@ FactoryBot.define do end trait :namespace_upload do - model { build(:group) } + model { create(:group) } path { File.join(secret, filename) } uploader { "NamespaceFileUploader" } secret { SecureRandom.hex } + mount_point { nil } end trait :favicon_upload do - model { build(:appearance) } - path { File.join(secret, filename) } + model { create(:appearance) } uploader { "FaviconUploader" } secret { SecureRandom.hex } + mount_point { :favicon } end trait :attachment_upload do mount_point { :attachment } - model { build(:note) } + model { create(:note) } uploader { "AttachmentUploader" } end end diff --git a/spec/fast_spec_helper.rb b/spec/fast_spec_helper.rb index f5a487b4d57..a7810d72671 100644 --- a/spec/fast_spec_helper.rb +++ b/spec/fast_spec_helper.rb @@ -5,10 +5,12 @@ ENV['IN_MEMORY_APPLICATION_SETTINGS'] = 'true' require 'active_support/dependencies' require_relative '../config/initializers/0_inject_enterprise_edition_module' +require_relative '../config/initializers/0_inject_com_module' require_relative '../config/settings' require_relative 'support/rspec' require 'active_support/all' ActiveSupport::Dependencies.autoload_paths << 'lib' ActiveSupport::Dependencies.autoload_paths << 'ee/lib' +ActiveSupport::Dependencies.autoload_paths << 'com/lib' ActiveSupport::XmlMini.backend = 'Nokogiri' diff --git a/spec/features/merge_request/user_views_merge_request_from_deleted_fork_spec.rb b/spec/features/merge_request/user_views_merge_request_from_deleted_fork_spec.rb new file mode 100644 index 00000000000..26984a1fb5e --- /dev/null +++ b/spec/features/merge_request/user_views_merge_request_from_deleted_fork_spec.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true +require 'spec_helper' + +# This is a feature spec because the problems arrise when rendering the view for +# an actual project for which the repository is removed but the cached not +# updated. +# This can occur when the fork a merge request is created from is in the process +# of being destroyed. +describe 'User views merged merge request from deleted fork' do + include ProjectForksHelper + + let(:project) { create(:project, :repository) } + let(:source_project) { fork_project(project, nil, repository: true) } + let(:user) { project.owner } + let!(:merge_request) { create(:merge_request, :merged, source_project: source_project, target_project: project) } + + before do + sign_in user + + fork_owner = source_project.namespace.owners.first + # Place the source_project in the weird in between state + source_project.update_attribute(:pending_delete, true) + Projects::DestroyService.new(source_project, fork_owner, {}).__send__(:trash_repositories!) + end + + it 'correctly shows the merge request' do + visit(merge_request_path(merge_request)) + + expect(page).to have_content(merge_request.title) + end +end diff --git a/spec/frontend/commit/__snapshots__/commit_pipeline_status_component_spec.js.snap b/spec/frontend/commit/__snapshots__/commit_pipeline_status_component_spec.js.snap deleted file mode 100644 index 9199db69fed..00000000000 --- a/spec/frontend/commit/__snapshots__/commit_pipeline_status_component_spec.js.snap +++ /dev/null @@ -1,39 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`Commit pipeline status component when polling is not successful renders not found CI icon without loader 1`] = ` -<div - class="ci-status-link" -> - <a> - <ciicon-stub - aria-label="Pipeline: not found" - cssclasses="" - data-container="body" - data-original-title="Pipeline: not found" - size="24" - status="[object Object]" - title="" - /> - </a> -</div> -`; - -exports[`Commit pipeline status component when polling is successful renders CI icon without loader 1`] = ` -<div - class="ci-status-link" -> - <a - href="/frontend-fixtures/pipelines-project/pipelines/47" - > - <ciicon-stub - aria-label="Pipeline: pending" - cssclasses="" - data-container="body" - data-original-title="Pipeline: pending" - size="24" - status="[object Object]" - title="" - /> - </a> -</div> -`; diff --git a/spec/frontend/commit/commit_pipeline_status_component_spec.js b/spec/frontend/commit/commit_pipeline_status_component_spec.js index 1768fd745c9..1736d1d0df8 100644 --- a/spec/frontend/commit/commit_pipeline_status_component_spec.js +++ b/spec/frontend/commit/commit_pipeline_status_component_spec.js @@ -2,6 +2,7 @@ import Visibility from 'visibilityjs'; import { GlLoadingIcon } from '@gitlab/ui'; import Poll from '~/lib/utils/poll'; import flash from '~/flash'; +import CiIcon from '~/vue_shared/components/ci_icon.vue'; import CommitPipelineStatus from '~/projects/tree/components/commit_pipeline_status_component.vue'; import { shallowMount } from '@vue/test-utils'; import { getJSONFixture } from '../helpers/fixtures'; @@ -36,6 +37,10 @@ describe('Commit pipeline status component', () => { }); }; + const findLoader = () => wrapper.find(GlLoadingIcon); + const findLink = () => wrapper.find('a'); + const findCiIcon = () => findLink().find(CiIcon); + afterEach(() => { wrapper.destroy(); wrapper = null; @@ -111,14 +116,14 @@ describe('Commit pipeline status component', () => { it('shows the loading icon at start', () => { createComponent(); - expect(wrapper.find(GlLoadingIcon).exists()).toBe(true); + expect(findLoader().exists()).toBe(true); pollConfig.successCallback({ data: { pipelines: [] }, }); return wrapper.vm.$nextTick().then(() => { - expect(wrapper.find(GlLoadingIcon).exists()).toBe(false); + expect(findLoader().exists()).toBe(false); }); }); @@ -130,8 +135,17 @@ describe('Commit pipeline status component', () => { return wrapper.vm.$nextTick(); }); - it('renders CI icon without loader', () => { - expect(wrapper.element).toMatchSnapshot(); + it('does not render loader', () => { + expect(findLoader().exists()).toBe(false); + }); + + it('renders link with href', () => { + expect(findLink().attributes('href')).toEqual(mockCiStatus.details_path); + }); + + it('renders CI icon', () => { + expect(findCiIcon().attributes('data-original-title')).toEqual('Pipeline: pending'); + expect(findCiIcon().props('status')).toEqual(mockCiStatus); }); }); @@ -140,8 +154,21 @@ describe('Commit pipeline status component', () => { pollConfig.errorCallback(); }); - it('renders not found CI icon without loader', () => { - expect(wrapper.element).toMatchSnapshot(); + it('does not render loader', () => { + expect(findLoader().exists()).toBe(false); + }); + + it('renders link with href', () => { + expect(findLink().attributes('href')).toBeUndefined(); + }); + + it('renders not found CI icon', () => { + expect(findCiIcon().attributes('data-original-title')).toEqual('Pipeline: not found'); + expect(findCiIcon().props('status')).toEqual({ + text: 'not found', + icon: 'status_notfound', + group: 'notfound', + }); }); it('displays flash error message', () => { diff --git a/spec/frontend/create_cluster/eks_cluster/components/eks_cluster_configuration_form_spec.js b/spec/frontend/create_cluster/eks_cluster/components/eks_cluster_configuration_form_spec.js index 1bd16d356b9..cc7c6735a80 100644 --- a/spec/frontend/create_cluster/eks_cluster/components/eks_cluster_configuration_form_spec.js +++ b/spec/frontend/create_cluster/eks_cluster/components/eks_cluster_configuration_form_spec.js @@ -4,6 +4,7 @@ import Vue from 'vue'; import EksClusterConfigurationForm from '~/create_cluster/eks_cluster/components/eks_cluster_configuration_form.vue'; import RegionDropdown from '~/create_cluster/eks_cluster/components/region_dropdown.vue'; +import eksClusterFormState from '~/create_cluster/eks_cluster/store/state'; import clusterDropdownStoreState from '~/create_cluster/eks_cluster/store/cluster_dropdown/state'; const localVue = createLocalVue(); @@ -12,29 +13,59 @@ localVue.use(Vuex); describe('EksClusterConfigurationForm', () => { let store; let actions; + let state; let regionsState; + let vpcsState; + let subnetsState; + let vpcsActions; let regionsActions; + let subnetsActions; let vm; beforeEach(() => { + state = eksClusterFormState(); actions = { setRegion: jest.fn(), setVpc: jest.fn(), + setSubnet: jest.fn(), }; regionsActions = { fetchItems: jest.fn(), }; + vpcsActions = { + fetchItems: jest.fn(), + }; + subnetsActions = { + fetchItems: jest.fn(), + }; regionsState = { ...clusterDropdownStoreState(), }; + vpcsState = { + ...clusterDropdownStoreState(), + }; + subnetsState = { + ...clusterDropdownStoreState(), + }; store = new Vuex.Store({ + state, actions, modules: { + vpcs: { + namespaced: true, + state: vpcsState, + actions: vpcsActions, + }, regions: { namespaced: true, state: regionsState, actions: regionsActions, }, + subnets: { + namespaced: true, + state: subnetsState, + actions: subnetsActions, + }, }, }); }); @@ -51,6 +82,8 @@ describe('EksClusterConfigurationForm', () => { }); const findRegionDropdown = () => vm.find(RegionDropdown); + const findVpcDropdown = () => vm.find('[field-id="eks-vpc"]'); + const findSubnetDropdown = () => vm.find('[field-id="eks-subnet"]'); describe('when mounted', () => { it('fetches available regions', () => { @@ -62,16 +95,72 @@ describe('EksClusterConfigurationForm', () => { regionsState.isLoadingItems = true; return Vue.nextTick().then(() => { - expect(findRegionDropdown().props('loading')).toEqual(regionsState.isLoadingItems); + expect(findRegionDropdown().props('loading')).toBe(regionsState.isLoadingItems); }); }); it('sets regions to RegionDropdown regions property', () => { - expect(findRegionDropdown().props('regions')).toEqual(regionsState.items); + expect(findRegionDropdown().props('regions')).toBe(regionsState.items); }); it('sets loadingRegionsError to RegionDropdown error property', () => { - expect(findRegionDropdown().props('error')).toEqual(regionsState.loadingItemsError); + expect(findRegionDropdown().props('error')).toBe(regionsState.loadingItemsError); + }); + + it('disables VpcDropdown when no region is selected', () => { + expect(findVpcDropdown().props('disabled')).toBe(true); + }); + + it('enables VpcDropdown when no region is selected', () => { + state.selectedRegion = { name: 'west-1 ' }; + + return Vue.nextTick().then(() => { + expect(findVpcDropdown().props('disabled')).toBe(false); + }); + }); + + it('sets isLoadingVpcs to VpcDropdown loading property', () => { + vpcsState.isLoadingItems = true; + + return Vue.nextTick().then(() => { + expect(findVpcDropdown().props('loading')).toBe(vpcsState.isLoadingItems); + }); + }); + + it('sets vpcs to VpcDropdown items property', () => { + expect(findVpcDropdown().props('items')).toBe(vpcsState.items); + }); + + it('sets loadingVpcsError to VpcDropdown hasErrors property', () => { + expect(findVpcDropdown().props('hasErrors')).toBe(vpcsState.loadingItemsError); + }); + + it('disables SubnetDropdown when no vpc is selected', () => { + expect(findSubnetDropdown().props('disabled')).toBe(true); + }); + + it('enables SubnetDropdown when a vpc is selected', () => { + state.selectedVpc = { name: 'vpc-1 ' }; + + return Vue.nextTick().then(() => { + expect(findSubnetDropdown().props('disabled')).toBe(false); + }); + }); + + it('sets isLoadingSubnets to SubnetDropdown loading property', () => { + subnetsState.isLoadingItems = true; + + return Vue.nextTick().then(() => { + expect(findSubnetDropdown().props('loading')).toBe(subnetsState.isLoadingItems); + }); + }); + + it('sets subnets to SubnetDropdown items property', () => { + expect(findSubnetDropdown().props('items')).toBe(subnetsState.items); + }); + + it('sets loadingSubnetsError to SubnetDropdown hasErrors property', () => { + expect(findSubnetDropdown().props('hasErrors')).toBe(subnetsState.loadingItemsError); }); describe('when region is selected', () => { @@ -84,5 +173,37 @@ describe('EksClusterConfigurationForm', () => { it('dispatches setRegion action', () => { expect(actions.setRegion).toHaveBeenCalledWith(expect.anything(), { region }, undefined); }); + + it('fetches available vpcs', () => { + expect(vpcsActions.fetchItems).toHaveBeenCalledWith(expect.anything(), { region }, undefined); + }); + }); + + describe('when vpc is selected', () => { + const vpc = { name: 'vpc-1' }; + + beforeEach(() => { + findVpcDropdown().vm.$emit('input', vpc); + }); + + it('dispatches setVpc action', () => { + expect(actions.setVpc).toHaveBeenCalledWith(expect.anything(), { vpc }, undefined); + }); + + it('dispatches fetchSubnets action', () => { + expect(subnetsActions.fetchItems).toHaveBeenCalledWith(expect.anything(), { vpc }, undefined); + }); + }); + + describe('when a subnet is selected', () => { + const subnet = { name: 'subnet-1' }; + + beforeEach(() => { + findSubnetDropdown().vm.$emit('input', subnet); + }); + + it('dispatches setSubnet action', () => { + expect(actions.setSubnet).toHaveBeenCalledWith(expect.anything(), { subnet }, undefined); + }); }); }); diff --git a/spec/frontend/create_cluster/eks_cluster/store/actions_spec.js b/spec/frontend/create_cluster/eks_cluster/store/actions_spec.js index 3744ddb3dc6..893c657e699 100644 --- a/spec/frontend/create_cluster/eks_cluster/store/actions_spec.js +++ b/spec/frontend/create_cluster/eks_cluster/store/actions_spec.js @@ -1,17 +1,28 @@ import testAction from 'helpers/vuex_action_helper'; import createState from '~/create_cluster/eks_cluster/store/state'; -import * as types from '~/create_cluster/eks_cluster/store/mutation_types'; import * as actions from '~/create_cluster/eks_cluster/store/actions'; +import { SET_REGION, SET_VPC, SET_SUBNET } from '~/create_cluster/eks_cluster/store/mutation_types'; describe('EKS Cluster Store Actions', () => { - describe('setRegion', () => { - it(`commits ${types.SET_REGION} mutation`, () => { - const region = { name: 'west-1' }; + let region; + let vpc; + let subnet; - testAction(actions.setRegion, { region }, createState(), [ - { type: types.SET_REGION, payload: { region } }, - ]); - }); + beforeEach(() => { + region = { name: 'regions-1' }; + vpc = { name: 'vpc-1' }; + subnet = { name: 'subnet-1' }; + }); + + it.each` + action | mutation | payload | payloadDescription + ${'setRegion'} | ${SET_REGION} | ${{ region }} | ${'region'} + ${'setVpc'} | ${SET_VPC} | ${{ vpc }} | ${'vpc'} + ${'setSubnet'} | ${SET_SUBNET} | ${{ subnet }} | ${'subnet'} + `(`$action commits $mutation with $payloadDescription payload`, data => { + const { action, mutation, payload } = data; + + testAction(actions[action], payload, createState(), [{ type: mutation, payload }]); }); }); diff --git a/spec/frontend/create_cluster/eks_cluster/store/mutations_spec.js b/spec/frontend/create_cluster/eks_cluster/store/mutations_spec.js index 99f05b0f449..38199471f79 100644 --- a/spec/frontend/create_cluster/eks_cluster/store/mutations_spec.js +++ b/spec/frontend/create_cluster/eks_cluster/store/mutations_spec.js @@ -1,19 +1,26 @@ -import { SET_REGION } from '~/create_cluster/eks_cluster/store/mutation_types'; +import { SET_REGION, SET_VPC, SET_SUBNET } from '~/create_cluster/eks_cluster/store/mutation_types'; import createState from '~/create_cluster/eks_cluster/store/state'; import mutations from '~/create_cluster/eks_cluster/store/mutations'; describe('Create EKS cluster store mutations', () => { let state; let region; + let vpc; + let subnet; beforeEach(() => { region = { name: 'regions-1' }; + vpc = { name: 'vpc-1' }; + subnet = { name: 'subnet-1' }; + state = createState(); }); it.each` mutation | mutatedProperty | payload | expectedValue | expectedValueDescription ${SET_REGION} | ${'selectedRegion'} | ${{ region }} | ${region} | ${'selected region payload'} + ${SET_VPC} | ${'selectedVpc'} | ${{ vpc }} | ${vpc} | ${'selected vpc payload'} + ${SET_SUBNET} | ${'selectedSubnet'} | ${{ subnet }} | ${subnet} | ${'selected sybnet payload'} `(`$mutation sets $mutatedProperty to $expectedValueDescription`, data => { const { mutation, mutatedProperty, payload, expectedValue } = data; 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 7d67dc0251d..c1eaf1d3433 100644 --- a/spec/lib/gitlab/background_migration/legacy_upload_mover_spec.rb +++ b/spec/lib/gitlab/background_migration/legacy_upload_mover_spec.rb @@ -32,7 +32,7 @@ describe Gitlab::BackgroundMigration::LegacyUploadMover do if with_file upload = create(:upload, :with_file, :attachment_upload, params) - model.update(attachment: upload.build_uploader) + model.update(attachment: upload.retrieve_uploader) model.attachment.upload else create(:upload, :attachment_upload, params) 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 ed8cbfeb11f..cabca3dbef9 100644 --- a/spec/lib/gitlab/background_migration/legacy_uploads_migrator_spec.rb +++ b/spec/lib/gitlab/background_migration/legacy_uploads_migrator_spec.rb @@ -24,7 +24,7 @@ describe Gitlab::BackgroundMigration::LegacyUploadsMigrator do if with_file upload = create(:upload, :with_file, :attachment_upload, params) - model.update(attachment: upload.build_uploader) + model.update(attachment: upload.retrieve_uploader) model.attachment.upload else create(:upload, :attachment_upload, params) diff --git a/spec/lib/gitlab/import_export/uploads_manager_spec.rb b/spec/lib/gitlab/import_export/uploads_manager_spec.rb index 792117e1df1..f13f639d6b7 100644 --- a/spec/lib/gitlab/import_export/uploads_manager_spec.rb +++ b/spec/lib/gitlab/import_export/uploads_manager_spec.rb @@ -83,7 +83,7 @@ describe Gitlab::ImportExport::UploadsManager do it 'restores the file' do manager.restore - expect(project.uploads.map { |u| u.build_uploader.filename }).to include('dummy.txt') + expect(project.uploads.map { |u| u.retrieve_uploader.filename }).to include('dummy.txt') end end end diff --git a/spec/lib/gitlab/import_export/uploads_restorer_spec.rb b/spec/lib/gitlab/import_export/uploads_restorer_spec.rb index 6072f18b8c7..e2e8204b2fa 100644 --- a/spec/lib/gitlab/import_export/uploads_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/uploads_restorer_spec.rb @@ -27,7 +27,7 @@ describe Gitlab::ImportExport::UploadsRestorer do it 'copies the uploads to the project path' do subject.restore - expect(project.uploads.map { |u| u.build_uploader.filename }).to include('dummy.txt') + expect(project.uploads.map { |u| u.retrieve_uploader.filename }).to include('dummy.txt') end end @@ -43,7 +43,7 @@ describe Gitlab::ImportExport::UploadsRestorer do it 'copies the uploads to the project path' do subject.restore - expect(project.uploads.map { |u| u.build_uploader.filename }).to include('dummy.txt') + expect(project.uploads.map { |u| u.retrieve_uploader.filename }).to include('dummy.txt') end end end diff --git a/spec/lib/gitlab/sanitizers/exif_spec.rb b/spec/lib/gitlab/sanitizers/exif_spec.rb index f882dbbdb5c..11e430e0be4 100644 --- a/spec/lib/gitlab/sanitizers/exif_spec.rb +++ b/spec/lib/gitlab/sanitizers/exif_spec.rb @@ -58,7 +58,7 @@ describe Gitlab::Sanitizers::Exif do end describe '#clean' do - let(:uploader) { create(:upload, :with_file, :issuable_upload).build_uploader } + let(:uploader) { create(:upload, :with_file, :issuable_upload).retrieve_uploader } context "no dry run" do it "removes exif from the image" do diff --git a/spec/lib/gitlab_spec.rb b/spec/lib/gitlab_spec.rb index ccb5cb3aa43..7e318017a05 100644 --- a/spec/lib/gitlab_spec.rb +++ b/spec/lib/gitlab_spec.rb @@ -71,26 +71,30 @@ describe Gitlab do end describe '.com?' do + before do + Thread.current[:is_com] = nil + end + it 'is true when on GitLab.com' do - stub_config_setting(url: 'https://gitlab.com') + allow(LightSettings).to receive(:host).and_return('gitlab.com') expect(described_class.com?).to eq true end it 'is true when on staging' do - stub_config_setting(url: 'https://staging.gitlab.com') + allow(LightSettings).to receive(:host).and_return('staging.gitlab.com') expect(described_class.com?).to eq true end it 'is true when on other gitlab subdomain' do - stub_config_setting(url: 'https://example.gitlab.com') + allow(LightSettings).to receive(:host).and_return('example.gitlab.com') expect(described_class.com?).to eq true end it 'is false when not on GitLab.com' do - stub_config_setting(url: 'http://example.com') + allow(LightSettings).to receive(:host).and_return('example.com') expect(described_class.com?).to eq false end diff --git a/spec/models/commit_collection_spec.rb b/spec/models/commit_collection_spec.rb index a8957bbfdd0..d49b71db5f8 100644 --- a/spec/models/commit_collection_spec.rb +++ b/spec/models/commit_collection_spec.rb @@ -149,6 +149,17 @@ describe CommitCollection do collection.enrich! end + + it 'returns the original commit if the commit could not be lazy loaded' do + collection = described_class.new(project, [hash_commit]) + unexisting_lazy_commit = Commit.lazy(project, Gitlab::Git::BLANK_SHA) + + expect(Commit).to receive(:lazy).with(project, hash_commit.id).and_return(unexisting_lazy_commit) + + collection.enrich! + + expect(collection.commits).to contain_exactly(hash_commit) + end end end diff --git a/spec/models/upload_spec.rb b/spec/models/upload_spec.rb index d97bb8cfb90..03434c95218 100644 --- a/spec/models/upload_spec.rb +++ b/spec/models/upload_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe Upload do - describe 'assocations' do + describe 'associations' do it { is_expected.to belong_to(:model) } end @@ -107,6 +107,52 @@ describe Upload do end end + describe '#build_uploader' do + it 'returns a uploader object with current upload associated with it' do + subject = build(:upload) + uploader = subject.build_uploader + + expect(uploader.upload).to eq(subject) + expect(uploader.mounted_as).to eq(subject.send(:mount_point)) + expect(uploader.file).to be_nil + end + end + + describe '#retrieve_uploader' do + it 'returns a uploader object with current uploader associated with and cache retrieved' do + subject = build(:upload) + uploader = subject.retrieve_uploader + + expect(uploader.upload).to eq(subject) + expect(uploader.mounted_as).to eq(subject.send(:mount_point)) + expect(uploader.file).not_to be_nil + end + end + + describe '#needs_checksum?' do + context 'with local storage' do + it 'returns true when no checksum exists' do + subject = create(:upload, :with_file, checksum: nil) + + expect(subject.needs_checksum?).to be_truthy + end + + it 'returns false when checksum is already present' do + subject = create(:upload, :with_file, checksum: 'something') + + expect(subject.needs_checksum?).to be_falsey + end + end + + context 'with remote storage' do + subject { build(:upload, :object_storage) } + + it 'returns false' do + expect(subject.needs_checksum?).to be_falsey + end + end + end + describe '#exist?' do it 'returns true when the file exists' do upload = described_class.new(path: __FILE__, store: ObjectStorage::Store::LOCAL) diff --git a/spec/models/uploads/fog_spec.rb b/spec/models/uploads/fog_spec.rb index 4a44cf5ab0f..b93d9449da9 100644 --- a/spec/models/uploads/fog_spec.rb +++ b/spec/models/uploads/fog_spec.rb @@ -44,7 +44,7 @@ describe Uploads::Fog do subject { data_store.delete_keys(keys) } before do - uploads.each { |upload| upload.build_uploader.migrate!(2) } + uploads.each { |upload| upload.retrieve_uploader.migrate!(2) } end it 'deletes multiple data' do diff --git a/spec/support/shared_examples/controllers/uploads_actions_shared_examples.rb b/spec/support/shared_examples/controllers/uploads_actions_shared_examples.rb index 4bc22861d58..0b4ab9941fc 100644 --- a/spec/support/shared_examples/controllers/uploads_actions_shared_examples.rb +++ b/spec/support/shared_examples/controllers/uploads_actions_shared_examples.rb @@ -104,7 +104,7 @@ shared_examples 'handle uploads' do context "when neither the uploader nor the model exists" do before do - allow_any_instance_of(Upload).to receive(:build_uploader).and_return(nil) + allow_any_instance_of(Upload).to receive(:retrieve_uploader).and_return(nil) allow(controller).to receive(:find_model).and_return(nil) end diff --git a/spec/support/shared_examples/models/with_uploads_shared_examples.rb b/spec/support/shared_examples/models/with_uploads_shared_examples.rb index eb1ade03017..822836c771e 100644 --- a/spec/support/shared_examples/models/with_uploads_shared_examples.rb +++ b/spec/support/shared_examples/models/with_uploads_shared_examples.rb @@ -41,7 +41,8 @@ shared_examples_for 'model with uploads' do |supports_fileuploads| end it 'deletes remote files' do - expect_any_instance_of(Uploads::Fog).to receive(:delete_keys).with(uploads.map(&:path)) + expected_array = array_including(*uploads.map(&:path)) + expect_any_instance_of(Uploads::Fog).to receive(:delete_keys).with(expected_array) model_object.destroy end diff --git a/spec/uploaders/file_uploader_spec.rb b/spec/uploaders/file_uploader_spec.rb index 04206de3dc6..3c14edc7e0e 100644 --- a/spec/uploaders/file_uploader_spec.rb +++ b/spec/uploaders/file_uploader_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe FileUploader do let(:group) { create(:group, name: 'awesome') } let(:project) { create(:project, :legacy_storage, namespace: group, name: 'project') } - let(:uploader) { described_class.new(project) } + let(:uploader) { described_class.new(project, :avatar) } let(:upload) { double(model: project, path: 'secret/foo.jpg') } subject { uploader } @@ -184,6 +184,14 @@ describe FileUploader do end end + describe '#replace_file_without_saving!' do + let(:replacement) { Tempfile.create('replacement.jpg') } + + it 'replaces an existing file without changing its metadata' do + expect { subject.replace_file_without_saving! CarrierWave::SanitizedFile.new(replacement) }.not_to change { subject.upload } + end + end + context 'when remote file is used' do let(:temp_file) { Tempfile.new("test") } diff --git a/spec/uploaders/gitlab_uploader_spec.rb b/spec/uploaders/gitlab_uploader_spec.rb index 3bee4875348..4425dd947c0 100644 --- a/spec/uploaders/gitlab_uploader_spec.rb +++ b/spec/uploaders/gitlab_uploader_spec.rb @@ -69,6 +69,16 @@ describe GitlabUploader do end end + describe '#replace_file_without_saving!' do + it 'allows file to be replaced without triggering any callbacks' do + new_file = CarrierWave::SanitizedFile.new(Tempfile.new) + + expect(subject).not_to receive(:with_callbacks) + + subject.replace_file_without_saving!(new_file) + end + end + describe '#open' do context 'when trace is stored in File storage' do context 'when file exists' do diff --git a/spec/uploaders/workers/object_storage/migrate_uploads_worker_spec.rb b/spec/uploaders/workers/object_storage/migrate_uploads_worker_spec.rb index 6c2544d2efd..97e8a43f7fd 100644 --- a/spec/uploaders/workers/object_storage/migrate_uploads_worker_spec.rb +++ b/spec/uploaders/workers/object_storage/migrate_uploads_worker_spec.rb @@ -42,33 +42,23 @@ describe ObjectStorage::MigrateUploadsWorker, :sidekiq do end describe '.sanity_check!' do - shared_examples 'raises a SanityCheckError' do + shared_examples 'raises a SanityCheckError' do |expected_message| let(:mount_point) { nil } it do expect { described_class.sanity_check!(uploads, model_class, mount_point) } - .to raise_error(described_class::SanityCheckError) + .to raise_error(described_class::SanityCheckError).with_message(expected_message) end end - before do - stub_const("WrongModel", Class.new) - end - context 'uploader types mismatch' do let!(:outlier) { create(:upload, uploader: 'GitlabUploader') } - include_examples 'raises a SanityCheckError' - end - - context 'model types mismatch' do - let!(:outlier) { create(:upload, model_type: 'WrongModel') } - - include_examples 'raises a SanityCheckError' + include_examples 'raises a SanityCheckError', /Multiple uploaders found/ end context 'mount point not found' do - include_examples 'raises a SanityCheckError' do + include_examples 'raises a SanityCheckError', /Mount point [a-z:]+ not found in/ do let(:mount_point) { :potato } end end |