From bbf320c9734da0886afca06b8a959d478a920d71 Mon Sep 17 00:00:00 2001 From: David Dieulivol <ddieulivol@gitlab.com> Date: Tue, 11 Apr 2023 15:18:38 +0000 Subject: [PATCH] Remove keyword arguments for find_changes We want all classes to have the same methods signature. --- .gitlab/ci/setup.gitlab-ci.yml | 13 +- .rubocop_todo/layout/line_length.yml | 1 - doc/development/pipelines/index.md | 4 +- spec/tooling/lib/tooling/find_changes_spec.rb | 136 +++++++++--------- spec/tooling/lib/tooling/find_tests_spec.rb | 52 +++---- .../lib/tooling/helpers/file_handler_spec.rb | 35 +++-- .../graphql_base_type_mappings_spec.rb | 48 ++++--- .../js_to_system_specs_mappings_spec.rb | 48 ++++--- .../partial_to_views_mappings_spec.rb | 59 ++++---- .../mappings/view_to_js_mappings_spec.rb | 52 +++---- .../view_to_system_specs_mappings_spec.rb | 45 +++--- .../lib/tooling/predictive_tests_spec.rb | 109 ++++++++++++++ tooling/bin/find_changes | 14 -- tooling/bin/find_tests | 9 -- tooling/bin/graphql_base_type_mappings | 9 -- tooling/bin/js_to_system_specs_mappings | 9 -- tooling/bin/partial_to_views_mappings | 9 -- tooling/bin/predictive_tests | 6 + tooling/bin/view_to_js_mappings | 9 -- tooling/bin/view_to_system_specs_mappings | 9 -- tooling/lib/tooling/find_changes.rb | 45 +++--- tooling/lib/tooling/find_tests.rb | 10 +- tooling/lib/tooling/helpers/file_handler.rb | 12 +- .../mappings/graphql_base_type_mappings.rb | 10 +- .../mappings/js_to_system_specs_mappings.rb | 16 +-- .../mappings/partial_to_views_mappings.rb | 12 +- .../tooling/mappings/view_to_js_mappings.rb | 14 +- .../mappings/view_to_system_specs_mappings.rb | 12 +- tooling/lib/tooling/predictive_tests.rb | 53 +++++++ 29 files changed, 506 insertions(+), 354 deletions(-) create mode 100644 spec/tooling/lib/tooling/predictive_tests_spec.rb delete mode 100755 tooling/bin/find_changes delete mode 100755 tooling/bin/find_tests delete mode 100755 tooling/bin/graphql_base_type_mappings delete mode 100755 tooling/bin/js_to_system_specs_mappings delete mode 100755 tooling/bin/partial_to_views_mappings create mode 100755 tooling/bin/predictive_tests delete mode 100755 tooling/bin/view_to_js_mappings delete mode 100755 tooling/bin/view_to_system_specs_mappings create mode 100644 tooling/lib/tooling/predictive_tests.rb diff --git a/.gitlab/ci/setup.gitlab-ci.yml b/.gitlab/ci/setup.gitlab-ci.yml index 089d5a20ceb94..0b5860b3ddb5e 100644 --- a/.gitlab/ci/setup.gitlab-ci.yml +++ b/.gitlab/ci/setup.gitlab-ci.yml @@ -118,17 +118,12 @@ detect-tests: - | if [ -n "$CI_MERGE_REQUEST_IID" ]; then mkdir -p $(dirname "$RSPEC_CHANGED_FILES_PATH") - tooling/bin/find_changes ${RSPEC_CHANGED_FILES_PATH}; - tooling/bin/find_tests ${RSPEC_CHANGED_FILES_PATH} ${RSPEC_MATCHING_TESTS_PATH}; - tooling/bin/partial_to_views_mappings ${RSPEC_CHANGED_FILES_PATH} ${RSPEC_VIEWS_INCLUDING_PARTIALS_PATH}; - tooling/bin/find_tests ${RSPEC_VIEWS_INCLUDING_PARTIALS_PATH} ${RSPEC_MATCHING_TESTS_PATH}; - tooling/bin/js_to_system_specs_mappings ${RSPEC_CHANGED_FILES_PATH} ${RSPEC_MATCHING_TESTS_PATH}; - tooling/bin/graphql_base_type_mappings ${RSPEC_CHANGED_FILES_PATH} ${RSPEC_MATCHING_TESTS_PATH}; - tooling/bin/view_to_system_specs_mappings ${RSPEC_CHANGED_FILES_PATH} ${RSPEC_MATCHING_TESTS_PATH}; - tooling/bin/find_changes ${RSPEC_CHANGED_FILES_PATH} ${RSPEC_MATCHING_TESTS_PATH} ${FRONTEND_FIXTURES_MAPPING_PATH}; + + tooling/bin/predictive_tests + filter_rspec_matched_foss_tests ${RSPEC_MATCHING_TESTS_PATH} ${RSPEC_MATCHING_TESTS_FOSS_PATH}; filter_rspec_matched_ee_tests ${RSPEC_MATCHING_TESTS_PATH} ${RSPEC_MATCHING_TESTS_EE_PATH}; - tooling/bin/view_to_js_mappings ${RSPEC_CHANGED_FILES_PATH} ${RSPEC_MATCHING_JS_FILES_PATH}; + echoinfo "Changed files: $(cat $RSPEC_CHANGED_FILES_PATH)"; echoinfo "Related FOSS RSpec tests: $(cat $RSPEC_MATCHING_TESTS_FOSS_PATH)"; echoinfo "Related EE RSpec tests: $(cat $RSPEC_MATCHING_TESTS_EE_PATH)"; diff --git a/.rubocop_todo/layout/line_length.yml b/.rubocop_todo/layout/line_length.yml index a7d5a7588e5bd..71c9dccb76a1d 100644 --- a/.rubocop_todo/layout/line_length.yml +++ b/.rubocop_todo/layout/line_length.yml @@ -5587,7 +5587,6 @@ Layout/LineLength: - 'spec/workers/todos_destroyer/confidential_issue_worker_spec.rb' - 'spec/workers/update_head_pipeline_for_merge_request_worker_spec.rb' - 'spec/workers/users/deactivate_dormant_users_worker_spec.rb' - - 'tooling/bin/find_changes' - 'tooling/danger/product_intelligence.rb' - 'tooling/danger/project_helper.rb' - 'tooling/danger/specs.rb' diff --git a/doc/development/pipelines/index.md b/doc/development/pipelines/index.md index c19580203e64b..87c7345491dfd 100644 --- a/doc/development/pipelines/index.md +++ b/doc/development/pipelines/index.md @@ -54,9 +54,9 @@ In summary: To identify the RSpec tests that are likely to fail in a merge request, we use the [`test_file_finder` gem](https://gitlab.com/gitlab-org/ci-cd/test_file_finder), with two strategies: - dynamic mapping from test coverage tracing (generated via the [`Crystalball` gem](https://github.com/toptal/crystalball)) - ([see where it's used](https://gitlab.com/gitlab-org/gitlab/-/blob/47d507c93779675d73a05002e2ec9c3c467cd698/tooling/bin/find_tests#L15)) + ([see where it's used](https://gitlab.com/gitlab-org/gitlab/-/blob/2e19d43ba0d456808916650088c0f70d905e7810/tooling/lib/tooling/find_tests.rb#L20)) - static mapping maintained in the [`tests.yml` file](https://gitlab.com/gitlab-org/gitlab/-/blob/master/tests.yml) for special cases that cannot - be mapped via coverage tracing ([see where it's used](https://gitlab.com/gitlab-org/gitlab/-/blob/47d507c93779675d73a05002e2ec9c3c467cd698/tooling/bin/find_tests#L12)) + be mapped via coverage tracing ([see where it's used](https://gitlab.com/gitlab-org/gitlab/-/blob/2e19d43ba0d456808916650088c0f70d905e7810/tooling/lib/tooling/find_tests.rb#L20)) The test mappings contain a map of each source files to a list of test files which is dependent of the source file. diff --git a/spec/tooling/lib/tooling/find_changes_spec.rb b/spec/tooling/lib/tooling/find_changes_spec.rb index 3616732e328cb..5932eb5e91902 100644 --- a/spec/tooling/lib/tooling/find_changes_spec.rb +++ b/spec/tooling/lib/tooling/find_changes_spec.rb @@ -3,57 +3,66 @@ 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( - output_file: output_file, - matched_tests_file: matched_tests_file, - frontend_fixtures_mapping_path: frontend_fixtures_mapping_path - ) + described_class.new(changed_files_pathname, predictive_tests_pathname, frontend_fixtures_mapping_pathname) end - let(:gitlab_client) { double('GitLab') } # rubocop:disable RSpec/VerifiedDoubles - let(:output_file) { 'output.txt' } - let(:output_file_content) { 'first_file second_file' } - let(:matched_tests_file) { 'matched_tests.txt' } - let(:frontend_fixtures_mapping_path) { 'frontend_fixtures_mapping.json' } - let(:file_changes) { ['file1.rb', 'file2.rb'] } + 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(: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', - 'RSPEC_TESTS_MAPPING_PATH' => '/tmp/does-not-exist.out' + 'PROJECT_TOKEN_FOR_CI_SCRIPTS_API_USAGE' => 'dummy-token' ) allow(instance).to receive(:gitlab).and_return(gitlab_client) - allow(File).to receive(:exist?).and_call_original - allow(File).to receive(:read).and_call_original - allow(File).to receive(:write) end describe '#execute' do subject { instance.execute } - context 'when there is no output file' do - let(:output_file) { nil } + 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, "An path to an output file must be given as first argument.") + expect { subject }.to raise_error( + ArgumentError, "A path to the changed files file must be given as first argument." + ) end end - context 'when an output file is provided' do - before do - allow(File).to receive(:exist?).with(output_file).and_return(true) - allow(File).to receive(:read).with(output_file).and_return(output_file_content) - end - + context 'when an changed files file is provided' do it 'does not call GitLab API to retrieve the MR diff' do expect(gitlab_client).not_to receive(:merge_request_changes) @@ -61,64 +70,57 @@ end context 'when there are no file changes' do - let(:output_file_content) { '' } - - it 'writes an empty string to output file' do - expect(File).to receive(:write).with(output_file, '') - - subject + 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 - let(:output_file_content) { 'first_file_changed second_file_changed' } + before do + File.write(changed_files_pathname, changed_files_file_content) + end - it 'writes file changes to output file' do - expect(File).to receive(:write).with(output_file, output_file_content) + let(:changed_files_file_content) { 'first_file_changed second_file_changed' } - subject + # 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(:matched_tests_file) { '' } - - it 'does not add frontend fixtures mapping to the output file' do - expect(File).to receive(:write).with(output_file, output_file_content) + let(:predictive_tests_pathname) { nil } - subject + 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_path) { '' } + let(:frontend_fixtures_mapping_pathname) { nil } - it 'does not add frontend fixtures mapping to the output file' do - expect(File).to receive(:write).with(output_file, output_file_content) - - subject + 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 - allow(File).to receive(:exist?).with(matched_tests_file).and_return(true) - allow(File).to receive(:exist?).with(frontend_fixtures_mapping_path).and_return(true) - - allow(File).to receive(:read).with(matched_tests_file).and_return(matched_tests) - allow(File).to receive(:read).with(frontend_fixtures_mapping_path).and_return(frontend_fixtures_mapping_json) + 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 add frontend fixtures mapping to the output file' do - expect(File).to receive(:write).with(output_file, output_file_content) - - subject + it 'does not change the changed files file' do + expect { subject }.not_to change { File.read(changed_files_pathname) } end end @@ -129,10 +131,20 @@ { match_spec1: spec_mappings }.to_json end - it 'adds the frontend fixtures mappings to the output file' do - expect(File).to receive(:write).with(output_file, "#{output_file_content} #{spec_mappings.join(' ')}") + 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' } - subject + 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 @@ -155,15 +167,9 @@ end context 'when a file is passed as an argument' do - let(:output_file) { 'output_file.out' } - - it 'does not read the output file' do - expect(File).not_to receive(:read).with(output_file) - - subject - end + let(:changed_files_pathname) { 'does-not-exist.out' } - it 'calls GitLab API anyways' do + it 'calls GitLab API' do expect(gitlab_client).to receive(:merge_request_changes) .with('dummy-project', '1234') diff --git a/spec/tooling/lib/tooling/find_tests_spec.rb b/spec/tooling/lib/tooling/find_tests_spec.rb index e531fb9548e79..905f81c4bbdc3 100644 --- a/spec/tooling/lib/tooling/find_tests_spec.rb +++ b/spec/tooling/lib/tooling/find_tests_spec.rb @@ -7,27 +7,29 @@ RSpec.describe Tooling::FindTests, feature_category: :tooling do include StubENV - attr_accessor :changes_file, :matching_tests_paths + attr_accessor :changed_files_file, :predictive_tests_file - let(:instance) { described_class.new(changes_file, matching_tests_paths) } + 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(:changes_file_content) { "changed_file1 changed_file2" } - let(:matching_tests_paths_content) { "previously_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.changes_file = Tempfile.new('changes') - self.matching_tests_paths = Tempfile.new('matching_tests') + 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 - changes_file.close - matching_tests_paths.close - changes_file.unlink - matching_tests_paths.unlink + changed_files_file.close + predictive_tests_file.close + changed_files_file.unlink + predictive_tests_file.unlink end end @@ -42,44 +44,44 @@ ) # We write into the temp files initially, to later check how the code modified those files - File.write(changes_file, changes_file_content) - File.write(matching_tests_paths, matching_tests_paths_content) + 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 matching_tests_paths file does not exist' do - let(:instance) { described_class.new(non_existing_output_file, matching_tests_paths) } - let(:non_existing_output_file) { 'tmp/another_file.out' } + 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_file) + FileUtils.rm_rf(non_existing_output_pathname) end it 'creates the file' do - expect { subject }.to change { File.exist?(non_existing_output_file) }.from(false).to(true) + expect { subject }.to change { File.exist?(non_existing_output_pathname) }.from(false).to(true) end end - context 'when the matching_tests_paths file already exists' do + 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(matching_tests_paths, '') + 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(changes_file) } + 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(matching_tests_paths) } - .from(matching_tests_paths_content) - .to("#{matching_tests_paths_content} #{new_matching_tests.uniq.join(' ')}") + 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 @@ -148,8 +150,8 @@ it 'writes uniquely matching specs to the output' do subject - expect(File.read(matching_tests_paths).split(' ')).to match_array( - matching_tests_paths_content.split(' ') + new_matching_tests.uniq + expect(File.read(predictive_tests_pathname).split(' ')).to match_array( + predictive_tests_content.split(' ') + new_matching_tests.uniq ) end end diff --git a/spec/tooling/lib/tooling/helpers/file_handler_spec.rb b/spec/tooling/lib/tooling/helpers/file_handler_spec.rb index 7c8310c4bd958..d6f68baeb901b 100644 --- a/spec/tooling/lib/tooling/helpers/file_handler_spec.rb +++ b/spec/tooling/lib/tooling/helpers/file_handler_spec.rb @@ -42,18 +42,18 @@ class MockClass # rubocop:disable Gitlab/NamespacedClass subject { instance.read_array_from_file(input_file_path) } context 'when the input file does not exist' do - let(:non_existing_input_file) { 'tmp/another_file.out' } + let(:non_existing_input_pathname) { 'tmp/another_file.out' } - subject { instance.read_array_from_file(non_existing_input_file) } + subject { instance.read_array_from_file(non_existing_input_pathname) } around do |example| example.run ensure - FileUtils.rm_rf(non_existing_input_file) + FileUtils.rm_rf(non_existing_input_pathname) end it 'creates the file' do - expect { subject }.to change { File.exist?(non_existing_input_file) }.from(false).to(true) + expect { subject }.to change { File.exist?(non_existing_input_pathname) }.from(false).to(true) end end @@ -67,9 +67,10 @@ class MockClass # rubocop:disable Gitlab/NamespacedClass end describe '#write_array_to_file' do - let(:content_array) { %w[new_entry] } + let(:content_array) { %w[new_entry] } + let(:overwrite_flag) { false } - subject { instance.write_array_to_file(output_file_path, content_array) } + subject { instance.write_array_to_file(output_file_path, content_array, overwrite: overwrite_flag) } context 'when the output file does not exist' do let(:non_existing_output_file) { 'tmp/another_file.out' } @@ -93,6 +94,14 @@ class MockClass # rubocop:disable Gitlab/NamespacedClass 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 @@ -100,8 +109,18 @@ class MockClass # rubocop:disable Gitlab/NamespacedClass 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(' ')) + .from(initial_content) + .to((initial_content.split(' ') + content_array).join(' ')) + end + + context 'when the overwrite flag is set to true' do + let(:overwrite_flag) { true } + + 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 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 index 521958573fd8f..b6459428214db 100644 --- a/spec/tooling/lib/tooling/mappings/graphql_base_type_mappings_spec.rb +++ b/spec/tooling/lib/tooling/mappings/graphql_base_type_mappings_spec.rb @@ -6,11 +6,17 @@ 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 :changes_file, :matching_tests_paths + 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.changes_file = Tempfile.new('changes') - self.matching_tests_paths = Tempfile.new('matching_tests') + 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| @@ -24,20 +30,16 @@ begin example.run ensure - changes_file.close - matching_tests_paths.close - changes_file.unlink - matching_tests_paths.unlink + changed_files_file.close + predictive_tests_file.close + changed_files_file.unlink + predictive_tests_file.unlink end end end end end - let(:instance) { described_class.new(changes_file, matching_tests_paths) } - let(:changes_file_content) { "changed_file1 changed_file2" } - let(:matching_tests_paths_initial_content) { "previously_matching_spec.rb" } - before do stub_const("Tooling::Mappings::GraphqlBaseTypeMappings::GRAPHQL_TYPES_FOLDERS", { nil => [foss_folder], @@ -46,23 +48,23 @@ }) # We write into the temp files initially, to later check how the code modified those files - File.write(changes_file, changes_file_content) - File.write(matching_tests_paths, matching_tests_paths_initial_content) + 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(:changes_file_content) { '' } + let(:changed_files_content) { '' } it 'does not change the output file' do - expect { subject }.not_to change { File.read(matching_tests_paths) } + expect { subject }.not_to change { File.read(predictive_tests_pathname) } end end context 'when some GraphQL files were changed' do - let(:changes_file_content) do + let(:changed_files_content) do [ "#{foss_folder}/my_graphql_file.rb", "#{foss_folder}/my_other_graphql_file.rb" @@ -76,7 +78,7 @@ end it 'does not change the output file' do - expect { subject }.not_to change { File.read(matching_tests_paths) } + expect { subject }.not_to change { File.read(predictive_tests_pathname) } end end @@ -92,9 +94,9 @@ end it 'writes the correct specs in the output' do - expect { subject }.to change { File.read(matching_tests_paths) } - .from(matching_tests_paths_initial_content) - .to("#{matching_tests_paths_initial_content} spec/my_graphql_file_spec.rb") + 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 @@ -110,7 +112,7 @@ end context 'when no files were changed' do - let(:changes_file_content) { '' } + let(:changed_files_content) { '' } it 'returns an empty array' do expect(subject).to match_array([]) @@ -118,7 +120,7 @@ end context 'when GraphQL files were changed' do - let(:changes_file_content) do + let(:changed_files_content) do [ "#{foss_folder}/my_graphql_file.rb", "#{foss_folder}/my_other_graphql_file.rb", @@ -135,7 +137,7 @@ end context 'when files are deleted' do - let(:changes_file_content) { "#{foss_folder}/deleted.rb" } + let(:changed_files_content) { "#{foss_folder}/deleted.rb" } it 'returns an empty array' do expect(subject).to match_array([]) 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 c36d14cd267d8..e1f35bedebbe2 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,23 +6,25 @@ 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 :changes_file, :matching_tests_paths + 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( - changes_file, - matching_tests_paths, + changed_files_pathname, + predictive_tests_pathname, system_specs_base_folder: system_specs_base_folder, js_base_folder: js_base_folder ) end - let(:changes_file_content) { "changed_file1 changed_file2" } - let(:matching_tests_paths_content) { "previously_matching_spec.rb" } - around do |example| - self.changes_file = Tempfile.new('changes') - self.matching_tests_paths = Tempfile.new('matching_tests') + 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| @@ -34,10 +36,10 @@ begin example.run ensure - changes_file.close - matching_tests_paths.close - changes_file.unlink - matching_tests_paths.unlink + changed_files_file.close + predictive_tests_file.close + changed_files_file.unlink + predictive_tests_file.unlink end end end @@ -45,22 +47,22 @@ before do # We write into the temp files initially, to later check how the code modified those files - File.write(changes_file, changes_file_content) - File.write(matching_tests_paths, matching_tests_paths_content) + File.write(changed_files_pathname, changed_files_content) + File.write(predictive_tests_pathname, predictive_tests_content) end describe '#execute' do subject { instance.execute } before do - File.write(changes_file, changed_files.join(' ')) + File.write(changed_files_pathname, changed_files.join(' ')) end context 'when no JS files were changed' do let(:changed_files) { [] } it 'does not change the output file' do - expect { subject }.not_to change { File.read(matching_tests_paths) } + expect { subject }.not_to change { File.read(predictive_tests_pathname) } end end @@ -69,7 +71,7 @@ context 'when the JS files are not present on disk' do it 'does not change the output file' do - expect { subject }.not_to change { File.read(matching_tests_paths) } + expect { subject }.not_to change { File.read(predictive_tests_pathname) } end end @@ -81,7 +83,7 @@ context 'when no system specs match the JS keyword' do it 'does not change the output file' do - expect { subject }.not_to change { File.read(matching_tests_paths) } + expect { subject }.not_to change { File.read(predictive_tests_pathname) } end end @@ -92,9 +94,9 @@ end it 'adds the new specs to the output file' do - expect { subject }.to change { File.read(matching_tests_paths) } - .from(matching_tests_paths_content) - .to("#{matching_tests_paths_content} #{system_specs_base_folder}/confidential_issues/issues_spec.rb") + 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 @@ -108,7 +110,7 @@ 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(changes_file, changed_files.join(' ')) + File.write(changed_files_pathname, changed_files.join(' ')) end context 'when no files were changed' do @@ -148,7 +150,7 @@ end describe '#construct_js_keywords' do - subject { described_class.new(changes_file, matching_tests_paths).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[ 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 index 30965ed985da6..75ddee189857f 100644 --- a/spec/tooling/lib/tooling/mappings/partial_to_views_mappings_spec.rb +++ b/spec/tooling/lib/tooling/mappings/partial_to_views_mappings_spec.rb @@ -5,15 +5,20 @@ require_relative '../../../../../tooling/lib/tooling/mappings/partial_to_views_mappings' RSpec.describe Tooling::Mappings::PartialToViewsMappings, feature_category: :tooling do - attr_accessor :view_base_folder, :changes_file, :output_file + attr_accessor :view_base_folder, :changed_files_file, :views_with_partials_file - let(:instance) { described_class.new(changes_file, output_file, view_base_folder: view_base_folder) } - let(:changes_file_content) { "changed_file1 changed_file2" } - let(:output_file_content) { "previously_added_view.html.haml" } + 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.changes_file = Tempfile.new('changes') - self.output_file = Tempfile.new('output_file') + 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 @@ -23,24 +28,24 @@ example.run end ensure - changes_file.close - output_file.close - changes_file.unlink - output_file.unlink + 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(changes_file, changes_file_content) - File.write(output_file, output_file_content) + 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(:changes_file_content) { changed_files.join(" ") } + 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 @@ -48,12 +53,12 @@ end it 'does not modify the content of the input file' do - expect { subject }.not_to change { File.read(changes_file) } + 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(output_file) } + expect { subject }.not_to change { File.read(views_with_partials_pathname) } end end @@ -77,7 +82,7 @@ end it 'does not change the output file' do - expect { subject }.not_to change { File.read(output_file) } + expect { subject }.not_to change { File.read(views_with_partials_pathname) } end end @@ -87,9 +92,9 @@ end it 'writes the view including the partial to the output' do - expect { subject }.to change { File.read(output_file) } - .from(output_file_content) - .to(output_file_content + " #{view_base_folder}/my_view.html.haml") + 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 @@ -98,7 +103,7 @@ describe '#filter_files' do subject { instance.filter_files } - let(:changes_file_content) { file_path } + 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" } @@ -164,11 +169,11 @@ before do FileUtils.mkdir_p("#{view_base_folder}/components/subfolder") - File.write(changes_file_content, "I am a partial!") + File.write(changed_files_content, "I am a partial!") end context 'when the partial is not part of the changed files' do - let(:changes_file_content) { "#{view_base_folder}/components/subfolder/_not_the_partial.html.haml" } + let(:changed_files_content) { "#{view_base_folder}/components/subfolder/_not_the_partial.html.haml" } it 'returns false' do expect(subject).to be_falsey @@ -176,7 +181,7 @@ end context 'when the partial is part of the changed files' do - let(:changes_file_content) { "#{view_base_folder}/components/subfolder/_relative_partial.html.haml" } + let(:changed_files_content) { "#{view_base_folder}/components/subfolder/_relative_partial.html.haml" } it 'returns true' do expect(subject).to be_truthy @@ -191,11 +196,11 @@ before do FileUtils.mkdir_p("#{view_base_folder}/components") FileUtils.mkdir_p("#{view_base_folder}/shared") - File.write(changes_file_content, "I am a partial!") + File.write(changed_files_content, "I am a partial!") end context 'when the partial is not part of the changed files' do - let(:changes_file_content) { "#{view_base_folder}/shared/not_the_partial" } + let(:changed_files_content) { "#{view_base_folder}/shared/not_the_partial" } it 'returns false' do expect(subject).to be_falsey @@ -203,7 +208,7 @@ end context 'when the partial is part of the changed files' do - let(:changes_file_content) { "#{view_base_folder}/shared/_absolute_partial.html.haml" } + let(:changed_files_content) { "#{view_base_folder}/shared/_absolute_partial.html.haml" } it 'returns true' do expect(subject).to be_truthy 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 bf07ad1951bfb..6d007843716ca 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,23 +6,25 @@ 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 :changes_file, :matching_tests_paths + 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( - changes_file, - matching_tests_paths, + changed_files_pathname, + predictive_tests_pathname, view_base_folder: view_base_folder, js_base_folder: js_base_folder ) end - let(:changes_file_content) { "changed_file1 changed_file2" } - let(:matching_tests_paths_content) { "previously_matching_spec.rb" } - around do |example| - self.changes_file = Tempfile.new('changes') - self.matching_tests_paths = Tempfile.new('matching_tests') + 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| @@ -34,10 +36,10 @@ begin example.run ensure - changes_file.close - matching_tests_paths.close - changes_file.unlink - matching_tests_paths.unlink + changed_files_file.close + predictive_tests_file.close + changed_files_file.unlink + predictive_tests_file.unlink end end end @@ -45,8 +47,8 @@ before do # We write into the temp files initially, to later check how the code modified those files - File.write(changes_file, changes_file_content) - File.write(matching_tests_paths, matching_tests_paths_content) + File.write(changed_files_pathname, changed_files_content) + File.write(predictive_tests_pathname, predictive_tests_content) end describe '#execute' do @@ -55,7 +57,7 @@ subject { instance.execute } before do - File.write(changes_file, changed_files.join(' ')) + File.write(changed_files_pathname, changed_files.join(' ')) end context 'when no view files have been changed' do @@ -64,7 +66,7 @@ end it 'does not change the output file' do - expect { subject }.not_to change { File.read(matching_tests_paths) } + expect { subject }.not_to change { File.read(predictive_tests_pathname) } end end @@ -82,7 +84,7 @@ end it 'does not change the output file' do - expect { subject }.not_to change { File.read(matching_tests_paths) } + expect { subject }.not_to change { File.read(predictive_tests_pathname) } end end @@ -99,7 +101,7 @@ context 'when no matching JS files are found' do it 'does not change the output file' do - expect { subject }.not_to change { File.read(matching_tests_paths) } + expect { subject }.not_to change { File.read(predictive_tests_pathname) } end end @@ -119,9 +121,9 @@ end it 'adds the matching JS files to the output' do - expect { subject }.to change { File.read(matching_tests_paths) } - .from(matching_tests_paths_content) - .to("#{matching_tests_paths_content} #{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 @@ -165,9 +167,9 @@ end it 'scans those partials for the HTML attribute value' do - expect { subject }.to change { File.read(matching_tests_paths) } - .from(matching_tests_paths_content) - .to("#{matching_tests_paths_content} #{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 @@ -178,7 +180,7 @@ before do File.write("#{js_base_folder}/index.js", "index.js") File.write("#{view_base_folder}/index.html", "index.html") - File.write(changes_file, changed_files.join(' ')) + File.write(changed_files_pathname, changed_files.join(' ')) end context 'when no files were changed' 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 index d4f4bbd7f06fb..b8a13c50c9b0f 100644 --- 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 @@ -5,15 +5,20 @@ 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, :changes_file, :output_file + attr_accessor :view_base_folder, :changed_files_file, :predictive_tests_file - let(:instance) { described_class.new(changes_file, output_file, view_base_folder: view_base_folder) } - let(:changes_file_content) { "changed_file1 changed_file2" } - let(:output_file_initial_content) { "previously_added_spec.rb" } + 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.changes_file = Tempfile.new('changes') - self.output_file = Tempfile.new('output_file') + 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 @@ -23,10 +28,10 @@ example.run end ensure - changes_file.close - output_file.close - changes_file.unlink - output_file.unlink + changed_files_file.close + predictive_tests_file.close + changed_files_file.unlink + predictive_tests_file.unlink end end @@ -34,13 +39,13 @@ 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(changes_file, changes_file_content) - File.write(output_file, output_file_initial_content) + 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(changes_file) } + expect { subject }.not_to change { File.read(changed_files_pathname) } end end @@ -48,7 +53,7 @@ subject { instance.execute } let(:changed_files) { ["#{view_base_folder}/app/views/dashboard/my_view.html.haml"] } - let(:changes_file_content) { changed_files.join(" ") } + 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 @@ -88,9 +93,9 @@ end it 'writes that feature spec to the output file' do - expect { subject }.to change { File.read(output_file) } - .from(output_file_initial_content) - .to("#{output_file_initial_content} #{expected_feature_spec}") + expect { subject }.to change { File.read(predictive_tests_pathname) } + .from(predictive_tests_initial_content) + .to("#{predictive_tests_initial_content} #{expected_feature_spec}") end end @@ -111,9 +116,9 @@ end it 'writes all of the feature specs for the parent folder to the output file' do - expect { subject }.to change { File.read(output_file) } - .from(output_file_initial_content) - .to("#{output_file_initial_content} #{expected_feature_specs.join(' ')}") + 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 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 0000000000000..79554037c4862 --- /dev/null +++ b/spec/tooling/lib/tooling/predictive_tests_spec.rb @@ -0,0 +1,109 @@ +# frozen_string_literal: true + +require 'tempfile' +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' } + + attr_accessor :changed_files, :fixtures_mapping, :matching_js_files, :matching_tests, :views_with_partials + + around do |example| + self.changed_files = Tempfile.new('test-folder/changed_files.txt') + 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 + example.run + ensure + changed_files.close + changed_files.unlink + 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, '{}') # We write valid JSON, so that the file can be processed + 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 + File.write(changed_files, changed_files_content) + end + + context 'when no files were changed' do + let(:changed_files_content) { '' } + + it 'does not change any files' do + expect { subject }.not_to change { File.read(changed_files.path) } + 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 were changed' do + let(:changed_files_content) { 'tooling/lib/tooling/predictive_tests.rb' } + + 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} spec/tooling/lib/tooling/predictive_tests_spec.rb") + end + + it 'does not change files other than RSPEC_MATCHING_TESTS_PATH' do + expect { subject }.not_to change { File.read(changed_files.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 + end + end +end diff --git a/tooling/bin/find_changes b/tooling/bin/find_changes deleted file mode 100755 index 331fae45a5308..0000000000000 --- a/tooling/bin/find_changes +++ /dev/null @@ -1,14 +0,0 @@ -#!/usr/bin/env ruby -# frozen_string_literal: true - -require_relative '../lib/tooling/find_changes' - -output_file = ARGV.shift -matched_tests_file = ARGV.shift -frontend_fixtures_mapping_path = ARGV.shift - -Tooling::FindChanges - .new( - output_file: output_file, - matched_tests_file: matched_tests_file, - frontend_fixtures_mapping_path: frontend_fixtures_mapping_path).execute diff --git a/tooling/bin/find_tests b/tooling/bin/find_tests deleted file mode 100755 index 31afa4d598bde..0000000000000 --- a/tooling/bin/find_tests +++ /dev/null @@ -1,9 +0,0 @@ -#!/usr/bin/env ruby -# frozen_string_literal: true - -require_relative '../lib/tooling/find_tests' - -changes_file = ARGV.shift -matching_tests_paths = ARGV.shift - -Tooling::FindTests.new(changes_file, matching_tests_paths).execute diff --git a/tooling/bin/graphql_base_type_mappings b/tooling/bin/graphql_base_type_mappings deleted file mode 100755 index 2fc06ab639fbb..0000000000000 --- a/tooling/bin/graphql_base_type_mappings +++ /dev/null @@ -1,9 +0,0 @@ -#!/usr/bin/env ruby -# frozen_string_literal: true - -require_relative '../lib/tooling/mappings/graphql_base_type_mappings' - -changes_file = ARGV.shift -matching_tests_paths = ARGV.shift - -Tooling::Mappings::GraphqlBaseTypeMappings.new(changes_file, matching_tests_paths).execute diff --git a/tooling/bin/js_to_system_specs_mappings b/tooling/bin/js_to_system_specs_mappings deleted file mode 100755 index 22d1dbb65907a..0000000000000 --- a/tooling/bin/js_to_system_specs_mappings +++ /dev/null @@ -1,9 +0,0 @@ -#!/usr/bin/env ruby -# frozen_string_literal: true - -require_relative '../lib/tooling/mappings/js_to_system_specs_mappings' - -changes_file = ARGV.shift -matching_tests_paths = ARGV.shift - -Tooling::Mappings::JsToSystemSpecsMappings.new(changes_file, matching_tests_paths).execute diff --git a/tooling/bin/partial_to_views_mappings b/tooling/bin/partial_to_views_mappings deleted file mode 100755 index 12c994ee556c0..0000000000000 --- a/tooling/bin/partial_to_views_mappings +++ /dev/null @@ -1,9 +0,0 @@ -#!/usr/bin/env ruby -# frozen_string_literal: true - -require_relative '../lib/tooling/mappings/partial_to_views_mappings' - -changes_file = ARGV.shift -output_file = ARGV.shift - -Tooling::Mappings::PartialToViewsMappings.new(changes_file, output_file).execute diff --git a/tooling/bin/predictive_tests b/tooling/bin/predictive_tests new file mode 100755 index 0000000000000..61543494a69ca --- /dev/null +++ b/tooling/bin/predictive_tests @@ -0,0 +1,6 @@ +#!/usr/bin/env ruby +# frozen_string_literal: true + +require_relative '../lib/tooling/predictive_tests' + +Tooling::PredictiveTests.new.execute diff --git a/tooling/bin/view_to_js_mappings b/tooling/bin/view_to_js_mappings deleted file mode 100755 index 51d7cc11ed8b9..0000000000000 --- a/tooling/bin/view_to_js_mappings +++ /dev/null @@ -1,9 +0,0 @@ -#!/usr/bin/env ruby -# frozen_string_literal: true - -require_relative '../lib/tooling/mappings/view_to_js_mappings' - -changes_file = ARGV.shift -matching_tests_paths = ARGV.shift - -Tooling::Mappings::ViewToJsMappings.new(changes_file, matching_tests_paths).execute diff --git a/tooling/bin/view_to_system_specs_mappings b/tooling/bin/view_to_system_specs_mappings deleted file mode 100755 index b2e3f2a132ad8..0000000000000 --- a/tooling/bin/view_to_system_specs_mappings +++ /dev/null @@ -1,9 +0,0 @@ -#!/usr/bin/env ruby -# frozen_string_literal: true - -require_relative '../lib/tooling/mappings/view_to_system_specs_mappings' - -changes_file = ARGV.shift -output_file = ARGV.shift - -Tooling::Mappings::ViewToSystemSpecsMappings.new(changes_file, output_file).execute diff --git a/tooling/lib/tooling/find_changes.rb b/tooling/lib/tooling/find_changes.rb index e151734e41c5f..b4439bd4abe05 100755 --- a/tooling/lib/tooling/find_changes.rb +++ b/tooling/lib/tooling/find_changes.rb @@ -2,28 +2,35 @@ # frozen_string_literal: true require 'gitlab' +require_relative 'helpers/file_handler' module Tooling class FindChanges - def initialize(output_file: nil, matched_tests_file: nil, frontend_fixtures_mapping_path: nil) - @gitlab_token = ENV['PROJECT_TOKEN_FOR_CI_SCRIPTS_API_USAGE'] || '' - @gitlab_endpoint = ENV['CI_API_V4_URL'] - @mr_project_path = ENV['CI_MERGE_REQUEST_PROJECT_PATH'] - @mr_iid = ENV['CI_MERGE_REQUEST_IID'] - @output_file = output_file - @matched_tests_file = matched_tests_file - @frontend_fixtures_mapping_path = frontend_fixtures_mapping_path + include Helpers::FileHandler + + def initialize( + changed_files_pathname = nil, predictive_tests_pathname = nil, frontend_fixtures_mapping_pathname = nil + ) + @gitlab_token = ENV['PROJECT_TOKEN_FOR_CI_SCRIPTS_API_USAGE'] || '' + @gitlab_endpoint = ENV['CI_API_V4_URL'] + @mr_project_path = ENV['CI_MERGE_REQUEST_PROJECT_PATH'] + @mr_iid = ENV['CI_MERGE_REQUEST_IID'] + @changed_files_pathname = changed_files_pathname + @predictive_tests_pathname = predictive_tests_pathname + @frontend_fixtures_mapping_pathname = frontend_fixtures_mapping_pathname end def execute - raise ArgumentError, "An path to an output file must be given as first argument." if output_file.nil? + if changed_files_pathname.nil? + raise ArgumentError, "A path to the changed files file must be given as first argument." + end add_frontend_fixture_files! - File.write(output_file, file_changes.join(' ')) + write_array_to_file(changed_files_pathname, file_changes, overwrite: true) end def only_js_files_changed - @output_file = nil # We ensure that we'll get the diff from the MR directly, not from a file. + @changed_files_pathname = nil # We ensure that we'll get the diff from the MR directly, not from a file. file_changes.any? && file_changes.all? { |file| file.end_with?('.js') } end @@ -31,7 +38,7 @@ def only_js_files_changed private attr_reader :gitlab_token, :gitlab_endpoint, :mr_project_path, - :mr_iid, :output_file, :matched_tests_file, :frontend_fixtures_mapping_path + :mr_iid, :changed_files_pathname, :predictive_tests_pathname, :frontend_fixtures_mapping_pathname def gitlab @gitlab ||= begin @@ -45,7 +52,7 @@ def gitlab end def add_frontend_fixture_files? - matched_tests_file && frontend_fixtures_mapping_path + predictive_tests_pathname && frontend_fixtures_mapping_pathname end def add_frontend_fixture_files! @@ -61,8 +68,8 @@ def add_frontend_fixture_files! def file_changes @file_changes ||= - if output_file && File.exist?(output_file) - File.read(output_file).split(' ') + if changed_files_pathname && File.exist?(changed_files_pathname) + read_array_from_file(changed_files_pathname) else mr_changes.changes.flat_map do |change| change.to_h.values_at('old_path', 'new_path') @@ -75,15 +82,15 @@ def mr_changes end def test_files - return [] if !matched_tests_file || !File.exist?(matched_tests_file) + return [] if !predictive_tests_pathname || !File.exist?(predictive_tests_pathname) - File.read(matched_tests_file).split(' ') + read_array_from_file(predictive_tests_pathname) end def frontend_fixtures_mapping - return {} if !frontend_fixtures_mapping_path || !File.exist?(frontend_fixtures_mapping_path) + return {} if !frontend_fixtures_mapping_pathname || !File.exist?(frontend_fixtures_mapping_pathname) - JSON.parse(File.read(frontend_fixtures_mapping_path)) # rubocop:disable Gitlab/Json + JSON.parse(File.read(frontend_fixtures_mapping_pathname)) # rubocop:disable Gitlab/Json end end end diff --git a/tooling/lib/tooling/find_tests.rb b/tooling/lib/tooling/find_tests.rb index b63b207c58bb0..f26c1eacdc7e3 100644 --- a/tooling/lib/tooling/find_tests.rb +++ b/tooling/lib/tooling/find_tests.rb @@ -7,9 +7,9 @@ module Tooling class FindTests include Helpers::FileHandler - def initialize(changes_file, matching_tests_paths) - @matching_tests_paths = matching_tests_paths - @changed_files = read_array_from_file(changes_file) + def initialize(changed_files_pathname, predictive_tests_pathname) + @predictive_tests_pathname = predictive_tests_pathname + @changed_files = read_array_from_file(changed_files_pathname) end def execute @@ -21,11 +21,11 @@ def execute end end - write_array_to_file(matching_tests_paths, tff.test_files.uniq) + write_array_to_file(predictive_tests_pathname, tff.test_files.uniq) end private - attr_reader :changed_files, :matching_tests, :matching_tests_paths + attr_reader :changed_files, :matching_tests, :predictive_tests_pathname end end diff --git a/tooling/lib/tooling/helpers/file_handler.rb b/tooling/lib/tooling/helpers/file_handler.rb index ec4f42ea3637a..2778bb1ffbcd1 100644 --- a/tooling/lib/tooling/helpers/file_handler.rb +++ b/tooling/lib/tooling/helpers/file_handler.rb @@ -11,10 +11,18 @@ def read_array_from_file(file) File.read(file).split(' ') end - def write_array_to_file(file, content_array) + def write_array_to_file(file, content_array, overwrite: false) FileUtils.touch file - output_content = (File.read(file).split(' ') + content_array).join(' ') + # We sort the array to make it easier to read the output file + content_array.sort! + + output_content = + if overwrite + content_array.join(' ') + else + (File.read(file).split(' ') + content_array).join(' ') + end File.write(file, output_content) end diff --git a/tooling/lib/tooling/mappings/graphql_base_type_mappings.rb b/tooling/lib/tooling/mappings/graphql_base_type_mappings.rb index cd8b55d5820a2..569a82781634b 100644 --- a/tooling/lib/tooling/mappings/graphql_base_type_mappings.rb +++ b/tooling/lib/tooling/mappings/graphql_base_type_mappings.rb @@ -25,9 +25,9 @@ class GraphqlBaseTypeMappings < Base 'jh' => GRAPHQL_TYPES_FOLDERS_JH }.freeze - def initialize(changes_file, matching_tests_paths) - @matching_tests_paths = matching_tests_paths - @changed_files = read_array_from_file(changes_file) + def initialize(changed_files_pathname, predictive_tests_pathname) + @predictive_tests_pathname = predictive_tests_pathname + @changed_files = read_array_from_file(changed_files_pathname) end def execute @@ -46,7 +46,7 @@ def execute end end.compact.uniq - write_array_to_file(matching_tests_paths, matching_graphql_tests) + write_array_to_file(predictive_tests_pathname, matching_graphql_tests) end def filter_files @@ -113,7 +113,7 @@ def filename_to_spec_filename(filename) private - attr_reader :changed_files, :matching_tests_paths + attr_reader :changed_files, :predictive_tests_pathname end end end diff --git a/tooling/lib/tooling/mappings/js_to_system_specs_mappings.rb b/tooling/lib/tooling/mappings/js_to_system_specs_mappings.rb index 0c61b921f839b..b2fca3a765a1c 100644 --- a/tooling/lib/tooling/mappings/js_to_system_specs_mappings.rb +++ b/tooling/lib/tooling/mappings/js_to_system_specs_mappings.rb @@ -10,13 +10,13 @@ module Tooling module Mappings class JsToSystemSpecsMappings < Base def initialize( - changes_file, matching_tests_paths, + changed_files_pathname, predictive_tests_pathname, js_base_folder: 'app/assets/javascripts', system_specs_base_folder: 'spec/features') - @changed_files = read_array_from_file(changes_file) - @matching_tests_paths = matching_tests_paths - @js_base_folder = js_base_folder - @js_base_folders = folders_for_available_editions(js_base_folder) - @system_specs_base_folder = system_specs_base_folder + @changed_files = read_array_from_file(changed_files_pathname) + @predictive_tests_pathname = predictive_tests_pathname + @js_base_folder = js_base_folder + @js_base_folders = folders_for_available_editions(js_base_folder) + @system_specs_base_folder = system_specs_base_folder # Cannot be extracted to a constant, as it depends on a variable @first_js_folder_extract_regexp = %r{ @@ -36,7 +36,7 @@ def execute end end - write_array_to_file(matching_tests_paths, matching_system_tests) + write_array_to_file(predictive_tests_pathname, matching_system_tests) end # Keep the files that are in the @js_base_folders folders @@ -66,7 +66,7 @@ def system_specs_for_edition(edition) private - attr_reader :changed_files, :matching_tests_paths + attr_reader :changed_files, :predictive_tests_pathname end end end diff --git a/tooling/lib/tooling/mappings/partial_to_views_mappings.rb b/tooling/lib/tooling/mappings/partial_to_views_mappings.rb index 3109da685f1f0..8b0a5ed4ecd48 100644 --- a/tooling/lib/tooling/mappings/partial_to_views_mappings.rb +++ b/tooling/lib/tooling/mappings/partial_to_views_mappings.rb @@ -7,10 +7,10 @@ module Tooling module Mappings class PartialToViewsMappings < Base - def initialize(changes_file, output_file, view_base_folder: 'app/views') - @output_file = output_file - @changed_files = read_array_from_file(changes_file) - @view_base_folders = folders_for_available_editions(view_base_folder) + def initialize(changed_files_pathname, views_with_partials_pathname, view_base_folder: 'app/views') + @views_with_partials_pathname = views_with_partials_pathname + @changed_files = read_array_from_file(changed_files_pathname) + @view_base_folders = folders_for_available_editions(view_base_folder) end def execute @@ -28,7 +28,7 @@ def execute end end - write_array_to_file(output_file, views_including_modified_partials) + write_array_to_file(views_with_partials_pathname, views_including_modified_partials) end def filter_files @@ -99,7 +99,7 @@ def find_pattern_in_file(file, pattern) private - attr_reader :changed_files, :output_file, :view_base_folders + attr_reader :changed_files, :views_with_partials_pathname, :view_base_folders end end end diff --git a/tooling/lib/tooling/mappings/view_to_js_mappings.rb b/tooling/lib/tooling/mappings/view_to_js_mappings.rb index fd2db6b6d7ef4..f2098d6acd570 100644 --- a/tooling/lib/tooling/mappings/view_to_js_mappings.rb +++ b/tooling/lib/tooling/mappings/view_to_js_mappings.rb @@ -14,12 +14,12 @@ class ViewToJsMappings < Base RAILS_PARTIAL_INVOCATION_REGEXP = %r{(?:render|render_if_exists)(?: |\()(?:partial: ?)?['"]([\w/-]+)['"]}.freeze def initialize( - changes_file, matching_tests_paths, + changed_files_pathname, predictive_tests_pathname, view_base_folder: 'app/views', js_base_folder: 'app/assets/javascripts') - @changed_files = read_array_from_file(changes_file) - @matching_tests_paths = matching_tests_paths - @view_base_folders = folders_for_available_editions(view_base_folder) - @js_base_folders = folders_for_available_editions(js_base_folder) + @changed_files = read_array_from_file(changed_files_pathname) + @predictive_tests_pathname = predictive_tests_pathname + @view_base_folders = folders_for_available_editions(view_base_folder) + @js_base_folders = folders_for_available_editions(js_base_folder) end def execute @@ -40,7 +40,7 @@ def execute end end - write_array_to_file(matching_tests_paths, matching_js_files) + write_array_to_file(predictive_tests_pathname, matching_js_files) end # Keep the files that are in the @view_base_folders folder @@ -76,7 +76,7 @@ def find_pattern_in_file(file, pattern) private - attr_reader :changed_files, :matching_tests_paths + attr_reader :changed_files, :predictive_tests_pathname end end end diff --git a/tooling/lib/tooling/mappings/view_to_system_specs_mappings.rb b/tooling/lib/tooling/mappings/view_to_system_specs_mappings.rb index 227f2c6e62b24..6d840dcbd7166 100644 --- a/tooling/lib/tooling/mappings/view_to_system_specs_mappings.rb +++ b/tooling/lib/tooling/mappings/view_to_system_specs_mappings.rb @@ -7,10 +7,10 @@ module Tooling module Mappings class ViewToSystemSpecsMappings < Base - def initialize(changes_file, output_file, view_base_folder: 'app/views') - @output_file = output_file - @changed_files = read_array_from_file(changes_file) - @view_base_folders = folders_for_available_editions(view_base_folder) + def initialize(changed_files_pathname, predictive_tests_pathname, view_base_folder: 'app/views') + @predictive_tests_pathname = predictive_tests_pathname + @changed_files = read_array_from_file(changed_files_pathname) + @view_base_folders = folders_for_available_editions(view_base_folder) end def execute @@ -27,12 +27,12 @@ def execute end end - write_array_to_file(output_file, found_system_specs.compact.uniq.sort) + write_array_to_file(predictive_tests_pathname, found_system_specs.compact.uniq.sort) end private - attr_reader :changed_files, :output_file, :view_base_folders + attr_reader :changed_files, :predictive_tests_pathname, :view_base_folders # Keep the views files that are in the @view_base_folders folder def filter_files diff --git a/tooling/lib/tooling/predictive_tests.rb b/tooling/lib/tooling/predictive_tests.rb new file mode 100644 index 0000000000000..2691e1ba56d60 --- /dev/null +++ b/tooling/lib/tooling/predictive_tests.rb @@ -0,0 +1,53 @@ +# frozen_string_literal: true + +require_relative 'find_changes' +require_relative 'find_tests' +require_relative 'mappings/graphql_base_type_mappings' +require_relative 'mappings/js_to_system_specs_mappings' +require_relative 'mappings/partial_to_views_mappings' +require_relative 'mappings/view_to_js_mappings' +require_relative 'mappings/view_to_system_specs_mappings' + +module Tooling + class PredictiveTests + REQUIRED_ENV_VARIABLES = %w[ + RSPEC_CHANGED_FILES_PATH + RSPEC_MATCHING_TESTS_PATH + RSPEC_VIEWS_INCLUDING_PARTIALS_PATH + FRONTEND_FIXTURES_MAPPING_PATH + RSPEC_MATCHING_JS_FILES_PATH + ].freeze + + def initialize + missing_env_variables = REQUIRED_ENV_VARIABLES.select { |key| ENV[key.to_s] == '' } + unless missing_env_variables.empty? + raise "[predictive tests] Missing ENV variable(s): #{missing_env_variables.join(',')}." + end + + @rspec_changed_files_path = ENV['RSPEC_CHANGED_FILES_PATH'] + @rspec_matching_tests_path = ENV['RSPEC_MATCHING_TESTS_PATH'] + @rspec_views_including_partials_path = ENV['RSPEC_VIEWS_INCLUDING_PARTIALS_PATH'] + @frontend_fixtures_mapping_path = ENV['FRONTEND_FIXTURES_MAPPING_PATH'] + @rspec_matching_js_files_path = ENV['RSPEC_MATCHING_JS_FILES_PATH'] + end + + def execute + Tooling::FindChanges.new(rspec_changed_files_path).execute + Tooling::FindTests.new(rspec_changed_files_path, rspec_matching_tests_path).execute + Tooling::Mappings::PartialToViewsMappings.new( + rspec_changed_files_path, rspec_views_including_partials_path).execute + Tooling::FindTests.new(rspec_views_including_partials_path, rspec_matching_tests_path).execute + Tooling::Mappings::JsToSystemSpecsMappings.new(rspec_changed_files_path, rspec_matching_tests_path).execute + Tooling::Mappings::GraphqlBaseTypeMappings.new(rspec_changed_files_path, rspec_matching_tests_path).execute + Tooling::Mappings::ViewToSystemSpecsMappings.new(rspec_changed_files_path, rspec_matching_tests_path).execute + Tooling::FindChanges.new( + rspec_changed_files_path, rspec_matching_tests_path, frontend_fixtures_mapping_path).execute + Tooling::Mappings::ViewToJsMappings.new(rspec_changed_files_path, rspec_matching_js_files_path).execute + end + + private + + attr_reader :rspec_changed_files_path, :rspec_matching_tests_path, :rspec_views_including_partials_path, + :frontend_fixtures_mapping_path, :rspec_matching_js_files_path + end +end -- GitLab