Add latest changes from gitlab-org/gitlab@master

app/assets/javascripts/diffs/components/diff_table_cell.vue

@@ -4,15 +4,13 @@ import { GlIcon } from '@gitlab/ui';
 import { getParameterByName, parseBoolean } from '~/lib/utils/common_utils';
 import DiffGutterAvatars from './diff_gutter_avatars.vue';
 import {
-  MATCH_LINE_TYPE,
   CONTEXT_LINE_TYPE,
   LINE_POSITION_RIGHT,
   EMPTY_CELL_TYPE,
-  OLD_LINE_TYPE,
   OLD_NO_NEW_LINE_TYPE,
+  OLD_LINE_TYPE,
   NEW_NO_NEW_LINE_TYPE,
   LINE_HOVER_CLASS_NAME,
-  LINE_UNFOLD_CLASS_NAME,
 } from '../constants';
 
 export default {
@@ -29,10 +27,6 @@ export default {
       type: String,
       required: true,
     },
-    contextLinesPath: {
-      type: String,
-      required: true,
-    },
     isHighlighted: {
       type: Boolean,
       required: true,
@@ -52,11 +46,6 @@ export default {
       required: false,
       default: '',
     },
-    isContentLine: {
-      type: Boolean,
-      required: false,
-      default: false,
-    },
     isBottom: {
       type: Boolean,
       required: false,
@@ -68,6 +57,11 @@ export default {
       default: false,
     },
   },
+  data() {
+    return {
+      isCommentButtonRendered: false,
+    };
+  },
   computed: {
     ...mapGetters(['isLoggedIn']),
     lineCode() {
@@ -81,13 +75,7 @@ export default {
       return `#${this.line.line_code || ''}`;
     },
     shouldShowCommentButton() {
-      return (
-        this.isHover &&
-        !this.isMatchLine &&
-        !this.isContextLine &&
-        !this.isMetaLine &&
-        !this.hasDiscussions
-      );
+      return this.isHover && !this.isContextLine && !this.isMetaLine && !this.hasDiscussions;
     },
     hasDiscussions() {
       return this.line.discussions && this.line.discussions.length > 0;
@@ -99,6 +87,10 @@ export default {
       return this.showCommentButton && this.hasDiscussions;
     },
     shouldRenderCommentButton() {
+      if (!this.isCommentButtonRendered) {
+        return false;
+      }
+
       if (this.isLoggedIn && this.showCommentButton) {
         const isDiffHead = parseBoolean(getParameterByName('diff_head'));
         return !isDiffHead || gon.features?.mergeRefHeadComments;
@@ -106,9 +98,6 @@ export default {
 
       return false;
     },
-    isMatchLine() {
-      return this.line.type === MATCH_LINE_TYPE;
-    },
     isContextLine() {
       return this.line.type === CONTEXT_LINE_TYPE;
     },
@@ -126,13 +115,8 @@ export default {
         type,
         {
           hll: this.isHighlighted,
-          [LINE_UNFOLD_CLASS_NAME]: this.isMatchLine,
           [LINE_HOVER_CLASS_NAME]:
-            this.isLoggedIn &&
-            this.isHover &&
-            !this.isMatchLine &&
-            !this.isContextLine &&
-            !this.isMetaLine,
+            this.isLoggedIn && this.isHover && !this.isContextLine && !this.isMetaLine,
         },
       ];
     },
@@ -140,6 +124,17 @@ export default {
       return this.lineType === OLD_LINE_TYPE ? this.line.old_line : this.line.new_line;
     },
   },
+  mounted() {
+    this.unwatchShouldShowCommentButton = this.$watch('shouldShowCommentButton', newVal => {
+      if (newVal) {
+        this.isCommentButtonRendered = true;
+        this.unwatchShouldShowCommentButton();
+      }
+    });
+  },
+  beforeDestroy() {
+    this.unwatchShouldShowCommentButton();
+  },
   methods: {
     ...mapActions('diffs', ['showCommentForm', 'setHighlightedRow', 'toggleLineDiscussions']),
     handleCommentButton() {
@@ -151,34 +146,32 @@ export default {
 
 <template>
   <td ref="td" :class="classNameMap">
-    <div>
-      <button
-        v-if="shouldRenderCommentButton"
-        v-show="shouldShowCommentButton"
-        ref="addDiffNoteButton"
-        type="button"
-        class="add-diff-note js-add-diff-note-button qa-diff-comment"
-        title="Add a comment to this line"
-        @click="handleCommentButton"
-      >
-        <gl-icon :size="12" name="comment" />
-      </button>
-      <a
-        v-if="lineNumber"
-        ref="lineNumberRef"
-        :data-linenumber="lineNumber"
-        :href="lineHref"
-        @click="setHighlightedRow(lineCode)"
-      >
-      </a>
-      <diff-gutter-avatars
-        v-if="shouldShowAvatarsOnGutter"
-        :discussions="line.discussions"
-        :discussions-expanded="line.discussionsExpanded"
-        @toggleLineDiscussions="
-          toggleLineDiscussions({ lineCode, fileHash, expanded: !line.discussionsExpanded })
-        "
-      />
-    </div>
+    <button
+      v-if="shouldRenderCommentButton"
+      v-show="shouldShowCommentButton"
+      ref="addDiffNoteButton"
+      type="button"
+      class="add-diff-note js-add-diff-note-button qa-diff-comment"
+      title="Add a comment to this line"
+      @click="handleCommentButton"
+    >
+      <gl-icon :size="12" name="comment" />
+    </button>
+    <a
+      v-if="lineNumber"
+      ref="lineNumberRef"
+      :data-linenumber="lineNumber"
+      :href="lineHref"
+      @click="setHighlightedRow(lineCode)"
+    >
+    </a>
+    <diff-gutter-avatars
+      v-if="shouldShowAvatarsOnGutter"
+      :discussions="line.discussions"
+      :discussions-expanded="line.discussionsExpanded"
+      @toggleLineDiscussions="
+        toggleLineDiscussions({ lineCode, fileHash, expanded: !line.discussionsExpanded })
+      "
+    />
   </td>
 </template>

app/assets/javascripts/diffs/components/inline_diff_table_row.vue

@@ -28,10 +28,6 @@ export default {
       type: String,
       required: true,
     },
-    contextLinesPath: {
-      type: String,
-      required: true,
-    },
     line: {
       type: Object,
       required: true,
@@ -106,7 +102,6 @@ export default {
   >
     <diff-table-cell
       :file-hash="fileHash"
-      :context-lines-path="contextLinesPath"
       :line="line"
       :line-type="oldLineType"
       :is-bottom="isBottom"
@@ -117,7 +112,6 @@ export default {
     />
     <diff-table-cell
       :file-hash="fileHash"
-      :context-lines-path="contextLinesPath"
       :line="line"
       :line-type="newLineType"
       :is-bottom="isBottom"

app/assets/javascripts/diffs/components/inline_diff_view.vue

@@ -65,7 +65,6 @@ export default {
           :key="`${line.line_code || index}`"
           :file-hash="diffFile.file_hash"
           :file-path="diffFile.file_path"
-          :context-lines-path="diffFile.context_lines_path"
           :line="line"
           :is-bottom="index + 1 === diffLinesLength"
         />

app/assets/javascripts/diffs/components/parallel_diff_table_row.vue

@@ -31,10 +31,6 @@ export default {
       type: String,
       required: true,
     },
-    contextLinesPath: {
-      type: String,
-      required: true,
-    },
     line: {
       type: Object,
       required: true,
@@ -144,7 +140,6 @@ export default {
     <template v-if="line.left && !isMatchLineLeft">
       <diff-table-cell
         :file-hash="fileHash"
-        :context-lines-path="contextLinesPath"
         :line="line.left"
         :line-type="oldLineType"
         :is-bottom="isBottom"
@@ -172,7 +167,6 @@ export default {
     <template v-if="line.right && !isMatchLineRight">
       <diff-table-cell
         :file-hash="fileHash"
-        :context-lines-path="contextLinesPath"
         :line="line.right"
         :line-type="newLineType"
         :is-bottom="isBottom"

app/assets/javascripts/diffs/components/parallel_diff_view.vue

@@ -67,7 +67,6 @@ export default {
           :key="line.line_code"
           :file-hash="diffFile.file_hash"
           :file-path="diffFile.file_path"
-          :context-lines-path="diffFile.context_lines_path"
           :line="line"
           :is-bottom="index + 1 === diffLinesLength"
         />

app/models/concerns/reactive_caching.rb

@@ -29,7 +29,7 @@ module ReactiveCaching
     self.reactive_cache_lease_timeout = 2.minutes
     self.reactive_cache_refresh_interval = 1.minute
     self.reactive_cache_lifetime = 10.minutes
-    self.reactive_cache_hard_limit = 1.megabyte
+    self.reactive_cache_hard_limit = nil # this value should be set in megabytes. E.g: 1.megabyte
     self.reactive_cache_work_type = :default
     self.reactive_cache_worker_finder = ->(id, *_args) do
       find_by(primary_key => id)
@@ -159,8 +159,12 @@ module ReactiveCaching
       WORK_TYPE.fetch(self.class.reactive_cache_work_type.to_sym)
     end
 
+    def reactive_cache_limit_enabled?
+      !!self.reactive_cache_hard_limit
+    end
+
     def check_exceeded_reactive_cache_limit!(data)
-      return unless Feature.enabled?(:reactive_cache_limit)
+      return unless reactive_cache_limit_enabled?
 
       data_deep_size = Gitlab::Utils::DeepSize.new(data, max_size: self.class.reactive_cache_hard_limit)
 

app/models/environment.rb

@@ -362,6 +362,11 @@ class Environment < ApplicationRecord
   def generate_slug
     self.slug = Gitlab::Slug::Environment.new(name).generate
   end
+
+  # Overrides ReactiveCaching default to activate limit checking behind a FF
+  def reactive_cache_limit_enabled?
+    Feature.enabled?(:reactive_caching_limit_environment, project)
+  end
 end
 
 Environment.prepend_if_ee('EE::Environment')

app/models/merge_request.rb

@@ -26,7 +26,6 @@ class MergeRequest < ApplicationRecord
   self.reactive_cache_key = ->(model) { [model.project.id, model.iid] }
   self.reactive_cache_refresh_interval = 10.minutes
   self.reactive_cache_lifetime = 10.minutes
-  self.reactive_cache_hard_limit = 20.megabytes
 
   SORTING_PREFERENCE_FIELD = :merge_requests_sort
 

changelogs/unreleased/update_android_ci.yml

+---
+title: Updated the Android CI Script
+merge_request: 34007
+author: s-ayush2903
+type: fixed

config/initializers/01_secret_token.rb

-# WARNING: If you add a new secret to this file, make sure you also
-# update Omnibus GitLab or updates will fail. Omnibus is responsible for
-# writing the `secrets.yml` file.  If Omnibus doesn't know about a
-# secret, Rails will attempt to write to the file, but this will fail
-# because Rails doesn't have write access.
-#
-# As an example:
-# * https://gitlab.com/gitlab-org/gitlab-foss/merge_requests/27581
-# * https://gitlab.com/gitlab-org/omnibus-gitlab/merge_requests/3267
-#
+# WARNING: Before you make a change to secrets.yml, read the development guide for GitLab secrets
+# doc/development/application_secrets.md.
 #
 # This file needs to be loaded BEFORE any initializers that attempt to
 # prepend modules that require access to secrets (e.g. EE's 0_as_concern.rb).

doc/development/application_secrets.md

+# Application secrets
+
+This page is a development guide for application secrets.
+
+## Secret entries
+
+|Entry                             |Description                                                        |
+|---                               |---                                                                |
+|`secret_key_base`                 | The base key to be used for generating a various secrets          |
+| `otp_key_base`                   | The base key for One Time Passwords, described in [User management](../raketasks/user_management.md#rotate-two-factor-authentication-encryption-key)              |
+|`db_key_base`                     | The base key to encrypt the data for `attr_encrypted` columns     |
+|`openid_connect_signing_key`      | The singing key for OpenID Connect                                |
+
+## Where the secrets are stored
+
+|Installation type                  |Location                                                          |
+|---                                |---                                                               |
+|Omnibus                            |[`/etc/gitlab/gitlab-secrets.json`](https://docs.gitlab.com/omnibus/settings/backups.html#backup-and-restore-omnibus-gitlab-configuration)                                 |
+|Cloud Native GitLab Charts         |[Kubernets Secrets](https://gitlab.com/gitlab-org/charts/gitlab/-/blob/f65c3d37fc8cf09a7987544680413552fb666aac/doc/installation/secrets.md#gitlab-rails-secret)|
+|Source                             |`<path-to-gitlab-rails>/config/secrets.yml` (Automatically generated by [01_secret_token.rb](https://gitlab.com/gitlab-org/gitlab/-/blob/master/config/initializers/01_secret_token.rb))                       |
+
+## Warning: Before you add a new secret to application secrets
+
+Before you add a new secret to [`config/initializers/01_secret_token.rb`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/config/initializers/01_secret_token.rb),
+make sure you also update Omnibus GitLab or updates will fail. Omnibus is responsible for writing the `secrets.yml` file.
+If Omnibus doesn't know about a secret, Rails will attempt to write to the file, but this will fail because Rails doesn't have write access.
+The same rules apply to Cloud Native GitLab charts, you must update the charts at first.
+In case you need the secret to have same value on each node (which is usually the case) you need to make sure it's configured for all
+GitLab.com environments prior to changing this file.
+
+**Examples**
+
+- [Change for source installation](https://gitlab.com/gitlab-org/gitlab-foss/merge_requests/27581)
+- [Change for omnibus installation](https://gitlab.com/gitlab-org/omnibus-gitlab/merge_requests/3267)
+- [Change for omnibus installation](https://gitlab.com/gitlab-org/omnibus-gitlab/-/merge_requests/4158)
+- [Change for Cloud Native installation](https://gitlab.com/gitlab-org/charts/gitlab/-/merge_requests/1318)
+
+## Further iteration
+
+We might deprecate/remove this automatic secret generation '01_secret_token.rb' in the future.
+Please see [this issue](https://gitlab.com/gitlab-org/gitlab/-/issues/222690) for more information.

doc/development/foreign_keys.md

@@ -35,6 +35,17 @@ When adding a foreign key in PostgreSQL the column is not indexed automatically,
 thus you must also add a concurrent index. Not doing so will result in cascading
 deletes being very slow.
 
+## Naming foreign keys
+
+By default Ruby on Rails uses the `_id` suffix for foreign keys. So we should
+only use this suffix for associations between two tables. If you want to
+reference an ID on a third party platform the `_xid` suffix is recommended.
+
+The spec `spec/db/schema_spec.rb` will test if all columns with the `_id` suffix
+have a foreign key constraint. So if that spec fails, don't add the column to
+`IGNORED_FK_COLUMNS`, but instead add the FK constraint, or consider naming it
+differently.
+
 ## Dependent Removals
 
 Don't define options such as `dependent: :destroy` or `dependent: :delete` when

doc/user/project/integrations/prometheus.md

@@ -846,6 +846,29 @@ templating:
           default: true                  # (Optional) This option should be the default value of this variable.
 ```
 
+##### `metric_label_values` variable type
+
+CAUTION: **Warning:**
+This variable type is an _alpha_ feature, and is subject to change at any time
+without prior notice!
+
+###### Full syntax
+
+This example creates a variable called `variable2`. The values of the dropdown will
+be all the different values of the `backend` label in the Prometheus series described by
+`up{env="production"}`.
+
+```yaml
+templating:
+  variables:
+    variable2:                           # The variable name that can be interpolated in queries.
+      label: 'Variable 2'                # (Optional) label that will appear in the UI for this dropdown.
+      type: metric_label_values
+      options:
+        series_selector: 'up{env="production"}'
+        label: 'backend'
+```
+
 ### Add related links to custom dashboards
 
 > [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/216385) in GitLab 13.1.

lib/gitlab/ci/templates/Android.gitlab-ci.yml

@@ -4,32 +4,65 @@
 image: openjdk:8-jdk
 
 variables:
-  ANDROID_COMPILE_SDK: "28"
-  ANDROID_BUILD_TOOLS: "28.0.2"
-  ANDROID_SDK_TOOLS: "4333796"
 
+  # ANDROID_COMPILE_SDK is the version of Android you're compiling with.
+  # It should match compileSdkVersion.
+  ANDROID_COMPILE_SDK: "29"
+
+  # ANDROID_BUILD_TOOLS is the version of the Android build tools you are using.
+  # It should match buildToolsVersion.
+  ANDROID_BUILD_TOOLS: "29.0.3"
+
+  # It's what version of the command line tools we're going to download from the official site.
+  # Official Site-> https://developer.android.com/studio/index.html
+  # There, look down below at the cli tools only, sdk tools package is of format:
+  #        commandlinetools-os_type-ANDROID_SDK_TOOLS_latest.zip
+  # when the script was last modified for latest compileSdkVersion, it was which is written down below
+  ANDROID_SDK_TOOLS: "6514223"
+
+# Packages installation before running script
 before_script:
   - apt-get --quiet update --yes
   - apt-get --quiet install --yes wget tar unzip lib32stdc++6 lib32z1
-  - wget --quiet --output-document=android-sdk.zip https://dl.google.com/android/repository/sdk-tools-linux-${ANDROID_SDK_TOOLS}.zip
-  - unzip -d android-sdk-linux android-sdk.zip
-  - echo y | android-sdk-linux/tools/bin/sdkmanager "platforms;android-${ANDROID_COMPILE_SDK}" >/dev/null
-  - echo y | android-sdk-linux/tools/bin/sdkmanager "platform-tools" >/dev/null
-  - echo y | android-sdk-linux/tools/bin/sdkmanager "build-tools;${ANDROID_BUILD_TOOLS}" >/dev/null
-  - export ANDROID_HOME=$PWD/android-sdk-linux
-  - export PATH=$PATH:$PWD/android-sdk-linux/platform-tools/
+
+  # Setup path as android_home for moving/exporting the downloaded sdk into it
+  - export ANDROID_HOME="${PWD}/android-home"
+  # Create a new directory at specified location
+  - install -d $ANDROID_HOME
+  # Here we are installing androidSDK tools from official source,
+  # (the key thing here is the url from where you are downloading these sdk tool for command line, so please do note this url pattern there and here as well)
+  # after that unzipping those tools and
+  # then running a series of SDK manager commands to install necessary android SDK packages that'll allow the app to build
+  - wget --output-document=$ANDROID_HOME/cmdline-tools.zip https://dl.google.com/android/repository/commandlinetools-linux-${ANDROID_SDK_TOOLS}_latest.zip
+  # move to the archive at ANDROID_HOME
+  - pushd $ANDROID_HOME
+  - unzip -d cmdline-tools cmdline-tools.zip
+  - popd
+  - export PATH=$PATH:${ANDROID_HOME}/cmdline-tools/tools/bin/
+
+  # Nothing fancy here, just checking sdkManager version
+  - sdkmanager --version
+
+  # use yes to accept all licenses
+  - yes | sdkmanager --sdk_root=${ANDROID_HOME} --licenses || true
+  - sdkmanager --sdk_root=${ANDROID_HOME} "platforms;android-${ANDROID_COMPILE_SDK}"
+  - sdkmanager --sdk_root=${ANDROID_HOME} "platform-tools"
+  - sdkmanager --sdk_root=${ANDROID_HOME} "build-tools;${ANDROID_BUILD_TOOLS}"
+
+  # Not necessary, but just for surity
   - chmod +x ./gradlew
-  # temporarily disable checking for EPIPE error and use yes to accept all licenses
-  - set +o pipefail
-  - yes | android-sdk-linux/tools/bin/sdkmanager --licenses
-  - set -o pipefail
 
+# Basic android and gradle stuff
+# Check linting
 lintDebug:
+  interruptible: true
   stage: build
   script:
     - ./gradlew -Pci --console=plain :app:lintDebug -PbuildDir=lint
 
+# Make Project
 assembleDebug:
+  interruptible: true
   stage: build
   script:
     - ./gradlew assembleDebug
@@ -37,7 +70,9 @@ assembleDebug:
     paths:
       - app/build/outputs/
 
+# Run all tests, if any fails, interrupt the pipeline(fail it)
 debugTests:
+  interruptible: true
   stage: test
   script:
     - ./gradlew -Pci --console=plain :app:testDebug

locale/gitlab.pot

@@ -25434,6 +25434,9 @@ msgstr ""
 msgid "VulnerabilityManagement|Something went wrong while trying to delete the comment. Please try again later."
 msgstr ""
 
+msgid "VulnerabilityManagement|Something went wrong while trying to refresh the vulnerability. Please try again later."
+msgstr ""
+
 msgid "VulnerabilityManagement|Something went wrong while trying to retrieve the vulnerability history. Please try again later."
 msgstr ""
 

spec/db/schema_spec.rb

@@ -10,8 +10,8 @@ RSpec.describe 'Database schema' do
   let(:tables) { connection.tables }
   let(:columns_name_with_jsonb) { retrieve_columns_name_with_jsonb }
 
-  # Use if you are certain that this column should not have a foreign key
-  # EE: edit the ee/spec/db/schema_support.rb
+  # List of columns historically missing a FK, don't add more columns
+  # See: https://docs.gitlab.com/ce/development/foreign_keys.html#naming-foreign-keys
   IGNORED_FK_COLUMNS = {
     abuse_reports: %w[reporter_id user_id],
     application_settings: %w[performance_bar_allowed_group_id slack_app_id snowplow_app_id eks_account_id eks_access_key_id],
@@ -119,7 +119,11 @@ RSpec.describe 'Database schema' do
           let(:ignored_columns) { ignored_fk_columns(table) }
 
           it 'do have the foreign keys' do
-            expect(column_names_with_id - ignored_columns).to contain_exactly(*foreign_keys_columns)
+            expect(column_names_with_id - ignored_columns).to match_array(foreign_keys_columns)
+          end
+
+          it 'and having foreign key are not in the ignore list' do
+            expect(ignored_columns).to match_array(ignored_columns - foreign_keys)
           end
         end
       end

spec/frontend/diffs/components/diff_table_cell_spec.js

@@ -100,7 +100,11 @@ describe('DiffTableCell', () => {
         setWindowLocation({ href: `${TEST_HOST}?${query}` });
         createComponent({ showCommentButton });
 
-        expect(findNoteButton().exists()).toBe(expectation);
+        wrapper.setData({ isCommentButtonRendered: showCommentButton });
+
+        return wrapper.vm.$nextTick().then(() => {
+          expect(findNoteButton().exists()).toBe(expectation);
+        });
       },
     );
 
@@ -108,7 +112,6 @@ describe('DiffTableCell', () => {
       isHover  | otherProps                                      | discussions | expectation
       ${true}  | ${{}}                                           | ${[]}       | ${true}
       ${false} | ${{}}                                           | ${[]}       | ${false}
-      ${true}  | ${{ line: { ...line, type: 'match' } }}         | ${[]}       | ${false}
       ${true}  | ${{ line: { ...line, type: 'context' } }}       | ${[]}       | ${false}
       ${true}  | ${{ line: { ...line, type: 'old-nonewline' } }} | ${[]}       | ${false}
       ${true}  | ${{}}                                           | ${[{}]}     | ${false}
@@ -122,7 +125,13 @@ describe('DiffTableCell', () => {
           ...otherProps,
         });
 
-        expect(findNoteButton().isVisible()).toBe(expectation);
+        wrapper.setData({
+          isCommentButtonRendered: true,
+        });
+
+        return wrapper.vm.$nextTick().then(() => {
+          expect(findNoteButton().isVisible()).toBe(expectation);
+        });
       },
     );
   });

spec/initializers/secret_token_spec.rb

@@ -19,6 +19,30 @@ RSpec.describe 'create_tokens' do
     allow(self).to receive(:exit)
   end
 
+  describe 'ensure acknowledged secrets in any installations' do
+    let(:acknowledged_secrets) do
+      %w[secret_key_base otp_key_base db_key_base openid_connect_signing_key]
+    end
+
+    it 'does not allow to add a new secret without a proper handling' do
+      create_tokens
+
+      secrets_hash = YAML.load_file(Rails.root.join('config/secrets.yml'))
+
+      secrets_hash.each do |environment, secrets|
+        new_secrets = secrets.keys - acknowledged_secrets
+
+        expect(new_secrets).to be_empty,
+         <<~EOS
+           CAUTION:
+           It looks like you have just added new secret(s) #{new_secrets.inspect} to the secrets.yml.
+           Please read the development guide for GitLab secrets at doc/development/application_secrets.md before you proceed this change.
+           If you're absolutely sure that the change is safe, please add the new secrets to the 'acknowledged_secrets' in order to silence this warning.
+         EOS
+      end
+    end
+  end
+
   context 'setting secret keys' do
     context 'when none of the secrets exist' do
       before do

spec/models/concerns/reactive_caching_spec.rb

@@ -285,38 +285,30 @@ describe ReactiveCaching, :use_clean_rails_memory_store_caching do
         go!
       end
 
-      context 'when calculated object size exceeds default reactive_cache_hard_limit' do
-        let(:calculation) { -> { 'a' * 2 * 1.megabyte } }
+      context 'when reactive_cache_hard_limit is set' do
+        let(:test_class) { Class.new(cache_class_test) { self.reactive_cache_hard_limit = 1.megabyte } }
+        let(:instance) { test_class.new(666, &calculation) }
+
+        context 'when cache size is over the overridden limit' do
+          let(:calculation) { -> { 'a' * 2 * 1.megabyte } }
 
-        shared_examples 'ExceededReactiveCacheLimit' do
           it 'raises ExceededReactiveCacheLimit exception and does not cache new data' do
             expect { go! }.to raise_exception(ReactiveCaching::ExceededReactiveCacheLimit)
 
             expect(read_reactive_cache(instance)).not_to eq(calculation.call)
           end
-        end
 
-        context 'when reactive_cache_hard_limit feature flag is enabled' do
-          it_behaves_like 'ExceededReactiveCacheLimit'
-
-          context 'when reactive_cache_hard_limit is overridden' do
-            let(:test_class) { Class.new(cache_class_test) { self.reactive_cache_hard_limit = 3.megabytes } }
-            let(:instance) { test_class.new(666, &calculation) }
+          context 'when reactive_cache_limit_enabled? is overriden to return false' do
+            before do
+              allow(instance).to receive(:reactive_cache_limit_enabled?).and_return(false)
+            end
 
             it_behaves_like 'successful cache'
-
-            context 'when cache size is over the overridden limit' do
-              let(:calculation) { -> { 'a' * 4 * 1.megabyte } }
-
-              it_behaves_like 'ExceededReactiveCacheLimit'
-            end
           end
         end
 
-        context 'when reactive_cache_limit feature flag is disabled' do
-          before do
-            stub_feature_flags(reactive_cache_limit: false)
-          end
+        context 'when cache size is within the overridden limit' do
+          let(:calculation) { -> { 'Smaller than 1Mb reactive_cache_hard_limit' } }
 
           it_behaves_like 'successful cache'
         end
@@ -377,7 +369,7 @@ describe ReactiveCaching, :use_clean_rails_memory_store_caching do
     it { expect(subject.reactive_cache_refresh_interval).to be_a(ActiveSupport::Duration) }
     it { expect(subject.reactive_cache_lifetime).to be_a(ActiveSupport::Duration) }
     it { expect(subject.reactive_cache_key).to respond_to(:call) }
-    it { expect(subject.reactive_cache_hard_limit).to be_a(Integer) }
+    it { expect(subject.reactive_cache_hard_limit).to be_nil }
     it { expect(subject.reactive_cache_worker_finder).to respond_to(:call) }
   end
 end

spec/models/environment_spec.rb

@@ -847,6 +847,20 @@ describe Environment, :use_clean_rails_memory_store_caching do
 
     subject { environment.calculate_reactive_cache }
 
+    it 'overrides default reactive_cache_hard_limit to 10 Mb' do
+      expect(described_class.reactive_cache_hard_limit).to eq(10.megabyte)
+    end
+
+    it 'overrides reactive_cache_limit_enabled? with a FF' do
+      environment_with_enabled_ff = FactoryBot.build(:environment)
+      environment_with_disabled_ff = FactoryBot.build(:environment)
+
+      stub_feature_flags(reactive_caching_limit_environment: environment_with_enabled_ff.project)
+
+      expect(environment_with_enabled_ff.send(:reactive_cache_limit_enabled?)).to be_truthy
+      expect(environment_with_disabled_ff.send(:reactive_cache_limit_enabled?)).to be_falsey
+    end
+
     it 'returns cache data from the deployment platform' do
       expect(environment.deployment_platform).to receive(:calculate_reactive_cache_for)
         .with(environment).and_return(pods: %w(pod1 pod2))

spec/tooling/lib/tooling/test_file_finder_spec.rb

@@ -88,6 +88,62 @@ RSpec.describe Tooling::TestFileFinder do
       end
     end
 
+    context 'when given a factory file' do
+      let(:file) { 'spec/factories/users.rb' }
+
+      it 'returns spec/factories_spec.rb file' do
+        expect(subject.test_files).to contain_exactly('spec/factories_spec.rb')
+      end
+    end
+
+    context 'when given an ee factory file' do
+      let(:file) { 'ee/spec/factories/users.rb' }
+
+      it 'returns spec/factories_spec.rb file' do
+        expect(subject.test_files).to contain_exactly('spec/factories_spec.rb')
+      end
+    end
+
+    context 'when given db/structure.sql' do
+      let(:file) { 'db/structure.sql' }
+
+      it 'returns spec/db/schema_spec.rb' do
+        expect(subject.test_files).to contain_exactly('spec/db/schema_spec.rb')
+      end
+    end
+
+    context 'when given an initializer' do
+      let(:file) { 'config/initializers/action_mailer_hooks.rb' }
+
+      it 'returns the matching initializer spec' do
+        expect(subject.test_files).to contain_exactly('spec/initializers/action_mailer_hooks_spec.rb')
+      end
+    end
+
+    context 'when given a migration file' do
+      let(:file) { 'db/migrate/20191023152913_add_default_and_free_plans.rb' }
+
+      it 'returns the matching migration spec' do
+        test_files = %w[
+          spec/migrations/add_default_and_free_plans_spec.rb
+          spec/migrations/20191023152913_add_default_and_free_plans_spec.rb
+        ]
+        expect(subject.test_files).to contain_exactly(*test_files)
+      end
+    end
+
+    context 'when given a post-migration file' do
+      let(:file) { 'db/post_migrate/20200608072931_backfill_imported_snippet_repositories.rb' }
+
+      it 'returns the matching migration spec' do
+        test_files = %w[
+          spec/migrations/backfill_imported_snippet_repositories_spec.rb
+          spec/migrations/20200608072931_backfill_imported_snippet_repositories_spec.rb
+        ]
+        expect(subject.test_files).to contain_exactly(*test_files)
+      end
+    end
+
     context 'with foss_test_only: true' do
       subject { Tooling::TestFileFinder.new(file, foss_test_only: true) }
 

tooling/lib/tooling/test_file_finder.rb

@@ -12,7 +12,7 @@ module Tooling
     end
 
     def test_files
-      impacted_tests = ee_impact | non_ee_impact
+      impacted_tests = ee_impact | non_ee_impact | either_impact
       impacted_tests.impact(@file)
     end
 
@@ -23,20 +23,22 @@ module Tooling
     class ImpactedTestFile
       attr_reader :pattern_matchers
 
-      def initialize
+      def initialize(prefix: nil)
         @pattern_matchers = {}
+        @prefix = prefix
 
         yield self if block_given?
       end
 
       def associate(pattern, &block)
-        @pattern_matchers[pattern] = block
+        @pattern_matchers[%r{^#{@prefix}#{pattern}}] = block
       end
 
       def impact(file)
         @pattern_matchers.each_with_object(Set.new) do |(pattern, block), result|
           if (match = pattern.match(file))
-            result << block.call(match)
+            test_files = block.call(match)
+            result.merge(Array(test_files))
           end
         end.to_a
       end
@@ -54,24 +56,37 @@ module Tooling
     end
 
     def ee_impact
-      ImpactedTestFile.new do |impact|
+      ImpactedTestFile.new(prefix: EE_PREFIX) do |impact|
         unless foss_test_only
-          impact.associate(%r{^#{EE_PREFIX}app/(.+)\.rb$}) { |match| "#{EE_PREFIX}spec/#{match[1]}_spec.rb" }
-          impact.associate(%r{^#{EE_PREFIX}app/(.*/)ee/(.+)\.rb$}) { |match| "#{EE_PREFIX}spec/#{match[1]}#{match[2]}_spec.rb" }
-          impact.associate(%r{^#{EE_PREFIX}lib/(.+)\.rb$}) { |match| "#{EE_PREFIX}spec/lib/#{match[1]}_spec.rb" }
-          impact.associate(%r{^#{EE_PREFIX}spec/(.+)_spec.rb$}) { |match| match[0] }
+          impact.associate(%r{app/(.+)\.rb$}) { |match| "#{EE_PREFIX}spec/#{match[1]}_spec.rb" }
+          impact.associate(%r{app/(.*/)ee/(.+)\.rb$}) { |match| "#{EE_PREFIX}spec/#{match[1]}#{match[2]}_spec.rb" }
+          impact.associate(%r{lib/(.+)\.rb$}) { |match| "#{EE_PREFIX}spec/lib/#{match[1]}_spec.rb" }
         end
 
-        impact.associate(%r{^#{EE_PREFIX}(?!spec)(.*/)ee/(.+)\.rb$}) { |match| "spec/#{match[1]}#{match[2]}_spec.rb" }
-        impact.associate(%r{^#{EE_PREFIX}spec/(.*/)ee/(.+)\.rb$}) { |match| "spec/#{match[1]}#{match[2]}.rb" }
+        impact.associate(%r{(?!spec)(.*/)ee/(.+)\.rb$}) { |match| "spec/#{match[1]}#{match[2]}_spec.rb" }
+        impact.associate(%r{spec/(.*/)ee/(.+)\.rb$}) { |match| "spec/#{match[1]}#{match[2]}.rb" }
       end
     end
 
     def non_ee_impact
       ImpactedTestFile.new do |impact|
-        impact.associate(%r{^app/(.+)\.rb$}) { |match| "spec/#{match[1]}_spec.rb" }
-        impact.associate(%r{^(tooling/)?lib/(.+)\.rb$}) { |match| "spec/#{match[1]}lib/#{match[2]}_spec.rb" }
-        impact.associate(%r{^spec/(.+)_spec.rb$}) { |match| match[0] }
+        impact.associate(%r{app/(.+)\.rb$}) { |match| "spec/#{match[1]}_spec.rb" }
+        impact.associate(%r{(tooling/)?lib/(.+)\.rb$}) { |match| "spec/#{match[1]}lib/#{match[2]}_spec.rb" }
+        impact.associate(%r{config/initializers/(.+).rb$}) { |match| "spec/initializers/#{match[1]}_spec.rb" }
+        impact.associate('db/structure.sql') { 'spec/db/schema_spec.rb' }
+        impact.associate(%r{db/(?:post_)?migrate/([0-9]+)_(.+).rb$}) do |match|
+          [
+            "spec/migrations/#{match[2]}_spec.rb",
+            "spec/migrations/#{match[1]}_#{match[2]}_spec.rb"
+          ]
+        end
+      end
+    end
+
+    def either_impact
+      ImpactedTestFile.new(prefix: %r{^(#{EE_PREFIX})?}) do |impact|
+        impact.associate(%r{spec/(.+)_spec.rb$}) { |match| match[0] }
+        impact.associate(%r{spec/factories/.+\.rb$}) { 'spec/factories_spec.rb' }
       end
     end
   end