Add latest changes from gitlab-org/gitlab@master

spec/frontend/ci_variable_list/store/utils_spec.js

@@ -19,6 +19,7 @@ describe('CI variables store utils', () => {
       key: 'test_var',
       masked: 'false',
       protected: 'false',
+      secret_value: 'test_val',
       value: 'test_val',
       variable_type: 'env_var',
     });
@@ -29,6 +30,7 @@ describe('CI variables store utils', () => {
       key: 'test_var_2',
       masked: 'false',
       protected: 'false',
+      secret_value: 'test_val_2',
       value: 'test_val_2',
       variable_type: 'file',
     });

spec/frontend/contributors/store/getters_spec.js

@@ -29,33 +29,39 @@ describe('Contributors Store Getters', () => {
       beforeAll(() => {
         state.chartData = [
+          { author_name: 'John Smith', author_email: 'jawnnypoo@gmail.com', date: '2019-05-05' },
           { author_name: 'John', author_email: 'jawnnypoo@gmail.com', date: '2019-05-05' },
-          { author_name: 'John', author_email: 'jawnnypoo@gmail.com', date: '2019-05-05' },
-          { author_name: 'Carlson', author_email: 'jawnnypoo@gmail.com', date: '2019-03-03' },
-          { author_name: 'Carlson', author_email: 'jawnnypoo@gmail.com', date: '2019-05-05' },
-          { author_name: 'John', author_email: 'jawnnypoo@gmail.com', date: '2019-04-04' },
+          { author_name: 'Carlson', author_email: 'carlson123@gitlab.com', date: '2019-03-03' },
+          { author_name: 'Carlson', author_email: 'carlson123@gmail.com', date: '2019-05-05' },
           { author_name: 'John', author_email: 'jawnnypoo@gmail.com', date: '2019-04-04' },
+          { author_name: 'Johan', author_email: 'jawnnypoo@gmail.com', date: '2019-04-04' },
           { author_name: 'John', author_email: 'jawnnypoo@gmail.com', date: '2019-03-03' },
         ];
         parsed = getters.parsedData(state);
       });
-      it('should group contributions by date ', () => {
+      it('should group contributions by date', () => {
         expect(parsed.total).toMatchObject({ '2019-05-05': 3, '2019-03-03': 2, '2019-04-04': 2 });
       });
-      it('should group contributions by author ', () => {
-        expect(parsed.byAuthor).toMatchObject({
-          Carlson: {
-            email: 'jawnnypoo@gmail.com',
-            commits: 2,
+      it('should group contributions by email and use most recent name', () => {
+        expect(parsed.byAuthorEmail).toMatchObject({
+          'carlson123@gmail.com': {
+            name: 'Carlson',
+            commits: 1,
             dates: {
-              '2019-03-03': 1,
               '2019-05-05': 1,
             },
           },
-          John: {
-            email: 'jawnnypoo@gmail.com',
+          'carlson123@gitlab.com': {
+            name: 'Carlson',
+            commits: 1,
+            dates: {
+              '2019-03-03': 1,
+            },
+          },
+          'jawnnypoo@gmail.com': {
+            name: 'John Smith',
             commits: 5,
             dates: {
               '2019-03-03': 1,

spec/frontend/reports/store/utils_spec.js

@@ -30,9 +30,7 @@ describe('Reports store utils', () => {
       const data = { failed: 3, total: 10 };
       const result = utils.summaryTextBuilder(name, data);
-      expect(result).toBe(
-        'Test summary contained 3 failed/error test results out of 10 total tests',
-      );
+      expect(result).toBe('Test summary contained 3 failed out of 10 total tests');
     });
     it('should render text for multiple errored results', () => {
@@ -40,9 +38,7 @@ describe('Reports store utils', () => {
       const data = { errored: 7, total: 10 };
       const result = utils.summaryTextBuilder(name, data);
-      expect(result).toBe(
-        'Test summary contained 7 failed/error test results out of 10 total tests',
-      );
+      expect(result).toBe('Test summary contained 7 errors out of 10 total tests');
     });
     it('should render text for multiple fixed results', () => {
@@ -59,7 +55,7 @@ describe('Reports store utils', () => {
       const result = utils.summaryTextBuilder(name, data);
       expect(result).toBe(
-        'Test summary contained 3 failed/error test results and 4 fixed test results out of 10 total tests',
+        'Test summary contained 3 failed and 4 fixed test results out of 10 total tests',
       );
     });
@@ -69,18 +65,17 @@ describe('Reports store utils', () => {
       const result = utils.summaryTextBuilder(name, data);
       expect(result).toBe(
-        'Test summary contained 1 failed/error test result and 1 fixed test result out of 10 total tests',
+        'Test summary contained 1 failed and 1 fixed test result out of 10 total tests',
       );
     });
     it('should render text for singular failed, errored, and fixed results', () => {
-      // these will be singular when the copy is updated
       const name = 'Test summary';
       const data = { failed: 1, errored: 1, resolved: 1, total: 10 };
       const result = utils.summaryTextBuilder(name, data);
       expect(result).toBe(
-        'Test summary contained 2 failed/error test results and 1 fixed test result out of 10 total tests',
+        'Test summary contained 1 failed, 1 error and 1 fixed test result out of 10 total tests',
       );
     });
@@ -90,7 +85,7 @@ describe('Reports store utils', () => {
       const result = utils.summaryTextBuilder(name, data);
       expect(result).toBe(
-        'Test summary contained 5 failed/error test results and 4 fixed test results out of 10 total tests',
+        'Test summary contained 2 failed, 3 errors and 4 fixed test results out of 10 total tests',
       );
     });
   });
@@ -117,7 +112,7 @@ describe('Reports store utils', () => {
       const data = { failed: 3, total: 10 };
       const result = utils.reportTextBuilder(name, data);
-      expect(result).toBe('Rspec found 3 failed/error test results out of 10 total tests');
+      expect(result).toBe('Rspec found 3 failed out of 10 total tests');
     });
     it('should render text for multiple errored results', () => {
@@ -125,7 +120,7 @@ describe('Reports store utils', () => {
       const data = { errored: 7, total: 10 };
       const result = utils.reportTextBuilder(name, data);
-      expect(result).toBe('Rspec found 7 failed/error test results out of 10 total tests');
+      expect(result).toBe('Rspec found 7 errors out of 10 total tests');
     });
     it('should render text for multiple fixed results', () => {
@@ -141,9 +136,7 @@ describe('Reports store utils', () => {
       const data = { failed: 3, resolved: 4, total: 10 };
       const result = utils.reportTextBuilder(name, data);
-      expect(result).toBe(
-        'Rspec found 3 failed/error test results and 4 fixed test results out of 10 total tests',
-      );
+      expect(result).toBe('Rspec found 3 failed and 4 fixed test results out of 10 total tests');
     });
     it('should render text for a singular fixed, and a singular failed result', () => {
@@ -151,19 +144,16 @@ describe('Reports store utils', () => {
       const data = { failed: 1, resolved: 1, total: 10 };
       const result = utils.reportTextBuilder(name, data);
-      expect(result).toBe(
-        'Rspec found 1 failed/error test result and 1 fixed test result out of 10 total tests',
-      );
+      expect(result).toBe('Rspec found 1 failed and 1 fixed test result out of 10 total tests');
     });
     it('should render text for singular failed, errored, and fixed results', () => {
-      // these will be singular when the copy is updated
       const name = 'Rspec';
       const data = { failed: 1, errored: 1, resolved: 1, total: 10 };
       const result = utils.reportTextBuilder(name, data);
       expect(result).toBe(
-        'Rspec found 2 failed/error test results and 1 fixed test result out of 10 total tests',
+        'Rspec found 1 failed, 1 error and 1 fixed test result out of 10 total tests',
       );
     });
@@ -173,7 +163,7 @@ describe('Reports store utils', () => {
       const result = utils.reportTextBuilder(name, data);
       expect(result).toBe(
-        'Rspec found 5 failed/error test results and 4 fixed test results out of 10 total tests',
+        'Rspec found 2 failed, 3 errors and 4 fixed test results out of 10 total tests',
       );
     });
   });

spec/frontend/sidebar/mock_data.js

@@ -204,7 +204,7 @@ const mockData = {
     },
     rootPath: '/',
     fullPath: '/gitlab-org/gitlab-shell',
-    id: 1,
+    iid: 1,
   },
   time: {
     time_estimate: 3600,

spec/javascripts/reports/components/grouped_test_reports_app_spec.js

@@ -84,12 +84,10 @@ describe('Grouped Test Reports App', () => {
       setTimeout(() => {
         expect(vm.$el.querySelector('.gl-spinner')).toBeNull();
         expect(vm.$el.querySelector('.js-code-text').textContent.trim()).toEqual(
-          'Test summary contained 2 failed/error test results out of 11 total tests',
+          'Test summary contained 2 failed out of 11 total tests',
         );
-        expect(vm.$el.textContent).toContain(
-          'rspec:pg found 2 failed/error test results out of 8 total tests',
-        );
+        expect(vm.$el.textContent).toContain('rspec:pg found 2 failed out of 8 total tests');
         expect(vm.$el.textContent).toContain('New');
         expect(vm.$el.textContent).toContain(
@@ -112,12 +110,10 @@ describe('Grouped Test Reports App', () => {
       setTimeout(() => {
         expect(vm.$el.querySelector('.gl-spinner')).toBeNull();
         expect(vm.$el.querySelector('.js-code-text').textContent.trim()).toEqual(
-          'Test summary contained 2 failed/error test results out of 11 total tests',
+          'Test summary contained 2 errors out of 11 total tests',
         );
-        expect(vm.$el.textContent).toContain(
-          'karma found 2 failed/error test results out of 3 total tests',
-        );
+        expect(vm.$el.textContent).toContain('karma found 2 errors out of 3 total tests');
         expect(vm.$el.textContent).toContain('New');
         expect(vm.$el.textContent).toContain(
@@ -140,17 +136,15 @@ describe('Grouped Test Reports App', () => {
       setTimeout(() => {
         expect(vm.$el.querySelector('.gl-spinner')).toBeNull();
         expect(vm.$el.querySelector('.js-code-text').textContent.trim()).toEqual(
-          'Test summary contained 2 failed/error test results and 2 fixed test results out of 11 total tests',
+          'Test summary contained 2 failed and 2 fixed test results out of 11 total tests',
         );
         expect(vm.$el.textContent).toContain(
-          'rspec:pg found 1 failed/error test result and 2 fixed test results out of 8 total tests',
+          'rspec:pg found 1 failed and 2 fixed test results out of 8 total tests',
         );
         expect(vm.$el.textContent).toContain('New');
-        expect(vm.$el.textContent).toContain(
-          ' java ant found 1 failed/error test result out of 3 total tests',
-        );
+        expect(vm.$el.textContent).toContain(' java ant found 1 failed out of 3 total tests');
         done();
       }, 0);
     });

spec/lib/gitlab/git/repository_spec.rb

@@ -2151,11 +2151,11 @@ describe Gitlab::Git::Repository, :seed_helper do
         'gitaly_address' => Gitlab.config.repositories.storages.default.gitaly_address,
         'path' => TestEnv::SECOND_STORAGE_PATH
       })
-      Gitlab::Shell.new.create_repository('test_second_storage', TEST_REPO_PATH, 'group/project')
+      new_repository.create_repository
     end
     after do
-      Gitlab::Shell.new.remove_repository('test_second_storage', TEST_REPO_PATH)
+      new_repository.remove
     end
     it 'mirrors the source repository' do

spec/lib/gitlab/import_export/fast_hash_serializer_spec.rb

@@ -215,6 +215,14 @@ describe Gitlab::ImportExport::FastHashSerializer do
     expect(subject['boards'].first['lists']).not_to be_empty
   end
+  context 'relation ordering' do
+    it 'orders exported pipelines by primary key' do
+      expected_order = project.ci_pipelines.reorder(:id).ids
+
+      expect(subject['ci_pipelines'].pluck('id')).to eq(expected_order)
+    end
+  end
+
   def setup_project
     release = create(:release)
     group = create(:group)
@@ -246,6 +254,8 @@ describe Gitlab::ImportExport::FastHashSerializer do
     ci_build.pipeline.update(project: project)
     create(:commit_status, project: project, pipeline: ci_build.pipeline)
+    create_list(:ci_pipeline, 5, :success, project: project)
+
     create(:milestone, project: project)
     create(:discussion_note, noteable: issue, project: project)
     create(:note, noteable: merge_request, project: project)

spec/models/concerns/has_ref_spec.rb → spec/models/concerns/ci/has_ref_spec.rb

@@ -2,7 +2,7 @@
 require 'spec_helper'
-describe HasRef do
+describe Ci::HasRef do
   describe '#branch?' do
     let(:build) { create(:ci_build) }

spec/requests/api/pipelines_spec.rb

@@ -254,7 +254,9 @@ describe API::Pipelines do
         context 'when order_by and sort are specified' do
           context 'when order_by user_id' do
             before do
-              create_list(:ci_pipeline, 3, project: project, user: create(:user))
+              create_list(:user, 3).each do |some_user|
+                create(:ci_pipeline, project: project, user: some_user)
+              end
             end
             context 'when sort parameter is valid' do

spec/rubocop/cop/migration/with_lock_retries_with_change_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+require 'rubocop'
+require 'rubocop/rspec/support'
+
+require_relative '../../../../rubocop/cop/migration/with_lock_retries_with_change'
+
+describe RuboCop::Cop::Migration::WithLockRetriesWithChange do
+  include CopHelper
+
+  subject(:cop) { described_class.new }
+
+  context 'in migration' do
+    before do
+      allow(cop).to receive(:in_migration?).and_return(true)
+    end
+
+    it 'registers an offense when `with_lock_retries` is used inside a `change` method' do
+      inspect_source('def change; with_lock_retries {}; end')
+
+      aggregate_failures do
+        expect(cop.offenses.size).to eq(1)
+        expect(cop.offenses.map(&:line)).to eq([1])
+      end
+    end
+
+    it 'registers no offense when `with_lock_retries` is used inside an `up` method' do
+      inspect_source('def up; with_lock_retries {}; end')
+
+      expect(cop.offenses.size).to eq(0)
+    end
+  end
+
+  context 'outside of migration' do
+    it 'registers no offense' do
+      inspect_source('def change; with_lock_retries {}; end')
+
+      expect(cop.offenses.size).to eq(0)
+    end
+  end
+end

spec/serializers/group_variable_entity_spec.rb

@@ -10,7 +10,7 @@ describe GroupVariableEntity do
     subject { entity.as_json }
     it 'contains required fields' do
-      expect(subject).to include(:id, :key, :value, :protected)
+      expect(subject).to include(:id, :key, :value, :protected, :variable_type)
     end
   end
 end

spec/support/matchers/exceed_query_limit.rb

 # frozen_string_literal: true
 module ExceedQueryLimitHelpers
+  MARGINALIA_ANNOTATION_REGEX = %r{\s*\/\*.*\*\/}.freeze
+
   def with_threshold(threshold)
     @threshold = threshold
     self
@@ -41,8 +43,8 @@ module ExceedQueryLimitHelpers
   def log_message
     if expected.is_a?(ActiveRecord::QueryRecorder)
-      counts = count_queries(expected.log)
-      extra_queries = @recorder.log.reject { |query| counts[query] -= 1 unless counts[query].zero? }
+      counts = count_queries(strip_marginalia_annotations(expected.log))
+      extra_queries = strip_marginalia_annotations(@recorder.log).reject { |query| counts[query] -= 1 unless counts[query].zero? }
       extra_queries_display = count_queries(extra_queries).map { |query, count| "[#{count}] #{query}" }
       (['Extra queries:'] + extra_queries_display).join("\n\n")
@@ -65,6 +67,10 @@ module ExceedQueryLimitHelpers
     counts = "#{expected_count}#{threshold_message}"
     "Expected a maximum of #{counts} queries, got #{actual_count}:\n\n#{log_message}"
   end
+
+  def strip_marginalia_annotations(logs)
+    logs.map { |log| log.sub(MARGINALIA_ANNOTATION_REGEX, '') }
+  end
 end
 RSpec::Matchers.define :exceed_all_query_limit do |expected|

spec/support_specs/matchers/exceed_query_limit_helpers_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe ExceedQueryLimitHelpers do
+  class TestQueries < ActiveRecord::Base
+    self.table_name = 'schema_migrations'
+  end
+
+  class TestMatcher
+    include ExceedQueryLimitHelpers
+
+    def expected
+      ActiveRecord::QueryRecorder.new do
+        2.times { TestQueries.count }
+      end
+    end
+  end
+
+  it 'does not contain marginalia annotations' do
+    test_matcher = TestMatcher.new
+    test_matcher.verify_count do
+      2.times { TestQueries.count }
+      TestQueries.first
+    end
+
+    aggregate_failures do
+      expect(test_matcher.log_message)
+        .to match(%r{ORDER BY.*#{TestQueries.table_name}.*LIMIT 1})
+      expect(test_matcher.log_message)
+        .not_to match(%r{\/\*.*correlation_id.*\*\/})
+    end
+  end
+end