diff options
Diffstat (limited to 'spec/tooling')
30 files changed, 3184 insertions, 728 deletions
diff --git a/spec/tooling/danger/product_intelligence_spec.rb b/spec/tooling/danger/analytics_instrumentation_spec.rb index c4cd0e5bfb6..5d12647e02f 100644 --- a/spec/tooling/danger/product_intelligence_spec.rb +++ b/spec/tooling/danger/analytics_instrumentation_spec.rb @@ -3,39 +3,41 @@ require 'gitlab-dangerfiles' require 'gitlab/dangerfiles/spec_helper' -require_relative '../../../tooling/danger/product_intelligence' +require_relative '../../../tooling/danger/analytics_instrumentation' require_relative '../../../tooling/danger/project_helper' -RSpec.describe Tooling::Danger::ProductIntelligence do +RSpec.describe Tooling::Danger::AnalyticsInstrumentation, feature_category: :service_ping do include_context "with dangerfile" - subject(:product_intelligence) { fake_danger.new(helper: fake_helper) } + subject(:analytics_instrumentation) { fake_danger.new(helper: fake_helper) } let(:fake_danger) { DangerSpecHelper.fake_danger.include(described_class) } 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 } + let(:has_analytics_instrumentation_label) { true } before do allow(fake_helper).to receive(:changed_lines).and_return(changed_lines) if defined?(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) + allow(fake_helper).to receive(:mr_has_labels?).with('analytics instrumentation').and_return(has_analytics_instrumentation_label) end describe '#check!' do - subject { product_intelligence.check! } + subject { analytics_instrumentation.check! } let(:markdown_formatted_list) { 'markdown formatted list' } - let(:review_pending_label) { 'product intelligence::review pending' } - let(:approved_label) { 'product intelligence::approved' } + let(:review_pending_label) { 'analytics instrumentation::review pending' } + let(:approved_label) { 'analytics instrumentation::approved' } let(:changed_files) { ['metrics/counts_7d/test_metric.yml'] } let(:changed_lines) { ['+tier: ee'] } + let(:fake_changes) { instance_double(Gitlab::Dangerfiles::Changes, files: changed_files) } before do + allow(fake_changes).to receive(:by_category).with(:analytics_instrumentation).and_return(fake_changes) + allow(fake_helper).to receive(:changes).and_return(fake_changes) 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(:markdown_list).with(changed_files).and_return(markdown_formatted_list) end @@ -49,7 +51,7 @@ RSpec.describe Tooling::Danger::ProductIntelligence do shared_examples "doesn't add new warnings" do it "doesn't add new warnings" do - expect(product_intelligence).not_to receive(:warn) + expect(analytics_instrumentation).not_to receive(:warn) subject end @@ -61,6 +63,15 @@ RSpec.describe Tooling::Danger::ProductIntelligence do expect(labels_to_add).to match_array [previous_label_to_add, review_pending_label] end + + it 'receives all the changed files by calling the correct helper method', :aggregate_failures do + expect(fake_helper).not_to receive(:changes_by_category) + expect(fake_helper).to receive(:changes) + expect(fake_changes).to receive(:by_category).with(:analytics_instrumentation) + expect(fake_changes).to receive(:files) + + subject + end end context 'with growth experiment label' do @@ -88,26 +99,26 @@ RSpec.describe Tooling::Danger::ProductIntelligence do include_examples 'adds new labels' it 'warns with proper message' do - expect(product_intelligence).to receive(:warn).with(%r{#{markdown_formatted_list}}) + expect(analytics_instrumentation).to receive(:warn).with(%r{#{markdown_formatted_list}}) subject end end - context 'with product intelligence::review pending label' do - let(:mr_labels) { ['product intelligence::review pending'] } + context 'with analytics instrumentation::review pending label' do + let(:mr_labels) { ['analytics instrumentation::review pending'] } include_examples "doesn't add new labels" end - context 'with product intelligence::approved label' do - let(:mr_labels) { ['product intelligence::approved'] } + context 'with analytics instrumentation::approved label' do + let(:mr_labels) { ['analytics instrumentation::approved'] } include_examples "doesn't add new labels" end - context 'with the product intelligence label' do - let(:has_product_intelligence_label) { true } + context 'with the analytics instrumentation label' do + let(:has_analytics_instrumentation_label) { true } context 'with ci? false' do let(:ci_env) { false } @@ -137,9 +148,9 @@ RSpec.describe Tooling::Danger::ProductIntelligence do 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}}) + expect(analytics_instrumentation).to receive(:warn).with(%r{#{modified_files}}) - product_intelligence.check_affected_scopes! + analytics_instrumentation.check_affected_scopes! end end @@ -147,9 +158,9 @@ RSpec.describe Tooling::Danger::ProductIntelligence do let(:changed_lines) { ['+scope :foo, -> { iwhere(email: Array(emails)) }'] } it 'doesnt do anything' do - expect(product_intelligence).not_to receive(:warn) + expect(analytics_instrumentation).not_to receive(:warn) - product_intelligence.check_affected_scopes! + analytics_instrumentation.check_affected_scopes! end end end @@ -159,9 +170,9 @@ RSpec.describe Tooling::Danger::ProductIntelligence do let(:modified_files) { ['app/models/post_box.rb'] } it 'doesnt do anything' do - expect(product_intelligence).not_to receive(:warn) + expect(analytics_instrumentation).not_to receive(:warn) - product_intelligence.check_affected_scopes! + analytics_instrumentation.check_affected_scopes! end end @@ -169,9 +180,9 @@ RSpec.describe Tooling::Danger::ProductIntelligence do let(:modified_files) { ['spec/app/models/user_spec.rb'] } it 'doesnt do anything' do - expect(product_intelligence).not_to receive(:warn) + expect(analytics_instrumentation).not_to receive(:warn) - product_intelligence.check_affected_scopes! + analytics_instrumentation.check_affected_scopes! end end end @@ -188,9 +199,9 @@ RSpec.describe Tooling::Danger::ProductIntelligence do let(:changed_lines) { ['+ ci_runners: count(::Ci::CiRunner),'] } it 'produces warning' do - expect(product_intelligence).to receive(:warn).with(/usage_data\.rb has been deprecated/) + expect(analytics_instrumentation).to receive(:warn).with(/usage_data\.rb has been deprecated/) - product_intelligence.check_usage_data_insertions! + analytics_instrumentation.check_usage_data_insertions! end end @@ -198,24 +209,24 @@ RSpec.describe Tooling::Danger::ProductIntelligence do let(:changed_lines) { ['- ci_runners: count(::Ci::CiRunner),'] } it 'doesnt do anything' do - expect(product_intelligence).not_to receive(:warn) + expect(analytics_instrumentation).not_to receive(:warn) - product_intelligence.check_usage_data_insertions! + analytics_instrumentation.check_usage_data_insertions! end end end context 'when usage_data.rb is not modified' do context 'and another file has insertions' do - let(:modified_files) { ['tooling/danger/product_intelligence.rb'] } + let(:modified_files) { ['tooling/danger/analytics_instrumentation.rb'] } it 'doesnt do anything' do expect(fake_helper).to receive(:changed_lines).with("lib/gitlab/usage_data.rb").and_return([]) - allow(fake_helper).to receive(:changed_lines).with("tooling/danger/product_intelligence.rb").and_return(["+ Inserting"]) + allow(fake_helper).to receive(:changed_lines).with("tooling/danger/analytics_instrumentation.rb").and_return(["+ Inserting"]) - expect(product_intelligence).not_to receive(:warn) + expect(analytics_instrumentation).not_to receive(:warn) - product_intelligence.check_usage_data_insertions! + analytics_instrumentation.check_usage_data_insertions! end end end diff --git a/spec/tooling/danger/database_dictionary_spec.rb b/spec/tooling/danger/database_dictionary_spec.rb new file mode 100644 index 00000000000..1a771a6cec0 --- /dev/null +++ b/spec/tooling/danger/database_dictionary_spec.rb @@ -0,0 +1,152 @@ +# frozen_string_literal: true + +require 'gitlab-dangerfiles' +require 'gitlab/dangerfiles/spec_helper' + +require_relative '../../../tooling/danger/database_dictionary' + +RSpec.describe Tooling::Danger::DatabaseDictionary, feature_category: :shared do + include_context "with dangerfile" + + let(:fake_danger) { DangerSpecHelper.fake_danger.include(described_class) } + + subject(:database_dictionary) { fake_danger.new(helper: fake_helper) } + + describe '#database_dictionary_files' do + let(:database_dictionary_files) do + [ + 'db/docs/ci_pipelines.yml', + 'db/docs/projects.yml' + ] + end + + let(:other_files) do + [ + 'app/models/model.rb', + 'app/assets/javascripts/file.js' + ] + end + + shared_examples 'an array of Found objects' do |change_type| + it 'returns an array of Found objects' do + expect(database_dictionary.database_dictionary_files(change_type: change_type)) + .to contain_exactly( + an_instance_of(described_class::Found), + an_instance_of(described_class::Found) + ) + + expect(database_dictionary.database_dictionary_files(change_type: change_type).map(&:path)) + .to eq(database_dictionary_files) + end + end + + shared_examples 'an empty array' do |change_type| + it 'returns an array of Found objects' do + expect(database_dictionary.database_dictionary_files(change_type: change_type)).to be_empty + end + end + + describe 'retrieves added database dictionary files' do + context 'with added added database dictionary files' do + let(:added_files) { database_dictionary_files } + + include_examples 'an array of Found objects', :added + end + + context 'without added added database dictionary files' do + let(:added_files) { other_files } + + include_examples 'an empty array', :added + end + end + + describe 'retrieves modified database dictionary files' do + context 'with modified modified database dictionary files' do + let(:modified_files) { database_dictionary_files } + + include_examples 'an array of Found objects', :modified + end + + context 'without modified modified database dictionary files' do + let(:modified_files) { other_files } + + include_examples 'an empty array', :modified + end + end + + describe 'retrieves deleted database dictionary files' do + context 'with deleted deleted database dictionary files' do + let(:deleted_files) { database_dictionary_files } + + include_examples 'an array of Found objects', :deleted + end + + context 'without deleted deleted database dictionary files' do + let(:deleted_files) { other_files } + + include_examples 'an empty array', :deleted + end + end + end + + describe described_class::Found do + let(:database_dictionary_path) { 'db/docs/ci_pipelines.yml' } + let(:gitlab_schema) { 'gitlab_ci' } + + let(:yaml) do + { + + 'table_name' => 'ci_pipelines', + 'classes' => ['Ci::Pipeline'], + 'feature_categories' => ['continuous_integration'], + 'description' => 'TODO', + 'introduced_by_url' => 'https://gitlab.com/gitlab-org/gitlab/-/commit/c6ae290cea4b88ecaa9cfe0bc9d88e8fd32070c1', + 'milestone' => '9.0', + 'gitlab_schema' => gitlab_schema + } + end + + let(:raw_yaml) { YAML.dump(yaml) } + + subject(:found) { described_class.new(database_dictionary_path) } + + before do + allow(File).to receive(:read).and_call_original + allow(File).to receive(:read).with(database_dictionary_path).and_return(raw_yaml) + end + + described_class::ATTRIBUTES.each do |attribute| + describe "##{attribute}" do + it 'returns value from the YAML' do + expect(found.public_send(attribute)).to eq(yaml[attribute]) + end + end + end + + describe '#raw' do + it 'returns the raw YAML' do + expect(found.raw).to eq(raw_yaml) + end + end + + describe '#ci_schema?' do + it { expect(found.ci_schema?).to be_truthy } + + context 'with main schema' do + let(:gitlab_schema) { 'gitlab_main' } + + it { expect(found.ci_schema?).to be_falsey } + end + end + + describe '#main_schema?' do + it { expect(found.main_schema?).to be_falsey } + + context 'with main schema' do + let(:gitlab_schema) { 'gitlab_main' } + + it { expect(found.main_schema?).to be_truthy } + end + end + end +end diff --git a/spec/tooling/danger/feature_flag_spec.rb b/spec/tooling/danger/feature_flag_spec.rb index 4575d8ca981..f4df2e1226c 100644 --- a/spec/tooling/danger/feature_flag_spec.rb +++ b/spec/tooling/danger/feature_flag_spec.rb @@ -83,6 +83,28 @@ RSpec.describe Tooling::Danger::FeatureFlag do end end + describe '#stage_label' do + before do + allow(fake_helper).to receive(:mr_labels).and_return(labels) + end + + context 'when there is no stage label' do + let(:labels) { [] } + + it 'returns nil' do + expect(feature_flag.stage_label).to be_nil + end + end + + context 'when there is a stage label' do + let(:labels) { ['devops::verify', 'group::pipeline execution'] } + + it 'returns the stage label' do + expect(feature_flag.stage_label).to eq(labels.first) + end + end + end + describe described_class::Found do let(:feature_flag_path) { 'config/feature_flags/development/entry.yml' } let(:group) { 'group::source code' } diff --git a/spec/tooling/danger/multiversion_spec.rb b/spec/tooling/danger/multiversion_spec.rb new file mode 100644 index 00000000000..90edad61d47 --- /dev/null +++ b/spec/tooling/danger/multiversion_spec.rb @@ -0,0 +1,79 @@ +# frozen_string_literal: true + +require 'rspec-parameterized' +require 'gitlab-dangerfiles' +require 'gitlab/dangerfiles/spec_helper' + +require_relative '../../../tooling/danger/multiversion' +require_relative '../../../tooling/danger/project_helper' + +RSpec.describe Tooling::Danger::Multiversion, feature_category: :shared do + include_context "with dangerfile" + + subject(:multiversion) { fake_danger.new(helper: fake_helper, git: fake_git) } + + let(:fake_danger) { DangerSpecHelper.fake_danger.include(described_class) } + let(:ci_env) { true } + + before do + allow(fake_helper).to receive(:ci?).and_return(ci_env) + allow(fake_git).to receive(:modified_files).and_return(modified_files) + allow(fake_git).to receive(:added_files).and_return(added_files) + end + + describe '#check!' do + using RSpec::Parameterized::TableSyntax + + context 'when not in ci environment' do + let(:ci_env) { false } + + it 'does not add the warning markdown section' do + expect(multiversion).not_to receive(:markdown) + + multiversion.check! + end + end + + context 'when GraphQL API and frontend assets have not been simultaneously updated' do + where(:modified_files, :added_files) do + %w[app/assets/helloworld.vue] | %w[] + %w[app/assets/helloworld.vue] | %w[app/type.rb] + %w[app/assets/helloworld.js] | %w[app/graphql.rb] + %w[app/assets/helloworld.graphql] | %w[app/models/graphql.rb] + %w[] | %w[app/graphql/type.rb] + %w[app/vue.txt] | %w[app/graphql/type.rb] + %w[app/views/foo.haml] | %w[app/graphql/type.rb] + %w[foo] | %w[] + %w[] | %w[] + end + + with_them do + it 'does not add the warning markdown section' do + expect(multiversion).not_to receive(:markdown) + + multiversion.check! + end + end + end + + context 'when GraphQL API and frontend assets have been simultaneously updated' do + where(:modified_files, :added_files) do + %w[app/assets/helloworld.vue] | %w[app/graphql/type.rb] + %w[app/assets/helloworld.vue] | %w[app/graphql/type.rb] + %w[app/assets/helloworld.js] | %w[app/graphql/type.rb] + %w[ee/app/assets/helloworld.js] | %w[app/graphql/type.rb] + %w[app/assets/helloworld.graphql] | %w[ee/app/graphql/type.rb] + %w[ee/app/assets/helloworld.graphql] | %w[ee/app/graphql/type.rb] + %w[ee/app/assets/helloworld.graphql] | %w[jh/app/graphql/type.rb] + end + + with_them do + it 'adds the warning markdown section' do + expect(multiversion).to receive(:markdown) + + multiversion.check! + end + end + end + end +end diff --git a/spec/tooling/danger/project_helper_spec.rb b/spec/tooling/danger/project_helper_spec.rb index 48050649f54..898c0ffa10c 100644 --- a/spec/tooling/danger/project_helper_spec.rb +++ b/spec/tooling/danger/project_helper_spec.rb @@ -39,7 +39,7 @@ RSpec.describe Tooling::Danger::ProjectHelper do 'doc/api/openapi/openapi.yaml' | [:docs, :backend] 'doc/api/openapi/any_other_file.yaml' | [:docs, :backend] - 'usage_data.rb' | [:database, :backend, :product_intelligence] + 'usage_data.rb' | [:database, :backend, :analytics_instrumentation] 'doc/foo.md' | [:docs] 'CONTRIBUTING.md' | [:docs] 'LICENSE' | [:docs] @@ -178,75 +178,75 @@ RSpec.describe Tooling::Danger::ProjectHelper do 'foo/bar.txt' | [:none] 'foo/bar.md' | [:none] - 'ee/config/metrics/counts_7d/20210216174919_g_analytics_issues_weekly.yml' | [:product_intelligence] - 'lib/gitlab/usage_data_counters/aggregated_metrics/common.yml' | [:product_intelligence] - 'lib/gitlab/usage_data_counters/hll_redis_counter.rb' | [:backend, :product_intelligence] - 'lib/gitlab/tracking.rb' | [:backend, :product_intelligence] - 'lib/gitlab/usage/service_ping_report.rb' | [:backend, :product_intelligence] - 'lib/gitlab/usage/metrics/key_path_processor.rb' | [:backend, :product_intelligence] - 'spec/lib/gitlab/tracking_spec.rb' | [:backend, :product_intelligence] - 'app/helpers/tracking_helper.rb' | [:backend, :product_intelligence] - 'spec/helpers/tracking_helper_spec.rb' | [:backend, :product_intelligence] - 'lib/generators/rails/usage_metric_definition_generator.rb' | [:backend, :product_intelligence] - 'spec/lib/generators/usage_metric_definition_generator_spec.rb' | [:backend, :product_intelligence] - 'config/metrics/schema.json' | [:product_intelligence] - 'app/assets/javascripts/tracking/foo.js' | [:frontend, :product_intelligence] - 'spec/frontend/tracking/foo.js' | [:frontend, :product_intelligence] - 'spec/frontend/tracking_spec.js' | [:frontend, :product_intelligence] + 'ee/config/metrics/counts_7d/20210216174919_g_analytics_issues_weekly.yml' | [:analytics_instrumentation] + 'lib/gitlab/usage_data_counters/aggregated_metrics/common.yml' | [:analytics_instrumentation] + 'lib/gitlab/usage_data_counters/hll_redis_counter.rb' | [:backend, :analytics_instrumentation] + 'lib/gitlab/tracking.rb' | [:backend, :analytics_instrumentation] + 'lib/gitlab/usage/service_ping_report.rb' | [:backend, :analytics_instrumentation] + 'lib/gitlab/usage/metrics/key_path_processor.rb' | [:backend, :analytics_instrumentation] + 'spec/lib/gitlab/tracking_spec.rb' | [:backend, :analytics_instrumentation] + 'app/helpers/tracking_helper.rb' | [:backend, :analytics_instrumentation] + 'spec/helpers/tracking_helper_spec.rb' | [:backend, :analytics_instrumentation] + 'lib/generators/rails/usage_metric_definition_generator.rb' | [:backend, :analytics_instrumentation] + 'spec/lib/generators/usage_metric_definition_generator_spec.rb' | [:backend, :analytics_instrumentation] + 'config/metrics/schema.json' | [:analytics_instrumentation] + 'app/assets/javascripts/tracking/foo.js' | [:frontend, :analytics_instrumentation] + 'spec/frontend/tracking/foo.js' | [:frontend, :analytics_instrumentation] + 'spec/frontend/tracking_spec.js' | [:frontend, :analytics_instrumentation] 'lib/gitlab/usage_database/foo.rb' | [:backend] - 'config/metrics/counts_7d/test_metric.yml' | [:product_intelligence] - 'config/events/snowplow_event.yml' | [:product_intelligence] - 'config/metrics/schema.json' | [:product_intelligence] - 'doc/api/usage_data.md' | [:product_intelligence] - 'spec/lib/gitlab/usage_data_spec.rb' | [:product_intelligence] - 'spec/lib/gitlab/usage/service_ping_report.rb' | [:backend, :product_intelligence] - 'spec/lib/gitlab/usage/metrics/key_path_processor.rb' | [:backend, :product_intelligence] - - 'app/models/integration.rb' | [:integrations_be, :backend] - 'ee/app/models/integrations/github.rb' | [:integrations_be, :backend] - 'ee/app/models/ee/integrations/jira.rb' | [:integrations_be, :backend] - 'app/models/integrations/chat_message/pipeline_message.rb' | [:integrations_be, :backend] - 'app/models/jira_connect_subscription.rb' | [:integrations_be, :backend] - 'app/models/hooks/service_hook.rb' | [:integrations_be, :backend] - 'ee/app/models/ee/hooks/system_hook.rb' | [:integrations_be, :backend] - 'app/services/concerns/integrations/project_test_data.rb' | [:integrations_be, :backend] - 'ee/app/services/ee/integrations/test/project_service.rb' | [:integrations_be, :backend] - 'app/controllers/concerns/integrations/actions.rb' | [:integrations_be, :backend] - 'ee/app/controllers/concerns/ee/integrations/params.rb' | [:integrations_be, :backend] - 'ee/app/controllers/projects/integrations/jira/issues_controller.rb' | [:integrations_be, :backend] - 'app/controllers/projects/hooks_controller.rb' | [:integrations_be, :backend] - 'app/controllers/admin/hook_logs_controller.rb' | [:integrations_be, :backend] - 'app/controllers/groups/settings/integrations_controller.rb' | [:integrations_be, :backend] - 'app/controllers/jira_connect/branches_controller.rb' | [:integrations_be, :backend] - 'app/controllers/oauth/jira/authorizations_controller.rb' | [:integrations_be, :backend] - 'ee/app/finders/projects/integrations/jira/by_ids_finder.rb' | [:integrations_be, :database, :backend] - 'app/workers/jira_connect/sync_merge_request_worker.rb' | [:integrations_be, :backend] - 'app/workers/propagate_integration_inherit_worker.rb' | [:integrations_be, :backend] - 'app/workers/web_hooks/log_execution_worker.rb' | [:integrations_be, :backend] - 'app/workers/web_hook_worker.rb' | [:integrations_be, :backend] - 'app/workers/project_service_worker.rb' | [:integrations_be, :backend] - 'lib/atlassian/jira_connect/serializers/commit_entity.rb' | [:integrations_be, :backend] - 'lib/api/entities/project_integration.rb' | [:integrations_be, :backend] - 'lib/gitlab/hook_data/note_builder.rb' | [:integrations_be, :backend] - 'lib/gitlab/data_builder/note.rb' | [:integrations_be, :backend] - 'lib/gitlab/web_hooks/recursion_detection.rb' | [:integrations_be, :backend] - 'ee/lib/ee/gitlab/integrations/sti_type.rb' | [:integrations_be, :backend] - 'ee/lib/ee/api/helpers/integrations_helpers.rb' | [:integrations_be, :backend] - 'ee/app/serializers/integrations/jira_serializers/issue_entity.rb' | [:integrations_be, :backend] - 'app/serializers/jira_connect/app_data_serializer.rb' | [:integrations_be, :backend] - 'lib/api/github/entities.rb' | [:integrations_be, :backend] - 'lib/api/v3/github.rb' | [:integrations_be, :backend] + 'config/metrics/counts_7d/test_metric.yml' | [:analytics_instrumentation] + 'config/events/snowplow_event.yml' | [:analytics_instrumentation] + 'config/metrics/schema.json' | [:analytics_instrumentation] + 'doc/api/usage_data.md' | [:analytics_instrumentation] + 'spec/lib/gitlab/usage_data_spec.rb' | [:analytics_instrumentation] + 'spec/lib/gitlab/usage/service_ping_report.rb' | [:backend, :analytics_instrumentation] + 'spec/lib/gitlab/usage/metrics/key_path_processor.rb' | [:backend, :analytics_instrumentation] + + 'app/models/integration.rb' | [:import_integrate_be, :backend] + 'ee/app/models/integrations/github.rb' | [:import_integrate_be, :backend] + 'ee/app/models/ee/integrations/jira.rb' | [:import_integrate_be, :backend] + 'app/models/integrations/chat_message/pipeline_message.rb' | [:import_integrate_be, :backend] + 'app/models/jira_connect_subscription.rb' | [:import_integrate_be, :backend] + 'app/models/hooks/service_hook.rb' | [:import_integrate_be, :backend] + 'ee/app/models/ee/hooks/system_hook.rb' | [:import_integrate_be, :backend] + 'app/services/concerns/integrations/project_test_data.rb' | [:import_integrate_be, :backend] + 'ee/app/services/ee/integrations/test/project_service.rb' | [:import_integrate_be, :backend] + 'app/controllers/concerns/integrations/actions.rb' | [:import_integrate_be, :backend] + 'ee/app/controllers/concerns/ee/integrations/params.rb' | [:import_integrate_be, :backend] + 'ee/app/controllers/projects/integrations/jira/issues_controller.rb' | [:import_integrate_be, :backend] + 'app/controllers/projects/hooks_controller.rb' | [:import_integrate_be, :backend] + 'app/controllers/admin/hook_logs_controller.rb' | [:import_integrate_be, :backend] + 'app/controllers/groups/settings/integrations_controller.rb' | [:import_integrate_be, :backend] + 'app/controllers/jira_connect/branches_controller.rb' | [:import_integrate_be, :backend] + 'app/controllers/oauth/jira/authorizations_controller.rb' | [:import_integrate_be, :backend] + 'ee/app/finders/projects/integrations/jira/by_ids_finder.rb' | [:import_integrate_be, :database, :backend] + 'app/workers/jira_connect/sync_merge_request_worker.rb' | [:import_integrate_be, :backend] + 'app/workers/propagate_integration_inherit_worker.rb' | [:import_integrate_be, :backend] + 'app/workers/web_hooks/log_execution_worker.rb' | [:import_integrate_be, :backend] + 'app/workers/web_hook_worker.rb' | [:import_integrate_be, :backend] + 'app/workers/project_service_worker.rb' | [:import_integrate_be, :backend] + 'lib/atlassian/jira_connect/serializers/commit_entity.rb' | [:import_integrate_be, :backend] + 'lib/api/entities/project_integration.rb' | [:import_integrate_be, :backend] + 'lib/gitlab/hook_data/note_builder.rb' | [:import_integrate_be, :backend] + 'lib/gitlab/data_builder/note.rb' | [:import_integrate_be, :backend] + 'lib/gitlab/web_hooks/recursion_detection.rb' | [:import_integrate_be, :backend] + 'ee/lib/ee/gitlab/integrations/sti_type.rb' | [:import_integrate_be, :backend] + 'ee/lib/ee/api/helpers/integrations_helpers.rb' | [:import_integrate_be, :backend] + 'ee/app/serializers/integrations/jira_serializers/issue_entity.rb' | [:import_integrate_be, :backend] + 'app/serializers/jira_connect/app_data_serializer.rb' | [:import_integrate_be, :backend] + 'lib/api/github/entities.rb' | [:import_integrate_be, :backend] + 'lib/api/v3/github.rb' | [:import_integrate_be, :backend] 'app/controllers/clusters/integrations_controller.rb' | [:backend] 'app/services/clusters/integrations/prometheus_health_check_service.rb' | [:backend] 'app/graphql/types/alert_management/integration_type.rb' | [:backend] - 'app/views/jira_connect/branches/new.html.haml' | [:integrations_fe, :frontend] - 'app/views/layouts/jira_connect.html.haml' | [:integrations_fe, :frontend] - 'app/assets/javascripts/jira_connect/branches/pages/index.vue' | [:integrations_fe, :frontend] - 'ee/app/views/projects/integrations/jira/issues/show.html.haml' | [:integrations_fe, :frontend] - 'ee/app/assets/javascripts/integrations/zentao/issues_list/graphql/queries/get_zentao_issues.query.graphql' | [:integrations_fe, :frontend] - 'app/assets/javascripts/pages/projects/settings/integrations/show/index.js' | [:integrations_fe, :frontend] - 'ee/app/assets/javascripts/pages/groups/hooks/index.js' | [:integrations_fe, :frontend] + 'app/views/jira_connect/branches/new.html.haml' | [:import_integrate_fe, :frontend] + 'app/views/layouts/jira_connect.html.haml' | [:import_integrate_fe, :frontend] + 'app/assets/javascripts/jira_connect/branches/pages/index.vue' | [:import_integrate_fe, :frontend] + 'ee/app/views/projects/integrations/jira/issues/show.html.haml' | [:import_integrate_fe, :frontend] + 'ee/app/assets/javascripts/integrations/zentao/issues_list/graphql/queries/get_zentao_issues.query.graphql' | [:import_integrate_fe, :frontend] + 'app/assets/javascripts/pages/projects/settings/integrations/show/index.js' | [:import_integrate_fe, :frontend] + 'ee/app/assets/javascripts/pages/groups/hooks/index.js' | [:import_integrate_fe, :frontend] 'app/views/clusters/clusters/_integrations_tab.html.haml' | [:frontend, :backend] 'app/assets/javascripts/alerts_settings/graphql/fragments/integration_item.fragment.graphql' | [:frontend] 'app/assets/javascripts/filtered_search/droplab/hook_input.js' | [:frontend] @@ -263,22 +263,22 @@ RSpec.describe Tooling::Danger::ProjectHelper do context 'having specific changes' do where(:expected_categories, :patch, :changed_files) do - [:product_intelligence] | '+data-track-action' | ['components/welcome.vue'] - [:product_intelligence] | '+ data: { track_label:' | ['admin/groups/_form.html.haml'] - [:product_intelligence] | '+ Gitlab::Tracking.event' | ['dashboard/todos_controller.rb', 'admin/groups/_form.html.haml'] - [:database, :backend, :product_intelligence] | '+ count(User.active)' | ['usage_data.rb', 'lib/gitlab/usage_data.rb', 'ee/lib/ee/gitlab/usage_data.rb'] - [:database, :backend, :product_intelligence] | '+ estimate_batch_distinct_count(User.active)' | ['usage_data.rb'] - [:backend, :product_intelligence] | '+ alt_usage_data(User.active)' | ['lib/gitlab/usage_data.rb'] - [:backend, :product_intelligence] | '+ count(User.active)' | ['lib/gitlab/usage_data/topology.rb'] - [:backend, :product_intelligence] | '+ foo_count(User.active)' | ['lib/gitlab/usage_data.rb'] - [:backend] | '+ count(User.active)' | ['user.rb'] - [:integrations_be, :database, :migration] | '+ add_column :integrations, :foo, :text' | ['db/migrate/foo.rb'] - [:integrations_be, :database, :migration] | '+ create_table :zentao_tracker_data do |t|' | ['ee/db/post_migrate/foo.rb'] - [:integrations_be, :backend] | '+ Integrations::Foo' | ['app/foo/bar.rb'] - [:integrations_be, :backend] | '+ project.execute_hooks(foo, :bar)' | ['ee/lib/ee/foo.rb'] - [:integrations_be, :backend] | '+ project.execute_integrations(foo, :bar)' | ['app/foo.rb'] - [:frontend, :product_intelligence] | '+ api.trackRedisCounterEvent("foo")' | ['app/assets/javascripts/telemetry.js', 'ee/app/assets/javascripts/mr_widget.vue'] - [:frontend, :product_intelligence] | '+ api.trackRedisHllUserEvent("bar")' | ['app/assets/javascripts/telemetry.js', 'ee/app/assets/javascripts/mr_widget.vue'] + [:analytics_instrumentation] | '+data-track-action' | ['components/welcome.vue'] + [:analytics_instrumentation] | '+ data: { track_label:' | ['admin/groups/_form.html.haml'] + [:analytics_instrumentation] | '+ Gitlab::Tracking.event' | ['dashboard/todos_controller.rb', 'admin/groups/_form.html.haml'] + [:database, :backend, :analytics_instrumentation] | '+ count(User.active)' | ['usage_data.rb', 'lib/gitlab/usage_data.rb', 'ee/lib/ee/gitlab/usage_data.rb'] + [:database, :backend, :analytics_instrumentation] | '+ estimate_batch_distinct_count(User.active)' | ['usage_data.rb'] + [:backend, :analytics_instrumentation] | '+ alt_usage_data(User.active)' | ['lib/gitlab/usage_data.rb'] + [:backend, :analytics_instrumentation] | '+ count(User.active)' | ['lib/gitlab/usage_data/topology.rb'] + [:backend, :analytics_instrumentation] | '+ foo_count(User.active)' | ['lib/gitlab/usage_data.rb'] + [:backend] | '+ count(User.active)' | ['user.rb'] + [:import_integrate_be, :database, :migration] | '+ add_column :integrations, :foo, :text' | ['db/migrate/foo.rb'] + [:import_integrate_be, :database, :migration] | '+ create_table :zentao_tracker_data do |t|' | ['ee/db/post_migrate/foo.rb'] + [:import_integrate_be, :backend] | '+ Integrations::Foo' | ['app/foo/bar.rb'] + [:import_integrate_be, :backend] | '+ project.execute_hooks(foo, :bar)' | ['ee/lib/ee/foo.rb'] + [:import_integrate_be, :backend] | '+ project.execute_integrations(foo, :bar)' | ['app/foo.rb'] + [:frontend, :analytics_instrumentation] | '+ api.trackRedisCounterEvent("foo")' | ['app/assets/javascripts/telemetry.js', 'ee/app/assets/javascripts/mr_widget.vue'] + [:frontend, :analytics_instrumentation] | '+ api.trackRedisHllUserEvent("bar")' | ['app/assets/javascripts/telemetry.js', 'ee/app/assets/javascripts/mr_widget.vue'] end with_them do diff --git a/spec/tooling/danger/sidekiq_args_spec.rb b/spec/tooling/danger/sidekiq_args_spec.rb new file mode 100644 index 00000000000..bfa9ef169de --- /dev/null +++ b/spec/tooling/danger/sidekiq_args_spec.rb @@ -0,0 +1,125 @@ +# frozen_string_literal: true + +require 'rspec-parameterized' +require 'gitlab-dangerfiles' +require 'danger' +require 'danger/plugins/internal/helper' +require 'gitlab/dangerfiles/spec_helper' + +require_relative '../../../tooling/danger/sidekiq_args' +require_relative '../../../tooling/danger/project_helper' + +RSpec.describe Tooling::Danger::SidekiqArgs, feature_category: :tooling do + include_context "with dangerfile" + + let(:fake_danger) { DangerSpecHelper.fake_danger.include(described_class) } + let(:fake_project_helper) { Tooling::Danger::ProjectHelper } + + subject(:specs) { fake_danger.new(helper: fake_helper) } + + before do + allow(specs).to receive(:project_helper).and_return(fake_project_helper) + end + + describe '#args_changed?' do + using RSpec::Parameterized::TableSyntax + + where(:before, :after, :result) do + " - def perform" | " + def perform(abc)" | true + " - def perform" | " + def perform(abc)" | true + " - def perform(abc)" | " + def perform(def)" | true + " - def perform(abc, def)" | " + def perform(abc)" | true + " - def perform(abc, def)" | " + def perform(def, abc)" | true + " - def perform" | " - def perform" | false + " + def perform" | " + def perform" | false + " - def perform(abc)" | " - def perform(abc)" | false + " + def perform(abc)" | " + def perform(abc)" | false + " - def perform(abc)" | " + def perform_foo(abc)" | false + end + + with_them do + it 'returns correct result' do + expect(specs.args_changed?([before, after])).to eq(result) + end + end + end + + describe '#add_comment_for_matched_line' do + let(:filename) { 'app/workers/hello_worker.rb' } + let(:file_lines) do + [ + "Module Worker", + " def perform", + " puts hello world", + " end", + "end" + ] + end + + before do + allow(specs.project_helper).to receive(:file_lines).and_return(file_lines) + end + + context 'when args are changed' do + before do + allow(specs.helper).to receive(:changed_lines).and_return([" - def perform", " + def perform(abc)"]) + allow(specs).to receive(:args_changed?).and_return(true) + end + + it 'adds suggestion at the correct lines' do + expect(specs).to receive(:markdown).with(format(described_class::SUGGEST_MR_COMMENT), file: filename, line: 2) + + specs.add_comment_for_matched_line(filename) + end + end + + context 'when args are not changed' do + before do + allow(specs.helper).to receive(:changed_lines).and_return([" - def perform", " - def perform"]) + allow(specs).to receive(:args_changed?).and_return(false) + end + + it 'does not add suggestion' do + expect(specs).not_to receive(:markdown) + + specs.add_comment_for_matched_line(filename) + end + end + end + + describe '#changed_worker_files' do + let(:base_expected_files) { %w[app/workers/a.rb app/workers/b.rb ee/app/workers/e.rb] } + + before do + all_changed_files = %w[ + app/workers/a.rb + app/workers/b.rb + ee/app/workers/e.rb + spec/foo_spec.rb + ee/spec/foo_spec.rb + spec/bar_spec.rb + ee/spec/bar_spec.rb + spec/zab_spec.rb + ee/spec/zab_spec.rb + ] + + allow(specs.helper).to receive(:all_changed_files).and_return(all_changed_files) + end + + it 'returns added, modified, and renamed_after files by default' do + expect(specs.changed_worker_files).to match_array(base_expected_files) + end + + context 'with include_ee: :exclude' do + it 'returns spec files without EE-specific files' do + expect(specs.changed_worker_files(ee: :exclude)).not_to include(%w[ee/app/workers/e.rb]) + end + end + + context 'with include_ee: :only' do + it 'returns EE-specific spec files only' do + expect(specs.changed_worker_files(ee: :only)).to match_array(%w[ee/app/workers/e.rb]) + end + end + end +end diff --git a/spec/tooling/danger/specs/feature_category_suggestion_spec.rb b/spec/tooling/danger/specs/feature_category_suggestion_spec.rb new file mode 100644 index 00000000000..87eb20e5e50 --- /dev/null +++ b/spec/tooling/danger/specs/feature_category_suggestion_spec.rb @@ -0,0 +1,99 @@ +# frozen_string_literal: true + +require 'gitlab/dangerfiles/spec_helper' + +require_relative '../../../../tooling/danger/specs' +require_relative '../../../../tooling/danger/project_helper' + +RSpec.describe Tooling::Danger::Specs::FeatureCategorySuggestion, feature_category: :tooling do + include_context "with dangerfile" + + let(:fake_danger) { DangerSpecHelper.fake_danger.include(Tooling::Danger::Specs) } + let(:fake_project_helper) { instance_double('Tooling::Danger::ProjectHelper') } + let(:filename) { 'spec/foo_spec.rb' } + + let(:template) do + <<~SUGGESTION_MARKDOWN.chomp + ```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::SummaryController, feature_category: :team_planning do", + " end", + "RSpec.describe Projects::SummaryController do", + " let_it_be(:user) { create(:user) }", + " end", + " describe 'GET \"time_summary\"' do", + " end", + " RSpec.describe Projects::SummaryController do", + " let_it_be(:user) { create(:user) }", + " end", + " describe 'GET \"time_summary\"' do", + " end", + " \n", + "RSpec.describe Projects :aggregate_failures,", + " feature_category :team_planning do", + " \n", + "RSpec.describe Epics :aggregate_failures,", + " ee: true do", + "\n", + "RSpec.describe Issues :aggregate_failures,", + " feature_category: :team_planning do", + "\n", + "RSpec.describe MergeRequest :aggregate_failures,", + " :js,", + " feature_category: :team_planning do" + ] + end + + let(:changed_lines) do + [ + "+ RSpec.describe Projects::SummaryController, feature_category: :team_planning do", + "+RSpec.describe Projects::SummaryController do", + "+ let_it_be(:user) { create(:user) }", + "- end", + "+ describe 'GET \"time_summary\"' do", + "+ RSpec.describe Projects::SummaryController do", + "+RSpec.describe Projects :aggregate_failures,", + "+ feature_category: :team_planning do", + "+RSpec.describe Epics :aggregate_failures,", + "+ ee: true do", + "+RSpec.describe Issues :aggregate_failures,", + "+RSpec.describe MergeRequest :aggregate_failures,", + "+ :js,", + "+ feature_category: :team_planning do", + "+RSpec.describe 'line in commit diff but no longer in working copy' do" + ] + end + + subject(:specs) { fake_danger.new(helper: fake_helper) } + + before do + allow(specs).to receive(:project_helper).and_return(fake_project_helper) + allow(specs.helper).to receive(:changed_lines).with(filename).and_return(changed_lines) + allow(specs.project_helper).to receive(:file_lines).and_return(file_lines) + end + + it 'adds suggestions at the correct lines', :aggregate_failures do + [ + { suggested_line: "RSpec.describe Projects::SummaryController do", number: 5 }, + { suggested_line: " RSpec.describe Projects::SummaryController do", number: 10 }, + { suggested_line: "RSpec.describe Epics :aggregate_failures,", number: 19 } + + ].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(filename) + end +end diff --git a/spec/tooling/danger/specs/match_with_array_suggestion_spec.rb b/spec/tooling/danger/specs/match_with_array_suggestion_spec.rb new file mode 100644 index 00000000000..b065772a09b --- /dev/null +++ b/spec/tooling/danger/specs/match_with_array_suggestion_spec.rb @@ -0,0 +1,99 @@ +# frozen_string_literal: true + +require 'gitlab/dangerfiles/spec_helper' + +require_relative '../../../../tooling/danger/specs' +require_relative '../../../../tooling/danger/project_helper' + +RSpec.describe Tooling::Danger::Specs::MatchWithArraySuggestion, feature_category: :tooling do + include_context "with dangerfile" + + let(:fake_danger) { DangerSpecHelper.fake_danger.include(Tooling::Danger::Specs) } + let(:fake_project_helper) { instance_double('Tooling::Danger::ProjectHelper') } + let(:filename) { 'spec/foo_spec.rb' } + + let(:file_lines) do + [ + " describe 'foo' do", + " expect(foo).to match(['bar', 'baz'])", + " end", + " expect(foo).to match(['bar', 'baz'])", # same line as line 1 above, we expect two different suggestions + " ", + " expect(foo).to match ['bar', 'baz']", + " expect(foo).to eq(['bar', 'baz'])", + " expect(foo).to eq ['bar', 'baz']", + " expect(foo).to(match(['bar', 'baz']))", + " expect(foo).to(eq(['bar', 'baz']))", + " expect(foo).to(eq([bar, baz]))", + " expect(foo).to(eq(['bar']))", + " foo.eq(['bar'])" + ] + end + + let(:matching_lines) do + [ + "+ expect(foo).to match(['should not error'])", + "+ expect(foo).to match(['bar', 'baz'])", + "+ expect(foo).to match(['bar', 'baz'])", + "+ expect(foo).to match ['bar', 'baz']", + "+ expect(foo).to eq(['bar', 'baz'])", + "+ expect(foo).to eq ['bar', 'baz']", + "+ expect(foo).to(match(['bar', 'baz']))", + "+ expect(foo).to(eq(['bar', 'baz']))", + "+ expect(foo).to(eq([bar, baz]))" + ] + end + + let(:changed_lines) do + [ + " expect(foo).to match(['bar', 'baz'])", + " expect(foo).to match(['bar', 'baz'])", + " expect(foo).to match ['bar', 'baz']", + " expect(foo).to eq(['bar', 'baz'])", + " expect(foo).to eq ['bar', 'baz']", + "- expect(foo).to match(['bar', 'baz'])", + "- expect(foo).to match(['bar', 'baz'])", + "- expect(foo).to match ['bar', 'baz']", + "- expect(foo).to eq(['bar', 'baz'])", + "- expect(foo).to eq ['bar', 'baz']", + "- expect(foo).to eq [bar, foo]", + "+ expect(foo).to eq([])" + ] + matching_lines + end + + let(:template) do + <<~MARKDOWN.chomp + ```suggestion + %<suggested_line>s + ``` + + If order of the result is not important, please consider using `match_array` to avoid flakiness. + MARKDOWN + end + + subject(:specs) { fake_danger.new(helper: fake_helper) } + + before do + allow(specs).to receive(:project_helper).and_return(fake_project_helper) + allow(specs.helper).to receive(:changed_lines).with(filename).and_return(changed_lines) + allow(specs.project_helper).to receive(:file_lines).and_return(file_lines) + end + + it 'adds suggestions at the correct lines' do + [ + { suggested_line: " expect(foo).to match_array(['bar', 'baz'])", number: 2 }, + { suggested_line: " expect(foo).to match_array(['bar', 'baz'])", number: 4 }, + { suggested_line: " expect(foo).to match_array ['bar', 'baz']", number: 6 }, + { suggested_line: " expect(foo).to match_array(['bar', 'baz'])", number: 7 }, + { suggested_line: " expect(foo).to match_array ['bar', 'baz']", number: 8 }, + { suggested_line: " expect(foo).to(match_array(['bar', 'baz']))", number: 9 }, + { suggested_line: " expect(foo).to(match_array(['bar', 'baz']))", number: 10 }, + { suggested_line: " expect(foo).to(match_array([bar, baz]))", number: 11 } + ].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(filename) + end +end diff --git a/spec/tooling/danger/specs/project_factory_suggestion_spec.rb b/spec/tooling/danger/specs/project_factory_suggestion_spec.rb new file mode 100644 index 00000000000..9b10ab1a6f4 --- /dev/null +++ b/spec/tooling/danger/specs/project_factory_suggestion_spec.rb @@ -0,0 +1,112 @@ +# frozen_string_literal: true + +require 'gitlab/dangerfiles/spec_helper' + +require_relative '../../../../tooling/danger/specs' +require_relative '../../../../tooling/danger/project_helper' + +RSpec.describe Tooling::Danger::Specs::ProjectFactorySuggestion, feature_category: :tooling do + include_context "with dangerfile" + + let(:fake_danger) { DangerSpecHelper.fake_danger.include(Tooling::Danger::Specs) } + let(:fake_project_helper) { instance_double('Tooling::Danger::ProjectHelper') } + let(:filename) { 'spec/foo_spec.rb' } + + let(:template) do + <<~MARKDOWN.chomp + ```suggestion + %<suggested_line>s + ``` + + Project creations are very slow. Using `let_it_be`, `build` or `build_stubbed` can improve test performance. + + Warning: `let_it_be` may not be suitable if your test modifies data as this could result in state leaks! + + In those cases, please use `let_it_be_with_reload` or `let_it_be_with_refind` instead. + + If your are unsure which is the right method to use, + please refer to [testing best practices](https://docs.gitlab.com/ee/development/testing_guide/best_practices.html#optimize-factory-usage) + for background information and alternative options for optimizing factory usage. + + Feel free to ignore this comment if you know `let` or `let!` are the better options and/or worry about causing state leaks. + MARKDOWN + end + + let(:file_lines) do + [ + " let(:project) { create(:project) }", + " let_it_be(:project) { create(:project, :repository)", + " let!(:project) { create(:project) }", + " let(:var) { create(:project) }", + " let(:merge_request) { create(:merge_request, project: project)", + " context 'when merge request exists' do", + " it { is_expected.to be_success }", + " end", + " let!(:var) { create(:project) }", + " let(:project) { create(:thing) }", + " let(:project) { build(:project) }", + " let(:project) do", + " create(:project)", + " end", + " let(:project) { create(:project, :repository) }", + " str = 'let(:project) { create(:project) }'", + " let(:project) { create(:project_empty_repo) }", + " let(:project) { create(:forked_project_with_submodules) }", + " let(:project) { create(:project_with_design) }", + " let(:authorization) { create(:project_authorization) }" + ] + end + + let(:matching_lines) do + [ + "+ let(:should_not_error) { create(:project) }", + "+ let(:project) { create(:project) }", + "+ let!(:project) { create(:project) }", + "+ let(:var) { create(:project) }", + "+ let!(:var) { create(:project) }", + "+ let(:project) { create(:project, :repository) }", + "+ let(:project) { create(:project_empty_repo) }", + "+ let(:project) { create(:forked_project_with_submodules) }", + "+ let(:project) { create(:project_with_design) }" + ] + end + + let(:changed_lines) do + [ + "+ line which doesn't exist in the file and should not cause an error", + "+ let_it_be(:project) { create(:project, :repository)", + "+ let(:project) { create(:thing) }", + "+ let(:project) do", + "+ create(:project)", + "+ end", + "+ str = 'let(:project) { create(:project) }'", + "+ let(:authorization) { create(:project_authorization) }" + ] + matching_lines + end + + subject(:specs) { fake_danger.new(helper: fake_helper) } + + before do + allow(specs).to receive(:project_helper).and_return(fake_project_helper) + allow(specs.helper).to receive(:changed_lines).with(filename).and_return(changed_lines) + allow(specs.project_helper).to receive(:file_lines).and_return(file_lines) + end + + it 'adds suggestions at the correct lines', :aggregate_failures do + [ + { suggested_line: " let_it_be(:project) { create(:project) }", number: 1 }, + { suggested_line: " let_it_be(:project) { create(:project) }", number: 3 }, + { suggested_line: " let_it_be(:var) { create(:project) }", number: 4 }, + { suggested_line: " let_it_be(:var) { create(:project) }", number: 9 }, + { suggested_line: " let_it_be(:project) { create(:project, :repository) }", number: 15 }, + { suggested_line: " let_it_be(:project) { create(:project_empty_repo) }", number: 17 }, + { suggested_line: " let_it_be(:project) { create(:forked_project_with_submodules) }", number: 18 }, + { suggested_line: " let_it_be(:project) { create(:project_with_design) }", number: 19 } + ].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(filename) + end +end diff --git a/spec/tooling/danger/specs_spec.rb b/spec/tooling/danger/specs_spec.rb index cdac5954f92..b4953858ef7 100644 --- a/spec/tooling/danger/specs_spec.rb +++ b/spec/tooling/danger/specs_spec.rb @@ -1,80 +1,24 @@ # frozen_string_literal: true -require 'rspec-parameterized' -require 'gitlab-dangerfiles' -require 'danger' -require 'danger/plugins/internal/helper' require 'gitlab/dangerfiles/spec_helper' require_relative '../../../tooling/danger/specs' -require_relative '../../../tooling/danger/project_helper' RSpec.describe Tooling::Danger::Specs, feature_category: :tooling do include_context "with dangerfile" let(:fake_danger) { DangerSpecHelper.fake_danger.include(described_class) } - let(:fake_project_helper) { instance_double('Tooling::Danger::ProjectHelper') } let(:filename) { 'spec/foo_spec.rb' } - let(:file_lines) do - [ - " describe 'foo' do", - " expect(foo).to match(['bar', 'baz'])", - " end", - " expect(foo).to match(['bar', 'baz'])", # same line as line 1 above, we expect two different suggestions - " ", - " expect(foo).to match ['bar', 'baz']", - " expect(foo).to eq(['bar', 'baz'])", - " expect(foo).to eq ['bar', 'baz']", - " expect(foo).to(match(['bar', 'baz']))", - " expect(foo).to(eq(['bar', 'baz']))", - " expect(foo).to(eq([bar, baz]))", - " expect(foo).to(eq(['bar']))", - " foo.eq(['bar'])" - ] - end - - let(:matching_lines) do - [ - "+ expect(foo).to match(['should not error'])", - "+ expect(foo).to match(['bar', 'baz'])", - "+ expect(foo).to match(['bar', 'baz'])", - "+ expect(foo).to match ['bar', 'baz']", - "+ expect(foo).to eq(['bar', 'baz'])", - "+ expect(foo).to eq ['bar', 'baz']", - "+ expect(foo).to(match(['bar', 'baz']))", - "+ expect(foo).to(eq(['bar', 'baz']))", - "+ expect(foo).to(eq([bar, baz]))" - ] - end - - let(:changed_lines) do - [ - " expect(foo).to match(['bar', 'baz'])", - " expect(foo).to match(['bar', 'baz'])", - " expect(foo).to match ['bar', 'baz']", - " expect(foo).to eq(['bar', 'baz'])", - " expect(foo).to eq ['bar', 'baz']", - "- expect(foo).to match(['bar', 'baz'])", - "- expect(foo).to match(['bar', 'baz'])", - "- expect(foo).to match ['bar', 'baz']", - "- expect(foo).to eq(['bar', 'baz'])", - "- expect(foo).to eq ['bar', 'baz']", - "- expect(foo).to eq [bar, foo]", - "+ expect(foo).to eq([])" - ] + matching_lines - end - subject(:specs) { fake_danger.new(helper: fake_helper) } - before do - allow(specs).to receive(:project_helper).and_return(fake_project_helper) - allow(specs.helper).to receive(:changed_lines).with(filename).and_return(matching_lines) - allow(specs.project_helper).to receive(:file_lines).and_return(file_lines) - end - describe '#changed_specs_files' do - let(:base_expected_files) { %w[spec/foo_spec.rb ee/spec/foo_spec.rb spec/bar_spec.rb ee/spec/bar_spec.rb spec/zab_spec.rb ee/spec/zab_spec.rb] } + let(:base_expected_files) do + %w[ + spec/foo_spec.rb ee/spec/foo_spec.rb spec/bar_spec.rb + ee/spec/bar_spec.rb spec/zab_spec.rb ee/spec/zab_spec.rb + ] + end before do all_changed_files = %w[ @@ -98,203 +42,16 @@ RSpec.describe Tooling::Danger::Specs, feature_category: :tooling do context 'with include_ee: :exclude' do it 'returns spec files without EE-specific files' do - expect(specs.changed_specs_files(ee: :exclude)).not_to include(%w[ee/spec/foo_spec.rb ee/spec/bar_spec.rb ee/spec/zab_spec.rb]) + expect(specs.changed_specs_files(ee: :exclude)) + .not_to include(%w[ee/spec/foo_spec.rb ee/spec/bar_spec.rb ee/spec/zab_spec.rb]) end end context 'with include_ee: :only' do it 'returns EE-specific spec files only' do - expect(specs.changed_specs_files(ee: :only)).to match_array(%w[ee/spec/foo_spec.rb ee/spec/bar_spec.rb ee/spec/zab_spec.rb]) - end - end - end - - describe '#add_suggestions_for_match_with_array' do - let(:template) do - <<~MARKDOWN.chomp - ```suggestion - %<suggested_line>s - ``` - - If order of the result is not important, please consider using `match_array` to avoid flakiness. - MARKDOWN - end - - it 'adds suggestions at the correct lines' do - [ - { suggested_line: " expect(foo).to match_array(['bar', 'baz'])", number: 2 }, - { suggested_line: " expect(foo).to match_array(['bar', 'baz'])", number: 4 }, - { suggested_line: " expect(foo).to match_array ['bar', 'baz']", number: 6 }, - { suggested_line: " expect(foo).to match_array(['bar', 'baz'])", number: 7 }, - { suggested_line: " expect(foo).to match_array ['bar', 'baz']", number: 8 }, - { suggested_line: " expect(foo).to(match_array(['bar', 'baz']))", number: 9 }, - { suggested_line: " expect(foo).to(match_array(['bar', 'baz']))", number: 10 }, - { suggested_line: " expect(foo).to(match_array([bar, baz]))", number: 11 } - ].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_match_with_array(filename) - end - end - - describe '#add_suggestions_for_project_factory_usage' do - let(:template) do - <<~MARKDOWN.chomp - ```suggestion - %<suggested_line>s - ``` - - Project creations are very slow. Use `let_it_be`, `build` or `build_stubbed` if possible. - See [testing best practices](https://docs.gitlab.com/ee/development/testing_guide/best_practices.html#optimize-factory-usage) - for background information and alternative options. - MARKDOWN - end - - let(:file_lines) do - [ - " let(:project) { create(:project) }", - " let_it_be(:project) { create(:project, :repository)", - " let!(:project) { create(:project) }", - " let(:var) { create(:project) }", - " let(:merge_request) { create(:merge_request, project: project)", - " context 'when merge request exists' do", - " it { is_expected.to be_success }", - " end", - " let!(:var) { create(:project) }", - " let(:project) { create(:thing) }", - " let(:project) { build(:project) }", - " let(:project) do", - " create(:project)", - " end", - " let(:project) { create(:project, :repository) }", - " str = 'let(:project) { create(:project) }'", - " let(:project) { create(:project_empty_repo) }", - " let(:project) { create(:forked_project_with_submodules) }", - " let(:project) { create(:project_with_design) }", - " let(:authorization) { create(:project_authorization) }" - ] - end - - let(:matching_lines) do - [ - "+ let(:should_not_error) { create(:project) }", - "+ let(:project) { create(:project) }", - "+ let!(:project) { create(:project) }", - "+ let(:var) { create(:project) }", - "+ let!(:var) { create(:project) }", - "+ let(:project) { create(:project, :repository) }", - "+ let(:project) { create(:project_empty_repo) }", - "+ let(:project) { create(:forked_project_with_submodules) }", - "+ let(:project) { create(:project_with_design) }" - ] - end - - let(:changed_lines) do - [ - "+ line which doesn't exist in the file and should not cause an error", - "+ let_it_be(:project) { create(:project, :repository)", - "+ let(:project) { create(:thing) }", - "+ let(:project) do", - "+ create(:project)", - "+ end", - "+ str = 'let(:project) { create(:project) }'", - "+ let(:authorization) { create(:project_authorization) }" - ] + matching_lines - end - - it 'adds suggestions at the correct lines', :aggregate_failures do - [ - { suggested_line: " let_it_be(:project) { create(:project) }", number: 1 }, - { suggested_line: " let_it_be(:project) { create(:project) }", number: 3 }, - { suggested_line: " let_it_be(:var) { create(:project) }", number: 4 }, - { suggested_line: " let_it_be(:var) { create(:project) }", number: 9 }, - { suggested_line: " let_it_be(:project) { create(:project, :repository) }", number: 15 }, - { suggested_line: " let_it_be(:project) { create(:project_empty_repo) }", number: 17 }, - { suggested_line: " let_it_be(:project) { create(:forked_project_with_submodules) }", number: 18 }, - { suggested_line: " let_it_be(:project) { create(:project_with_design) }", number: 19 } - ].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_project_factory_usage(filename) - end - end - - describe '#add_suggestions_for_feature_category' do - let(:template) do - <<~SUGGESTION_MARKDOWN.chomp - ```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", - " \n", - "RSpec.describe Projects :aggregate_failures,", - " feature_category: planning_analytics do", - " \n", - "RSpec.describe Epics :aggregate_failures,", - " ee: true do", - "\n", - "RSpec.describe Issues :aggregate_failures,", - " feature_category: :team_planning 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", - "+RSpec.describe Projects :aggregate_failures,", - "+ feature_category: planning_analytics do", - "+RSpec.describe Epics :aggregate_failures,", - "+ ee: true do", - "+RSpec.describe Issues :aggregate_failures," - ] - end - - before do - allow(specs.helper).to receive(:changed_lines).with(filename).and_return(changed_lines) - 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 }, - { suggested_line: "RSpec.describe Epics :aggregate_failures,", number: 19 } - - ].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]) + expect(specs.changed_specs_files(ee: :only)) + .to match_array(%w[ee/spec/foo_spec.rb ee/spec/bar_spec.rb ee/spec/zab_spec.rb]) 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 index 4d86e066c20..439a878a5e6 100644 --- a/spec/tooling/danger/stable_branch_spec.rb +++ b/spec/tooling/danger/stable_branch_spec.rb @@ -93,7 +93,7 @@ RSpec.describe Tooling::Danger::StableBranch, feature_category: :delivery do let(:pipeline_bridges_response) do [ { - 'name' => 'e2e:package-and-test', + 'name' => 'e2e:package-and-test-ee', 'status' => pipeline_bridge_state, 'downstream_pipeline' => { 'id' => '123', @@ -197,7 +197,7 @@ RSpec.describe Tooling::Danger::StableBranch, feature_category: :delivery do let(:pipeline_bridges_response) do [ { - 'name' => 'e2e:package-and-test', + 'name' => 'e2e:package-and-test-ee', 'status' => pipeline_bridge_state, 'downstream_pipeline' => nil } @@ -241,13 +241,23 @@ RSpec.describe Tooling::Danger::StableBranch, feature_category: :delivery do context 'when not an applicable version' do let(:target_branch) { '14-9-stable-ee' } - it_behaves_like 'with a warning', described_class::VERSION_WARNING_MESSAGE + it 'warns about the package-and-test pipeline and the version' do + expect(stable_branch).to receive(:warn).with(described_class::WARN_PACKAGE_AND_TEST_MESSAGE) + expect(stable_branch).to receive(:warn).with(described_class::VERSION_WARNING_MESSAGE) + + subject + end end context 'when the version API request fails' do let(:response_success) { false } - it_behaves_like 'with a warning', described_class::FAILED_VERSION_REQUEST_MESSAGE + it 'warns about the package-and-test pipeline and the version request' do + expect(stable_branch).to receive(:warn).with(described_class::WARN_PACKAGE_AND_TEST_MESSAGE) + 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 @@ -293,6 +303,7 @@ RSpec.describe Tooling::Danger::StableBranch, feature_category: :delivery do 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::WARN_PACKAGE_AND_TEST_MESSAGE) expect(stable_branch).to receive(:warn).with(described_class::FAILED_VERSION_REQUEST_MESSAGE) subject @@ -351,4 +362,26 @@ RSpec.describe Tooling::Danger::StableBranch, feature_category: :delivery do it { is_expected.to eq(result) } end end + + describe '#valid_stable_branch?' do + it "returns false when on the default branch" do + allow(fake_helper).to receive(:mr_target_branch).and_return('main') + + expect(stable_branch.valid_stable_branch?).to be(false) + end + + it "returns true when on a stable branch" do + allow(fake_helper).to receive(:mr_target_branch).and_return('15-1-stable-ee') + allow(fake_helper).to receive(:security_mr?).and_return(false) + + expect(stable_branch.valid_stable_branch?).to be(true) + end + + it "returns false when on a stable branch on a security MR" do + allow(fake_helper).to receive(:mr_target_branch).and_return('15-1-stable-ee') + allow(fake_helper).to receive(:security_mr?).and_return(true) + + expect(stable_branch.valid_stable_branch?).to be(false) + end + end end diff --git a/spec/tooling/docs/deprecation_handling_spec.rb b/spec/tooling/docs/deprecation_handling_spec.rb index 94c93d99b94..78e613c37c7 100644 --- a/spec/tooling/docs/deprecation_handling_spec.rb +++ b/spec/tooling/docs/deprecation_handling_spec.rb @@ -17,7 +17,7 @@ RSpec.describe Docs::DeprecationHandling do allow(YAML).to receive(:load_file) do |file_name| { 'title' => file_name[/[a-z]*\.yml/], - 'announcement_milestone' => file_name[/\d+-\d+/].tr('-', '.') + 'removal_milestone' => file_name[/\d+-\d+/].tr('-', '.') } end end diff --git a/spec/tooling/graphql/docs/renderer_spec.rb b/spec/tooling/graphql/docs/renderer_spec.rb index bf2383507aa..911dab09701 100644 --- a/spec/tooling/graphql/docs/renderer_spec.rb +++ b/spec/tooling/graphql/docs/renderer_spec.rb @@ -377,7 +377,7 @@ RSpec.describe Tooling::Graphql::Docs::Renderer do | Name | Type | Description | | ---- | ---- | ----------- | - | <a id="alphatestfoofooarg"></a>`fooArg` **{warning-solid}** | [`String`](#string) | **Introduced** in 101.2. This feature is in Alpha. It can be changed or removed at any time. Argument description. | + | <a id="alphatestfoofooarg"></a>`fooArg` **{warning-solid}** | [`String`](#string) | **Introduced** in 101.2. This feature is an Experiment. It can be changed or removed at any time. Argument description. | DOC end @@ -415,7 +415,7 @@ RSpec.describe Tooling::Graphql::Docs::Renderer do | Name | Type | Description | | ---- | ---- | ----------- | - | <a id="alphatestfoo"></a>`foo` **{warning-solid}** | [`String!`](#string) | **Introduced** in 1.10. This feature is in Alpha. It can be changed or removed at any time. A description. | + | <a id="alphatestfoo"></a>`foo` **{warning-solid}** | [`String!`](#string) | **Introduced** in 1.10. This feature is an Experiment. It can be changed or removed at any time. A description. | #### Fields with arguments @@ -425,7 +425,7 @@ RSpec.describe Tooling::Graphql::Docs::Renderer do WARNING: **Introduced** in 1.10. - This feature is in Alpha. It can be changed or removed at any time. + This feature is an Experiment. It can be changed or removed at any time. Returns [`String!`](#string). @@ -460,7 +460,7 @@ RSpec.describe Tooling::Graphql::Docs::Renderer do WARNING: **Introduced** in 10.11. - This feature is in Alpha. It can be changed or removed at any time. + This feature is an Experiment. It can be changed or removed at any time. Returns [`Int`](#int). DOC diff --git a/spec/tooling/lib/tooling/fast_quarantine_spec.rb b/spec/tooling/lib/tooling/fast_quarantine_spec.rb new file mode 100644 index 00000000000..bb60a335ce2 --- /dev/null +++ b/spec/tooling/lib/tooling/fast_quarantine_spec.rb @@ -0,0 +1,193 @@ +# frozen_string_literal: true + +require_relative '../../../../tooling/lib/tooling/fast_quarantine' +require 'tempfile' + +RSpec.describe Tooling::FastQuarantine, feature_category: :tooling do + attr_accessor :fast_quarantine_file + + around do |example| + self.fast_quarantine_file = Tempfile.new('fast_quarantine_file') + + # See https://ruby-doc.org/stdlib-1.9.3/libdoc/tempfile/rdoc/ + # Tempfile.html#class-Tempfile-label-Explicit+close + begin + example.run + ensure + fast_quarantine_file.close + fast_quarantine_file.unlink + end + end + + let(:fast_quarantine_path) { fast_quarantine_file.path } + let(:fast_quarantine_file_content) { '' } + let(:instance) do + described_class.new(fast_quarantine_path: fast_quarantine_path) + end + + before do + File.write(fast_quarantine_path, fast_quarantine_file_content) + end + + describe '#initialize' do + context 'when fast_quarantine_path does not exist' do + it 'prints a warning' do + allow(File).to receive(:exist?).and_return(false) + + expect { instance }.to output("#{fast_quarantine_path} doesn't exist!\n").to_stderr + end + end + + context 'when fast_quarantine_path exists' do + it 'does not raise an error' do + expect { instance }.not_to raise_error + end + end + end + + describe '#identifiers' do + before do + allow(File).to receive(:read).and_call_original + end + + context 'when the fast quarantine file is empty' do + let(:fast_quarantine_file_content) { '' } + + it 'returns []' do + expect(instance.identifiers).to eq([]) + end + end + + context 'when the fast quarantine file is not empty' do + let(:fast_quarantine_file_content) { "./spec/foo_spec.rb\nspec/foo_spec.rb:42\n./spec/baz_spec.rb[1:2:3]" } + + it 'returns parsed and sanitized lines' do + expect(instance.identifiers).to eq(%w[ + spec/foo_spec.rb + spec/foo_spec.rb:42 + spec/baz_spec.rb[1:2:3] + ]) + end + + context 'when reading the file raises an error' do + before do + allow(File).to receive(:read).with(fast_quarantine_path).and_raise('') + end + + it 'returns []' do + expect(instance.identifiers).to eq([]) + end + end + + describe 'memoization' do + it 'memoizes the identifiers list' do + expect(File).to receive(:read).with(fast_quarantine_path).once.and_call_original + + instance.identifiers + + # calling #identifiers again doesn't call File.read + instance.identifiers + end + end + end + end + + describe '#skip_example?' do + let(:fast_quarantine_file_content) { "./spec/foo_spec.rb\nspec/bar_spec.rb:42\n./spec/baz_spec.rb[1:2:3]" } + let(:example_id) { './spec/foo_spec.rb[1:2:3]' } + let(:example_metadata) { {} } + let(:example) { instance_double(RSpec::Core::Example, id: example_id, metadata: example_metadata) } + + describe 'skipping example by id' do + let(:example_id) { './spec/baz_spec.rb[1:2:3]' } + + it 'skips example by id' do + expect(instance.skip_example?(example)).to be_truthy + end + end + + describe 'skipping example by line' do + context 'when example location matches' do + let(:example_metadata) do + { location: './spec/bar_spec.rb:42' } + end + + it 'skips example by line' do + expect(instance.skip_example?(example)).to be_truthy + end + end + + context 'when example group location matches' do + let(:example_metadata) do + { + example_group: { location: './spec/bar_spec.rb:42' } + } + end + + it 'skips example by line' do + expect(instance.skip_example?(example)).to be_truthy + end + end + + context 'when nested parent example group location matches' do + let(:example_metadata) do + { + example_group: { + parent_example_group: { + parent_example_group: { + parent_example_group: { location: './spec/bar_spec.rb:42' } + } + } + } + } + end + + it 'skips example by line' do + expect(instance.skip_example?(example)).to be_truthy + end + end + end + + describe 'skipping example by file' do + context 'when example file_path matches' do + let(:example_metadata) do + { file_path: './spec/foo_spec.rb' } + end + + it 'skips example by file' do + expect(instance.skip_example?(example)).to be_truthy + end + end + + context 'when example group file_path matches' do + let(:example_metadata) do + { + example_group: { file_path: './spec/foo_spec.rb' } + } + end + + it 'skips example by file' do + expect(instance.skip_example?(example)).to be_truthy + end + end + + context 'when nested parent example group file_path matches' do + let(:example_metadata) do + { + example_group: { + parent_example_group: { + parent_example_group: { + parent_example_group: { file_path: './spec/foo_spec.rb' } + } + } + } + } + end + + it 'skips example by file' do + expect(instance.skip_example?(example)).to be_truthy + end + end + end + end +end diff --git a/spec/tooling/lib/tooling/find_changes_spec.rb b/spec/tooling/lib/tooling/find_changes_spec.rb new file mode 100644 index 00000000000..43c3da5699d --- /dev/null +++ b/spec/tooling/lib/tooling/find_changes_spec.rb @@ -0,0 +1,289 @@ +# frozen_string_literal: true + +require_relative '../../../../tooling/lib/tooling/find_changes' +require_relative '../../../support/helpers/stub_env' +require 'json' +require 'tempfile' + +RSpec.describe Tooling::FindChanges, feature_category: :tooling do + include StubENV + + attr_accessor :changed_files_file, :predictive_tests_file, :frontend_fixtures_mapping_file + + let(:instance) do + described_class.new( + changed_files_pathname: changed_files_pathname, + predictive_tests_pathname: predictive_tests_pathname, + frontend_fixtures_mapping_pathname: frontend_fixtures_mapping_pathname, + from: from) + end + + let(:changed_files_pathname) { changed_files_file.path } + let(:predictive_tests_pathname) { predictive_tests_file.path } + let(:frontend_fixtures_mapping_pathname) { frontend_fixtures_mapping_file.path } + let(:from) { :api } + let(:gitlab_client) { double('GitLab') } # rubocop:disable RSpec/VerifiedDoubles + + around do |example| + self.changed_files_file = Tempfile.new('changed_files_file') + self.predictive_tests_file = Tempfile.new('predictive_tests_file') + self.frontend_fixtures_mapping_file = Tempfile.new('frontend_fixtures_mapping_file') + + # See https://ruby-doc.org/stdlib-1.9.3/libdoc/tempfile/rdoc/ + # Tempfile.html#class-Tempfile-label-Explicit+close + begin + example.run + ensure + frontend_fixtures_mapping_file.close + frontend_fixtures_mapping_file.unlink + predictive_tests_file.close + predictive_tests_file.unlink + changed_files_file.close + changed_files_file.unlink + end + end + + before do + stub_env( + 'CI_API_V4_URL' => 'gitlab_api_url', + 'CI_MERGE_REQUEST_IID' => '1234', + 'CI_MERGE_REQUEST_PROJECT_PATH' => 'dummy-project', + 'PROJECT_TOKEN_FOR_CI_SCRIPTS_API_USAGE' => 'dummy-token' + ) + end + + describe '#initialize' do + context 'when fetching changes from unknown' do + let(:from) { :unknown } + + it 'raises an ArgumentError' do + expect { instance }.to raise_error( + ArgumentError, ":from can only be :api or :changed_files" + ) + end + end + end + + describe '#execute' do + subject { instance.execute } + + before do + allow(instance).to receive(:gitlab).and_return(gitlab_client) + end + + context 'when there is no changed files file' do + let(:changed_files_pathname) { nil } + + it 'raises an ArgumentError' do + expect { subject }.to raise_error( + ArgumentError, "A path to the changed files file must be given as :changed_files_pathname" + ) + end + end + + context 'when fetching changes from API' do + let(:from) { :api } + + it 'calls GitLab API to retrieve the MR diff' do + expect(gitlab_client).to receive_message_chain(:merge_request_changes, :changes).and_return([]) + + subject + end + end + + context 'when fetching changes from changed files' do + let(:from) { :changed_files } + + it 'does not call GitLab API to retrieve the MR diff' do + expect(gitlab_client).not_to receive(:merge_request_changes) + + subject + end + + context 'when there are no file changes' do + it 'writes an empty string to changed files file' do + expect { subject }.not_to change { File.read(changed_files_pathname) } + end + end + + context 'when there are file changes' do + before do + File.write(changed_files_pathname, changed_files_file_content) + end + + let(:changed_files_file_content) { 'first_file_changed second_file_changed' } + + # This is because we don't have frontend fixture mappings: we will just write the same data that we read. + it 'does not change the changed files file' do + expect { subject }.not_to change { File.read(changed_files_pathname) } + end + end + + context 'when there is no matched tests file' do + let(:predictive_tests_pathname) { nil } + + it 'does not add frontend fixtures mapping to the changed files file' do + expect { subject }.not_to change { File.read(changed_files_pathname) } + end + end + + context 'when there is no frontend fixture files' do + let(:frontend_fixtures_mapping_pathname) { nil } + + it 'does not add frontend fixtures mapping to the changed files file' do + expect { subject }.not_to change { File.read(changed_files_pathname) } + end + end + + context 'when the matched tests file and frontend fixture files are provided' do + before do + File.write(predictive_tests_pathname, matched_tests) + File.write(frontend_fixtures_mapping_pathname, frontend_fixtures_mapping_json) + File.write(changed_files_pathname, changed_files_file_content) + end + + let(:changed_files_file_content) { '' } + + context 'when there are no mappings for the matched tests' do + let(:matched_tests) { 'match_spec1 match_spec_2' } + let(:frontend_fixtures_mapping_json) do + { other_spec: ['other_mapping'] }.to_json + end + + it 'does not change the changed files file' do + expect { subject }.not_to change { File.read(changed_files_pathname) } + end + end + + context 'when there are available mappings for the matched tests' do + let(:matched_tests) { 'match_spec1 match_spec_2' } + let(:spec_mappings) { %w[spec1_mapping1 spec1_mapping2] } + let(:frontend_fixtures_mapping_json) do + { match_spec1: spec_mappings }.to_json + end + + context 'when the changed files file is initially empty' do + it 'adds the frontend fixtures mappings to the changed files file' do + expect { subject }.to change { File.read(changed_files_pathname) }.from('').to(spec_mappings.join(' ')) + end + end + + context 'when the changed files file is initially not empty' do + let(:changed_files_file_content) { 'initial_content1 initial_content2' } + + it 'adds the frontend fixtures mappings to the changed files file' do + expect { subject }.to change { File.read(changed_files_pathname) } + .from(changed_files_file_content) + .to("#{changed_files_file_content} #{spec_mappings.join(' ')}") + end + end + end + end + end + end + + describe '#only_allowed_files_changed' do + subject { instance.only_allowed_files_changed } + + context 'when fetching changes from changed files' do + let(:from) { :changed_files } + + before do + File.write(changed_files_pathname, changed_files_file_content) + end + + context 'when changed files contain only *.js changes' do + let(:changed_files_file_content) { 'a.js b.js' } + + it 'returns true' do + expect(subject).to be true + end + end + + context 'when changed files contain both *.vue and *.js changes' do + let(:changed_files_file_content) { 'a.js b.vue' } + + it 'returns true' do + expect(subject).to be true + end + end + + context 'when changed files contain not allowed changes' do + let(:changed_files_file_content) { 'a.js b.vue c.rb' } + + it 'returns false' do + expect(subject).to be false + end + end + end + + context 'when fetching changes from API' do + let(:from) { :api } + + let(:mr_changes_array) { [] } + + before do + allow(instance).to receive(:gitlab).and_return(gitlab_client) + + # The class from the GitLab gem isn't public, so we cannot use verified doubles for it. + # + # rubocop:disable RSpec/VerifiedDoubles + allow(gitlab_client).to receive(:merge_request_changes) + .with('dummy-project', '1234') + .and_return(double(changes: mr_changes_array)) + # rubocop:enable RSpec/VerifiedDoubles + end + + context 'when a file is passed as an argument' do + it 'calls GitLab API' do + expect(gitlab_client).to receive(:merge_request_changes) + .with('dummy-project', '1234') + + subject + end + end + + context 'when there are no file changes' do + let(:mr_changes_array) { [] } + + it 'returns false' do + expect(subject).to be false + end + end + + context 'when there are changes to files other than JS files' do + let(:mr_changes_array) do + [ + { + "new_path" => "scripts/gitlab_component_helpers.sh", + "old_path" => "scripts/gitlab_component_helpers.sh" + }, + { + "new_path" => "scripts/test.js", + "old_path" => "scripts/test.js" + } + ] + end + + it 'returns false' do + expect(subject).to be false + end + end + + context 'when there are changes only to JS files' do + let(:mr_changes_array) do + [ + { + "new_path" => "scripts/test.js", + "old_path" => "scripts/test.js" + } + ] + end + + it 'returns true' do + expect(subject).to be true + end + end + end + end +end 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/find_tests_spec.rb b/spec/tooling/lib/tooling/find_tests_spec.rb new file mode 100644 index 00000000000..905f81c4bbd --- /dev/null +++ b/spec/tooling/lib/tooling/find_tests_spec.rb @@ -0,0 +1,159 @@ +# frozen_string_literal: true + +require 'tempfile' +require_relative '../../../../tooling/lib/tooling/find_tests' +require_relative '../../../support/helpers/stub_env' + +RSpec.describe Tooling::FindTests, feature_category: :tooling do + include StubENV + + attr_accessor :changed_files_file, :predictive_tests_file + + let(:instance) { described_class.new(changed_files_pathname, predictive_tests_pathname) } + let(:mock_test_file_finder) { instance_double(TestFileFinder::FileFinder) } + let(:new_matching_tests) { ["new_matching_spec.rb"] } + let(:changed_files_pathname) { changed_files_file.path } + let(:predictive_tests_pathname) { predictive_tests_file.path } + let(:changed_files_content) { "changed_file1 changed_file2" } + let(:predictive_tests_content) { "previously_matching_spec.rb" } + + around do |example| + self.changed_files_file = Tempfile.new('changed_files_file') + self.predictive_tests_file = Tempfile.new('predictive_tests_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 + predictive_tests_file.close + changed_files_file.unlink + predictive_tests_file.unlink + end + end + + before do + allow(mock_test_file_finder).to receive(:use) + allow(mock_test_file_finder).to receive(:test_files).and_return(new_matching_tests) + allow(TestFileFinder::FileFinder).to receive(:new).and_return(mock_test_file_finder) + + stub_env( + 'RSPEC_TESTS_MAPPING_ENABLED' => nil, + 'RSPEC_TESTS_MAPPING_PATH' => '/tmp/does-not-exist.out' + ) + + # We write into the temp files initially, to later check how the code modified those files + File.write(changed_files_pathname, changed_files_content) + File.write(predictive_tests_pathname, predictive_tests_content) + end + + describe '#execute' do + subject { instance.execute } + + context 'when the predictive_tests_pathname file does not exist' do + let(:instance) { described_class.new(non_existing_output_pathname, predictive_tests_pathname) } + let(:non_existing_output_pathname) { 'tmp/another_file.out' } + + around do |example| + example.run + ensure + FileUtils.rm_rf(non_existing_output_pathname) + end + + it 'creates the file' do + expect { subject }.to change { File.exist?(non_existing_output_pathname) }.from(false).to(true) + end + end + + context 'when the predictive_tests_pathname file already exists' do + it 'does not create an empty file' do + expect(File).not_to receive(:write).with(predictive_tests_pathname, '') + + subject + end + end + + it 'does not modify the content of the input file' do + expect { subject }.not_to change { File.read(changed_files_pathname) } + end + + it 'does not overwrite the output file' do + expect { subject }.to change { File.read(predictive_tests_pathname) } + .from(predictive_tests_content) + .to("#{predictive_tests_content} #{new_matching_tests.uniq.join(' ')}") + end + + it 'loads the tests.yml file with a pattern matching mapping' do + expect(TestFileFinder::MappingStrategies::PatternMatching).to receive(:load).with('tests.yml') + + subject + end + + context 'when RSPEC_TESTS_MAPPING_ENABLED env variable is set' do + before do + stub_env( + 'RSPEC_TESTS_MAPPING_ENABLED' => 'true', + 'RSPEC_TESTS_MAPPING_PATH' => 'crystalball-test/mapping.json' + ) + end + + it 'loads the direct matching pattern file' do + expect(TestFileFinder::MappingStrategies::DirectMatching) + .to receive(:load_json) + .with('crystalball-test/mapping.json') + + subject + end + end + + context 'when RSPEC_TESTS_MAPPING_ENABLED env variable is not set' do + let(:rspec_tests_mapping_enabled) { '' } + + before do + stub_env( + 'RSPEC_TESTS_MAPPING_ENABLED' => rspec_tests_mapping_enabled, + 'RSPEC_TESTS_MAPPING_PATH' => rspec_tests_mapping_path + ) + end + + context 'when RSPEC_TESTS_MAPPING_PATH is set' do + let(:rspec_tests_mapping_path) { 'crystalball-test/mapping.json' } + + it 'does not load the direct matching pattern file' do + expect(TestFileFinder::MappingStrategies::DirectMatching).not_to receive(:load_json) + + subject + end + end + + context 'when RSPEC_TESTS_MAPPING_PATH is not set' do + let(:rspec_tests_mapping_path) { nil } + + it 'does not load the direct matching pattern file' do + expect(TestFileFinder::MappingStrategies::DirectMatching).not_to receive(:load_json) + + subject + end + end + end + + context 'when the same spec is matching multiple times' do + let(:new_matching_tests) do + [ + "new_matching_spec.rb", + "duplicate_spec.rb", + "duplicate_spec.rb" + ] + end + + it 'writes uniquely matching specs to the output' do + subject + + expect(File.read(predictive_tests_pathname).split(' ')).to match_array( + predictive_tests_content.split(' ') + new_matching_tests.uniq + ) + end + end + end +end diff --git a/spec/tooling/lib/tooling/gettext_extractor_spec.rb b/spec/tooling/lib/tooling/gettext_extractor_spec.rb new file mode 100644 index 00000000000..3c0f91342c2 --- /dev/null +++ b/spec/tooling/lib/tooling/gettext_extractor_spec.rb @@ -0,0 +1,276 @@ +# frozen_string_literal: true + +require 'rspec/parameterized' + +require_relative '../../../../tooling/lib/tooling/gettext_extractor' +require_relative '../../../support/helpers/stub_env' +require_relative '../../../support/tmpdir' + +RSpec.describe Tooling::GettextExtractor, feature_category: :tooling do + include StubENV + include TmpdirHelper + + let(:base_dir) { mktmpdir } + let(:instance) { described_class.new(backend_glob: '*.{rb,haml,erb}', glob_base: base_dir) } + let(:frontend_status) { true } + + let(:files) do + { + rb_file: File.join(base_dir, 'ruby.rb'), + haml_file: File.join(base_dir, 'template.haml'), + erb_file: File.join(base_dir, 'template.erb') + } + end + + before do + # Disable parallelism in specs in order to suppress some confusing stack traces + stub_env( + 'PARALLEL_PROCESSOR_COUNT' => 0 + ) + # Mock Backend files + File.write(files[:rb_file], '[_("RB"), _("All"), n_("Apple", "Apples", size), s_("Context|A"), N_("All2") ]') + File.write( + files[:erb_file], + '<h1><%= _("ERB") + _("All") + n_("Pear", "Pears", size) + s_("Context|B") + N_("All2") %></h1>' + ) + File.write( + files[:haml_file], + '%h1= _("HAML") + _("All") + n_("Cabbage", "Cabbages", size) + s_("Context|C") + N_("All2")' + ) + # Stub out Frontend file parsing + status = {} + allow(status).to receive(:success?).and_return(frontend_status) + allow(Open3).to receive(:capture2) + .with("node scripts/frontend/extract_gettext_all.js --all") + .and_return([ + '{"example.js": [ ["JS"], ["All"], ["Mango\u0000Mangoes"], ["Context|D"], ["All2"] ] }', + status + ]) + end + + describe '::HamlParser' do + # Testing with a non-externalized string, as the functionality + # is properly tested later on + it '#parse_source' do + expect(described_class::HamlParser.new(files[:haml_file]).parse_source('%h1= "Test"')).to match_array([]) + end + end + + describe '#parse' do + it 'collects and merges translatable strings from frontend and backend' do + expect(instance.parse([]).to_h { |entry| [entry.msgid, entry.msgid_plural] }).to eq({ + 'All' => nil, + 'All2' => nil, + 'Context|A' => nil, + 'Context|B' => nil, + 'Context|C' => nil, + 'Context|D' => nil, + 'ERB' => nil, + 'HAML' => nil, + 'JS' => nil, + 'RB' => nil, + 'Apple' => 'Apples', + 'Cabbage' => 'Cabbages', + 'Mango' => 'Mangoes', + 'Pear' => 'Pears' + }) + end + + it 're-raises error from backend extraction' do + allow(instance).to receive(:parse_backend_file).and_raise(StandardError) + + expect { instance.parse([]) }.to raise_error(StandardError) + end + + context 'when frontend extraction raises an error' do + let(:frontend_status) { false } + + it 'is re-raised' do + expect { instance.parse([]) }.to raise_error(StandardError, 'Could not parse frontend files') + end + end + end + + describe '#generate_pot' do + subject { instance.generate_pot } + + it 'produces pot without date headers' do + expect(subject).not_to include('POT-Creation-Date:') + expect(subject).not_to include('PO-Revision-Date:') + end + + it 'produces pot file with all translated strings, sorted by msg id' do + expect(subject).to eql <<~POT_FILE + # SOME DESCRIPTIVE TITLE. + # Copyright (C) YEAR THE PACKAGE'S COPYRIGHT HOLDER + # This file is distributed under the same license as the gitlab package. + # FIRST AUTHOR <EMAIL@ADDRESS>, YEAR. + # + #, fuzzy + msgid "" + msgstr "" + "Project-Id-Version: gitlab 1.0.0\\n" + "Report-Msgid-Bugs-To: \\n" + "Last-Translator: FULL NAME <EMAIL@ADDRESS>\\n" + "Language-Team: LANGUAGE <LL@li.org>\\n" + "Language: \\n" + "MIME-Version: 1.0\\n" + "Content-Type: text/plain; charset=UTF-8\\n" + "Content-Transfer-Encoding: 8bit\\n" + "Plural-Forms: nplurals=INTEGER; plural=EXPRESSION;\\n" + + msgid "All" + msgstr "" + + msgid "All2" + msgstr "" + + msgid "Apple" + msgid_plural "Apples" + msgstr[0] "" + msgstr[1] "" + + msgid "Cabbage" + msgid_plural "Cabbages" + msgstr[0] "" + msgstr[1] "" + + msgid "Context|A" + msgstr "" + + msgid "Context|B" + msgstr "" + + msgid "Context|C" + msgstr "" + + msgid "Context|D" + msgstr "" + + msgid "ERB" + msgstr "" + + msgid "HAML" + msgstr "" + + msgid "JS" + msgstr "" + + msgid "Mango" + msgid_plural "Mangoes" + msgstr[0] "" + msgstr[1] "" + + msgid "Pear" + msgid_plural "Pears" + msgstr[0] "" + msgstr[1] "" + + msgid "RB" + msgstr "" + POT_FILE + end + end + + # This private methods is tested directly, because unfortunately it is called + # with the "Parallel" gem. As the parallel gem executes this function in a different + # thread, our coverage reporting is confused + # + # On the other hand, the tests are also more readable, so maybe a win-win + describe '#parse_backend_file' do + subject { instance.send(:parse_backend_file, curr_file) } + + where do + { + 'with ruby file' => { + invalid_syntax: 'x = {id: _("RB")', + file: :rb_file, + result: { + 'All' => nil, + 'All2' => nil, + 'Context|A' => nil, + 'RB' => nil, 'Apple' => 'Apples' + }, + parser: GetText::RubyParser + }, + 'with haml file' => { + invalid_syntax: " %a\n- content = _('HAML')", + file: :haml_file, + result: { + 'All' => nil, + 'All2' => nil, + 'Context|C' => nil, + 'HAML' => nil, + 'Cabbage' => 'Cabbages' + }, + parser: described_class::HamlParser + }, + 'with erb file' => { + invalid_syntax: "<% x = {id: _('ERB') %>", + file: :erb_file, + result: { + 'All' => nil, + 'All2' => nil, + 'Context|B' => nil, + 'ERB' => nil, + 'Pear' => 'Pears' + }, + parser: GetText::ErbParser + } + } + end + + with_them do + let(:curr_file) { files[file] } + + context 'when file has valid syntax' do + before do + allow(parser).to receive(:new).and_call_original + end + + it 'parses file and returns extracted strings as POEntries' do + expect(subject.map(&:class).uniq).to match_array([GetText::POEntry]) + expect(subject.to_h { |entry| [entry.msgid, entry.msgid_plural] }).to eq(result) + expect(parser).to have_received(:new) + end + end + + # We do not worry about syntax errors in these file types, as it is _not_ the job of + # gettext extractor to ensure correctness of the files. These errors should raise + # in other places + context 'when file has invalid syntax' do + before do + File.write(curr_file, invalid_syntax) + end + + it 'does not raise error' do + expect { subject }.not_to raise_error + end + end + + context 'when file does not contain "_("' do + before do + allow(parser).to receive(:new).and_call_original + File.write(curr_file, '"abcdef"') + end + + it 'never parses the file and returns empty array' do + expect(subject).to match_array([]) + expect(parser).not_to have_received(:new) + end + end + end + + context 'with unsupported file containing "_("' do + let(:curr_file) { File.join(base_dir, 'foo.unsupported') } + + before do + File.write(curr_file, '_("Test")') + end + + it 'raises error' do + expect { subject }.to raise_error(NotImplementedError) + end + end + end +end diff --git a/spec/tooling/lib/tooling/helpers/file_handler_spec.rb b/spec/tooling/lib/tooling/helpers/file_handler_spec.rb new file mode 100644 index 00000000000..b78f0a3bb6b --- /dev/null +++ b/spec/tooling/lib/tooling/helpers/file_handler_spec.rb @@ -0,0 +1,127 @@ +# frozen_string_literal: true + +require 'tempfile' +require_relative '../../../../../tooling/lib/tooling/helpers/file_handler' + +class MockClass # rubocop:disable Gitlab/NamespacedClass + include Tooling::Helpers::FileHandler +end + +RSpec.describe Tooling::Helpers::FileHandler, feature_category: :tooling do + attr_accessor :input_file_path, :output_file_path + + around do |example| + input_file = Tempfile.new('input') + output_file = Tempfile.new('output') + + self.input_file_path = input_file.path + self.output_file_path = output_file.path + + # See https://ruby-doc.org/stdlib-1.9.3/libdoc/tempfile/rdoc/ + # Tempfile.html#class-Tempfile-label-Explicit+close + begin + example.run + ensure + output_file.close + input_file.close + output_file.unlink + input_file.unlink + end + end + + let(:instance) { MockClass.new } + let(:initial_content) { 'previous_content1 previous_content2' } + + before do + # We write into the temp files initially, to later check how the code modified those files + File.write(input_file_path, initial_content) + File.write(output_file_path, initial_content) + end + + describe '#read_array_from_file' do + subject { instance.read_array_from_file(input_file_path) } + + context 'when the input file does not exist' do + let(:non_existing_input_pathname) { 'tmp/another_file.out' } + + subject { instance.read_array_from_file(non_existing_input_pathname) } + + around do |example| + example.run + ensure + FileUtils.rm_rf(non_existing_input_pathname) + end + + it 'creates the file' do + expect { subject }.to change { File.exist?(non_existing_input_pathname) }.from(false).to(true) + end + end + + context 'when the input file is not empty' do + let(:initial_content) { 'previous_content1 previous_content2' } + + it 'returns the content of the file in an array' do + expect(subject).to eq(initial_content.split(' ')) + end + end + end + + describe '#write_array_to_file' do + let(:content_array) { %w[new_entry] } + let(:append_flag) { true } + + subject { instance.write_array_to_file(output_file_path, content_array, append: append_flag) } + + context 'when the output file does not exist' do + let(:non_existing_output_file) { 'tmp/another_file.out' } + + subject { instance.write_array_to_file(non_existing_output_file, content_array) } + + around do |example| + example.run + ensure + FileUtils.rm_rf(non_existing_output_file) + end + + it 'creates the file' do + expect { subject }.to change { File.exist?(non_existing_output_file) }.from(false).to(true) + end + end + + context 'when the output file is empty' do + let(:initial_content) { '' } + + it 'writes the correct content to the file' do + expect { subject }.to change { File.read(output_file_path) }.from('').to(content_array.join(' ')) + end + + context 'when the content array is not sorted' do + let(:content_array) { %w[new_entry a_new_entry] } + + it 'sorts the array before writing it to file' do + expect { subject }.to change { File.read(output_file_path) }.from('').to(content_array.sort.join(' ')) + end + end + end + + context 'when the output file is not empty' do + let(:initial_content) { 'previous_content1 previous_content2' } + + it 'appends the correct content to the file' do + expect { subject }.to change { File.read(output_file_path) } + .from(initial_content) + .to((initial_content.split(' ') + content_array).join(' ')) + end + + context 'when the append flag is set to false' do + let(:append_flag) { false } + + it 'overwrites the previous content' do + expect { subject }.to change { File.read(output_file_path) } + .from(initial_content) + .to(content_array.join(' ')) + 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/tooling/lib/tooling/kubernetes_client_spec.rb b/spec/tooling/lib/tooling/kubernetes_client_spec.rb index 50d33182a42..8d127f1345b 100644 --- a/spec/tooling/lib/tooling/kubernetes_client_spec.rb +++ b/spec/tooling/lib/tooling/kubernetes_client_spec.rb @@ -1,286 +1,200 @@ # frozen_string_literal: true +require 'time' require_relative '../../../../tooling/lib/tooling/kubernetes_client' RSpec.describe Tooling::KubernetesClient do - let(:namespace) { 'review-apps' } - let(:release_name) { 'my-release' } - let(:pod_for_release) { "pod-my-release-abcd" } - let(:raw_resource_names_str) { "NAME\nfoo\n#{pod_for_release}\nbar" } - let(:raw_resource_names) { raw_resource_names_str.lines.map(&:strip) } - - subject { described_class.new(namespace: namespace) } - - describe 'RESOURCE_LIST' do - it 'returns the correct list of resources separated by commas' do - expect(described_class::RESOURCE_LIST).to eq('ingress,svc,pdb,hpa,deploy,statefulset,job,pod,secret,configmap,pvc,secret,clusterrole,clusterrolebinding,role,rolebinding,sa,crd') - end + let(:instance) { described_class.new } + let(:one_day_ago) { Time.now - 3600 * 24 * 1 } + let(:two_days_ago) { Time.now - 3600 * 24 * 2 } + let(:three_days_ago) { Time.now - 3600 * 24 * 3 } + + before do + # Global mock to ensure that no kubectl commands are run by accident in a test. + allow(instance).to receive(:run_command) end - describe '#cleanup_by_release' do - before do - allow(subject).to receive(:raw_resource_names).and_return(raw_resource_names) - end - - shared_examples 'a kubectl command to delete resources' do - let(:wait) { true } - let(:release_names_in_command) { release_name.respond_to?(:join) ? %(-l 'release in (#{release_name.join(', ')})') : %(-l release="#{release_name}") } - - specify do - expect(Gitlab::Popen).to receive(:popen_with_detail) - .with(["kubectl delete #{described_class::RESOURCE_LIST} " + - %(--namespace "#{namespace}" --now --ignore-not-found --wait=#{wait} #{release_names_in_command})]) - .and_return(Gitlab::Popen::Result.new([], '', '', double(success?: true))) - - expect(Gitlab::Popen).to receive(:popen_with_detail) - .with([%(kubectl delete --namespace "#{namespace}" --ignore-not-found #{pod_for_release})]) - .and_return(Gitlab::Popen::Result.new([], '', '', double(success?: true))) - - # We're not verifying the output here, just silencing it - expect { subject.cleanup_by_release(release_name: release_name) }.to output.to_stdout - end - end - - it 'raises an error if the Kubernetes command fails' do - expect(Gitlab::Popen).to receive(:popen_with_detail) - .with(["kubectl delete #{described_class::RESOURCE_LIST} " + - %(--namespace "#{namespace}" --now --ignore-not-found --wait=true -l release="#{release_name}")]) - .and_return(Gitlab::Popen::Result.new([], '', '', double(success?: false))) - - expect { subject.cleanup_by_release(release_name: release_name) }.to raise_error(described_class::CommandFailedError) - end - - it_behaves_like 'a kubectl command to delete resources' - - context 'with multiple releases' do - let(:release_name) { %w[my-release my-release-2] } - - it_behaves_like 'a kubectl command to delete resources' - end - - context 'with `wait: false`' do - let(:wait) { false } - - it_behaves_like 'a kubectl command to delete resources' - end - end - - describe '#cleanup_by_created_at' do - let(:two_days_ago) { Time.now - 3600 * 24 * 2 } - let(:resource_type) { 'pvc' } - let(:resource_names) { [pod_for_release] } + describe '#cleanup_namespaces_by_created_at' do + let(:namespace_1_created_at) { three_days_ago } + let(:namespace_2_created_at) { three_days_ago } + let(:namespace_1_name) { 'review-first-review-app' } + let(:namespace_2_name) { 'review-second-review-app' } + let(:kubectl_namespaces_json) do + <<~JSON + { + "apiVersion": "v1", + "items": [ + { + "apiVersion": "v1", + "kind": "namespace", + "metadata": { + "creationTimestamp": "#{namespace_1_created_at.utc.iso8601}", + "name": "#{namespace_1_name}" + } + }, + { + "apiVersion": "v1", + "kind": "namespace", + "metadata": { + "creationTimestamp": "#{namespace_2_created_at.utc.iso8601}", + "name": "#{namespace_2_name}" + } + } + ] + } + JSON + end + + subject { instance.cleanup_namespaces_by_created_at(created_before: two_days_ago) } before do - allow(subject).to receive(:resource_names_created_before).with(resource_type: resource_type, created_before: two_days_ago).and_return(resource_names) + allow(instance).to receive(:run_command).with( + "kubectl get namespace --all-namespaces --sort-by='{.metadata.creationTimestamp}' -o json" + ).and_return(kubectl_namespaces_json) end - shared_examples 'a kubectl command to delete resources by older than given creation time' do - let(:wait) { true } - let(:release_names_in_command) { resource_names.join(' ') } + context 'when no namespaces are stale' do + let(:namespace_1_created_at) { one_day_ago } + let(:namespace_2_created_at) { one_day_ago } - specify do - expect(Gitlab::Popen).to receive(:popen_with_detail) - .with(["kubectl delete #{resource_type} ".squeeze(' ') + - %(--namespace "#{namespace}" --now --ignore-not-found --wait=#{wait} #{release_names_in_command})]) - .and_return(Gitlab::Popen::Result.new([], '', '', double(success?: true))) + it 'does not delete any namespace' do + expect(instance).not_to receive(:run_command).with(/kubectl delete namespace/) - # We're not verifying the output here, just silencing it - expect { subject.cleanup_by_created_at(resource_type: resource_type, created_before: two_days_ago) }.to output.to_stdout + subject end end - it 'raises an error if the Kubernetes command fails' do - expect(Gitlab::Popen).to receive(:popen_with_detail) - .with(["kubectl delete #{resource_type} " + - %(--namespace "#{namespace}" --now --ignore-not-found --wait=true #{pod_for_release})]) - .and_return(Gitlab::Popen::Result.new([], '', '', double(success?: false))) - - expect { subject.cleanup_by_created_at(resource_type: resource_type, created_before: two_days_ago) }.to raise_error(described_class::CommandFailedError) - end + context 'when some namespaces are stale' do + let(:namespace_1_created_at) { three_days_ago } + let(:namespace_2_created_at) { three_days_ago } - it_behaves_like 'a kubectl command to delete resources by older than given creation time' + context 'when some namespaces are not review app namespaces' do + let(:namespace_1_name) { 'review-my-review-app' } + let(:namespace_2_name) { 'review-apps' } # This is not a review apps namespace, so we should not try to delete it - context 'with multiple resource names' do - let(:resource_names) { %w[pod-1 pod-2] } + it 'only deletes the review app namespaces' do + expect(instance).to receive(:run_command).with("kubectl delete namespace --now --ignore-not-found #{namespace_1_name}") - it_behaves_like 'a kubectl command to delete resources by older than given creation time' - end - - context 'with `wait: false`' do - let(:wait) { false } - - it_behaves_like 'a kubectl command to delete resources by older than given creation time' - end - - context 'with no resource_type given' do - let(:resource_type) { nil } - - it_behaves_like 'a kubectl command to delete resources by older than given creation time' - end - - context 'with multiple resource_type given' do - let(:resource_type) { 'pvc,service' } - - it_behaves_like 'a kubectl command to delete resources by older than given creation time' - end + subject + end + end - context 'with no resources found' do - let(:resource_names) { [] } + context 'when all namespaces are review app namespaces' do + let(:namespace_1_name) { 'review-my-review-app' } + let(:namespace_2_name) { 'review-another-review-app' } - it 'does not call #delete_by_exact_names' do - expect(subject).not_to receive(:delete_by_exact_names) + it 'deletes all of the stale namespaces' do + expect(instance).to receive(:run_command).with("kubectl delete namespace --now --ignore-not-found #{namespace_1_name} #{namespace_2_name}") - subject.cleanup_by_created_at(resource_type: resource_type, created_before: two_days_ago) + subject + end end end end - describe '#cleanup_review_app_namespaces' do - let(:two_days_ago) { Time.now - 3600 * 24 * 2 } - let(:namespaces) { %w[review-abc-123 review-xyz-789] } + describe '#delete_namespaces' do + subject { instance.delete_namespaces(namespaces) } - subject { described_class.new(namespace: nil) } + context 'when at least one namespace is not a review app namespace' do + let(:namespaces) { %w[review-ns-1 default] } - before do - allow(subject).to receive(:review_app_namespaces_created_before).with(created_before: two_days_ago).and_return(namespaces) - end - - shared_examples 'a kubectl command to delete namespaces older than given creation time' do - let(:wait) { true } - - specify do - expect(Gitlab::Popen).to receive(:popen_with_detail) - .with(["kubectl delete namespace " + - %(--now --ignore-not-found --wait=#{wait} #{namespaces.join(' ')})]) - .and_return(Gitlab::Popen::Result.new([], '', '', double(success?: true))) + it 'does not delete any namespace' do + expect(instance).not_to receive(:run_command).with(/kubectl delete namespace/) - # We're not verifying the output here, just silencing it - expect { subject.cleanup_review_app_namespaces(created_before: two_days_ago) }.to output.to_stdout + subject end end - it_behaves_like 'a kubectl command to delete namespaces older than given creation time' - - it 'raises an error if the Kubernetes command fails' do - expect(Gitlab::Popen).to receive(:popen_with_detail) - .with(["kubectl delete namespace " + - %(--now --ignore-not-found --wait=true #{namespaces.join(' ')})]) - .and_return(Gitlab::Popen::Result.new([], '', '', double(success?: false))) - - expect { subject.cleanup_review_app_namespaces(created_before: two_days_ago) }.to raise_error(described_class::CommandFailedError) - end - - context 'with no namespaces found' do - let(:namespaces) { [] } + context 'when all namespaces are review app namespaces' do + let(:namespaces) { %w[review-ns-1 review-ns-2] } - it 'does not call #delete_namespaces_by_exact_names' do - expect(subject).not_to receive(:delete_namespaces_by_exact_names) + it 'deletes the namespaces' do + expect(instance).to receive(:run_command).with("kubectl delete namespace --now --ignore-not-found #{namespaces.join(' ')}") - subject.cleanup_review_app_namespaces(created_before: two_days_ago) + subject end end end - describe '#raw_resource_names' do - it 'calls kubectl to retrieve the resource names' do - expect(Gitlab::Popen).to receive(:popen_with_detail) - .with(["kubectl get #{described_class::RESOURCE_LIST} " + - %(--namespace "#{namespace}" -o name)]) - .and_return(Gitlab::Popen::Result.new([], raw_resource_names_str, '', double(success?: true))) - - expect(subject.__send__(:raw_resource_names)).to eq(raw_resource_names) + describe '#namespaces_created_before' do + subject { instance.namespaces_created_before(created_before: two_days_ago) } + + let(:namespace_1_created_at) { three_days_ago } + let(:namespace_2_created_at) { one_day_ago } + let(:namespace_1_name) { 'review-first-review-app' } + let(:namespace_2_name) { 'review-second-review-app' } + let(:kubectl_namespaces_json) do + <<~JSON + { + "apiVersion": "v1", + "items": [ + { + "apiVersion": "v1", + "kind": "namespace", + "metadata": { + "creationTimestamp": "#{namespace_1_created_at.utc.iso8601}", + "name": "#{namespace_1_name}" + } + }, + { + "apiVersion": "v1", + "kind": "namespace", + "metadata": { + "creationTimestamp": "#{namespace_2_created_at.utc.iso8601}", + "name": "#{namespace_2_name}" + } + } + ] + } + JSON + end + + it 'returns an array of namespaces' do + allow(instance).to receive(:run_command).with( + "kubectl get namespace --all-namespaces --sort-by='{.metadata.creationTimestamp}' -o json" + ).and_return(kubectl_namespaces_json) + + expect(subject).to match_array(%w[review-first-review-app]) end end - describe '#resource_names_created_before' do - let(:three_days_ago) { Time.now - 3600 * 24 * 3 } - let(:two_days_ago) { Time.now - 3600 * 24 * 2 } - let(:pvc_created_three_days_ago) { 'pvc-created-three-days-ago' } - let(:resource_type) { 'pvc' } - let(:raw_resources) do - { - items: [ - { - apiVersion: "v1", - kind: "PersistentVolumeClaim", - metadata: { - creationTimestamp: three_days_ago, - name: pvc_created_three_days_ago - } - }, - { - apiVersion: "v1", - kind: "PersistentVolumeClaim", - metadata: { - creationTimestamp: Time.now, - name: 'another-pvc' - } - } - ] - }.to_json - end + describe '#run_command' do + subject { instance.run_command(command) } - shared_examples 'a kubectl command to retrieve resource names sorted by creationTimestamp' do - specify do - expect(Gitlab::Popen).to receive(:popen_with_detail) - .with(["kubectl get #{resource_type} ".squeeze(' ') + - %(--namespace "#{namespace}" ) + - "--sort-by='{.metadata.creationTimestamp}' -o json"]) - .and_return(Gitlab::Popen::Result.new([], raw_resources, '', double(success?: true))) + before do + # We undo the global mock just for this method + allow(instance).to receive(:run_command).and_call_original - expect(subject.__send__(:resource_names_created_before, resource_type: resource_type, created_before: two_days_ago)).to contain_exactly(pvc_created_three_days_ago) - end + # Mock stdout + allow(instance).to receive(:puts) end - it_behaves_like 'a kubectl command to retrieve resource names sorted by creationTimestamp' - - context 'with no resource_type given' do - let(:resource_type) { nil } + context 'when executing a successful command' do + let(:command) { 'true' } # https://linux.die.net/man/1/true - it_behaves_like 'a kubectl command to retrieve resource names sorted by creationTimestamp' - end + it 'displays the name of the command to stdout' do + expect(instance).to receive(:puts).with("Running command: `#{command}`") - context 'with multiple resource_type given' do - let(:resource_type) { 'pvc,service' } + subject + end - it_behaves_like 'a kubectl command to retrieve resource names sorted by creationTimestamp' + it 'does not raise an error' do + expect { subject }.not_to raise_error + end end - end - describe '#review_app_namespaces_created_before' do - let(:three_days_ago) { Time.now - 3600 * 24 * 3 } - let(:two_days_ago) { Time.now - 3600 * 24 * 2 } - let(:namespace_created_three_days_ago) { 'review-ns-created-three-days-ago' } - let(:resource_type) { 'namespace' } - let(:raw_resources) do - { - items: [ - { - apiVersion: "v1", - kind: "Namespace", - metadata: { - creationTimestamp: three_days_ago, - name: namespace_created_three_days_ago - } - }, - { - apiVersion: "v1", - kind: "Namespace", - metadata: { - creationTimestamp: Time.now, - name: 'another-namespace' - } - } - ] - }.to_json - end + context 'when executing an unsuccessful command' do + let(:command) { 'false' } # https://linux.die.net/man/1/false - specify do - expect(Gitlab::Popen).to receive(:popen_with_detail) - .with(["kubectl get namespace --sort-by='{.metadata.creationTimestamp}' -o json"]) - .and_return(Gitlab::Popen::Result.new([], raw_resources, '', double(success?: true))) + it 'displays the name of the command to stdout' do + expect(instance).to receive(:puts).with("Running command: `#{command}`") - expect(subject.__send__(:review_app_namespaces_created_before, created_before: two_days_ago)).to eq([namespace_created_three_days_ago]) + expect { subject }.to raise_error(described_class::CommandFailedError) + end + + it 'raises an error' do + expect { subject }.to raise_error(described_class::CommandFailedError) + end end end end diff --git a/spec/tooling/lib/tooling/mappings/graphql_base_type_mappings_spec.rb b/spec/tooling/lib/tooling/mappings/graphql_base_type_mappings_spec.rb new file mode 100644 index 00000000000..b6459428214 --- /dev/null +++ b/spec/tooling/lib/tooling/mappings/graphql_base_type_mappings_spec.rb @@ -0,0 +1,251 @@ +# frozen_string_literal: true + +require 'tempfile' +require_relative '../../../../../tooling/lib/tooling/mappings/graphql_base_type_mappings' + +RSpec.describe Tooling::Mappings::GraphqlBaseTypeMappings, feature_category: :tooling do + # We set temporary folders, and those readers give access to those folder paths + attr_accessor :foss_folder, :ee_folder, :jh_folder + attr_accessor :changed_files_file, :predictive_tests_file + + let(:changed_files_pathname) { changed_files_file.path } + let(:predictive_tests_pathname) { predictive_tests_file.path } + let(:instance) { described_class.new(changed_files_pathname, predictive_tests_pathname) } + let(:changed_files_content) { "changed_file1 changed_file2" } + let(:predictive_tests_initial_content) { "previously_matching_spec.rb" } + + around do |example| + self.changed_files_file = Tempfile.new('changed_files_file') + self.predictive_tests_file = Tempfile.new('predictive_tests_file') + + Dir.mktmpdir('FOSS') do |foss_folder| + Dir.mktmpdir('EE') do |ee_folder| + Dir.mktmpdir('JH') do |jh_folder| + self.foss_folder = foss_folder + self.ee_folder = ee_folder + self.jh_folder = jh_folder + + # 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 + predictive_tests_file.close + changed_files_file.unlink + predictive_tests_file.unlink + end + end + end + end + end + + before do + stub_const("Tooling::Mappings::GraphqlBaseTypeMappings::GRAPHQL_TYPES_FOLDERS", { + nil => [foss_folder], + 'ee' => [foss_folder, ee_folder], + 'jh' => [foss_folder, ee_folder, jh_folder] + }) + + # We write into the temp files initially, to later check how the code modified those files + File.write(changed_files_pathname, changed_files_content) + File.write(predictive_tests_pathname, predictive_tests_initial_content) + end + + describe '#execute' do + subject { instance.execute } + + context 'when no GraphQL files were changed' do + let(:changed_files_content) { '' } + + it 'does not change the output file' do + expect { subject }.not_to change { File.read(predictive_tests_pathname) } + end + end + + context 'when some GraphQL files were changed' do + let(:changed_files_content) do + [ + "#{foss_folder}/my_graphql_file.rb", + "#{foss_folder}/my_other_graphql_file.rb" + ].join(' ') + end + + context 'when none of those GraphQL types are included in other GraphQL types' do + before do + File.write("#{foss_folder}/my_graphql_file.rb", "some graphQL code; implements-test MyOtherGraphqlFile") + File.write("#{foss_folder}/my_other_graphql_file.rb", "some graphQL code") + end + + it 'does not change the output file' do + expect { subject }.not_to change { File.read(predictive_tests_pathname) } + end + end + + context 'when the GraphQL types are included in other GraphQL types' do + before do + File.write("#{foss_folder}/my_graphql_file.rb", "some graphQL code; implements MyOtherGraphqlFile") + File.write("#{foss_folder}/my_other_graphql_file.rb", "some graphQL code") + + # We mock this because we are using temp directories, so we cannot rely on just replacing `app`` with `spec` + allow(instance).to receive(:filename_to_spec_filename) + .with("#{foss_folder}/my_graphql_file.rb") + .and_return('spec/my_graphql_file_spec.rb') + end + + it 'writes the correct specs in the output' do + expect { subject }.to change { File.read(predictive_tests_pathname) } + .from(predictive_tests_initial_content) + .to("#{predictive_tests_initial_content} spec/my_graphql_file_spec.rb") + end + end + end + end + + describe '#filter_files' do + subject { instance.filter_files } + + before do + File.write("#{foss_folder}/my_graphql_file.rb", "my_graphql_file.rb") + File.write("#{foss_folder}/my_other_graphql_file.rb", "my_other_graphql_file.rb") + File.write("#{foss_folder}/another_file.erb", "another_file.erb") + end + + context 'when no files were changed' do + let(:changed_files_content) { '' } + + it 'returns an empty array' do + expect(subject).to match_array([]) + end + end + + context 'when GraphQL files were changed' do + let(:changed_files_content) do + [ + "#{foss_folder}/my_graphql_file.rb", + "#{foss_folder}/my_other_graphql_file.rb", + "#{foss_folder}/another_file.erb" + ].join(' ') + end + + it 'returns the path to the GraphQL files' do + expect(subject).to match_array([ + "#{foss_folder}/my_graphql_file.rb", + "#{foss_folder}/my_other_graphql_file.rb" + ]) + end + end + + context 'when files are deleted' do + let(:changed_files_content) { "#{foss_folder}/deleted.rb" } + + it 'returns an empty array' do + expect(subject).to match_array([]) + end + end + end + + describe '#types_hierarchies' do + subject { instance.types_hierarchies } + + context 'when no types are implementing other types' do + before do + File.write("#{foss_folder}/foss_file.rb", "some graphQL code") + File.write("#{ee_folder}/ee_file.rb", "some graphQL code") + File.write("#{jh_folder}/jh_file.rb", "some graphQL code") + end + + it 'returns nothing' do + expect(subject).to eq( + nil => {}, + 'ee' => {}, + 'jh' => {} + ) + end + end + + context 'when types are implementing other types' do + before do + File.write("#{foss_folder}/foss_file.rb", "some graphQL code; implements NoteableInterface") + File.write("#{ee_folder}/ee_file.rb", "some graphQL code; implements NoteableInterface") + File.write("#{jh_folder}/jh_file.rb", "some graphQL code; implements NoteableInterface") + end + + context 'when FOSS' do + it 'returns only FOSS types' do + expect(subject).to include( + nil => { + 'NoteableInterface' => [ + "#{foss_folder}/foss_file.rb" + ] + } + ) + end + end + + context 'when EE' do + it 'returns the correct children types' do + expect(subject).to include( + 'ee' => { + 'NoteableInterface' => [ + "#{foss_folder}/foss_file.rb", + "#{ee_folder}/ee_file.rb" + ] + } + ) + end + end + + context 'when JH' do + it 'returns the correct children types' do + expect(subject).to include( + 'jh' => { + 'NoteableInterface' => [ + "#{foss_folder}/foss_file.rb", + "#{ee_folder}/ee_file.rb", + "#{jh_folder}/jh_file.rb" + ] + } + ) + end + end + end + end + + describe '#filename_to_class_name' do + let(:filename) { 'app/graphql/types/user_merge_request_interaction_type.rb' } + + subject { instance.filename_to_class_name(filename) } + + it 'returns the correct class name' do + expect(subject).to eq('UserMergeRequestInteractionType') + end + end + + describe '#filename_to_spec_filename' do + let(:filename) { 'ee/app/graphql/ee/types/application_type.rb' } + let(:expected_spec_filename) { 'ee/spec/graphql/ee/types/application_type_spec.rb' } + + subject { instance.filename_to_spec_filename(filename) } + + context 'when the spec file exists' do + before do + allow(File).to receive(:exist?).with(expected_spec_filename).and_return(true) + end + + it 'returns the correct spec filename' do + expect(subject).to eq(expected_spec_filename) + end + end + + context 'when the spec file does not exist' do + before do + allow(File).to receive(:exist?).with(expected_spec_filename).and_return(false) + end + + it 'returns nil' do + expect(subject).to eq(nil) + end + end + end +end diff --git a/spec/tooling/lib/tooling/mappings/js_to_system_specs_mappings_spec.rb b/spec/tooling/lib/tooling/mappings/js_to_system_specs_mappings_spec.rb index 72e02547938..e1f35bedebb 100644 --- a/spec/tooling/lib/tooling/mappings/js_to_system_specs_mappings_spec.rb +++ b/spec/tooling/lib/tooling/mappings/js_to_system_specs_mappings_spec.rb @@ -6,33 +6,63 @@ require_relative '../../../../../tooling/lib/tooling/mappings/js_to_system_specs RSpec.describe Tooling::Mappings::JsToSystemSpecsMappings, feature_category: :tooling do # We set temporary folders, and those readers give access to those folder paths attr_accessor :js_base_folder, :system_specs_base_folder + attr_accessor :changed_files_file, :predictive_tests_file + + let(:changed_files_pathname) { changed_files_file.path } + let(:predictive_tests_pathname) { predictive_tests_file.path } + let(:changed_files_content) { "changed_file1 changed_file2" } + let(:predictive_tests_content) { "previously_matching_spec.rb" } + + let(:instance) do + described_class.new( + changed_files_pathname, + predictive_tests_pathname, + system_specs_base_folder: system_specs_base_folder, + js_base_folder: js_base_folder + ) + end around do |example| + self.changed_files_file = Tempfile.new('changed_files_file') + self.predictive_tests_file = Tempfile.new('predictive_tests_file') + Dir.mktmpdir do |tmp_js_base_folder| Dir.mktmpdir do |tmp_system_specs_base_folder| self.system_specs_base_folder = tmp_system_specs_base_folder self.js_base_folder = tmp_js_base_folder - example.run + # 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 + predictive_tests_file.close + changed_files_file.unlink + predictive_tests_file.unlink + end end end end + before do + # We write into the temp files initially, to later check how the code modified those files + File.write(changed_files_pathname, changed_files_content) + File.write(predictive_tests_pathname, predictive_tests_content) + end + describe '#execute' do - let(:instance) do - described_class.new( - system_specs_base_folder: system_specs_base_folder, - js_base_folder: js_base_folder - ) - end + subject { instance.execute } - subject { instance.execute(changed_files) } + before do + File.write(changed_files_pathname, changed_files.join(' ')) + end context 'when no JS files were changed' do let(:changed_files) { [] } - it 'returns nothing' do - expect(subject).to match_array([]) + it 'does not change the output file' do + expect { subject }.not_to change { File.read(predictive_tests_pathname) } end end @@ -40,8 +70,8 @@ RSpec.describe Tooling::Mappings::JsToSystemSpecsMappings, feature_category: :to let(:changed_files) { ["#{js_base_folder}/issues/secret_values.js"] } context 'when the JS files are not present on disk' do - it 'returns nothing' do - expect(subject).to match_array([]) + it 'does not change the output file' do + expect { subject }.not_to change { File.read(predictive_tests_pathname) } end end @@ -52,8 +82,8 @@ RSpec.describe Tooling::Mappings::JsToSystemSpecsMappings, feature_category: :to end context 'when no system specs match the JS keyword' do - it 'returns nothing' do - expect(subject).to match_array([]) + it 'does not change the output file' do + expect { subject }.not_to change { File.read(predictive_tests_pathname) } end end @@ -63,8 +93,10 @@ RSpec.describe Tooling::Mappings::JsToSystemSpecsMappings, feature_category: :to File.write("#{system_specs_base_folder}/confidential_issues/issues_spec.rb", "a test") end - it 'returns something' do - expect(subject).to match_array(["#{system_specs_base_folder}/confidential_issues/issues_spec.rb"]) + it 'adds the new specs to the output file' do + expect { subject }.to change { File.read(predictive_tests_pathname) } + .from(predictive_tests_content) + .to("#{predictive_tests_content} #{system_specs_base_folder}/confidential_issues/issues_spec.rb") end end end @@ -72,12 +104,13 @@ RSpec.describe Tooling::Mappings::JsToSystemSpecsMappings, feature_category: :to end describe '#filter_files' do - subject { described_class.new(js_base_folder: js_base_folder).filter_files(changed_files) } + subject { instance.filter_files } before do File.write("#{js_base_folder}/index.js", "index.js") File.write("#{js_base_folder}/index-with-ee-in-it.js", "index-with-ee-in-it.js") File.write("#{js_base_folder}/index-with-jh-in-it.js", "index-with-jh-in-it.js") + File.write(changed_files_pathname, changed_files.join(' ')) end context 'when no files were changed' do @@ -117,7 +150,7 @@ RSpec.describe Tooling::Mappings::JsToSystemSpecsMappings, feature_category: :to end describe '#construct_js_keywords' do - subject { described_class.new.construct_js_keywords(js_files) } + subject { described_class.new(changed_files_file, predictive_tests_file).construct_js_keywords(js_files) } let(:js_files) do %w[ @@ -129,11 +162,49 @@ RSpec.describe Tooling::Mappings::JsToSystemSpecsMappings, feature_category: :to it 'returns a singularized keyword based on the first folder the file is in' do expect(subject).to eq(%w[board query]) end + + context 'when the files are under the pages folder' do + let(:js_files) do + %w[ + app/assets/javascripts/pages/boards/issue_board_filters.js + ee/app/assets/javascripts/pages2/queries/epic_due_date.query.graphql + ee/app/assets/javascripts/queries/epic_due_date.query.graphql + ] + end + + it 'captures the second folder' do + expect(subject).to eq(%w[board pages2 query]) + end + end end describe '#system_specs_for_edition' do subject do - described_class.new(system_specs_base_folder: system_specs_base_folder).system_specs_for_edition(edition) + instance.system_specs_for_edition(edition) + end + + let(:edition) { nil } + + context 'when a file is not a ruby spec' do + before do + File.write("#{system_specs_base_folder}/issues_spec.tar.gz", "a test") + end + + it 'does not return that file' do + expect(subject).to be_empty + end + end + + context 'when a file is a ruby spec' do + let(:spec_pathname) { "#{system_specs_base_folder}/issues_spec.rb" } + + before do + File.write(spec_pathname, "a test") + end + + it 'returns that file' do + expect(subject).to match_array(spec_pathname) + end end context 'when FOSS' do diff --git a/spec/tooling/lib/tooling/mappings/partial_to_views_mappings_spec.rb b/spec/tooling/lib/tooling/mappings/partial_to_views_mappings_spec.rb new file mode 100644 index 00000000000..75ddee18985 --- /dev/null +++ b/spec/tooling/lib/tooling/mappings/partial_to_views_mappings_spec.rb @@ -0,0 +1,280 @@ +# frozen_string_literal: true + +require 'tempfile' +require 'fileutils' +require_relative '../../../../../tooling/lib/tooling/mappings/partial_to_views_mappings' + +RSpec.describe Tooling::Mappings::PartialToViewsMappings, feature_category: :tooling do + attr_accessor :view_base_folder, :changed_files_file, :views_with_partials_file + + let(:instance) do + described_class.new(changed_files_pathname, views_with_partials_pathname, view_base_folder: view_base_folder) + end + + let(:changed_files_pathname) { changed_files_file.path } + let(:views_with_partials_pathname) { views_with_partials_file.path } + let(:changed_files_content) { "changed_file1 changed_file2" } + let(:views_with_partials_content) { "previously_added_view.html.haml" } + + around do |example| + self.changed_files_file = Tempfile.new('changed_files_file') + self.views_with_partials_file = Tempfile.new('views_with_partials_file') + + # See https://ruby-doc.org/stdlib-1.9.3/libdoc/tempfile/rdoc/ + # Tempfile.html#class-Tempfile-label-Explicit+close + begin + Dir.mktmpdir do |tmp_views_base_folder| + self.view_base_folder = tmp_views_base_folder + example.run + end + ensure + changed_files_file.close + views_with_partials_file.close + changed_files_file.unlink + views_with_partials_file.unlink + end + end + + before do + # We write into the temp files initially, to check how the code modified those files + File.write(changed_files_pathname, changed_files_content) + File.write(views_with_partials_pathname, views_with_partials_content) + end + + describe '#execute' do + subject { instance.execute } + + let(:changed_files) { ["#{view_base_folder}/my_view.html.haml"] } + let(:changed_files_content) { changed_files.join(" ") } + + before do + # We create all of the changed_files, so that they are part of the filtered files + changed_files.each { |changed_file| FileUtils.touch(changed_file) } + end + + it 'does not modify the content of the input file' do + expect { subject }.not_to change { File.read(changed_files_pathname) } + end + + context 'when no partials were modified' do + it 'does not change the output file' do + expect { subject }.not_to change { File.read(views_with_partials_pathname) } + end + end + + context 'when some partials were modified' do + let(:changed_files) do + [ + "#{view_base_folder}/my_view.html.haml", + "#{view_base_folder}/_my_partial.html.haml", + "#{view_base_folder}/_my_other_partial.html.haml" + ] + end + + before do + # We create a red-herring partial to have a more convincing test suite + FileUtils.touch("#{view_base_folder}/_another_partial.html.haml") + end + + context 'when the partials are not included in any views' do + before do + File.write("#{view_base_folder}/my_view.html.haml", "render 'another_partial'") + end + + it 'does not change the output file' do + expect { subject }.not_to change { File.read(views_with_partials_pathname) } + end + end + + context 'when the partials are included in views' do + before do + File.write("#{view_base_folder}/my_view.html.haml", "render 'my_partial'") + end + + it 'writes the view including the partial to the output' do + expect { subject }.to change { File.read(views_with_partials_pathname) } + .from(views_with_partials_content) + .to(views_with_partials_content + " #{view_base_folder}/my_view.html.haml") + end + end + end + end + + describe '#filter_files' do + subject { instance.filter_files } + + let(:changed_files_content) { file_path } + + context 'when the file does not exist on disk' do + let(:file_path) { "#{view_base_folder}/_index.html.erb" } + + it 'returns an empty array' do + expect(subject).to be_empty + end + end + + context 'when the file exists on disk' do + before do + File.write(file_path, "I am a partial!") + end + + context 'when the file is not in the view base folders' do + let(:file_path) { "/tmp/_index.html.haml" } + + it 'returns an empty array' do + expect(subject).to be_empty + end + end + + context 'when the filename does not start with an underscore' do + let(:file_path) { "#{view_base_folder}/index.html.haml" } + + 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(:file_path) { "#{view_base_folder}/_index.html.erb" } + + it 'returns an empty array' do + expect(subject).to be_empty + end + end + + context 'when the file is a partial' do + let(:file_path) { "#{view_base_folder}/_index.html.haml" } + + it 'returns the file' do + expect(subject).to match_array(file_path) + end + end + end + end + + describe '#extract_partial_keyword' do + subject { instance.extract_partial_keyword('ee/app/views/shared/_new_project_item_vue_select.html.haml') } + + it 'returns the correct partial keyword' do + expect(subject).to eq('new_project_item_vue_select') + end + end + + describe '#view_includes_modified_partial?' do + subject { instance.view_includes_modified_partial?(view_file, included_partial_name) } + + context 'when the included partial name is relative to the view file' do + let(:view_file) { "#{view_base_folder}/components/my_view.html.haml" } + let(:included_partial_name) { 'subfolder/relative_partial' } + + before do + FileUtils.mkdir_p("#{view_base_folder}/components/subfolder") + File.write(changed_files_content, "I am a partial!") + end + + context 'when the partial is not part of the changed files' do + let(:changed_files_content) { "#{view_base_folder}/components/subfolder/_not_the_partial.html.haml" } + + it 'returns false' do + expect(subject).to be_falsey + end + end + + context 'when the partial is part of the changed files' do + let(:changed_files_content) { "#{view_base_folder}/components/subfolder/_relative_partial.html.haml" } + + it 'returns true' do + expect(subject).to be_truthy + end + end + end + + context 'when the included partial name is relative to the base views folder' do + let(:view_file) { "#{view_base_folder}/components/my_view.html.haml" } + let(:included_partial_name) { 'shared/absolute_partial' } + + before do + FileUtils.mkdir_p("#{view_base_folder}/components") + FileUtils.mkdir_p("#{view_base_folder}/shared") + File.write(changed_files_content, "I am a partial!") + end + + context 'when the partial is not part of the changed files' do + let(:changed_files_content) { "#{view_base_folder}/shared/not_the_partial" } + + it 'returns false' do + expect(subject).to be_falsey + end + end + + context 'when the partial is part of the changed files' do + let(:changed_files_content) { "#{view_base_folder}/shared/_absolute_partial.html.haml" } + + it 'returns true' do + expect(subject).to be_truthy + end + end + end + end + + describe '#reconstruct_partial_filename' do + subject { instance.reconstruct_partial_filename(partial_name) } + + context 'when the partial does not contain a path' do + let(:partial_name) { 'sidebar' } + + it 'returns the correct filename' do + expect(subject).to eq('_sidebar.html.haml') + end + end + + context 'when the partial contains a path' do + let(:partial_name) { 'shared/components/sidebar' } + + it 'returns the correct filename' do + expect(subject).to eq('shared/components/_sidebar.html.haml') + end + end + end + + describe '#find_pattern_in_file' do + let(:subject) { instance.find_pattern_in_file(file.path, /pattern/) } + let(:file) { Tempfile.new('find_pattern_in_file') } + + before do + file.write(file_content) + file.close + end + + context 'when the file contains the pattern' do + let(:file_content) do + <<~FILE + Beginning of file + + pattern + pattern + pattern + + End of file + FILE + end + + it 'returns the pattern once' do + expect(subject).to match_array(%w[pattern]) + end + end + + context 'when the file does not contain the pattern' do + let(:file_content) do + <<~FILE + Beginning of file + End of file + FILE + end + + it 'returns an empty array' do + expect(subject).to match_array([]) + end + end + end +end diff --git a/spec/tooling/lib/tooling/mappings/view_to_js_mappings_spec.rb b/spec/tooling/lib/tooling/mappings/view_to_js_mappings_spec.rb index eaa0124370d..6d007843716 100644 --- a/spec/tooling/lib/tooling/mappings/view_to_js_mappings_spec.rb +++ b/spec/tooling/lib/tooling/mappings/view_to_js_mappings_spec.rb @@ -6,37 +6,67 @@ require_relative '../../../../../tooling/lib/tooling/mappings/view_to_js_mapping RSpec.describe Tooling::Mappings::ViewToJsMappings, feature_category: :tooling do # We set temporary folders, and those readers give access to those folder paths attr_accessor :view_base_folder, :js_base_folder + attr_accessor :changed_files_file, :predictive_tests_file + + let(:changed_files_pathname) { changed_files_file.path } + let(:predictive_tests_pathname) { predictive_tests_file.path } + let(:changed_files_content) { "changed_file1 changed_file2" } + let(:predictive_tests_content) { "previously_matching_spec.rb" } + + let(:instance) do + described_class.new( + changed_files_pathname, + predictive_tests_pathname, + view_base_folder: view_base_folder, + js_base_folder: js_base_folder + ) + end around do |example| + self.changed_files_file = Tempfile.new('changed_files_file') + self.predictive_tests_file = Tempfile.new('matching_tests') + Dir.mktmpdir do |tmp_js_base_folder| Dir.mktmpdir do |tmp_views_base_folder| self.js_base_folder = tmp_js_base_folder self.view_base_folder = tmp_views_base_folder - example.run + # 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 + predictive_tests_file.close + changed_files_file.unlink + predictive_tests_file.unlink + end end end end - describe '#execute' do - let(:instance) do - described_class.new( - view_base_folder: view_base_folder, - js_base_folder: js_base_folder - ) - end + before do + # We write into the temp files initially, to later check how the code modified those files + File.write(changed_files_pathname, changed_files_content) + File.write(predictive_tests_pathname, predictive_tests_content) + end + describe '#execute' do let(:changed_files) { %W[#{view_base_folder}/index.html] } - subject { instance.execute(changed_files) } + subject { instance.execute } + + before do + File.write(changed_files_pathname, changed_files.join(' ')) + end context 'when no view files have been changed' do before do allow(instance).to receive(:filter_files).and_return([]) end - it 'returns nothing' do - expect(subject).to match_array([]) + it 'does not change the output file' do + expect { subject }.not_to change { File.read(predictive_tests_pathname) } end end @@ -53,8 +83,8 @@ RSpec.describe Tooling::Mappings::ViewToJsMappings, feature_category: :tooling d FILE end - it 'returns nothing' do - expect(subject).to match_array([]) + it 'does not change the output file' do + expect { subject }.not_to change { File.read(predictive_tests_pathname) } end end @@ -70,8 +100,8 @@ RSpec.describe Tooling::Mappings::ViewToJsMappings, feature_category: :tooling d end context 'when no matching JS files are found' do - it 'returns nothing' do - expect(subject).to match_array([]) + it 'does not change the output file' do + expect { subject }.not_to change { File.read(predictive_tests_pathname) } end end @@ -90,8 +120,10 @@ RSpec.describe Tooling::Mappings::ViewToJsMappings, feature_category: :tooling d File.write("#{js_base_folder}/index.js", index_js_content) end - it 'returns the matching JS files' do - expect(subject).to match_array(["#{js_base_folder}/index.js"]) + it 'adds the matching JS files to the output' do + expect { subject }.to change { File.read(predictive_tests_pathname) } + .from(predictive_tests_content) + .to("#{predictive_tests_content} #{js_base_folder}/index.js") end end end @@ -135,17 +167,20 @@ RSpec.describe Tooling::Mappings::ViewToJsMappings, feature_category: :tooling d end it 'scans those partials for the HTML attribute value' do - expect(subject).to match_array(["#{js_base_folder}/index.js"]) + expect { subject }.to change { File.read(predictive_tests_pathname) } + .from(predictive_tests_content) + .to("#{predictive_tests_content} #{js_base_folder}/index.js") end end end describe '#filter_files' do - subject { described_class.new(view_base_folder: view_base_folder).filter_files(changed_files) } + subject { instance.filter_files } before do File.write("#{js_base_folder}/index.js", "index.js") File.write("#{view_base_folder}/index.html", "index.html") + File.write(changed_files_pathname, changed_files.join(' ')) end context 'when no files were changed' do @@ -182,7 +217,7 @@ RSpec.describe Tooling::Mappings::ViewToJsMappings, feature_category: :tooling d end describe '#find_partials' do - subject { described_class.new(view_base_folder: view_base_folder).find_partials(file_path) } + subject { instance.find_partials(file_path) } let(:file_path) { "#{view_base_folder}/my_html_file.html" } @@ -230,12 +265,12 @@ RSpec.describe Tooling::Mappings::ViewToJsMappings, feature_category: :tooling d = render partial: "subfolder/my-partial4" = render(partial:"subfolder/my-partial5", path: 'else') = render partial:"subfolder/my-partial6" - = render_if_exist("subfolder/my-partial7", path: 'else') - = render_if_exist "subfolder/my-partial8" - = render_if_exist(partial: "subfolder/my-partial9", path: 'else') - = render_if_exist partial: "subfolder/my-partial10" - = render_if_exist(partial:"subfolder/my-partial11", path: 'else') - = render_if_exist partial:"subfolder/my-partial12" + = render_if_exists("subfolder/my-partial7", path: 'else') + = render_if_exists "subfolder/my-partial8" + = render_if_exists(partial: "subfolder/my-partial9", path: 'else') + = render_if_exists partial: "subfolder/my-partial10" + = render_if_exists(partial:"subfolder/my-partial11", path: 'else') + = render_if_exists partial:"subfolder/my-partial12" End of file FILE @@ -275,7 +310,7 @@ RSpec.describe Tooling::Mappings::ViewToJsMappings, feature_category: :tooling d end describe '#find_pattern_in_file' do - let(:subject) { described_class.new.find_pattern_in_file(file.path, /pattern/) } + let(:subject) { instance.find_pattern_in_file(file.path, /pattern/) } let(:file) { Tempfile.new('find_pattern_in_file') } before do diff --git a/spec/tooling/lib/tooling/mappings/view_to_system_specs_mappings_spec.rb b/spec/tooling/lib/tooling/mappings/view_to_system_specs_mappings_spec.rb new file mode 100644 index 00000000000..b8a13c50c9b --- /dev/null +++ b/spec/tooling/lib/tooling/mappings/view_to_system_specs_mappings_spec.rb @@ -0,0 +1,127 @@ +# frozen_string_literal: true + +require 'tempfile' +require 'fileutils' +require_relative '../../../../../tooling/lib/tooling/mappings/view_to_system_specs_mappings' + +RSpec.describe Tooling::Mappings::ViewToSystemSpecsMappings, feature_category: :tooling do + attr_accessor :view_base_folder, :changed_files_file, :predictive_tests_file + + let(:instance) do + described_class.new(changed_files_pathname, predictive_tests_pathname, view_base_folder: view_base_folder) + end + + let(:changed_files_pathname) { changed_files_file.path } + let(:predictive_tests_pathname) { predictive_tests_file.path } + let(:changed_files_content) { "changed_file1 changed_file2" } + let(:predictive_tests_initial_content) { "previously_added_spec.rb" } + + around do |example| + self.changed_files_file = Tempfile.new('changed_files_file') + self.predictive_tests_file = Tempfile.new('predictive_tests_file') + + # See https://ruby-doc.org/stdlib-1.9.3/libdoc/tempfile/rdoc/ + # Tempfile.html#class-Tempfile-label-Explicit+close + begin + Dir.mktmpdir do |tmp_views_base_folder| + self.view_base_folder = tmp_views_base_folder + example.run + end + ensure + changed_files_file.close + predictive_tests_file.close + changed_files_file.unlink + predictive_tests_file.unlink + end + end + + before do + FileUtils.mkdir_p("#{view_base_folder}/app/views/dashboard") + + # We write into the temp files initially, to check how the code modified those files + File.write(changed_files_pathname, changed_files_content) + File.write(predictive_tests_pathname, predictive_tests_initial_content) + end + + shared_examples 'writes nothing to the output file' do + it 'writes nothing to the output file' do + expect { subject }.not_to change { File.read(changed_files_pathname) } + end + end + + describe '#execute' do + subject { instance.execute } + + let(:changed_files) { ["#{view_base_folder}/app/views/dashboard/my_view.html.haml"] } + let(:changed_files_content) { changed_files.join(" ") } + + before do + # We create all of the changed_files, so that they are part of the filtered files + changed_files.each { |changed_file| FileUtils.touch(changed_file) } + end + + context 'when the changed files are not view files' do + let(:changed_files) { ["#{view_base_folder}/app/views/dashboard/my_helper.rb"] } + + it_behaves_like 'writes nothing to the output file' + end + + context 'when the changed files are view files' do + let(:changed_files) { ["#{view_base_folder}/app/views/dashboard/my_view.html.haml"] } + + context 'when the view files do not exist on disk' do + before do + allow(File).to receive(:exist?).with(changed_files.first).and_return(false) + end + + it_behaves_like 'writes nothing to the output file' + end + + context 'when the view files exist on disk' do + context 'when no feature match the view' do + # Nothing in this context, because the spec corresponding to `changed_files` doesn't exist + + it_behaves_like 'writes nothing to the output file' + end + + context 'when there is a feature spec that exactly matches the view' do + let(:expected_feature_spec) { "#{view_base_folder}/spec/features/dashboard/my_view_spec.rb" } + + before do + allow(File).to receive(:exist?).and_call_original + allow(File).to receive(:exist?).with(expected_feature_spec).and_return(true) + end + + it 'writes that feature spec to the output file' do + expect { subject }.to change { File.read(predictive_tests_pathname) } + .from(predictive_tests_initial_content) + .to("#{predictive_tests_initial_content} #{expected_feature_spec}") + end + end + + context 'when there is a feature spec that matches the parent folder of the view' do + let(:expected_feature_specs) do + [ + "#{view_base_folder}/spec/features/dashboard/another_feature_spec.rb", + "#{view_base_folder}/spec/features/dashboard/other_feature_spec.rb" + ] + end + + before do + FileUtils.mkdir_p("#{view_base_folder}/spec/features/dashboard") + + expected_feature_specs.each do |expected_feature_spec| + FileUtils.touch(expected_feature_spec) + end + end + + it 'writes all of the feature specs for the parent folder to the output file' do + expect { subject }.to change { File.read(predictive_tests_pathname) } + .from(predictive_tests_initial_content) + .to("#{predictive_tests_initial_content} #{expected_feature_specs.join(' ')}") + end + end + end + end + end +end diff --git a/spec/tooling/lib/tooling/parallel_rspec_runner_spec.rb b/spec/tooling/lib/tooling/parallel_rspec_runner_spec.rb index 4b44d991d89..b7b39c37819 100644 --- a/spec/tooling/lib/tooling/parallel_rspec_runner_spec.rb +++ b/spec/tooling/lib/tooling/parallel_rspec_runner_spec.rb @@ -1,90 +1,84 @@ # frozen_string_literal: true +require 'tempfile' + require_relative '../../../../tooling/lib/tooling/parallel_rspec_runner' RSpec.describe Tooling::ParallelRSpecRunner do # rubocop:disable RSpec/FilePath describe '#run' do - let(:allocator) { instance_double(Knapsack::Allocator) } - let(:rspec_args) { '--seed 123' } - let(:filter_tests_file) { 'tests.txt' } - let(:node_tests) { %w[01_spec.rb 03_spec.rb 05_spec.rb] } - let(:filter_tests) { '01_spec.rb 02_spec.rb 03_spec.rb' } let(:test_dir) { 'spec' } + let(:node_tests) { %w[01_spec.rb 03_spec.rb] } + let(:allocator) { instance_double(Knapsack::Allocator, test_dir: test_dir, node_tests: node_tests) } + let(:allocator_builder) { double(Knapsack::AllocatorBuilder, allocator: allocator) } # rubocop:disable RSpec/VerifiedDoubles + + let(:filter_tests) { [] } + let(:filter_tests_file) { nil } + let(:filter_tests_file_path) { nil } before do + allow(Knapsack::AllocatorBuilder).to receive(:new).and_return(allocator_builder) allow(Knapsack.logger).to receive(:info) - allow(allocator).to receive(:node_tests).and_return(node_tests) - allow(allocator).to receive(:test_dir).and_return(test_dir) - allow(File).to receive(:exist?).with(filter_tests_file).and_return(true) - allow(File).to receive(:read).and_call_original - allow(File).to receive(:read).with(filter_tests_file).and_return(filter_tests) - allow(subject).to receive(:exec) end - subject { described_class.new(allocator: allocator, filter_tests_file: filter_tests_file, rspec_args: rspec_args) } - - shared_examples 'runs node tests' do - it 'runs rspec with tests allocated for this node' do - expect_command(%w[bundle exec rspec --seed 123 --default-path spec -- 01_spec.rb 03_spec.rb 05_spec.rb]) - - subject.run + after do + if filter_tests_file.respond_to?(:close) + filter_tests_file.close + File.unlink(filter_tests_file) end end - context 'given filter tests' do - it 'reads filter tests file for list of tests' do - expect(File).to receive(:read).with(filter_tests_file) + subject { described_class.new(filter_tests_file: filter_tests_file_path, rspec_args: rspec_args) } - subject.run - end + shared_examples 'runs node tests' do + let(:rspec_args) { nil } - it 'runs rspec filter tests that are allocated for this node' do - expect_command(%w[bundle exec rspec --seed 123 --default-path spec -- 01_spec.rb 03_spec.rb]) + it 'runs rspec with tests allocated for this node' do + expect(allocator_builder).to receive(:filter_tests=).with(filter_tests) + expect_command(%W[bundle exec rspec#{rspec_args} --] + node_tests) subject.run end - - context 'when there is no intersect between allocated tests and filtered tests' do - let(:filter_tests) { '99_spec.rb' } - - it 'does not run rspec' do - expect(subject).not_to receive(:exec) - - subject.run - end - end end - context 'with empty filter tests file' do - let(:filter_tests) { '' } + context 'without filter_tests_file option' do + subject { described_class.new(rspec_args: rspec_args) } it_behaves_like 'runs node tests' end - context 'without filter_tests_file option' do - let(:filter_tests_file) { nil } + context 'given filter tests file' do + let(:filter_tests_file) do + Tempfile.create.tap do |f| # rubocop:disable Rails/SaveBang + f.write(filter_tests.join(' ')) + f.rewind + end + end - it_behaves_like 'runs node tests' - end + let(:filter_tests_file_path) { filter_tests_file.path } - context 'if filter_tests_file does not exist' do - before do - allow(File).to receive(:exist?).with(filter_tests_file).and_return(false) + context 'when filter_tests_file is empty' do + it_behaves_like 'runs node tests' end - it_behaves_like 'runs node tests' - end + context 'when filter_tests_file does not exist' do + let(:filter_tests_file_path) { 'doesnt_exist' } - context 'without rspec args' do - let(:rspec_args) { nil } + it_behaves_like 'runs node tests' + end - it 'runs rspec with without extra arguments' do - expect_command(%w[bundle exec rspec --default-path spec -- 01_spec.rb 03_spec.rb]) + context 'when filter_tests_file is not empty' do + let(:filter_tests) { %w[01_spec.rb 02_spec.rb 03_spec.rb] } - subject.run + it_behaves_like 'runs node tests' end end + context 'with rspec args' do + let(:rspec_args) { ' --seed 123' } + + it_behaves_like 'runs node tests' + end + def expect_command(cmd) expect(subject).to receive(:exec).with(*cmd) end diff --git a/spec/tooling/lib/tooling/predictive_tests_spec.rb b/spec/tooling/lib/tooling/predictive_tests_spec.rb new file mode 100644 index 00000000000..b82364fe6f6 --- /dev/null +++ b/spec/tooling/lib/tooling/predictive_tests_spec.rb @@ -0,0 +1,134 @@ +# frozen_string_literal: true + +require 'tempfile' +require 'fileutils' +require_relative '../../../../tooling/lib/tooling/predictive_tests' +require_relative '../../../support/helpers/stub_env' + +RSpec.describe Tooling::PredictiveTests, feature_category: :tooling do + include StubENV + + let(:instance) { described_class.new } + let(:matching_tests_initial_content) { 'initial_matching_spec' } + let(:fixtures_mapping_content) { '{}' } + + attr_accessor :changed_files, :changed_files_path, :fixtures_mapping, + :matching_js_files, :matching_tests, :views_with_partials + + around do |example| + self.changed_files = Tempfile.new('test-folder/changed_files.txt') + self.changed_files_path = changed_files.path + self.fixtures_mapping = Tempfile.new('test-folder/fixtures_mapping.txt') + self.matching_js_files = Tempfile.new('test-folder/matching_js_files.txt') + self.matching_tests = Tempfile.new('test-folder/matching_tests.txt') + self.views_with_partials = Tempfile.new('test-folder/views_with_partials.txt') + + # See https://ruby-doc.org/stdlib-1.9.3/libdoc/tempfile/rdoc/ + # Tempfile.html#class-Tempfile-label-Explicit+close + begin + # In practice, we let PredictiveTests create the file, and we just + # use its file name. + changed_files.close + changed_files.unlink + + example.run + ensure + # Since example.run can create the file again, let's remove it again + FileUtils.rm_f(changed_files_path) + fixtures_mapping.close + fixtures_mapping.unlink + matching_js_files.close + matching_js_files.unlink + matching_tests.close + matching_tests.unlink + views_with_partials.close + views_with_partials.unlink + end + end + + before do + stub_env( + 'RSPEC_CHANGED_FILES_PATH' => changed_files_path, + 'RSPEC_MATCHING_TESTS_PATH' => matching_tests.path, + 'RSPEC_VIEWS_INCLUDING_PARTIALS_PATH' => views_with_partials.path, + 'FRONTEND_FIXTURES_MAPPING_PATH' => fixtures_mapping.path, + 'RSPEC_MATCHING_JS_FILES_PATH' => matching_js_files.path, + 'RSPEC_TESTS_MAPPING_ENABLED' => "false", + 'RSPEC_TESTS_MAPPING_PATH' => '/tmp/does-not-exist.out' + ) + + # We write some data to later on verify that we only append to this file. + File.write(matching_tests.path, matching_tests_initial_content) + File.write(fixtures_mapping.path, fixtures_mapping_content) + + allow(Gitlab).to receive(:configure) + end + + describe '#execute' do + subject { instance.execute } + + context 'when ENV variables are missing' do + before do + stub_env( + 'RSPEC_CHANGED_FILES_PATH' => '', + 'FRONTEND_FIXTURES_MAPPING_PATH' => '' + ) + end + + it 'raises an error' do + expect { subject }.to raise_error( + '[predictive tests] Missing ENV variable(s): RSPEC_CHANGED_FILES_PATH,FRONTEND_FIXTURES_MAPPING_PATH.' + ) + end + end + + context 'when all ENV variables are provided' do + before do + change = double('GitLab::Change') # rubocop:disable RSpec/VerifiedDoubles + allow(change).to receive_message_chain(:to_h, :values_at) + .and_return([changed_files_content, changed_files_content]) + + allow(Gitlab).to receive_message_chain(:merge_request_changes, :changes) + .and_return([change]) + end + + context 'when no files were changed' do + let(:changed_files_content) { '' } + + it 'does not change files other than RSPEC_CHANGED_FILES_PATH' do + expect { subject }.not_to change { File.read(matching_tests.path) } + expect { subject }.not_to change { File.read(views_with_partials.path) } + expect { subject }.not_to change { File.read(fixtures_mapping.path) } + expect { subject }.not_to change { File.read(matching_js_files.path) } + end + end + + context 'when some files used for frontend fixtures were changed' do + let(:changed_files_content) { 'app/models/todo.rb' } + let(:changed_files_matching_test) { 'spec/models/todo_spec.rb' } + let(:matching_frontend_fixture) { 'tmp/tests/frontend/fixtures-ee/todos/todos.html' } + let(:fixtures_mapping_content) do + JSON.dump(changed_files_matching_test => [matching_frontend_fixture]) # rubocop:disable Gitlab/Json + end + + it 'writes to RSPEC_CHANGED_FILES_PATH with API contents and appends with matching fixtures' do + subject + + expect(File.read(changed_files_path)).to eq("#{changed_files_content} #{matching_frontend_fixture}") + end + + it 'appends the spec file to RSPEC_MATCHING_TESTS_PATH' do + expect { subject }.to change { File.read(matching_tests.path) } + .from(matching_tests_initial_content) + .to("#{matching_tests_initial_content} #{changed_files_matching_test}") + end + + it 'does not change files other than RSPEC_CHANGED_FILES_PATH nor RSPEC_MATCHING_TESTS_PATH' do + expect { subject }.not_to change { File.read(views_with_partials.path) } + expect { subject }.not_to change { File.read(fixtures_mapping.path) } + expect { subject }.not_to change { File.read(matching_js_files.path) } + end + end + end + end +end diff --git a/spec/tooling/quality/test_level_spec.rb b/spec/tooling/quality/test_level_spec.rb index aac7d19c079..a7e4e42206a 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, feature_category: :tooling do context 'when level is unit' do it 'returns a pattern' do expect(subject.pattern(:unit)) - .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") + .to eq("spec/{bin,channels,components,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}{,/**/}*_spec.rb") end end @@ -121,7 +121,7 @@ RSpec.describe Quality::TestLevel, feature_category: :tooling do context 'when level is unit' do it 'returns a regexp' do expect(subject.regexp(:unit)) - .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)/}) + .to eq(%r{spec/(bin|channels|components|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)/}) end end @@ -167,6 +167,13 @@ RSpec.describe Quality::TestLevel, feature_category: :tooling do end end + context 'when start_with == true' do + it 'returns a regexp' do + expect(described_class.new(['ee/']).regexp(:system, true)) + .to eq(%r{^(ee/)spec/(features)/}) + end + end + describe 'performance' do it 'memoizes the regexp for a given level' do expect(subject.regexp(:system).object_id).to eq(subject.regexp(:system).object_id) diff --git a/spec/tooling/rspec_flaky/config_spec.rb b/spec/tooling/rspec_flaky/config_spec.rb index c95e5475d66..63f42d7c6cc 100644 --- a/spec/tooling/rspec_flaky/config_spec.rb +++ b/spec/tooling/rspec_flaky/config_spec.rb @@ -14,7 +14,6 @@ RSpec.describe RspecFlaky::Config, :aggregate_failures do stub_env('FLAKY_RSPEC_SUITE_REPORT_PATH', nil) stub_env('FLAKY_RSPEC_REPORT_PATH', nil) stub_env('NEW_FLAKY_RSPEC_REPORT_PATH', nil) - stub_env('SKIPPED_FLAKY_TESTS_REPORT_PATH', nil) # Ensure the behavior is the same locally and on CI (where Rails is defined since we run this test as part of the whole suite), i.e. Rails isn't defined allow(described_class).to receive(:rails_path).and_wrap_original do |method, path| path @@ -104,22 +103,4 @@ RSpec.describe RspecFlaky::Config, :aggregate_failures do end end end - - describe '.skipped_flaky_tests_report_path' do - context "when ENV['SKIPPED_FLAKY_TESTS_REPORT_PATH'] is not set" do - it 'returns the default path' do - expect(described_class.skipped_flaky_tests_report_path).to eq('rspec/flaky/skipped_flaky_tests_report.txt') - end - end - - context "when ENV['SKIPPED_FLAKY_TESTS_REPORT_PATH'] is set" do - before do - stub_env('SKIPPED_FLAKY_TESTS_REPORT_PATH', 'foo/skipped_flaky_tests_report.txt') - end - - it 'returns the value of the env variable' do - expect(described_class.skipped_flaky_tests_report_path).to eq('foo/skipped_flaky_tests_report.txt') - end - end - end end |