From f05cbbf6cfcb90ec5ed61db304b9f09f37dd6d4f Mon Sep 17 00:00:00 2001
From: David Dieulivol <ddieulivol@gitlab.com>
Date: Wed, 29 Mar 2023 08:11:51 +0000
Subject: [PATCH] Resolve "test-selection gap: run system specs when some views
changed"
---
.gitlab/ci/setup.gitlab-ci.yml | 1 +
.../view_to_system_specs_mappings_spec.rb | 122 ++++++++++++++++++
tooling/bin/view_to_system_specs_mappings | 9 ++
.../mappings/view_to_system_specs_mappings.rb | 64 +++++++++
4 files changed, 196 insertions(+)
create mode 100644 spec/tooling/lib/tooling/mappings/view_to_system_specs_mappings_spec.rb
create mode 100755 tooling/bin/view_to_system_specs_mappings
create mode 100644 tooling/lib/tooling/mappings/view_to_system_specs_mappings.rb
diff --git a/.gitlab/ci/setup.gitlab-ci.yml b/.gitlab/ci/setup.gitlab-ci.yml
index 3ebbe1e0fed66..6e24bf946bdbe 100644
--- a/.gitlab/ci/setup.gitlab-ci.yml
+++ b/.gitlab/ci/setup.gitlab-ci.yml
@@ -124,6 +124,7 @@ detect-tests:
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};
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};
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 0000000000000..d4f4bbd7f06fb
--- /dev/null
+++ b/spec/tooling/lib/tooling/mappings/view_to_system_specs_mappings_spec.rb
@@ -0,0 +1,122 @@
+# 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, :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_initial_content) { "previously_added_spec.rb" }
+
+ 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
+ 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)
+ 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) }
+ end
+ end
+
+ describe '#execute' do
+ subject { instance.execute }
+
+ let(:changed_files) { ["#{view_base_folder}/app/views/dashboard/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
+
+ 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(output_file) }
+ .from(output_file_initial_content)
+ .to("#{output_file_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(output_file) }
+ .from(output_file_initial_content)
+ .to("#{output_file_initial_content} #{expected_feature_specs.join(' ')}")
+ end
+ end
+ end
+ end
+ end
+end
diff --git a/tooling/bin/view_to_system_specs_mappings b/tooling/bin/view_to_system_specs_mappings
new file mode 100755
index 0000000000000..b2e3f2a132ad8
--- /dev/null
+++ b/tooling/bin/view_to_system_specs_mappings
@@ -0,0 +1,9 @@
+#!/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/mappings/view_to_system_specs_mappings.rb b/tooling/lib/tooling/mappings/view_to_system_specs_mappings.rb
new file mode 100644
index 0000000000000..227f2c6e62b24
--- /dev/null
+++ b/tooling/lib/tooling/mappings/view_to_system_specs_mappings.rb
@@ -0,0 +1,64 @@
+# frozen_string_literal: true
+
+require_relative 'base'
+require_relative '../../../../lib/gitlab_edition'
+
+# Returns system specs files that are related to the Rails views files that were changed in the MR.
+module Tooling
+ module Mappings
+ class ViewToSystemSpecsMappings < Base
+ 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)
+ end
+
+ def execute
+ found_system_specs = []
+
+ filter_files.each do |modified_view_file|
+ system_specs_exact_match = find_system_specs_exact_match(modified_view_file)
+ if system_specs_exact_match
+ found_system_specs << system_specs_exact_match
+ next
+ else
+ system_specs_parent_folder_match = find_system_specs_parent_folder_match(modified_view_file)
+ found_system_specs += system_specs_parent_folder_match unless system_specs_parent_folder_match.empty?
+ end
+ end
+
+ write_array_to_file(output_file, found_system_specs.compact.uniq.sort)
+ end
+
+ private
+
+ attr_reader :changed_files, :output_file, :view_base_folders
+
+ # Keep the views files that are in the @view_base_folders folder
+ def filter_files
+ @_filter_files ||= changed_files.select do |filename|
+ filename.start_with?(*view_base_folders) &&
+ File.basename(filename).end_with?('.html.haml') &&
+ File.exist?(filename)
+ end
+ end
+
+ def find_system_specs_exact_match(view_file)
+ potential_spec_file = to_feature_spec_folder(view_file).sub('.html.haml', '_spec.rb')
+
+ potential_spec_file if File.exist?(potential_spec_file)
+ end
+
+ def find_system_specs_parent_folder_match(view_file)
+ parent_system_specs_folder = File.dirname(to_feature_spec_folder(view_file))
+
+ Dir["#{parent_system_specs_folder}/**/*_spec.rb"]
+ end
+
+ # e.g. go from app/views/groups/merge_requests.html.haml to spec/features/groups/merge_requests.html.haml
+ def to_feature_spec_folder(view_file)
+ view_file.sub(%r{(ee/|jh/)?app/views}, '\1spec/features')
+ end
+ end
+ end
+end
--
GitLab