From d889ffa00641e4d84fd08eb83903915c42256aa6 Mon Sep 17 00:00:00 2001
From: David Dieulivol <ddieulivol@gitlab.com>
Date: Wed, 11 Jan 2023 18:10:10 +0000
Subject: [PATCH] Make "jest minimal" CI jobs smarter with an "HTML->JS
mapping"
---
.gitlab-ci.yml | 25 +-
.gitlab/ci/frontend.gitlab-ci.yml | 4 +-
.gitlab/ci/setup.gitlab-ci.yml | 7 +-
.../_saml_group_links.html.haml | 2 +-
.../_ldap_group_links.html.haml | 2 +-
package.json | 2 +-
.../lib/tooling/view_to_js_mappings_spec.rb | 343 ++++++++++++++++++
tooling/bin/view_to_js_mappings | 10 +
tooling/lib/tooling/view_to_js_mappings.rb | 74 ++++
9 files changed, 450 insertions(+), 19 deletions(-)
create mode 100644 spec/tooling/lib/tooling/view_to_js_mappings_spec.rb
create mode 100755 tooling/bin/view_to_js_mappings
create mode 100644 tooling/lib/tooling/view_to_js_mappings.rb
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 79c402c7fa7f3..9c7b66676a8ab 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -126,22 +126,23 @@ variables:
RUBY_VERSION: "2.7"
GO_VERSION: "1.18"
- TMP_TEST_FOLDER: "${CI_PROJECT_DIR}/tmp/tests"
- GITLAB_WORKHORSE_FOLDER: "gitlab-workhorse"
- TMP_TEST_GITLAB_WORKHORSE_PATH: "${TMP_TEST_FOLDER}/${GITLAB_WORKHORSE_FOLDER}"
- KNAPSACK_RSPEC_SUITE_REPORT_PATH: knapsack/report-master.json
FLAKY_RSPEC_SUITE_REPORT_PATH: rspec/flaky/report-suite.json
- RSPEC_TESTS_MAPPING_PATH: crystalball/mapping.json
- RSPEC_PACKED_TESTS_MAPPING_PATH: crystalball/packed-mapping.json
- RSPEC_PROFILING_FOLDER_PATH: rspec/profiling
FRONTEND_FIXTURES_MAPPING_PATH: crystalball/frontend_fixtures_mapping.json
- RSPEC_CHANGED_FILES_PATH: rspec/changed_files.txt
- RSPEC_MATCHING_TESTS_PATH: rspec/matching_tests.txt
- RSPEC_MATCHING_TESTS_FOSS_PATH: rspec/matching_tests-foss.txt
- RSPEC_LAST_RUN_RESULTS_FILE: rspec/rspec_last_run_results.txt
- RSPEC_FOSS_IMPACT_PIPELINE_YML: rspec-foss-impact-pipeline.yml
+ GITLAB_WORKHORSE_FOLDER: "gitlab-workhorse"
JUNIT_RESULT_FILE: rspec/junit_rspec.xml
JUNIT_RETRY_FILE: rspec/junit_rspec-retry.xml
+ KNAPSACK_RSPEC_SUITE_REPORT_PATH: knapsack/report-master.json
+ RSPEC_CHANGED_FILES_PATH: rspec/changed_files.txt
+ RSPEC_FOSS_IMPACT_PIPELINE_YML: rspec-foss-impact-pipeline.yml
+ RSPEC_LAST_RUN_RESULTS_FILE: rspec/rspec_last_run_results.txt
+ RSPEC_MATCHING_JS_FILES_PATH: rspec/js_matching_files.txt
+ RSPEC_MATCHING_TESTS_FOSS_PATH: rspec/matching_tests-foss.txt
+ RSPEC_MATCHING_TESTS_PATH: rspec/matching_tests.txt
+ RSPEC_PACKED_TESTS_MAPPING_PATH: crystalball/packed-mapping.json
+ RSPEC_PROFILING_FOLDER_PATH: rspec/profiling
+ RSPEC_TESTS_MAPPING_PATH: crystalball/mapping.json
+ TMP_TEST_FOLDER: "${CI_PROJECT_DIR}/tmp/tests"
+ TMP_TEST_GITLAB_WORKHORSE_PATH: "${TMP_TEST_FOLDER}/${GITLAB_WORKHORSE_FOLDER}"
ES_JAVA_OPTS: "-Xms256m -Xmx256m"
ELASTIC_URL: "http://elastic:changeme@elasticsearch:9200"
diff --git a/.gitlab/ci/frontend.gitlab-ci.yml b/.gitlab/ci/frontend.gitlab-ci.yml
index cde023c149a18..a309adb071550 100644
--- a/.gitlab/ci/frontend.gitlab-ci.yml
+++ b/.gitlab/ci/frontend.gitlab-ci.yml
@@ -212,7 +212,7 @@ jest minimal:
- !reference [jest, needs]
- "detect-tests"
script:
- - if [[ -s "$RSPEC_CHANGED_FILES_PATH" ]]; then run_timed_command "yarn jest:ci:minimal"; fi
+ - if [[ -s "$RSPEC_CHANGED_FILES_PATH" ]] || [[ -s "$RSPEC_MATCHING_JS_FILES_PATH" ]]; then run_timed_command "yarn jest:ci:minimal"; fi
jest as-if-foss:
extends:
@@ -231,7 +231,7 @@ jest minimal as-if-foss:
- "rspec-all frontend_fixture as-if-foss"
- "detect-tests"
script:
- - if [[ -s "$RSPEC_CHANGED_FILES_PATH" ]]; then run_timed_command "yarn jest:ci:minimal"; fi
+ - if [[ -s "$RSPEC_CHANGED_FILES_PATH" ]] || [[ -s "$RSPEC_MATCHING_JS_FILES_PATH" ]]; then run_timed_command "yarn jest:ci:minimal"; fi
jest-integration:
extends:
diff --git a/.gitlab/ci/setup.gitlab-ci.yml b/.gitlab/ci/setup.gitlab-ci.yml
index b7e69314c972b..326b32435b134 100644
--- a/.gitlab/ci/setup.gitlab-ci.yml
+++ b/.gitlab/ci/setup.gitlab-ci.yml
@@ -137,17 +137,20 @@ detect-tests:
tooling/bin/find_tests ${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};
+ 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 RSpec tests: $(cat $RSPEC_MATCHING_TESTS_PATH)";
echoinfo "Related FOSS RSpec tests: $(cat $RSPEC_MATCHING_TESTS_FOSS_PATH)";
+ echoinfo "Related JS files: $(cat $RSPEC_MATCHING_JS_FILES_PATH)";
fi
artifacts:
expire_in: 7d
paths:
+ - ${FRONTEND_FIXTURES_MAPPING_PATH}
- ${RSPEC_CHANGED_FILES_PATH}
- - ${RSPEC_MATCHING_TESTS_PATH}
+ - ${RSPEC_MATCHING_JS_FILES_PATH}
- ${RSPEC_MATCHING_TESTS_FOSS_PATH}
- - ${FRONTEND_FIXTURES_MAPPING_PATH}
+ - ${RSPEC_MATCHING_TESTS_PATH}
detect-previous-failed-tests:
extends:
diff --git a/ee/app/views/groups/saml_group_links/_saml_group_links.html.haml b/ee/app/views/groups/saml_group_links/_saml_group_links.html.haml
index 3d4507428f191..ef3810936a487 100644
--- a/ee/app/views/groups/saml_group_links/_saml_group_links.html.haml
+++ b/ee/app/views/groups/saml_group_links/_saml_group_links.html.haml
@@ -6,6 +6,6 @@
- c.body do
- if group_links.any?
%ul.content-list
- = render collection: group_links, partial: 'saml_group_link', locals: { group: group }
+ = render partial: 'saml_group_link', collection: group_links, locals: { group: group }
- else
= s_('GroupSAML|No active SAML group links')
diff --git a/ee/app/views/ldap_group_links/_ldap_group_links.html.haml b/ee/app/views/ldap_group_links/_ldap_group_links.html.haml
index 9699b80e031c8..df35e8090455d 100644
--- a/ee/app/views/ldap_group_links/_ldap_group_links.html.haml
+++ b/ee/app/views/ldap_group_links/_ldap_group_links.html.haml
@@ -5,7 +5,7 @@
- if group.ldap_group_links.any?
%ul.content-list
- = render collection: group.ldap_group_links, partial: 'ldap_group_links/ldap_group_link', locals: { group: group }
+ = render partial: 'ldap_group_links/ldap_group_link', collection: group.ldap_group_links, locals: { group: group }
- else
.card-body
No LDAP synchronizations
diff --git a/package.json b/package.json
index 92de33c53efcb..1f57e1aaf3366 100644
--- a/package.json
+++ b/package.json
@@ -14,7 +14,7 @@
"jest": "jest --config jest.config.js",
"jest-debug": "node --inspect-brk node_modules/.bin/jest --runInBand",
"jest:ci": "jest --config jest.config.js --ci --coverage --testSequencer ./scripts/frontend/parallel_ci_sequencer.js",
- "jest:ci:minimal": "jest --config jest.config.js --ci --coverage --findRelatedTests $(cat $RSPEC_CHANGED_FILES_PATH) --passWithNoTests --testSequencer ./scripts/frontend/parallel_ci_sequencer.js",
+ "jest:ci:minimal": "jest --config jest.config.js --ci --coverage --findRelatedTests $(cat $RSPEC_CHANGED_FILES_PATH) $(cat $RSPEC_MATCHING_JS_FILES_PATH) --passWithNoTests --testSequencer ./scripts/frontend/parallel_ci_sequencer.js",
"jest:contract": "PACT_DO_NOT_TRACK=true jest --config jest.config.contract.js --runInBand",
"jest:integration": "jest --config jest.config.integration.js",
"lint:eslint": "node scripts/frontend/eslint.js",
diff --git a/spec/tooling/lib/tooling/view_to_js_mappings_spec.rb b/spec/tooling/lib/tooling/view_to_js_mappings_spec.rb
new file mode 100644
index 0000000000000..e4c5737df54c6
--- /dev/null
+++ b/spec/tooling/lib/tooling/view_to_js_mappings_spec.rb
@@ -0,0 +1,343 @@
+# frozen_string_literal: true
+
+require 'tempfile'
+require_relative '../../../../tooling/lib/tooling/view_to_js_mappings'
+
+RSpec.describe Tooling::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
+
+ around do |example|
+ Dir.mktmpdir do |tmp_js_base_folder|
+ Dir.mktmpdir do |tmp_views_base_folder|
+ self.js_base_folder = tmp_js_base_folder
+ self.view_base_folder = tmp_views_base_folder
+
+ example.run
+ end
+ end
+ end
+
+ describe '#execute' do
+ let(:instance) do
+ described_class.new(
+ view_base_folder: view_base_folder,
+ js_base_folder: js_base_folder
+ )
+ end
+
+ let(:changed_files) { %W[#{view_base_folder}/index.html] }
+
+ subject { instance.execute(changed_files) }
+
+ context 'when no view files have been changed' do
+ before do
+ allow(instance).to receive(:view_files).and_return([])
+ end
+
+ it 'returns nothing' do
+ expect(subject).to match_array([])
+ end
+ end
+
+ context 'when some view files have been changed' do
+ before do
+ File.write("#{view_base_folder}/index.html", index_html_content)
+ end
+
+ context 'when they do not contain the HTML attribute value we search for' do
+ let(:index_html_content) do
+ <<~FILE
+ Beginning of file
+ End of file
+ FILE
+ end
+
+ it 'returns nothing' do
+ expect(subject).to match_array([])
+ end
+ end
+
+ context 'when they contain the HTML attribute value we search for' do
+ let(:index_html_content) do
+ <<~FILE
+ Beginning of file
+
+ <a id="js-some-id">A link</a>
+
+ End of file
+ FILE
+ end
+
+ context 'when no matching JS files are found' do
+ it 'returns nothing' do
+ expect(subject).to match_array([])
+ end
+ end
+
+ context 'when some matching JS files are found' do
+ let(:index_js_content) do
+ <<~FILE
+ Beginning of file
+
+ const isMainAwardsBlock = votesBlock.closest('#js-some-id.some_class').length;
+
+ End of file
+ FILE
+ end
+
+ before do
+ File.write("#{js_base_folder}/index.js", index_js_content)
+ end
+
+ it 'returns the matching JS files' do
+ expect(subject).to match_array(["#{js_base_folder}/index.js"])
+ end
+ end
+ end
+ end
+
+ context 'when rails partials are included in the file' do
+ before do
+ File.write("#{view_base_folder}/index.html", index_html_content)
+ File.write("#{view_base_folder}/_my-partial.html.haml", partial_file_content)
+ File.write("#{js_base_folder}/index.js", index_js_content)
+ end
+
+ let(:index_html_content) do
+ <<~FILE
+ Beginning of file
+
+ = render 'my-partial'
+
+ End of file
+ FILE
+ end
+
+ let(:partial_file_content) do
+ <<~FILE
+ Beginning of file
+
+ <a class="js-some-class">A link with class</a>
+
+ End of file
+ FILE
+ end
+
+ let(:index_js_content) do
+ <<~FILE
+ Beginning of file
+
+ const isMainAwardsBlock = votesBlock.closest('.js-some-class').length;
+
+ End of file
+ FILE
+ end
+
+ it 'scans those partials for the HTML attribute value' do
+ expect(subject).to match_array(["#{js_base_folder}/index.js"])
+ end
+ end
+ end
+
+ describe '#view_files' do
+ subject { described_class.new(view_base_folder: view_base_folder).view_files(changed_files) }
+
+ context 'when no files were changed' do
+ let(:changed_files) { [] }
+
+ it 'returns an empty array' do
+ expect(subject).to match_array([])
+ end
+ end
+
+ context 'when no view files were changed' do
+ let(:changed_files) { ["#{js_base_folder}/index.js"] }
+
+ it 'returns an empty array' do
+ expect(subject).to match_array([])
+ end
+ end
+
+ context 'when view files were changed' do
+ let(:changed_files) { ["#{js_base_folder}/index.js", "#{view_base_folder}/index.html"] }
+
+ it 'returns the path to the view files' do
+ expect(subject).to match_array(["#{view_base_folder}/index.html"])
+ end
+ end
+ end
+
+ describe '#folders_for_available_editions' do
+ let(:base_folder_path) { 'app/views' }
+
+ subject { described_class.new.folders_for_available_editions(base_folder_path) }
+
+ context 'when FOSS' do
+ before do
+ allow(GitlabEdition).to receive(:ee?).and_return(false)
+ allow(GitlabEdition).to receive(:jh?).and_return(false)
+ end
+
+ it 'returns the correct paths' do
+ expect(subject).to match_array([base_folder_path])
+ end
+ end
+
+ context 'when EE' do
+ before do
+ allow(GitlabEdition).to receive(:ee?).and_return(true)
+ allow(GitlabEdition).to receive(:jh?).and_return(false)
+ end
+
+ it 'returns the correct paths' do
+ expect(subject).to eq([base_folder_path, "ee/#{base_folder_path}"])
+ end
+ end
+
+ context 'when JiHu' do
+ before do
+ allow(GitlabEdition).to receive(:ee?).and_return(true)
+ allow(GitlabEdition).to receive(:jh?).and_return(true)
+ end
+
+ it 'returns the correct paths' do
+ expect(subject).to eq([base_folder_path, "ee/#{base_folder_path}", "jh/#{base_folder_path}"])
+ end
+ end
+ end
+
+ describe '#find_partials' do
+ subject { described_class.new(view_base_folder: view_base_folder).find_partials(file_path) }
+
+ let(:file_path) { "#{view_base_folder}/my_html_file.html" }
+
+ before do
+ File.write(file_path, file_content)
+ end
+
+ context 'when the file includes a partial' do
+ context 'when the partial is in the same folder as the view file' do
+ before do
+ File.write("#{view_base_folder}/_my-partial.html.haml", 'Hello from partial')
+ end
+
+ let(:file_content) do
+ <<~FILE
+ Beginning of file
+
+ = render "my-partial"
+
+ End of file
+ FILE
+ end
+
+ it "returns the partial file path" do
+ expect(subject).to match_array(["#{view_base_folder}/_my-partial.html.haml"])
+ end
+ end
+
+ context 'when the partial is in a subfolder' do
+ before do
+ FileUtils.mkdir_p("#{view_base_folder}/subfolder")
+
+ (1..12).each do |i|
+ FileUtils.touch "#{view_base_folder}/subfolder/_my-partial#{i}.html.haml"
+ end
+ end
+
+ let(:file_content) do
+ <<~FILE
+ Beginning of file
+
+ = render("subfolder/my-partial1")
+ = render "subfolder/my-partial2"
+ = render(partial: "subfolder/my-partial3")
+ = render partial: "subfolder/my-partial4"
+ = render(partial:"subfolder/my-partial5", path: 'else')
+ = render partial:"subfolder/my-partial6"
+ = render_if_exist("subfolder/my-partial7", path: 'else')
+ = render_if_exist "subfolder/my-partial8"
+ = render_if_exist(partial: "subfolder/my-partial9", path: 'else')
+ = render_if_exist partial: "subfolder/my-partial10"
+ = render_if_exist(partial:"subfolder/my-partial11", path: 'else')
+ = render_if_exist partial:"subfolder/my-partial12"
+
+ End of file
+ FILE
+ end
+
+ it "returns the partials file path" do
+ expect(subject).to match_array([
+ "#{view_base_folder}/subfolder/_my-partial1.html.haml",
+ "#{view_base_folder}/subfolder/_my-partial2.html.haml",
+ "#{view_base_folder}/subfolder/_my-partial3.html.haml",
+ "#{view_base_folder}/subfolder/_my-partial4.html.haml",
+ "#{view_base_folder}/subfolder/_my-partial5.html.haml",
+ "#{view_base_folder}/subfolder/_my-partial6.html.haml",
+ "#{view_base_folder}/subfolder/_my-partial7.html.haml",
+ "#{view_base_folder}/subfolder/_my-partial8.html.haml",
+ "#{view_base_folder}/subfolder/_my-partial9.html.haml",
+ "#{view_base_folder}/subfolder/_my-partial10.html.haml",
+ "#{view_base_folder}/subfolder/_my-partial11.html.haml",
+ "#{view_base_folder}/subfolder/_my-partial12.html.haml"
+ ])
+ end
+ end
+
+ context 'when the file does not include a partial' 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
+
+ describe '#find_pattern_in_file' do
+ let(:subject) { described_class.new.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/view_to_js_mappings b/tooling/bin/view_to_js_mappings
new file mode 100755
index 0000000000000..2cebb91892e75
--- /dev/null
+++ b/tooling/bin/view_to_js_mappings
@@ -0,0 +1,10 @@
+#!/usr/bin/env ruby
+# frozen_string_literal: true
+
+require_relative '../lib/tooling/view_to_js_mappings'
+
+changes = ARGV.shift
+output_file = ARGV.shift
+changed_files = File.read(changes).split(' ')
+
+File.write(output_file, Tooling::ViewToJsMappings.new.execute(changed_files).join(' '))
diff --git a/tooling/lib/tooling/view_to_js_mappings.rb b/tooling/lib/tooling/view_to_js_mappings.rb
new file mode 100644
index 0000000000000..48568db382b25
--- /dev/null
+++ b/tooling/lib/tooling/view_to_js_mappings.rb
@@ -0,0 +1,74 @@
+# frozen_string_literal: true
+
+require_relative '../../../lib/gitlab_edition'
+
+# Returns JS files that are related to the Rails views files that were changed in the MR.
+module Tooling
+ class ViewToJsMappings
+ # 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
+
+ # Search for Rails partials included in an HTML file
+ RAILS_PARTIAL_INVOCATION_REGEXP = %r{(?:render|render_if_exist)(?: |\()(?:partial: ?)?['"]([\w/-]+)['"]}.freeze
+
+ def initialize(view_base_folder: 'app/views', js_base_folder: 'app/assets/javascripts')
+ @view_base_folders = folders_for_available_editions(view_base_folder)
+ @js_base_folders = folders_for_available_editions(js_base_folder)
+ end
+
+ def execute(changed_files)
+ changed_view_files = view_files(changed_files)
+
+ partials = changed_view_files.flat_map do |file|
+ find_partials(file)
+ end
+
+ files_to_scan = changed_view_files + partials
+ js_tags = files_to_scan.flat_map do |file|
+ find_pattern_in_file(file, HTML_ATTRIBUTE_VALUE_REGEXP)
+ end
+ js_tags_regexp = Regexp.union(js_tags)
+
+ @js_base_folders.flat_map do |js_base_folder|
+ Dir["#{js_base_folder}/**/*.{js,vue}"].select do |js_file|
+ file_content = File.read(js_file)
+ js_tags_regexp.match?(file_content)
+ end
+ end
+ end
+
+ # Keep the files that are in the @view_base_folders folder
+ def view_files(changed_files)
+ changed_files.select { |filename| filename.start_with?(*@view_base_folders) }
+ end
+
+ def folders_for_available_editions(base_folder)
+ foss_prefix = base_folder
+ extension_prefixes = ::GitlabEdition.extensions.map { |prefix| "#{prefix}/#{foss_prefix}" }
+ [foss_prefix, *extension_prefixes]
+ end
+
+ # Note: We only search for partials with depth 1. We don't do recursive search, as
+ # it is probably not necessary for a first iteration.
+ def find_partials(file)
+ partial_paths = find_pattern_in_file(file, RAILS_PARTIAL_INVOCATION_REGEXP)
+ partial_paths.flat_map do |partial_path|
+ view_file_folder = File.dirname(file)
+ partial_relative_folder = File.dirname(partial_path)
+
+ dirname =
+ if partial_relative_folder == '.' # The partial is in the same folder as the HTML file
+ view_file_folder
+ else
+ File.join(view_file_folder, partial_relative_folder)
+ end
+
+ Dir["#{dirname}/_#{File.basename(partial_path)}.*"]
+ end
+ end
+
+ def find_pattern_in_file(file, pattern)
+ File.read(file).scan(pattern).flatten.uniq
+ end
+ end
+end
--
GitLab