diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index c3087716b133970a54af839edca75eb3e823bcd7..b2521c7249914fb14f130f71fc4dddb3bf0986a7 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -165,6 +165,7 @@ variables: RSPEC_FOSS_IMPACT_PIPELINE_TEMPLATE_YML: .gitlab/ci/rails/rspec-foss-impact.gitlab-ci.yml.erb RSPEC_LAST_RUN_RESULTS_FILE: rspec/rspec_last_run_results.txt RSPEC_MATCHING_JS_FILES_PATH: rspec/js_matching_files.txt + RSPEC_VIEWS_INCLUDING_PARTIALS_PATH: rspec/views_including_partials.txt RSPEC_MATCHING_TESTS_PATH: rspec/matching_tests.txt RSPEC_MATCHING_TESTS_FOSS_PATH: rspec/matching_tests-foss.txt RSPEC_MATCHING_TESTS_EE_PATH: rspec/matching_tests-ee.txt diff --git a/.gitlab/ci/setup.gitlab-ci.yml b/.gitlab/ci/setup.gitlab-ci.yml index 76c7af2753ecc3eb74248b1a37dbebbbe1381938..9620f0b87bd8f4f57c6ca7d6eee829925a126861 100644 --- a/.gitlab/ci/setup.gitlab-ci.yml +++ b/.gitlab/ci/setup.gitlab-ci.yml @@ -120,6 +120,8 @@ detect-tests: 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/find_changes ${RSPEC_CHANGED_FILES_PATH} ${RSPEC_MATCHING_TESTS_PATH} ${FRONTEND_FIXTURES_MAPPING_PATH}; filter_rspec_matched_foss_tests ${RSPEC_MATCHING_TESTS_PATH} ${RSPEC_MATCHING_TESTS_FOSS_PATH}; @@ -136,9 +138,10 @@ detect-tests: - ${FRONTEND_FIXTURES_MAPPING_PATH} - ${RSPEC_CHANGED_FILES_PATH} - ${RSPEC_MATCHING_JS_FILES_PATH} - - ${RSPEC_MATCHING_TESTS_PATH} - - ${RSPEC_MATCHING_TESTS_FOSS_PATH} - ${RSPEC_MATCHING_TESTS_EE_PATH} + - ${RSPEC_MATCHING_TESTS_FOSS_PATH} + - ${RSPEC_MATCHING_TESTS_PATH} + - ${RSPEC_VIEWS_INCLUDING_PARTIALS_PATH} detect-previous-failed-tests: extends: 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 0000000000000000000000000000000000000000..69dddb0ae3d6db6dbc03202dc061408e76c3590b --- /dev/null +++ b/spec/tooling/lib/tooling/mappings/partial_to_views_mappings_spec.rb @@ -0,0 +1,274 @@ +# 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, :changes_file, :output_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" } + + around do |example| + self.changes_file = Tempfile.new('changes') + self.output_file = Tempfile.new('output_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 + changes_file.close + output_file.close + changes_file.unlink + output_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) + end + + describe '#execute' do + subject { instance.execute } + + let(:changed_files) { ["#{view_base_folder}/my_view.html.haml"] } + let(:changes_file_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(changes_file) } + end + + context 'when no partials were modified' do + it 'empties the output file' do + expect { subject }.to change { File.read(output_file) }.from(output_file_content).to('') + 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" + ] + 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 'empties the output file' do + expect { subject }.to change { File.read(output_file) }.from(output_file_content).to('') + 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(output_file) } + .from(output_file_content) + .to("#{view_base_folder}/my_view.html.haml") + end + end + end + end + + describe '#filter_files' do + subject { instance.filter_files } + + let(:changes_file_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(changes_file_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" } + + it 'returns false' do + expect(subject).to be_falsey + end + 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" } + + 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(changes_file_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" } + + it 'returns false' do + expect(subject).to be_falsey + end + end + + context 'when the partial is part of the changed files' do + let(:changes_file_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/tooling/bin/partial_to_views_mappings b/tooling/bin/partial_to_views_mappings new file mode 100755 index 0000000000000000000000000000000000000000..12c994ee556c085234965faf56ccc7ceec8cca8a --- /dev/null +++ b/tooling/bin/partial_to_views_mappings @@ -0,0 +1,9 @@ +#!/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/lib/tooling/mappings/partial_to_views_mappings.rb b/tooling/lib/tooling/mappings/partial_to_views_mappings.rb new file mode 100644 index 0000000000000000000000000000000000000000..1b36894d88105d3d98b59432fde63e108162261d --- /dev/null +++ b/tooling/lib/tooling/mappings/partial_to_views_mappings.rb @@ -0,0 +1,105 @@ +# frozen_string_literal: true + +require_relative 'base' +require_relative '../../../../lib/gitlab_edition' + +# Returns view files that include the potential rails partials from the changed files passed as input. +module Tooling + module Mappings + class PartialToViewsMappings < Base + def initialize(changes_file, output_file, view_base_folder: 'app/views') + @output_file = output_file + @changed_files = File.read(changes_file).split(' ') + @view_base_folders = folders_for_available_editions(view_base_folder) + end + + def execute + views_including_modified_partials = [] + + views_globs = view_base_folders.map { |view_base_folder| "#{view_base_folder}/**/*.html.haml" } + Dir[*views_globs].each do |view_file| + included_partial_names = find_pattern_in_file(view_file, partials_keywords_regexp) + next if included_partial_names.empty? + + included_partial_names.each do |included_partial_name| + if view_includes_modified_partial?(view_file, included_partial_name) + views_including_modified_partials << view_file + end + end + end + + File.write(output_file, views_including_modified_partials.join(' ')) + end + + def filter_files + @_filter_files ||= changed_files.select do |filename| + filename.start_with?(*view_base_folders) && + File.basename(filename).start_with?('_') && + File.basename(filename).end_with?('.html.haml') && + File.exist?(filename) + end + end + + def partials_keywords_regexp + partial_keywords = filter_files.map do |partial_filename| + extract_partial_keyword(partial_filename) + end + + partial_regexps = partial_keywords.map do |keyword| + %r{(?:render|render_if_exists)(?: |\()(?:partial: ?)?['"]([\w\-_/]*#{keyword})['"]} + end + + Regexp.union(partial_regexps) + end + + # e.g. if app/views/clusters/clusters/_sidebar.html.haml was modified, the partial keyword is `sidebar`. + def extract_partial_keyword(partial_filename) + File.basename(partial_filename).delete_prefix('_').delete_suffix('.html.haml') + end + + # Why do we need this method? + # + # Assume app/views/clusters/clusters/_sidebar.html.haml was modified in the MR. + # + # Suppose now you find = render 'sidebar' in a view. Is this view including the sidebar partial + # that was modified, or another partial called "_sidebar.html.haml" somewhere else? + def view_includes_modified_partial?(view_file, included_partial_name) + view_file_parent_folder = File.dirname(view_file) + included_partial_filename = reconstruct_partial_filename(included_partial_name) + included_partial_relative_path = File.join(view_file_parent_folder, included_partial_filename) + + # We do this because in render (or render_if_exists) + # apparently looks for partials in other GitLab editions + # + # Example: + # + # ee/app/views/events/_epics_filter.html.haml is used in app/views/shared/_event_filter.html.haml + # with render_if_exists 'events/epics_filter' + included_partial_absolute_paths = view_base_folders.map do |view_base_folder| + File.join(view_base_folder, included_partial_filename) + end + + filter_files.include?(included_partial_relative_path) || + (filter_files & included_partial_absolute_paths).any? + end + + def reconstruct_partial_filename(partial_name) + partial_path = partial_name.split('/')[..-2] + partial_filename = partial_name.split('/').last + full_partial_filename = "_#{partial_filename}.html.haml" + + return full_partial_filename if partial_path.empty? + + File.join(partial_path.join('/'), full_partial_filename) + end + + def find_pattern_in_file(file, pattern) + File.read(file).scan(pattern).flatten.uniq + end + + private + + attr_reader :changed_files, :output_file, :view_base_folders + end + end +end