From fe19eae84ca17e89373c58475cf89e5c5f4025c2 Mon Sep 17 00:00:00 2001 From: Rodrigo Nardi Date: Wed, 3 Jul 2024 11:12:24 -0300 Subject: [PATCH 1/5] Old Checks support In this PR we will deal with the old GitHub Checks. When a PR runs and has the old entries, it will be marked as skipped and create the 3 new stages. It also fixed a strange behavior when the build stage fails and leaves the testing stage in a peculiar state Signed-off-by: Rodrigo Nardi --- lib/github/build/action.rb | 2 + lib/github/build/skip.rb | 36 +++++++++++++++ lib/github/build/summary.rb | 31 ++++++------- lib/github/check.rb | 4 ++ lib/github_ci_app.rb | 1 + spec/lib/github/build/action_spec.rb | 61 ++++++++++++++++++++++++++ spec/lib/github/build_plan_spec.rb | 2 + spec/lib/github/re_run/command_spec.rb | 1 + spec/lib/github/re_run/comment_spec.rb | 5 +++ 9 files changed, 125 insertions(+), 18 deletions(-) create mode 100644 lib/github/build/skip.rb diff --git a/lib/github/build/action.rb b/lib/github/build/action.rb index 4e62eb1..cb004a6 100644 --- a/lib/github/build/action.rb +++ b/lib/github/build/action.rb @@ -29,6 +29,8 @@ def initialize(check_suite, github, jobs, logger_level: Logger::INFO) def create_summary(rerun: false) logger(Logger::INFO, "SUMMARY #{@stages.inspect}") + Skip.new(@check_suite).skip_old_tests + @stages.each do |stage_config| create_check_run_stage(stage_config) end diff --git a/lib/github/build/skip.rb b/lib/github/build/skip.rb new file mode 100644 index 0000000..03eef38 --- /dev/null +++ b/lib/github/build/skip.rb @@ -0,0 +1,36 @@ +# SPDX-License-Identifier: BSD-2-Clause +# +# skip.rb +# Part of NetDEF CI System +# +# Copyright (c) 2024 by +# Network Device Education Foundation, Inc. ("NetDEF") +# +# frozen_string_literal: true + +class Skip + def initialize(check_suite) + @check_suite = check_suite + @github = Github::Check.new(@check_suite) + @stages = StageConfiguration.all.map(&:github_check_run_name) + @logger = GithubLogger.instance.create('github_skip_old_tests.log', Logger::INFO) + end + + def skip_old_tests + @github + .check_runs_for_ref(@check_suite.pull_request.repository, @check_suite.commit_sha_ref)[:check_runs] + &.each { |check_run| skipping_old_test(check_run) } + end + + private + + def skipping_old_test(check_run) + return if @stages.include?(check_run[:name]) or check_run[:app][:name] != 'NetDEF CI Hook' + + @logger.info("Skipping old test suite: #{check_run[:name]}") + + message = 'Old test suite, skipping...' + @github.create(check_run[:name]) + @github.skipped(check_run[:id], { title: "#{check_run[:name]} summary", summary: message }) + end +end diff --git a/lib/github/build/summary.rb b/lib/github/build/summary.rb index 4ab7170..f83cf17 100644 --- a/lib/github/build/summary.rb +++ b/lib/github/build/summary.rb @@ -40,6 +40,8 @@ def build_summary msg = "Github::Build::Summary - #{@job.inspect}, #{current_stage.inspect}, bamboo info: #{bamboo_info}" @pr_log.info(msg) + return if current_stage.cancelled? + # Update current stage update_summary(current_stage) # Check if current stage finished @@ -71,7 +73,9 @@ def must_update_previous_stage(current_stage) end def must_cancel_next_stages(current_stage) - return if @job.success? or @job.in_progress? or @job.queued? + logger(Logger::INFO, "must_cancel_next_stages: #{current_stage.inspect}") + + return unless current_stage.failure? or current_stage.skipped? or current_stage.cancelled? return unless current_stage.configuration.mandatory? Stage @@ -96,7 +100,7 @@ def must_continue_next_stage(current_stage) .where(configuration: { position: current_stage.configuration.position + 1 }) .first - return if next_stage.nil? + return if next_stage.nil? or next_stage.cancelled? update_summary(next_stage) end @@ -110,24 +114,19 @@ def cancelling_next_stage(pending_stage) "The previous stage failed and the remaining tests will be canceled.\nDetails at [#{url}](#{url})." } - logger(Logger::INFO, "cancelling_next_stage - pending_stage: #{pending_stage.inspect}\n#{output}") + logger(Logger::INFO, "cancelling_next_stage - pending_stage: #{pending_stage.inspect}") pending_stage.cancelled(@github, output: output) pending_stage.jobs.each { |job| job.cancelled(@github) } end def finished_summary(stage) - logger(Logger::INFO, "Finished stage: #{stage.inspect}, CiJob status: #{@job.status}") - logger(Logger::INFO, "Finished stage: #{stage.inspect}, running? #{stage.reload.running?}") - return if @job.in_progress? or stage.running? finished_stage_summary(stage) end def finished_stage_summary(stage) - logger(Logger::INFO, "finished_stage_summary: #{stage.inspect}. Reason Job: #{@job.inspect}") - url = "https://ci1.netdef.org/browse/#{stage.check_suite.bamboo_ci_ref}" output = { title: "#{stage.name} summary", @@ -135,31 +134,27 @@ def finished_stage_summary(stage) } finished_stage_update(stage, output) - - logger(Logger::INFO, "finished_stage_summary: #{stage.inspect} #{output.inspect}") end def finished_stage_update(stage, output) if stage.jobs.failure.empty? - logger(Logger::WARN, "Stage: #{stage.name} finished - failure") - stage.success(@github, output: output, agent: @agent) - else logger(Logger::WARN, "Stage: #{stage.name} finished - success") - stage.failure(@github, output: output, agent: @agent) + stage.success(@github, output: output, agent: @agent) + + return end + + logger(Logger::WARN, "Stage: #{stage.name} finished - failure") + stage.failure(@github, output: output, agent: @agent) end def update_summary(stage) - logger(Logger::INFO, "Updating summary status #{stage.inspect} -> @job.status: #{@job.status}") - url = "https://ci1.netdef.org/browse/#{@check_suite.bamboo_ci_ref}" output = { title: "#{stage.name} summary", summary: "#{summary_basic_output(stage)}\nDetails at [#{url}](#{url}).".force_encoding('utf-8') } - logger(Logger::INFO, "update_summary: #{stage.inspect} #{output.inspect}") - logger(Logger::WARN, "Updating stage: #{stage.name} to in_progress") stage.in_progress(@github, output: output) stage.update_output(@github, output: output) diff --git a/lib/github/check.rb b/lib/github/check.rb index a5f6b20..24fbb35 100644 --- a/lib/github/check.rb +++ b/lib/github/check.rb @@ -60,6 +60,10 @@ def comment_reaction_thumb_down(repo, comment_id) accept: Octokit::Preview::PREVIEW_TYPES[:reactions]) end + def check_runs_for_ref(repo, sha) + @app.check_runs_for_ref(repo, sha) + end + def create(name) @app.create_check_run( @check_suite.pull_request.repository, diff --git a/lib/github_ci_app.rb b/lib/github_ci_app.rb index 58cf6b3..0ea6267 100644 --- a/lib/github_ci_app.rb +++ b/lib/github_ci_app.rb @@ -28,6 +28,7 @@ require_relative 'github/update_status' require_relative 'github/plan_execution/finished' require_relative 'github/user_info' +require_relative 'github/build/skip' # Helpers libs require_relative 'helpers/configuration' diff --git a/spec/lib/github/build/action_spec.rb b/spec/lib/github/build/action_spec.rb index 1830d2e..d0ef533 100644 --- a/spec/lib/github/build/action_spec.rb +++ b/spec/lib/github/build/action_spec.rb @@ -44,11 +44,72 @@ allow(fake_github_check).to receive(:success).and_return(ci_job.check_suite) allow(fake_github_check).to receive(:cancelled).and_return(ci_job.check_suite) allow(fake_github_check).to receive(:queued).and_return(ci_job.check_suite) + allow(fake_github_check).to receive(:check_runs_for_ref).and_return({}) allow(BambooCi::Result).to receive(:fetch).and_return({}) stage end + context 'when previous check suite has old tests' do + let(:ci_job) { create(:ci_job, stage: stage, check_suite: check_suite) } + let(:old_test) { create(:ci_job, stage: stage, check_suite: check_suite) } + let(:skip_info) do + { + check_runs: [ + { + app: { + name: 'NetDEF CI Hook' + }, + name: old_test.name, + id: 1 + } + ] + } + end + + before do + old_test + allow(Stage).to receive(:create).and_return(stage) + allow(stage).to receive(:persisted?).and_return(false) + allow(fake_github_check).to receive(:check_runs_for_ref).and_return(skip_info) + end + + it 'must create a stage' do + action.create_summary(rerun: false) + expect(check_suite.reload.stages.size).to eq(1) + end + end + + context 'when previous check suite has old tests - but wrong app' do + let(:ci_job) { create(:ci_job, stage: stage, check_suite: check_suite) } + let(:old_test) { create(:ci_job, stage: stage, check_suite: check_suite) } + let(:skip_info) do + { + check_runs: [ + { + app: { + name: 'NetDEF CI' + }, + name: old_test.name, + id: 1 + } + ] + } + end + + before do + old_test + allow(Stage).to receive(:create).and_return(stage) + allow(stage).to receive(:persisted?).and_return(false) + allow(fake_github_check).to receive(:check_runs_for_ref).and_return(skip_info) + end + + it 'must create a stage' do + action.create_summary(rerun: false) + expect(check_suite.reload.stages.size).to eq(1) + end + end + context 'when could not create stage' do let(:ci_job) { create(:ci_job, stage: stage, check_suite: check_suite) } diff --git a/spec/lib/github/build_plan_spec.rb b/spec/lib/github/build_plan_spec.rb index a1cdbd4..f6b7e81 100644 --- a/spec/lib/github/build_plan_spec.rb +++ b/spec/lib/github/build_plan_spec.rb @@ -64,6 +64,8 @@ allow(fake_github_check).to receive(:in_progress).and_return(fake_check_run) allow(fake_github_check).to receive(:queued).and_return(fake_check_run) allow(fake_github_check).to receive(:fetch_username).and_return({}) + allow(fake_github_check).to receive(:fetch_username).and_return({}) + allow(fake_github_check).to receive(:check_runs_for_ref).and_return({}) allow(BambooCi::RunningPlan).to receive(:fetch).with(fake_plan_run.bamboo_reference).and_return(ci_jobs) end diff --git a/spec/lib/github/re_run/command_spec.rb b/spec/lib/github/re_run/command_spec.rb index a291fd4..02e0f74 100644 --- a/spec/lib/github/re_run/command_spec.rb +++ b/spec/lib/github/re_run/command_spec.rb @@ -70,6 +70,7 @@ allow(fake_github_check).to receive(:comment_reaction_thumb_up) allow(fake_github_check).to receive(:skipped) allow(fake_github_check).to receive(:fetch_username).and_return({}) + allow(fake_github_check).to receive(:check_runs_for_ref).and_return({}) allow(BambooCi::PlanRun).to receive(:new).and_return(fake_plan_run) allow(fake_plan_run).to receive(:start_plan).and_return(200) diff --git a/spec/lib/github/re_run/comment_spec.rb b/spec/lib/github/re_run/comment_spec.rb index ec63fcb..15e8de5 100644 --- a/spec/lib/github/re_run/comment_spec.rb +++ b/spec/lib/github/re_run/comment_spec.rb @@ -77,6 +77,7 @@ allow(fake_github_check).to receive(:queued) allow(fake_github_check).to receive(:comment_reaction_thumb_up) allow(fake_github_check).to receive(:fetch_username).and_return({}) + allow(fake_github_check).to receive(:check_runs_for_ref).and_return({}) allow(BambooCi::PlanRun).to receive(:new).and_return(fake_plan_run) allow(fake_plan_run).to receive(:start_plan).and_return(200) @@ -126,6 +127,7 @@ allow(fake_github_check).to receive(:in_progress) allow(fake_github_check).to receive(:comment_reaction_thumb_up) allow(fake_github_check).to receive(:fetch_username).and_return({}) + allow(fake_github_check).to receive(:check_runs_for_ref).and_return({}) allow(BambooCi::PlanRun).to receive(:new).and_return(fake_plan_run) allow(fake_plan_run).to receive(:start_plan).and_return(200) @@ -195,6 +197,7 @@ allow(fake_github_check).to receive(:queued) allow(fake_github_check).to receive(:pull_request_info).and_return(pull_request_info) allow(fake_github_check).to receive(:fetch_username).and_return({}) + allow(fake_github_check).to receive(:check_runs_for_ref).and_return({}) allow(BambooCi::PlanRun).to receive(:new).and_return(fake_plan_run) allow(fake_plan_run).to receive(:start_plan).and_return(200) @@ -275,6 +278,7 @@ allow(fake_github_check).to receive(:queued) allow(fake_github_check).to receive(:pull_request_info).and_return(pull_request_info) allow(fake_github_check).to receive(:fetch_username).and_return({}) + allow(fake_github_check).to receive(:check_runs_for_ref).and_return({}) allow(BambooCi::PlanRun).to receive(:new).and_return(fake_plan_run) allow(fake_plan_run).to receive(:start_plan).and_return(200) @@ -309,6 +313,7 @@ allow(fake_github_check).to receive(:queued) allow(fake_github_check).to receive(:pull_request_info).and_return(pull_request_info) allow(fake_github_check).to receive(:fetch_username).and_return({}) + allow(fake_github_check).to receive(:check_runs_for_ref).and_return({}) allow(BambooCi::PlanRun).to receive(:new).and_return(fake_plan_run) allow(fake_plan_run).to receive(:start_plan).and_return(200) From 93b6f00a9d04566ce9531a80622b30fe22d797c2 Mon Sep 17 00:00:00 2001 From: Rodrigo Nardi Date: Wed, 3 Jul 2024 11:14:07 -0300 Subject: [PATCH 2/5] Old Checks support Code lint Signed-off-by: Rodrigo Nardi --- lib/github/build/summary.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/github/build/summary.rb b/lib/github/build/summary.rb index f83cf17..e27b7fa 100644 --- a/lib/github/build/summary.rb +++ b/lib/github/build/summary.rb @@ -73,8 +73,6 @@ def must_update_previous_stage(current_stage) end def must_cancel_next_stages(current_stage) - logger(Logger::INFO, "must_cancel_next_stages: #{current_stage.inspect}") - return unless current_stage.failure? or current_stage.skipped? or current_stage.cancelled? return unless current_stage.configuration.mandatory? From bbbf22f94129c6bfc377f775b2210fb47292b0ea Mon Sep 17 00:00:00 2001 From: Rodrigo Nardi Date: Wed, 3 Jul 2024 11:27:30 -0300 Subject: [PATCH 3/5] Old Checks support Fixing class name Signed-off-by: Rodrigo Nardi --- lib/github/build/action.rb | 2 +- lib/github/build/skip.rb | 36 -------------------------- lib/github/build/skip_old_tests.rb | 41 ++++++++++++++++++++++++++++++ 3 files changed, 42 insertions(+), 37 deletions(-) delete mode 100644 lib/github/build/skip.rb create mode 100644 lib/github/build/skip_old_tests.rb diff --git a/lib/github/build/action.rb b/lib/github/build/action.rb index cb004a6..fe61f2a 100644 --- a/lib/github/build/action.rb +++ b/lib/github/build/action.rb @@ -29,7 +29,7 @@ def initialize(check_suite, github, jobs, logger_level: Logger::INFO) def create_summary(rerun: false) logger(Logger::INFO, "SUMMARY #{@stages.inspect}") - Skip.new(@check_suite).skip_old_tests + Github::Build::SkipOldTests.new(@check_suite).skip_old_tests @stages.each do |stage_config| create_check_run_stage(stage_config) diff --git a/lib/github/build/skip.rb b/lib/github/build/skip.rb deleted file mode 100644 index 03eef38..0000000 --- a/lib/github/build/skip.rb +++ /dev/null @@ -1,36 +0,0 @@ -# SPDX-License-Identifier: BSD-2-Clause -# -# skip.rb -# Part of NetDEF CI System -# -# Copyright (c) 2024 by -# Network Device Education Foundation, Inc. ("NetDEF") -# -# frozen_string_literal: true - -class Skip - def initialize(check_suite) - @check_suite = check_suite - @github = Github::Check.new(@check_suite) - @stages = StageConfiguration.all.map(&:github_check_run_name) - @logger = GithubLogger.instance.create('github_skip_old_tests.log', Logger::INFO) - end - - def skip_old_tests - @github - .check_runs_for_ref(@check_suite.pull_request.repository, @check_suite.commit_sha_ref)[:check_runs] - &.each { |check_run| skipping_old_test(check_run) } - end - - private - - def skipping_old_test(check_run) - return if @stages.include?(check_run[:name]) or check_run[:app][:name] != 'NetDEF CI Hook' - - @logger.info("Skipping old test suite: #{check_run[:name]}") - - message = 'Old test suite, skipping...' - @github.create(check_run[:name]) - @github.skipped(check_run[:id], { title: "#{check_run[:name]} summary", summary: message }) - end -end diff --git a/lib/github/build/skip_old_tests.rb b/lib/github/build/skip_old_tests.rb new file mode 100644 index 0000000..15400c3 --- /dev/null +++ b/lib/github/build/skip_old_tests.rb @@ -0,0 +1,41 @@ +# SPDX-License-Identifier: BSD-2-Clause +# +# skip_old_tests.rb +# Part of NetDEF CI System +# +# Copyright (c) 2024 by +# Network Device Education Foundation, Inc. ("NetDEF") +# +# frozen_string_literal: true + +module Github + module Build + class SkipOldTests + def initialize(check_suite) + @check_suite = check_suite + @github = Github::Check.new(@check_suite) + @stages = StageConfiguration.all.map(&:github_check_run_name) + @logger = GithubLogger.instance.create('github_skip_old_tests.log', Logger::INFO) + end + + def skip_old_tests + @github + .check_runs_for_ref(@check_suite.pull_request.repository, @check_suite.commit_sha_ref)[:check_runs] + &.each { |check_run| skipping_old_test(check_run) } + end + + private + + def skipping_old_test(check_run) + return if @stages.include?(check_run[:name]) or check_run[:app][:name] != 'NetDEF CI Hook' + + @logger.info("Skipping old test suite: #{check_run[:name]}") + + message = 'Old test suite, skipping...' + @github.create(check_run[:name]) + @github.skipped(check_run[:id], { title: "#{check_run[:name]} summary", summary: message }) + end + end + end +end + From 8b9894f76ce4b8f0b33e8ca0a0f6e2faec3c1aaa Mon Sep 17 00:00:00 2001 From: Rodrigo Nardi Date: Wed, 3 Jul 2024 11:28:51 -0300 Subject: [PATCH 4/5] Old Checks support Fixing class name Signed-off-by: Rodrigo Nardi --- lib/github/build/skip_old_tests.rb | 1 - lib/github_ci_app.rb | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/github/build/skip_old_tests.rb b/lib/github/build/skip_old_tests.rb index 15400c3..8d6616b 100644 --- a/lib/github/build/skip_old_tests.rb +++ b/lib/github/build/skip_old_tests.rb @@ -38,4 +38,3 @@ def skipping_old_test(check_run) end end end - diff --git a/lib/github_ci_app.rb b/lib/github_ci_app.rb index 0ea6267..129347d 100644 --- a/lib/github_ci_app.rb +++ b/lib/github_ci_app.rb @@ -28,7 +28,7 @@ require_relative 'github/update_status' require_relative 'github/plan_execution/finished' require_relative 'github/user_info' -require_relative 'github/build/skip' +require_relative 'github/build/skip_old_tests' # Helpers libs require_relative 'helpers/configuration' From baff48380abfc69a1a1f060e3cc4c7fcd8b9d325 Mon Sep 17 00:00:00 2001 From: Rodrigo Nardi Date: Wed, 3 Jul 2024 16:28:30 -0300 Subject: [PATCH 5/5] Old Checks support It is correcting a small behavior of the GitHub API that does not return all checks and sometimes does not return the queued status in the first request. Signed-off-by: Rodrigo Nardi --- lib/github/build/skip_old_tests.rb | 17 +++++++++++------ lib/github/check.rb | 4 ++-- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/lib/github/build/skip_old_tests.rb b/lib/github/build/skip_old_tests.rb index 8d6616b..19efdc3 100644 --- a/lib/github/build/skip_old_tests.rb +++ b/lib/github/build/skip_old_tests.rb @@ -11,28 +11,33 @@ module Github module Build class SkipOldTests + attr_reader :stages + def initialize(check_suite) @check_suite = check_suite @github = Github::Check.new(@check_suite) - @stages = StageConfiguration.all.map(&:github_check_run_name) + @stages = StageConfiguration.all.map { |config| "[CI] #{config.github_check_run_name}" } @logger = GithubLogger.instance.create('github_skip_old_tests.log', Logger::INFO) end def skip_old_tests - @github - .check_runs_for_ref(@check_suite.pull_request.repository, @check_suite.commit_sha_ref)[:check_runs] - &.each { |check_run| skipping_old_test(check_run) } + %w[queued in_progress success failure queued].each do |status| + @github + .check_runs_for_ref(@check_suite.pull_request.repository, + @check_suite.commit_sha_ref, status: status)[:check_runs] + &.each { |check_run| skipping_old_test(check_run) } + end end private def skipping_old_test(check_run) - return if @stages.include?(check_run[:name]) or check_run[:app][:name] != 'NetDEF CI Hook' + return if check_run[:app][:name] != 'NetDEF CI Hook' or @stages.include?(check_run[:name]) @logger.info("Skipping old test suite: #{check_run[:name]}") + puts("Skipping old test suite: #{check_run[:name]}") message = 'Old test suite, skipping...' - @github.create(check_run[:name]) @github.skipped(check_run[:id], { title: "#{check_run[:name]} summary", summary: message }) end end diff --git a/lib/github/check.rb b/lib/github/check.rb index 24fbb35..5649f67 100644 --- a/lib/github/check.rb +++ b/lib/github/check.rb @@ -60,8 +60,8 @@ def comment_reaction_thumb_down(repo, comment_id) accept: Octokit::Preview::PREVIEW_TYPES[:reactions]) end - def check_runs_for_ref(repo, sha) - @app.check_runs_for_ref(repo, sha) + def check_runs_for_ref(repo, sha, status: 'queued') + @app.check_runs_for_ref(repo, sha, status: status, accept: Octokit::Preview::PREVIEW_TYPES[:checks]) end def create(name)