Add latest changes from gitlab-org/gitlab@master

39 files changed, 604 insertions, 183 deletions

diff --git a/spec/bin/feature_flag_spec.rb b/spec/bin/feature_flag_spec.rb
index 41117880f95..a30e5032d38 100644
--- a/spec/bin/feature_flag_spec.rb
+++ b/spec/bin/feature_flag_spec.rb
@@ -8,6 +8,10 @@ load File.expand_path('../../bin/feature-flag', __dir__)
 RSpec.describe 'bin/feature-flag' do
   using RSpec::Parameterized::TableSyntax
 
+  before do
+    skip_feature_flags_yaml_validation
+  end
+
   describe FeatureFlagCreator do
     let(:argv) { %w[feature-flag-name -t development -g group::memory -i https://url -m http://url] }
     let(:options) { FeatureFlagOptionParser.parse(argv) }
diff --git a/spec/factories/todos.rb b/spec/factories/todos.rb
index 0b5d00cff67..97a1265c46a 100644
--- a/spec/factories/todos.rb
+++ b/spec/factories/todos.rb
@@ -12,6 +12,10 @@ FactoryBot.define do
       action { Todo::ASSIGNED }
     end
 
+    trait :review_requested do
+      action { Todo::REVIEW_REQUESTED }
+    end
+
     trait :mentioned do
       action { Todo::MENTIONED }
     end
diff --git a/spec/features/admin/admin_uses_repository_checks_spec.rb b/spec/features/admin/admin_uses_repository_checks_spec.rb
index b8851c28531..44642983a36 100644
--- a/spec/features/admin/admin_uses_repository_checks_spec.rb
+++ b/spec/features/admin/admin_uses_repository_checks_spec.rb
@@ -46,7 +46,7 @@ RSpec.describe 'Admin uses repository checks', :request_store, :clean_gitlab_red
       )
       visit_admin_project_page(project)
 
-      page.within('.alert') do
+      page.within('.gl-alert') do
         expect(page.text).to match(/Last repository check \(just now\) failed/)
       end
     end
diff --git a/spec/features/dashboard/todos/todos_filtering_spec.rb b/spec/features/dashboard/todos/todos_filtering_spec.rb
index f60b07c976e..b1464af4194 100644
--- a/spec/features/dashboard/todos/todos_filtering_spec.rb
+++ b/spec/features/dashboard/todos/todos_filtering_spec.rb
@@ -130,6 +130,7 @@ RSpec.describe 'Dashboard > User filters todos', :js do
     before do
       create(:todo, :build_failed, user: user_1, author: user_2, project: project_1)
       create(:todo, :marked, user: user_1, author: user_2, project: project_1, target: issue1)
+      create(:todo, :review_requested, user: user_1, author: user_2, project: project_1, target: issue1)
     end
 
     it 'filters by Assigned' do
@@ -138,6 +139,12 @@ RSpec.describe 'Dashboard > User filters todos', :js do
       expect_to_see_action(:assigned)
     end
 
+    it 'filters by Review Requested' do
+      filter_action('Review requested')
+
+      expect_to_see_action(:review_requested)
+    end
+
     it 'filters by Mentioned' do
       filter_action('Mentioned')
 
@@ -168,6 +175,7 @@ RSpec.describe 'Dashboard > User filters todos', :js do
     def expect_to_see_action(action_name)
       action_names = {
         assigned: ' assigned you ',
+        review_requested: ' requested a review of ',
         mentioned: ' mentioned ',
         marked: ' added a todo for ',
         build_failed: ' build failed for '
diff --git a/spec/features/dashboard/todos/todos_spec.rb b/spec/features/dashboard/todos/todos_spec.rb
index cf773d2caed..7243b5d3483 100644
--- a/spec/features/dashboard/todos/todos_spec.rb
+++ b/spec/features/dashboard/todos/todos_spec.rb
@@ -197,6 +197,21 @@ RSpec.describe 'Dashboard Todos' do
         end
       end
     end
+
+    context 'review request todo' do
+      let(:merge_request) { create(:merge_request, title: "Fixes issue") }
+
+      before do
+        create(:todo, :review_requested, user: user, project: project, target: merge_request, author: user)
+        visit dashboard_todos_path
+      end
+
+      it 'shows you set yourself as an reviewer message' do
+        page.within('.js-todos-all') do
+          expect(page).to have_content("You requested a review of merge request #{merge_request.to_reference} \"Fixes issue\" at #{project.namespace.owner_name} / #{project.name} from yourself")
+        end
+      end
+    end
   end
 
   context 'User has done todos', :js do
diff --git a/spec/frontend/incidents/components/incidents_list_spec.js b/spec/frontend/incidents/components/incidents_list_spec.js
index 307806e0a8a..2a56881cab2 100644
--- a/spec/frontend/incidents/components/incidents_list_spec.js
+++ b/spec/frontend/incidents/components/incidents_list_spec.js
@@ -5,7 +5,6 @@ import {
   GlTable,
   GlAvatar,
   GlPagination,
-  GlSearchBoxByType,
   GlTab,
   GlTabs,
   GlBadge,
@@ -15,13 +14,18 @@ import { visitUrl, joinPaths, mergeUrlParams } from '~/lib/utils/url_utility';
 import IncidentsList from '~/incidents/components/incidents_list.vue';
 import SeverityToken from '~/sidebar/components/severity/severity.vue';
 import TimeAgoTooltip from '~/vue_shared/components/time_ago_tooltip.vue';
+import FilteredSearchBar from '~/vue_shared/components/filtered_search_bar/filtered_search_bar_root.vue';
+import AuthorToken from '~/vue_shared/components/filtered_search_bar/tokens/author_token.vue';
 import { I18N, INCIDENT_STATUS_TABS } from '~/incidents/constants';
 import mockIncidents from '../mocks/incidents.json';
+import mockFilters from '../mocks/incidents_filter.json';
 
 jest.mock('~/lib/utils/url_utility', () => ({
   visitUrl: jest.fn().mockName('visitUrlMock'),
   joinPaths: jest.fn().mockName('joinPaths'),
   mergeUrlParams: jest.fn().mockName('mergeUrlParams'),
+  setUrlParams: jest.fn().mockName('setUrlParams'),
+  updateHistory: jest.fn().mockName('updateHistory'),
 }));
 
 describe('Incidents List', () => {
@@ -43,7 +47,7 @@ describe('Incidents List', () => {
   const findTimeAgo = () => wrapper.findAll(TimeAgoTooltip);
   const findDateColumnHeader = () =>
     wrapper.find('[data-testid="incident-management-created-at-sort"]');
-  const findSearch = () => wrapper.find(GlSearchBoxByType);
+  const findSearch = () => wrapper.find(FilteredSearchBar);
   const findAssingees = () => wrapper.findAll('[data-testid="incident-assignees"]');
   const findCreateIncidentBtn = () => wrapper.find('[data-testid="createIncidentBtn"]');
   const findClosedIcon = () => wrapper.findAll("[data-testid='incident-closed']");
@@ -76,6 +80,9 @@ describe('Incidents List', () => {
         issuePath: '/project/isssues',
         publishedAvailable: true,
         emptyListSvgPath,
+        textQuery: '',
+        authorUsernamesQuery: '',
+        assigneeUsernamesQuery: '',
       },
       stubs: {
         GlButton: true,
@@ -315,7 +322,7 @@ describe('Incidents List', () => {
       });
     });
 
-    describe('Search', () => {
+    describe('Filtered search component', () => {
       beforeEach(() => {
         mountComponent({
           data: {
@@ -331,15 +338,62 @@ describe('Incidents List', () => {
       });
 
       it('renders the search component for incidents', () => {
-        expect(findSearch().exists()).toBe(true);
+        expect(findSearch().props('searchInputPlaceholder')).toBe('Search or filter results…');
+        expect(findSearch().props('tokens')).toEqual([
+          {
+            type: 'author_username',
+            icon: 'user',
+            title: 'Author',
+            unique: true,
+            symbol: '@',
+            token: AuthorToken,
+            operators: [{ value: '=', description: 'is', default: 'true' }],
+            fetchPath: '/project/path',
+            fetchAuthors: expect.any(Function),
+          },
+          {
+            type: 'assignee_username',
+            icon: 'user',
+            title: 'Assignees',
+            unique: true,
+            symbol: '@',
+            token: AuthorToken,
+            operators: [{ value: '=', description: 'is', default: 'true' }],
+            fetchPath: '/project/path',
+            fetchAuthors: expect.any(Function),
+          },
+        ]);
+        expect(findSearch().props('recentSearchesStorageKey')).toBe('incidents');
+      });
+
+      it('returns correctly applied filter search values', async () => {
+        const searchTerm = 'foo';
+        wrapper.setData({
+          searchTerm,
+        });
+
+        await wrapper.vm.$nextTick();
+        expect(wrapper.vm.filteredSearchValue).toEqual([searchTerm]);
       });
 
-      it('sets the `searchTerm` graphql variable', () => {
-        const SEARCH_TERM = 'Simple Incident';
+      it('updates props tied to getIncidents GraphQL query', () => {
+        wrapper.vm.handleFilterIncidents(mockFilters);
+
+        expect(wrapper.vm.authorUsername).toBe('root');
+        expect(wrapper.vm.assigneeUsernames).toEqual(['root2']);
+        expect(wrapper.vm.searchTerm).toBe(mockFilters[2].value.data);
+      });
+
+      it('updates props `searchTerm` and `authorUsername` with empty values when passed filters param is empty', () => {
+        wrapper.setData({
+          authorUsername: 'foo',
+          searchTerm: 'bar',
+        });
 
-        findSearch().vm.$emit('input', SEARCH_TERM);
+        wrapper.vm.handleFilterIncidents([]);
 
-        expect(wrapper.vm.$data.searchTerm).toBe(SEARCH_TERM);
+        expect(wrapper.vm.authorUsername).toBe('');
+        expect(wrapper.vm.searchTerm).toBe('');
       });
     });
 
diff --git a/spec/frontend/incidents/mocks/incidents_filter.json b/spec/frontend/incidents/mocks/incidents_filter.json
new file mode 100644
index 00000000000..9f54e259b1d
--- /dev/null
+++ b/spec/frontend/incidents/mocks/incidents_filter.json
@@ -0,0 +1,14 @@
+  [
+    {
+      "type": "assignee_username",
+      "value": { "data": "root2" }
+    },
+    {
+      "type": "author_username",
+      "value": { "data": "root" }
+    },
+    {
+      "type": "filtered-search-term",
+      "value": { "data": "bar" }
+    }
+  ]
\ No newline at end of file
diff --git a/spec/graphql/features/feature_flag_spec.rb b/spec/graphql/features/feature_flag_spec.rb
index b484663d675..9ebc6e595a6 100644
--- a/spec/graphql/features/feature_flag_spec.rb
+++ b/spec/graphql/features/feature_flag_spec.rb
@@ -12,6 +12,10 @@ RSpec.describe 'Graphql Field feature flags' do
   let(:query_string) { '{ item { name } }' }
   let(:result) { execute_query(query_type)['data'] }
 
+  before do
+    skip_feature_flags_yaml_validation
+  end
+
   subject { result }
 
   describe 'Feature flagged field' do
diff --git a/spec/graphql/resolvers/issues_resolver_spec.rb b/spec/graphql/resolvers/issues_resolver_spec.rb
index db5d009f0e7..f5d2effe31a 100644
--- a/spec/graphql/resolvers/issues_resolver_spec.rb
+++ b/spec/graphql/resolvers/issues_resolver_spec.rb
@@ -54,10 +54,21 @@ RSpec.describe Resolvers::IssuesResolver do
         expect(resolve_issues(assignee_id: IssuableFinder::Params::FILTER_ANY)).to contain_exactly(issue2)
       end
 
+      it 'filters by two assignees' do
+        user_2 = create(:user)
+        issue2.update!(assignees: [assignee, user_2])
+
+        expect(resolve_issues(assignee_id: [assignee.id, user_2.id])).to contain_exactly(issue2)
+      end
+
       it 'filters by no assignee' do
         expect(resolve_issues(assignee_id: IssuableFinder::Params::FILTER_NONE)).to contain_exactly(issue1)
       end
 
+      it 'filters by author' do
+        expect(resolve_issues(author_username: issue1.author.username)).to contain_exactly(issue1, issue2)
+      end
+
       it 'filters by labels' do
         expect(resolve_issues(label_name: [label1.title])).to contain_exactly(issue1, issue2)
         expect(resolve_issues(label_name: [label1.title, label2.title])).to contain_exactly(issue2)
diff --git a/spec/graphql/types/base_field_spec.rb b/spec/graphql/types/base_field_spec.rb
index bcfbd7f2480..d61ea6aa6e9 100644
--- a/spec/graphql/types/base_field_spec.rb
+++ b/spec/graphql/types/base_field_spec.rb
@@ -126,6 +126,10 @@ RSpec.describe Types::BaseField do
         let(:field) { described_class.new(name: 'test', type: GraphQL::STRING_TYPE, feature_flag: flag, null: false) }
         let(:context) { {} }
 
+        before do
+          skip_feature_flags_yaml_validation
+        end
+
         it 'returns false if the feature is not enabled' do
           stub_feature_flags(flag => false)
 
diff --git a/spec/helpers/emails_helper_spec.rb b/spec/helpers/emails_helper_spec.rb
index 96ac4015c77..ef8b342a3f6 100644
--- a/spec/helpers/emails_helper_spec.rb
+++ b/spec/helpers/emails_helper_spec.rb
@@ -361,4 +361,116 @@ RSpec.describe EmailsHelper do
       end
     end
   end
+
+  describe '#change_reviewer_notification_text' do
+    let(:mary) { build(:user, name: 'Mary') }
+    let(:john) { build(:user, name: 'John') }
+    let(:ted) { build(:user, name: 'Ted') }
+
+    context 'to new reviewers only' do
+      let(:previous_reviewers) { [] }
+      let(:new_reviewers) { [john] }
+
+      context 'with no html tag' do
+        let(:expected_output) do
+          'Reviewer changed to John'
+        end
+
+        it 'returns the expected output' do
+          expect(change_reviewer_notification_text(new_reviewers, previous_reviewers)).to eq(expected_output)
+        end
+      end
+
+      context 'with <strong> tag' do
+        let(:expected_output) do
+          'Reviewer changed to <strong>John</strong>'
+        end
+
+        it 'returns the expected output' do
+          expect(change_reviewer_notification_text(new_reviewers, previous_reviewers, :strong)).to eq(expected_output)
+        end
+      end
+    end
+
+    context 'from previous reviewers to new reviewers' do
+      let(:previous_reviewers) { [john, mary] }
+      let(:new_reviewers) { [ted] }
+
+      context 'with no html tag' do
+        let(:expected_output) do
+          'Reviewer changed from John and Mary to Ted'
+        end
+
+        it 'returns the expected output' do
+          expect(change_reviewer_notification_text(new_reviewers, previous_reviewers)).to eq(expected_output)
+        end
+      end
+
+      context 'with <strong> tag' do
+        let(:expected_output) do
+          'Reviewer changed from <strong>John and Mary</strong> to <strong>Ted</strong>'
+        end
+
+        it 'returns the expected output' do
+          expect(change_reviewer_notification_text(new_reviewers, previous_reviewers, :strong)).to eq(expected_output)
+        end
+      end
+    end
+
+    context 'from previous reviewers to no reviewers' do
+      let(:previous_reviewers) { [john, mary] }
+      let(:new_reviewers) { [] }
+
+      context 'with no html tag' do
+        let(:expected_output) do
+          'Reviewer changed from John and Mary to Unassigned'
+        end
+
+        it 'returns the expected output' do
+          expect(change_reviewer_notification_text(new_reviewers, previous_reviewers)).to eq(expected_output)
+        end
+      end
+
+      context 'with <strong> tag' do
+        let(:expected_output) do
+          'Reviewer changed from <strong>John and Mary</strong> to <strong>Unassigned</strong>'
+        end
+
+        it 'returns the expected output' do
+          expect(change_reviewer_notification_text(new_reviewers, previous_reviewers, :strong)).to eq(expected_output)
+        end
+      end
+    end
+
+    context "with a <script> tag in user's name" do
+      let(:previous_reviewers) { [] }
+      let(:new_reviewers) { [fishy_user] }
+      let(:fishy_user) { build(:user, name: "<script>alert('hi')</script>") }
+
+      let(:expected_output) do
+        'Reviewer changed to <strong>&lt;script&gt;alert(&#39;hi&#39;)&lt;/script&gt;</strong>'
+      end
+
+      it 'escapes the html tag' do
+        expect(change_reviewer_notification_text(new_reviewers, previous_reviewers, :strong)).to eq(expected_output)
+      end
+    end
+
+    context "with url in user's name" do
+      subject(:email_helper) { Object.new.extend(described_class) }
+
+      let(:previous_reviewers) { [] }
+      let(:new_reviewers) { [fishy_user] }
+      let(:fishy_user) { build(:user, name: "example.com") }
+
+      let(:expected_output) do
+        'Reviewer changed to example_com'
+      end
+
+      it "sanitizes user's name" do
+        expect(email_helper).to receive(:sanitize_name).and_call_original
+        expect(email_helper.change_reviewer_notification_text(new_reviewers, previous_reviewers)).to eq(expected_output)
+      end
+    end
+  end
 end
diff --git a/spec/helpers/projects/incidents_helper_spec.rb b/spec/helpers/projects/incidents_helper_spec.rb
index 0affa67a902..68a5ce4eb91 100644
--- a/spec/helpers/projects/incidents_helper_spec.rb
+++ b/spec/helpers/projects/incidents_helper_spec.rb
@@ -9,9 +9,16 @@ RSpec.describe Projects::IncidentsHelper do
   let(:project_path) { project.full_path }
   let(:new_issue_path) { new_project_issue_path(project) }
   let(:issue_path) { project_issues_path(project) }
+  let(:params) do
+    {
+      search: 'search text',
+      author_username: 'root',
+      assignee_username: 'max.power'
+    }
+  end
 
   describe '#incidents_data' do
-    subject(:data) { helper.incidents_data(project) }
+    subject(:data) { helper.incidents_data(project, params) }
 
     it 'returns frontend configuration' do
       expect(data).to match(
@@ -20,7 +27,10 @@ RSpec.describe Projects::IncidentsHelper do
         'incident-template-name' => 'incident',
         'incident-type' => 'incident',
         'issue-path' => issue_path,
-        'empty-list-svg-path' => match_asset_path('/assets/illustrations/incident-empty-state.svg')
+        'empty-list-svg-path' => match_asset_path('/assets/illustrations/incident-empty-state.svg'),
+        'text-query': 'search text',
+        'author-usernames-query': 'root',
+        'assignee-usernames-query': 'max.power'
       )
     end
   end
diff --git a/spec/lib/api/helpers_spec.rb b/spec/lib/api/helpers_spec.rb
index 51a45dff6a4..92f30a0b341 100644
--- a/spec/lib/api/helpers_spec.rb
+++ b/spec/lib/api/helpers_spec.rb
@@ -195,6 +195,10 @@ RSpec.describe API::Helpers do
     let(:unknown_event) { 'unknown' }
     let(:feature) { "usage_data_#{event_name}" }
 
+    before do
+      skip_feature_flags_yaml_validation
+    end
+
     context 'with feature enabled' do
       before do
         stub_feature_flags(feature => true)
diff --git a/spec/lib/feature/definition_spec.rb b/spec/lib/feature/definition_spec.rb
index 49224cf4279..4344b5d0586 100644
--- a/spec/lib/feature/definition_spec.rb
+++ b/spec/lib/feature/definition_spec.rb
@@ -105,6 +105,7 @@ RSpec.describe Feature::Definition do
   describe '.load_all!' do
     let(:store1) { Dir.mktmpdir('path1') }
     let(:store2) { Dir.mktmpdir('path2') }
+    let(:definitions) { {} }
 
     before do
       allow(described_class).to receive(:paths).and_return(
@@ -113,6 +114,10 @@ RSpec.describe Feature::Definition do
           File.join(store2, '**', '*.yml')
         ]
       )
+
+      # We stub `definitions` to ensure that they
+      # are not overwritten by `.load_all!`
+      allow(described_class).to receive(:definitions).and_return(definitions)
     end
 
     it "when there's no feature flags a list of definitions is empty" do
diff --git a/spec/lib/feature_spec.rb b/spec/lib/feature_spec.rb
index acd7d97ac85..5dff9dbd995 100644
--- a/spec/lib/feature_spec.rb
+++ b/spec/lib/feature_spec.rb
@@ -6,6 +6,7 @@ RSpec.describe Feature, stub_feature_flags: false do
   before do
     # reset Flipper AR-engine
     Feature.reset
+    skip_feature_flags_yaml_validation
   end
 
   describe '.get' do
@@ -253,6 +254,9 @@ RSpec.describe Feature, stub_feature_flags: false do
       end
 
       before do
+        stub_env('LAZILY_CREATE_FEATURE_FLAG', '0')
+
+        allow(Feature::Definition).to receive(:valid_usage!).and_call_original
         allow(Feature::Definition).to receive(:definitions) do
           { definition.key => definition }
         end
diff --git a/spec/lib/gitlab/background_migration/merge_request_assignees_migration_progress_check_spec.rb b/spec/lib/gitlab/background_migration/merge_request_assignees_migration_progress_check_spec.rb
index a3840e3a22e..85a9c88ebff 100644
--- a/spec/lib/gitlab/background_migration/merge_request_assignees_migration_progress_check_spec.rb
+++ b/spec/lib/gitlab/background_migration/merge_request_assignees_migration_progress_check_spec.rb
@@ -73,7 +73,7 @@ RSpec.describe Gitlab::BackgroundMigration::MergeRequestAssigneesMigrationProgre
 
       described_class.new.perform
 
-      expect(Feature.enabled?(:multiple_merge_request_assignees)).to eq(true)
+      expect(Feature.enabled?(:multiple_merge_request_assignees, type: :licensed)).to eq(true)
     end
   end
 
diff --git a/spec/lib/gitlab/database/concurrent_reindex_spec.rb b/spec/lib/gitlab/database/concurrent_reindex_spec.rb
index 4e2c3f547d4..1d1c0946cda 100644
--- a/spec/lib/gitlab/database/concurrent_reindex_spec.rb
+++ b/spec/lib/gitlab/database/concurrent_reindex_spec.rb
@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe Gitlab::Database::ConcurrentReindex, '#execute' do
+RSpec.describe Gitlab::Database::ConcurrentReindex, '#perform' do
   subject { described_class.new(index_name, logger: logger) }
 
   let(:table_name) { '_test_reindex_table' }
@@ -29,7 +29,7 @@ RSpec.describe Gitlab::Database::ConcurrentReindex, '#execute' do
     end
 
     it 'raises an error' do
-      expect { subject.execute }.to raise_error(described_class::ReindexError, /does not exist/)
+      expect { subject.perform }.to raise_error(described_class::ReindexError, /does not exist/)
     end
   end
 
@@ -43,7 +43,7 @@ RSpec.describe Gitlab::Database::ConcurrentReindex, '#execute' do
 
     it 'raises an error' do
       expect do
-        subject.execute
+        subject.perform
       end.to raise_error(described_class::ReindexError, /UNIQUE indexes are currently not supported/)
     end
   end
@@ -57,35 +57,20 @@ RSpec.describe Gitlab::Database::ConcurrentReindex, '#execute' do
 
     let!(:original_index) { find_index_create_statement }
 
-    before do
-      allow(subject).to receive(:connection).and_return(connection)
-      allow(subject).to receive(:disable_statement_timeout).and_yield
-    end
-
-    it 'replaces the existing index with an identical index' do
-      expect(subject).to receive(:disable_statement_timeout).exactly(3).times.and_yield
-
-      expect_to_execute_concurrently_in_order(drop_index)
-      expect_to_execute_concurrently_in_order(create_index)
-
-      expect_next_instance_of(::Gitlab::Database::WithLockRetries) do |instance|
-        expect(instance).to receive(:run).with(raise_on_exhaustion: true).and_yield
+    it 'integration test: executing full index replacement without mocks' do
+      allow(connection).to receive(:execute).and_wrap_original do |method, sql|
+        method.call(sql.sub(/CONCURRENTLY/, ''))
       end
 
-      expect_to_execute_in_order("ALTER INDEX #{index_name} RENAME TO #{replaced_name}")
-      expect_to_execute_in_order("ALTER INDEX #{replacement_name} RENAME TO #{index_name}")
-      expect_to_execute_in_order("ALTER INDEX #{replaced_name} RENAME TO #{replacement_name}")
-
-      expect_to_execute_concurrently_in_order(drop_index)
-
-      subject.execute
+      subject.perform
 
       check_index_exists
     end
 
-    context 'when a dangling index is left from a previous run' do
+    context 'mocked specs' do
       before do
-        connection.execute("CREATE INDEX #{replacement_name} ON #{table_name} (#{column_name})")
+        allow(subject).to receive(:connection).and_return(connection)
+        allow(subject).to receive(:disable_statement_timeout).and_yield
       end
 
       it 'replaces the existing index with an identical index' do
@@ -104,78 +89,105 @@ RSpec.describe Gitlab::Database::ConcurrentReindex, '#execute' do
 
         expect_to_execute_concurrently_in_order(drop_index)
 
-        subject.execute
+        subject.perform
 
         check_index_exists
       end
-    end
 
-    context 'when it fails to create the replacement index' do
-      it 'safely cleans up and signals the error' do
-        expect_to_execute_concurrently_in_order(drop_index)
+      context 'when a dangling index is left from a previous run' do
+        before do
+          connection.execute("CREATE INDEX #{replacement_name} ON #{table_name} (#{column_name})")
+        end
 
-        expect(connection).to receive(:execute).with(create_index).ordered
-          .and_raise(ActiveRecord::ConnectionTimeoutError, 'connect timeout')
+        it 'replaces the existing index with an identical index' do
+          expect(subject).to receive(:disable_statement_timeout).exactly(3).times.and_yield
 
-        expect_to_execute_concurrently_in_order(drop_index)
+          expect_to_execute_concurrently_in_order(drop_index)
+          expect_to_execute_concurrently_in_order(create_index)
 
-        expect { subject.execute }.to raise_error(described_class::ReindexError, /connect timeout/)
+          expect_next_instance_of(::Gitlab::Database::WithLockRetries) do |instance|
+            expect(instance).to receive(:run).with(raise_on_exhaustion: true).and_yield
+          end
 
-        check_index_exists
+          expect_to_execute_in_order("ALTER INDEX #{index_name} RENAME TO #{replaced_name}")
+          expect_to_execute_in_order("ALTER INDEX #{replacement_name} RENAME TO #{index_name}")
+          expect_to_execute_in_order("ALTER INDEX #{replaced_name} RENAME TO #{replacement_name}")
+
+          expect_to_execute_concurrently_in_order(drop_index)
+
+          subject.perform
+
+          check_index_exists
+        end
       end
-    end
 
-    context 'when the replacement index is not valid' do
-      it 'safely cleans up and signals the error' do
-        expect_to_execute_concurrently_in_order(drop_index)
-        expect_to_execute_concurrently_in_order(create_index)
+      context 'when it fails to create the replacement index' do
+        it 'safely cleans up and signals the error' do
+          expect_to_execute_concurrently_in_order(drop_index)
 
-        expect(subject).to receive(:replacement_index_valid?).and_return(false)
+          expect(connection).to receive(:execute).with(create_index).ordered
+            .and_raise(ActiveRecord::ConnectionTimeoutError, 'connect timeout')
 
-        expect_to_execute_concurrently_in_order(drop_index)
+          expect_to_execute_concurrently_in_order(drop_index)
 
-        expect { subject.execute }.to raise_error(described_class::ReindexError, /replacement index was created as INVALID/)
+          expect { subject.perform }.to raise_error(described_class::ReindexError, /connect timeout/)
 
-        check_index_exists
+          check_index_exists
+        end
       end
-    end
 
-    context 'when a database error occurs while swapping the indexes' do
-      it 'safely cleans up and signals the error' do
-        expect_to_execute_concurrently_in_order(drop_index)
-        expect_to_execute_concurrently_in_order(create_index)
+      context 'when the replacement index is not valid' do
+        it 'safely cleans up and signals the error' do
+          expect_to_execute_concurrently_in_order(drop_index)
+          expect_to_execute_concurrently_in_order(create_index)
 
-        expect_next_instance_of(::Gitlab::Database::WithLockRetries) do |instance|
-          expect(instance).to receive(:run).with(raise_on_exhaustion: true).and_yield
+          expect(subject).to receive(:replacement_index_valid?).and_return(false)
+
+          expect_to_execute_concurrently_in_order(drop_index)
+
+          expect { subject.perform }.to raise_error(described_class::ReindexError, /replacement index was created as INVALID/)
+
+          check_index_exists
         end
+      end
 
-        expect(connection).to receive(:execute).ordered
-          .with("ALTER INDEX #{index_name} RENAME TO #{replaced_name}")
-          .and_raise(ActiveRecord::ConnectionTimeoutError, 'connect timeout')
+      context 'when a database error occurs while swapping the indexes' do
+        it 'safely cleans up and signals the error' do
+          expect_to_execute_concurrently_in_order(drop_index)
+          expect_to_execute_concurrently_in_order(create_index)
 
-        expect_to_execute_concurrently_in_order(drop_index)
+          expect_next_instance_of(::Gitlab::Database::WithLockRetries) do |instance|
+            expect(instance).to receive(:run).with(raise_on_exhaustion: true).and_yield
+          end
 
-        expect { subject.execute }.to raise_error(described_class::ReindexError, /connect timeout/)
+          expect(connection).to receive(:execute).ordered
+            .with("ALTER INDEX #{index_name} RENAME TO #{replaced_name}")
+            .and_raise(ActiveRecord::ConnectionTimeoutError, 'connect timeout')
 
-        check_index_exists
-      end
-    end
+          expect_to_execute_concurrently_in_order(drop_index)
 
-    context 'when with_lock_retries fails to acquire the lock' do
-      it 'safely cleans up and signals the error' do
-        expect_to_execute_concurrently_in_order(drop_index)
-        expect_to_execute_concurrently_in_order(create_index)
+          expect { subject.perform }.to raise_error(described_class::ReindexError, /connect timeout/)
 
-        expect_next_instance_of(::Gitlab::Database::WithLockRetries) do |instance|
-          expect(instance).to receive(:run).with(raise_on_exhaustion: true)
-            .and_raise(::Gitlab::Database::WithLockRetries::AttemptsExhaustedError, 'exhausted')
+          check_index_exists
         end
+      end
 
-        expect_to_execute_concurrently_in_order(drop_index)
+      context 'when with_lock_retries fails to acquire the lock' do
+        it 'safely cleans up and signals the error' do
+          expect_to_execute_concurrently_in_order(drop_index)
+          expect_to_execute_concurrently_in_order(create_index)
 
-        expect { subject.execute }.to raise_error(described_class::ReindexError, /exhausted/)
+          expect_next_instance_of(::Gitlab::Database::WithLockRetries) do |instance|
+            expect(instance).to receive(:run).with(raise_on_exhaustion: true)
+              .and_raise(::Gitlab::Database::WithLockRetries::AttemptsExhaustedError, 'exhausted')
+          end
 
-        check_index_exists
+          expect_to_execute_concurrently_in_order(drop_index)
+
+          expect { subject.perform }.to raise_error(described_class::ReindexError, /exhausted/)
+
+          check_index_exists
+        end
       end
     end
   end
diff --git a/spec/lib/gitlab/database/partitioning_migration_helpers/table_management_helpers_spec.rb b/spec/lib/gitlab/database/partitioning_migration_helpers/table_management_helpers_spec.rb
index 44ef0b307fe..bfbd7d1879b 100644
--- a/spec/lib/gitlab/database/partitioning_migration_helpers/table_management_helpers_spec.rb
+++ b/spec/lib/gitlab/database/partitioning_migration_helpers/table_management_helpers_spec.rb
@@ -514,6 +514,7 @@ RSpec.describe Gitlab::Database::PartitioningMigrationHelpers::TableManagementHe
         allow(migration).to receive(:table_exists?).with(partitioned_table).and_return(true)
         allow(migration).to receive(:copy_missed_records)
         allow(migration).to receive(:execute).with(/VACUUM/)
+        allow(migration).to receive(:execute).with(/^(RE)?SET/)
       end
 
       it 'finishes remaining jobs for the correct table' do
@@ -567,6 +568,7 @@ RSpec.describe Gitlab::Database::PartitioningMigrationHelpers::TableManagementHe
 
         allow(Gitlab::BackgroundMigration).to receive(:steal)
         allow(migration).to receive(:execute).with(/VACUUM/)
+        allow(migration).to receive(:execute).with(/^(RE)?SET/)
       end
 
       it 'idempotently cleans up after failed background migrations' do
diff --git a/spec/lib/gitlab/git/rugged_impl/use_rugged_spec.rb b/spec/lib/gitlab/git/rugged_impl/use_rugged_spec.rb
index 4f6a3fb823e..16cea1dc1a3 100644
--- a/spec/lib/gitlab/git/rugged_impl/use_rugged_spec.rb
+++ b/spec/lib/gitlab/git/rugged_impl/use_rugged_spec.rb
@@ -7,7 +7,7 @@ require 'tempfile'
 RSpec.describe Gitlab::Git::RuggedImpl::UseRugged, :seed_helper do
   let(:project) { create(:project, :repository) }
   let(:repository) { project.repository }
-  let(:feature_flag_name) { 'feature-flag-name' }
+  let(:feature_flag_name) { wrapper.rugged_feature_keys.first }
   let(:temp_gitaly_metadata_file) { create_temporary_gitaly_metadata_file }
 
   before(:all) do
@@ -47,7 +47,7 @@ RSpec.describe Gitlab::Git::RuggedImpl::UseRugged, :seed_helper do
     end
   end
 
-  context 'when feature flag is not persisted' do
+  context 'when feature flag is not persisted', stub_feature_flags: false do
     context 'when running puma with multiple threads' do
       before do
         allow(subject).to receive(:running_puma_with_multiple_threads?).and_return(true)
diff --git a/spec/lib/gitlab/gon_helper_spec.rb b/spec/lib/gitlab/gon_helper_spec.rb
index 95db6b2b4e0..3d3f381b6d2 100644
--- a/spec/lib/gitlab/gon_helper_spec.rb
+++ b/spec/lib/gitlab/gon_helper_spec.rb
@@ -10,6 +10,10 @@ RSpec.describe Gitlab::GonHelper do
   end
 
   describe '#push_frontend_feature_flag' do
+    before do
+      skip_feature_flags_yaml_validation
+    end
+
     it 'pushes a feature flag to the frontend' do
       gon = instance_double('gon')
       thing = stub_feature_flag_gate('thing')
diff --git a/spec/lib/gitlab/job_waiter_spec.rb b/spec/lib/gitlab/job_waiter_spec.rb
index 7aa0a3485fb..53d6fbad87a 100644
--- a/spec/lib/gitlab/job_waiter_spec.rb
+++ b/spec/lib/gitlab/job_waiter_spec.rb
@@ -19,6 +19,10 @@ RSpec.describe Gitlab::JobWaiter do
   describe '#wait' do
     let(:waiter) { described_class.new(2) }
 
+    before do
+      allow_any_instance_of(described_class).to receive(:wait).and_call_original
+    end
+
     it 'returns when all jobs have been completed' do
       described_class.notify(waiter.key, 'a')
       described_class.notify(waiter.key, 'b')
diff --git a/spec/lib/gitlab/utils/usage_data_spec.rb b/spec/lib/gitlab/utils/usage_data_spec.rb
index 362cbaa78e9..21ba0251c56 100644
--- a/spec/lib/gitlab/utils/usage_data_spec.rb
+++ b/spec/lib/gitlab/utils/usage_data_spec.rb
@@ -216,6 +216,10 @@ RSpec.describe Gitlab::Utils::UsageData do
     let(:unknown_event) { 'unknown' }
     let(:feature) { "usage_data_#{event_name}" }
 
+    before do
+      skip_feature_flags_yaml_validation
+    end
+
     context 'with feature enabled' do
       before do
         stub_feature_flags(feature => true)
diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb
index a14549ebff3..c54064a182a 100644
--- a/spec/models/merge_request_spec.rb
+++ b/spec/models/merge_request_spec.rb
@@ -4256,24 +4256,6 @@ RSpec.describe MergeRequest, factory_default: :keep do
     end
   end
 
-  describe '#allows_reviewers?' do
-    it 'returns false without merge_request_reviewers feature' do
-      stub_feature_flags(merge_request_reviewers: false)
-
-      merge_request = build_stubbed(:merge_request)
-
-      expect(merge_request.allows_reviewers?).to be(false)
-    end
-
-    it 'returns true with merge_request_reviewers feature' do
-      stub_feature_flags(merge_request_reviewers: true)
-
-      merge_request = build_stubbed(:merge_request)
-
-      expect(merge_request.allows_reviewers?).to be(true)
-    end
-  end
-
   describe '#merge_ref_head' do
     let(:merge_request) { create(:merge_request) }
 
@@ -4299,4 +4281,22 @@ RSpec.describe MergeRequest, factory_default: :keep do
       end
     end
   end
+
+  describe '#allows_reviewers?' do
+    it 'returns false without merge_request_reviewers feature' do
+      stub_feature_flags(merge_request_reviewers: false)
+
+      merge_request = build_stubbed(:merge_request)
+
+      expect(merge_request.allows_reviewers?).to be(false)
+    end
+
+    it 'returns true with merge_request_reviewers feature' do
+      stub_feature_flags(merge_request_reviewers: true)
+
+      merge_request = build_stubbed(:merge_request)
+
+      expect(merge_request.allows_reviewers?).to be(true)
+    end
+  end
 end
diff --git a/spec/models/notification_setting_spec.rb b/spec/models/notification_setting_spec.rb
index 0f765d6b09b..bc50e2af373 100644
--- a/spec/models/notification_setting_spec.rb
+++ b/spec/models/notification_setting_spec.rb
@@ -175,6 +175,7 @@ RSpec.describe NotificationSetting do
         :reopen_merge_request,
         :close_merge_request,
         :reassign_merge_request,
+        :change_reviewer_merge_request,
         :merge_merge_request,
         :failed_pipeline,
         :success_pipeline,
diff --git a/spec/models/todo_spec.rb b/spec/models/todo_spec.rb
index 44e81455a67..0e3473ec0d1 100644
--- a/spec/models/todo_spec.rb
+++ b/spec/models/todo_spec.rb
@@ -200,26 +200,42 @@ RSpec.describe Todo do
   describe '#self_assigned?' do
     let(:user_1) { build(:user) }
 
-    before do
-      subject.user = user_1
-      subject.author = user_1
-      subject.action = Todo::ASSIGNED
-    end
+    context 'when self_added' do
+      before do
+        subject.user = user_1
+        subject.author = user_1
+      end
 
-    it 'is true when todo is ASSIGNED and self_added' do
-      expect(subject).to be_self_assigned
-    end
+      it 'returns true for ASSIGNED' do
+        subject.action = Todo::ASSIGNED
+
+        expect(subject).to be_self_assigned
+      end
 
-    it 'is false when the todo is not ASSIGNED' do
-      subject.action = Todo::MENTIONED
+      it 'returns true for REVIEW_REQUESTED' do
+        subject.action = Todo::REVIEW_REQUESTED
 
-      expect(subject).not_to be_self_assigned
+        expect(subject).to be_self_assigned
+      end
+
+      it 'returns false for other action' do
+        subject.action = Todo::MENTIONED
+
+        expect(subject).not_to be_self_assigned
+      end
     end
 
-    it 'is false when todo is not self_added' do
-      subject.author = build(:user)
+    context 'when todo is not self_added' do
+      before do
+        subject.user = user_1
+        subject.author = build(:user)
+      end
 
-      expect(subject).not_to be_self_assigned
+      it 'returns false' do
+        subject.action = Todo::ASSIGNED
+
+        expect(subject).not_to be_self_assigned
+      end
     end
   end
 
diff --git a/spec/requests/api/features_spec.rb b/spec/requests/api/features_spec.rb
index 2746e777306..3f443b4f92b 100644
--- a/spec/requests/api/features_spec.rb
+++ b/spec/requests/api/features_spec.rb
@@ -12,6 +12,8 @@ RSpec.describe API::Features, stub_feature_flags: false do
     Flipper.register(:perf_team) do |actor|
       actor.respond_to?(:admin) && actor.admin?
     end
+
+    skip_feature_flags_yaml_validation
   end
 
   describe 'GET /features' do
diff --git a/spec/requests/api/usage_data_spec.rb b/spec/requests/api/usage_data_spec.rb
index 46dd54dcc73..4f4f386e9db 100644
--- a/spec/requests/api/usage_data_spec.rb
+++ b/spec/requests/api/usage_data_spec.rb
@@ -66,6 +66,10 @@ RSpec.describe API::UsageData do
       end
 
       context 'with unknown event' do
+        before do
+          skip_feature_flags_yaml_validation
+        end
+
         it 'returns status ok' do
           expect(Gitlab::Redis::HLL).not_to receive(:add)
 
diff --git a/spec/services/issuable/bulk_update_service_spec.rb b/spec/services/issuable/bulk_update_service_spec.rb
index 168a80a97c0..f2bc4f717af 100644
--- a/spec/services/issuable/bulk_update_service_spec.rb
+++ b/spec/services/issuable/bulk_update_service_spec.rb
@@ -254,7 +254,7 @@ RSpec.describe Issuable::BulkUpdateService do
     describe 'unsubscribe from issues' do
       let(:issues) do
         create_list(:closed_issue, 2, project: project) do |issue|
-          issue.subscriptions.create(user: user, project: project, subscribed: true)
+          issue.subscriptions.create!(user: user, project: project, subscribed: true)
         end
       end
 
diff --git a/spec/services/issuable/clone/attributes_rewriter_spec.rb b/spec/services/issuable/clone/attributes_rewriter_spec.rb
index 372e6d480e3..7f434b8b246 100644
--- a/spec/services/issuable/clone/attributes_rewriter_spec.rb
+++ b/spec/services/issuable/clone/attributes_rewriter_spec.rb
@@ -20,7 +20,7 @@ RSpec.describe Issuable::Clone::AttributesRewriter do
       group_label = create(:group_label, title: 'group_label', group: group)
       create(:label, title: 'label3', project: project2)
 
-      original_issue.update(labels: [project1_label_1, project1_label_2, group_label])
+      original_issue.update!(labels: [project1_label_1, project1_label_2, group_label])
 
       subject.execute
 
@@ -48,7 +48,7 @@ RSpec.describe Issuable::Clone::AttributesRewriter do
     it 'sets milestone to nil when old issue milestone is not in the new project' do
       milestone = create(:milestone, title: 'milestone', project: project1)
 
-      original_issue.update(milestone: milestone)
+      original_issue.update!(milestone: milestone)
 
       subject.execute
 
@@ -59,7 +59,7 @@ RSpec.describe Issuable::Clone::AttributesRewriter do
       milestone_project1 = create(:milestone, title: 'milestone', project: project1)
       milestone_project2 = create(:milestone, title: 'milestone', project: project2)
 
-      original_issue.update(milestone: milestone_project1)
+      original_issue.update!(milestone: milestone_project1)
 
       subject.execute
 
@@ -69,7 +69,7 @@ RSpec.describe Issuable::Clone::AttributesRewriter do
     it 'copies the milestone when old issue milestone is a group milestone' do
       milestone = create(:milestone, title: 'milestone', group: group)
 
-      original_issue.update(milestone: milestone)
+      original_issue.update!(milestone: milestone)
 
       subject.execute
 
@@ -85,7 +85,7 @@ RSpec.describe Issuable::Clone::AttributesRewriter do
       let!(:milestone2_project2) { create(:milestone, title: 'milestone2', project: project2) }
 
       before do
-        original_issue.update(milestone: milestone2_project1)
+        original_issue.update!(milestone: milestone2_project1)
 
         create_event(milestone1_project1)
         create_event(milestone2_project1)
diff --git a/spec/services/issuable/common_system_notes_service_spec.rb b/spec/services/issuable/common_system_notes_service_spec.rb
index 217550542bb..fc01ee8f672 100644
--- a/spec/services/issuable/common_system_notes_service_spec.rb
+++ b/spec/services/issuable/common_system_notes_service_spec.rb
@@ -19,7 +19,7 @@ RSpec.describe Issuable::CommonSystemNotesService do
 
       before do
         issuable.labels << label
-        issuable.save
+        issuable.save!
       end
 
       it 'creates a resource label event' do
@@ -69,7 +69,7 @@ RSpec.describe Issuable::CommonSystemNotesService do
     subject { described_class.new(project, user).execute(issuable, old_labels: [], is_update: false) }
 
     it 'does not create system note for title and description' do
-      issuable.save
+      issuable.save!
 
       expect { subject }.not_to change { issuable.notes.count }
     end
@@ -78,7 +78,7 @@ RSpec.describe Issuable::CommonSystemNotesService do
       label = create(:label, project: project)
 
       issuable.labels << label
-      issuable.save
+      issuable.save!
 
       expect { subject }.to change { issuable.resource_label_events.count }.from(0).to(1)
 
@@ -104,7 +104,7 @@ RSpec.describe Issuable::CommonSystemNotesService do
 
     it 'creates a system note for due_date set' do
       issuable.due_date = Date.today
-      issuable.save
+      issuable.save!
 
       expect { subject }.to change { issuable.notes.count }.from(0).to(1)
       expect(issuable.notes.last.note).to match('changed due date')
diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb
index 6b7463d4996..d512cb5983c 100644
--- a/spec/services/merge_requests/update_service_spec.rb
+++ b/spec/services/merge_requests/update_service_spec.rb
@@ -52,7 +52,6 @@ RSpec.describe MergeRequests::UpdateService, :mailer do
           title: 'New title',
           description: 'Also please fix',
           assignee_ids: [user.id],
-          reviewer_ids: [user.id],
           state_event: 'close',
           label_ids: [label.id],
           target_branch: 'target',
@@ -76,7 +75,7 @@ RSpec.describe MergeRequests::UpdateService, :mailer do
         expect(@merge_request).to be_valid
         expect(@merge_request.title).to eq('New title')
         expect(@merge_request.assignees).to match_array([user])
-        expect(@merge_request.reviewers).to match_array([user])
+        expect(@merge_request.reviewers).to match_array([])
         expect(@merge_request).to be_closed
         expect(@merge_request.labels.count).to eq(1)
         expect(@merge_request.labels.first.title).to eq(label.name)
@@ -94,6 +93,7 @@ RSpec.describe MergeRequests::UpdateService, :mailer do
               labels: [],
               mentioned_users: [user2],
               assignees: [user3],
+              reviewers: [],
               milestone: nil,
               total_time_spent: 0,
               description: "FYI #{user2.to_reference}"
@@ -405,15 +405,15 @@ RSpec.describe MergeRequests::UpdateService, :mailer do
       end
 
       context 'when reviewers gets changed' do
-        before do
+        it 'marks pending todo as done' do
           update_merge_request({ reviewer_ids: [user2.id] })
-        end
 
-        it 'marks pending todo as done' do
           expect(pending_todo.reload).to be_done
         end
 
         it 'creates a pending todo for new review request' do
+          update_merge_request({ reviewer_ids: [user2.id] })
+
           attributes = {
             project: project,
             author: user,
@@ -426,6 +426,17 @@ RSpec.describe MergeRequests::UpdateService, :mailer do
 
           expect(Todo.where(attributes).count).to eq 1
         end
+
+        it 'sends email reviewer change notifications to old and new reviewers', :sidekiq_might_not_need_inline do
+          merge_request.reviewers = [user2]
+
+          perform_enqueued_jobs do
+            update_merge_request({ reviewer_ids: [user3.id] })
+          end
+
+          should_email(user2)
+          should_email(user3)
+        end
       end
 
       context 'when the milestone is removed' do
diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb
index 03e24524f9f..af988381017 100644
--- a/spec/services/notification_service_spec.rb
+++ b/spec/services/notification_service_spec.rb
@@ -150,6 +150,16 @@ RSpec.describe NotificationService, :mailer do
     end
   end
 
+  shared_examples 'participating by reviewer notification' do
+    it 'emails the participant' do
+      issuable.reviewers << participant
+
+      notification_trigger
+
+      should_email(participant)
+    end
+  end
+
   shared_examples_for 'participating notifications' do
     it_behaves_like 'participating by note notification'
     it_behaves_like 'participating by author notification'
@@ -1778,6 +1788,60 @@ RSpec.describe NotificationService, :mailer do
       end
     end
 
+    describe '#changed_reviewer_of_merge_request' do
+      let(:merge_request) { create(:merge_request, author: author, source_project: project, reviewers: [reviewer], description: 'cc @participant') }
+
+      let_it_be(:current_user) { create(:user) }
+      let_it_be(:reviewer) { create(:user) }
+
+      before do
+        update_custom_notification(:change_reviewer_merge_request, @u_guest_custom, resource: project)
+        update_custom_notification(:change_reviewer_merge_request, @u_custom_global)
+      end
+
+      it 'sends emails to relevant users only', :aggregate_failures do
+        notification.changed_reviewer_of_merge_request(merge_request, current_user, [reviewer])
+
+        merge_request.reviewers.each { |reviewer| should_email(reviewer) }
+        should_email(merge_request.author)
+        should_email(@u_watcher)
+        should_email(@u_participant_mentioned)
+        should_email(@subscriber)
+        should_email(@watcher_and_subscriber)
+        should_email(@u_guest_watcher)
+        should_email(@u_guest_custom)
+        should_email(@u_custom_global)
+        should_not_email(@unsubscriber)
+        should_not_email(@u_participating)
+        should_not_email(@u_disabled)
+        should_not_email(@u_lazy_participant)
+      end
+
+      it 'adds "review requested" reason for new reviewer' do
+        notification.changed_reviewer_of_merge_request(merge_request, current_user, [reviewer])
+
+        merge_request.reviewers.each do |assignee|
+          email = find_email_for(assignee)
+
+          expect(email).to have_header('X-GitLab-NotificationReason', NotificationReason::REVIEW_REQUESTED)
+        end
+      end
+
+      context 'participating notifications with reviewers' do
+        let(:participant) { create(:user, username: 'user-participant') }
+        let(:issuable) { merge_request }
+        let(:notification_trigger) { notification.changed_reviewer_of_merge_request(merge_request, current_user, [reviewer]) }
+
+        it_behaves_like 'participating notifications'
+        it_behaves_like 'participating by reviewer notification'
+      end
+
+      it_behaves_like 'project emails are disabled' do
+        let(:notification_target)  { merge_request }
+        let(:notification_trigger) { notification.changed_reviewer_of_merge_request(merge_request, current_user, [reviewer]) }
+      end
+    end
+
     describe '#push_to_merge_request' do
       before do
         update_custom_notification(:push_to_merge_request, @u_guest_custom, resource: project)
diff --git a/spec/services/projects/update_remote_mirror_service_spec.rb b/spec/services/projects/update_remote_mirror_service_spec.rb
index 1de04888e0a..f0e76bb81b3 100644
--- a/spec/services/projects/update_remote_mirror_service_spec.rb
+++ b/spec/services/projects/update_remote_mirror_service_spec.rb
@@ -68,25 +68,12 @@ RSpec.describe Projects::UpdateRemoteMirrorService do
     end
 
     context "when given URLs containing escaped elements" do
-      using RSpec::Parameterized::TableSyntax
+      it_behaves_like "URLs containing escaped elements return expected status" do
+        let(:result) { execute! }
 
-      where(:url, :result_status) do
-        "https://user:0a%23@test.example.com/project.git"                               | :success
-        "https://git.example.com:1%2F%2F@source.developers.google.com/project.git"      | :success
-        CGI.escape("git://localhost:1234/some-path?some-query=some-val\#@example.com/") | :error
-        CGI.escape(CGI.escape("https://user:0a%23@test.example.com/project.git"))       | :error
-      end
-
-      with_them do
         before do
           allow(remote_mirror).to receive(:url).and_return(url)
         end
-
-        it "returns expected status" do
-          result = execute!
-
-          expect(result[:status]).to eq(result_status)
-        end
       end
     end
 
diff --git a/spec/services/quick_actions/interpret_service_spec.rb b/spec/services/quick_actions/interpret_service_spec.rb
index b970a48051f..93eca2edb5e 100644
--- a/spec/services/quick_actions/interpret_service_spec.rb
+++ b/spec/services/quick_actions/interpret_service_spec.rb
@@ -3,23 +3,27 @@
 require 'spec_helper'
 
 RSpec.describe QuickActions::InterpretService do
-  let(:project) { create(:project, :public) }
-  let(:developer) { create(:user) }
+  let_it_be(:public_project) { create(:project, :public) }
+  let_it_be(:repository_project) { create(:project, :repository) }
+  let_it_be(:project) { public_project }
+  let_it_be(:developer) { create(:user) }
   let(:developer2) { create(:user) }
-  let(:issue) { create(:issue, project: project) }
+  let_it_be_with_reload(:issue) { create(:issue, project: project) }
   let(:milestone) { create(:milestone, project: project, title: '9.10') }
   let(:commit) { create(:commit, project: project) }
-  let(:inprogress) { create(:label, project: project, title: 'In Progress') }
-  let(:helmchart) { create(:label, project: project, title: 'Helm Chart Registry') }
-  let(:bug) { create(:label, project: project, title: 'Bug') }
-  let(:note) { build(:note, commit_id: merge_request.diff_head_sha) }
+  let_it_be(:inprogress) { create(:label, project: project, title: 'In Progress') }
+  let_it_be(:helmchart) { create(:label, project: project, title: 'Helm Chart Registry') }
+  let_it_be(:bug) { create(:label, project: project, title: 'Bug') }
   let(:service) { described_class.new(project, developer) }
 
+  before_all do
+    public_project.add_developer(developer)
+    repository_project.add_developer(developer)
+  end
+
   before do
     stub_licensed_features(multiple_issue_assignees: false,
                            multiple_merge_request_assignees: false)
-
-    project.add_developer(developer)
   end
 
   describe '#execute' do
@@ -146,7 +150,6 @@ RSpec.describe QuickActions::InterpretService do
 
     shared_examples 'multiword label name starting without ~' do
       it 'fetches label ids and populates add_label_ids if content contains /label' do
-        helmchart # populate the label
         _, updates = service.execute(content, issuable)
 
         expect(updates).to eq(add_label_ids: [helmchart.id])
@@ -155,7 +158,6 @@ RSpec.describe QuickActions::InterpretService do
 
     shared_examples 'label name is included in the middle of another label name' do
       it 'ignores the sublabel when the content contains the includer label name' do
-        helmchart # populate the label
         create(:label, project: project, title: 'Chart')
 
         _, updates = service.execute(content, issuable)
@@ -493,7 +495,7 @@ RSpec.describe QuickActions::InterpretService do
     end
 
     shared_examples 'merge immediately command' do
-      let(:project) { create(:project, :repository) }
+      let(:project) { repository_project }
 
       it 'runs merge command if content contains /merge' do
         _, updates, _ = service.execute(content, issuable)
@@ -509,7 +511,7 @@ RSpec.describe QuickActions::InterpretService do
     end
 
     shared_examples 'merge automatically command' do
-      let(:project) { create(:project, :repository) }
+      let(:project) { repository_project }
 
       it 'runs merge command if content contains /merge and returns merge message' do
         _, updates, message = service.execute(content, issuable)
@@ -600,7 +602,7 @@ RSpec.describe QuickActions::InterpretService do
 
       context 'when issuable is already confidential' do
         before do
-          issuable.update(confidential: true)
+          issuable.update!(confidential: true)
         end
 
         it 'does not return the success message' do
@@ -722,7 +724,7 @@ RSpec.describe QuickActions::InterpretService do
       end
 
       context 'when sha is missing' do
-        let(:project) { create(:project, :repository) }
+        let(:project) { repository_project }
         let(:service) { described_class.new(project, developer, {}) }
 
         it 'precheck passes and returns merge command' do
@@ -844,7 +846,7 @@ RSpec.describe QuickActions::InterpretService do
         end
 
         it 'returns the unassign message for all the assignee if content contains /unassign' do
-          issue.update(assignee_ids: [developer.id, developer2.id])
+          issue.update!(assignee_ids: [developer.id, developer2.id])
           _, _, message = service.execute(content, issue)
 
           expect(message).to eq("Removed assignees #{developer.to_reference} and #{developer2.to_reference}.")
@@ -860,7 +862,7 @@ RSpec.describe QuickActions::InterpretService do
         end
 
         it 'returns the unassign message for all the assignee if content contains /unassign' do
-          merge_request.update(assignee_ids: [developer.id, developer2.id])
+          merge_request.update!(assignee_ids: [developer.id, developer2.id])
           _, _, message = service.execute(content, merge_request)
 
           expect(message).to eq("Removed assignees #{developer.to_reference} and #{developer2.to_reference}.")
@@ -879,10 +881,14 @@ RSpec.describe QuickActions::InterpretService do
     end
 
     context 'only group milestones available' do
-      let(:ancestor_group) { create(:group) }
-      let(:group) { create(:group, parent: ancestor_group) }
-      let(:project) { create(:project, :public, namespace: group) }
-      let(:milestone) { create(:milestone, group: ancestor_group, title: '10.0') }
+      let_it_be(:ancestor_group) { create(:group) }
+      let_it_be(:group) { create(:group, parent: ancestor_group) }
+      let_it_be(:project) { create(:project, :public, namespace: group) }
+      let_it_be(:milestone) { create(:milestone, group: ancestor_group, title: '10.0') }
+
+      before_all do
+        project.add_developer(developer)
+      end
 
       it_behaves_like 'milestone command' do
         let(:content) { "/milestone %#{milestone.title}" }
@@ -1457,14 +1463,14 @@ RSpec.describe QuickActions::InterpretService do
     end
 
     context '/board_move command' do
-      let(:todo) { create(:label, project: project, title: 'To Do') }
-      let(:inreview) { create(:label, project: project, title: 'In Review') }
+      let_it_be(:todo) { create(:label, project: project, title: 'To Do') }
+      let_it_be(:inreview) { create(:label, project: project, title: 'In Review') }
       let(:content) { %{/board_move ~"#{inreview.title}"} }
 
-      let!(:board) { create(:board, project: project) }
-      let!(:todo_list) { create(:list, board: board, label: todo) }
-      let!(:inreview_list) { create(:list, board: board, label: inreview) }
-      let!(:inprogress_list) { create(:list, board: board, label: inprogress) }
+      let_it_be(:board) { create(:board, project: project) }
+      let_it_be(:todo_list) { create(:list, board: board, label: todo) }
+      let_it_be(:inreview_list) { create(:list, board: board, label: inreview) }
+      let_it_be(:inprogress_list) { create(:list, board: board, label: inprogress) }
 
       it 'populates remove_label_ids for all current board columns' do
         issue.update!(label_ids: [todo.id, inprogress.id])
@@ -1599,6 +1605,10 @@ RSpec.describe QuickActions::InterpretService do
       context "when logged user cannot create_merge_requests in the project" do
         let(:project) { create(:project, :archived) }
 
+        before do
+          project.add_developer(developer)
+        end
+
         it_behaves_like 'empty command'
       end
 
@@ -1844,8 +1854,7 @@ RSpec.describe QuickActions::InterpretService do
     end
 
     describe 'relabel command' do
-      let(:content) { '/relabel Bug' }
-      let!(:bug) { create(:label, project: project, title: 'Bug') }
+      let(:content) { "/relabel #{bug.title}" }
       let(:feature) { create(:label, project: project, title: 'Feature') }
 
       it 'includes label name' do
@@ -1938,8 +1947,7 @@ RSpec.describe QuickActions::InterpretService do
     end
 
     describe 'board move command' do
-      let(:content) { '/board_move ~bug' }
-      let!(:bug) { create(:label, project: project, title: 'bug') }
+      let(:content) { "/board_move ~#{bug.title}" }
       let!(:board) { create(:board, project: project) }
 
       it 'includes the label name' do
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
index 6ddced09699..e076d50c3a0 100644
--- a/spec/spec_helper.rb
+++ b/spec/spec_helper.rb
@@ -233,6 +233,12 @@ RSpec.configure do |config|
     # expect(Gitlab::Git::KeepAround).to receive(:execute).and_call_original
     allow(Gitlab::Git::KeepAround).to receive(:execute)
 
+    # Stub these calls due to being expensive operations
+    # It can be reenabled for specific tests via:
+    #
+    # expect(Gitlab::JobWaiter).to receive(:wait).and_call_original
+    allow_any_instance_of(Gitlab::JobWaiter).to receive(:wait)
+
     Gitlab::ProcessMemoryCache.cache_backend.clear
 
     Sidekiq::Worker.clear_all
diff --git a/spec/support/helpers/stub_feature_flags.rb b/spec/support/helpers/stub_feature_flags.rb
index 792a1c21c31..7f30a2a70cd 100644
--- a/spec/support/helpers/stub_feature_flags.rb
+++ b/spec/support/helpers/stub_feature_flags.rb
@@ -62,4 +62,8 @@ module StubFeatureFlags
 
     StubFeatureGate.new(object)
   end
+
+  def skip_feature_flags_yaml_validation
+    allow(Feature::Definition).to receive(:valid_usage!)
+  end
 end
diff --git a/spec/support/shared_examples/services/projects/urls_with_escaped_elements_shared_example.rb b/spec/support/shared_examples/services/projects/urls_with_escaped_elements_shared_example.rb
new file mode 100644
index 00000000000..df8b1f91629
--- /dev/null
+++ b/spec/support/shared_examples/services/projects/urls_with_escaped_elements_shared_example.rb
@@ -0,0 +1,20 @@
+# frozen_string_literal: true
+
+# Shared examples that test requests against URLs with escaped elements
+#
+RSpec.shared_examples "URLs containing escaped elements return expected status" do
+  using RSpec::Parameterized::TableSyntax
+
+  where(:url, :result_status) do
+    "https://user:0a%23@test.example.com/project.git"                               | :success
+    "https://git.example.com:1%2F%2F@source.developers.google.com/project.git"      | :success
+    CGI.escape("git://localhost:1234/some-path?some-query=some-val\#@example.com/") | :error
+    CGI.escape(CGI.escape("https://user:0a%23@test.example.com/project.git"))       | :error
+  end
+
+  with_them do
+    it "returns expected status" do
+      expect(result[:status]).to eq(result_status)
+    end
+  end
+end
diff --git a/spec/support_specs/helpers/stub_feature_flags_spec.rb b/spec/support_specs/helpers/stub_feature_flags_spec.rb
index 5d1e4e1627d..7434929d665 100644
--- a/spec/support_specs/helpers/stub_feature_flags_spec.rb
+++ b/spec/support_specs/helpers/stub_feature_flags_spec.rb
@@ -3,12 +3,31 @@
 require 'spec_helper'
 
 RSpec.describe StubFeatureFlags do
-  let(:feature_name) { :test_feature }
+  DUMMY_FEATURE_FLAG = :dummy_feature_flag # rubocop:disable RSpec/LeakyConstantDeclaration
+
+  # We inject dummy feature flag defintion
+  # to ensure that we strong validate it's usage
+  # as well
+  before(:all) do
+    definition = Feature::Definition.new(
+      nil,
+      name: DUMMY_FEATURE_FLAG,
+      type: 'development',
+      # we allow ambigious usage of `default_enabled:`
+      default_enabled: [false, true]
+    )
+
+    Feature::Definition.definitions[DUMMY_FEATURE_FLAG] = definition
+  end
+
+  after(:all) do
+    Feature::Definition.definitions.delete(DUMMY_FEATURE_FLAG)
+  end
 
   describe '#stub_feature_flags' do
     using RSpec::Parameterized::TableSyntax
 
-    let(:feature_name) { :test_feature }
+    let(:feature_name) { DUMMY_FEATURE_FLAG }
 
     context 'when checking global state' do
       where(:feature_actors, :expected_result) do
@@ -121,14 +140,14 @@ RSpec.describe StubFeatureFlags do
 
   describe 'stub timing' do
     context 'let_it_be variable' do
-      let_it_be(:let_it_be_var) { Feature.enabled?(:any_feature_flag) }
+      let_it_be(:let_it_be_var) { Feature.enabled?(DUMMY_FEATURE_FLAG) }
 
       it { expect(let_it_be_var).to eq true }
     end
 
     context 'before_all variable' do
       before_all do
-        @suite_var = Feature.enabled?(:any_feature_flag)
+        @suite_var = Feature.enabled?(DUMMY_FEATURE_FLAG)
       end
 
       it { expect(@suite_var).to eq true }
@@ -136,14 +155,14 @@ RSpec.describe StubFeatureFlags do
 
     context 'before(:all) variable' do
       before(:all) do
-        @suite_var = Feature.enabled?(:any_feature_flag)
+        @suite_var = Feature.enabled?(DUMMY_FEATURE_FLAG)
       end
 
       it { expect(@suite_var).to eq true }
     end
 
     context 'with stub_feature_flags meta' do
-      let(:var) { Feature.enabled?(:any_feature_flag) }
+      let(:var) { Feature.enabled?(DUMMY_FEATURE_FLAG) }
 
       context 'as true', :stub_feature_flags do
         it { expect(var).to eq true }
diff --git a/spec/tasks/gitlab/db_rake_spec.rb b/spec/tasks/gitlab/db_rake_spec.rb
index 99efd394e83..7a65aca0ee7 100644
--- a/spec/tasks/gitlab/db_rake_spec.rb
+++ b/spec/tasks/gitlab/db_rake_spec.rb
@@ -180,7 +180,7 @@ RSpec.describe 'gitlab:db namespace rake task' do
         .with('some_index_name', logger: instance_of(Logger))
         .and_return(reindex)
 
-      expect(reindex).to receive(:execute)
+      expect(reindex).to receive(:perform)
 
       run_rake_task('gitlab:db:reindex', '[some_index_name]')
     end