Merge branch 'master' into zj/gitlab-ce-merge-if-green

19 files changed, 330 insertions, 21 deletions

diff --git a/spec/controllers/commit_controller_spec.rb b/spec/controllers/commit_controller_spec.rb
index 5337a69e84b..7793bf1e421 100644
--- a/spec/controllers/commit_controller_spec.rb
+++ b/spec/controllers/commit_controller_spec.rb
@@ -110,6 +110,26 @@ describe Projects::CommitController do
         expect(response.body).to match(/^diff --git/)
       end
     end
+
+    context 'commit that removes a submodule' do
+      render_views
+
+      let(:fork_project) { create(:forked_project_with_submodules) }
+      let(:commit) { fork_project.commit('remove-submodule') }
+
+      before do
+        fork_project.team << [user, :master]
+      end
+
+      it 'renders it' do
+        get(:show,
+            namespace_id: fork_project.namespace.to_param,
+            project_id: fork_project.to_param,
+            id: commit.id)
+
+        expect(response).to be_success
+      end
+    end
   end
 
   describe "#branches" do
diff --git a/spec/controllers/groups/milestones_controller_spec.rb b/spec/controllers/groups/milestones_controller_spec.rb
new file mode 100644
index 00000000000..eb0c6ac6d80
--- /dev/null
+++ b/spec/controllers/groups/milestones_controller_spec.rb
@@ -0,0 +1,27 @@
+require 'spec_helper'
+
+describe Groups::MilestonesController do
+  let(:group) { create(:group) }
+  let(:project) { create(:project, group: group) }
+  let(:project2) { create(:empty_project, group: group) }
+  let(:user)    { create(:user) }
+  let(:title) { '肯定不是中文的问题' }
+
+  before do
+    sign_in(user)
+    group.add_owner(user)
+    project.team << [user, :master]
+    controller.instance_variable_set(:@group, group)
+  end
+
+  describe "#create" do
+    it "should create group milestone with Chinese title" do
+      post :create,
+           group_id: group.id,
+           milestone: { project_ids: [project.id, project2.id], title: title }
+
+      expect(response).to redirect_to(group_milestone_path(group, title.to_slug.to_s, title: title))
+      expect(Milestone.where(title: title).count).to eq(2)
+    end
+  end
+end
diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb
index 3e5e1fa87ae..6aaec224f6e 100644
--- a/spec/controllers/projects/merge_requests_controller_spec.rb
+++ b/spec/controllers/projects/merge_requests_controller_spec.rb
@@ -10,6 +10,30 @@ describe Projects::MergeRequestsController do
     project.team << [user, :master]
   end
 
+  describe '#new' do
+    context 'merge request that removes a submodule' do
+      render_views
+
+      let(:fork_project) { create(:forked_project_with_submodules) }
+
+      before do
+        fork_project.team << [user, :master]
+      end
+
+      it 'renders it' do
+        get :new,
+            namespace_id: fork_project.namespace.to_param,
+            project_id: fork_project.to_param,
+            merge_request: {
+              source_branch: 'remove-submodule',
+              target_branch: 'master'
+            }
+
+        expect(response).to be_success
+      end
+    end
+  end
+
   describe "#show" do
     shared_examples "export merge as" do |format|
       it "should generally work" do
diff --git a/spec/controllers/projects/milestones_controller_spec.rb b/spec/controllers/projects/milestones_controller_spec.rb
index 8127efabe6e..d173bb350f1 100644
--- a/spec/controllers/projects/milestones_controller_spec.rb
+++ b/spec/controllers/projects/milestones_controller_spec.rb
@@ -5,7 +5,7 @@ describe Projects::MilestonesController do
   let(:user)    { create(:user) }
   let(:milestone) { create(:milestone, project: project) }
   let(:issue) { create(:issue, project: project, milestone: milestone) }
-  let(:merge_request) { create(:merge_request, source_project: project, target_project: project, milestone: milestone) }
+  let!(:merge_request) { create(:merge_request, source_project: project, target_project: project, milestone: milestone) }
 
   before do
     sign_in(user)
@@ -15,10 +15,9 @@ describe Projects::MilestonesController do
 
   describe "#destroy" do
     it "should remove milestone" do
-      merge_request.reload
       expect(issue.milestone_id).to eq(milestone.id)
 
-      delete :destroy, namespace_id: project.namespace.id, project_id: project.id, id: milestone.id, format: :js
+      delete :destroy, namespace_id: project.namespace.id, project_id: project.id, id: milestone.iid, format: :js
       expect(response).to be_success
 
       expect(Event.first.action).to eq(Event::DESTROYED)
diff --git a/spec/features/notes_on_merge_requests_spec.rb b/spec/features/notes_on_merge_requests_spec.rb
index d7cb3b2e86e..f0fc6916c4d 100644
--- a/spec/features/notes_on_merge_requests_spec.rb
+++ b/spec/features/notes_on_merge_requests_spec.rb
@@ -2,6 +2,7 @@ require 'spec_helper'
 
 describe 'Comments', feature: true do
   include RepoHelpers
+  include WaitForAjax
 
   describe 'On a merge request', js: true, feature: true do
     let!(:merge_request) { create(:merge_request) }
@@ -123,8 +124,8 @@ describe 'Comments', feature: true do
         it 'removes the attachment div and resets the edit form' do
           find('.js-note-attachment-delete').click
           is_expected.not_to have_css('.note-attachment')
-          expect(find('.current-note-edit-form', visible: false)).
-            not_to be_visible
+          is_expected.not_to have_css('.current-note-edit-form')
+          wait_for_ajax
         end
       end
     end
diff --git a/spec/helpers/application_helper_spec.rb b/spec/helpers/application_helper_spec.rb
index 0a64b70d6a6..5568f06639c 100644
--- a/spec/helpers/application_helper_spec.rb
+++ b/spec/helpers/application_helper_spec.rb
@@ -278,7 +278,7 @@ describe ApplicationHelper do
       el = element.next_element
 
       expect(el.name).to eq 'script'
-      expect(el.text).to include "$('.js-timeago').timeago()"
+      expect(el.text).to include "$('.js-timeago').last().timeago()"
     end
 
     it 'allows the script tag to be excluded' do
diff --git a/spec/lib/gitlab/push_data_builder_spec.rb b/spec/lib/gitlab/push_data_builder_spec.rb
index 02710742625..2170399ab5c 100644
--- a/spec/lib/gitlab/push_data_builder_spec.rb
+++ b/spec/lib/gitlab/push_data_builder_spec.rb
@@ -17,9 +17,9 @@ describe 'Gitlab::PushDataBuilder' do
     it { expect(data[:repository][:git_ssh_url]).to eq(project.ssh_url_to_repo) }
     it { expect(data[:repository][:visibility_level]).to eq(project.visibility_level) }
     it { expect(data[:total_commits_count]).to eq(3) }
-    it { expect(data[:added]).to eq(["gitlab-grack"]) }
-    it { expect(data[:modified]).to eq([".gitmodules", "files/ruby/popen.rb", "files/ruby/regex.rb"]) }
-    it { expect(data[:removed]).to eq([]) }
+    it { expect(data[:commits].first[:added]).to eq(["gitlab-grack"]) }
+    it { expect(data[:commits].first[:modified]).to eq([".gitmodules"]) }
+    it { expect(data[:commits].first[:removed]).to eq([]) }
   end
 
   describe :build do
@@ -38,8 +38,5 @@ describe 'Gitlab::PushDataBuilder' do
     it { expect(data[:ref]).to eq('refs/tags/v1.1.0') }
     it { expect(data[:commits]).to be_empty }
     it { expect(data[:total_commits_count]).to be_zero }
-    it { expect(data[:added]).to eq([]) }
-    it { expect(data[:modified]).to eq([]) }
-    it { expect(data[:removed]).to eq([]) }
   end
 end
diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb
index dfbac7b4004..b67b84959d9 100644
--- a/spec/models/application_setting_spec.rb
+++ b/spec/models/application_setting_spec.rb
@@ -36,6 +36,22 @@ describe ApplicationSetting, models: true do
 
   it { expect(setting).to be_valid }
 
+  describe 'validations' do
+    let(:http)  { 'http://example.com' }
+    let(:https) { 'https://example.com' }
+    let(:ftp)   { 'ftp://example.com' }
+
+    it { is_expected.to allow_value(nil).for(:home_page_url) }
+    it { is_expected.to allow_value(http).for(:home_page_url) }
+    it { is_expected.to allow_value(https).for(:home_page_url) }
+    it { is_expected.not_to allow_value(ftp).for(:home_page_url) }
+
+    it { is_expected.to allow_value(nil).for(:after_sign_out_path) }
+    it { is_expected.to allow_value(http).for(:after_sign_out_path) }
+    it { is_expected.to allow_value(https).for(:after_sign_out_path) }
+    it { is_expected.not_to allow_value(ftp).for(:after_sign_out_path) }
+  end
+
   context 'restricted signup domains' do
     it 'set single domain' do
       setting.restricted_signup_domains_raw = 'example.com'
diff --git a/spec/models/broadcast_message_spec.rb b/spec/models/broadcast_message_spec.rb
index d80748f23a4..2b325f44f64 100644
--- a/spec/models/broadcast_message_spec.rb
+++ b/spec/models/broadcast_message_spec.rb
@@ -20,6 +20,21 @@ describe BroadcastMessage do
 
   it { is_expected.to be_valid }
 
+  describe 'validations' do
+    let(:triplet) { '#000' }
+    let(:hex)     { '#AABBCC' }
+
+    it { is_expected.to allow_value(nil).for(:color) }
+    it { is_expected.to allow_value(triplet).for(:color) }
+    it { is_expected.to allow_value(hex).for(:color) }
+    it { is_expected.not_to allow_value('000').for(:color) }
+
+    it { is_expected.to allow_value(nil).for(:font) }
+    it { is_expected.to allow_value(triplet).for(:font) }
+    it { is_expected.to allow_value(hex).for(:font) }
+    it { is_expected.not_to allow_value('000').for(:font) }
+  end
+
   describe :current do
     it "should return last message if time match" do
       broadcast_message = create(:broadcast_message, starts_at: Time.now.yesterday, ends_at: Time.now.tomorrow)
diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb
index 974b52c1833..38a3dc1f4a6 100644
--- a/spec/models/commit_spec.rb
+++ b/spec/models/commit_spec.rb
@@ -107,4 +107,15 @@ eos
     # Include the subject in the repository stub.
     let(:extra_commits) { [subject] }
   end
+
+  describe '#hook_attrs' do
+    let(:data) { commit.hook_attrs(with_changed_files: true) }
+
+    it { expect(data).to be_a(Hash) }
+    it { expect(data[:message]).to include('Add submodule from gitlab.com') }
+    it { expect(data[:timestamp]).to eq('2014-02-27T11:01:38+02:00') }
+    it { expect(data[:added]).to eq(["gitlab-grack"]) }
+    it { expect(data[:modified]).to eq([".gitmodules"]) }
+    it { expect(data[:removed]).to eq([]) }
+  end
 end
diff --git a/spec/models/hooks/web_hook_spec.rb b/spec/models/hooks/web_hook_spec.rb
index 2fdc49f02ee..35042788c65 100644
--- a/spec/models/hooks/web_hook_spec.rb
+++ b/spec/models/hooks/web_hook_spec.rb
@@ -71,5 +71,11 @@ describe ProjectHook do
 
       expect { @project_hook.execute(@data, 'push_hooks') }.to raise_error(RuntimeError)
     end
+
+    it "handles SSL exceptions" do
+      expect(WebHook).to receive(:post).and_raise(OpenSSL::SSL::SSLError.new('SSL error'))
+
+      expect(@project_hook.execute(@data, 'push_hooks')).to eq([false, 'SSL error'])
+    end
   end
 end
diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb
index 319fa0a7c8d..fa261e64c35 100644
--- a/spec/models/repository_spec.rb
+++ b/spec/models/repository_spec.rb
@@ -4,6 +4,7 @@ describe Repository do
   include RepoHelpers
 
   let(:repository) { create(:project).repository }
+  let(:user) { create(:user) }
 
   describe :branch_names_contains do
     subject { repository.branch_names_contains(sample_commit.id) }
@@ -99,5 +100,104 @@ describe Repository do
       it { expect(subject.startline).to eq(186) }
       it { expect(subject.data.lines[2]).to eq("  - Feature: Replace teams with group membership\n") }
     end
+
   end
+
+  describe :add_branch do
+    context 'when pre hooks were successful' do
+      it 'should run without errors' do
+        hook = double(trigger: true)
+        expect(Gitlab::Git::Hook).to receive(:new).exactly(3).times.and_return(hook)
+
+        expect { repository.add_branch(user, 'new_feature', 'master') }.not_to raise_error
+      end
+
+      it 'should create the branch' do
+        allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return(true)
+
+        branch = repository.add_branch(user, 'new_feature', 'master')
+
+        expect(branch.name).to eq('new_feature')
+      end
+    end
+
+    context 'when pre hooks failed' do
+      it 'should get an error' do
+        allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return(false)
+
+        expect do
+          repository.add_branch(user, 'new_feature', 'master')
+        end.to raise_error(GitHooksService::PreReceiveError)
+      end
+
+      it 'should not create the branch' do
+        allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return(false)
+
+        expect do
+          repository.add_branch(user, 'new_feature', 'master')
+        end.to raise_error(GitHooksService::PreReceiveError)
+        expect(repository.find_branch('new_feature')).to be_nil
+      end
+    end
+  end
+
+  describe :rm_branch do
+    context 'when pre hooks were successful' do
+      it 'should run without errors' do
+        allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return(true)
+
+        expect { repository.rm_branch(user, 'feature') }.not_to raise_error
+      end
+
+      it 'should delete the branch' do
+        allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return(true)
+
+        expect { repository.rm_branch(user, 'feature') }.not_to raise_error
+
+        expect(repository.find_branch('feature')).to be_nil
+      end
+    end
+
+    context 'when pre hooks failed' do
+      it 'should get an error' do
+        allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return(false)
+
+        expect do
+          repository.rm_branch(user, 'new_feature')
+        end.to raise_error(GitHooksService::PreReceiveError)
+      end
+
+      it 'should not delete the branch' do
+        allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return(false)
+
+        expect do
+          repository.rm_branch(user, 'feature')
+        end.to raise_error(GitHooksService::PreReceiveError)
+        expect(repository.find_branch('feature')).not_to be_nil
+      end
+    end
+  end
+
+  describe :commit_with_hooks do
+    context 'when pre hooks were successful' do
+      it 'should run without errors' do
+        expect_any_instance_of(GitHooksService).to receive(:execute).and_return(true)
+
+        expect do
+          repository.commit_with_hooks(user, 'feature') { sample_commit.id }
+        end.not_to raise_error
+      end
+    end
+
+    context 'when pre hooks failed' do
+      it 'should get an error' do
+        allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return(false)
+
+        expect do
+          repository.commit_with_hooks(user, 'feature') { sample_commit.id }
+        end.to raise_error(GitHooksService::PreReceiveError)
+      end
+    end
+  end
+
 end
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 4631b12faf1..a0f78d3b336 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -91,7 +91,23 @@ describe User do
   end
 
   describe 'validations' do
-    it { is_expected.to validate_presence_of(:username) }
+    describe 'username' do
+      it 'validates presence' do
+        expect(subject).to validate_presence_of(:username)
+      end
+
+      it 'rejects blacklisted names' do
+        user = build(:user, username: 'dashboard')
+
+        expect(user).not_to be_valid
+        expect(user.errors.values).to eq [['dashboard is a reserved name']]
+      end
+
+      it 'validates uniqueness' do
+        expect(subject).to validate_uniqueness_of(:username)
+      end
+    end
+
     it { is_expected.to validate_presence_of(:projects_limit) }
     it { is_expected.to validate_numericality_of(:projects_limit) }
     it { is_expected.to allow_value(0).for(:projects_limit) }
diff --git a/spec/requests/api/labels_spec.rb b/spec/requests/api/labels_spec.rb
index aff109a9424..667f0dbea5c 100644
--- a/spec/requests/api/labels_spec.rb
+++ b/spec/requests/api/labels_spec.rb
@@ -47,7 +47,7 @@ describe API::API, api: true  do
            name: 'Foo',
            color: '#FFAA'
       expect(response.status).to eq(400)
-      expect(json_response['message']['color']).to eq(['is invalid'])
+      expect(json_response['message']['color']).to eq(['must be a valid color code'])
     end
 
     it 'should return 400 for too long color code' do
@@ -55,7 +55,7 @@ describe API::API, api: true  do
            name: 'Foo',
            color: '#FFAAFFFF'
       expect(response.status).to eq(400)
-      expect(json_response['message']['color']).to eq(['is invalid'])
+      expect(json_response['message']['color']).to eq(['must be a valid color code'])
     end
 
     it 'should return 400 for invalid name' do
@@ -151,12 +151,12 @@ describe API::API, api: true  do
       expect(json_response['message']['title']).to eq(['is invalid'])
     end
 
-    it 'should return 400 for invalid name' do
+    it 'should return 400 when color code is too short' do
       put api("/projects/#{project.id}/labels", user),
           name: 'label1',
           color: '#FF'
       expect(response.status).to eq(400)
-      expect(json_response['message']['color']).to eq(['is invalid'])
+      expect(json_response['message']['color']).to eq(['must be a valid color code'])
     end
 
     it 'should return 400 for too long color code' do
@@ -164,7 +164,7 @@ describe API::API, api: true  do
            name: 'Foo',
            color: '#FFAAFFFF'
       expect(response.status).to eq(400)
-      expect(json_response['message']['color']).to eq(['is invalid'])
+      expect(json_response['message']['color']).to eq(['must be a valid color code'])
     end
   end
 end
diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb
index c59ee7af8ab..24b765f4979 100644
--- a/spec/requests/api/projects_spec.rb
+++ b/spec/requests/api/projects_spec.rb
@@ -742,6 +742,18 @@ describe API::API, api: true  do
         end
       end
 
+      it 'should update visibility_level from public to private' do
+        project3.update_attributes({ visibility_level: Gitlab::VisibilityLevel::PUBLIC })
+
+        project_param = { public: false }
+        put api("/projects/#{project3.id}", user), project_param
+        expect(response.status).to eq(200)
+        project_param.each_pair do |k, v|
+          expect(json_response[k.to_s]).to eq(v)
+        end
+        expect(json_response['visibility_level']).to eq(Gitlab::VisibilityLevel::PRIVATE)
+      end
+
       it 'should not update name to existing name' do
         project_param = { name: project3.name }
         put api("/projects/#{project.id}", user), project_param
diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb
index a9ef2fe5885..2f609c63330 100644
--- a/spec/requests/api/users_spec.rb
+++ b/spec/requests/api/users_spec.rb
@@ -153,7 +153,7 @@ describe API::API, api: true  do
       expect(json_response['message']['projects_limit']).
         to eq(['must be greater than or equal to 0'])
       expect(json_response['message']['username']).
-        to eq([Gitlab::Regex.send(:namespace_regex_message)])
+        to eq([Gitlab::Regex.namespace_regex_message])
     end
 
     it "shouldn't available for non admin users" do
@@ -296,7 +296,7 @@ describe API::API, api: true  do
       expect(json_response['message']['projects_limit']).
         to eq(['must be greater than or equal to 0'])
       expect(json_response['message']['username']).
-        to eq([Gitlab::Regex.send(:namespace_regex_message)])
+        to eq([Gitlab::Regex.namespace_regex_message])
     end
 
     context "with existing user" do
diff --git a/spec/services/git_hooks_service_spec.rb b/spec/services/git_hooks_service_spec.rb
new file mode 100644
index 00000000000..7e018d3c9fe
--- /dev/null
+++ b/spec/services/git_hooks_service_spec.rb
@@ -0,0 +1,53 @@
+require 'spec_helper'
+
+describe GitHooksService do
+  include RepoHelpers
+
+  let(:user)    { create :user }
+  let(:project) { create :project }
+  let(:service) { GitHooksService.new }
+
+  before do
+    @blankrev = Gitlab::Git::BLANK_SHA
+    @oldrev = sample_commit.parent_id
+    @newrev = sample_commit.id
+    @ref = 'refs/heads/feature'
+    @repo_path = project.repository.path_to_repo
+  end
+
+  describe '#execute' do
+
+    context 'when receive hooks were successful' do
+      it 'should call post-receive hook' do
+        hook = double(trigger: true)
+        expect(Gitlab::Git::Hook).to receive(:new).exactly(3).times.and_return(hook)
+
+        expect(service.execute(user, @repo_path, @blankrev, @newrev, @ref) { }).to eq(true)
+      end
+    end
+
+    context 'when pre-receive hook failed' do
+      it 'should not call post-receive hook' do
+        expect(service).to receive(:run_hook).with('pre-receive').and_return(false)
+        expect(service).not_to receive(:run_hook).with('post-receive')
+
+        expect do
+          service.execute(user, @repo_path, @blankrev, @newrev, @ref)
+        end.to raise_error(GitHooksService::PreReceiveError)
+      end
+    end
+
+    context 'when update hook failed' do
+      it 'should not call post-receive hook' do
+        expect(service).to receive(:run_hook).with('pre-receive').and_return(true)
+        expect(service).to receive(:run_hook).with('update').and_return(false)
+        expect(service).not_to receive(:run_hook).with('post-receive')
+
+        expect do
+          service.execute(user, @repo_path, @blankrev, @newrev, @ref)
+        end.to raise_error(GitHooksService::PreReceiveError)
+      end
+    end
+
+  end
+end
diff --git a/spec/support/test_env.rb b/spec/support/test_env.rb
index 787670e9297..78b9a0f42fa 100644
--- a/spec/support/test_env.rb
+++ b/spec/support/test_env.rb
@@ -21,7 +21,8 @@ module TestEnv
   # We currently only need a subset of the branches
   FORKED_BRANCH_SHA = {
     'add-submodule-version-bump' => '3f547c08',
-    'master' => '5937ac0'
+    'master' => '5937ac0',
+    'remove-submodule' => '2a33e0c0'
   }
 
   # Test environment
diff --git a/spec/support/wait_for_ajax.rb b/spec/support/wait_for_ajax.rb
new file mode 100644
index 00000000000..692d219e9f1
--- /dev/null
+++ b/spec/support/wait_for_ajax.rb
@@ -0,0 +1,11 @@
+module WaitForAjax
+  def wait_for_ajax
+    Timeout.timeout(Capybara.default_wait_time) do
+      loop until finished_all_ajax_requests?
+    end
+  end
+
+  def finished_all_ajax_requests?
+    page.evaluate_script('jQuery.active').zero?
+  end
+end