Refactor PR/MR creation

* Move implementation into separate modules
* Lazy-load dependencies
* Reuse GitHub/GitLab clients
* Add some basic specs for the GitLab implementation

Signed-off-by: Manuel Hutter <manuel@vshn.ch>

lib/modulesync.rb

@@ -1,6 +1,4 @@
 require 'fileutils'
-require 'gitlab'
-require 'octokit'
 require 'pathname'
 require 'modulesync/cli'
 require 'modulesync/constants'
@@ -14,14 +12,6 @@
 GITHUB_TOKEN = ENV.fetch('GITHUB_TOKEN', '')
 GITLAB_TOKEN = ENV.fetch('GITLAB_TOKEN', '')
 
-Octokit.configure do |c|
-  c.api_endpoint = ENV.fetch('GITHUB_BASE_URL', 'https://api.github.com')
-end
-
-Gitlab.configure do |c|
-  c.endpoint = ENV.fetch('GITLAB_BASE_URL', 'https://gitlab.com/api/v4')
-end
-
 module ModuleSync # rubocop:disable Metrics/ModuleLength
   include Constants
 
@@ -161,68 +151,13 @@ def self.manage_pr_or_mr(git_repo, files_to_manage, namespace, module_name, opti
     # If there's nothing pushed, we're done
     return nil unless pushed
 
-    if !GITHUB_TOKEN.empty?
-      manage_pr(namespace, module_name, options)
-    elsif !GITLAB_TOKEN.empty?
-      manage_mr(namespace, module_name, options)
-    else
-      $stderr.puts 'Environment variables GITHUB_TOKEN or GITLAB_TOKEN must be set to use --pr!'
-      raise unless options[:skip_broken]
-    end
-  end
-
-  def self.manage_mr(namespace, module_name, options)
-    repo_path = File.join(namespace, module_name)
-    api = Gitlab::Client.new(:private_token => GITLAB_TOKEN)
-
-    head = "#{namespace}:#{options[:branch]}"
-    merge_requests = api.merge_requests(repo_path,
-                                        :state => 'opened',
-                                        :source_branch => head,
-                                        :target_branch => 'master')
-    if merge_requests.empty?
-      mr_labels = Util.parse_list(options[:pr_labels])
-      mr = api.create_merge_request(repo_path, options[:pr_title],
-                                    :source_branch => options[:branch],
-                                    :target_branch => 'master',
-                                    :labels => mr_labels)
-      $stdout.puts "Submitted MR '#{options[:pr_title]}' to #{repo_path} - merges #{options[:branch]} into master"
-      return if mr_labels.empty?
-      $stdout.puts "Attaching the following labels to MR #{mr.iid}: #{mr_labels.join(', ')}"
-    else
-      # Skip creating the MR if it exists already.
-      $stdout.puts "Skipped! #{pull_requests.length} MRs found for branch #{options[:branch]}"
-    end
-  end
-
-  def self.manage_pr(namespace, module_name, options)
-    repo_path = File.join(namespace, module_name)
-    api = Octokit::Client.new(:access_token => GITHUB_TOKEN)
-
-    head = "#{namespace}:#{options[:branch]}"
-    pull_requests = api.pull_requests(repo_path, :state => 'open', :base => 'master', :head => head)
-    if pull_requests.empty?
-      pr = api.create_pull_request(repo_path, 'master', options[:branch], options[:pr_title], options[:message])
-      $stdout.puts "Submitted PR '#{options[:pr_title]}' to #{repo_path} - merges #{options[:branch]} into master"
-    else
-      # Skip creating the PR if it exists already.
-      $stdout.puts "Skipped! #{pull_requests.length} PRs found for branch #{options[:branch]}"
-    end
-
-    # PR labels can either be a list in the YAML file or they can pass in a comma
-    # separated list via the command line argument.
-    pr_labels = Util.parse_list(options[:pr_labels])
-
-    # We only assign labels to the PR if we've discovered a list > 1. The labels MUST
-    # already exist. We DO NOT create missing labels.
-    return if pr_labels.empty?
-    $stdout.puts "Attaching the following labels to PR #{pr['number']}: #{pr_labels.join(', ')}"
-    api.add_labels_to_an_issue(repo_path, pr['number'], pr_labels)
+    @pr.manage(namespace, module_name, options)
   end
 
   def self.update(options)
     options = config_defaults.merge(options)
     defaults = Util.parse_config(File.join(options[:configs], CONF_FILE))
+    @pr = get_pr_manager if options[:pr]
 
     local_template_dir = File.join(options[:configs], MODULE_FILES_DIR)
     local_files = find_template_files(local_template_dir)
@@ -246,4 +181,18 @@ def self.update(options)
     end
     exit 1 if errors && options[:fail_on_warnings]
   end
+
+  def self.get_pr_manager
+    case
+    when !GITHUB_TOKEN.empty?
+      require 'modulesync/pr/github'
+      return ModuleSync::PR::GitHub.new(GITHUB_TOKEN, ENV.fetch('GITHUB_BASE_URL', 'https://api.github.com'))
+    when !GITLAB_TOKEN.empty?
+      require 'modulesync/pr/github'
+      return ModuleSync::PR::GitLab.new(GITLAB_TOKEN, ENV.fetch('GITLAB_BASE_URL', 'https://gitlab.com/api/v4'))
+    else
+      $stderr.puts 'Environment variables GITHUB_TOKEN or GITLAB_TOKEN must be set to use --pr!'
+      raise
+    end
+  end
 end

lib/modulesync/pr/github.rb

@@ -0,0 +1,37 @@
+require 'octokit'
+require 'modulesync/util'
+
+module ModuleSync::PR
+  class GitHub
+    def initialize(token, endpoint)
+      Octokit.configure do |c|
+        c.api_endpoint = endpoint
+      end
+      @api = Octokit::Client.new(:access_token => token)
+    end
+
+    def manage(namespace, module_name, options)
+      repo_path = File.join(namespace, module_name)
+      head = "#{namespace}:#{options[:branch]}"
+
+      pull_requests = @api.pull_requests(repo_path, :state => 'open', :base => 'master', :head => head)
+      if pull_requests.empty?
+        pr = @api.create_pull_request(repo_path, 'master', options[:branch], options[:pr_title], options[:message])
+        $stdout.puts "Submitted PR '#{options[:pr_title]}' to #{repo_path} - merges #{options[:branch]} into master"
+      else
+        # Skip creating the PR if it exists already.
+        $stdout.puts "Skipped! #{pull_requests.length} PRs found for branch #{options[:branch]}"
+      end
+
+      # PR labels can either be a list in the YAML file or they can pass in a comma
+      # separated list via the command line argument.
+      pr_labels = ModuleSync::Util.parse_list(options[:pr_labels])
+
+      # We only assign labels to the PR if we've discovered a list > 1. The labels MUST
+      # already exist. We DO NOT create missing labels.
+      return if pr_labels.empty?
+      $stdout.puts "Attaching the following labels to PR #{pr['number']}: #{pr_labels.join(', ')}"
+      @api.add_labels_to_an_issue(repo_path, pr['number'], pr_labels)
+    end
+  end
+end

lib/modulesync/pr/gitlab.rb

@@ -0,0 +1,36 @@
+require 'gitlab'
+require 'modulesync/util'
+
+module ModuleSync::PR
+  class GitLab
+    def initialize(token, endpoint)
+      @api = Gitlab::Client.new(
+        :endpoint => endpoint,
+        :private_token => token,
+      )
+    end
+
+    def manage(namespace, module_name, options)
+      repo_path = File.join(namespace, module_name)
+
+      head = "#{namespace}:#{options[:branch]}"
+      merge_requests = @api.merge_requests(repo_path,
+                                           :state => 'opened',
+                                           :source_branch => head,
+                                           :target_branch => 'master')
+      if merge_requests.empty?
+        mr_labels = ModuleSync::Util.parse_list(options[:pr_labels])
+        mr = @api.create_merge_request(repo_path, options[:pr_title],
+                                       :source_branch => options[:branch],
+                                       :target_branch => 'master',
+                                       :labels => mr_labels)
+        $stdout.puts "Submitted MR '#{options[:pr_title]}' to #{repo_path} - merges #{options[:branch]} into master"
+        return if mr_labels.empty?
+        $stdout.puts "Attached the following labels to MR #{mr.iid}: #{mr_labels.join(', ')}"
+      else
+        # Skip creating the MR if it exists already.
+        $stdout.puts "Skipped! #{merge_requests.length} MRs found for branch #{options[:branch]}"
+      end
+    end
+  end
+end

spec/unit/modulesync/pr/github_spec.rb

@@ -0,0 +1,49 @@
+require 'spec_helper'
+require 'modulesync/pr/github'
+
+describe ModuleSync::PR::GitHub do
+  context '::manage' do
+    before(:each) do
+      @git_repo = 'test/modulesync'
+      @namespace, @repo_name = @git_repo.split('/')
+      @options = {
+        :pr => true,
+        :pr_title => 'Test PR is submitted',
+        :branch => 'test',
+        :message => 'Hello world',
+        :pr_auto_merge => false,
+      }
+
+      @client = double()
+      allow(Octokit::Client).to receive(:new).and_return(@client)
+      @it = ModuleSync::PR::GitHub.new('test', 'https://api.github.com')
+    end
+
+    it 'submits PR when --pr is set' do
+      allow(@client).to receive(:pull_requests).with(@git_repo, :state => 'open', :base => 'master', :head => "#{@namespace}:#{@options[:branch]}").and_return([])
+      expect(@client).to receive(:create_pull_request).with(@git_repo, 'master', @options[:branch], @options[:pr_title], @options[:message]).and_return({"html_url" => "http://example.com/pulls/22"})
+      expect { @it.manage(@namespace, @repo_name, @options) }.to output(/Submitted PR/).to_stdout
+    end
+
+    it 'skips submitting PR if one has already been issued' do
+      pr = {
+        "title" => "Test title",
+        "html_url" => "https://example.com/pulls/44",
+        "number" => "44"
+      }
+
+      expect(@client).to receive(:pull_requests).with(@git_repo, :state => 'open', :base => 'master', :head => "#{@namespace}:#{@options[:branch]}").and_return([pr])
+      expect { @it.manage(@namespace, @repo_name, @options) }.to output(/Skipped! 1 PRs found for branch test/).to_stdout
+    end
+
+    it 'adds labels to PR when --pr-labels is set' do
+      @options[:pr_labels] = "HELLO,WORLD"
+
+      allow(@client).to receive(:create_pull_request).and_return({"html_url" => "http://example.com/pulls/22", "number" => "44"})
+      allow(@client).to receive(:pull_requests).with(@git_repo, :state => 'open', :base => 'master', :head => "#{@namespace}:#{@options[:branch]}").and_return([])
+
+      expect(@client).to receive(:add_labels_to_an_issue).with(@git_repo, "44", ["HELLO", "WORLD"])
+      expect { @it.manage(@namespace, @repo_name, @options) }.to output(/Attaching the following labels to PR 44: HELLO, WORLD/).to_stdout
+    end
+  end
+end

spec/unit/modulesync/pr/gitlab_spec.rb

@@ -0,0 +1,81 @@
+require 'spec_helper'
+require 'modulesync/pr/gitlab'
+
+describe ModuleSync::PR::GitLab do
+  context '::manage' do
+    before(:each) do
+      @git_repo = 'test/modulesync'
+      @namespace, @repo_name = @git_repo.split('/')
+      @options = {
+        :pr => true,
+        :pr_title => 'Test PR is submitted',
+        :branch => 'test',
+        :message => 'Hello world',
+        :pr_auto_merge => false,
+      }
+
+      @client = double()
+      allow(Gitlab::Client).to receive(:new).and_return(@client)
+      @it = ModuleSync::PR::GitLab.new('test', 'https://gitlab.com/api/v4')
+    end
+
+    it 'submits MR when --pr is set' do
+      allow(@client).to receive(:merge_requests)
+        .with(@git_repo,
+              :state => 'opened',
+              :source_branch => "#{@namespace}:#{@options[:branch]}",
+              :target_branch => 'master',
+             ).and_return([])
+
+      expect(@client).to receive(:create_merge_request)
+        .with(@git_repo,
+              @options[:pr_title],
+              :labels => [],
+              :source_branch => @options[:branch],
+              :target_branch => 'master',
+             ).and_return({"html_url" => "http://example.com/pulls/22"})
+
+      expect { @it.manage(@namespace, @repo_name, @options) }.to output(/Submitted MR/).to_stdout
+    end
+
+    it 'skips submitting MR if one has already been issued' do
+      mr = {
+        "title" => "Test title",
+        "html_url" => "https://example.com/pulls/44",
+        "iid" => "44"
+      }
+
+      expect(@client).to receive(:merge_requests)
+        .with(@git_repo,
+              :state => 'opened',
+              :source_branch => "#{@namespace}:#{@options[:branch]}",
+              :target_branch => 'master',
+             ).and_return([mr])
+
+      expect { @it.manage(@namespace, @repo_name, @options) }.to output(/Skipped! 1 MRs found for branch test/).to_stdout
+    end
+
+    it 'adds labels to MR when --pr-labels is set' do
+      @options[:pr_labels] = "HELLO,WORLD"
+      mr = double()
+      allow(mr).to receive(:iid).and_return("42")
+
+      expect(@client).to receive(:create_merge_request)
+        .with(@git_repo,
+              @options[:pr_title],
+              :labels => ["HELLO", "WORLD"],
+              :source_branch => @options[:branch],
+              :target_branch => 'master',
+             ).and_return(mr)
+
+      allow(@client).to receive(:merge_requests)
+        .with(@git_repo,
+              :state => 'opened',
+              :source_branch => "#{@namespace}:#{@options[:branch]}",
+              :target_branch => 'master',
+             ).and_return([])
+
+      expect { @it.manage(@namespace, @repo_name, @options) }.to output(/Attached the following labels to MR 42: HELLO, WORLD/).to_stdout
+    end
+  end
+end