Merge pull request #188 from vshn/bugfix/pr-module_options-nil

Fix NoMethodError for --pr option (caused by `module_options = nil`)
voxpupuli · Aug 11, 2020 · f20cfa5 · f20cfa5
2 parents 22334c0 + ebc888f
commit f20cfa5
Show file tree

Hide file tree

Showing 4 changed files with 68 additions and 24 deletions.
diff --git a/features/update.feature b/features/update.feature
@@ -905,6 +905,28 @@ Feature: update
     Then the output should not contain "error"
     Then the output should not contain "rejected"
 
+  Scenario: Creating a GitHub PR with an update
+    Given a mocked git configuration
+    And a remote module repository
+    And a directory named "moduleroot"
+    And I set the environment variables to:
+      | variable     | value  |
+      | GITHUB_TOKEN | foobar |
+    When I run `msync update --noop --branch managed_update --pr`
+    Then the output should contain "Would submit PR "
+    And the exit status should be 0
+
+  Scenario: Creating a GitLab MR with an update
+    Given a mocked git configuration
+    And a remote module repository
+    And a directory named "moduleroot"
+    And I set the environment variables to:
+      | variable     | value  |
+      | GITLAB_TOKEN | foobar |
+    When I run `msync update --noop --branch managed_update --pr`
+    Then the output should contain "Would submit MR "
+    And the exit status should be 0
+
   Scenario: Repository with a default branch other than master
     Given a mocked git configuration
     And a remote module repository with "develop" as the default branch

diff --git a/lib/modulesync.rb b/lib/modulesync.rb
@@ -132,6 +132,7 @@ def self.manage_module(puppet_module, module_files, module_options, defaults, op
 
     if options[:noop]
       Git.update_noop(git_repo, options)
+      options[:pr] && pr(module_options).manage(namespace, module_name, options)
     elsif !options[:offline]
       pushed = Git.update(git_repo, files_to_manage, options)
       pushed && options[:pr] && pr(module_options).manage(namespace, module_name, options)
@@ -183,7 +184,8 @@ def self.update(options)
     exit 1 if errors && options[:fail_on_warnings]
   end
 
-  def self.pr(module_options = {})
+  def self.pr(module_options)
+    module_options ||= {}
     github_conf = module_options[:github]
     gitlab_conf = module_options[:gitlab]
 

diff --git a/lib/modulesync/pr/github.rb b/lib/modulesync/pr/github.rb
@@ -18,23 +18,32 @@ def manage(namespace, module_name, options)
         head = "#{namespace}:#{options[:branch]}"
         target_branch = options[:pr_target_branch] || 'master'
 
-        pull_requests = @api.pull_requests(repo_path, :state => 'open', :base => target_branch, :head => head)
-        if pull_requests.empty?
-          pr = @api.create_pull_request(repo_path,
-                                        target_branch,
-                                        options[:branch],
-                                        options[:pr_title],
-                                        options[:message])
+        if options[:noop]
           $stdout.puts \
-            "Submitted PR '#{options[:pr_title]}' to #{repo_path} - merges #{options[:branch]} into #{target_branch}"
-        else
+            "Using no-op. Would submit PR '#{options[:pr_title]}' to #{repo_path} " \
+            "- merges #{options[:branch]} into #{target_branch}"
+          return
+        end
+
+        pull_requests = @api.pull_requests(repo_path,
+                                           :state => 'open',
+                                           :base => target_branch,
+                                           :head => head)
+        unless pull_requests.empty?
           # Skip creating the PR if it exists already.
           $stdout.puts "Skipped! #{pull_requests.length} PRs found for branch #{options[:branch]}"
+          return
         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])
+        pr = @api.create_pull_request(repo_path,
+                                      target_branch,
+                                      options[:branch],
+                                      options[:pr_title],
+                                      options[:message])
+        $stdout.puts \
+          "Submitted PR '#{options[:pr_title]}' to #{repo_path} " \
+          "- merges #{options[:branch]} into #{target_branch}"
 
         # 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.

diff --git a/lib/modulesync/pr/gitlab.rb b/lib/modulesync/pr/gitlab.rb
@@ -15,27 +15,38 @@ def initialize(token, endpoint)
 
       def manage(namespace, module_name, options)
         repo_path = File.join(namespace, module_name)
-
         head = "#{namespace}:#{options[:branch]}"
         target_branch = options[:pr_target_branch] || 'master'
+
+        if options[:noop]
+          $stdout.puts \
+            "Using no-op. Would submit MR '#{options[:pr_title]}' to #{repo_path} " \
+            "- merges #{options[:branch]} into #{target_branch}"
+          return
+        end
+
         merge_requests = @api.merge_requests(repo_path,
                                              :state => 'opened',
                                              :source_branch => head,
                                              :target_branch => target_branch)
-        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 => target_branch,
-                                         :labels => mr_labels)
-          $stdout.puts \
-            "Submitted MR '#{options[:pr_title]}' to #{repo_path} - merges #{options[:branch]} into #{target_branch}"
-          return if mr_labels.empty?
-          $stdout.puts "Attached the following labels to MR #{mr.iid}: #{mr_labels.join(', ')}"
-        else
+        unless merge_requests.empty?
           # Skip creating the MR if it exists already.
           $stdout.puts "Skipped! #{merge_requests.length} MRs found for branch #{options[:branch]}"
+          return
         end
+
+        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 => target_branch,
+                                       :labels => mr_labels)
+        $stdout.puts \
+          "Submitted MR '#{options[:pr_title]}' to #{repo_path} " \
+          "- merges #{options[:branch]} into #{target_branch}"
+
+        return if mr_labels.empty?
+        $stdout.puts "Attached the following labels to MR #{mr.iid}: #{mr_labels.join(', ')}"
       end
     end
   end