Merge branch 'sh-simplify-system-hook-testing' into 'master'

Simplify system hook testing and guarantee test will fire

Closes #37067

See merge request !13858

4 files changed, 63 insertions, 61 deletions

diff --git a/app/services/test_hooks/system_service.rb b/app/services/test_hooks/system_service.rb
index 76c3c19bd74..67552edefc9 100644
--- a/app/services/test_hooks/system_service.rb
+++ b/app/services/test_hooks/system_service.rb
@@ -2,47 +2,16 @@ module TestHooks
   class SystemService < TestHooks::BaseService
     private
 
-    def project
-      @project ||= begin
-        project = Project.first
-
-        throw(:validation_error, 'Ensure that at least one project exists.') unless project
-
-        project
-      end
-    end
-
     def push_events_data
-      if project.empty_repo?
-        throw(:validation_error, "Ensure project \"#{project.human_name}\" has commits.")
-      end
-
-      Gitlab::DataBuilder::Push.build_sample(project, current_user)
+      Gitlab::DataBuilder::Push.sample_data
     end
 
     def tag_push_events_data
-      if project.repository.tags.empty?
-        throw(:validation_error, "Ensure project \"#{project.human_name}\" has tags.")
-      end
-
-      Gitlab::DataBuilder::Push.build_sample(project, current_user)
+      Gitlab::DataBuilder::Push.sample_data
     end
 
     def repository_update_events_data
-      commit = project.commit
-      ref = "#{Gitlab::Git::BRANCH_REF_PREFIX}#{project.default_branch}"
-
-      unless commit
-        throw(:validation_error, "Ensure project \"#{project.human_name}\" has commits.")
-      end
-
-      change = Gitlab::DataBuilder::Repository.single_change(
-        commit.parent_id || Gitlab::Git::BLANK_SHA,
-        commit.id,
-        ref
-      )
-
-      Gitlab::DataBuilder::Repository.update(project, current_user, [change], [ref])
+      Gitlab::DataBuilder::Repository.sample_data
     end
   end
 end
diff --git a/lib/gitlab/data_builder/push.rb b/lib/gitlab/data_builder/push.rb
index 5c5f507d44d..4ab5b3455a5 100644
--- a/lib/gitlab/data_builder/push.rb
+++ b/lib/gitlab/data_builder/push.rb
@@ -3,6 +3,35 @@ module Gitlab
     module Push
       extend self
 
+      SAMPLE_DATA =
+        {
+          object_kind: "push",
+          event_name: "push",
+          before: "95790bf891e76fee5e1747ab589903a6a1f80f22",
+          after: "da1560886d4f094c3e6c9ef40349f7d38b5d27d7",
+          ref: "refs/heads/master",
+          checkout_sha: "da1560886d4f094c3e6c9ef40349f7d38b5d27d7",
+          message: "Hello World",
+          user_id: 4,
+          user_name: "John Smith",
+          user_email: "john@example.com",
+          user_avatar: "https://s.gravatar.com/avatar/d4c74594d841139328695756648b6bd6?s=8://s.gravatar.com/avatar/d4c74594d841139328695756648b6bd6?s=80",
+          project_id: 15,
+          commits: [
+            {
+              id: "c5feabde2d8cd023215af4d2ceeb7a64839fc428",
+              message: "Add simple search to projects in public area",
+              timestamp: "2013-05-13T18:18:08+00:00",
+              url: "https://test.example.com/gitlab/gitlabhq/commit/c5feabde2d8cd023215af4d2ceeb7a64839fc428",
+              author: {
+                name: "Test User",
+                email: "test@example.com"
+              }
+            }
+          ],
+          total_commits_count: 1
+        }.freeze
+
       # Produce a hash of post-receive data
       #
       # data = {
@@ -74,6 +103,10 @@ module Gitlab
         build(project, user, commits.last&.id, commits.first&.id, ref, commits)
       end
 
+      def sample_data
+        SAMPLE_DATA
+      end
+
       private
 
       def checkout_sha(repository, newrev, ref)
diff --git a/lib/gitlab/data_builder/repository.rb b/lib/gitlab/data_builder/repository.rb
index b42dc052949..c9c13ec6487 100644
--- a/lib/gitlab/data_builder/repository.rb
+++ b/lib/gitlab/data_builder/repository.rb
@@ -3,6 +3,23 @@ module Gitlab
     module Repository
       extend self
 
+      SAMPLE_DATA = {
+        event_name: 'repository_update',
+        user_id: 10,
+        user_name: 'john.doe',
+        user_email: 'test@example.com',
+        user_avatar: 'http://example.com/avatar/user.png',
+        project_id: 40,
+        changes: [
+          {
+            before: "8205ea8d81ce0c6b90fbe8280d118cc9fdad6130",
+            after: "4045ea7a3df38697b3730a20fb73c8bed8a3e69e",
+            ref: "refs/heads/master"
+          }
+        ],
+        "refs": ["refs/heads/master"]
+      }.freeze
+
       # Produce a hash of post-receive data
       def update(project, user, changes, refs)
         {
@@ -30,6 +47,10 @@ module Gitlab
           ref: ref
         }
       end
+
+      def sample_data
+        SAMPLE_DATA
+      end
     end
   end
 end
diff --git a/spec/services/test_hooks/system_service_spec.rb b/spec/services/test_hooks/system_service_spec.rb
index 00d89924766..a15708bf82f 100644
--- a/spec/services/test_hooks/system_service_spec.rb
+++ b/spec/services/test_hooks/system_service_spec.rb
@@ -7,7 +7,6 @@ describe TestHooks::SystemService do
     let(:project) { create(:project, :repository) }
     let(:hook)    { create(:system_hook) }
     let(:service) { described_class.new(hook, current_user, trigger) }
-    let(:sample_data) { { data: 'sample' }}
     let(:success_result) { { status: :success, http_status: 200, message: 'ok' } }
 
     before do
@@ -26,18 +25,11 @@ describe TestHooks::SystemService do
     context 'push_events' do
       let(:trigger) { 'push_events' }
 
-      it 'returns error message if not enough data' do
-        allow(project).to receive(:empty_repo?).and_return(true)
-
-        expect(hook).not_to receive(:execute)
-        expect(service.execute).to include({ status: :error, message: "Ensure project \"#{project.human_name}\" has commits." })
-      end
-
       it 'executes hook' do
         allow(project).to receive(:empty_repo?).and_return(false)
-        allow(Gitlab::DataBuilder::Push).to receive(:build_sample).and_return(sample_data)
+        expect(Gitlab::DataBuilder::Push).to receive(:sample_data).and_call_original
 
-        expect(hook).to receive(:execute).with(sample_data, trigger).and_return(success_result)
+        expect(hook).to receive(:execute).with(Gitlab::DataBuilder::Push::SAMPLE_DATA, trigger).and_return(success_result)
         expect(service.execute).to include(success_result)
       end
     end
@@ -45,18 +37,11 @@ describe TestHooks::SystemService do
     context 'tag_push_events' do
       let(:trigger) { 'tag_push_events' }
 
-      it 'returns error message if not enough data' do
-        allow(project.repository).to receive(:tags).and_return([])
-
-        expect(hook).not_to receive(:execute)
-        expect(service.execute).to include({ status: :error, message: "Ensure project \"#{project.human_name}\" has tags." })
-      end
-
       it 'executes hook' do
         allow(project.repository).to receive(:tags).and_return(['tag'])
-        allow(Gitlab::DataBuilder::Push).to receive(:build_sample).and_return(sample_data)
+        expect(Gitlab::DataBuilder::Push).to receive(:sample_data).and_call_original
 
-        expect(hook).to receive(:execute).with(sample_data, trigger).and_return(success_result)
+        expect(hook).to receive(:execute).with(Gitlab::DataBuilder::Push::SAMPLE_DATA, trigger).and_return(success_result)
         expect(service.execute).to include(success_result)
       end
     end
@@ -64,17 +49,11 @@ describe TestHooks::SystemService do
     context 'repository_update_events' do
       let(:trigger) { 'repository_update_events' }
 
-      it 'returns error message if not enough data' do
-        allow(project).to receive(:commit).and_return(nil)
-        expect(hook).not_to receive(:execute)
-        expect(service.execute).to include({ status: :error, message: "Ensure project \"#{project.human_name}\" has commits." })
-      end
-
       it 'executes hook' do
         allow(project).to receive(:empty_repo?).and_return(false)
-        allow(Gitlab::DataBuilder::Repository).to receive(:update).and_return(sample_data)
+        expect(Gitlab::DataBuilder::Repository).to receive(:sample_data).and_call_original
 
-        expect(hook).to receive(:execute).with(sample_data, trigger).and_return(success_result)
+        expect(hook).to receive(:execute).with(Gitlab::DataBuilder::Repository::SAMPLE_DATA, trigger).and_return(success_result)
         expect(service.execute).to include(success_result)
       end
     end