Add latest changes from gitlab-org/gitlab@master

8 files changed, 160 insertions, 89 deletions

diff --git a/spec/features/dashboard/group_spec.rb b/spec/features/dashboard/group_spec.rb
index 0b99fed2a2d..bc6f449edc5 100644
--- a/spec/features/dashboard/group_spec.rb
+++ b/spec/features/dashboard/group_spec.rb
@@ -15,7 +15,7 @@ RSpec.describe 'Dashboard Group' do
 
   it 'creates new group', :js do
     visit dashboard_groups_path
-    find('.btn-success').click
+    find('[data-testid="new-group-button"]').click
     new_name = 'Samurai'
 
     fill_in 'group_name', with: new_name
diff --git a/spec/fixtures/api/schemas/entities/test_suite_comparer.json b/spec/fixtures/api/schemas/entities/test_suite_comparer.json
index ecb331ae013..ac001ef8843 100644
--- a/spec/fixtures/api/schemas/entities/test_suite_comparer.json
+++ b/spec/fixtures/api/schemas/entities/test_suite_comparer.json
@@ -26,7 +26,14 @@
     "existing_failures": { "type": "array", "items": { "$ref": "test_case.json" } },
     "new_errors": { "type": "array", "items": { "$ref": "test_case.json" } },
     "resolved_errors": { "type": "array", "items": { "$ref": "test_case.json" } },
-    "existing_errors": { "type": "array", "items": { "$ref": "test_case.json" } }
+    "existing_errors": { "type": "array", "items": { "$ref": "test_case.json" } },
+    "suite_errors": {
+      "type": ["object", "null"],
+      "properties": {
+        "head": { "type": ["string", "null"] },
+        "base": { "type": ["string", "null"] }
+      }
+    }
   },
   "additionalProperties": false
 }
diff --git a/spec/lib/gitlab/ci/variables/collection/sort_spec.rb b/spec/lib/gitlab/ci/variables/collection/sort_spec.rb
index 6420798d6f5..c36dbe5bc07 100644
--- a/spec/lib/gitlab/ci/variables/collection/sort_spec.rb
+++ b/spec/lib/gitlab/ci/variables/collection/sort_spec.rb
@@ -214,7 +214,6 @@ RSpec.describe Gitlab::Ci::Variables::Collection::Sort do
     context 'when FF :variable_inside_variable is enabled' do
       before do
         stub_licensed_features(group_saml_group_sync: true)
-        stub_feature_flags(saml_group_links: true)
         stub_feature_flags(variable_inside_variable: true)
       end
 
diff --git a/spec/lib/release_highlights/validator/entry_spec.rb b/spec/lib/release_highlights/validator/entry_spec.rb
index da44938f165..5f7ccbf4310 100644
--- a/spec/lib/release_highlights/validator/entry_spec.rb
+++ b/spec/lib/release_highlights/validator/entry_spec.rb
@@ -40,8 +40,8 @@ RSpec.describe ReleaseHighlights::Validator::Entry do
       end
 
       it 'validates boolean value of "self-managed" and "gitlab-com"' do
-        allow(entry).to receive(:value_for).with('self-managed').and_return('nope')
-        allow(entry).to receive(:value_for).with('gitlab-com').and_return('yerp')
+        allow(entry).to receive(:value_for).with(:'self-managed').and_return('nope')
+        allow(entry).to receive(:value_for).with(:'gitlab-com').and_return('yerp')
 
         subject.valid?
 
@@ -50,17 +50,18 @@ RSpec.describe ReleaseHighlights::Validator::Entry do
       end
 
       it 'validates URI of "url" and "image_url"' do
-        allow(entry).to receive(:value_for).with('image_url').and_return('imgur/gitlab_feature.gif')
-        allow(entry).to receive(:value_for).with('url').and_return('gitlab/newest_release.html')
+        stub_env('RSPEC_ALLOW_INVALID_URLS', 'false')
+        allow(entry).to receive(:value_for).with(:image_url).and_return('https://foobar.x/images/ci/gitlab-ci-cd-logo_2x.png')
+        allow(entry).to receive(:value_for).with(:url).and_return('')
 
         subject.valid?
 
-        expect(subject.errors[:url]).to include(/must be a URL/)
-        expect(subject.errors[:image_url]).to include(/must be a URL/)
+        expect(subject.errors[:url]).to include(/must be a valid URL/)
+        expect(subject.errors[:image_url]).to include(/is blocked: Host cannot be resolved or invalid/)
       end
 
       it 'validates release is numerical' do
-        allow(entry).to receive(:value_for).with('release').and_return('one')
+        allow(entry).to receive(:value_for).with(:release).and_return('one')
 
         subject.valid?
 
@@ -68,7 +69,7 @@ RSpec.describe ReleaseHighlights::Validator::Entry do
       end
 
       it 'validates published_at is a date' do
-        allow(entry).to receive(:value_for).with('published_at').and_return('christmas day')
+        allow(entry).to receive(:value_for).with(:published_at).and_return('christmas day')
 
         subject.valid?
 
@@ -76,7 +77,7 @@ RSpec.describe ReleaseHighlights::Validator::Entry do
       end
 
       it 'validates packages are included in list' do
-        allow(entry).to receive(:value_for).with('packages').and_return(['ALL'])
+        allow(entry).to receive(:value_for).with(:packages).and_return(['ALL'])
 
         subject.valid?
 
diff --git a/spec/lib/release_highlights/validator_spec.rb b/spec/lib/release_highlights/validator_spec.rb
index a423e8cc5f6..f30754b4167 100644
--- a/spec/lib/release_highlights/validator_spec.rb
+++ b/spec/lib/release_highlights/validator_spec.rb
@@ -78,7 +78,10 @@ RSpec.describe ReleaseHighlights::Validator do
   end
 
   describe 'when validating all files' do
-    it 'they should have no errors' do
+    # Permit DNS requests to validate all URLs in the YAML files
+    it 'they should have no errors', :permit_dns do
+      stub_env('RSPEC_ALLOW_INVALID_URLS', 'false')
+
       expect(described_class.validate_all!).to be_truthy, described_class.error_message
     end
   end
diff --git a/spec/rubocop/cop/graphql/descriptions_spec.rb b/spec/rubocop/cop/graphql/descriptions_spec.rb
index 9ad40fad83d..276e4e03c15 100644
--- a/spec/rubocop/cop/graphql/descriptions_spec.rb
+++ b/spec/rubocop/cop/graphql/descriptions_spec.rb
@@ -91,6 +91,50 @@ RSpec.describe RuboCop::Cop::Graphql::Descriptions do
     end
   end
 
+  context 'enum values' do
+    it 'adds an offense when there is no description' do
+      expect_offense(<<~TYPE)
+        module Types
+          class FakeEnum < BaseEnum
+            value 'FOO', value: 'foo'
+            ^^^^^^^^^^^^^^^^^^^^^^^^^ Please add a `description` property.
+          end
+        end
+      TYPE
+    end
+
+    it 'adds an offense when description does not end in a period' do
+      expect_offense(<<~TYPE)
+        module Types
+          class FakeEnum < BaseEnum
+            value 'FOO', value: 'foo', description: 'bar'
+            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `description` strings must end with a `.`.
+          end
+        end
+      TYPE
+    end
+
+    it 'does not add an offense when description is correct (defined using `description:`)' do
+      expect_no_offenses(<<~TYPE.strip)
+        module Types
+          class FakeEnum < BaseEnum
+            value 'FOO', value: 'foo', description: 'bar.'
+          end
+        end
+      TYPE
+    end
+
+    it 'does not add an offense when description is correct (defined as a second argument)' do
+      expect_no_offenses(<<~TYPE.strip)
+        module Types
+          class FakeEnum < BaseEnum
+            value 'FOO', 'bar.', value: 'foo'
+          end
+        end
+      TYPE
+    end
+  end
+
   describe 'autocorrecting descriptions without periods' do
     it 'can autocorrect' do
       expect_offense(<<~TYPE)
diff --git a/spec/serializers/test_suite_comparer_entity_spec.rb b/spec/serializers/test_suite_comparer_entity_spec.rb
index a63f5683779..318d1d3c1e3 100644
--- a/spec/serializers/test_suite_comparer_entity_spec.rb
+++ b/spec/serializers/test_suite_comparer_entity_spec.rb
@@ -35,6 +35,7 @@ RSpec.describe TestSuiteComparerEntity do
         end
         expect(subject[:resolved_failures]).to be_empty
         expect(subject[:existing_failures]).to be_empty
+        expect(subject[:suite_errors]).to be_nil
       end
     end
 
@@ -56,6 +57,7 @@ RSpec.describe TestSuiteComparerEntity do
         end
         expect(subject[:resolved_failures]).to be_empty
         expect(subject[:existing_failures]).to be_empty
+        expect(subject[:suite_errors]).to be_nil
       end
     end
 
@@ -77,6 +79,7 @@ RSpec.describe TestSuiteComparerEntity do
           expect(existing_failure[:execution_time]).to eq(test_case_failed.execution_time)
           expect(existing_failure[:system_output]).to eq(test_case_failed.system_output)
         end
+        expect(subject[:suite_errors]).to be_nil
       end
     end
 
@@ -98,6 +101,47 @@ RSpec.describe TestSuiteComparerEntity do
           expect(resolved_failure[:system_output]).to eq(test_case_success.system_output)
         end
         expect(subject[:existing_failures]).to be_empty
+        expect(subject[:suite_errors]).to be_nil
+      end
+    end
+
+    context 'when head suite has suite error' do
+      before do
+        allow(head_suite).to receive(:suite_error).and_return('some error')
+      end
+
+      it 'contains suite error for head suite' do
+        expect(subject[:suite_errors]).to eq(
+          head: 'some error',
+          base: nil
+        )
+      end
+    end
+
+    context 'when base suite has suite error' do
+      before do
+        allow(base_suite).to receive(:suite_error).and_return('some error')
+      end
+
+      it 'contains suite error for head suite' do
+        expect(subject[:suite_errors]).to eq(
+          head: nil,
+          base: 'some error'
+        )
+      end
+    end
+
+    context 'when base and head suite both have suite errors' do
+      before do
+        allow(head_suite).to receive(:suite_error).and_return('head error')
+        allow(base_suite).to receive(:suite_error).and_return('base error')
+      end
+
+      it 'contains suite error for head suite' do
+        expect(subject[:suite_errors]).to eq(
+          head: 'head error',
+          base: 'base error'
+        )
       end
     end
   end
diff --git a/spec/services/notes/build_service_spec.rb b/spec/services/notes/build_service_spec.rb
index 2a8c3bd75fa..deeab66c4e9 100644
--- a/spec/services/notes/build_service_spec.rb
+++ b/spec/services/notes/build_service_spec.rb
@@ -8,26 +8,33 @@ RSpec.describe Notes::BuildService do
   let(:note) { create(:discussion_note_on_issue) }
   let(:project) { note.project }
   let(:author) { note.author }
+  let(:user) { author }
   let(:merge_request) { create(:merge_request, source_project: project) }
-  let(:mr_note) { create(:discussion_note_on_merge_request, noteable: merge_request, project: project, author: author) }
+  let(:mr_note) { create(:discussion_note_on_merge_request, noteable: merge_request, project: project, author: note.author) }
+  let(:base_params) { { note: 'Test' } }
+  let(:params) { {} }
+
+  subject(:new_note) { described_class.new(project, user, base_params.merge(params)).execute }
 
   describe '#execute' do
     context 'when in_reply_to_discussion_id is specified' do
+      let(:params) { { in_reply_to_discussion_id: note.discussion_id } }
+
       context 'when a note with that original discussion ID exists' do
         it 'sets the note up to be in reply to that note' do
-          new_note = described_class.new(project, author, note: 'Test', in_reply_to_discussion_id: note.discussion_id).execute
           expect(new_note).to be_valid
           expect(new_note.in_reply_to?(note)).to be_truthy
           expect(new_note.resolved?).to be_falsey
         end
 
         context 'when discussion is resolved' do
+          let(:params) { { in_reply_to_discussion_id: mr_note.discussion_id } }
+
           before do
             mr_note.resolve!(author)
           end
 
           it 'resolves the note' do
-            new_note = described_class.new(project, author, note: 'Test', in_reply_to_discussion_id: mr_note.discussion_id).execute
             expect(new_note).to be_valid
             expect(new_note.resolved?).to be_truthy
           end
@@ -36,24 +43,23 @@ RSpec.describe Notes::BuildService do
 
       context 'when a note with that discussion ID exists' do
         it 'sets the note up to be in reply to that note' do
-          new_note = described_class.new(project, author, note: 'Test', in_reply_to_discussion_id: note.discussion_id).execute
           expect(new_note).to be_valid
           expect(new_note.in_reply_to?(note)).to be_truthy
         end
       end
 
       context 'when no note with that discussion ID exists' do
+        let(:params) { { in_reply_to_discussion_id: 'foo' } }
+
         it 'sets an error' do
-          new_note = described_class.new(project, author, note: 'Test', in_reply_to_discussion_id: 'foo').execute
           expect(new_note.errors[:base]).to include('Discussion to reply to cannot be found')
         end
       end
 
       context 'when user has no access to discussion' do
-        it 'sets an error' do
-          another_user = create(:user)
-          new_note = described_class.new(project, another_user, note: 'Test', in_reply_to_discussion_id: note.discussion_id).execute
+        let(:user) { create(:user) }
 
+        it 'sets an error' do
           expect(new_note.errors[:base]).to include('Discussion to reply to cannot be found')
         end
       end
@@ -129,22 +135,21 @@ RSpec.describe Notes::BuildService do
 
     context 'when replying to individual note' do
       let(:note) { create(:note_on_issue) }
-
-      subject { described_class.new(project, author, note: 'Test', in_reply_to_discussion_id: note.discussion_id).execute }
+      let(:params) { { in_reply_to_discussion_id: note.discussion_id } }
 
       it 'sets the note up to be in reply to that note' do
-        expect(subject).to be_valid
-        expect(subject).to be_a(DiscussionNote)
-        expect(subject.discussion_id).to eq(note.discussion_id)
+        expect(new_note).to be_valid
+        expect(new_note).to be_a(DiscussionNote)
+        expect(new_note.discussion_id).to eq(note.discussion_id)
       end
 
       context 'when noteable does not support replies' do
         let(:note) { create(:note_on_commit) }
 
         it 'builds another individual note' do
-          expect(subject).to be_valid
-          expect(subject).to be_a(Note)
-          expect(subject.discussion_id).not_to eq(note.discussion_id)
+          expect(new_note).to be_valid
+          expect(new_note).to be_a(Note)
+          expect(new_note.discussion_id).not_to eq(note.discussion_id)
         end
       end
     end
@@ -156,124 +161,92 @@ RSpec.describe Notes::BuildService do
 
       context 'when replying to a confidential comment' do
         let(:note) { create(:note_on_issue, confidential: true) }
+        let(:params) { { in_reply_to_discussion_id: note.discussion_id, confidential: false } }
 
         context 'when the user can read confidential comments' do
-          subject do
-            described_class.new(
-              project,
-              author,
-              note: 'Test',
-              in_reply_to_discussion_id: note.discussion_id,
-              confidential: false
-            ).execute
-          end
-
           it '`confidential` param is ignored and set to `true`' do
-            expect(subject.confidential).to be_truthy
+            expect(new_note.confidential).to be_truthy
           end
         end
 
         context 'when the user cannot read confidential comments' do
-          let(:another_user) { create(:user) }
-
-          subject do
-            described_class.new(
-              project,
-              another_user,
-              note: 'Test',
-              in_reply_to_discussion_id: note.discussion_id,
-              confidential: false
-            ).execute
-          end
+          let(:user) { create(:user) }
 
           it 'returns `Discussion to reply to cannot be found` error' do
-            expect(subject.errors.first).to include("Discussion to reply to cannot be found")
+            expect(new_note.errors.first).to include("Discussion to reply to cannot be found")
           end
         end
       end
 
       context 'when replying to a public comment' do
         let(:note) { create(:note_on_issue, confidential: false) }
-
-        subject do
-          described_class.new(
-            project,
-            author,
-            note: 'Test',
-            in_reply_to_discussion_id: note.discussion_id,
-            confidential: true
-          ).execute
-        end
+        let(:params) { { in_reply_to_discussion_id: note.discussion_id, confidential: true } }
 
         it '`confidential` param is ignored and set to `false`' do
-          expect(subject.confidential).to be_falsey
+          expect(new_note.confidential).to be_falsey
         end
       end
 
       context 'when creating a new comment' do
         context 'when the `confidential` note flag is set to `true`' do
           context 'when the user is allowed (reporter)' do
-            subject { described_class.new(project, author, note: 'Test', noteable: merge_request, confidential: true).execute }
+            let(:params) { { confidential: true, noteable: merge_request } }
 
             it 'note `confidential` flag is set to `true`' do
-              expect(subject.confidential).to be_truthy
+              expect(new_note.confidential).to be_truthy
             end
           end
 
           context 'when the user is allowed (issuable author)' do
-            let(:another_user) { create(:user) }
-            let(:issue) { create(:issue, author: another_user) }
-
-            subject { described_class.new(project, another_user, note: 'Test', noteable: issue, confidential: true).execute }
+            let(:user) { create(:user) }
+            let(:issue) { create(:issue, author: user) }
+            let(:params) { { confidential: true, noteable: issue } }
 
             it 'note `confidential` flag is set to `true`' do
-              expect(subject.confidential).to be_truthy
+              expect(new_note.confidential).to be_truthy
             end
           end
 
           context 'when the user is allowed (admin)' do
             before do
-              enable_admin_mode!(another_user)
+              enable_admin_mode!(admin)
             end
 
-            let(:another_user) { create(:admin) }
-
-            subject { described_class.new(project, another_user, note: 'Test', noteable: merge_request, confidential: true).execute }
+            let(:admin) { create(:admin) }
+            let(:params) { { confidential: true, noteable: merge_request } }
 
             it 'note `confidential` flag is set to `true`' do
-              expect(subject.confidential).to be_truthy
+              expect(new_note.confidential).to be_truthy
             end
           end
 
           context 'when the user is not allowed' do
-            let(:another_user) { create(:user) }
-
-            subject { described_class.new(project, another_user, note: 'Test', noteable: merge_request, confidential: true).execute }
+            let(:user) { create(:user) }
+            let(:params) { { confidential: true, noteable: merge_request } }
 
             it 'note `confidential` flag is set to `false`' do
-              expect(subject.confidential).to be_falsey
+              expect(new_note.confidential).to be_falsey
             end
           end
         end
 
         context 'when the `confidential` note flag is set to `false`' do
-          subject { described_class.new(project, author, note: 'Test', noteable: merge_request, confidential: false).execute }
+          let(:params) { { confidential: false, noteable: merge_request } }
 
           it 'note `confidential` flag is set to `false`' do
-            expect(subject.confidential).to be_falsey
+            expect(new_note.confidential).to be_falsey
           end
         end
       end
     end
 
-    it 'builds a note without saving it' do
-      new_note = described_class.new(project,
-                                    author,
-                                    noteable_type: note.noteable_type,
-                                    noteable_id: note.noteable_id,
-                                    note: 'Test').execute
-      expect(new_note).to be_valid
-      expect(new_note).not_to be_persisted
+    context 'when noteable is not set' do
+      let(:params) { { noteable_type: note.noteable_type, noteable_id: note.noteable_id } }
+
+      it 'builds a note without saving it' do
+        expect(new_note).to be_valid
+        expect(new_note).not_to be_persisted
+      end
     end
   end
 end