Merge branch 'fix-build-group-name' into 'master'

Refactor Ci::Build#group_name [RUN ALL RSPEC] [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!52644

app/models/commit_status.rb

@@ -209,6 +209,17 @@ class CommitStatus < ApplicationRecord
   end
 
   def group_name
+    simplified_commit_status_group_name_feature_flag = Gitlab::SafeRequestStore.fetch("project:#{project_id}:simplified_commit_status_group_name") do
+      Feature.enabled?(:simplified_commit_status_group_name, project, default_enabled: false)
+    end
+
+    if simplified_commit_status_group_name_feature_flag
+      # Only remove one or more [...] "X/Y" "X Y" from the end of build names.
+      # More about the regular expression logic: https://docs.gitlab.com/ee/ci/jobs/#group-jobs-in-a-pipeline
+
+      name.to_s.sub(%r{([\b\s:]+((\[.*\])|(\d+[\s:\/\\]+\d+)))+\s*\z}, '').strip
+    else
+      # Prior implementation, remove [...] "X/Y" "X Y" from the beginning and middle of build names
       # 'rspec:linux: 1/10' => 'rspec:linux'
       common_name = name.to_s.gsub(%r{\b\d+[\s:\/\\]+\d+\s*}, '')
 
@@ -218,6 +229,7 @@ class CommitStatus < ApplicationRecord
       common_name.strip!
       common_name
     end
+  end
 
   def failed_but_allowed?
     allow_failure? && (failed? || canceled?)

config/feature_flags/development/simplified_commit_status_group_name.yml

+---
+name: simplified_commit_status_group_name
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/52644
+rollout_issue_url:
+milestone: '13.9'
+type: development
+group: group::testing
+default_enabled: false

doc/ci/jobs/index.md

@@ -139,6 +139,23 @@ usually want the first number to be the index and the second number to be the to
 [This regular expression](https://gitlab.com/gitlab-org/gitlab/blob/2f3dc314f42dbd79813e6251792853bc231e69dd/app/models/commit_status.rb#L99)
 evaluates the job names: `\d+[\s:\/\\]+\d+\s*`.
 
+### Improved job grouping
+
+> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/52644) in GitLab 13.9.
+> - It's [deployed behind a feature flag](../../user/feature_flags.md), disabled by default.
+> - It's enabled on GitLab.com.
+> - To use it in GitLab self-managed instances, ask a GitLab administrator to [enable it](../../administration/feature_flags.md). **(FREE SELF)**
+
+Job grouping is evaluated with an improved regular expression to group jobs by name:
+
+- `([\b\s:]+((\[.*\])|(\d+[\s:\/\\]+\d+)))+\s*\z`.
+
+The new implementation removes one or more `: [...]`, `X Y`, `X/Y`, or `X\Y` sequences
+from the **end** of job names only.
+
+Matching substrings occuring at the beginning or in the middle of build names are
+no longer removed.
+
 ## Specifying variables when running manual jobs
 
 > [Introduced](https://gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/30485) in GitLab 12.2.

spec/models/commit_status_spec.rb

@@ -510,6 +510,10 @@ RSpec.describe CommitStatus do
   end
 
   describe '#group_name' do
+    before do
+      stub_feature_flags(simplified_commit_status_group_name: false)
+    end
+
     using RSpec::Parameterized::TableSyntax
 
     let(:commit_status) do
@@ -557,6 +561,58 @@ RSpec.describe CommitStatus do
         is_expected.to eq(group_name)
       end
     end
+
+    context 'with simplified_commit_status_group_name' do
+      before do
+        stub_feature_flags(simplified_commit_status_group_name: true)
+      end
+
+      where(:name, :group_name) do
+        'rspec1'                                              | 'rspec1'
+        'rspec1 0 1'                                          | 'rspec1'
+        'rspec1 0/2'                                          | 'rspec1'
+        'rspec:windows'                                       | 'rspec:windows'
+        'rspec:windows 0'                                     | 'rspec:windows 0'
+        'rspec:windows 0 2/2'                                 | 'rspec:windows 0'
+        'rspec:windows 0 test'                                | 'rspec:windows 0 test'
+        'rspec:windows 0 test 2/2'                            | 'rspec:windows 0 test'
+        'rspec:windows 0 1 2/2'                               | 'rspec:windows'
+        'rspec:windows 0 1 [aws] 2/2'                         | 'rspec:windows'
+        'rspec:windows 0 1 name [aws] 2/2'                    | 'rspec:windows 0 1 name'
+        'rspec:windows 0 1 name'                              | 'rspec:windows 0 1 name'
+        'rspec:windows 0 1 name 1/2'                          | 'rspec:windows 0 1 name'
+        'rspec:windows 0/1'                                   | 'rspec:windows'
+        'rspec:windows 0/1 name'                              | 'rspec:windows 0/1 name'
+        'rspec:windows 0/1 name 1/2'                          | 'rspec:windows 0/1 name'
+        'rspec:windows 0:1'                                   | 'rspec:windows'
+        'rspec:windows 0:1 name'                              | 'rspec:windows 0:1 name'
+        'rspec:windows 10000 20000'                           | 'rspec:windows'
+        'rspec:windows 0 : / 1'                               | 'rspec:windows'
+        'rspec:windows 0 : / 1 name'                          | 'rspec:windows 0 : / 1 name'
+        '0 1 name ruby'                                       | '0 1 name ruby'
+        '0 :/ 1 name ruby'                                    | '0 :/ 1 name ruby'
+        'rspec: [aws]'                                        | 'rspec'
+        'rspec: [aws] 0/1'                                    | 'rspec'
+        'rspec: [aws, max memory]'                            | 'rspec'
+        'rspec:linux: [aws, max memory, data]'                | 'rspec:linux'
+        'rspec: [inception: [something, other thing], value]' | 'rspec'
+        'rspec:windows 0/1: [name, other]'                    | 'rspec:windows'
+        'rspec:windows: [name, other] 0/1'                    | 'rspec:windows'
+        'rspec:windows: [name, 0/1] 0/1'                      | 'rspec:windows'
+        'rspec:windows: [0/1, name]'                          | 'rspec:windows'
+        'rspec:windows: [, ]'                                 | 'rspec:windows'
+        'rspec:windows: [name]'                               | 'rspec:windows'
+        'rspec:windows: [name,other]'                         | 'rspec:windows'
+      end
+
+      with_them do
+        it "#{params[:name]} puts in #{params[:group_name]}" do
+          commit_status.name = name
+
+          is_expected.to eq(group_name)
+        end
+      end
+    end
   end
 
   describe '#detailed_status' do

spec/serializers/ci/dag_pipeline_entity_spec.rb

@@ -73,11 +73,11 @@ RSpec.describe Ci::DagPipelineEntity do
         end
       end
 
-      it 'performs the smallest number of queries' do
+      it 'performs the smallest number of queries', :request_store do
         log = ActiveRecord::QueryRecorder.new { subject }
 
-        # stages, project, builds, build_needs
-        expect(log.count).to eq 4
+        # stages, project, builds, build_needs, feature_flag
+        expect(log.count).to eq 5
       end
 
       it 'contains all the data' do

spec/services/ci/daily_build_group_report_result_service_spec.rb

@@ -5,9 +5,10 @@ require 'spec_helper'
 RSpec.describe Ci::DailyBuildGroupReportResultService, '#execute' do
   let_it_be(:group) { create(:group, :private) }
   let_it_be(:pipeline) { create(:ci_pipeline, project: create(:project, group: group), created_at: '2020-02-06 00:01:10') }
-  let_it_be(:rspec_job) { create(:ci_build, pipeline: pipeline, name: '3/3 rspec', coverage: 80) }
-  let_it_be(:karma_job) { create(:ci_build, pipeline: pipeline, name: '2/2 karma', coverage: 90) }
+  let_it_be(:rspec_job) { create(:ci_build, pipeline: pipeline, name: 'rspec 3/3', coverage: 80) }
+  let_it_be(:karma_job) { create(:ci_build, pipeline: pipeline, name: 'karma 2/2', coverage: 90) }
   let_it_be(:extra_job) { create(:ci_build, pipeline: pipeline, name: 'extra', coverage: nil) }
+
   let(:coverages) { Ci::DailyBuildGroupReportResult.all }
 
   it 'creates daily code coverage record for each job in the pipeline that has coverage value' do
@@ -41,8 +42,8 @@ RSpec.describe Ci::DailyBuildGroupReportResultService, '#execute' do
   end
 
   context 'when there are multiple builds with the same group name that report coverage' do
-    let!(:test_job_1) { create(:ci_build, pipeline: pipeline, name: '1/2 test', coverage: 70) }
-    let!(:test_job_2) { create(:ci_build, pipeline: pipeline, name: '2/2 test', coverage: 80) }
+    let!(:test_job_1) { create(:ci_build, pipeline: pipeline, name: 'test 1/2', coverage: 70) }
+    let!(:test_job_2) { create(:ci_build, pipeline: pipeline, name: 'test 2/2', coverage: 80) }
 
     it 'creates daily code coverage record with the average as the value' do
       described_class.new.execute(pipeline)
@@ -70,8 +71,8 @@ RSpec.describe Ci::DailyBuildGroupReportResultService, '#execute' do
       )
     end
 
-    let!(:new_rspec_job) { create(:ci_build, pipeline: new_pipeline, name: '4/4 rspec', coverage: 84) }
-    let!(:new_karma_job) { create(:ci_build, pipeline: new_pipeline, name: '3/3 karma', coverage: 92) }
+    let!(:new_rspec_job) { create(:ci_build, pipeline: new_pipeline, name: 'rspec 4/4', coverage: 84) }
+    let!(:new_karma_job) { create(:ci_build, pipeline: new_pipeline, name: 'karma 3/3', coverage: 92) }
 
     before do
       # Create the existing daily code coverage records
@@ -110,8 +111,8 @@ RSpec.describe Ci::DailyBuildGroupReportResultService, '#execute' do
       )
     end
 
-    let!(:new_rspec_job) { create(:ci_build, pipeline: new_pipeline, name: '4/4 rspec', coverage: 84) }
-    let!(:new_karma_job) { create(:ci_build, pipeline: new_pipeline, name: '3/3 karma', coverage: 92) }
+    let!(:new_rspec_job) { create(:ci_build, pipeline: new_pipeline, name: 'rspec 4/4', coverage: 84) }
+    let!(:new_karma_job) { create(:ci_build, pipeline: new_pipeline, name: 'karma 3/3', coverage: 92) }
 
     before do
       # Create the existing daily code coverage records