Automatic merge of gitlab-org/gitlab master

app/models/merge_request.rb

@@ -1853,25 +1853,29 @@ class MergeRequest < ApplicationRecord
 
   override :ensure_metrics
   def ensure_metrics
-    # Backward compatibility: some merge request metrics records will not have target_project_id filled in.
-    # In that case the first `safe_find_or_create_by` will return false.
-    # The second finder call will be eliminated in https://gitlab.com/gitlab-org/gitlab/-/issues/233507
-    metrics_record = MergeRequest::Metrics.safe_find_or_create_by(merge_request_id: id, target_project_id: target_project_id) || MergeRequest::Metrics.safe_find_or_create_by(merge_request_id: id)
-
-    metrics_record.tap do |metrics_record|
-      # Make sure we refresh the loaded association object with the newly created/loaded item.
-      # This is needed in order to have the exact functionality than before.
-      #
-      # Example:
-      #
-      # merge_request.metrics.destroy
-      # merge_request.ensure_metrics
-      # merge_request.metrics # should return the metrics record and not nil
-      # merge_request.metrics.merge_request # should return the same MR record
-
-      metrics_record.target_project_id = target_project_id
-      metrics_record.association(:merge_request).target = self
-      association(:metrics).target = metrics_record
+    if Feature.enabled?(:use_upsert_query_for_mr_metrics)
+      MergeRequest::Metrics.record!(self)
+    else
+      # Backward compatibility: some merge request metrics records will not have target_project_id filled in.
+      # In that case the first `safe_find_or_create_by` will return false.
+      # The second finder call will be eliminated in https://gitlab.com/gitlab-org/gitlab/-/issues/233507
+      metrics_record = MergeRequest::Metrics.safe_find_or_create_by(merge_request_id: id, target_project_id: target_project_id) || MergeRequest::Metrics.safe_find_or_create_by(merge_request_id: id)
+
+      metrics_record.tap do |metrics_record|
+        # Make sure we refresh the loaded association object with the newly created/loaded item.
+        # This is needed in order to have the exact functionality than before.
+        #
+        # Example:
+        #
+        # merge_request.metrics.destroy
+        # merge_request.ensure_metrics
+        # merge_request.metrics # should return the metrics record and not nil
+        # merge_request.metrics.merge_request # should return the same MR record
+
+        metrics_record.target_project_id = target_project_id
+        metrics_record.association(:merge_request).target = self
+        association(:metrics).target = metrics_record
+      end
     end
   end
 

app/models/merge_request/metrics.rb

@@ -14,8 +14,23 @@ class MergeRequest::Metrics < ApplicationRecord
   scope :with_valid_time_to_merge, -> { where(arel_table[:merged_at].gt(arel_table[:created_at])) }
   scope :by_target_project, ->(project) { where(target_project_id: project) }
 
-  def self.time_to_merge_expression
-    Arel.sql('EXTRACT(epoch FROM SUM(AGE(merge_request_metrics.merged_at, merge_request_metrics.created_at)))')
+  class << self
+    def time_to_merge_expression
+      Arel.sql('EXTRACT(epoch FROM SUM(AGE(merge_request_metrics.merged_at, merge_request_metrics.created_at)))')
+    end
+
+    def record!(mr)
+      sql = <<~SQL
+        INSERT INTO #{self.table_name} (merge_request_id, target_project_id, updated_at, created_at)
+        VALUES (#{mr.id}, #{mr.target_project_id}, NOW(), NOW())
+        ON CONFLICT (merge_request_id)
+        DO UPDATE SET
+        target_project_id = EXCLUDED.target_project_id,
+        updated_at = NOW()
+      SQL
+
+      connection.execute(sql)
+    end
   end
 
   private

config/feature_flags/development/use_upsert_query_for_mr_metrics.yml

+---
+name: use_upsert_query_for_mr_metrics
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/69240
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/339677
+milestone: '14.3'
+type: development
+group: group::optimize
+default_enabled: false

doc/development/testing_guide/end_to_end/rspec_metadata_tests.md

@@ -44,3 +44,4 @@ This is a partial list of the [RSpec metadata](https://relishapp.com/rspec/rspec
 | `:smtp`           | The test requires a GitLab instance to be configured to use an SMTP server. Tests SMTP notification email delivery from GitLab by using MailHog. |
 | `:testcase`       | The link to the test case issue in the [Quality Test Cases project](https://gitlab.com/gitlab-org/quality/testcases/). |
 | `:transient`      | The test tests transient bugs. It is excluded by default. |
+| `:issue`, `:issue_${num}` | Optional links to issues which might be related to the spec. Helps keeping track of related issues and can also be used by tools that create test reports. Currently added automatically to `Allure` test report. Multiple tags can be used by adding optional number postfix like `issue_1`, `issue_2` etc. |

ee/lib/analytics/merge_request_metrics_refresh.rb

@@ -8,7 +8,9 @@ module Analytics
 
     def execute(force: false)
       merge_requests.each do |mr|
-        metrics = mr.ensure_metrics
+        mr.ensure_metrics
+        mr.reset # clear already loaded (nil) metrics association
+        metrics = mr.metrics
 
         next if !force && metric_already_present?(metrics)
 

qa/Gemfile

@@ -4,7 +4,7 @@ source 'https://rubygems.org'
 
 gem 'gitlab-qa', require: 'gitlab/qa'
 gem 'activesupport', '~> 6.1.3.2' # This should stay in sync with the root's Gemfile
-gem 'allure-rspec', '~> 2.14.1'
+gem 'allure-rspec', '~> 2.14.5'
 gem 'capybara', '~> 3.35.0'
 gem 'capybara-screenshot', '~> 1.0.23'
 gem 'rake', '~> 12.3.3'

qa/Gemfile.lock

@@ -19,10 +19,10 @@ GEM
       rack-test (>= 1.1.0, < 2.0)
       rest-client (>= 2.0.2, < 3.0)
       rspec (~> 3.8)
-    allure-rspec (2.14.2)
-      allure-ruby-commons (= 2.14.2)
+    allure-rspec (2.14.5)
+      allure-ruby-commons (= 2.14.5)
       rspec-core (>= 3.8, < 4)
-    allure-ruby-commons (2.14.2)
+    allure-ruby-commons (2.14.5)
       mime-types (>= 3.3, < 4)
       oj (>= 3.10, < 4)
       require_all (>= 2, < 4)
@@ -111,7 +111,7 @@ GEM
     octokit (4.21.0)
       faraday (>= 0.9)
       sawyer (~> 0.8.0, >= 0.5.3)
-    oj (3.12.1)
+    oj (3.13.2)
     parallel (1.19.2)
     parallel_tests (2.29.0)
       parallel
@@ -212,7 +212,7 @@ PLATFORMS
 DEPENDENCIES
   activesupport (~> 6.1.3.2)
   airborne (~> 0.3.4)
-  allure-rspec (~> 2.14.1)
+  allure-rspec (~> 2.14.5)
   capybara (~> 3.35.0)
   capybara-screenshot (~> 1.0.23)
   chemlab (~> 0.7)

qa/qa/runtime/allure_report.rb

@@ -29,6 +29,13 @@ module QA
           AllureRspec.configure do |config|
             config.results_directory = 'tmp/allure-results'
             config.clean_results_directory = true
+
+            # automatically attach links to testcases and issues
+            config.tms_tag = :testcase
+            config.link_tms_pattern = '{}'
+            config.issue_tag = :issue
+            config.link_issue_pattern = '{}'
+
             config.environment_properties = environment_info if Env.running_in_ci?
 
             # Set custom environment name to separate same specs executed on different environments

qa/qa/specs/features/api/1_manage/bulk_import_group_spec.rb

@@ -71,9 +71,6 @@ module QA
         end
       end
 
-      # Non blocking issues:
-      # https://gitlab.com/gitlab-org/gitlab/-/issues/331252
-      # https://gitlab.com/gitlab-org/gitlab/-/issues/333678 <- can cause 500 when creating user and group back to back
       it(
         'imports group with subgroups and labels',
         testcase: 'https://gitlab.com/gitlab-org/quality/testcases/-/issues/1871'

qa/qa/specs/features/browser_ui/1_manage/group/bulk_import_group_spec.rb

@@ -54,10 +54,14 @@ module QA
         end
       end
 
-      # Non blocking issues:
-      # https://gitlab.com/gitlab-org/gitlab/-/issues/331252
-      # https://gitlab.com/gitlab-org/gitlab/-/issues/333678 <- can cause 500 when creating user and group back to back
-      it 'imports group from UI', testcase: 'https://gitlab.com/gitlab-org/quality/testcases/-/issues/1785' do
+      it(
+        'imports group from UI',
+        testcase: 'https://gitlab.com/gitlab-org/quality/testcases/-/issues/1785',
+        issue_1: 'https://gitlab.com/gitlab-org/gitlab/-/issues/331252',
+        issue_2: 'https://gitlab.com/gitlab-org/gitlab/-/issues/333678',
+        # mostly impacts testing as it makes small groups import slower
+        issue_3: 'https://gitlab.com/gitlab-org/gitlab/-/issues/332351'
+      ) do
         Page::Group::BulkImport.perform do |import_page|
           import_page.import_group(imported_group.path, imported_group.sandbox.path)
 

qa/qa/support/allure_metadata_formatter.rb

@@ -17,9 +17,6 @@ module QA
       def example_started(example_notification)
         example = example_notification.example
 
-        testcase = example.metadata[:testcase]
-        example.tms('Testcase', testcase) if testcase
-
         quarantine_issue = example.metadata.dig(:quarantine, :issue)
         example.issue('Quarantine issue', quarantine_issue) if quarantine_issue
 

qa/spec/support/allure_metadata_formatter_spec.rb

@@ -34,7 +34,6 @@ describe QA::Support::AllureMetadataFormatter do
     formatter.example_started(rspec_example_notification)
 
     aggregate_failures do
-      expect(rspec_example).to have_received(:tms).with('Testcase', 'testcase')
       expect(rspec_example).to have_received(:issue).with('Quarantine issue', 'issue')
       expect(rspec_example).to have_received(:add_link).with(name: "Job(#{ci_job})", url: ci_job_url)
       expect(rspec_example).to have_received(:issue).with(