diff --git a/spec/tooling/lib/tooling/find_changes_spec.rb b/spec/tooling/lib/tooling/find_changes_spec.rb
index 5932eb5e9190238d7393d40fbf0182eb2c235c1d..0e34659e42aac961604ca64c22bd79ad2043accb 100644
--- a/spec/tooling/lib/tooling/find_changes_spec.rb
+++ b/spec/tooling/lib/tooling/find_changes_spec.rb
@@ -11,12 +11,17 @@
attr_accessor :changed_files_file, :predictive_tests_file, :frontend_fixtures_mapping_file
let(:instance) do
- described_class.new(changed_files_pathname, predictive_tests_pathname, frontend_fixtures_mapping_pathname)
+ described_class.new(
+ changed_files_pathname: changed_files_pathname,
+ predictive_tests_pathname: predictive_tests_pathname,
+ frontend_fixtures_mapping_pathname: frontend_fixtures_mapping_pathname,
+ append: append)
end
let(:changed_files_pathname) { changed_files_file.path }
let(:predictive_tests_pathname) { predictive_tests_file.path }
let(:frontend_fixtures_mapping_pathname) { frontend_fixtures_mapping_file.path }
+ let(:append) { false }
let(:gitlab_client) { double('GitLab') } # rubocop:disable RSpec/VerifiedDoubles
around do |example|
@@ -57,12 +62,24 @@
it 'raises an ArgumentError' do
expect { subject }.to raise_error(
- ArgumentError, "A path to the changed files file must be given as first argument."
+ ArgumentError, "A path to the changed files file must be given as :changed_files_pathname"
)
end
end
- context 'when an changed files file is provided' do
+ context 'when not in append mode' do
+ let(:append) { false }
+
+ it 'calls GitLab API to retrieve the MR diff' do
+ expect(gitlab_client).to receive_message_chain(:merge_request_changes, :changes).and_return([])
+
+ subject
+ end
+ end
+
+ context 'when in append mode' do
+ let(:append) { true }
+
it 'does not call GitLab API to retrieve the MR diff' do
expect(gitlab_client).not_to receive(:merge_request_changes)
diff --git a/spec/tooling/lib/tooling/predictive_tests_spec.rb b/spec/tooling/lib/tooling/predictive_tests_spec.rb
index 79554037c4862eaf19fd0423b29b02a86d3e22f9..f5ff6d05f0a2e59c62eb740ea15f9bad4fc9c10e 100644
--- a/spec/tooling/lib/tooling/predictive_tests_spec.rb
+++ b/spec/tooling/lib/tooling/predictive_tests_spec.rb
@@ -1,6 +1,7 @@
# frozen_string_literal: true
require 'tempfile'
+require 'fileutils'
require_relative '../../../../tooling/lib/tooling/predictive_tests'
require_relative '../../../support/helpers/stub_env'
@@ -10,10 +11,12 @@
let(:instance) { described_class.new }
let(:matching_tests_initial_content) { 'initial_matching_spec' }
- attr_accessor :changed_files, :fixtures_mapping, :matching_js_files, :matching_tests, :views_with_partials
+ attr_accessor :changed_files, :changed_files_path, :fixtures_mapping,
+ :matching_js_files, :matching_tests, :views_with_partials
around do |example|
self.changed_files = Tempfile.new('test-folder/changed_files.txt')
+ self.changed_files_path = changed_files.path
self.fixtures_mapping = Tempfile.new('test-folder/fixtures_mapping.txt')
self.matching_js_files = Tempfile.new('test-folder/matching_js_files.txt')
self.matching_tests = Tempfile.new('test-folder/matching_tests.txt')
@@ -22,10 +25,15 @@
# See https://ruby-doc.org/stdlib-1.9.3/libdoc/tempfile/rdoc/
# Tempfile.html#class-Tempfile-label-Explicit+close
begin
- example.run
- ensure
+ # In practice, we let PredictiveTests create the file, and we just
+ # use its file name.
changed_files.close
changed_files.unlink
+
+ example.run
+ ensure
+ # Since example.run can create the file again, let's remove it again
+ FileUtils.rm_f(changed_files_path)
fixtures_mapping.close
fixtures_mapping.unlink
matching_js_files.close
@@ -39,7 +47,7 @@
before do
stub_env(
- 'RSPEC_CHANGED_FILES_PATH' => changed_files.path,
+ 'RSPEC_CHANGED_FILES_PATH' => changed_files_path,
'RSPEC_MATCHING_TESTS_PATH' => matching_tests.path,
'RSPEC_VIEWS_INCLUDING_PARTIALS_PATH' => views_with_partials.path,
'FRONTEND_FIXTURES_MAPPING_PATH' => fixtures_mapping.path,
@@ -51,6 +59,8 @@
# We write some data to later on verify that we only append to this file.
File.write(matching_tests.path, matching_tests_initial_content)
File.write(fixtures_mapping.path, '{}') # We write valid JSON, so that the file can be processed
+
+ allow(Gitlab).to receive(:configure)
end
describe '#execute' do
@@ -73,14 +83,18 @@
context 'when all ENV variables are provided' do
before do
- File.write(changed_files, changed_files_content)
+ change = double('GitLab::Change') # rubocop:disable RSpec/VerifiedDoubles
+ allow(change).to receive_message_chain(:to_h, :values_at)
+ .and_return([changed_files_content, changed_files_content])
+
+ allow(Gitlab).to receive_message_chain(:merge_request_changes, :changes)
+ .and_return([change])
end
context 'when no files were changed' do
let(:changed_files_content) { '' }
- it 'does not change any files' do
- expect { subject }.not_to change { File.read(changed_files.path) }
+ it 'does not change files other than RSPEC_CHANGED_FILES_PATH' do
expect { subject }.not_to change { File.read(matching_tests.path) }
expect { subject }.not_to change { File.read(views_with_partials.path) }
expect { subject }.not_to change { File.read(fixtures_mapping.path) }
@@ -91,14 +105,19 @@
context 'when some files were changed' do
let(:changed_files_content) { 'tooling/lib/tooling/predictive_tests.rb' }
+ it 'writes to RSPEC_CHANGED_FILES_PATH with API contents' do
+ subject
+
+ expect(File.read(changed_files_path)).to eq(changed_files_content)
+ end
+
it 'appends the spec file to RSPEC_MATCHING_TESTS_PATH' do
expect { subject }.to change { File.read(matching_tests.path) }
.from(matching_tests_initial_content)
.to("#{matching_tests_initial_content} spec/tooling/lib/tooling/predictive_tests_spec.rb")
end
- it 'does not change files other than RSPEC_MATCHING_TESTS_PATH' do
- expect { subject }.not_to change { File.read(changed_files.path) }
+ it 'does not change files other than RSPEC_CHANGED_FILES_PATH nor RSPEC_MATCHING_TESTS_PATH' do
expect { subject }.not_to change { File.read(views_with_partials.path) }
expect { subject }.not_to change { File.read(fixtures_mapping.path) }
expect { subject }.not_to change { File.read(matching_js_files.path) }
diff --git a/tooling/lib/tooling/find_changes.rb b/tooling/lib/tooling/find_changes.rb
index b4439bd4abe05e8f179aa98bfb83e1a529c8d92d..2544bcbbed4758cec3edfda16369c20d8ec21b35 100755
--- a/tooling/lib/tooling/find_changes.rb
+++ b/tooling/lib/tooling/find_changes.rb
@@ -9,7 +9,10 @@ class FindChanges
include Helpers::FileHandler
def initialize(
- changed_files_pathname = nil, predictive_tests_pathname = nil, frontend_fixtures_mapping_pathname = nil
+ changed_files_pathname: nil,
+ predictive_tests_pathname: nil,
+ frontend_fixtures_mapping_pathname: nil,
+ append: false
)
@gitlab_token = ENV['PROJECT_TOKEN_FOR_CI_SCRIPTS_API_USAGE'] || ''
@gitlab_endpoint = ENV['CI_API_V4_URL']
@@ -18,11 +21,12 @@ def initialize(
@changed_files_pathname = changed_files_pathname
@predictive_tests_pathname = predictive_tests_pathname
@frontend_fixtures_mapping_pathname = frontend_fixtures_mapping_pathname
+ @append = append
end
def execute
if changed_files_pathname.nil?
- raise ArgumentError, "A path to the changed files file must be given as first argument."
+ raise ArgumentError, "A path to the changed files file must be given as :changed_files_pathname"
end
add_frontend_fixture_files!
@@ -30,8 +34,6 @@ def execute
end
def only_js_files_changed
- @changed_files_pathname = nil # We ensure that we'll get the diff from the MR directly, not from a file.
-
file_changes.any? && file_changes.all? { |file| file.end_with?('.js') }
end
@@ -68,7 +70,7 @@ def add_frontend_fixture_files!
def file_changes
@file_changes ||=
- if changed_files_pathname && File.exist?(changed_files_pathname)
+ if @append && File.exist?(changed_files_pathname)
read_array_from_file(changed_files_pathname)
else
mr_changes.changes.flat_map do |change|
diff --git a/tooling/lib/tooling/predictive_tests.rb b/tooling/lib/tooling/predictive_tests.rb
index 2691e1ba56d6054b5e73f16e0ad027b57c4ece81..56b108a2ddf900b0d11d3b9f9895247a261ab34d 100644
--- a/tooling/lib/tooling/predictive_tests.rb
+++ b/tooling/lib/tooling/predictive_tests.rb
@@ -32,7 +32,7 @@ def initialize
end
def execute
- Tooling::FindChanges.new(rspec_changed_files_path).execute
+ Tooling::FindChanges.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
@@ -41,7 +41,11 @@ def execute
Tooling::Mappings::GraphqlBaseTypeMappings.new(rspec_changed_files_path, rspec_matching_tests_path).execute
Tooling::Mappings::ViewToSystemSpecsMappings.new(rspec_changed_files_path, rspec_matching_tests_path).execute
Tooling::FindChanges.new(
- rspec_changed_files_path, rspec_matching_tests_path, frontend_fixtures_mapping_path).execute
+ append: true,
+ changed_files_pathname: rspec_changed_files_path,
+ predictive_tests_pathname: rspec_matching_tests_path,
+ frontend_fixtures_mapping_pathname: frontend_fixtures_mapping_path
+ ).execute
Tooling::Mappings::ViewToJsMappings.new(rspec_changed_files_path, rspec_matching_js_files_path).execute
end