Merge remote-tracking branch 'origin/master' into...

Merge remote-tracking branch 'origin/master' into camilstaps/gitlab-ce-new-66023-public-private-fork-counts

spec/presenters/event_presenter_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe EventPresenter do
+  include Gitlab::Routing.url_helpers
+
+  set(:group) { create(:group) }
+  set(:project) { create(:project, group: group) }
+  set(:target) { create(:milestone, project: project) }
+  set(:group_event) { create(:event, :created, project: nil, group: group, target: target) }
+  set(:project_event) { create(:event, :created, project: project, target: target) }
+
+  describe '#resource_parent_name' do
+    context 'with group event' do
+      subject { group_event.present.resource_parent_name }
+
+      it { is_expected.to eq(group.full_name) }
+    end
+
+    context 'with project label' do
+      subject { project_event.present.resource_parent_name }
+
+      it { is_expected.to eq(project.full_name) }
+    end
+  end
+
+  describe '#target_link_options' do
+    context 'with group event' do
+      subject { group_event.present.target_link_options }
+
+      it { is_expected.to eq([group, target]) }
+    end
+
+    context 'with project label' do
+      subject { project_event.present.target_link_options }
+
+      it { is_expected.to eq([group.becomes(Namespace), project, target]) }
+    end
+  end
+end

spec/routing/project_routing_spec.rb

@@ -530,15 +530,22 @@ describe 'project routing' do
     end
   end
-  # project_blame GET    /:project_id/blame/:id(.:format) blame#show {id: /.+/, project_id: /[^\/]+/}
+  # project_blame GET    /:project_id/blame/:id(.:format) blame#show {id: /[^\0]+/, project_id: /[^\/]+/}
   describe Projects::BlameController, 'routing' do
     it 'to #show' do
       expect(get('/gitlab/gitlabhq/blame/master/app/models/project.rb')).to route_to('projects/blame#show', namespace_id: 'gitlab', project_id: 'gitlabhq', id: 'master/app/models/project.rb')
       expect(get('/gitlab/gitlabhq/blame/master/files.scss')).to route_to('projects/blame#show', namespace_id: 'gitlab', project_id: 'gitlabhq', id: 'master/files.scss')
+      newline_file = "new\n\nline.txt"
+      url_encoded_newline_file = ERB::Util.url_encode(newline_file)
+      assert_routing({ path: "/gitlab/gitlabhq/blame/master/#{url_encoded_newline_file}",
+                        method: :get },
+                    { controller: 'projects/blame', action: 'show',
+                      namespace_id: 'gitlab', project_id: 'gitlabhq',
+                      id: "master/#{newline_file}" })
     end
   end
-  # project_blob GET    /:project_id/blob/:id(.:format) blob#show {id: /.+/, project_id: /[^\/]+/}
+  # project_blob GET    /:project_id/blob/:id(.:format) blob#show {id: /[^\0]+/, project_id: /[^\/]+/}
   describe Projects::BlobController, 'routing' do
     it 'to #show' do
       expect(get('/gitlab/gitlabhq/blob/master/app/models/project.rb')).to route_to('projects/blob#show', namespace_id: 'gitlab', project_id: 'gitlabhq', id: 'master/app/models/project.rb')
@@ -547,28 +554,56 @@ describe 'project routing' do
       expect(get('/gitlab/gitlabhq/blob/master/files.scss')).to route_to('projects/blob#show', namespace_id: 'gitlab', project_id: 'gitlabhq', id: 'master/files.scss')
       expect(get('/gitlab/gitlabhq/blob/master/blob/index.js')).to route_to('projects/blob#show', namespace_id: 'gitlab', project_id: 'gitlabhq', id: 'master/blob/index.js')
       expect(get('/gitlab/gitlabhq/blob/blob/master/blob/index.js')).to route_to('projects/blob#show', namespace_id: 'gitlab', project_id: 'gitlabhq', id: 'blob/master/blob/index.js')
+      newline_file = "new\n\nline.txt"
+      url_encoded_newline_file = ERB::Util.url_encode(newline_file)
+      assert_routing({ path: "/gitlab/gitlabhq/blob/blob/master/blob/#{url_encoded_newline_file}",
+                        method: :get },
+                    { controller: 'projects/blob', action: 'show',
+                      namespace_id: 'gitlab', project_id: 'gitlabhq',
+                      id: "blob/master/blob/#{newline_file}" })
     end
   end
-  # project_tree GET    /:project_id/tree/:id(.:format) tree#show {id: /.+/, project_id: /[^\/]+/}
+  # project_tree GET    /:project_id/tree/:id(.:format) tree#show {id: /[^\0]+/, project_id: /[^\/]+/}
   describe Projects::TreeController, 'routing' do
     it 'to #show' do
       expect(get('/gitlab/gitlabhq/tree/master/app/models/project.rb')).to route_to('projects/tree#show', namespace_id: 'gitlab', project_id: 'gitlabhq', id: 'master/app/models/project.rb')
       expect(get('/gitlab/gitlabhq/tree/master/files.scss')).to route_to('projects/tree#show', namespace_id: 'gitlab', project_id: 'gitlabhq', id: 'master/files.scss')
       expect(get('/gitlab/gitlabhq/tree/master/tree/files')).to route_to('projects/tree#show', namespace_id: 'gitlab', project_id: 'gitlabhq', id: 'master/tree/files')
       expect(get('/gitlab/gitlabhq/tree/tree/master/tree/files')).to route_to('projects/tree#show', namespace_id: 'gitlab', project_id: 'gitlabhq', id: 'tree/master/tree/files')
+      newline_file = "new\n\nline.txt"
+      url_encoded_newline_file = ERB::Util.url_encode(newline_file)
+      assert_routing({ path: "/gitlab/gitlabhq/tree/master/#{url_encoded_newline_file}",
+                        method: :get },
+                    { controller: 'projects/tree', action: 'show',
+                      namespace_id: 'gitlab', project_id: 'gitlabhq',
+                      id: "master/#{newline_file}" })
     end
   end
-  # project_find_file GET /:namespace_id/:project_id/find_file/*id(.:format)  projects/find_file#show {:id=>/.+/, :namespace_id=>/[a-zA-Z.0-9_\-]+/, :project_id=>/[a-zA-Z.0-9_\-]+(?<!\.atom)/, :format=>/html/}
+  # project_find_file GET /:namespace_id/:project_id/find_file/*id(.:format)  projects/find_file#show {:id=>/[^\0]+/, :namespace_id=>/[a-zA-Z.0-9_\-]+/, :project_id=>/[a-zA-Z.0-9_\-]+(?<!\.atom)/, :format=>/html/}
   # project_files     GET /:namespace_id/:project_id/files/*id(.:format)      projects/find_file#list {:id=>/(?:[^.]|\.(?!json$))+/, :namespace_id=>/[a-zA-Z.0-9_\-]+/, :project_id=>/[a-zA-Z.0-9_\-]+(?<!\.atom)/, :format=>/json/}
   describe Projects::FindFileController, 'routing' do
     it 'to #show' do
       expect(get('/gitlab/gitlabhq/find_file/master')).to route_to('projects/find_file#show', namespace_id: 'gitlab', project_id: 'gitlabhq', id: 'master')
+      newline_file = "new\n\nline.txt"
+      url_encoded_newline_file = ERB::Util.url_encode(newline_file)
+      assert_routing({ path: "/gitlab/gitlabhq/find_file/#{url_encoded_newline_file}",
+                        method: :get },
+                    { controller: 'projects/find_file', action: 'show',
+                      namespace_id: 'gitlab', project_id: 'gitlabhq',
+                      id: "#{newline_file}" })
     end
     it 'to #list' do
       expect(get('/gitlab/gitlabhq/files/master.json')).to route_to('projects/find_file#list', namespace_id: 'gitlab', project_id: 'gitlabhq', id: 'master.json')
+      newline_file = "new\n\nline.txt"
+      url_encoded_newline_file = ERB::Util.url_encode(newline_file)
+      assert_routing({ path: "/gitlab/gitlabhq/files/#{url_encoded_newline_file}",
+                        method: :get },
+                    { controller: 'projects/find_file', action: 'list',
+                      namespace_id: 'gitlab', project_id: 'gitlabhq',
+                      id: "#{newline_file}" })
     end
   end
@@ -578,6 +613,13 @@ describe 'project routing' do
         route_to('projects/blob#edit',
                  namespace_id: 'gitlab', project_id: 'gitlabhq',
                  id: 'master/app/models/project.rb'))
+      newline_file = "new\n\nline.txt"
+      url_encoded_newline_file = ERB::Util.url_encode(newline_file)
+      assert_routing({ path: "/gitlab/gitlabhq/edit/master/docs/#{url_encoded_newline_file}",
+                        method: :get },
+                      { controller: 'projects/blob', action: 'edit',
+                        namespace_id: 'gitlab', project_id: 'gitlabhq',
+                        id: "master/docs/#{newline_file}" })
     end
     it 'to #preview' do
@@ -585,6 +627,26 @@ describe 'project routing' do
         route_to('projects/blob#preview',
                  namespace_id: 'gitlab', project_id: 'gitlabhq',
                  id: 'master/app/models/project.rb'))
+      newline_file = "new\n\nline.txt"
+      url_encoded_newline_file = ERB::Util.url_encode(newline_file)
+      assert_routing({ path: "/gitlab/gitlabhq/edit/master/docs/#{url_encoded_newline_file}",
+                        method: :get },
+                      { controller: 'projects/blob', action: 'edit',
+                        namespace_id: 'gitlab', project_id: 'gitlabhq',
+                        id: "master/docs/#{newline_file}" })
+    end
+  end
+
+  # project_raw GET    /:project_id/raw/:id(.:format) raw#show {id: /[^\0]+/, project_id: /[^\/]+/}
+  describe Projects::RawController, 'routing' do
+    it 'to #show' do
+      newline_file = "new\n\nline.txt"
+      url_encoded_newline_file = ERB::Util.url_encode(newline_file)
+      assert_routing({ path: "/gitlab/gitlabhq/raw/master/#{url_encoded_newline_file}",
+                        method: :get },
+                    { controller: 'projects/raw', action: 'show',
+                      namespace_id: 'gitlab', project_id: 'gitlabhq',
+                      id: "master/#{newline_file}" })
     end
   end

spec/rubocop/cop/scalability/file_uploads_spec.rb

+# frozen_string_literal: true
+
+require 'fast_spec_helper'
+require 'rubocop'
+require_relative '../../../support/helpers/expect_offense'
+require_relative '../../../../rubocop/cop/scalability/file_uploads'
+
+describe RuboCop::Cop::Scalability::FileUploads do
+  include CopHelper
+  include ExpectOffense
+
+  subject(:cop) { described_class.new }
+  let(:message) { 'Do not upload files without workhorse acceleration. Please refer to https://docs.gitlab.com/ee/development/uploads.html' }
+
+  context 'with required params' do
+    it 'detects File in types array' do
+      expect_offense(<<~PATTERN.strip_indent)
+      params do
+        requires :certificate, allow_blank: false, types: [String, File]
+                                                                   ^^^^ #{message}
+      end
+      PATTERN
+    end
+
+    it 'detects File as type argument' do
+      expect_offense(<<~PATTERN.strip_indent)
+      params do
+        requires :attachment, type: File
+                                    ^^^^ #{message}
+      end
+      PATTERN
+    end
+  end
+
+  context 'with optional params' do
+    it 'detects File in types array' do
+      expect_offense(<<~PATTERN.strip_indent)
+      params do
+        optional :certificate, allow_blank: false, types: [String, File]
+                                                                   ^^^^ #{message}
+      end
+      PATTERN
+    end
+
+    it 'detects File as type argument' do
+      expect_offense(<<~PATTERN.strip_indent)
+      params do
+        optional :attachment, type: File
+                                    ^^^^ #{message}
+      end
+      PATTERN
+    end
+  end
+end

spec/services/ci/retry_build_service_spec.rb

@@ -40,7 +40,7 @@ describe Ci::RetryBuildService do
        user_id auto_canceled_by_id retried failure_reason
        sourced_pipelines artifacts_file_store artifacts_metadata_store
        metadata runner_session trace_chunks upstream_pipeline_id
-       artifacts_file artifacts_metadata artifacts_size].freeze
+       artifacts_file artifacts_metadata artifacts_size commands].freeze
   shared_examples 'build duplication' do
     let(:another_pipeline) { create(:ci_empty_pipeline, project: project) }

spec/services/merge_requests/build_service_spec.rb

@@ -49,6 +49,22 @@ describe MergeRequests::BuildService do
     allow(project).to receive(:commit).and_return(commit_2)
   end
+  shared_examples 'allows the merge request to be created' do
+    it do
+      expect(merge_request.can_be_created).to eq(true)
+    end
+  end
+
+  shared_examples 'forbids the merge request from being created' do
+    it 'returns that the merge request cannot be created' do
+      expect(merge_request.can_be_created).to eq(false)
+    end
+
+    it 'adds an error message to the merge request' do
+      expect(merge_request.errors).to contain_exactly(*Array(error_message))
+    end
+  end
+
   describe '#execute' do
     it 'calls the compare service with the correct arguments' do
       allow_any_instance_of(described_class).to receive(:projects_and_branches_valid?).and_return(true)
@@ -79,12 +95,8 @@ describe MergeRequests::BuildService do
     context 'missing source branch' do
       let(:source_branch) { '' }
-      it 'forbids the merge request from being created' do
-        expect(merge_request.can_be_created).to eq(false)
-      end
-
-      it 'adds an error message to the merge request' do
-        expect(merge_request.errors).to contain_exactly('You must select source and target branch')
+      it_behaves_like 'forbids the merge request from being created' do
+        let(:error_message) { 'You must select source and target branch' }
       end
     end
@@ -96,25 +108,44 @@ describe MergeRequests::BuildService do
         stub_compare
       end
-      it 'creates compare object with target branch as default branch' do
-        expect(merge_request.compare).to be_present
-        expect(merge_request.target_branch).to eq(project.default_branch)
-      end
-      it 'allows the merge request to be created' do
-        expect(merge_request.can_be_created).to eq(true)
+      context 'when source branch' do
+        context 'is not the repository default branch' do
+          it 'creates compare object with target branch as default branch' do
+            expect(merge_request.compare).to be_present
+            expect(merge_request.target_branch).to eq(project.default_branch)
+          end
+
+          it_behaves_like 'allows the merge request to be created'
+        end
+
+        context 'the repository default branch' do
+          let(:source_branch) { 'master' }
+
+          it_behaves_like 'forbids the merge request from being created' do
+            let(:error_message) { 'You must select source and target branch' }
+          end
+          context 'when source project is different from the target project' do
+            let(:target_project) { create(:project, :public, :repository) }
+            let!(:project) { fork_project(target_project, user, namespace: user.namespace, repository: true) }
+            let(:source_project) { project }
+
+            it 'creates compare object with target branch as default branch' do
+              expect(merge_request.compare).to be_present
+              expect(merge_request.target_branch).to eq(project.default_branch)
+            end
+
+            it_behaves_like 'allows the merge request to be created'
+          end
+        end
       end
     end
     context 'same source and target branch' do
       let(:source_branch) { 'master' }
-      it 'forbids the merge request from being created' do
-        expect(merge_request.can_be_created).to eq(false)
-      end
-
-      it 'adds an error message to the merge request' do
-        expect(merge_request.errors).to contain_exactly('You must select different branches')
+      it_behaves_like 'forbids the merge request from being created' do
+        let(:error_message) { 'You must select different branches' }
       end
     end
@@ -125,9 +156,7 @@ describe MergeRequests::BuildService do
         stub_compare
       end
-      it 'allows the merge request to be created' do
-        expect(merge_request.can_be_created).to eq(true)
-      end
+      it_behaves_like 'allows the merge request to be created'
       it 'adds a WIP prefix to the merge request title' do
         expect(merge_request.title).to eq('WIP: Feature branch')
@@ -142,9 +171,7 @@ describe MergeRequests::BuildService do
         stub_compare
       end
-      it 'allows the merge request to be created' do
-        expect(merge_request.can_be_created).to eq(true)
-      end
+      it_behaves_like 'allows the merge request to be created'
       it 'uses the title of the commit as the title of the merge request' do
         expect(merge_request.title).to eq(commit_1.safe_message.split("\n").first)
@@ -254,9 +281,7 @@ describe MergeRequests::BuildService do
         stub_compare
       end
-      it 'allows the merge request to be created' do
-        expect(merge_request.can_be_created).to eq(true)
-      end
+      it_behaves_like 'allows the merge request to be created'
       it 'uses the title of the branch as the merge request title' do
         expect(merge_request.title).to eq('Feature branch')
@@ -340,12 +365,8 @@ describe MergeRequests::BuildService do
         allow(project).to receive(:commit).with(target_branch).and_return(commit_1)
       end
-      it 'forbids the merge request from being created' do
-        expect(merge_request.can_be_created).to eq(false)
-      end
-
-      it 'adds an error message to the merge request' do
-        expect(merge_request.errors).to contain_exactly('Source branch "feature-branch" does not exist')
+      it_behaves_like 'forbids the merge request from being created' do
+        let(:error_message) { 'Source branch "feature-branch" does not exist' }
       end
     end
@@ -355,12 +376,8 @@ describe MergeRequests::BuildService do
         allow(project).to receive(:commit).with(target_branch).and_return(nil)
       end
-      it 'forbids the merge request from being created' do
-        expect(merge_request.can_be_created).to eq(false)
-      end
-
-      it 'adds an error message to the merge request' do
-        expect(merge_request.errors).to contain_exactly('Target branch "master" does not exist')
+      it_behaves_like 'forbids the merge request from being created' do
+        let(:error_message) { 'Target branch "master" does not exist' }
       end
     end
@@ -369,15 +386,10 @@ describe MergeRequests::BuildService do
         allow(project).to receive(:commit).and_return(nil)
       end
-      it 'forbids the merge request from being created' do
-        expect(merge_request.can_be_created).to eq(false)
-      end
-
-      it 'adds both error messages to the merge request' do
-        expect(merge_request.errors).to contain_exactly(
-          'Source branch "feature-branch" does not exist',
-          'Target branch "master" does not exist'
-        )
+      it_behaves_like 'forbids the merge request from being created' do
+        let(:error_message) do
+          ['Source branch "feature-branch" does not exist', 'Target branch "master" does not exist']
+        end
       end
     end

spec/support/shared_examples/boards/multiple_issue_boards_shared_examples.rb

 # frozen_string_literal: true
 shared_examples_for 'multiple issue boards' do
-  dropdown_selector = '.js-boards-selector .dropdown-menu'
-
   context 'authorized user' do
     before do
       parent.add_maintainer(user)
@@ -20,18 +18,14 @@ shared_examples_for 'multiple issue boards' do
     end
     it 'shows a list of boards' do
-      click_button board.name
-
-      page.within(dropdown_selector) do
+      in_boards_switcher_dropdown do
         expect(page).to have_content(board.name)
         expect(page).to have_content(board2.name)
       end
     end
     it 'switches current board' do
-      click_button board.name
-
-      page.within(dropdown_selector) do
+      in_boards_switcher_dropdown do
         click_link board2.name
       end
@@ -43,9 +37,7 @@ shared_examples_for 'multiple issue boards' do
     end
     it 'creates new board without detailed configuration' do
-      click_button board.name
-
-      page.within(dropdown_selector) do
+      in_boards_switcher_dropdown do
         click_button 'Create new board'
       end
@@ -57,28 +49,23 @@ shared_examples_for 'multiple issue boards' do
     end
     it 'deletes board' do
-      click_button board.name
-
-      wait_for_requests
-
-      page.within(dropdown_selector) do
+      in_boards_switcher_dropdown do
         click_button 'Delete board'
       end
       expect(page).to have_content('Are you sure you want to delete this board?')
       click_button 'Delete'
-      click_button board2.name
-      page.within(dropdown_selector) do
+      wait_for_requests
+
+      in_boards_switcher_dropdown do
         expect(page).not_to have_content(board.name)
         expect(page).to have_content(board2.name)
       end
     end
     it 'adds a list to the none default board' do
-      click_button board.name
-
-      page.within(dropdown_selector) do
+      in_boards_switcher_dropdown do
         click_link board2.name
       end
@@ -100,9 +87,7 @@ shared_examples_for 'multiple issue boards' do
       expect(page).to have_selector('.board', count: 3)
-      click_button board2.name
-
-      page.within(dropdown_selector) do
+      in_boards_switcher_dropdown do
         click_link board.name
       end
@@ -114,9 +99,9 @@ shared_examples_for 'multiple issue boards' do
     it 'maintains sidebar state over board switch' do
       assert_boards_nav_active
-      find('.boards-switcher').click
-      wait_for_requests
-      click_link board2.name
+      in_boards_switcher_dropdown do
+        click_link board2.name
+      end
       assert_boards_nav_active
     end
@@ -129,15 +114,24 @@ shared_examples_for 'multiple issue boards' do
     end
     it 'does not show action links' do
-      click_button board.name
-
-      page.within(dropdown_selector) do
+      in_boards_switcher_dropdown do
         expect(page).not_to have_content('Create new board')
         expect(page).not_to have_content('Delete board')
       end
     end
   end
+  def in_boards_switcher_dropdown
+    find('.boards-switcher').click
+
+    wait_for_requests
+
+    dropdown_selector = '.js-boards-selector .dropdown-menu'
+    page.within(dropdown_selector) do
+      yield
+    end
+  end
+
   def assert_boards_nav_active
     expect(find('.nav-sidebar .active .active')).to have_selector('a', text: 'Boards')
   end