Merge branch 'master' of gitlab.com:gitlab-org/gitlab-ce into ce_upstream

spec/models/project_services/jira_service_spec.rb

@@ -33,6 +33,41 @@ describe JiraService, models: true do
     end
   end
+  describe '#can_test?' do
+    let(:jira_service) { described_class.new }
+
+    it 'returns false if username is blank' do
+      allow(jira_service).to receive_messages(
+        url: 'http://jira.example.com',
+        username: '',
+        password: '12345678'
+      )
+
+      expect(jira_service.can_test?).to be_falsy
+    end
+
+    it 'returns false if password is blank' do
+      allow(jira_service).to receive_messages(
+        url: 'http://jira.example.com',
+        username: 'tester',
+        password: ''
+      )
+
+      expect(jira_service.can_test?).to be_falsy
+    end
+
+    it 'returns true if password and username are present' do
+      jira_service = described_class.new
+      allow(jira_service).to receive_messages(
+        url: 'http://jira.example.com',
+        username: 'tester',
+        password: '12345678'
+      )
+
+      expect(jira_service.can_test?).to be_truthy
+    end
+  end
+
   describe "Execute" do
     let(:user)    { create(:user) }
     let(:project) { create(:project) }
@@ -46,16 +81,19 @@ describe JiraService, models: true do
         service_hook: true,
         url: 'http://jira.example.com',
         username: 'gitlab_jira_username',
-        password: 'gitlab_jira_password'
+        password: 'gitlab_jira_password',
+        project_key: 'GitLabProject'
       )
       @jira_service.save
-      project_url = 'http://gitlab_jira_username:gitlab_jira_password@jira.example.com/rest/api/2/issue/JIRA-123'
-      @transitions_url = 'http://gitlab_jira_username:gitlab_jira_password@jira.example.com/rest/api/2/issue/JIRA-123/transitions'
-      @comment_url = 'http://gitlab_jira_username:gitlab_jira_password@jira.example.com/rest/api/2/issue/JIRA-123/comment'
-      WebMock.stub_request(:get, project_url)
+      project_issues_url = 'http://gitlab_jira_username:gitlab_jira_password@jira.example.com/rest/api/2/issue/JIRA-123'
+      @project_url       = 'http://gitlab_jira_username:gitlab_jira_password@jira.example.com/rest/api/2/project/GitLabProject'
+      @transitions_url   = 'http://gitlab_jira_username:gitlab_jira_password@jira.example.com/rest/api/2/issue/JIRA-123/transitions'
+      @comment_url       = 'http://gitlab_jira_username:gitlab_jira_password@jira.example.com/rest/api/2/issue/JIRA-123/comment'
+      WebMock.stub_request(:get, @project_url)
+      WebMock.stub_request(:get, project_issues_url)
       WebMock.stub_request(:post, @transitions_url)
       WebMock.stub_request(:post, @comment_url)
     end
@@ -99,6 +137,14 @@ describe JiraService, models: true do
         body: /this-is-a-custom-id/
       ).once
     end
+
+    context "when testing" do
+      it "tries to get jira project" do
+        @jira_service.execute(nil)
+
+        expect(WebMock).to have_requested(:get, @project_url)
+      end
+    end
   end
   describe "Stored password invalidation" do

spec/models/project_spec.rb

@@ -953,16 +953,19 @@ describe Project, models: true do
   context 'repository storage by default' do
     let(:project) { create(:empty_project) }
-    subject { project.repository_storage }
-
     before do
-      storages = { 'alternative_storage' => '/some/path' }
+      storages = {
+        'default' => 'tmp/tests/repositories',
+        'picked'  => 'tmp/tests/repositories',
+      }
       allow(Gitlab.config.repositories).to receive(:storages).and_return(storages)
-      stub_application_setting(repository_storage: 'alternative_storage')
-      allow_any_instance_of(Project).to receive(:ensure_dir_exist).and_return(true)
     end
-    it { is_expected.to eq('alternative_storage') }
+    it 'picks storage from ApplicationSetting' do
+      expect_any_instance_of(ApplicationSetting).to receive(:pick_repository_storage).and_return('picked')
+
+      expect(project.repository_storage).to eq('picked')
+    end
   end
   context 'shared runners by default' do

spec/models/user_spec.rb

@@ -270,6 +270,20 @@ describe User, models: true do
         expect(users_without_two_factor).not_to include(user_with_2fa.id)
       end
     end
+
+    describe '.todo_authors' do
+      it 'filters users' do
+        create :user
+        user_2 = create :user
+        user_3 = create :user
+        current_user = create :user
+        create(:todo, user: current_user, author: user_2, state: :done)
+        create(:todo, user: current_user, author: user_3, state: :pending)
+
+        expect(User.todo_authors(current_user.id, 'pending')).to eq [user_3]
+        expect(User.todo_authors(current_user.id, 'done')).to eq [user_2]
+      end
+    end
   end
   describe "Respond to" do

spec/requests/api/labels_spec.rb

@@ -82,7 +82,20 @@ describe API::API, api: true  do
       expect(json_response['message']['title']).to eq(['is invalid'])
     end
-    it 'returns 409 if label already exists' do
+    it 'returns 409 if label already exists in group' do
+      group = create(:group)
+      group_label = create(:group_label, group: group)
+      project.update(group: group)
+
+      post api("/projects/#{project.id}/labels", user),
+           name: group_label.name,
+           color: '#FFAABB'
+
+      expect(response).to have_http_status(409)
+      expect(json_response['message']).to eq('Label already exists')
+    end
+
+    it 'returns 409 if label already exists in project' do
       post api("/projects/#{project.id}/labels", user),
            name: 'label1',
            color: '#FFAABB'

spec/requests/api/merge_request_diffs_spec.rb

@@ -14,14 +14,14 @@ describe API::API, 'MergeRequestDiffs', api: true  do
   end
   describe 'GET /projects/:id/merge_requests/:merge_request_id/versions' do
-    context 'valid merge request' do
-      before { get api("/projects/#{project.id}/merge_requests/#{merge_request.id}/versions", user) }
-      let(:merge_request_diff) { merge_request.merge_request_diffs.first }
-
-      it { expect(response.status).to eq 200 }
-      it { expect(json_response.size).to eq(merge_request.merge_request_diffs.size) }
-      it { expect(json_response.first['id']).to eq(merge_request_diff.id) }
-      it { expect(json_response.first['head_commit_sha']).to eq(merge_request_diff.head_commit_sha) }
+    it 'returns 200 for a valid merge request' do
+      get api("/projects/#{project.id}/merge_requests/#{merge_request.id}/versions", user)
+      merge_request_diff = merge_request.merge_request_diffs.first
+
+      expect(response.status).to eq 200
+      expect(json_response.size).to eq(merge_request.merge_request_diffs.size)
+      expect(json_response.first['id']).to eq(merge_request_diff.id)
+      expect(json_response.first['head_commit_sha']).to eq(merge_request_diff.head_commit_sha)
     end
     it 'returns a 404 when merge_request_id not found' do
@@ -31,14 +31,14 @@ describe API::API, 'MergeRequestDiffs', api: true  do
   end
   describe 'GET /projects/:id/merge_requests/:merge_request_id/versions/:version_id' do
-    context 'valid merge request' do
-      before { get api("/projects/#{project.id}/merge_requests/#{merge_request.id}/versions/#{merge_request_diff.id}", user) }
-      let(:merge_request_diff) { merge_request.merge_request_diffs.first }
-
-      it { expect(response.status).to eq 200 }
-      it { expect(json_response['id']).to eq(merge_request_diff.id) }
-      it { expect(json_response['head_commit_sha']).to eq(merge_request_diff.head_commit_sha) }
-      it { expect(json_response['diffs'].size).to eq(merge_request_diff.diffs.size) }
+    it 'returns a 200 for a valid merge request' do
+      merge_request_diff = merge_request.merge_request_diffs.first
+      get api("/projects/#{project.id}/merge_requests/#{merge_request.id}/versions/#{merge_request_diff.id}", user)
+
+      expect(response.status).to eq 200
+      expect(json_response['id']).to eq(merge_request_diff.id)
+      expect(json_response['head_commit_sha']).to eq(merge_request_diff.head_commit_sha)
+      expect(json_response['diffs'].size).to eq(merge_request_diff.diffs.size)
     end
     it 'returns a 404 when merge_request_id not found' do

spec/requests/api/merge_requests_spec.rb

@@ -186,14 +186,14 @@ describe API::API, api: true  do
   end
   describe 'GET /projects/:id/merge_requests/:merge_request_id/commits' do
-    context 'valid merge request' do
-      before { get api("/projects/#{project.id}/merge_requests/#{merge_request.id}/commits", user) }
-      let(:commit) { merge_request.commits.first }
-
-      it { expect(response.status).to eq 200 }
-      it { expect(json_response.size).to eq(merge_request.commits.size) }
-      it { expect(json_response.first['id']).to eq(commit.id) }
-      it { expect(json_response.first['title']).to eq(commit.title) }
+    it 'returns a 200 when merge request is valid' do
+      get api("/projects/#{project.id}/merge_requests/#{merge_request.id}/commits", user)
+      commit = merge_request.commits.first
+
+      expect(response.status).to eq 200
+      expect(json_response.size).to eq(merge_request.commits.size)
+      expect(json_response.first['id']).to eq(commit.id)
+      expect(json_response.first['title']).to eq(commit.title)
     end
     it 'returns a 404 when merge_request_id not found' do

spec/requests/api/projects_spec.rb

@@ -256,7 +256,8 @@ describe API::API, api: true  do
         merge_requests_enabled: false,
         wiki_enabled: false,
         only_allow_merge_if_build_succeeds: false,
-        request_access_enabled: true
+        request_access_enabled: true,
+        only_allow_merge_if_all_discussions_are_resolved: false
       })
       post api('/projects', user), project
@@ -327,6 +328,22 @@ describe API::API, api: true  do
       expect(json_response['only_allow_merge_if_build_succeeds']).to be_truthy
     end
+    it 'sets a project as allowing merge even if discussions are unresolved' do
+      project = attributes_for(:project, { only_allow_merge_if_all_discussions_are_resolved: false })
+
+      post api('/projects', user), project
+
+      expect(json_response['only_allow_merge_if_all_discussions_are_resolved']).to be_falsey
+    end
+
+    it 'sets a project as allowing merge only if all discussions are resolved' do
+      project = attributes_for(:project, { only_allow_merge_if_all_discussions_are_resolved: true })
+
+      post api('/projects', user), project
+
+      expect(json_response['only_allow_merge_if_all_discussions_are_resolved']).to be_truthy
+    end
+
     context 'when a visibility level is restricted' do
       before do
         @project = attributes_for(:project, { public: true })
@@ -448,6 +465,22 @@ describe API::API, api: true  do
       post api("/projects/user/#{user.id}", admin), project
       expect(json_response['only_allow_merge_if_build_succeeds']).to be_truthy
     end
+
+    it 'sets a project as allowing merge even if discussions are unresolved' do
+      project = attributes_for(:project, { only_allow_merge_if_all_discussions_are_resolved: false })
+
+      post api("/projects/user/#{user.id}", admin), project
+
+      expect(json_response['only_allow_merge_if_all_discussions_are_resolved']).to be_falsey
+    end
+
+    it 'sets a project as allowing merge only if all discussions are resolved' do
+      project = attributes_for(:project, { only_allow_merge_if_all_discussions_are_resolved: true })
+
+      post api("/projects/user/#{user.id}", admin), project
+
+      expect(json_response['only_allow_merge_if_all_discussions_are_resolved']).to be_truthy
+    end
   end
   describe "POST /projects/:id/uploads" do
@@ -509,6 +542,7 @@ describe API::API, api: true  do
       expect(json_response['shared_with_groups'][0]['group_name']).to eq(group.name)
       expect(json_response['shared_with_groups'][0]['group_access_level']).to eq(link.group_access)
       expect(json_response['only_allow_merge_if_build_succeeds']).to eq(project.only_allow_merge_if_build_succeeds)
+      expect(json_response['only_allow_merge_if_all_discussions_are_resolved']).to eq(project.only_allow_merge_if_all_discussions_are_resolved)
     end
     it 'returns a project by path name' do

spec/requests/api/settings_spec.rb

@@ -33,6 +33,7 @@ describe API::API, 'Settings', api: true  do
         expect(json_response['default_projects_limit']).to eq(3)
         expect(json_response['signin_enabled']).to be_falsey
         expect(json_response['repository_storage']).to eq('custom')
+        expect(json_response['repository_storages']).to eq(['custom'])
         expect(json_response['koding_enabled']).to be_truthy
         expect(json_response['koding_url']).to eq('http://koding.example.com')
       end

spec/requests/ci/api/builds_spec.rb

@@ -220,26 +220,33 @@ describe Ci::API::API do
       end
       context 'when request is valid' do
-        it { expect(response.status).to eq 202 }
+        it 'gets correct response' do
+          expect(response.status).to eq 202
+          expect(response.header).to have_key 'Range'
+          expect(response.header).to have_key 'Build-Status'
+        end
+
         it { expect(build.reload.trace).to eq 'BUILD TRACE appended' }
-        it { expect(response.header).to have_key 'Range' }
-        it { expect(response.header).to have_key 'Build-Status' }
       end
       context 'when content-range start is too big' do
         let(:headers_with_range) { headers.merge({ 'Content-Range' => '15-20' }) }
-        it { expect(response.status).to eq 416 }
-        it { expect(response.header).to have_key 'Range' }
-        it { expect(response.header['Range']).to eq '0-11' }
+        it 'gets correct response' do
+          expect(response.status).to eq 416
+          expect(response.header).to have_key 'Range'
+          expect(response.header['Range']).to eq '0-11'
+        end
       end
       context 'when content-range start is too small' do
         let(:headers_with_range) { headers.merge({ 'Content-Range' => '8-20' }) }
-        it { expect(response.status).to eq 416 }
-        it { expect(response.header).to have_key 'Range' }
-        it { expect(response.header['Range']).to eq '0-11' }
+        it 'gets correct response' do
+          expect(response.status).to eq 416
+          expect(response.header).to have_key 'Range'
+          expect(response.header['Range']).to eq '0-11'
+        end
       end
       context 'when Content-Range header is missing' do

spec/requests/ci/api/runners_spec.rb

@@ -109,10 +109,12 @@ describe Ci::API::API do
   end
   describe "DELETE /runners/delete" do
-    let!(:runner) { FactoryGirl.create(:ci_runner) }
-    before { delete ci_api("/runners/delete"), token: runner.token }
-    it { expect(response).to have_http_status 200 }
-    it { expect(Ci::Runner.count).to eq(0) }
+    it 'returns 200' do
+      runner = FactoryGirl.create(:ci_runner)
+      delete ci_api("/runners/delete"), token: runner.token
+      expect(response).to have_http_status 200
+      expect(Ci::Runner.count).to eq(0)
+    end
   end
 end

spec/serializers/build_entity_spec.rb

+require 'spec_helper'
+
+describe BuildEntity do
+  let(:entity) do
+    described_class.new(build, request: double)
+  end
+
+  subject { entity.as_json }
+
+  context 'when build is a regular job' do
+    let(:build) { create(:ci_build) }
+
+    it 'contains url to build page and retry action' do
+      expect(subject).to include(:build_url, :retry_url)
+      expect(subject).not_to include(:play_url)
+    end
+
+    it 'does not contain sensitive information' do
+      expect(subject).not_to include(/token/)
+      expect(subject).not_to include(/variables/)
+    end
+  end
+
+  context 'when build is a manual action' do
+    let(:build) { create(:ci_build, :manual) }
+
+    it 'contains url to play action' do
+      expect(subject).to include(:play_url)
+    end
+  end
+end

spec/serializers/commit_entity_spec.rb

+require 'spec_helper'
+
+describe CommitEntity do
+  let(:entity) do
+    described_class.new(commit, request: request)
+  end
+
+  let(:request) { double('request') }
+  let(:project) { create(:project) }
+  let(:commit) { project.commit }
+
+  subject { entity.as_json }
+
+  before do
+    allow(request).to receive(:project).and_return(project)
+  end
+
+  context 'when commit author is a user' do
+    before do
+      create(:user, email: commit.author_email)
+    end
+
+    it 'contains information about user' do
+      expect(subject.fetch(:author)).not_to be_nil
+    end
+  end
+
+  context 'when commit author is not a user' do
+    it 'does not contain author details' do
+      expect(subject.fetch(:author)).to be_nil
+    end
+  end
+
+  it 'contains commit URL' do
+    expect(subject).to include(:commit_url)
+  end
+
+  it 'needs to receive project in the request' do
+    expect(request).to receive(:project)
+      .and_return(project)
+
+    subject
+  end
+end

spec/serializers/deployment_entity_spec.rb

+require 'spec_helper'
+
+describe DeploymentEntity do
+  let(:entity) do
+    described_class.new(deployment, request: double)
+  end
+
+  let(:deployment) { create(:deployment) }
+
+  subject { entity.as_json }
+
+  it 'exposes internal deployment id'  do
+    expect(subject).to include(:iid)
+  end
+
+  it 'exposes nested information about branch' do
+    expect(subject[:ref][:name]).to eq 'master'
+    expect(subject[:ref][:ref_url]).not_to be_empty
+  end
+end

spec/serializers/entity_request_spec.rb

+require 'spec_helper'
+
+describe EntityRequest do
+  subject do
+    described_class.new(user: 'user', project: 'some project')
+  end
+
+  describe 'methods created' do
+    it 'defines accessible attributes' do
+      expect(subject.user).to eq 'user'
+      expect(subject.project).to eq 'some project'
+    end
+
+    it 'raises error when attribute is not defined' do
+      expect { subject.some_method }.to raise_error NoMethodError
+    end
+  end
+end

spec/serializers/environment_entity_spec.rb

+require 'spec_helper'
+
+describe EnvironmentEntity do
+  let(:entity) do
+    described_class.new(environment, request: double)
+  end
+
+  let(:environment) { create(:environment) }
+  subject { entity.as_json }
+
+  it 'exposes latest deployment' do
+    expect(subject).to include(:last_deployment)
+  end
+
+  it 'exposes core elements of environment' do
+    expect(subject).to include(:id, :name, :state, :environment_url)
+  end
+end

spec/serializers/environment_serializer_spec.rb

+require 'spec_helper'
+
+describe EnvironmentSerializer do
+  let(:serializer) do
+    described_class
+      .new(user: user, project: project)
+      .represent(resource)
+  end
+
+  let(:json) { serializer.as_json }
+  let(:user) { create(:user) }
+  let(:project) { create(:project) }
+
+  context 'when there is a single object provided' do
+    before do
+      create(:ci_build, :manual, name: 'manual1',
+                                 pipeline: deployable.pipeline)
+    end
+
+    let(:deployment) do
+      create(:deployment, deployable: deployable,
+                          user: user,
+                          project: project,
+                          sha: project.commit.id)
+    end
+
+    let(:deployable) { create(:ci_build) }
+    let(:resource) { deployment.environment }
+
+    it 'it generates payload for single object' do
+      expect(json).to be_an_instance_of Hash
+    end
+
+    it 'contains important elements of environment' do
+      expect(json)
+        .to include(:name, :external_url, :environment_url, :last_deployment)
+    end
+
+    it 'contains relevant information about last deployment' do
+      last_deployment = json.fetch(:last_deployment)
+
+      expect(last_deployment)
+        .to include(:ref, :user, :commit, :deployable, :manual_actions)
+    end
+  end
+
+  context 'when there is a collection of objects provided' do
+    let(:project) { create(:empty_project) }
+    let(:resource) { create_list(:environment, 2) }
+
+    it 'contains important elements of environment' do
+      expect(json.first)
+        .to include(:last_deployment, :name, :external_url)
+    end
+
+    it 'generates payload for collection' do
+      expect(json).to be_an_instance_of Array
+    end
+  end
+end

spec/serializers/user_entity_spec.rb

+require 'spec_helper'
+
+describe UserEntity do
+  let(:entity) { described_class.new(user) }
+  let(:user) { create(:user) }
+  subject { entity.as_json }
+
+  it 'exposes user name and login' do
+    expect(subject).to include(:username, :name)
+  end
+
+  it 'does not expose passwords' do
+    expect(subject).not_to include(/password/)
+  end
+
+  it 'does not expose tokens' do
+    expect(subject).not_to include(/token/)
+  end
+
+  it 'does not expose 2FA OTPs' do
+    expect(subject).not_to include(/otp/)
+  end
+end