diff options
Diffstat (limited to 'spec/tooling')
-rw-r--r-- | spec/tooling/danger/feature_flag_spec.rb | 2 | ||||
-rw-r--r-- | spec/tooling/danger/product_intelligence_spec.rb | 74 | ||||
-rw-r--r-- | spec/tooling/danger/project_helper_spec.rb | 2 | ||||
-rw-r--r-- | spec/tooling/danger/specs_spec.rb | 66 | ||||
-rw-r--r-- | spec/tooling/danger/stable_branch_spec.rb | 169 | ||||
-rw-r--r-- | spec/tooling/danger/user_types_spec.rb | 56 | ||||
-rw-r--r-- | spec/tooling/docs/deprecation_handling_spec.rb | 4 | ||||
-rw-r--r-- | spec/tooling/fixtures/metrics/sample_instrumentation_metric.rb | 15 | ||||
-rw-r--r-- | spec/tooling/quality/test_level_spec.rb | 4 |
9 files changed, 374 insertions, 18 deletions
diff --git a/spec/tooling/danger/feature_flag_spec.rb b/spec/tooling/danger/feature_flag_spec.rb index 7cae3e0a8b3..0e9eda54510 100644 --- a/spec/tooling/danger/feature_flag_spec.rb +++ b/spec/tooling/danger/feature_flag_spec.rb @@ -135,7 +135,7 @@ RSpec.describe Tooling::Danger::FeatureFlag do end context 'when MR labels does not match FF group' do - let(:mr_group_label) { 'group::access' } + let(:mr_group_label) { 'group::authentication and authorization' } specify { expect(result).to eq(false) } end diff --git a/spec/tooling/danger/product_intelligence_spec.rb b/spec/tooling/danger/product_intelligence_spec.rb index ea08e3bc6db..fab8b0c61fa 100644 --- a/spec/tooling/danger/product_intelligence_spec.rb +++ b/spec/tooling/danger/product_intelligence_spec.rb @@ -12,12 +12,16 @@ RSpec.describe Tooling::Danger::ProductIntelligence do subject(:product_intelligence) { fake_danger.new(helper: fake_helper) } let(:fake_danger) { DangerSpecHelper.fake_danger.include(described_class) } - let(:changed_files) { ['metrics/counts_7d/test_metric.yml'] } - let(:changed_lines) { ['+tier: ee'] } + let(:previous_label_to_add) { 'label_to_add' } + let(:labels_to_add) { [previous_label_to_add] } + let(:ci_env) { true } + let(:has_product_intelligence_label) { true } before do - allow(fake_helper).to receive(:all_changed_files).and_return(changed_files) allow(fake_helper).to receive(:changed_lines).and_return(changed_lines) + allow(fake_helper).to receive(:labels_to_add).and_return(labels_to_add) + allow(fake_helper).to receive(:ci?).and_return(ci_env) + allow(fake_helper).to receive(:mr_has_labels?).with('product intelligence').and_return(has_product_intelligence_label) end describe '#check!' do @@ -26,17 +30,13 @@ RSpec.describe Tooling::Danger::ProductIntelligence do let(:markdown_formatted_list) { 'markdown formatted list' } let(:review_pending_label) { 'product intelligence::review pending' } let(:approved_label) { 'product intelligence::approved' } - let(:ci_env) { true } - let(:previous_label_to_add) { 'label_to_add' } - let(:labels_to_add) { [previous_label_to_add] } - let(:has_product_intelligence_label) { true } + let(:changed_files) { ['metrics/counts_7d/test_metric.yml'] } + let(:changed_lines) { ['+tier: ee'] } before do + allow(fake_helper).to receive(:all_changed_files).and_return(changed_files) allow(fake_helper).to receive(:changes_by_category).and_return(product_intelligence: changed_files, database: ['other_files.yml']) - allow(fake_helper).to receive(:ci?).and_return(ci_env) - allow(fake_helper).to receive(:mr_has_labels?).with('product intelligence').and_return(has_product_intelligence_label) allow(fake_helper).to receive(:markdown_list).with(changed_files).and_return(markdown_formatted_list) - allow(fake_helper).to receive(:labels_to_add).and_return(labels_to_add) end shared_examples "doesn't add new labels" do @@ -121,4 +121,58 @@ RSpec.describe Tooling::Danger::ProductIntelligence do end end end + + describe '#check_affected_scopes!' do + let(:fixture_dir_glob) { Dir.glob(File.join('spec', 'tooling', 'fixtures', 'metrics', '*.rb')) } + let(:changed_lines) { ['+ scope :active, -> { iwhere(email: Array(emails)) }'] } + + before do + allow(Dir).to receive(:glob).and_return(fixture_dir_glob) + allow(fake_helper).to receive(:markdown_list).with({ 'active' => fixture_dir_glob }).and_return('a') + end + + context 'when a model was modified' do + let(:modified_files) { ['app/models/super_user.rb'] } + + context 'when a scope is changed' do + context 'and a metrics uses the affected scope' do + it 'producing warning' do + expect(product_intelligence).to receive(:warn).with(%r{#{modified_files}}) + + product_intelligence.check_affected_scopes! + end + end + + context 'when no metrics using the affected scope' do + let(:changed_lines) { ['+scope :foo, -> { iwhere(email: Array(emails)) }'] } + + it 'doesnt do anything' do + expect(product_intelligence).not_to receive(:warn) + + product_intelligence.check_affected_scopes! + end + end + end + end + + context 'when an unrelated model with matching scope was modified' do + let(:modified_files) { ['app/models/post_box.rb'] } + + it 'doesnt do anything' do + expect(product_intelligence).not_to receive(:warn) + + product_intelligence.check_affected_scopes! + end + end + + context 'when models arent modified' do + let(:modified_files) { ['spec/app/models/user_spec.rb'] } + + it 'doesnt do anything' do + expect(product_intelligence).not_to receive(:warn) + + product_intelligence.check_affected_scopes! + end + end + end end diff --git a/spec/tooling/danger/project_helper_spec.rb b/spec/tooling/danger/project_helper_spec.rb index f9ad9ed13c2..669867ffb4f 100644 --- a/spec/tooling/danger/project_helper_spec.rb +++ b/spec/tooling/danger/project_helper_spec.rb @@ -156,8 +156,6 @@ RSpec.describe Tooling::Danger::ProjectHelper do 'lib/gitlab/database.rb' | [:database, :backend] 'lib/gitlab/database/foo' | [:database, :backend] 'ee/lib/gitlab/database/foo' | [:database, :backend] - 'lib/gitlab/github_import.rb' | [:database, :backend] - 'lib/gitlab/github_import/foo' | [:database, :backend] 'lib/gitlab/sql/foo' | [:database, :backend] 'rubocop/cop/migration/foo' | [:database] diff --git a/spec/tooling/danger/specs_spec.rb b/spec/tooling/danger/specs_spec.rb index d6aed86e7dc..dcc1f592062 100644 --- a/spec/tooling/danger/specs_spec.rb +++ b/spec/tooling/danger/specs_spec.rb @@ -9,7 +9,7 @@ require 'gitlab/dangerfiles/spec_helper' require_relative '../../../tooling/danger/specs' require_relative '../../../tooling/danger/project_helper' -RSpec.describe Tooling::Danger::Specs do +RSpec.describe Tooling::Danger::Specs, feature_category: :tooling do include_context "with dangerfile" let(:fake_danger) { DangerSpecHelper.fake_danger.include(described_class) } @@ -217,4 +217,68 @@ RSpec.describe Tooling::Danger::Specs do specs.add_suggestions_for_project_factory_usage(filename) end end + + describe '#add_suggestions_for_feature_category' do + let(:template) do + <<~SUGGESTION_MARKDOWN + ```suggestion + %<suggested_line>s + ``` + + Consider adding `feature_category: <feature_category_name>` for this example if it is not set already. + See [testing best practices](https://docs.gitlab.com/ee/development/testing_guide/best_practices.html#feature-category-metadata). + SUGGESTION_MARKDOWN + end + + let(:file_lines) do + [ + " require 'spec_helper'", + " \n", + " RSpec.describe Projects::Analytics::CycleAnalytics::SummaryController, feature_category: :planning_analytics do", + " end", + "RSpec.describe Projects::Analytics::CycleAnalytics::SummaryController do", + " let_it_be(:user) { create(:user) }", + " end", + " describe 'GET \"time_summary\"' do", + " end", + " RSpec.describe Projects::Analytics::CycleAnalytics::SummaryController do", + " let_it_be(:user) { create(:user) }", + " end", + " describe 'GET \"time_summary\"' do", + " end" + ] + end + + let(:matching_lines) do + [ + "+ RSpec.describe Projects::Analytics::CycleAnalytics::SummaryController, feature_category: :planning_analytics do", + "+RSpec.describe Projects::Analytics::CycleAnalytics::SummaryController do", + "+ RSpec.describe Projects::Analytics::CycleAnalytics::SummaryController do" + ] + end + + let(:changed_lines) do + [ + "+ RSpec.describe Projects::Analytics::CycleAnalytics::SummaryController, feature_category: :planning_analytics do", + "+RSpec.describe Projects::Analytics::CycleAnalytics::SummaryController do", + "+ let_it_be(:user) { create(:user) }", + "- end", + "+ describe 'GET \"time_summary\"' do", + "+ RSpec.describe Projects::Analytics::CycleAnalytics::SummaryController do" + ] + end + + it 'adds suggestions at the correct lines', :aggregate_failures do + [ + { suggested_line: "RSpec.describe Projects::Analytics::CycleAnalytics::SummaryController do", number: 5 }, + { suggested_line: " RSpec.describe Projects::Analytics::CycleAnalytics::SummaryController do", number: 10 } + + ].each do |test_case| + comment = format(template, suggested_line: test_case[:suggested_line]) + expect(specs).to receive(:markdown).with(comment, file: filename, line: test_case[:number]) + end + + specs.add_suggestions_for_feature_category(filename) + end + end end diff --git a/spec/tooling/danger/stable_branch_spec.rb b/spec/tooling/danger/stable_branch_spec.rb new file mode 100644 index 00000000000..08fd25b30e0 --- /dev/null +++ b/spec/tooling/danger/stable_branch_spec.rb @@ -0,0 +1,169 @@ +# frozen_string_literal: true + +require 'gitlab-dangerfiles' +require 'gitlab/dangerfiles/spec_helper' +require 'rspec-parameterized' +require 'httparty' + +require_relative '../../../tooling/danger/stable_branch' + +RSpec.describe Tooling::Danger::StableBranch, feature_category: :delivery do + using RSpec::Parameterized::TableSyntax + + include_context 'with dangerfile' + let(:fake_danger) { DangerSpecHelper.fake_danger.include(described_class) } + + let(:stable_branch) { fake_danger.new(helper: fake_helper) } + + describe '#check!' do + subject { stable_branch.check! } + + shared_examples 'without a failure' do + it 'does not add a failure' do + expect(stable_branch).not_to receive(:fail) + + subject + end + end + + shared_examples 'with a failure' do |failure_message| + it 'fails' do + expect(stable_branch).to receive(:fail).with(failure_message) + + subject + end + end + + context 'when not applicable' do + where(:stable_branch?, :security_mr?) do + true | true + false | true + false | false + end + + with_them do + before do + allow(fake_helper).to receive(:mr_target_branch).and_return(stable_branch? ? '15-1-stable-ee' : 'main') + allow(fake_helper).to receive(:security_mr?).and_return(security_mr?) + end + + it_behaves_like "without a failure" + end + end + + context 'when applicable' do + let(:target_branch) { '15-1-stable-ee' } + let(:feature_label_present) { false } + let(:bug_label_present) { true } + let(:response_success) { true } + let(:parsed_response) do + [ + { 'version' => '15.1.1' }, + { 'version' => '15.1.0' }, + { 'version' => '15.0.2' }, + { 'version' => '15.0.1' }, + { 'version' => '15.0.0' }, + { 'version' => '14.10.3' }, + { 'version' => '14.10.2' }, + { 'version' => '14.9.3' } + ] + end + + let(:version_response) do + instance_double( + HTTParty::Response, + success?: response_success, + parsed_response: parsed_response + ) + end + + before do + allow(fake_helper).to receive(:mr_target_branch).and_return(target_branch) + allow(fake_helper).to receive(:security_mr?).and_return(false) + allow(fake_helper).to receive(:mr_has_labels?).with('type::feature').and_return(feature_label_present) + allow(fake_helper).to receive(:mr_has_labels?).with('type::bug').and_return(bug_label_present) + allow(HTTParty).to receive(:get).with(/page=1/).and_return(version_response) + end + + # the stubbed behavior above is the success path + it_behaves_like "without a failure" + + context 'with a feature label' do + let(:feature_label_present) { true } + + it_behaves_like 'with a failure', described_class::FEATURE_ERROR_MESSAGE + end + + context 'without a bug label' do + let(:bug_label_present) { false } + + it_behaves_like 'with a failure', described_class::BUG_ERROR_MESSAGE + end + + context 'when not an applicable version' do + let(:target_branch) { '14-9-stable-ee' } + + it_behaves_like 'with a failure', described_class::VERSION_ERROR_MESSAGE + end + + context 'when the version API request fails' do + let(:response_success) { false } + + it 'adds a warning' do + expect(stable_branch).to receive(:warn).with(described_class::FAILED_VERSION_REQUEST_MESSAGE) + + subject + end + end + + context 'when more than one page of versions is needed' do + # we target a version we know will not be returned in the first request + let(:target_branch) { '14-10-stable-ee' } + + let(:first_version_response) do + instance_double( + HTTParty::Response, + success?: response_success, + parsed_response: [ + { 'version' => '15.1.1' }, + { 'version' => '15.1.0' }, + { 'version' => '15.0.2' }, + { 'version' => '15.0.1' } + ] + ) + end + + let(:second_version_response) do + instance_double( + HTTParty::Response, + success?: response_success, + parsed_response: [ + { 'version' => '15.0.0' }, + { 'version' => '14.10.3' }, + { 'version' => '14.10.2' }, + { 'version' => '14.9.3' } + ] + ) + end + + before do + allow(HTTParty).to receive(:get).with(/page=1/).and_return(first_version_response) + allow(HTTParty).to receive(:get).with(/page=2/).and_return(second_version_response) + end + + it_behaves_like "without a failure" + end + + context 'when too many version API requests are made' do + let(:parsed_response) { [{ 'version' => '15.0.0' }] } + + it 'adds a warning' do + expect(HTTParty).to receive(:get).and_return(version_response).at_least(10).times + expect(stable_branch).to receive(:warn).with(described_class::FAILED_VERSION_REQUEST_MESSAGE) + + subject + end + end + end + end +end diff --git a/spec/tooling/danger/user_types_spec.rb b/spec/tooling/danger/user_types_spec.rb new file mode 100644 index 00000000000..53556601212 --- /dev/null +++ b/spec/tooling/danger/user_types_spec.rb @@ -0,0 +1,56 @@ +# frozen_string_literal: true + +require 'gitlab-dangerfiles' +require 'gitlab/dangerfiles/spec_helper' +require_relative '../../../tooling/danger/user_types' + +RSpec.describe Tooling::Danger::UserTypes, feature_category: :subscription_cost_management do + include_context 'with dangerfile' + + let(:fake_danger) { DangerSpecHelper.fake_danger.include(described_class) } + let(:user_types) { fake_danger.new(helper: fake_helper) } + + describe 'changed files' do + subject(:bot_user_types_change_warning) { user_types.bot_user_types_change_warning } + + before do + allow(fake_helper).to receive(:modified_files).and_return(modified_files) + allow(fake_helper).to receive(:changed_lines).and_return(changed_lines) + end + + context 'when has_user_type.rb file is not impacted' do + let(:modified_files) { ['app/models/concerns/importable.rb'] } + let(:changed_lines) { ['+ANY_CHANGES'] } + + it "doesn't add any warnings" do + expect(user_types).not_to receive(:warn) + + bot_user_types_change_warning + end + end + + context 'when the has_user_type.rb file is impacted' do + let(:modified_files) { ['app/models/concerns/has_user_type.rb'] } + + context 'with BOT_USER_TYPES changes' do + let(:changed_lines) { ['+BOT_USER_TYPES'] } + + it 'adds warning' do + expect(user_types).to receive(:warn).with(described_class::BOT_USER_TYPES_CHANGED_WARNING) + + bot_user_types_change_warning + end + end + + context 'without BOT_USER_TYPES changes' do + let(:changed_lines) { ['+OTHER_CHANGES'] } + + it "doesn't add any warnings" do + expect(user_types).not_to receive(:warn) + + bot_user_types_change_warning + end + end + end + end +end diff --git a/spec/tooling/docs/deprecation_handling_spec.rb b/spec/tooling/docs/deprecation_handling_spec.rb index 15dd69275c9..94c93d99b94 100644 --- a/spec/tooling/docs/deprecation_handling_spec.rb +++ b/spec/tooling/docs/deprecation_handling_spec.rb @@ -16,7 +16,7 @@ RSpec.describe Docs::DeprecationHandling do # Create dummy YAML data based on file name allow(YAML).to receive(:load_file) do |file_name| { - 'name' => file_name[/[a-z]*\.yml/], + 'title' => file_name[/[a-z]*\.yml/], 'announcement_milestone' => file_name[/\d+-\d+/].tr('-', '.') } end @@ -29,7 +29,7 @@ RSpec.describe Docs::DeprecationHandling do entries = arguments[:entries] expect(milestones).to eq(['14.10', '14.2']) - expect(entries.map { |e| e['name'] }).to eq(['a.yml', 'b.yml', 'c.yml']) + expect(entries.map { |e| e['title'] }).to eq(['a.yml', 'b.yml', 'c.yml']) end end diff --git a/spec/tooling/fixtures/metrics/sample_instrumentation_metric.rb b/spec/tooling/fixtures/metrics/sample_instrumentation_metric.rb new file mode 100644 index 00000000000..d6b86137b1b --- /dev/null +++ b/spec/tooling/fixtures/metrics/sample_instrumentation_metric.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +module Gitlab + module Usage + module Metrics + module Instrumentations + class ActiveUserCountMetric < DatabaseMetric + operation :count + + relation { SuperUser.active } + end + end + end + end +end diff --git a/spec/tooling/quality/test_level_spec.rb b/spec/tooling/quality/test_level_spec.rb index 6084dc194da..3f46b3e79f4 100644 --- a/spec/tooling/quality/test_level_spec.rb +++ b/spec/tooling/quality/test_level_spec.rb @@ -46,7 +46,7 @@ RSpec.describe Quality::TestLevel do context 'when level is unit' do it 'returns a pattern' do expect(subject.pattern(:unit)) - .to eq("spec/{bin,channels,config,db,dependencies,elastic,elastic_integration,experiments,factories,finders,frontend,graphql,haml_lint,helpers,initializers,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,components}{,/**/}*_spec.rb") + .to eq("spec/{bin,channels,config,contracts,db,dependencies,elastic,elastic_integration,experiments,factories,finders,frontend,graphql,haml_lint,helpers,initializers,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,components}{,/**/}*_spec.rb") end end @@ -121,7 +121,7 @@ RSpec.describe Quality::TestLevel do context 'when level is unit' do it 'returns a regexp' do expect(subject.regexp(:unit)) - .to eq(%r{spec/(bin|channels|config|db|dependencies|elastic|elastic_integration|experiments|factories|finders|frontend|graphql|haml_lint|helpers|initializers|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|components)/}) + .to eq(%r{spec/(bin|channels|config|contracts|db|dependencies|elastic|elastic_integration|experiments|factories|finders|frontend|graphql|haml_lint|helpers|initializers|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|components)/}) end end |