Add latest changes from gitlab-org/gitlab@master

3 files changed, 151 insertions, 67 deletions

diff --git a/spec/tooling/lib/tooling/find_changes_spec.rb b/spec/tooling/lib/tooling/find_changes_spec.rb
index 5932eb5e919..37e590858cf 100644
--- a/spec/tooling/lib/tooling/find_changes_spec.rb
+++ b/spec/tooling/lib/tooling/find_changes_spec.rb
@@ -11,12 +11,17 @@ RSpec.describe Tooling::FindChanges, feature_category: :tooling do
   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,
+      from: from)
   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(:from)                               { :api }
   let(:gitlab_client)                      { double('GitLab') } # rubocop:disable RSpec/VerifiedDoubles
 
   around do |example|
@@ -45,24 +50,50 @@ RSpec.describe Tooling::FindChanges, feature_category: :tooling do
       'CI_MERGE_REQUEST_PROJECT_PATH' => 'dummy-project',
       'PROJECT_TOKEN_FOR_CI_SCRIPTS_API_USAGE' => 'dummy-token'
     )
+  end
+
+  describe '#initialize' do
+    context 'when fetching changes from unknown' do
+      let(:from) { :unknown }
 
-    allow(instance).to receive(:gitlab).and_return(gitlab_client)
+      it 'raises an ArgumentError' do
+        expect { instance }.to raise_error(
+          ArgumentError, ":from can only be :api or :changed_files"
+        )
+      end
+    end
   end
 
   describe '#execute' do
     subject { instance.execute }
 
+    before do
+      allow(instance).to receive(:gitlab).and_return(gitlab_client)
+    end
+
     context 'when there is no changed files file' do
       let(:changed_files_pathname) { nil }
 
       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 fetching changes from API' do
+      let(:from) { :api }
+
+      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 fetching changes from changed files' do
+      let(:from) { :changed_files }
+
       it 'does not call GitLab API to retrieve the MR diff' do
         expect(gitlab_client).not_to receive(:merge_request_changes)
 
@@ -154,68 +185,96 @@ RSpec.describe Tooling::FindChanges, feature_category: :tooling do
   describe '#only_js_files_changed' do
     subject { instance.only_js_files_changed }
 
-    let(:mr_changes_array) { [] }
+    context 'when fetching changes from changed files' do
+      let(:from) { :changed_files }
 
-    before do
-      # The class from the GitLab gem isn't public, so we cannot use verified doubles for it.
-      #
-      # rubocop:disable RSpec/VerifiedDoubles
-      allow(gitlab_client).to receive(:merge_request_changes)
-        .with('dummy-project', '1234')
-        .and_return(double(changes: mr_changes_array))
-      # rubocop:enable RSpec/VerifiedDoubles
-    end
+      before do
+        File.write(changed_files_pathname, changed_files_file_content)
+      end
 
-    context 'when a file is passed as an argument' do
-      let(:changed_files_pathname) { 'does-not-exist.out' }
+      context 'when changed files contain only *.js changes' do
+        let(:changed_files_file_content) { 'a.js b.js' }
 
-      it 'calls GitLab API' do
-        expect(gitlab_client).to receive(:merge_request_changes)
-        .with('dummy-project', '1234')
+        it 'returns true' do
+          expect(subject).to be true
+        end
+      end
 
-        subject
+      context 'when changed files contain not only *.js changes' do
+        let(:changed_files_file_content) { 'a.js b.rb' }
+
+        it 'returns false' do
+          expect(subject).to be false
+        end
       end
     end
 
-    context 'when there are no file changes' do
+    context 'when fetching changes from API' do
+      let(:from) { :api }
+
       let(:mr_changes_array) { [] }
 
-      it 'returns false' do
-        expect(subject).to be false
+      before do
+        allow(instance).to receive(:gitlab).and_return(gitlab_client)
+
+        # The class from the GitLab gem isn't public, so we cannot use verified doubles for it.
+        #
+        # rubocop:disable RSpec/VerifiedDoubles
+        allow(gitlab_client).to receive(:merge_request_changes)
+          .with('dummy-project', '1234')
+          .and_return(double(changes: mr_changes_array))
+        # rubocop:enable RSpec/VerifiedDoubles
       end
-    end
 
-    context 'when there are changes to files other than JS files' do
-      let(:mr_changes_array) do
-        [
-          {
-            "new_path" => "scripts/gitlab_component_helpers.sh",
-            "old_path" => "scripts/gitlab_component_helpers.sh"
-          },
-          {
-            "new_path" => "scripts/test.js",
-            "old_path" => "scripts/test.js"
-          }
-        ]
+      context 'when a file is passed as an argument' do
+        it 'calls GitLab API' do
+          expect(gitlab_client).to receive(:merge_request_changes)
+          .with('dummy-project', '1234')
+
+          subject
+        end
       end
 
-      it 'returns false' do
-        expect(subject).to be false
+      context 'when there are no file changes' do
+        let(:mr_changes_array) { [] }
+
+        it 'returns false' do
+          expect(subject).to be false
+        end
       end
-    end
 
-    context 'when there are changes only to JS files' do
-      let(:mr_changes_array) do
-        [
-          {
-            "new_path" => "scripts/test.js",
-            "old_path" => "scripts/test.js"
-          }
-        ]
+      context 'when there are changes to files other than JS files' do
+        let(:mr_changes_array) do
+          [
+            {
+              "new_path" => "scripts/gitlab_component_helpers.sh",
+              "old_path" => "scripts/gitlab_component_helpers.sh"
+            },
+            {
+              "new_path" => "scripts/test.js",
+              "old_path" => "scripts/test.js"
+            }
+          ]
+        end
+
+        it 'returns false' do
+          expect(subject).to be false
+        end
       end
 
-      it 'returns true' do
-        expect(subject).to be true
+      context 'when there are changes only to JS files' do
+        let(:mr_changes_array) do
+          [
+            {
+              "new_path" => "scripts/test.js",
+              "old_path" => "scripts/test.js"
+            }
+          ]
+        end
+
+        it 'returns true' do
+          expect(subject).to be true
+        end
       end
     end
   end
diff --git a/spec/tooling/lib/tooling/helpers/file_handler_spec.rb b/spec/tooling/lib/tooling/helpers/file_handler_spec.rb
index d6f68baeb90..b78f0a3bb6b 100644
--- a/spec/tooling/lib/tooling/helpers/file_handler_spec.rb
+++ b/spec/tooling/lib/tooling/helpers/file_handler_spec.rb
@@ -67,10 +67,10 @@ RSpec.describe Tooling::Helpers::FileHandler, feature_category: :tooling do
   end
 
   describe '#write_array_to_file' do
-    let(:content_array)  { %w[new_entry] }
-    let(:overwrite_flag) { false }
+    let(:content_array) { %w[new_entry] }
+    let(:append_flag) { true }
 
-    subject { instance.write_array_to_file(output_file_path, content_array, overwrite: overwrite_flag) }
+    subject { instance.write_array_to_file(output_file_path, content_array, append: append_flag) }
 
     context 'when the output file does not exist' do
       let(:non_existing_output_file) { 'tmp/another_file.out' }
@@ -113,8 +113,8 @@ RSpec.describe Tooling::Helpers::FileHandler, feature_category: :tooling do
           .to((initial_content.split(' ') + content_array).join(' '))
       end
 
-      context 'when the overwrite flag is set to true' do
-        let(:overwrite_flag) { true }
+      context 'when the append flag is set to false' do
+        let(:append_flag) { false }
 
         it 'overwrites the previous content' do
           expect { subject }.to change { File.read(output_file_path) }
diff --git a/spec/tooling/lib/tooling/predictive_tests_spec.rb b/spec/tooling/lib/tooling/predictive_tests_spec.rb
index 79554037c48..b82364fe6f6 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'
 
@@ -9,11 +10,14 @@ RSpec.describe Tooling::PredictiveTests, feature_category: :tooling do
 
   let(:instance)                       { described_class.new }
   let(:matching_tests_initial_content) { 'initial_matching_spec' }
+  let(:fixtures_mapping_content)       { '{}' }
 
-  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 +26,15 @@ RSpec.describe Tooling::PredictiveTests, feature_category: :tooling do
     # 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 +48,7 @@ RSpec.describe Tooling::PredictiveTests, feature_category: :tooling do
 
   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,
@@ -50,7 +59,9 @@ RSpec.describe Tooling::PredictiveTests, feature_category: :tooling do
 
     # 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
+    File.write(fixtures_mapping.path, fixtures_mapping_content)
+
+    allow(Gitlab).to receive(:configure)
   end
 
   describe '#execute' do
@@ -73,14 +84,18 @@ RSpec.describe Tooling::PredictiveTests, feature_category: :tooling do
 
     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) }
@@ -88,17 +103,27 @@ RSpec.describe Tooling::PredictiveTests, feature_category: :tooling do
         end
       end
 
-      context 'when some files were changed' do
-        let(:changed_files_content) { 'tooling/lib/tooling/predictive_tests.rb' }
+      context 'when some files used for frontend fixtures were changed' do
+        let(:changed_files_content) { 'app/models/todo.rb' }
+        let(:changed_files_matching_test) { 'spec/models/todo_spec.rb' }
+        let(:matching_frontend_fixture) { 'tmp/tests/frontend/fixtures-ee/todos/todos.html' }
+        let(:fixtures_mapping_content) do
+          JSON.dump(changed_files_matching_test => [matching_frontend_fixture]) # rubocop:disable Gitlab/Json
+        end
+
+        it 'writes to RSPEC_CHANGED_FILES_PATH with API contents and appends with matching fixtures' do
+          subject
+
+          expect(File.read(changed_files_path)).to eq("#{changed_files_content} #{matching_frontend_fixture}")
+        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")
+            .to("#{matching_tests_initial_content} #{changed_files_matching_test}")
         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) }