summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRobert Speicher <robert@gitlab.com>2016-07-21 19:51:58 +0000
committerRobert Speicher <robert@gitlab.com>2016-07-21 19:51:58 +0000
commit3f03699e971b513ecf2bee5a91cf4149cdc122e6 (patch)
treec0af8b521bfaab5431f50e20580ab9acbc911413
parent89292551295418cf4b5b90ce904a6b41f19a8be3 (diff)
parent97c4a1dceab97cd1388f5bdb1d3f4fd3d228f94b (diff)
downloadgitlab-ce-3f03699e971b513ecf2bee5a91cf4149cdc122e6.tar.gz
Merge branch 'rs-bulk-issues-update-spec' into 'master'
Refactor Issues::BulkUpdateService spec - Create fewer Issue objects; 2 is as good as 5 for these cases. This gives us a nice little speed improvement. - Don't `describe` Symbols. - Simplify object creation. - Lessen "mystery guest" antipattern See merge request !5380
-rw-r--r--spec/factories/issues.rb10
-rw-r--r--spec/services/issues/bulk_update_service_spec.rb131
2 files changed, 65 insertions, 76 deletions
diff --git a/spec/factories/issues.rb b/spec/factories/issues.rb
index e72aa9479b7..2c0a2dd94ca 100644
--- a/spec/factories/issues.rb
+++ b/spec/factories/issues.rb
@@ -18,5 +18,15 @@ FactoryGirl.define do
factory :closed_issue, traits: [:closed]
factory :reopened_issue, traits: [:reopened]
+
+ factory :labeled_issue do
+ transient do
+ labels []
+ end
+
+ after(:create) do |issue, evaluator|
+ issue.update_attributes(labels: evaluator.labels)
+ end
+ end
end
end
diff --git a/spec/services/issues/bulk_update_service_spec.rb b/spec/services/issues/bulk_update_service_spec.rb
index ba3a4dfc048..321b54ac39d 100644
--- a/spec/services/issues/bulk_update_service_spec.rb
+++ b/spec/services/issues/bulk_update_service_spec.rb
@@ -1,118 +1,106 @@
require 'spec_helper'
describe Issues::BulkUpdateService, services: true do
- let(:user) { create(:user) }
- let(:project) { Projects::CreateService.new(user, namespace: user.namespace, name: 'test').execute }
+ let(:user) { create(:user) }
+ let(:project) { create(:empty_project, namespace: user.namespace) }
- let!(:result) { Issues::BulkUpdateService.new(project, user, params).execute }
+ def bulk_update(issues, extra_params = {})
+ bulk_update_params = extra_params
+ .reverse_merge(issues_ids: Array(issues).map(&:id).join(','))
- describe :close_issue do
- let(:issues) { create_list(:issue, 5, project: project) }
- let(:params) do
- {
- state_event: 'close',
- issues_ids: issues.map(&:id).join(',')
- }
- end
+ Issues::BulkUpdateService.new(project, user, bulk_update_params).execute
+ end
+
+ describe 'close issues' do
+ let(:issues) { create_list(:issue, 2, project: project) }
it 'succeeds and returns the correct number of issues updated' do
+ result = bulk_update(issues, state_event: 'close')
+
expect(result[:success]).to be_truthy
expect(result[:count]).to eq(issues.count)
end
it 'closes all the issues passed' do
+ bulk_update(issues, state_event: 'close')
+
expect(project.issues.opened).to be_empty
expect(project.issues.closed).not_to be_empty
end
end
- describe :reopen_issues do
- let(:issues) { create_list(:closed_issue, 5, project: project) }
- let(:params) do
- {
- state_event: 'reopen',
- issues_ids: issues.map(&:id).join(',')
- }
- end
+ describe 'reopen issues' do
+ let(:issues) { create_list(:closed_issue, 2, project: project) }
it 'succeeds and returns the correct number of issues updated' do
+ result = bulk_update(issues, state_event: 'reopen')
+
expect(result[:success]).to be_truthy
expect(result[:count]).to eq(issues.count)
end
it 'reopens all the issues passed' do
+ bulk_update(issues, state_event: 'reopen')
+
expect(project.issues.closed).to be_empty
expect(project.issues.opened).not_to be_empty
end
end
describe 'updating assignee' do
- let(:issue) do
- create(:issue, project: project) { |issue| issue.update_attributes(assignee: user) }
- end
-
- let(:params) do
- {
- assignee_id: assignee_id,
- issues_ids: issue.id.to_s
- }
- end
+ let(:issue) { create(:issue, project: project, assignee: user) }
context 'when the new assignee ID is a valid user' do
- let(:new_assignee) { create(:user) }
- let(:assignee_id) { new_assignee.id }
-
it 'succeeds' do
+ result = bulk_update(issue, assignee_id: create(:user).id)
+
expect(result[:success]).to be_truthy
expect(result[:count]).to eq(1)
end
it 'updates the assignee to the use ID passed' do
- expect(issue.reload.assignee).to eq(new_assignee)
+ assignee = create(:user)
+
+ expect { bulk_update(issue, assignee_id: assignee.id) }
+ .to change { issue.reload.assignee }.from(user).to(assignee)
end
end
context 'when the new assignee ID is -1' do
- let(:assignee_id) { -1 }
-
it 'unassigns the issues' do
- expect(issue.reload.assignee).to be_nil
+ expect { bulk_update(issue, assignee_id: -1) }
+ .to change { issue.reload.assignee }.to(nil)
end
end
context 'when the new assignee ID is not present' do
- let(:assignee_id) { nil }
-
it 'does not unassign' do
- expect(issue.reload.assignee).to eq(user)
+ expect { bulk_update(issue, assignee_id: nil) }
+ .not_to change { issue.reload.assignee }
end
end
end
describe 'updating milestones' do
- let(:issue) { create(:issue, project: project) }
+ let(:issue) { create(:issue, project: project) }
let(:milestone) { create(:milestone, project: project) }
- let(:params) do
- {
- issues_ids: issue.id.to_s,
- milestone_id: milestone.id
- }
- end
-
it 'succeeds' do
+ result = bulk_update(issue, milestone_id: milestone.id)
+
expect(result[:success]).to be_truthy
expect(result[:count]).to eq(1)
end
it 'updates the issue milestone' do
- expect(project.issues.first.milestone).to eq(milestone)
+ expect { bulk_update(issue, milestone_id: milestone.id) }
+ .to change { issue.reload.milestone }.from(nil).to(milestone)
end
end
describe 'updating labels' do
def create_issue_with_labels(labels)
- create(:issue, project: project) { |issue| issue.update_attributes(labels: labels) }
+ create(:labeled_issue, project: project, labels: labels)
end
let(:bug) { create(:label, project: project) }
@@ -129,15 +117,18 @@ describe Issues::BulkUpdateService, services: true do
let(:add_labels) { [] }
let(:remove_labels) { [] }
- let(:params) do
+ let(:bulk_update_params) do
{
- label_ids: labels.map(&:id),
- add_label_ids: add_labels.map(&:id),
+ label_ids: labels.map(&:id),
+ add_label_ids: add_labels.map(&:id),
remove_label_ids: remove_labels.map(&:id),
- issues_ids: issues.map(&:id).join(',')
}
end
+ before do
+ bulk_update(issues, bulk_update_params)
+ end
+
context 'when label_ids are passed' do
let(:issues) { [issue_all_labels, issue_no_labels] }
let(:labels) { [bug, regression] }
@@ -263,40 +254,28 @@ describe Issues::BulkUpdateService, services: true do
end
end
- describe :subscribe_issues do
- let(:issues) { create_list(:issue, 5, project: project) }
- let(:params) do
- {
- subscription_event: 'subscribe',
- issues_ids: issues.map(&:id).join(',')
- }
- end
+ describe 'subscribe to issues' do
+ let(:issues) { create_list(:issue, 2, project: project) }
it 'subscribes the given user' do
- issues.each do |issue|
- expect(issue.subscribed?(user)).to be_truthy
- end
- end
- end
+ bulk_update(issues, subscription_event: 'subscribe')
- describe :unsubscribe_issues do
- let(:issues) { create_list(:closed_issue, 5, project: project) }
- let(:params) do
- {
- subscription_event: 'unsubscribe',
- issues_ids: issues.map(&:id).join(',')
- }
+ expect(issues).to all(be_subscribed(user))
end
+ end
- before do
- issues.each do |issue|
+ describe 'unsubscribe from issues' do
+ let(:issues) do
+ create_list(:closed_issue, 2, project: project) do |issue|
issue.subscriptions.create(user: user, subscribed: true)
end
end
it 'unsubscribes the given user' do
+ bulk_update(issues, subscription_event: 'unsubscribe')
+
issues.each do |issue|
- expect(issue.subscribed?(user)).to be_falsey
+ expect(issue).not_to be_subscribed(user)
end
end
end