Make CommitsRunner create and run from webhook

Author: bmarkons

app/services/benchmark_pool.rb

@@ -13,5 +13,4 @@ def self.enqueue(initiator_type, initiator, repo_name, options = {})
       raise ArgumentError, "unknown repo: #{repo_name}"
     end
   end
-
 end

app/services/commits_runner.rb

@@ -1,12 +1,45 @@
-class CommitsRunner
-  def self.run(commits, pattern = '')
-    commits.select { |commit| valid?(commit) }
-           .each { |commit| create_and_run(commit, pattern) }
-           .count
+module CommitsRunner
+  def self.run(trigger_source, commits, repo, pattern = '')
+    formatted_commits =
+      if trigger_source == :webhook
+        format_webhook(commits, repo)
+      elsif trigger_source == :api
+        format_api(commits, repo)
+      end
+
+    formatted_commits.select { |commit| valid?(commit) }
+                     .each { |commit| create_and_run(commit, pattern) }
+                     .count
   end
 
   private
 
+  def self.format_api(commits, repo)
+    commits.map do |commit|
+      {
+        sha: commit['sha'],
+        message: commit['commit']['message'],
+        repo: repo,
+        url: commit['html_url'],
+        created_at: commit['commit']['author']['date'],
+        author_name: commit['commit']['author']['name']
+      }
+    end
+  end
+
+  def self.format_webhook(commits, repo)
+    commits.map do |commit|
+      {
+        sha: commit['id'],
+        message: commit['message'],
+        repo: repo,
+        url: commit['url'],
+        created_at: commit['timestamp'],
+        author_name: commit['author']['name']
+      }
+    end
+  end
+
   def self.valid?(commit)
     !Commit.merge_or_skip_ci?(commit[:message]) && Commit.valid_author?(commit[:author_name])
   end
@@ -20,8 +53,9 @@ def self.create_and_run(commit, pattern)
     end
 
     BenchmarkPool.enqueue(
-      commit[:repo].name,
+      :commit,
       commit[:sha],
+      commit[:repo].name,
       include_patterns: pattern
     )
   end

app/services/github_event_handler.rb

@@ -25,19 +25,14 @@ def branch_from_payload
     @payload['ref'].split('/')[2]
   end
 
-  # Grabs the commits hash and starts job to run benchmarks on remote server.
   def process_push
-    repo = first_or_create_repo(@payload['repository'])
+    repo = find_or_create_repo(@payload['repository'])
     commits = @payload['commits'] || [@payload['head_commit']]
 
-    commits.each do |commit|
-      if create_commit(commit, repo.id)
-        BenchmarkPool.enqueue(repo.name, commit['id'])
-      end
-    end
+    CommitsRunner.run(:webhook, commits, repo)
   end
 
-  def first_or_create_repo(repository)
+  def find_or_create_repo(repository)
     organization_name, repo_name = parse_full_name(repository['full_name'])
     repository_url = repository['html_url']
 
@@ -65,19 +60,4 @@ def parse_full_name(full_name)
     full_name =~ /\A(\w+)\/(\w+)/
     [$1, $2]
   end
-
-  def create_commit(commit, repo_id)
-    if valid_commit?(commit)
-      Commit.find_or_create_by(sha1: commit['id']) do |c|
-        c.url = commit['url']
-        c.message = commit['message']
-        c.repo_id = repo_id
-        c.created_at = commit['timestamp']
-      end
-    end
-  end
-
-  def valid_commit?(commit)
-    !Commit.merge_or_skip_ci?(commit['message']) && Commit.valid_author?(commit['author']['name'])
-  end
 end

app/services/manual_runner.rb

@@ -25,21 +25,7 @@ def run_paginated(commits_count, page: 1, pattern: '')
   end
 
   def run_commits(page: 1, per_page: 100, pattern: '')
-    fetched_commits = @octokit.commits("#{@repo.organization.name}/#{@repo.name}", per_page: per_page, page: page)
-    formatted_commits = format_commits(fetched_commits)
-    CommitsRunner.run(formatted_commits, pattern)
-  end
-
-  def format_commits(commits)
-    commits.map do |commit|
-      {
-        sha: commit['sha'],
-        message: commit['commit']['message'],
-        repo: @repo,
-        url: commit['html_url'],
-        created_at: commit['commit']['author']['date'],
-        author_name: commit['commit']['author']['name']
-      }
-    end
+    commits = @octokit.commits("#{@repo.organization.name}/#{@repo.name}", per_page: per_page, page: page)
+    CommitsRunner.run(:api, commits, @repo, pattern)
   end
 end

test/integration/github_event_handler_test.rb

@@ -2,7 +2,7 @@
 
 class GithubEventHandlerTest < ActionDispatch::IntegrationTest
   test '#handle for single commits pushed' do
-    BenchmarkPool.expects(:enqueue).with('ruby', '12345')
+    BenchmarkPool.expects(:enqueue).with(:commit, '12345', 'ruby', include_patterns: '')
 
     post_to_handler('ref' => 'refs/heads/master',
       'head_commit' => {
@@ -31,8 +31,8 @@ class GithubEventHandlerTest < ActionDispatch::IntegrationTest
   end
 
   test '#handle for multiple commits pushed' do
-    BenchmarkPool.expects(:enqueue).with('ruby', '12345')
-    BenchmarkPool.expects(:enqueue).with('ruby', '12346')
+    BenchmarkPool.expects(:enqueue).with(:commit, '12345', 'ruby', include_patterns: '')
+    BenchmarkPool.expects(:enqueue).with(:commit, '12346', 'ruby', include_patterns: '')
 
     post_to_handler('ref' => 'refs/heads/master',
       'commits' =>

test/services/benchmark_pool_test.rb

@@ -12,7 +12,7 @@ class BenchmarkPoolTest < ActiveSupport::TestCase
       BenchmarkPool.enqueue(:release, 'abc', 'ruby')
     end
 
-    assert_enqueued_jobs 2 
+    assert_enqueued_jobs 2
   end
 
   test '#enqueue for rails' do
@@ -24,14 +24,14 @@ class BenchmarkPoolTest < ActiveSupport::TestCase
       BenchmarkPool.enqueue(:release, 'abc', 'rails')
     end
 
-    assert_enqueued_jobs 2 
+    assert_enqueued_jobs 2
   end
 
   test '#enqueue for sequel' do
     assert_enqueued_with(job: RemoteServerJob) do
       BenchmarkPool.enqueue(:commit, 'abc', 'sequel')
     end
-    
+
     assert_enqueued_with(job: RemoteServerJob) do
       BenchmarkPool.enqueue(:release, 'abc', 'sequel')
     end
@@ -47,7 +47,7 @@ class BenchmarkPoolTest < ActiveSupport::TestCase
     assert_enqueued_with(job: RemoteServerJob) do
       BenchmarkPool.enqueue(:release, 'abc', 'ruby-pg')
     end
-    
-    assert_enqueued_jobs 2 
+
+    assert_enqueued_jobs 2
   end
 end

test/services/commits_runner_test.rb

@@ -7,34 +7,56 @@ class CommitsRunnerTest < ActiveSupport::TestCase
     @repo = create(:repo, name: 'rails')
   end
 
-  test '#run' do
+  test '#run commits triggered by webhook' do
     commits = [
       {
-        sha: '12345',
-        message: 'My beautiful commit message',
-        repo: @repo,
-        author_name: 'bmarkons',
-        url: 'https://github.com/commit',
-        created_at: 12345
+        'id' => '12345',
+        'message' => 'My beautiful commit message',
+        'author' => {
+          'name' => 'bmarkons'
+        },
+        'url' => 'https://github.com/commit',
+        'timestamp' => 12345
       }
     ]
 
-    CommitsRunner.run(commits)
+    CommitsRunner.run(:webhook, commits, @repo)
 
     expect_created(commits)
     assert_enqueued_jobs(commits.count)
   end
 
-  def expect_created(commits_hashes)
-    commits_hashes.each do |hash|
-      commit = Commit.find_by!(sha1: hash[:sha])
+  test '#run commits triggered manually' do
+    commits = [
+      {
+        'sha' => '12345',
+        'commit' => {
+          'message' => 'My beautiful commit message',
+          'author' => {
+            'date' => 12345,
+            'name' => 'bmarkons'
+          }
+        },
+        'html_url' => 'https://github.com/commit'
+      }
+    ]
+
+    CommitsRunner.run(:api, commits, @repo)
+
+    expect_created(commits)
+    assert_enqueued_jobs(commits.count)
+  end
+
+  def expect_created(commit_hashes)
+    commit_hashes.each do |commit_hash|
+      commit = Commit.find_by!(sha1: commit_hash['sha'] || commit_hash['id'])
 
-      assert_equal hash[:sha], commit.sha1
-      assert_equal hash[:message], commit.message
-      assert_equal hash[:repo].id, commit.repo.id
-      assert_equal hash[:repo].name, commit.repo.name
-      assert_equal hash[:url], commit.url
-      refute_equal hash[:created_at], commit.created_at
+      assert_equal commit_hash['id'] || commit_hash['sha'], commit.sha1
+      assert_equal commit_hash['message'] || commit_hash['commit']['message'], commit.message
+      assert_equal @repo.id, commit.repo.id
+      assert_equal @repo.name, commit.repo.name
+      assert_equal commit_hash['url'] || commit_hash['html_url'], commit.url
+      refute_equal commit_hash['timestamp'] || commit_hash['commit']['author']['date'], commit.created_at
     end
   end
 end

test/services/manual_runner_test.rb

@@ -17,18 +17,7 @@ class ManualRunnerTest < ActiveSupport::TestCase
     organization = create(:organization, name: 'jeremyevans')
     repo = create(:repo, name: 'sequel', organization: organization)
 
-    CommitsRunner.expects(:run).times(2).returns(100).with do |commits|
-      commits.each do |commit|
-        assert commit[:sha]
-        assert commit[:message]
-        assert commit[:url]
-        assert commit[:created_at]
-        assert commit[:repo]
-        assert commit[:author_name]
-      end
-
-      assert 100, commits.count
-    end
+    expect_to_run(100, 2)
 
     VCR.use_cassette('github 200 commits') do
       ManualRunner.new(repo).run_last(200)
@@ -39,21 +28,29 @@ class ManualRunnerTest < ActiveSupport::TestCase
     organization = create(:organization, name: 'jeremyevans')
     repo = create(:repo, name: 'sequel', organization: organization)
 
-    CommitsRunner.expects(:run).times(1).returns(20).with do |commits|
-      commits.each do |commit|
-        assert commit[:sha]
-        assert commit[:message]
-        assert commit[:url]
-        assert commit[:created_at]
-        assert commit[:repo]
-        assert commit[:author_name]
-      end
-
-      assert_equal 20, commits.count
-    end
+    expect_to_run(20, 1)
 
     VCR.use_cassette('github 20 commits') do
       ManualRunner.new(repo).run_last(20)
     end
   end
+
+  private
+
+  def expect_to_run(count, times)
+    CommitsRunner.expects(:run).times(times).returns(count).with do |source, commits, repo, pattern|
+      commits.each do |commit|
+        assert commit['sha']
+        assert commit['commit']['message']
+        assert commit['html_url']
+        assert commit['commit']['author']['date']
+        assert commit['commit']['author']['name']
+        assert repo
+        assert_equal '', pattern
+        assert_equal :api, source
+      end
+
+      assert_equal count, commits.count
+    end
+  end
 end