diff --git a/.gitlab/ci/rails.gitlab-ci.yml b/.gitlab/ci/rails.gitlab-ci.yml index a2fb4231abed74aafeb4ba902f2736a9cdb6183e..1e92858c2b650f09667c3b68bbdbcceaecd216f0 100644 --- a/.gitlab/ci/rails.gitlab-ci.yml +++ b/.gitlab/ci/rails.gitlab-ci.yml @@ -322,11 +322,6 @@ rspec fast_spec_helper: # Load fast_spec_helper as well just in case there are no specs available. - rspec_section bin/rspec --dry-run spec/fast_spec_helper.rb ${fast_spec_helper_specs} -rspec tooling: - extends: - - .rspec-base-pg14 - - .rails:rules:ee-and-foss-tooling - rspec unit clickhouse: extends: - .rspec-base-pg14-clickhouse23 @@ -411,14 +406,6 @@ rspec:artifact-collector remainder: - job: rspec background_migration pg14 # 5 jobs optional: true -rspec:artifact-collector tooling: - extends: - - .artifact-collector - - .rails:rules:artifact-collector-tooling - needs: - - job: rspec tooling # 1 job - optional: true - rspec:artifact-collector unit single-redis: extends: - .artifact-collector @@ -506,8 +493,6 @@ rspec:coverage: optional: true - job: rspec:artifact-collector remainder single-redis optional: true - - job: rspec:artifact-collector tooling - optional: true # as-if-foss jobs - job: rspec:artifact-collector as-if-foss optional: true diff --git a/.gitlab/ci/rails/rspec-foss-impact.gitlab-ci.yml.erb b/.gitlab/ci/rails/rspec-foss-impact.gitlab-ci.yml.erb index 1fc7dc283833e9ee210a1864308f866793862416..e9c66a49f72984fe7b7082e3c9c15f709eadd255 100644 --- a/.gitlab/ci/rails/rspec-foss-impact.gitlab-ci.yml.erb +++ b/.gitlab/ci/rails/rspec-foss-impact.gitlab-ci.yml.erb @@ -87,11 +87,3 @@ rspec system foss-impact: parallel: <%= rspec_files_per_test_level[:system][:parallelization] %> <% end %> <% end %> - -<% if rspec_files_per_test_level[:tooling][:files].size > 0 %> -rspec tooling foss-impact: - extends: .base-rspec-foss-impact -<% if rspec_files_per_test_level[:tooling][:parallelization] > 1 %> - parallel: <%= rspec_files_per_test_level[:tooling][:parallelization] %> -<% end %> -<% end %> diff --git a/.gitlab/ci/rails/rspec-predictive.gitlab-ci.yml.erb b/.gitlab/ci/rails/rspec-predictive.gitlab-ci.yml.erb index b47e7f59163a097d8781761f2550a7e7c836cc09..90f1b6fc5a8e1d258bcf84c84b98ae3635212070 100644 --- a/.gitlab/ci/rails/rspec-predictive.gitlab-ci.yml.erb +++ b/.gitlab/ci/rails/rspec-predictive.gitlab-ci.yml.erb @@ -97,15 +97,6 @@ rspec system predictive: <% end %> <% end %> -<% if rspec_files_per_test_level.dig(:tooling, :files).size > 0 %> -rspec tooling predictive: - extends: - - .base-rspec-predictive -<% if rspec_files_per_test_level.dig(:tooling, :parallelization) > 1 %> - parallel: <%= rspec_files_per_test_level.dig(:tooling, :parallelization) %> -<% end %> -<% end %> - <% end %> <% if test_suite_prefix == 'ee/' %> @@ -163,13 +154,4 @@ rspec-ee system predictive: <% end %> <% end %> -<% if rspec_files_per_test_level.dig(:tooling, :files).size > 0 %> -rspec-ee tooling predictive: - extends: - - .base-rspec-ee-predictive -<% if rspec_files_per_test_level.dig(:tooling, :parallelization) > 1 %> - parallel: <%= rspec_files_per_test_level.dig(:tooling, :parallelization) %> -<% end %> -<% end %> - <% end %> diff --git a/.gitlab/ci/rules.gitlab-ci.yml b/.gitlab/ci/rules.gitlab-ci.yml index 7e48a1a6395ad9d98ec0eb524b8440597747044f..2944dd8afc853f475403ed32da2e2a16dffee9ce 100644 --- a/.gitlab/ci/rules.gitlab-ci.yml +++ b/.gitlab/ci/rules.gitlab-ci.yml @@ -603,6 +603,11 @@ - ".gitlab/ci/static-analysis.gitlab-ci.yml" - "config/feature_categories.yml" # Used by RSpec/FeatureCategory +.danger-patterns: &danger-patterns + - "Dangerfile" + - "danger/**/*" + - "tooling/danger/**/*" + .core-backend-patterns: &core-backend-patterns - "{,jh/}Gemfile{,.lock}" - "{,ee/,jh/}config/**/*.rb" @@ -2053,13 +2058,6 @@ - <<: *if-default-refs changes: *core-backend-patterns -.rails:rules:ee-and-foss-tooling: - rules: - - <<: *if-fork-merge-request - when: never - - <<: *if-merge-request - - <<: *if-default-refs - .rails:rules:code-backstage-qa: rules: - <<: *if-default-refs @@ -2201,11 +2199,6 @@ - !reference ['.rails:rules:ee-and-foss-migration', rules] - !reference ['.rails:rules:ee-and-foss-background-migration', rules] -.rails:rules:artifact-collector-tooling: - rules: - - if: '$START_AS_IF_FOSS == "true"' - - !reference ['.rails:rules:ee-and-foss-tooling', rules] - .rails:rules:detect-tests: rules: - <<: *if-merge-request-labels-run-all-rspec @@ -2729,6 +2722,11 @@ rules: - <<: *if-merge-request +.review:rules:danger-local: + rules: + - <<: *if-merge-request + changes: *danger-patterns + ############### # Setup rules # ############### diff --git a/.rubocop_todo/layout/line_length.yml b/.rubocop_todo/layout/line_length.yml index eda7a655fc2f9d0844560e1b4998986506c00cd2..a028ecf14e11d828b3c5dcf53d1f563132dfa315 100644 --- a/.rubocop_todo/layout/line_length.yml +++ b/.rubocop_todo/layout/line_length.yml @@ -4806,3 +4806,4 @@ Layout/LineLength: - 'tooling/lib/tooling/helm3_client.rb' - 'tooling/lib/tooling/kubernetes_client.rb' - 'tooling/merge_request_rspec_failure_rake_task.rb' + - 'tooling/quality/test_level.rb' diff --git a/.rubocop_todo/lint/redundant_cop_disable_directive.yml b/.rubocop_todo/lint/redundant_cop_disable_directive.yml index 88299a342bd20cfbcf01d368e7ee519b9926473b..020690ef61d4f1d8a36dfbb3867ff9d17a1dc1d7 100644 --- a/.rubocop_todo/lint/redundant_cop_disable_directive.yml +++ b/.rubocop_todo/lint/redundant_cop_disable_directive.yml @@ -287,3 +287,4 @@ Lint/RedundantCopDisableDirective: - 'tooling/danger/suggestor.rb' - 'tooling/lib/tooling/helm3_client.rb' - 'tooling/lib/tooling/kubernetes_client.rb' + - 'tooling/quality/test_level.rb' diff --git a/.rubocop_todo/style/inline_disable_annotation.yml b/.rubocop_todo/style/inline_disable_annotation.yml index c64e2aa84d5494c3af259b7d2ef79abd2a19408d..8fc93cd84b096f89d4e7acfa6c03ef115cbea1e8 100644 --- a/.rubocop_todo/style/inline_disable_annotation.yml +++ b/.rubocop_todo/style/inline_disable_annotation.yml @@ -3150,3 +3150,4 @@ Style/InlineDisableAnnotation: - 'tooling/lib/tooling/helm3_client.rb' - 'tooling/lib/tooling/job_metrics.rb' - 'tooling/lib/tooling/kubernetes_client.rb' + - 'tooling/quality/test_level.rb' diff --git a/scripts/generate_rspec_pipeline.rb b/scripts/generate_rspec_pipeline.rb index d4f6253f15a717ad4e12b8c5466fa4ec43c199e7..70a0c87be35bd5bfbcbbe3dfabb5e162d5da36b4 100755 --- a/scripts/generate_rspec_pipeline.rb +++ b/scripts/generate_rspec_pipeline.rb @@ -11,7 +11,7 @@ # Class to generate RSpec test child pipeline with dynamically parallelized jobs. class GenerateRspecPipeline SKIP_PIPELINE_YML_FILE = ".gitlab/ci/_skip.yml" - TEST_LEVELS = %i[migration background_migration unit integration system tooling].freeze + TEST_LEVELS = %i[migration background_migration unit integration system].freeze MAX_NODES_COUNT = 50 # Maximum parallelization allowed by GitLab OPTIMAL_TEST_JOB_DURATION_IN_SECONDS = 600 # 10 MINUTES diff --git a/spec/tooling/quality/test_level_spec.rb b/spec/tooling/quality/test_level_spec.rb index ede375f729876702b41f3d83b8d18c554b415e50..dfbe5ce283ba5e2034be415bb42551d87962ade2 100644 --- a/spec/tooling/quality/test_level_spec.rb +++ b/spec/tooling/quality/test_level_spec.rb @@ -5,8 +5,6 @@ require_relative '../../../tooling/quality/test_level' RSpec.describe Quality::TestLevel, feature_category: :tooling do - subject(:test_level) { described_class.new } - describe 'TEST_LEVEL_FOLDERS constant' do it 'ensures all directories it refers to exists', :aggregate_failures do ee_only_directories = %w[ @@ -33,56 +31,49 @@ describe '#pattern' do context 'when level is all' do it 'returns a pattern' do - expect(test_level.pattern(:all)) + expect(subject.pattern(:all)) .to eq("spec/**{,/**/}*_spec.rb") end end context 'when level is frontend_fixture' do it 'returns a pattern' do - expect(test_level.pattern(:frontend_fixture)) + expect(subject.pattern(:frontend_fixture)) .to eq("spec/{frontend/fixtures}{,/**/}*.rb") end end context 'when level is unit' do it 'returns a pattern' do - expect(test_level.pattern(:unit)) - .to eq("spec/{bin,channels,components,config,contracts,db,dependencies,elastic,elastic_integration,experiments,factories,finders,frontend,graphql,haml_lint,helpers,initializers,keeps,lib,metrics_server,models,policies,presenters,rack_servers,replicators,routing,rubocop,scripts,serializers,services,sidekiq,sidekiq_cluster,spam,support_specs,tasks,uploaders,validators,views,workers}{,/**/}*_spec.rb") - end - end - - context 'when level is tooling' do - it 'returns a pattern' do - expect(test_level.pattern(:tooling)) - .to eq("spec/{dot_gitlab_ci,tooling}{,/**/}*_spec.rb") + expect(subject.pattern(:unit)) + .to eq("spec/{bin,channels,components,config,contracts,db,dependencies,elastic,elastic_integration,experiments,factories,finders,frontend,graphql,haml_lint,helpers,initializers,keeps,lib,metrics_server,models,policies,presenters,rack_servers,replicators,routing,rubocop,scripts,serializers,services,sidekiq,sidekiq_cluster,spam,support_specs,tasks,uploaders,validators,views,workers,tooling,dot_gitlab_ci}{,/**/}*_spec.rb") end end context 'when level is migration' do it 'returns a pattern' do - expect(test_level.pattern(:migration)) + expect(subject.pattern(:migration)) .to eq("spec/{migrations}{,/**/}*_spec.rb") end end context 'when level is background_migration' do it 'returns a pattern' do - expect(test_level.pattern(:background_migration)) + expect(subject.pattern(:background_migration)) .to eq("spec/{lib/gitlab/background_migration,lib/ee/gitlab/background_migration}{,/**/}*_spec.rb") end end context 'when level is integration' do it 'returns a pattern' do - expect(test_level.pattern(:integration)) + expect(subject.pattern(:integration)) .to eq("spec/{commands,controllers,mailers,requests}{,/**/}*_spec.rb") end end context 'when level is system' do it 'returns a pattern' do - expect(test_level.pattern(:system)) + expect(subject.pattern(:system)) .to eq("spec/{features}{,/**/}*_spec.rb") end end @@ -103,11 +94,11 @@ describe 'performance' do it 'memoizes the pattern for a given level' do - expect(test_level.pattern(:system).object_id).to eq(test_level.pattern(:system).object_id) + expect(subject.pattern(:system).object_id).to eq(subject.pattern(:system).object_id) end it 'freezes the pattern for a given level' do - expect(test_level.pattern(:system)).to be_frozen + expect(subject.pattern(:system)).to be_frozen end end end @@ -115,56 +106,49 @@ describe '#regexp' do context 'when level is all' do it 'returns a regexp' do - expect(test_level.regexp(:all)) + expect(subject.regexp(:all)) .to eq(%r{spec/}) end end context 'when level is frontend_fixture' do it 'returns a regexp' do - expect(test_level.regexp(:frontend_fixture)) + expect(subject.regexp(:frontend_fixture)) .to eq(%r{spec/(frontend/fixtures)/}) end end context 'when level is unit' do it 'returns a regexp' do - expect(test_level.regexp(:unit)) - .to eq(%r{spec/(bin|channels|components|config|contracts|db|dependencies|elastic|elastic_integration|experiments|factories|finders|frontend|graphql|haml_lint|helpers|initializers|keeps|lib|metrics_server|models|policies|presenters|rack_servers|replicators|routing|rubocop|scripts|serializers|services|sidekiq|sidekiq_cluster|spam|support_specs|tasks|uploaders|validators|views|workers)/}) - end - end - - context 'when level is tooling' do - it 'returns a regexp' do - expect(test_level.regexp(:tooling)) - .to eq(%r{spec/(dot_gitlab_ci|tooling)/}) + expect(subject.regexp(:unit)) + .to eq(%r{spec/(bin|channels|components|config|contracts|db|dependencies|elastic|elastic_integration|experiments|factories|finders|frontend|graphql|haml_lint|helpers|initializers|keeps|lib|metrics_server|models|policies|presenters|rack_servers|replicators|routing|rubocop|scripts|serializers|services|sidekiq|sidekiq_cluster|spam|support_specs|tasks|uploaders|validators|views|workers|tooling|dot_gitlab_ci)/}) end end context 'when level is migration' do it 'returns a regexp' do - expect(test_level.regexp(:migration)) + expect(subject.regexp(:migration)) .to eq(%r{spec/(migrations)/}) end end context 'when level is background_migration' do it 'returns a regexp' do - expect(test_level.regexp(:background_migration)) + expect(subject.regexp(:background_migration)) .to eq(%r{spec/(lib/gitlab/background_migration|lib/ee/gitlab/background_migration)/}) end end context 'when level is integration' do it 'returns a regexp' do - expect(test_level.regexp(:integration)) + expect(subject.regexp(:integration)) .to eq(%r{spec/(commands|controllers|mailers|requests)/}) end end context 'when level is system' do it 'returns a regexp' do - expect(test_level.regexp(:system)) + expect(subject.regexp(:system)) .to eq(%r{spec/(features)/}) end end @@ -192,39 +176,38 @@ describe 'performance' do it 'memoizes the regexp for a given level' do - expect(test_level.regexp(:system).object_id).to eq(test_level.regexp(:system).object_id) + expect(subject.regexp(:system).object_id).to eq(subject.regexp(:system).object_id) end it 'freezes the regexp for a given level' do - expect(test_level.regexp(:system)).to be_frozen + expect(subject.regexp(:system)).to be_frozen end end end describe '#level_for' do it 'returns the correct level for a unit test' do - expect(test_level.level_for('spec/models/abuse_report_spec.rb')).to eq(:unit) + expect(subject.level_for('spec/models/abuse_report_spec.rb')).to eq(:unit) end it 'returns the correct level for a frontend fixture test' do - expect(test_level.level_for('spec/frontend/fixtures/pipelines.rb')).to eq(:frontend_fixture) + expect(subject.level_for('spec/frontend/fixtures/pipelines.rb')).to eq(:frontend_fixture) end - it 'returns the correct level for a tooling test', :aggregate_failures do - expect(test_level.level_for('spec/dot_gitlab_ci/rules_spec.rb')).to eq(:tooling) - expect(test_level.level_for('spec/tooling/lib/tooling/test_file_finder_spec.rb')).to eq(:tooling) + it 'returns the correct level for a tooling test' do + expect(subject.level_for('spec/tooling/lib/tooling/test_file_finder_spec.rb')).to eq(:unit) end it 'returns the correct level for a migration test' do - expect(test_level.level_for('spec/migrations/add_default_and_free_plans_spec.rb')).to eq(:migration) + expect(subject.level_for('spec/migrations/add_default_and_free_plans_spec.rb')).to eq(:migration) end it 'returns the correct level for a background migration test' do - expect(test_level.level_for('spec/lib/gitlab/background_migration/archive_legacy_traces_spec.rb')).to eq(:background_migration) + expect(subject.level_for('spec/lib/gitlab/background_migration/archive_legacy_traces_spec.rb')).to eq(:background_migration) end it 'returns the correct level for an EE file without passing a prefix' do - expect(test_level.level_for('ee/spec/migrations/geo/migrate_ci_job_artifacts_to_separate_registry_spec.rb')).to eq(:migration) + expect(subject.level_for('ee/spec/migrations/geo/migrate_ci_job_artifacts_to_separate_registry_spec.rb')).to eq(:migration) end it 'returns the correct level for a geo migration test' do @@ -236,35 +219,32 @@ end it 'returns the correct level for an integration test' do - expect(test_level.level_for('spec/mailers/abuse_report_mailer_spec.rb')).to eq(:integration) + expect(subject.level_for('spec/mailers/abuse_report_mailer_spec.rb')).to eq(:integration) end it 'returns the correct level for an integration test in a subfolder' do - expect(test_level.level_for('spec/commands/sidekiq_cluster/cli.rb')).to eq(:integration) + expect(subject.level_for('spec/commands/sidekiq_cluster/cli.rb')).to eq(:integration) end it 'returns the correct level for a system test' do - expect(test_level.level_for('spec/features/abuse_report_spec.rb')).to eq(:system) + expect(subject.level_for('spec/features/abuse_report_spec.rb')).to eq(:system) end it 'returns the correct level for a keep test' do - expect(test_level.level_for('spec/keeps/helpers/postgres_ai_spec.rb')).to eq(:unit) + expect(subject.level_for('spec/keeps/helpers/postgres_ai_spec.rb')).to eq(:unit) end it 'raises an error for an unknown level' do - expect { test_level.level_for('spec/unknown/foo_spec.rb') } - .to raise_error(described_class::UnknownTestLevelError, <<~MESSAGE) - Test level for spec/unknown/foo_spec.rb couldn't be set. - - Please rename the file properly or change the test level detection regexes in tooling/quality/test_level.rb. - MESSAGE + expect { subject.level_for('spec/unknown/foo_spec.rb') } + .to raise_error(described_class::UnknownTestLevelError, + %r{Test level for spec/unknown/foo_spec.rb couldn't be set. Please rename the file properly or change the test level detection regexes in .+/tooling/quality/test_level.rb.}) end it 'ensures all spec/ folders are covered by a test level' do Dir['{,ee/}spec/**/*/'].each do |path| next if %r{\A(ee/)?spec/(benchmarks|docs_screenshots|fixtures|frontend_integration|support)/}.match?(path) - expect { test_level.level_for(path) }.not_to raise_error + expect { subject.level_for(path) }.not_to raise_error end end end diff --git a/tooling/quality/test_level.rb b/tooling/quality/test_level.rb index 62f3dc124c485d5c73fa98a8af774f741a823225..f1a76692e3f9347b76f80134b37dcbd11470149c 100644 --- a/tooling/quality/test_level.rb +++ b/tooling/quality/test_level.rb @@ -55,11 +55,9 @@ class TestLevel validators views workers - ], - tooling: %w[ - dot_gitlab_ci tooling - ], + dot_gitlab_ci + ], # ^ tooling and dot_gitlab_ci might be worth to move to another level integration: %w[ commands controllers @@ -78,7 +76,7 @@ def initialize(prefixes = nil) end def pattern(level) - @patterns[level] ||= "#{prefixes_for_pattern}spec/#{folders_pattern(level)}{,/**/}*#{suffix(level)}".freeze + @patterns[level] ||= "#{prefixes_for_pattern}spec/#{folders_pattern(level)}{,/**/}*#{suffix(level)}".freeze # rubocop:disable Style/RedundantFreeze end def regexp(level, start_with = false) @@ -99,20 +97,12 @@ def level_for(file_path) :frontend_fixture when regexp(:unit) :unit - when regexp(:tooling) - :tooling when regexp(:integration) :integration when regexp(:system) :system else - file = Pathname.new(__FILE__).relative_path_from(File.expand_path("../..", __dir__)).to_s - - raise UnknownTestLevelError, <<~MESSAGE - Test level for #{file_path} couldn't be set. - - Please rename the file properly or change the test level detection regexes in #{file}. - MESSAGE + raise UnknownTestLevelError, "Test level for #{file_path} couldn't be set. Please rename the file properly or change the test level detection regexes in #{__FILE__}." end end