diff options
29 files changed, 357 insertions, 230 deletions
diff --git a/app/assets/javascripts/pages/projects/blame/show/index.js b/app/assets/javascripts/pages/projects/blame/show/index.js index f0fdd18c828..1e4b9de90f2 100644 --- a/app/assets/javascripts/pages/projects/blame/show/index.js +++ b/app/assets/javascripts/pages/projects/blame/show/index.js @@ -1,10 +1,5 @@ import initBlob from '~/pages/projects/init_blob'; import redirectToCorrectPage from '~/blame/blame_redirect'; -import { renderBlamePageStreams } from '~/blame/streaming'; -if (new URLSearchParams(window.location.search).get('streaming')) { - renderBlamePageStreams(window.blamePageStream); -} else { - redirectToCorrectPage(); -} +redirectToCorrectPage(); initBlob(); diff --git a/app/assets/javascripts/pages/projects/blame/streaming/index.js b/app/assets/javascripts/pages/projects/blame/streaming/index.js new file mode 100644 index 00000000000..82cc09c1e76 --- /dev/null +++ b/app/assets/javascripts/pages/projects/blame/streaming/index.js @@ -0,0 +1,5 @@ +import initBlob from '~/pages/projects/init_blob'; +import { renderBlamePageStreams } from '~/blame/streaming'; + +renderBlamePageStreams(window.blamePageStream); +initBlob(); diff --git a/app/controllers/projects/blame_controller.rb b/app/controllers/projects/blame_controller.rb index dbc82f5b314..bb1b8760c42 100644 --- a/app/controllers/projects/blame_controller.rb +++ b/app/controllers/projects/blame_controller.rb @@ -18,6 +18,11 @@ class Projects::BlameController < Projects::ApplicationController load_blame end + def streaming + show + render action: 'show' + end + def page load_environment load_blame diff --git a/app/helpers/blame_helper.rb b/app/helpers/blame_helper.rb index 4eda89e2af2..f25917407a7 100644 --- a/app/helpers/blame_helper.rb +++ b/app/helpers/blame_helper.rb @@ -45,8 +45,10 @@ module BlameHelper end def entire_blame_path(id, project, blame_mode) - params = blame_mode.streaming_supported? ? { streaming: true } : { no_pagination: true } - - namespace_project_blame_path(namespace_id: project.namespace, project_id: project, id: id, **params) + if blame_mode.streaming_supported? + namespace_project_blame_streaming_path(namespace_id: project.namespace, project_id: project, id: id) + else + namespace_project_blame_path(namespace_id: project.namespace, project_id: project, id: id, no_pagination: true) + end end end diff --git a/app/workers/namespaces/root_statistics_worker.rb b/app/workers/namespaces/root_statistics_worker.rb index c8527182d35..02b3468c052 100644 --- a/app/workers/namespaces/root_statistics_worker.rb +++ b/app/workers/namespaces/root_statistics_worker.rb @@ -16,23 +16,15 @@ module Namespaces def perform(namespace_id) namespace = Namespace.find(namespace_id) - if Feature.enabled?(:remove_aggregation_schedule_lease, namespace) - Namespaces::StatisticsRefresherService.new.execute(namespace) - else - refresh_through_namespace_aggregation_schedule(namespace) - end - - notify_storage_usage(namespace) - rescue ::Namespaces::StatisticsRefresherService::RefresherError, ActiveRecord::RecordNotFound => ex - Gitlab::ErrorTracking.track_exception(ex, namespace_id: namespace_id, namespace: namespace&.full_path) - end - - def refresh_through_namespace_aggregation_schedule(namespace) return unless namespace.aggregation_scheduled? Namespaces::StatisticsRefresherService.new.execute(namespace) namespace.aggregation_schedule.destroy + + notify_storage_usage(namespace) + rescue ::Namespaces::StatisticsRefresherService::RefresherError, ActiveRecord::RecordNotFound => ex + Gitlab::ErrorTracking.track_exception(ex, namespace_id: namespace_id, namespace: namespace&.full_path) end private diff --git a/app/workers/namespaces/schedule_aggregation_worker.rb b/app/workers/namespaces/schedule_aggregation_worker.rb index bf48eb8180e..7cd7f5223d6 100644 --- a/app/workers/namespaces/schedule_aggregation_worker.rb +++ b/app/workers/namespaces/schedule_aggregation_worker.rb @@ -13,24 +13,16 @@ module Namespaces idempotent! def perform(namespace_id) + return unless aggregation_schedules_table_exists? + namespace = Namespace.find(namespace_id) root_ancestor = namespace.root_ancestor - if Feature.enabled?(:remove_aggregation_schedule_lease, root_ancestor) - Namespaces::RootStatisticsWorker.perform_async(root_ancestor.id) - else - schedule_through_aggregation_schedules_table(root_ancestor) - end - rescue ActiveRecord::RecordNotFound => ex - Gitlab::ErrorTracking.track_exception(ex, namespace_id: namespace_id) - end - - def schedule_through_aggregation_schedules_table(root_ancestor) - return unless aggregation_schedules_table_exists? - return if root_ancestor.aggregation_scheduled? Namespace::AggregationSchedule.safe_find_or_create_by!(namespace_id: root_ancestor.id) + rescue ActiveRecord::RecordNotFound => ex + Gitlab::ErrorTracking.track_exception(ex, namespace_id: namespace_id) end private diff --git a/config/feature_flags/development/remove_aggregation_schedule_lease.yml b/config/feature_flags/development/remove_aggregation_schedule_lease.yml deleted file mode 100644 index 77367be3200..00000000000 --- a/config/feature_flags/development/remove_aggregation_schedule_lease.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: remove_aggregation_schedule_lease -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/113959 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/395018 -milestone: '15.10' -type: development -group: group::utilization -default_enabled: false diff --git a/config/routes/repository.rb b/config/routes/repository.rb index 60d3d37bdc8..08aa113685a 100644 --- a/config/routes/repository.rb +++ b/config/routes/repository.rb @@ -76,6 +76,7 @@ scope format: false do get '/tree/*id', to: 'tree#show', as: :tree get '/raw/*id', to: 'raw#show', as: :raw get '/blame_page/*id', to: 'blame#page', as: :blame_page + get '/blame/*id/streaming', to: 'blame#streaming', as: :blame_streaming, defaults: { streaming: true } get '/blame/*id', to: 'blame#show', as: :blame get '/commits', to: 'commits#commits_root', as: :commits_root diff --git a/db/docs/ci_minutes_additional_packs.yml b/db/docs/ci_minutes_additional_packs.yml index be4b0a19621..c449e8c22e7 100644 --- a/db/docs/ci_minutes_additional_packs.yml +++ b/db/docs/ci_minutes_additional_packs.yml @@ -4,7 +4,8 @@ classes: - Ci::Minutes::AdditionalPack feature_categories: - purchase -description: TODO +- consumables_cost_management +description: Stores CI minutes purchases for a given namespace with fields for synchronizing and expiring available minutes between Customers Portal and GitLab. introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/62393 milestone: '14.0' gitlab_schema: gitlab_ci diff --git a/db/docs/historical_data.yml b/db/docs/historical_data.yml index a7af9e04c14..d822b0d32b4 100644 --- a/db/docs/historical_data.yml +++ b/db/docs/historical_data.yml @@ -4,7 +4,8 @@ classes: - HistoricalData feature_categories: - sm_provisioning -description: TODO +- seat_cost_management +description: Stores seat usage data as active_user_count. Used in service ping analytics, cloud licensing, user limits, and renewal workflows. introduced_by_url: https://dev.gitlab.org/gitlab/gitlab-ee/-/merge_requests/390 milestone: '7.11' gitlab_schema: gitlab_main diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 11b298c8264..c7c0ee8a54d 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -1851,7 +1851,7 @@ msgstr "" msgid "AI|Explain the code from %{filePath} in human understandable language presented in Markdown format. In the response add neither original code snippet nor any title. `%{text}`" msgstr "" -msgid "AI|Give feedback on code explanation" +msgid "AI|Helpful" msgstr "" msgid "AI|Responses generated by AI" @@ -1863,9 +1863,15 @@ msgstr "" msgid "AI|The container element wasn't found, stopping AI Genie." msgstr "" +msgid "AI|Unhelpful" +msgstr "" + msgid "AI|What does the selected code mean?" msgstr "" +msgid "AI|Wrong" +msgstr "" + msgid "AI|You are not allowed to copy any part of this output into issues, comments, GitLab source code, commit messages, merge requests or any other user interface in the %{gitlabOrg} or %{gitlabCom} groups." msgstr "" @@ -8701,6 +8707,9 @@ msgstr "" msgid "Changed the title to \"%{title_param}\"." msgstr "" +msgid "Changelog" +msgstr "" + msgid "Changes" msgstr "" @@ -28673,6 +28682,9 @@ msgstr "" msgid "Namespace ID:" msgstr "" +msgid "Namespace Limits" +msgstr "" + msgid "Namespace or group to import repository into does not exist." msgstr "" @@ -28682,10 +28694,22 @@ msgstr "" msgid "Namespace:" msgstr "" +msgid "NamespaceLimits|%{linkStart}%{username}%{linkEnd} changed the limit to %{limit} at %{date}" +msgstr "" + +msgid "NamespaceLimits|Confirm limits change" +msgstr "" + msgid "NamespaceLimits|Free Tier" msgstr "" -msgid "NamespaceLimit|Storage Phased Notification" +msgid "NamespaceLimits|Storage Phased Notification" +msgstr "" + +msgid "NamespaceLimits|This will limit the amount of notifications your namespace receives, this can be removed in the future." +msgstr "" + +msgid "NamespaceLimits|Update limit" msgstr "" msgid "NamespaceStorageSize|%{namespace_name} contains %{locked_project_count} locked project" diff --git a/qa/qa/page/project/settings/mirroring_repositories.rb b/qa/qa/page/project/settings/mirroring_repositories.rb index 61ee3e4f03c..4efe5212c14 100644 --- a/qa/qa/page/project/settings/mirroring_repositories.rb +++ b/qa/qa/page/project/settings/mirroring_repositories.rb @@ -120,7 +120,8 @@ module QA all_elements(:mirror_repository_url_content, minimum: 1).index do |url| # The url might be a sanitized url but the target_url won't be so # we compare just the paths instead of the full url - URI.parse(url.text).path == target_url.path + # We also must remove any badges from the url (e.g. All Branches) + URI.parse(url.text.split("\n").first).path == target_url.path end end end diff --git a/spec/controllers/projects/blame_controller_spec.rb b/spec/controllers/projects/blame_controller_spec.rb index 06c82bcb404..50556bdb652 100644 --- a/spec/controllers/projects/blame_controller_spec.rb +++ b/spec/controllers/projects/blame_controller_spec.rb @@ -54,4 +54,14 @@ RSpec.describe Projects::BlameController, feature_category: :source_code_managem it_behaves_like 'blame_response' end + + describe 'GET streaming' do + render_views + + before do + get :streaming, params: { namespace_id: project.namespace, project_id: project, id: id } + end + + it_behaves_like 'blame_response' + end end diff --git a/spec/helpers/blame_helper_spec.rb b/spec/helpers/blame_helper_spec.rb index d305c4c595e..54d714b2038 100644 --- a/spec/helpers/blame_helper_spec.rb +++ b/spec/helpers/blame_helper_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe BlameHelper do +RSpec.describe BlameHelper, feature_category: :source_code_management do describe '#get_age_map_start_date' do let(:dates) do [Time.zone.local(2014, 3, 17, 0, 0, 0), @@ -67,4 +67,25 @@ RSpec.describe BlameHelper do end end end + + describe '#entire_blame_path' do + subject { helper.entire_blame_path(id, project, blame_mode) } + + let_it_be(:project) { build_stubbed(:project) } + + let(:id) { 'main/README.md' } + let(:blame_mode) { instance_double('Gitlab::Git::BlameMode', 'streaming_supported?' => streaming_enabled) } + + context 'when streaming is supported' do + let(:streaming_enabled) { true } + + it { is_expected.to eq "/#{project.full_path}/-/blame/#{id}/streaming" } + end + + context 'when streaming is not supported' do + let(:streaming_enabled) { false } + + it { is_expected.to eq "/#{project.full_path}/-/blame/#{id}?no_pagination=true" } + end + end end diff --git a/spec/routing/project_routing_spec.rb b/spec/routing/project_routing_spec.rb index b1fafffb253..c2a3afc0e57 100644 --- a/spec/routing/project_routing_spec.rb +++ b/spec/routing/project_routing_spec.rb @@ -491,6 +491,14 @@ RSpec.describe 'project routing' do namespace_id: 'gitlab', project_id: 'gitlabhq', id: "master/#{newline_file}" }) end + + it 'to #streaming' do + expect(get('/gitlab/gitlabhq/-/blame/master/app/models/project.rb/streaming')).to route_to('projects/blame#streaming', namespace_id: 'gitlab', project_id: 'gitlabhq', id: 'master/app/models/project.rb', streaming: true) + end + + it 'to #page' do + expect(get('/gitlab/gitlabhq/-/blame_page/master/app/models/project.rb')).to route_to('projects/blame#page', namespace_id: 'gitlab', project_id: 'gitlabhq', id: 'master/app/models/project.rb') + end end # project_blob GET /:project_id/-/blob/:id(.:format) blob#show {id: /[^\0]+/, project_id: /[^\/]+/} diff --git a/spec/tooling/lib/tooling/find_files_using_feature_flags_spec.rb b/spec/tooling/lib/tooling/find_files_using_feature_flags_spec.rb new file mode 100644 index 00000000000..f553d34768f --- /dev/null +++ b/spec/tooling/lib/tooling/find_files_using_feature_flags_spec.rb @@ -0,0 +1,122 @@ +# frozen_string_literal: true + +require 'tempfile' +require_relative '../../../../tooling/lib/tooling/find_files_using_feature_flags' + +RSpec.describe Tooling::FindFilesUsingFeatureFlags, feature_category: :tooling do + attr_accessor :changed_files_file + + let(:changed_files_pathname) { changed_files_file.path } + let(:instance) { described_class.new(changed_files_pathname: changed_files_pathname) } + let(:changed_files_content) { '' } + + around do |example| + self.changed_files_file = Tempfile.new('changed_files_file') + + # See https://ruby-doc.org/stdlib-1.9.3/libdoc/tempfile/rdoc/ + # Tempfile.html#class-Tempfile-label-Explicit+close + begin + example.run + ensure + changed_files_file.close + changed_files_file.unlink + end + end + + before do + allow(File).to receive(:exist?).and_call_original + allow(File).to receive(:read).and_call_original + + File.write(changed_files_pathname, changed_files_content) + end + + describe '#execute' do + subject { instance.execute } + + let(:valid_ff_pathname_1) { 'config/feature_flags/development/my_feature_flag.yml' } + let(:valid_ff_pathname_2) { 'config/feature_flags/development/my_other_feature_flag.yml' } + let(:changed_files_content) { "#{valid_ff_pathname_1} #{valid_ff_pathname_2}" } + let(:ruby_files) { [] } + + before do + allow(File).to receive(:exist?).with(valid_ff_pathname_1).and_return(true) + allow(File).to receive(:exist?).with(valid_ff_pathname_2).and_return(true) + allow(Dir).to receive(:[]).with('**/*.rb').and_return(ruby_files) + end + + context 'when no ruby files are using the modified feature flag' do + let(:ruby_files) { [] } + + it 'does not add anything to the input file' do + expect { subject }.not_to change { File.read(changed_files_pathname) } + end + end + + context 'when some ruby files are using the modified feature flags' do + let(:matching_ruby_file_1) { 'first-ruby-file' } + let(:matching_ruby_file_2) { 'second-ruby-file' } + let(:not_matching_ruby_file) { 'third-ruby-file' } + let(:ruby_files) { [matching_ruby_file_1, matching_ruby_file_2, not_matching_ruby_file] } + + before do + allow(File).to receive(:read).with(matching_ruby_file_1).and_return('my_feature_flag') + allow(File).to receive(:read).with(matching_ruby_file_2).and_return('my_other_feature_flag') + allow(File).to receive(:read).with(not_matching_ruby_file).and_return('other text') + end + + it 'add the matching ruby files to the input file' do + expect { subject }.to change { File.read(changed_files_pathname) } + .from(changed_files_content) + .to("#{changed_files_content} #{matching_ruby_file_1} #{matching_ruby_file_2}") + end + end + end + + describe '#filter_files' do + subject { instance.filter_files } + + let(:changed_files_content) { path_to_file } + + context 'when the file does not exist on disk' do + let(:path_to_file) { "config/other_feature_flags_folder/feature.yml" } + + before do + allow(File).to receive(:exist?).with(path_to_file).and_return(false) + end + + it 'returns an empty array' do + expect(subject).to be_empty + end + end + + context 'when the file exists on disk' do + before do + allow(File).to receive(:exist?).with(path_to_file).and_return(true) + end + + context 'when the file is not in the features folder' do + let(:path_to_file) { "config/other_folder/development/feature.yml" } + + it 'returns an empty array' do + expect(subject).to be_empty + end + end + + context 'when the filename does not have the correct extension' do + let(:path_to_file) { "config/feature_flags/development/feature.rb" } + + it 'returns an empty array' do + expect(subject).to be_empty + end + end + + context 'when the ruby file uses a valid feature flag file' do + let(:path_to_file) { "config/feature_flags/development/feature.yml" } + + it 'returns the file' do + expect(subject).to match_array(path_to_file) + end + end + end + end +end diff --git a/spec/tooling/lib/tooling/mappings/base_spec.rb b/spec/tooling/lib/tooling/helpers/predictive_tests_helper_spec.rb index 935f833fa8b..48a5866ac56 100644 --- a/spec/tooling/lib/tooling/mappings/base_spec.rb +++ b/spec/tooling/lib/tooling/helpers/predictive_tests_helper_spec.rb @@ -1,12 +1,19 @@ # frozen_string_literal: true -require_relative '../../../../../tooling/lib/tooling/mappings/view_to_js_mappings' +require 'tempfile' +require_relative '../../../../../tooling/lib/tooling/helpers/predictive_tests_helper' + +class MockClass # rubocop:disable Gitlab/NamespacedClass + include Tooling::Helpers::PredictiveTestsHelper +end + +RSpec.describe Tooling::Helpers::PredictiveTestsHelper, feature_category: :tooling do + let(:instance) { MockClass.new } -RSpec.describe Tooling::Mappings::Base, feature_category: :tooling do describe '#folders_for_available_editions' do let(:base_folder_path) { 'app/views' } - subject { described_class.new.folders_for_available_editions(base_folder_path) } + subject { instance.folders_for_available_editions(base_folder_path) } context 'when FOSS' do before do diff --git a/spec/workers/namespaces/root_statistics_worker_spec.rb b/spec/workers/namespaces/root_statistics_worker_spec.rb index bc2eca86711..2224d94cd9d 100644 --- a/spec/workers/namespaces/root_statistics_worker_spec.rb +++ b/spec/workers/namespaces/root_statistics_worker_spec.rb @@ -7,84 +7,44 @@ RSpec.describe Namespaces::RootStatisticsWorker, '#perform', feature_category: : subject(:worker) { described_class.new } - RSpec.shared_examples 'bypasses aggregation schedule' do + context 'with a namespace' do it 'executes refresher service' do expect_any_instance_of(Namespaces::StatisticsRefresherService) .to receive(:execute).and_call_original - expect(group).not_to receive(:aggregation_scheduled?) worker.perform(group.id) end - it 'does not change AggregationSchedule count' do - expect do - worker.perform(group.id) - end.not_to change { Namespace::AggregationSchedule.count } - end - end - - context 'with a namespace' do - context 'with remove_aggregation_schedule_lease feature flag enabled' do - it_behaves_like 'bypasses aggregation schedule' - - context 'when something goes wrong when updating' do - before do - allow_any_instance_of(Namespaces::StatisticsRefresherService) - .to receive(:execute) - .and_raise(Namespaces::StatisticsRefresherService::RefresherError, 'error') - end - - it 'logs the error' do - expect(Gitlab::ErrorTracking).to receive(:track_exception).once + it 'deletes namespace aggregated schedule row' do + worker.perform(group.id) - worker.perform(group.id) - end - end + expect(group.reload.aggregation_schedule).to be_nil end - context 'with remove_aggregation_schedule_lease feature flag disabled' do + context 'when something goes wrong when updating' do before do - stub_feature_flags(remove_aggregation_schedule_lease: false) - end - - it 'executes refresher service' do - expect_any_instance_of(Namespaces::StatisticsRefresherService) - .to receive(:execute).and_call_original - - worker.perform(group.id) + allow_any_instance_of(Namespaces::StatisticsRefresherService) + .to receive(:execute) + .and_raise(Namespaces::StatisticsRefresherService::RefresherError, 'error') end - it 'deletes namespace aggregated schedule row' do + it 'does not delete the aggregation schedule' do worker.perform(group.id) - expect(group.reload.aggregation_schedule).to be_nil + expect(group.reload.aggregation_schedule).to be_present end - context 'when something goes wrong when updating' do - before do - allow_any_instance_of(Namespaces::StatisticsRefresherService) - .to receive(:execute) - .and_raise(Namespaces::StatisticsRefresherService::RefresherError, 'error') - end - - it 'does not delete the aggregation schedule' do - worker.perform(group.id) - - expect(group.reload.aggregation_schedule).to be_present - end - - it 'logs the error' do - # A Namespace::RootStatisticsWorker is scheduled when - # a Namespace::AggregationSchedule is created, so having - # create(:group, :with_aggregation_schedule), will execute - # another worker - allow_any_instance_of(Namespace::AggregationSchedule) - .to receive(:schedule_root_storage_statistics).and_return(nil) + it 'logs the error' do + # A Namespace::RootStatisticsWorker is scheduled when + # a Namespace::AggregationSchedule is created, so having + # create(:group, :with_aggregation_schedule), will execute + # another worker + allow_any_instance_of(Namespace::AggregationSchedule) + .to receive(:schedule_root_storage_statistics).and_return(nil) - expect(Gitlab::ErrorTracking).to receive(:track_exception).once + expect(Gitlab::ErrorTracking).to receive(:track_exception).once - worker.perform(group.id) - end + worker.perform(group.id) end end end @@ -107,42 +67,22 @@ RSpec.describe Namespaces::RootStatisticsWorker, '#perform', feature_category: : group.aggregation_schedule.destroy! end - context 'with remove_aggregation_schedule_lease feature flag disabled' do - before do - stub_feature_flags(remove_aggregation_schedule_lease: false) - end - - it 'does not execute the refresher service' do - expect_any_instance_of(Namespaces::StatisticsRefresherService) - .not_to receive(:execute) - - worker.perform(group.id) - end - end + it 'does not execute the refresher service' do + expect_any_instance_of(Namespaces::StatisticsRefresherService) + .not_to receive(:execute) - context 'with remove_aggregation_schedule_lease feature flag enabled' do - it_behaves_like 'bypasses aggregation schedule' + worker.perform(group.id) end end it_behaves_like 'an idempotent worker' do let(:job_args) { [group.id] } - context 'with remove_aggregation_schedule_lease feature flag disabled' do - before do - stub_feature_flags(remove_aggregation_schedule_lease: false) - end - - it 'deletes one aggregation schedule' do - # Make sure the group and it's aggregation schedule are created before - # counting - group - - expect { worker.perform(*job_args) } - .to change { Namespace::AggregationSchedule.count }.by(-1) - expect { worker.perform(*job_args) } - .not_to change { Namespace::AggregationSchedule.count } - end + it 'deletes one aggregation schedule' do + expect { worker.perform(*job_args) } + .to change { Namespace::AggregationSchedule.count }.by(-1) + expect { worker.perform(*job_args) } + .not_to change { Namespace::AggregationSchedule.count } end end diff --git a/spec/workers/namespaces/schedule_aggregation_worker_spec.rb b/spec/workers/namespaces/schedule_aggregation_worker_spec.rb index 249c143606f..7d0746a4bb0 100644 --- a/spec/workers/namespaces/schedule_aggregation_worker_spec.rb +++ b/spec/workers/namespaces/schedule_aggregation_worker_spec.rb @@ -7,64 +7,28 @@ RSpec.describe Namespaces::ScheduleAggregationWorker, '#perform', :clean_gitlab_ subject(:worker) { described_class.new } - RSpec.shared_examples 'schedule root statistic worker' do - it 'enqueues only RootStatisticsWorker' do - expect(Namespaces::RootStatisticsWorker).to receive(:perform_async).with(group.root_ancestor.id) - expect(Namespace::AggregationSchedule).not_to receive(:safe_find_or_create_by!) - .with(namespace_id: group.root_ancestor.id) - - worker.perform(group.id) - end - - it 'does not change AggregationSchedule count' do - expect do - worker.perform(group.root_ancestor.id) - end.not_to change { Namespace::AggregationSchedule.count } - end - end - context 'when group is the root ancestor' do - context 'with remove_aggregation_schedule_lease feature flag enabled' do - context 'when aggregation schedule does not exist' do - it_behaves_like "schedule root statistic worker" - end + context 'when aggregation schedule exists' do + it 'does not create a new one' do + stub_aggregation_schedule_statistics - context 'when aggregation schedule does exist' do - before do - Namespace::AggregationSchedule.safe_find_or_create_by!(namespace_id: group.id) - end + Namespace::AggregationSchedule.safe_find_or_create_by!(namespace_id: group.id) - it_behaves_like "schedule root statistic worker" + expect do + worker.perform(group.id) + end.not_to change { Namespace::AggregationSchedule.count } end end - context 'with remove_aggregation_schedule_lease feature flag disabled' do - before do - stub_feature_flags(remove_aggregation_schedule_lease: false) - end - - context 'when aggregation schedule exists' do - it 'does not create a new one' do - stub_aggregation_schedule_statistics - - Namespace::AggregationSchedule.safe_find_or_create_by!(namespace_id: group.id) - - expect do - worker.perform(group.id) - end.not_to change { Namespace::AggregationSchedule.count } - end - end - - context 'when aggregation schedule does not exist' do - it 'creates one' do - stub_aggregation_schedule_statistics + context 'when aggregation schedule does not exist' do + it 'creates one' do + stub_aggregation_schedule_statistics - expect do - worker.perform(group.id) - end.to change { Namespace::AggregationSchedule.count }.by(1) + expect do + worker.perform(group.id) + end.to change { Namespace::AggregationSchedule.count }.by(1) - expect(group.aggregation_schedule).to be_present - end + expect(group.aggregation_schedule).to be_present end end end @@ -73,22 +37,12 @@ RSpec.describe Namespaces::ScheduleAggregationWorker, '#perform', :clean_gitlab_ let(:parent_group) { create(:group) } let(:group) { create(:group, parent: parent_group) } - context 'with remove_aggregation_schedule_lease feature flag enabled' do - it_behaves_like "schedule root statistic worker" - end + it 'creates an aggregation schedule for the root' do + stub_aggregation_schedule_statistics - context 'with remove_aggregation_schedule_lease feature flag disabled' do - before do - stub_feature_flags(remove_aggregation_schedule_lease: false) - end - - it 'creates an aggregation schedule for the root' do - stub_aggregation_schedule_statistics - - worker.perform(group.id) + worker.perform(group.id) - expect(parent_group.aggregation_schedule).to be_present - end + expect(parent_group.aggregation_schedule).to be_present end end @@ -103,17 +57,11 @@ RSpec.describe Namespaces::ScheduleAggregationWorker, '#perform', :clean_gitlab_ it_behaves_like 'an idempotent worker' do let(:job_args) { [group.id] } - context 'with remove_aggregation_schedule_lease feature flag disabled' do - before do - stub_feature_flags(remove_aggregation_schedule_lease: false) - end - - it 'creates a single aggregation schedule' do - expect { worker.perform(*job_args) } - .to change { Namespace::AggregationSchedule.count }.by(1) - expect { worker.perform(*job_args) } - .not_to change { Namespace::AggregationSchedule.count } - end + it 'creates a single aggregation schedule' do + expect { worker.perform(*job_args) } + .to change { Namespace::AggregationSchedule.count }.by(1) + expect { worker.perform(*job_args) } + .not_to change { Namespace::AggregationSchedule.count } end end diff --git a/tooling/lib/tooling/find_changes.rb b/tooling/lib/tooling/find_changes.rb index d8373d11245..25381e1a894 100755 --- a/tooling/lib/tooling/find_changes.rb +++ b/tooling/lib/tooling/find_changes.rb @@ -2,11 +2,11 @@ # frozen_string_literal: true require 'gitlab' -require_relative 'helpers/file_handler' +require_relative 'helpers/predictive_tests_helper' module Tooling class FindChanges - include Helpers::FileHandler + include Helpers::PredictiveTestsHelper def initialize( from:, diff --git a/tooling/lib/tooling/find_files_using_feature_flags.rb b/tooling/lib/tooling/find_files_using_feature_flags.rb new file mode 100644 index 00000000000..27cace60408 --- /dev/null +++ b/tooling/lib/tooling/find_files_using_feature_flags.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +require 'test_file_finder' +require_relative 'helpers/predictive_tests_helper' + +module Tooling + class FindFilesUsingFeatureFlags + include Helpers::PredictiveTestsHelper + + def initialize(changed_files_pathname:, feature_flags_base_folder: 'config/feature_flags') + @changed_files_pathname = changed_files_pathname + @changed_files = read_array_from_file(changed_files_pathname) + @feature_flags_base_folders = folders_for_available_editions(feature_flags_base_folder) + end + + def execute + ff_union_regexp = Regexp.union(feature_flag_filenames) + + files_using_modified_feature_flags = ruby_files.select do |ruby_file| + ruby_file if ff_union_regexp.match?(File.read(ruby_file)) + end + + write_array_to_file(changed_files_pathname, files_using_modified_feature_flags.uniq) + end + + def filter_files + @_filter_files ||= changed_files.select do |filename| + filename.start_with?(*feature_flags_base_folders) && + File.basename(filename).end_with?('.yml') && + File.exist?(filename) + end + end + + private + + def feature_flag_filenames + filter_files.map do |feature_flag_pathname| + File.basename(feature_flag_pathname).delete_suffix('.yml') + end + end + + def ruby_files + Dir["**/*.rb"].reject { |pathname| pathname.start_with?('vendor') } + end + + attr_reader :changed_files, :changed_files_pathname, :feature_flags_base_folders + end +end diff --git a/tooling/lib/tooling/find_tests.rb b/tooling/lib/tooling/find_tests.rb index f26c1eacdc7..bf7a608878b 100644 --- a/tooling/lib/tooling/find_tests.rb +++ b/tooling/lib/tooling/find_tests.rb @@ -1,11 +1,11 @@ # frozen_string_literal: true require 'test_file_finder' -require_relative 'helpers/file_handler' +require_relative 'helpers/predictive_tests_helper' module Tooling class FindTests - include Helpers::FileHandler + include Helpers::PredictiveTestsHelper def initialize(changed_files_pathname, predictive_tests_pathname) @predictive_tests_pathname = predictive_tests_pathname diff --git a/tooling/lib/tooling/mappings/base.rb b/tooling/lib/tooling/helpers/predictive_tests_helper.rb index 27a9a0925b0..b8e5a30024e 100644 --- a/tooling/lib/tooling/mappings/base.rb +++ b/tooling/lib/tooling/helpers/predictive_tests_helper.rb @@ -5,9 +5,9 @@ require_relative '../helpers/file_handler' # Returns system specs files that are related to the JS files that were changed in the MR. module Tooling - module Mappings - class Base - include Helpers::FileHandler + module Helpers + module PredictiveTestsHelper + include FileHandler # Input: A folder # Output: An array of folders, each prefixed with a GitLab edition diff --git a/tooling/lib/tooling/mappings/graphql_base_type_mappings.rb b/tooling/lib/tooling/mappings/graphql_base_type_mappings.rb index 569a8278163..80aa99efc96 100644 --- a/tooling/lib/tooling/mappings/graphql_base_type_mappings.rb +++ b/tooling/lib/tooling/mappings/graphql_base_type_mappings.rb @@ -2,13 +2,15 @@ require 'active_support/inflector' -require_relative 'base' +require_relative '../helpers/predictive_tests_helper' require_relative '../../../../lib/gitlab_edition' # If a GraphQL type class changed, we try to identify the other GraphQL types that potentially include this type. module Tooling module Mappings - class GraphqlBaseTypeMappings < Base + class GraphqlBaseTypeMappings + include Helpers::PredictiveTestsHelper + # Checks for the implements keyword, and graphql_base_types the class name GRAPHQL_IMPLEMENTS_REGEXP = /implements[( ]([\w:]+)[)]?$/ diff --git a/tooling/lib/tooling/mappings/js_to_system_specs_mappings.rb b/tooling/lib/tooling/mappings/js_to_system_specs_mappings.rb index b2fca3a765a..bc2cd259fdc 100644 --- a/tooling/lib/tooling/mappings/js_to_system_specs_mappings.rb +++ b/tooling/lib/tooling/mappings/js_to_system_specs_mappings.rb @@ -2,13 +2,15 @@ require 'active_support/inflector' -require_relative 'base' +require_relative '../helpers/predictive_tests_helper' require_relative '../../../../lib/gitlab_edition' # Returns system specs files that are related to the JS files that were changed in the MR. module Tooling module Mappings - class JsToSystemSpecsMappings < Base + class JsToSystemSpecsMappings + include Helpers::PredictiveTestsHelper + def initialize( changed_files_pathname, predictive_tests_pathname, js_base_folder: 'app/assets/javascripts', system_specs_base_folder: 'spec/features') diff --git a/tooling/lib/tooling/mappings/partial_to_views_mappings.rb b/tooling/lib/tooling/mappings/partial_to_views_mappings.rb index 8b0a5ed4ecd..931cacea77f 100644 --- a/tooling/lib/tooling/mappings/partial_to_views_mappings.rb +++ b/tooling/lib/tooling/mappings/partial_to_views_mappings.rb @@ -1,12 +1,14 @@ # frozen_string_literal: true -require_relative 'base' +require_relative '../helpers/predictive_tests_helper' require_relative '../../../../lib/gitlab_edition' # Returns view files that include the potential rails partials from the changed files passed as input. module Tooling module Mappings - class PartialToViewsMappings < Base + class PartialToViewsMappings + include Helpers::PredictiveTestsHelper + def initialize(changed_files_pathname, views_with_partials_pathname, view_base_folder: 'app/views') @views_with_partials_pathname = views_with_partials_pathname @changed_files = read_array_from_file(changed_files_pathname) diff --git a/tooling/lib/tooling/mappings/view_to_js_mappings.rb b/tooling/lib/tooling/mappings/view_to_js_mappings.rb index f2098d6acd5..b78c354f9d2 100644 --- a/tooling/lib/tooling/mappings/view_to_js_mappings.rb +++ b/tooling/lib/tooling/mappings/view_to_js_mappings.rb @@ -1,12 +1,14 @@ # frozen_string_literal: true -require_relative 'base' +require_relative '../helpers/predictive_tests_helper' require_relative '../../../../lib/gitlab_edition' # Returns JS files that are related to the Rails views files that were changed in the MR. module Tooling module Mappings - class ViewToJsMappings < Base + class ViewToJsMappings + include Helpers::PredictiveTestsHelper + # The HTML attribute value pattern we're looking for to match an HTML file to a JS file. HTML_ATTRIBUTE_VALUE_REGEXP = /js-[-\w]+/.freeze diff --git a/tooling/lib/tooling/mappings/view_to_system_specs_mappings.rb b/tooling/lib/tooling/mappings/view_to_system_specs_mappings.rb index 6d840dcbd71..1542c817745 100644 --- a/tooling/lib/tooling/mappings/view_to_system_specs_mappings.rb +++ b/tooling/lib/tooling/mappings/view_to_system_specs_mappings.rb @@ -1,12 +1,14 @@ # frozen_string_literal: true -require_relative 'base' +require_relative '../helpers/predictive_tests_helper' require_relative '../../../../lib/gitlab_edition' # Returns system specs files that are related to the Rails views files that were changed in the MR. module Tooling module Mappings - class ViewToSystemSpecsMappings < Base + class ViewToSystemSpecsMappings + include Helpers::PredictiveTestsHelper + def initialize(changed_files_pathname, predictive_tests_pathname, view_base_folder: 'app/views') @predictive_tests_pathname = predictive_tests_pathname @changed_files = read_array_from_file(changed_files_pathname) diff --git a/tooling/lib/tooling/predictive_tests.rb b/tooling/lib/tooling/predictive_tests.rb index 503426b5520..1ad63e111e3 100644 --- a/tooling/lib/tooling/predictive_tests.rb +++ b/tooling/lib/tooling/predictive_tests.rb @@ -2,6 +2,7 @@ require_relative 'find_changes' require_relative 'find_tests' +require_relative 'find_files_using_feature_flags' require_relative 'mappings/graphql_base_type_mappings' require_relative 'mappings/js_to_system_specs_mappings' require_relative 'mappings/partial_to_views_mappings' @@ -36,6 +37,7 @@ module Tooling from: :api, changed_files_pathname: rspec_changed_files_path ).execute + Tooling::FindFilesUsingFeatureFlags.new(changed_files_pathname: rspec_changed_files_path).execute Tooling::FindTests.new(rspec_changed_files_path, rspec_matching_tests_path).execute Tooling::Mappings::PartialToViewsMappings.new( rspec_changed_files_path, rspec_views_including_partials_path).execute |