From 3f029144607428aa21e81d1d4b40544b835f3d80 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=A9my=20Coutable?= <remy@rymai.me>
Date: Wed, 26 Oct 2016 19:19:17 +0200
Subject: [PATCH] Complete and improve specs
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Signed-off-by: Rémy Coutable <remy@rymai.me>
---
app/controllers/projects_controller.rb | 4 +-
.../merge_requests_controller_spec.rb | 68 +++++++++++++++----
...f_mergeable_with_unresolved_discussions.rb | 50 +++++++++-----
spec/models/merge_request_spec.rb | 39 +++++------
4 files changed, 105 insertions(+), 56 deletions(-)
diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb
index ee72c8ba72f..6988527a3be 100644
--- a/app/controllers/projects_controller.rb
+++ b/app/controllers/projects_controller.rb
@@ -334,9 +334,9 @@ class ProjectsController < Projects::ApplicationController
:issues_tracker_id, :default_branch,
:visibility_level, :import_url, :last_activity_at, :namespace_id, :avatar,
:build_allow_git_fetch, :build_timeout_in_minutes, :build_coverage_regex,
- :public_builds, :only_allow_merge_if_build_succeeds,
+ :public_builds, :only_allow_merge_if_build_succeeds, :request_access_enabled,
:only_allow_merge_if_all_discussions_are_resolved,
- :request_access_enabled, :lfs_enabled, project_feature_attributes
+ :lfs_enabled, project_feature_attributes
)
end
diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb
index 79820e9a435..49127aecc63 100644
--- a/spec/controllers/projects/merge_requests_controller_spec.rb
+++ b/spec/controllers/projects/merge_requests_controller_spec.rb
@@ -298,28 +298,68 @@ describe Projects::MergeRequestsController do
end
end
- context 'when project project has unresolved discussion' do
- before do
- project.update_column(:only_allow_merge_if_all_discussions_are_resolved, allowed)
- end
+ describe 'only_allow_merge_if_all_discussions_are_resolved? setting' do
+ let(:merge_request) { create(:merge_request_with_diff_notes, source_project: project, author: user) }
+
+ context 'when enabled' do
+ before do
+ project.update_column(:only_allow_merge_if_all_discussions_are_resolved, true)
+ end
+
+ context 'with unresolved discussion' do
+ before do
+ expect(merge_request).not_to be_discussions_resolved
+ end
+
+ it 'returns :failed' do
+ merge_with_sha
+
+ expect(assigns(:status)).to eq(:failed)
+ end
+ end
- context "when the only_allow_merge_if_all_discussions_are_resolved? is true" do
- let(:allowed) { true }
+ context 'with all discussions resolved' do
+ before do
+ merge_request.discussions.each { |d| d.resolve!(user) }
+ expect(merge_request).to be_discussions_resolved
+ end
- it 'returns :failed' do
- merge_with_sha
+ it 'returns :success' do
+ merge_with_sha
- expect(assigns(:status)).to eq(:failed)
+ expect(assigns(:status)).to eq(:success)
+ end
end
end
- context "when the only_allow_merge_if_all_discussions_are_resolved? is false" do
- let(:allowed) { false }
+ context 'when disabled' do
+ before do
+ project.update_column(:only_allow_merge_if_all_discussions_are_resolved, false)
+ end
+
+ context 'with unresolved discussion' do
+ before do
+ expect(merge_request).not_to be_discussions_resolved
+ end
+
+ it 'returns :success' do
+ merge_with_sha
- it 'returns :failed' do
- merge_with_sha
+ expect(assigns(:status)).to eq(:success)
+ end
+ end
+
+ context 'with all discussions resolved' do
+ before do
+ merge_request.discussions.each { |d| d.resolve!(user) }
+ expect(merge_request).to be_discussions_resolved
+ end
+
+ it 'returns :success' do
+ merge_with_sha
- expect(assigns(:status)).to eq(:success)
+ expect(assigns(:status)).to eq(:success)
+ end
end
end
end
diff --git a/spec/features/merge_requests/check_if_mergeable_with_unresolved_discussions.rb b/spec/features/merge_requests/check_if_mergeable_with_unresolved_discussions.rb
index 100ddda0167..7f11db3c417 100644
--- a/spec/features/merge_requests/check_if_mergeable_with_unresolved_discussions.rb
+++ b/spec/features/merge_requests/check_if_mergeable_with_unresolved_discussions.rb
@@ -1,30 +1,30 @@
require 'spec_helper'
feature 'Check if mergeable with unresolved discussions', js: true, feature: true do
- let!(:user) { create(:user) }
- let!(:project) { create(:project, :public, only_allow_merge_if_all_discussions_are_resolved: allowed) }
- let!(:merge_request) { create(:merge_request_with_diff_notes, source_project: project, author: user, title: "Bug NS-04" ) }
+ let(:user) { create(:user) }
+ let(:project) { create(:project) }
+ let!(:merge_request) { create(:merge_request_with_diff_notes, source_project: project, author: user) }
before do
login_as user
project.team << [user, :master]
end
- context 'when only_allow_merge_if_all_discussions_are_resolved is false' do
- let(:allowed) { false }
-
- it 'allows MR to be merged' do
- visit_merge_request(merge_request)
-
- expect(page).to have_button 'Accept Merge Request'
+ context 'when project.only_allow_merge_if_all_discussions_are_resolved == true' do
+ before do
+ project.update_column(:only_allow_merge_if_all_discussions_are_resolved, true)
end
- end
- context 'when only_allow_merge_if_all_discussions_are_resolved is true' do
- let(:allowed) { true }
+ context 'with unresolved discussions' do
+ it 'does not allow to merge' do
+ visit_merge_request(merge_request)
- context "when discussions are resolved" do
+ expect(page).not_to have_button 'Accept Merge Request'
+ expect(page).to have_content('This merge request has unresolved discussions')
+ end
+ end
+ context 'with all discussions resolved' do
before do
merge_request.discussions.each { |d| d.resolve!(user) }
end
@@ -35,14 +35,30 @@ feature 'Check if mergeable with unresolved discussions', js: true, feature: tru
expect(page).to have_button 'Accept Merge Request'
end
end
+ end
- context "when discussions are unresolved" do
+ context 'when project.only_allow_merge_if_all_discussions_are_resolved == false' do
+ before do
+ project.update_column(:only_allow_merge_if_all_discussions_are_resolved, false)
+ end
+ context 'with unresolved discussions' do
it 'does not allow to merge' do
visit_merge_request(merge_request)
- expect(page).not_to have_button 'Accept Merge Request'
- expect(page).to have_content('This merge request has unresolved discussions')
+ expect(page).to have_button 'Accept Merge Request'
+ end
+ end
+
+ context 'with all discussions resolved' do
+ before do
+ merge_request.discussions.each { |d| d.resolve!(user) }
+ end
+
+ it 'allows MR to be merged' do
+ visit_merge_request(merge_request)
+
+ expect(page).to have_button 'Accept Merge Request'
end
end
end
diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb
index f3d0373e6d7..fb032a89d50 100644
--- a/spec/models/merge_request_spec.rb
+++ b/spec/models/merge_request_spec.rb
@@ -825,11 +825,8 @@ describe MergeRequest, models: true do
end
context 'when failed' do
- before { allow(subject).to receive(:broken?) { false } }
-
- context 'when project settings restrict to merge only if build succeeds and build failed' do
+ context 'when #mergeable_ci_state? is false' do
before do
- project.only_allow_merge_if_build_succeeds = true
allow(subject).to receive(:mergeable_ci_state?) { false }
end
@@ -838,9 +835,8 @@ describe MergeRequest, models: true do
end
end
- context "when project settings restrict to merge only when all the discussions are resolved" do
+ context 'when #mergeable_discussions_state? is false' do
before do
- project.only_allow_merge_if_all_discussions_are_resolved = true
allow(subject).to receive(:mergeable_discussions_state?) { false }
end
@@ -899,45 +895,42 @@ describe MergeRequest, models: true do
end
describe '#mergeable_discussions_state?' do
- let!(:user) { create(:user) }
- let!(:project) { create(:project, only_allow_merge_if_all_discussions_are_resolved: allowed) }
-
- subject { create(:merge_request_with_diff_notes, source_project: project) }
+ let(:merge_request) { create(:merge_request_with_diff_notes, source_project: project) }
- context 'when is true' do
- let(:allowed) { true }
+ context 'when project.only_allow_merge_if_all_discussions_are_resolved == true' do
+ let(:project) { create(:project, only_allow_merge_if_all_discussions_are_resolved: true) }
- context 'when discussions are resolved' do
+ context 'with all discussions resolved' do
before do
- subject.discussions.each { |d| d.resolve!(user) }
+ merge_request.discussions.each { |d| d.resolve!(merge_request.author) }
end
it 'returns true' do
- expect(subject.mergeable_discussions_state?).to be_truthy
+ expect(merge_request.mergeable_discussions_state?).to be_truthy
end
end
- context 'when discussions are unresolved' do
+ context 'with unresolved discussions' do
before do
- subject.discussions.map(&:unresolve!)
+ merge_request.discussions.each(&:unresolve!)
end
it 'returns false' do
- expect(subject.mergeable_discussions_state?).to be_falsey
+ expect(merge_request.mergeable_discussions_state?).to be_falsey
end
end
end
- context 'when is false' do
- let(:allowed) { false }
+ context 'when project.only_allow_merge_if_all_discussions_are_resolved == false' do
+ let(:project) { create(:project, only_allow_merge_if_all_discussions_are_resolved: false) }
- context 'when discussions are unresolved' do
+ context 'with unresolved discussions' do
before do
- subject.discussions.map(&:unresolve!)
+ merge_request.discussions.each(&:unresolve!)
end
it 'returns true' do
- expect(subject.mergeable_discussions_state?).to be_truthy
+ expect(merge_request.mergeable_discussions_state?).to be_truthy
end
end
end
--
GitLab