From ed3f9655838288e2ac570e4dbe949d1198953eaf Mon Sep 17 00:00:00 2001
From: Niklas <mc.taucher2003@gmail.com>
Date: Thu, 11 Jan 2024 17:29:51 +0000
Subject: [PATCH] Add only_new_paths option to Tooling::FindChanges
When a file in the documentation is moved, the old
path was also passed to markdownlint and vale.
That causes the linting to fail, because the files
don't exist.
In order to fix that, the new option will only return
the new paths of the changed files and not the old paths.
---
scripts/lint-doc.sh | 3 +-
spec/tooling/lib/tooling/find_changes_spec.rb | 35 ++++++++++++++++++-
tooling/lib/tooling/find_changes.rb | 8 +++--
3 files changed, 41 insertions(+), 5 deletions(-)
diff --git a/scripts/lint-doc.sh b/scripts/lint-doc.sh
index 46d7159d71fa6..eccc5c0c748eb 100755
--- a/scripts/lint-doc.sh
+++ b/scripts/lint-doc.sh
@@ -143,7 +143,8 @@ then
ruby -r './tooling/lib/tooling/find_changes' -e "Tooling::FindChanges.new(
from: :api,
changed_files_pathname: '${DOC_CHANGES_FILE}',
- file_filter: ->(file) { !file['deleted_file'] && file['new_path'] =~ %r{doc/.*\.md|lint-doc\.sh|docs\.gitlab-ci\.yml} }
+ file_filter: ->(file) { !file['deleted_file'] && file['new_path'] =~ %r{doc/.*\.md|\.vale|\.markdownlint|lint-doc\.sh|docs\.gitlab-ci\.yml} },
+ only_new_paths: true
).execute"
if grep -E "\.vale|\.markdownlint|lint-doc\.sh|docs\.gitlab-ci\.yml" < $DOC_CHANGES_FILE
then
diff --git a/spec/tooling/lib/tooling/find_changes_spec.rb b/spec/tooling/lib/tooling/find_changes_spec.rb
index 85e3eadac6fc8..be28b228eddf9 100644
--- a/spec/tooling/lib/tooling/find_changes_spec.rb
+++ b/spec/tooling/lib/tooling/find_changes_spec.rb
@@ -16,7 +16,8 @@
predictive_tests_pathname: predictive_tests_pathname,
frontend_fixtures_mapping_pathname: frontend_fixtures_mapping_pathname,
from: from,
- file_filter: file_filter)
+ file_filter: file_filter,
+ only_new_paths: only_new_paths)
end
let(:changed_files_pathname) { changed_files_file.path }
@@ -25,6 +26,7 @@
let(:from) { :api }
let(:gitlab_client) { double('GitLab') } # rubocop:disable RSpec/VerifiedDoubles
let(:file_filter) { ->(_) { true } }
+ let(:only_new_paths) { false }
around do |example|
self.changed_files_file = Tempfile.new('changed_files_file')
@@ -122,6 +124,37 @@
expect(File.read(changed_files_file)).to eq('doc/index.md')
end
end
+
+ context 'when used with only_new_paths' do
+ let(:only_new_paths) { true }
+
+ let(:mr_changes_array) do
+ [
+ {
+ "new_path" => "scripts/test.js",
+ "old_path" => "scripts/test.js"
+ },
+ {
+ "new_path" => "doc/renamed_index.md",
+ "old_path" => "doc/index.md"
+ }
+ ]
+ end
+
+ before do
+ # rubocop:disable RSpec/VerifiedDoubles -- The class from the GitLab gem isn't public, so we cannot use verified doubles for it.
+ allow(gitlab_client).to receive(:merge_request_changes)
+ .with('dummy-project', '1234')
+ .and_return(double(changes: mr_changes_array))
+ # rubocop:enable RSpec/VerifiedDoubles
+ end
+
+ it 'only writes new file paths to output' do
+ subject
+
+ expect(File.read(changed_files_file)).to eq('doc/renamed_index.md scripts/test.js')
+ end
+ end
end
context 'when fetching changes from changed files' do
diff --git a/tooling/lib/tooling/find_changes.rb b/tooling/lib/tooling/find_changes.rb
index f6fdf042c151e..8a92242e167b6 100755
--- a/tooling/lib/tooling/find_changes.rb
+++ b/tooling/lib/tooling/find_changes.rb
@@ -15,7 +15,8 @@ def initialize(
changed_files_pathname: nil,
predictive_tests_pathname: nil,
frontend_fixtures_mapping_pathname: nil,
- file_filter: ->(_) { true }
+ file_filter: ->(_) { true },
+ only_new_paths: false
)
raise ArgumentError, ':from can only be :api or :changed_files' unless
@@ -30,6 +31,7 @@ def initialize(
@frontend_fixtures_mapping_pathname = frontend_fixtures_mapping_pathname
@from = from
@file_filter = file_filter
+ @api_path_attributes = only_new_paths ? %w[new_path] : %w[new_path old_path]
end
def execute
@@ -53,7 +55,7 @@ def only_allowed_files_changed
attr_reader :gitlab_token, :gitlab_endpoint, :mr_project_path,
:mr_iid, :changed_files_pathname, :predictive_tests_pathname,
- :frontend_fixtures_mapping_pathname, :file_filter
+ :frontend_fixtures_mapping_pathname, :file_filter, :api_path_attributes
def gitlab
@gitlab ||= begin
@@ -86,7 +88,7 @@ def file_changes
case @from
when :api
mr_changes.changes.select(&file_filter).flat_map do |change|
- change.to_h.values_at('old_path', 'new_path')
+ change.to_h.values_at(*api_path_attributes)
end.uniq
else
read_array_from_file(changed_files_pathname)
--
GitLab