Improve PR search when gitarro is running with cron-like tools to avo…

…id exceeding GitHub API limits

.rubocop_todo.yml

@@ -1,6 +1,6 @@
 # This configuration was generated by
 # `rubocop --auto-gen-config`
-# on 2017-09-30 17:37:11 +0200 using RuboCop version 0.50.0.
+# on 2017-10-02 15:36:34 +0200 using RuboCop version 0.50.0.
 # The point is for the user to remove these configuration records
 # one by one as the offenses are removed from the code base.
 # Note that changes in the inspected code, or installation of new
@@ -9,9 +9,9 @@
 # Offense count: 2
 # Configuration parameters: CountComments, ExcludedMethods.
 Metrics/BlockLength:
-  Max: 32
+  Max: 43
 
 # Offense count: 2
 # Configuration parameters: CountComments.
 Metrics/ClassLength:
-  Max: 169
+  Max: 166

gitarro.rb

@@ -8,7 +8,9 @@
 require_relative 'lib/gitarro/backend'
 
 b = Backend.new
-prs = b.open_prs
+prs = b.open_newer_prs
+exit 0 if b.check_pr_list_empty(prs)
+
 prs.each do |pr|
   puts '=' * 30 + "\n" + "TITLE_PR: #{pr.title}, NR: #{pr.number}\n" + '=' * 30
   # this check the last commit state, catch for review or not reviewd status.

lib/gitarro/backend.rb

@@ -2,6 +2,7 @@
 
 require 'octokit'
 require 'optparse'
+require 'time'
 require 'English'
 require_relative 'opt_parser'
 require_relative 'git_op'
@@ -54,7 +55,6 @@ def initialize(option = nil)
     @client = Octokit::Client.new(netrc: true)
     @options = option.nil? ? OptParser.new.cmdline_options : option
     @j_status = ''
-    @pr_files = []
     # each options will generate a object variable dinamically
     @options.each do |key, value|
       instance_variable_set("@#{key}", value)
@@ -63,105 +63,119 @@ def initialize(option = nil)
     @gbexec = TestExecutor.new(@options)
   end
 
-  # public method for get prs opens
-  # given a repo
-  def open_prs
-    prs = @client.pull_requests(@repo, state: 'open')
-    puts 'no Pull request OPEN on the REPO!' unless prs.any?
+  # Check if a PR list is empty
+  # If it is and we do want to consider the update time, return true
+  # Otherwise return false
+  def check_pr_list_empty(prs)
+    prs.empty? && @options[:changed_since] >= 0 ? true : false
+  end
+
+  # public method for get prs opened and matching the changed_since
+  # condition
+  def open_newer_prs
+    prs = []
+    @client.pull_requests(@repo, state: 'open').each do |pr|
+      prs << pr if pr_last_update_less_than(pr, @options[:changed_since])
+    end
+    if check_pr_list_empty(prs)
+      puts 'There are no Pull Requests opened or with changes newer than ' \
+           "#{options[:changed_since]} seconds"
+    end
     prs
   end
 
   # public for etrigger the test
   def retrigger_check(pr)
     return unless retrigger_needed?(pr)
-    client.create_status(@repo, pr.head.sha, 'pending',
-                         context: @context, description: @description,
-                         target_url: @target_url)
+    create_status(pr, 'pending')
     exit 1 if @check
-    launch_test_and_setup_status(@repo, pr)
+    launch_test_and_setup_status(pr)
     j_status == 'success' ? exit(0) : exit(1)
   end
 
   # public always rerun tests against the pr number if this exists
   def trigger_by_pr_number(pr)
-    return false if @pr_number.nil?
-    return false if @pr_number != pr.number
+    return false if @pr_number.nil? || @pr_number != pr.number
     puts "Got triggered by PR_NUMBER OPTION, rerunning on #{@pr_number}"
-    launch_test_and_setup_status(@repo, pr)
-    true
+    launch_test_and_setup_status(pr)
   end
 
   # public method, trigger changelogtest if option active
   def changelog_active(pr, comm_st)
     return false unless @changelog_test
-    return false unless changelog_changed(@repo, pr, comm_st)
+    return false unless changelog_changed(pr, comm_st)
     true
   end
 
   def unreviewed_pr_test(pr, comm_st)
     return unless unreviewed_pr_ck(comm_st)
-    pr_all_files_type(@repo, pr.number, @file_type)
+    pr_all_files_type(pr.number, @file_type)
     return if empty_files_changed_by_pr
     # gb.check is true when there is a job running as scheduler
     # which doesn't execute the test but trigger another job
     return false if @check
-    launch_test_and_setup_status(@repo, pr)
-    true
+    launch_test_and_setup_status(pr)
   end
 
   def reviewed_pr_test(comm_st, pr)
     # if PR status is not on pending and the context is not set,
     #  we dont run the tests
     return false unless context_pr(comm_st) == false ||
                         pending_pr(comm_st) == true
-    pr_all_files_type(@repo, pr.number, @file_type)
     return true if changelog_active(pr, comm_st)
-    return false unless @pr_files.any?
-    exit 1 if @check
-    launch_test_and_setup_status(@repo, pr)
-    true
-  end
-
-  private
-
-  # this function setup first pending to PR, then execute the tests
-  # then set the status according to the results of script executed.
-  # pr_head = is the PR branch
-  # base = is a the upstream branch, where the pr targets
-  def launch_test_and_setup_status(repo, pr)
-    # pending
-    @client.create_status(repo, pr.head.sha, 'pending',
-                          context: @context, description: @description,
-                          target_url: @target_url)
-    # do tests
-    @j_status = gbexec.pr_test(pr)
-    # set status
-    @client.create_status(repo, pr.head.sha, @j_status,
-                          context: @context, description: @description,
-                          target_url: @target_url)
+    return false unless pr_all_files_type(pr.number, @file_type).any?
+    @check ? exit(1) : launch_test_and_setup_status(pr)
   end
 
   # this function will check if the PR contains in comment the magic word
   # # for retrigger all the tests.
-  def magicword(repo, pr_number, context)
+  def magicword(pr_number, context)
     magic_word_trigger = "gitarro rerun #{context} !!!"
-    pr_comment = @client.issue_comments(repo, pr_number)
     # a pr contain always a comments, cannot be nil
-    pr_comment.each do |com|
+    @client.issue_comments(@repo, pr_number).each do |com|
       # delete comment otherwise it will be retrigger infinetely
       if com.body.include? magic_word_trigger
-        @client.delete_comment(repo, com.id)
+        @client.delete_comment(@repo, com.id)
         return true
       end
     end
     false
   end
 
+  private
+
+  # Create a status for a PR
+  def create_status(pr, status)
+    client.create_status(@repo, pr.head.sha, status, context: @context,
+                                                     description: @description,
+                                                     target_url: @target_url)
+    true
+  end
+
+  # Return true if the PR was updated in less than the value of variable sec
+  # or if sec < 0 (the check was disabled)
+  # GitHub considers a PR updated when there is a new commit or a new comment
+  def pr_last_update_less_than(pr, sec)
+    Time.now.utc - pr.updated_at < sec || sec < 0 ? true : false
+  end
+
+  # this function setup first pending to PR, then execute the tests
+  # then set the status according to the results of script executed.
+  # pr_head = is the PR branch
+  # base = is a the upstream branch, where the pr targets
+  def launch_test_and_setup_status(pr)
+    # pending
+    create_status(pr, 'pending')
+    # do tests
+    @j_status = gbexec.pr_test(pr)
+    # set status
+    create_status(pr, @j_status)
+  end
+
   # check all files of a Prs Number if they are a specific type
   # EX: Pr 56, we check if files are '.rb'
-  def pr_all_files_type(repo, pr_number, type)
-    files = @client.pull_request_files(repo, pr_number)
-    @pr_files = filter_files_by_type(files, type)
+  def pr_all_files_type(pr_number, type)
+    filter_files_by_type(@client.pull_request_files(@repo, pr_number), type)
   end
 
   # by default type is 'notype', which imply we get all files
@@ -192,8 +206,8 @@ def pending_pr(comm_st)
 
   # if the Pr contains magic word, test changelog
   # is true
-  def magic_comment(repo, pr_num)
-    @client.issue_comments(repo, pr_num).each do |com|
+  def magic_comment(pr_num)
+    @client.issue_comments(@repo, pr_num).each do |com|
       if com.body.include?('no changelog needed!')
         @j_status = 'success'
         break
@@ -251,37 +265,31 @@ def empty_files_changed_by_pr
     true
   end
 
-  def do_changelog_test(repo, pr)
+  def do_changelog_test(pr)
     @j_status = 'failure'
-    pr_all_files_type(repo, pr.number, @file_type)
     # if the pr contains changes on .changes file, test ok
-    @j_status = 'success' if @pr_files.any?
-    magic_comment(repo, pr.number)
-    @client.create_status(repo, pr.head.sha, @j_status,
-                          context: @context, description: @description,
-                          target_url: @target_url)
-    true
+    @j_status = 'success' if pr_all_files_type(pr.number, @file_type).any?
+    magic_comment(pr.number)
+    create_status(pr, @j_status)
   end
 
   # do the changelog test and set status
-  def changelog_changed(repo, pr, comm_st)
+  def changelog_changed(pr, comm_st)
     return false unless @changelog_test
     # only execute 1 time, don"t run if test is failed, or ok
-    return false if failed_status?(comm_st)
-    return false if success_status?(comm_st)
-    do_changelog_test(repo, pr)
+    return false if failed_status?(comm_st) || success_status?(comm_st)
+    do_changelog_test(pr)
   end
 
   def retrigger_needed?(pr)
     # we want redo sometimes tests
-    return false unless magicword(@repo, pr.number, @context)
+    return false unless magicword(pr.number, @context)
     # changelog trigger
     if @changelog_test
-      do_changelog_test(@repo, pr)
+      do_changelog_test(pr)
       return false
     end
-    pr_all_files_type(@repo, pr.number, @file_type)
-    return false unless @pr_files.any?
+    return false unless pr_all_files_type(pr.number, @file_type).any?
     # if check is set, the comment in the trigger job will be del.
     # so setting it to pending, it will be remembered
     true

lib/gitarro/opt_parser.rb

@@ -90,6 +90,15 @@ def pr_number(opt)
     end
   end
 
+  def changed_since(opt)
+    changed_since_desc = 'If present, will only check PRs with a ' \
+                       'change in the last X seconds'
+    opt.on("--changed_since 'SECONDS'",
+           changed_since_desc) do |changed_since|
+      @options[:changed_since] = Integer(changed_since)
+    end
+  end
+
   def optional_options(opt)
     opt.separator ''
     opt.separator 'Optional options:'
@@ -100,6 +109,7 @@ def optional_options(opt)
     url_opt(opt)
     pr_number(opt)
     https_opt(opt)
+    changed_since(opt)
   end
 end
 
@@ -164,6 +174,7 @@ def defaults_false
     @options[:changelog_test] = false if @options[:changelog_test].nil?
     @options[:target_url] = '' if @options[:target_url].nil?
     @options[:https] = false if @options[:https].nil?
+    @options[:changed_since] = -1 if @options[:changed_since].nil?
   end
 
   def defaults_to_text

tests/spec/cmdline_spec.rb

@@ -74,4 +74,15 @@
       expect(result).to be true
     end
   end
+
+  describe '.changed_since' do
+    it 'gitarro should see at least PR #30 changed in the last 60 seconds' do
+      context = 'changed-since'
+      desc = 'changed-since'
+      @comment = @rgit.create_comment(@pr, "gitarro rerun #{context} !!!")
+      result = @test.change_since_should_find(@comm_st, 60, context, desc)
+      @rgit.delete_c(@comment.id)
+      expect(result).to be true
+    end
+  end
 end

tests/spec/test_lib.rb

@@ -124,6 +124,17 @@ def file_type_unset(comm_st, cont)
     true
   end
 
+  def change_since_should_find(com_st, sec, context, desc)
+    `echo '#! /bin/bash' > #{valid_test}`
+    `chmod +x #{valid_test}`
+    gitarro = "#{script} -r #{repo} -c #{context} -d #{desc} -g #{git_dir}" \
+                   " -t #{valid_test} -f #{ftype} -u #{url}" \
+                   " --change_since #{sec}"
+    puts `ruby #{gitarro}`
+    return false if failed_status(com_st, context)
+    true
+  end
+
   private
 
   def create_test_script(script)

tests/unit_tests/t_backend.rb

@@ -5,10 +5,13 @@
 Dir.chdir Dir.pwd
 # Test the option parser
 class BackendTest2 < Minitest::Test
-  def test_full_option_import2
+  def setup
     @full_hash = { repo: 'gino/gitarro', context: 'python-t', description:
                    'functional', test_file: 'gino.sh', file_type: '.sh',
-                   git_dir: 'gitty' }
+                   git_dir: 'gitty', changed_since: -1 }
+  end
+
+  def test_full_option_import2
     gitarro = Backend.new(@full_hash)
     puts gitarro.j_status
     gitarro.j_status = 'foo'
@@ -25,9 +28,7 @@ def gitarro_assert(gitarro)
   end
 
   def test_run_script
-    @full_hash = { repo: 'gino/gitarro', context: 'python-t', description:
-                   'functional', test_file: 'test_data/script_ok.sh',
-                   file_type: '.sh', git_dir: 'gitty' }
+    @full_hash[:test_file] = 'test_data/script_ok.sh'
     gbex = TestExecutor.new(@full_hash)
     ck_files(gbex)
     test_file = 'nofile.txt'
@@ -48,4 +49,20 @@ def assert_file_non_ex(gbex, test_file)
     assert_equal("'#{test_file}\' doesn't exists.Enter valid file, -t option",
                  ex.message)
   end
+
+  # We always have PR #30 opened
+  def test_get_all_prs
+    @full_hash[:repo] = 'openSUSE/gitarro'
+    gitarro = Backend.new(@full_hash)
+    prs = gitarro.open_newer_prs
+    assert(true, prs.any?)
+  end
+
+  def test_get_no_prs
+    @full_hash[:repo] = 'openSUSE/gitarro'
+    @full_hash[:changed_since] = 0
+    gitarro = Backend.new(@full_hash)
+    prs = gitarro.open_newer_prs
+    assert(0, prs.count)
+  end
 end