From 65df934e7afb0ea02dbd674b1d591be371c70c31 Mon Sep 17 00:00:00 2001 From: GordonDiggs Date: Mon, 20 Apr 2015 11:16:47 -0400 Subject: [PATCH 1/2] Remove rollout call for quality stats --- .../github_pull_requests_presenter.rb | 2 +- test/github_pull_requests_test.rb | 4 +++ .../github_pull_requests_presenter_test.rb | 28 +++++++++---------- 3 files changed, 19 insertions(+), 15 deletions(-) diff --git a/lib/cc/presenters/github_pull_requests_presenter.rb b/lib/cc/presenters/github_pull_requests_presenter.rb index bb2830a..8d6ab8d 100644 --- a/lib/cc/presenters/github_pull_requests_presenter.rb +++ b/lib/cc/presenters/github_pull_requests_presenter.rb @@ -13,7 +13,7 @@ def initialize(payload, repo_config) end def success_message - if @repo_config.pr_status_quality_stats? + if issue_counts_in_payload? if both_issue_counts_zero? "Code Climate didn't find any new or fixed issues." else diff --git a/test/github_pull_requests_test.rb b/test/github_pull_requests_test.rb index 1bf5583..502829a 100644 --- a/test/github_pull_requests_test.rb +++ b/test/github_pull_requests_test.rb @@ -179,6 +179,10 @@ def test_pull_request_comment number: 1, state: "success", compare_url: "http://example.com", + issue_comparison_counts: { + "fixed" => 2, + "new" => 1, + } }) end diff --git a/test/presenters/github_pull_requests_presenter_test.rb b/test/presenters/github_pull_requests_presenter_test.rb index 6ea8ab9..7dfd943 100644 --- a/test/presenters/github_pull_requests_presenter_test.rb +++ b/test/presenters/github_pull_requests_presenter_test.rb @@ -2,45 +2,45 @@ require "cc/presenters/github_pull_requests_presenter" class TestGitHubPullRequestsPresenter < CC::Service::TestCase - def test_message_quality_stats_not_enabled - assert_equal( - "Code Climate has analyzed this pull request.", - build_presenter(false, "fixed" => 1, "new" => 1).success_message - ) - end - def test_message_singular assert_equal( "Code Climate found 1 new issue and 1 fixed issue.", - build_presenter(true, "fixed" => 1, "new" => 1).success_message + build_presenter("fixed" => 1, "new" => 1).success_message ) end def test_message_plural assert_equal( "Code Climate found 2 new issues and 1 fixed issue.", - build_presenter(true, "fixed" => 1, "new" => 2).success_message + build_presenter("fixed" => 1, "new" => 2).success_message ) end def test_message_only_fixed assert_equal( "Code Climate found 1 fixed issue.", - build_presenter(true, "fixed" => 1, "new" => 0).success_message + build_presenter("fixed" => 1, "new" => 0).success_message ) end def test_message_only_new assert_equal( "Code Climate found 3 new issues.", - build_presenter(true, "fixed" => 0, "new" => 3).success_message + build_presenter("fixed" => 0, "new" => 3).success_message ) end def test_message_no_new_or_fixed assert_equal( "Code Climate didn't find any new or fixed issues.", - build_presenter(true, "fixed" => 0, "new" => 0).success_message + build_presenter("fixed" => 0, "new" => 0).success_message + ) + end + + def test_message_no_issue_counts + assert_equal( + "Code Climate has analyzed this pull request.", + build_presenter({}).success_message ) end @@ -50,10 +50,10 @@ def build_payload(issue_counts) { "issue_comparison_counts" => issue_counts } end - def build_presenter(quality_stats_enabled, issue_counts) + def build_presenter(issue_counts) CC::Service::GitHubPullRequestsPresenter.new( build_payload(issue_counts), - OpenStruct.new(pr_status_quality_stats?: quality_stats_enabled) + OpenStruct.new ) end end From 7a2610cdb02de3f1184eb0b38bcb7969ef2a7ff6 Mon Sep 17 00:00:00 2001 From: GordonDiggs Date: Mon, 20 Apr 2015 15:38:02 -0400 Subject: [PATCH 2/2] Remove check for empty counts --- .../presenters/github_pull_requests_presenter.rb | 14 +++----------- test/github_pull_requests_test.rb | 14 +++++--------- .../github_pull_requests_presenter_test.rb | 7 ------- 3 files changed, 8 insertions(+), 27 deletions(-) diff --git a/lib/cc/presenters/github_pull_requests_presenter.rb b/lib/cc/presenters/github_pull_requests_presenter.rb index 8d6ab8d..af85178 100644 --- a/lib/cc/presenters/github_pull_requests_presenter.rb +++ b/lib/cc/presenters/github_pull_requests_presenter.rb @@ -13,14 +13,10 @@ def initialize(payload, repo_config) end def success_message - if issue_counts_in_payload? - if both_issue_counts_zero? - "Code Climate didn't find any new or fixed issues." - else - "Code Climate found #{formatted_issue_counts}." - end + if both_issue_counts_zero? + "Code Climate didn't find any new or fixed issues." else - "Code Climate has analyzed this pull request." + "Code Climate found #{formatted_issue_counts}." end end @@ -53,10 +49,6 @@ def formatted_issue_counts def issue_counts [@new_count, @fixed_count] end - - def issue_counts_in_payload? - issue_counts.all? - end end end end diff --git a/test/github_pull_requests_test.rb b/test/github_pull_requests_test.rb index 502829a..4d298db 100644 --- a/test/github_pull_requests_test.rb +++ b/test/github_pull_requests_test.rb @@ -17,7 +17,7 @@ def test_pull_request_status_pending def test_pull_request_status_success_detailed expect_status_update("pbrisbin/foo", "abc123", { "state" => "success", - "description" => "Code Climate found 1 new issue and 2 fixed issues.", + "description" => "Code Climate found 2 new issues and 1 fixed issue.", }) receive_pull_request( @@ -25,11 +25,7 @@ def test_pull_request_status_success_detailed { github_slug: "pbrisbin/foo", commit_sha: "abc123", - state: "success", - issue_comparison_counts: { - "fixed" => 2, - "new" => 1, - } + state: "success" }, true ) @@ -38,7 +34,7 @@ def test_pull_request_status_success_detailed def test_pull_request_status_success_generic expect_status_update("pbrisbin/foo", "abc123", { "state" => "success", - "description" => /has analyzed/, + "description" => /found 2 new issues and 1 fixed issue/, }) receive_pull_request({ update_status: true }, { @@ -252,7 +248,7 @@ def receive_pull_request(config, event_data, truthy_repo_config = false) receive( CC::Service::GitHubPullRequests, { oauth_token: "123" }.merge(config), - { name: "pull_request" }.merge(event_data), + { name: "pull_request", issue_comparison_counts: {'fixed' => 1, 'new' => 2} }.merge(event_data), truthy_repo_config ? TruthyRepoConfig.new : FalsyRepoConfig.new ) end @@ -261,7 +257,7 @@ def receive_test(config, event_data = {}) receive( CC::Service::GitHubPullRequests, { oauth_token: "123" }.merge(config), - { name: "test" }.merge(event_data) + { name: "test", issue_comparison_counts: {'fixed' => 1, 'new' => 2} }.merge(event_data) ) end end diff --git a/test/presenters/github_pull_requests_presenter_test.rb b/test/presenters/github_pull_requests_presenter_test.rb index 7dfd943..a3b9d24 100644 --- a/test/presenters/github_pull_requests_presenter_test.rb +++ b/test/presenters/github_pull_requests_presenter_test.rb @@ -37,13 +37,6 @@ def test_message_no_new_or_fixed ) end - def test_message_no_issue_counts - assert_equal( - "Code Climate has analyzed this pull request.", - build_presenter({}).success_message - ) - end - private def build_payload(issue_counts)