diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2020-09-29 03:09:59 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2020-09-29 03:09:59 +0300 |
commit | a111182345afb4625cf044ca829f946c7fc3d8a6 (patch) | |
tree | 934b134682850661ef81134e72faf6a2fc7917ba /spec | |
parent | e994d22d1d0b24e6920a6cf7202601c8318ec0a2 (diff) |
Add latest changes from gitlab-org/gitlab@master
Diffstat (limited to 'spec')
11 files changed, 440 insertions, 19 deletions
diff --git a/spec/graphql/resolvers/concerns/looks_ahead_spec.rb b/spec/graphql/resolvers/concerns/looks_ahead_spec.rb index f13823085b8..ebea9e5522b 100644 --- a/spec/graphql/resolvers/concerns/looks_ahead_spec.rb +++ b/spec/graphql/resolvers/concerns/looks_ahead_spec.rb @@ -117,20 +117,6 @@ RSpec.describe LooksAhead do query.result end - context 'the feature flag is off' do - before do - stub_feature_flags(described_class::FEATURE_FLAG => false) - end - - it_behaves_like 'a working query on the test schema' - - it 'does not preload labels on issues' do - expect(the_user.issues).not_to receive(:preload).with(:labels) - - query.result - end - end - it 'issues fewer queries than the naive approach' do the_user.reload # ensure no attributes are loaded before we begin naive = <<-GQL diff --git a/spec/lib/gitlab/ci/trace/metrics_spec.rb b/spec/lib/gitlab/ci/trace/metrics_spec.rb index 1163eb6fc25..6518d0ab075 100644 --- a/spec/lib/gitlab/ci/trace/metrics_spec.rb +++ b/spec/lib/gitlab/ci/trace/metrics_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Gitlab::Ci::Trace::Metrics do +RSpec.describe Gitlab::Ci::Trace::Metrics, :prometheus do describe '#increment_trace_bytes' do context 'when incrementing by more than one' do it 'increments a single counter' do diff --git a/spec/models/design_management/design_collection_spec.rb b/spec/models/design_management/design_collection_spec.rb index 8575cc80b5b..bc8330c7dd3 100644 --- a/spec/models/design_management/design_collection_spec.rb +++ b/spec/models/design_management/design_collection_spec.rb @@ -101,6 +101,18 @@ RSpec.describe DesignManagement::DesignCollection do end end + describe "#empty?" do + it "is true when the design collection has no designs" do + expect(collection).to be_empty + end + + it "is false when the design collection has designs" do + create(:design, issue: issue) + + expect(collection).not_to be_empty + end + end + describe "#versions" do it "includes versions for all designs" do version_1 = create(:design_version) diff --git a/spec/models/design_management/design_spec.rb b/spec/models/design_management/design_spec.rb index d4adc0d42d0..2ce9f00a056 100644 --- a/spec/models/design_management/design_spec.rb +++ b/spec/models/design_management/design_spec.rb @@ -206,6 +206,15 @@ RSpec.describe DesignManagement::Design do end end + describe ".build_full_path" do + it "builds the full path for a design" do + design = build(:design, issue: issue, filename: "hello.jpg") + expected_path = "#{DesignManagement.designs_directory}/issue-#{design.issue.iid}/hello.jpg" + + expect(described_class.build_full_path(issue, design)).to eq(expected_path) + end + end + describe '#visible_in?' do let_it_be(:issue) { create(:issue, project: issue.project) } diff --git a/spec/services/design_management/copy_design_collection/copy_service_spec.rb b/spec/services/design_management/copy_design_collection/copy_service_spec.rb new file mode 100644 index 00000000000..09a4df59c6a --- /dev/null +++ b/spec/services/design_management/copy_design_collection/copy_service_spec.rb @@ -0,0 +1,251 @@ +# frozen_string_literal: true +require 'spec_helper' + +RSpec.describe DesignManagement::CopyDesignCollection::CopyService, :clean_gitlab_redis_shared_state do + include DesignManagementTestHelpers + + let_it_be(:user) { create(:user) } + let_it_be(:project) { create(:project) } + let_it_be(:issue, refind: true) { create(:issue, project: project) } + let(:target_issue) { create(:issue) } + + subject { described_class.new(project, user, issue: issue, target_issue: target_issue).execute } + + before do + enable_design_management + end + + shared_examples 'service error' do |message:| + it 'returns an error response', :aggregate_failures do + expect(subject).to be_kind_of(ServiceResponse) + expect(subject).to be_error + expect(subject.message).to eq(message) + end + end + + shared_examples 'service success' do + it 'returns a success response', :aggregate_failures do + expect(subject).to be_kind_of(ServiceResponse) + expect(subject).to be_success + end + end + + include_examples 'service error', message: 'User cannot copy design collection to issue' + + context 'when user has permission to read the design collection' do + before_all do + project.add_reporter(user) + end + + include_examples 'service error', message: 'User cannot copy design collection to issue' + + context 'when the user also has permission to admin the target issue' do + let(:target_repository) { target_issue.project.design_repository } + + before do + target_issue.project.add_reporter(user) + end + + include_examples 'service error', message: 'Target design collection must first be queued' + + context 'when the target design collection has been queued' do + before do + target_issue.design_collection.start_copy! + end + + include_examples 'service error', message: 'Design collection has no designs' + + context 'when design collection has designs' do + let_it_be(:designs) do + create_list(:design, 3, :with_lfs_file, :with_relative_position, issue: issue, project: project) + end + + context 'when target issue already has designs' do + before do + create(:design, issue: target_issue, project: target_issue.project) + end + + include_examples 'service error', message: 'Target design collection already has designs' + end + + include_examples 'service success' + + it 'creates a design repository for the target project' do + expect { subject }.to change { target_repository.exists? }.from(false).to(true) + end + + context 'when the target project already has a design repository' do + before do + target_repository.create_if_not_exists + end + + include_examples 'service success' + end + + it 'copies the designs correctly', :aggregate_failures do + expect { subject }.to change { target_issue.designs.count }.by(3) + + old_designs = issue.designs.ordered + new_designs = target_issue.designs.ordered + + new_designs.zip(old_designs).each do |new_design, old_design| + expect(new_design).to have_attributes( + filename: old_design.filename, + relative_position: old_design.relative_position, + issue: target_issue, + project: target_issue.project + ) + end + end + + it 'copies the design versions correctly', :aggregate_failures do + expect { subject }.to change { target_issue.design_versions.count }.by(3) + + old_versions = issue.design_versions.ordered + new_versions = target_issue.design_versions.ordered + + new_versions.zip(old_versions).each do |new_version, old_version| + expect(new_version).to have_attributes( + created_at: old_version.created_at, + author_id: old_version.author_id + ) + expect(new_version.designs.pluck(:filename)).to eq(old_version.designs.pluck(:filename)) + expect(new_version.actions.pluck(:event)).to eq(old_version.actions.pluck(:event)) + end + end + + it 'copies the design actions correctly', :aggregate_failures do + expect { subject }.to change { DesignManagement::Action.count }.by(3) + + old_actions = issue.design_versions.ordered.flat_map(&:actions) + new_actions = target_issue.design_versions.ordered.flat_map(&:actions) + + new_actions.zip(old_actions).each do |new_action, old_action| + # This is a way to identify if the versions linked to the actions + # are correct is to compare design filenames, as the SHA changes. + new_design_filenames = new_action.version.designs.ordered.pluck(:filename) + old_design_filenames = old_action.version.designs.ordered.pluck(:filename) + + expect(new_design_filenames).to eq(old_design_filenames) + expect(new_action.event).to eq(old_action.event) + expect(new_action.design.filename).to eq(old_action.design.filename) + end + end + + it 'copies design notes correctly', :aggregate_failures, :sidekiq_inline do + note = create(:diff_note_on_design, noteable: designs.first, project: project) + + expect { subject }.to change { Note.count }.by(1) + + new_note = target_issue.designs.first.notes.first + + expect(new_note).to have_attributes( + type: note.type, + author_id: note.author_id, + note: note.note, + position: note.position + ) + end + + it 'links the LfsObjects' do + expect { subject }.to change { target_issue.project.lfs_objects.count }.by(3) + end + + it 'copies the Git repository data', :aggregate_failures do + subject + + commit_shas = target_repository.commits('master', limit: 99).map(&:id) + + expect(commit_shas).to include(*target_issue.design_versions.ordered.pluck(:sha)) + end + + it 'creates a master branch if none previously existed' do + expect { subject }.to change { target_repository.branch_names }.from([]).to(['master']) + end + + it 'leaves the design collection in the correct copy state' do + subject + + expect(target_issue.design_collection).to be_copy_ready + end + + describe 'rollback' do + before do + # Ensure the very last step throws an error + expect_next_instance_of(described_class) do |service| + expect(service).to receive(:finalize!).and_raise + end + end + + include_examples 'service error', message: 'Designs were unable to be copied successfully' + + it 'rollsback all PostgreSQL data created', :aggregate_failures do + expect { subject }.not_to change { + [ + DesignManagement::Design.count, + DesignManagement::Action.count, + DesignManagement::Version.count, + Note.count + ] + } + + collections = [ + target_issue.design_collection, + target_issue.designs, + target_issue.design_versions + ] + + expect(collections).to all(be_empty) + end + + it 'does not alter master branch', :aggregate_failures do + # Add some Git data to the target_repository, so we are testing + # that any original data remains + issue_2 = create(:issue, project: target_issue.project) + create(:design, :with_file, issue: issue_2, project: target_issue.project) + + expect { subject }.not_to change { + expect(target_repository.commits('master', limit: 10).size).to eq(1) + } + end + + it 'sets the design collection copy state' do + subject + + expect(target_issue.design_collection).to be_copy_error + end + end + end + end + end + end + + describe 'Alert if schema changes', :aggregate_failures do + let_it_be(:config_file) { Rails.root.join('lib/gitlab/design_management/copy_design_collection_model_attributes.yml') } + let_it_be(:config) { YAML.load_file(config_file).symbolize_keys } + + %w(Design Action Version).each do |model| + specify do + attributes = config["#{model.downcase}_attributes".to_sym] || [] + ignored_attributes = config["ignore_#{model.downcase}_attributes".to_sym] + + expect(attributes + ignored_attributes).to contain_exactly( + *DesignManagement.const_get(model, false).column_names + ), failure_message(model) + end + end + + def failure_message(model) + <<-MSG + The schema of the `#{model}` model has changed. + + `#{described_class.name}` refers to specific lists of attributes of `#{model}` to either + copy or ignore, so that we continue to copy designs correctly after schema changes. + + Please update: + #{config_file} + to reflect the latest changes to `#{model}`. See that file for more information. + MSG + end + end +end diff --git a/spec/services/design_management/copy_design_collection/queue_service_spec.rb b/spec/services/design_management/copy_design_collection/queue_service_spec.rb new file mode 100644 index 00000000000..2d9ea4633a0 --- /dev/null +++ b/spec/services/design_management/copy_design_collection/queue_service_spec.rb @@ -0,0 +1,51 @@ +# frozen_string_literal: true +require 'spec_helper' + +RSpec.describe DesignManagement::CopyDesignCollection::QueueService, :clean_gitlab_redis_shared_state do + include DesignManagementTestHelpers + + let_it_be(:user) { create(:user) } + let_it_be(:issue) { create(:issue) } + let_it_be(:target_issue, refind: true) { create(:issue) } + let_it_be(:design) { create(:design, issue: issue, project: issue.project) } + + subject { described_class.new(user, issue, target_issue).execute } + + before do + enable_design_management + end + + it 'returns an error if user does not have permission' do + expect(subject).to be_kind_of(ServiceResponse) + expect(subject).to be_error + expect(subject.message).to eq('User cannot copy designs to issue') + end + + context 'when user has permission' do + before_all do + issue.project.add_reporter(user) + target_issue.project.add_reporter(user) + end + + it 'returns an error if design collection copy_state is not queuable' do + target_issue.design_collection.start_copy! + + expect(subject).to be_kind_of(ServiceResponse) + expect(subject).to be_error + expect(subject.message).to eq('Target design collection copy state must be `ready`') + end + + it 'sets the design collection copy state' do + expect { subject }.to change { target_issue.design_collection.copy_state }.from('ready').to('in_progress') + end + + it 'queues a DesignManagement::CopyDesignCollectionWorker' do + expect { subject }.to change(DesignManagement::CopyDesignCollectionWorker.jobs, :size).by(1) + end + + it 'returns success' do + expect(subject).to be_kind_of(ServiceResponse) + expect(subject).to be_success + end + end +end diff --git a/spec/services/design_management/delete_designs_service_spec.rb b/spec/services/design_management/delete_designs_service_spec.rb index ace63b6e59c..9b2866cab11 100644 --- a/spec/services/design_management/delete_designs_service_spec.rb +++ b/spec/services/design_management/delete_designs_service_spec.rb @@ -105,7 +105,7 @@ RSpec.describe DesignManagement::DeleteDesignsService do end it 'informs the new-version-worker' do - expect(::DesignManagement::NewVersionWorker).to receive(:perform_async).with(Integer) + expect(::DesignManagement::NewVersionWorker).to receive(:perform_async).with(Integer, false) run_service end diff --git a/spec/services/design_management/save_designs_service_spec.rb b/spec/services/design_management/save_designs_service_spec.rb index 94ec12ab8d1..ec241daf3cd 100644 --- a/spec/services/design_management/save_designs_service_spec.rb +++ b/spec/services/design_management/save_designs_service_spec.rb @@ -32,7 +32,7 @@ RSpec.describe DesignManagement::SaveDesignsService do end allow(::DesignManagement::NewVersionWorker) - .to receive(:perform_async).with(Integer).and_return(nil) + .to receive(:perform_async).with(Integer, false).and_return(nil) end def run_service(files_to_upload = nil) @@ -128,6 +128,25 @@ RSpec.describe DesignManagement::SaveDesignsService do expect { run_parallel(blocks) }.to change(DesignManagement::Version, :count).by(parellism) end + context 'when the design collection is in the process of being copied', :clean_gitlab_redis_shared_state do + before do + issue.design_collection.start_copy! + end + + it_behaves_like 'a service error' + end + + context 'when the design collection has a copy error', :clean_gitlab_redis_shared_state do + before do + issue.design_collection.copy_state = 'error' + issue.design_collection.send(:set_stored_copy_state!) + end + + it 'resets the copy state' do + expect { run_service }.to change { issue.design_collection.copy_state }.from('error').to('ready') + end + end + describe 'the response' do it 'includes designs with the expected properties' do updated_designs = response[:designs] @@ -220,7 +239,7 @@ RSpec.describe DesignManagement::SaveDesignsService do counter = Gitlab::UsageDataCounters::DesignsCounter expect(::DesignManagement::NewVersionWorker) - .to receive(:perform_async).once.with(Integer).and_return(nil) + .to receive(:perform_async).once.with(Integer, false).and_return(nil) expect { run_service } .to change { Event.count }.by(2) @@ -254,7 +273,7 @@ RSpec.describe DesignManagement::SaveDesignsService do design_repository.has_visible_content? expect(::DesignManagement::NewVersionWorker) - .to receive(:perform_async).once.with(Integer).and_return(nil) + .to receive(:perform_async).once.with(Integer, false).and_return(nil) expect { service.execute } .to change { issue.designs.count }.from(0).to(2) diff --git a/spec/services/issues/move_service_spec.rb b/spec/services/issues/move_service_spec.rb index c2989dc86cf..7997b8de3fd 100644 --- a/spec/services/issues/move_service_spec.rb +++ b/spec/services/issues/move_service_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' RSpec.describe Issues::MoveService do + include DesignManagementTestHelpers + let_it_be(:user) { create(:user) } let_it_be(:author) { create(:user) } let_it_be(:title) { 'Some issue' } @@ -201,6 +203,54 @@ RSpec.describe Issues::MoveService do expect(copied_notes.order('id ASC').pluck(:note)).to eq(notes.map(&:note)) end end + + context 'issue with a design', :clean_gitlab_redis_shared_state do + let!(:design) { create(:design, :with_lfs_file, issue: old_issue) } + let!(:note) { create(:diff_note_on_design, noteable: design, issue: old_issue, project: old_issue.project) } + let(:subject) { move_service.execute(old_issue, new_project) } + + before do + enable_design_management + end + + it 'calls CopyDesignCollection::QueueService' do + expect(DesignManagement::CopyDesignCollection::QueueService).to receive(:new) + .with(user, old_issue, kind_of(Issue)) + .and_call_original + + subject + end + + it 'logs if QueueService returns an error', :aggregate_failures do + error_message = 'error' + + expect_next_instance_of(DesignManagement::CopyDesignCollection::QueueService) do |service| + expect(service).to receive(:execute).and_return( + ServiceResponse.error(message: error_message) + ) + end + expect(Gitlab::AppLogger).to receive(:error).with(error_message) + + subject + end + + it 'does not call QueueService when the feature flag is disabled' do + stub_feature_flags(design_management_copy_designs: false) + + expect(DesignManagement::CopyDesignCollection::QueueService).not_to receive(:new) + + subject + end + + # Perform a small integration test to ensure the services and worker + # can correctly create designs. + it 'copies the design and its notes', :sidekiq_inline, :aggregate_failures do + new_issue = subject + + expect(new_issue.designs.size).to eq(1) + expect(new_issue.designs.first.notes.size).to eq(1) + end + end end describe 'move permissions' do diff --git a/spec/workers/design_management/copy_design_collection_worker_spec.rb b/spec/workers/design_management/copy_design_collection_worker_spec.rb new file mode 100644 index 00000000000..45bfc47ca7e --- /dev/null +++ b/spec/workers/design_management/copy_design_collection_worker_spec.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe DesignManagement::CopyDesignCollectionWorker, :clean_gitlab_redis_shared_state do + describe '#perform' do + let_it_be(:user) { create(:user) } + let_it_be(:issue) { create(:issue) } + let_it_be(:target_issue) { create(:issue) } + + subject { described_class.new.perform(user.id, issue.id, target_issue.id) } + + it_behaves_like 'an idempotent worker' do + let(:job_args) { [user.id, issue.id, target_issue.id] } + + specify { subject } + end + + it 'calls DesignManagement::CopyDesignCollection::CopyService' do + expect_next_instance_of(DesignManagement::CopyDesignCollection::CopyService) do |service| + expect(service).to receive(:execute).and_return(ServiceResponse.success) + end + + subject + end + + it 'logs if there was an error calling the service' do + message = 'Error message' + + allow_next_instance_of(DesignManagement::CopyDesignCollection::CopyService) do |service| + allow(service).to receive(:execute).and_return(ServiceResponse.error(message: message)) + end + + expect(Gitlab::AppLogger).to receive(:warn).with(message) + + subject + end + end +end diff --git a/spec/workers/design_management/new_version_worker_spec.rb b/spec/workers/design_management/new_version_worker_spec.rb index 4d57c46487e..3320d7a062d 100644 --- a/spec/workers/design_management/new_version_worker_spec.rb +++ b/spec/workers/design_management/new_version_worker_spec.rb @@ -36,6 +36,10 @@ RSpec.describe DesignManagement::NewVersionWorker do expect { worker.perform(version.id) }.to change { Note.system.count }.by(1) end + it 'does not create a system note if skip_system_notes is true' do + expect { worker.perform(version.id, true) }.not_to change { Note.system.count } + end + it 'invokes GenerateImageVersionsService' do expect_next_instance_of(DesignManagement::GenerateImageVersionsService) do |service| expect(service).to receive(:execute) |