From 264fc6e1b02dc53380c3ba2f7cdcbbab82a6b546 Mon Sep 17 00:00:00 2001
From: David Dieulivol <ddieulivol@gitlab.com>
Date: Mon, 24 Apr 2023 20:18:17 +0000
Subject: [PATCH] Move mappings base class to a module
We need to reuse the methods from Mappings::Base class
outside of the Mappings module, so we extract the content
of that class to a module that can be included anywhere.
---
.../find_files_using_feature_flags_spec.rb | 122 ++++++++++++++++++
.../predictive_tests_helper_spec.rb} | 13 +-
tooling/lib/tooling/find_changes.rb | 4 +-
.../tooling/find_files_using_feature_flags.rb | 48 +++++++
tooling/lib/tooling/find_tests.rb | 4 +-
.../predictive_tests_helper.rb} | 6 +-
.../mappings/graphql_base_type_mappings.rb | 6 +-
.../mappings/js_to_system_specs_mappings.rb | 6 +-
.../mappings/partial_to_views_mappings.rb | 6 +-
.../tooling/mappings/view_to_js_mappings.rb | 6 +-
.../mappings/view_to_system_specs_mappings.rb | 6 +-
tooling/lib/tooling/predictive_tests.rb | 2 +
12 files changed, 209 insertions(+), 20 deletions(-)
create mode 100644 spec/tooling/lib/tooling/find_files_using_feature_flags_spec.rb
rename spec/tooling/lib/tooling/{mappings/base_spec.rb => helpers/predictive_tests_helper_spec.rb} (72%)
create mode 100644 tooling/lib/tooling/find_files_using_feature_flags.rb
rename tooling/lib/tooling/{mappings/base.rb => helpers/predictive_tests_helper.rb} (88%)
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 0000000000000..f553d34768fec
--- /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/mappings/base_spec.rb b/spec/tooling/lib/tooling/helpers/predictive_tests_helper_spec.rb
similarity index 72%
rename from spec/tooling/lib/tooling/mappings/base_spec.rb
rename to spec/tooling/lib/tooling/helpers/predictive_tests_helper_spec.rb
index 935f833fa8b09..48a5866ac568c 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/tooling/lib/tooling/find_changes.rb b/tooling/lib/tooling/find_changes.rb
index d8373d1124581..25381e1a89446 100755
--- a/tooling/lib/tooling/find_changes.rb
+++ b/tooling/lib/tooling/find_changes.rb
@@ -2,11 +2,11 @@
# frozen_string_literal: true
require 'gitlab'
-require_relative 'helpers/file_handler'
+require_relative 'helpers/predictive_tests_helper'
module Tooling
class FindChanges
- include Helpers::FileHandler
+ include Helpers::PredictiveTestsHelper
def initialize(
from:,
diff --git a/tooling/lib/tooling/find_files_using_feature_flags.rb b/tooling/lib/tooling/find_files_using_feature_flags.rb
new file mode 100644
index 0000000000000..27cace604089b
--- /dev/null
+++ b/tooling/lib/tooling/find_files_using_feature_flags.rb
@@ -0,0 +1,48 @@
+# frozen_string_literal: true
+
+require 'test_file_finder'
+require_relative 'helpers/predictive_tests_helper'
+
+module Tooling
+ class FindFilesUsingFeatureFlags
+ include Helpers::PredictiveTestsHelper
+
+ def initialize(changed_files_pathname:, feature_flags_base_folder: 'config/feature_flags')
+ @changed_files_pathname = changed_files_pathname
+ @changed_files = read_array_from_file(changed_files_pathname)
+ @feature_flags_base_folders = folders_for_available_editions(feature_flags_base_folder)
+ end
+
+ def execute
+ ff_union_regexp = Regexp.union(feature_flag_filenames)
+
+ files_using_modified_feature_flags = ruby_files.select do |ruby_file|
+ ruby_file if ff_union_regexp.match?(File.read(ruby_file))
+ end
+
+ write_array_to_file(changed_files_pathname, files_using_modified_feature_flags.uniq)
+ end
+
+ def filter_files
+ @_filter_files ||= changed_files.select do |filename|
+ filename.start_with?(*feature_flags_base_folders) &&
+ File.basename(filename).end_with?('.yml') &&
+ File.exist?(filename)
+ end
+ end
+
+ private
+
+ def feature_flag_filenames
+ filter_files.map do |feature_flag_pathname|
+ File.basename(feature_flag_pathname).delete_suffix('.yml')
+ end
+ end
+
+ def ruby_files
+ Dir["**/*.rb"].reject { |pathname| pathname.start_with?('vendor') }
+ end
+
+ attr_reader :changed_files, :changed_files_pathname, :feature_flags_base_folders
+ end
+end
diff --git a/tooling/lib/tooling/find_tests.rb b/tooling/lib/tooling/find_tests.rb
index f26c1eacdc7e3..bf7a608878b67 100644
--- a/tooling/lib/tooling/find_tests.rb
+++ b/tooling/lib/tooling/find_tests.rb
@@ -1,11 +1,11 @@
# frozen_string_literal: true
require 'test_file_finder'
-require_relative 'helpers/file_handler'
+require_relative 'helpers/predictive_tests_helper'
module Tooling
class FindTests
- include Helpers::FileHandler
+ include Helpers::PredictiveTestsHelper
def initialize(changed_files_pathname, predictive_tests_pathname)
@predictive_tests_pathname = predictive_tests_pathname
diff --git a/tooling/lib/tooling/mappings/base.rb b/tooling/lib/tooling/helpers/predictive_tests_helper.rb
similarity index 88%
rename from tooling/lib/tooling/mappings/base.rb
rename to tooling/lib/tooling/helpers/predictive_tests_helper.rb
index 27a9a0925b04f..b8e5a30024e5f 100644
--- a/tooling/lib/tooling/mappings/base.rb
+++ b/tooling/lib/tooling/helpers/predictive_tests_helper.rb
@@ -5,9 +5,9 @@
# Returns system specs files that are related to the JS files that were changed in the MR.
module Tooling
- module Mappings
- class Base
- include Helpers::FileHandler
+ module Helpers
+ module PredictiveTestsHelper
+ include FileHandler
# Input: A folder
# Output: An array of folders, each prefixed with a GitLab edition
diff --git a/tooling/lib/tooling/mappings/graphql_base_type_mappings.rb b/tooling/lib/tooling/mappings/graphql_base_type_mappings.rb
index 569a82781634b..80aa99efc9639 100644
--- a/tooling/lib/tooling/mappings/graphql_base_type_mappings.rb
+++ b/tooling/lib/tooling/mappings/graphql_base_type_mappings.rb
@@ -2,13 +2,15 @@
require 'active_support/inflector'
-require_relative 'base'
+require_relative '../helpers/predictive_tests_helper'
require_relative '../../../../lib/gitlab_edition'
# If a GraphQL type class changed, we try to identify the other GraphQL types that potentially include this type.
module Tooling
module Mappings
- class GraphqlBaseTypeMappings < Base
+ class GraphqlBaseTypeMappings
+ include Helpers::PredictiveTestsHelper
+
# Checks for the implements keyword, and graphql_base_types the class name
GRAPHQL_IMPLEMENTS_REGEXP = /implements[( ]([\w:]+)[)]?$/
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 b2fca3a765a1c..bc2cd259fdcca 100644
--- a/tooling/lib/tooling/mappings/js_to_system_specs_mappings.rb
+++ b/tooling/lib/tooling/mappings/js_to_system_specs_mappings.rb
@@ -2,13 +2,15 @@
require 'active_support/inflector'
-require_relative 'base'
+require_relative '../helpers/predictive_tests_helper'
require_relative '../../../../lib/gitlab_edition'
# Returns system specs files that are related to the JS files that were changed in the MR.
module Tooling
module Mappings
- class JsToSystemSpecsMappings < Base
+ class JsToSystemSpecsMappings
+ include Helpers::PredictiveTestsHelper
+
def initialize(
changed_files_pathname, predictive_tests_pathname,
js_base_folder: 'app/assets/javascripts', system_specs_base_folder: 'spec/features')
diff --git a/tooling/lib/tooling/mappings/partial_to_views_mappings.rb b/tooling/lib/tooling/mappings/partial_to_views_mappings.rb
index 8b0a5ed4ecd48..931cacea77f21 100644
--- a/tooling/lib/tooling/mappings/partial_to_views_mappings.rb
+++ b/tooling/lib/tooling/mappings/partial_to_views_mappings.rb
@@ -1,12 +1,14 @@
# frozen_string_literal: true
-require_relative 'base'
+require_relative '../helpers/predictive_tests_helper'
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
+ class PartialToViewsMappings
+ include Helpers::PredictiveTestsHelper
+
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)
diff --git a/tooling/lib/tooling/mappings/view_to_js_mappings.rb b/tooling/lib/tooling/mappings/view_to_js_mappings.rb
index f2098d6acd570..b78c354f9d2e4 100644
--- a/tooling/lib/tooling/mappings/view_to_js_mappings.rb
+++ b/tooling/lib/tooling/mappings/view_to_js_mappings.rb
@@ -1,12 +1,14 @@
# frozen_string_literal: true
-require_relative 'base'
+require_relative '../helpers/predictive_tests_helper'
require_relative '../../../../lib/gitlab_edition'
# Returns JS files that are related to the Rails views files that were changed in the MR.
module Tooling
module Mappings
- class ViewToJsMappings < Base
+ class ViewToJsMappings
+ include Helpers::PredictiveTestsHelper
+
# The HTML attribute value pattern we're looking for to match an HTML file to a JS file.
HTML_ATTRIBUTE_VALUE_REGEXP = /js-[-\w]+/.freeze
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 6d840dcbd7166..1542c81774549 100644
--- a/tooling/lib/tooling/mappings/view_to_system_specs_mappings.rb
+++ b/tooling/lib/tooling/mappings/view_to_system_specs_mappings.rb
@@ -1,12 +1,14 @@
# frozen_string_literal: true
-require_relative 'base'
+require_relative '../helpers/predictive_tests_helper'
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
+ class ViewToSystemSpecsMappings
+ include Helpers::PredictiveTestsHelper
+
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)
diff --git a/tooling/lib/tooling/predictive_tests.rb b/tooling/lib/tooling/predictive_tests.rb
index 503426b552096..1ad63e111e3d1 100644
--- a/tooling/lib/tooling/predictive_tests.rb
+++ b/tooling/lib/tooling/predictive_tests.rb
@@ -2,6 +2,7 @@
require_relative 'find_changes'
require_relative 'find_tests'
+require_relative 'find_files_using_feature_flags'
require_relative 'mappings/graphql_base_type_mappings'
require_relative 'mappings/js_to_system_specs_mappings'
require_relative 'mappings/partial_to_views_mappings'
@@ -36,6 +37,7 @@ def execute
from: :api,
changed_files_pathname: rspec_changed_files_path
).execute
+ Tooling::FindFilesUsingFeatureFlags.new(changed_files_pathname: 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
--
GitLab