diff --git a/Gemfile.lock b/Gemfile.lock index ee8db6a..b0bbb22 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,7 +1,7 @@ PATH remote: . specs: - danger-lgtm (1.0.2) + danger-lgtm (1.0.3) danger-plugin-api (~> 1.0) GEM diff --git a/lib/lgtm/error_handleable.rb b/lib/lgtm/error_handleable.rb new file mode 100644 index 0000000..1625e31 --- /dev/null +++ b/lib/lgtm/error_handleable.rb @@ -0,0 +1,33 @@ +module Lgtm + # ErrorHandleable is module of error handling + module ErrorHandleable + # 4xx http status. + CLIENT_ERRORS = [ + Net::HTTPBadRequest, + Net::HTTPForbidden, + Net::HTTPNotFound + ].freeze + # 5xx http status. + SERVER_ERRORS = [ + Net::HTTPInternalServerError, + Net::HTTPBadGateway, + Net::HTTPServiceUnavailable, + Net::HTTPGatewayTimeOut + ].freeze + + # validate_response is response validating + # + # @param [Net::HTTPxxx] response Net::HTTP responses + # @raise ::Lgtm::Errors::UnexpectedError + # @return [void] + # + def validate_response(response) + case response + when *SERVER_ERRORS + raise ::Lgtm::Errors::UnexpectedError + when *CLIENT_ERRORS + raise ::Lgtm::Errors::UnexpectedError + end + end + end +end diff --git a/lib/lgtm/errors.rb b/lib/lgtm/errors.rb new file mode 100644 index 0000000..81158ae --- /dev/null +++ b/lib/lgtm/errors.rb @@ -0,0 +1,6 @@ +module Lgtm + class Errors + class UnexpectedError < StandardError + end + end +end diff --git a/lib/lgtm/gem_version.rb b/lib/lgtm/gem_version.rb index 2c6250d..71ffd74 100644 --- a/lib/lgtm/gem_version.rb +++ b/lib/lgtm/gem_version.rb @@ -1,5 +1,5 @@ # frozen_string_literal: true module Lgtm - VERSION = '1.0.2'.freeze + VERSION = '1.0.3'.freeze end diff --git a/lib/lgtm/plugin.rb b/lib/lgtm/plugin.rb index a5f3bc4..1b3fd29 100644 --- a/lib/lgtm/plugin.rb +++ b/lib/lgtm/plugin.rb @@ -3,6 +3,8 @@ require 'uri' require 'json' require 'net/http' +require './lib/lgtm/errors' +require './lib/lgtm/error_handleable' module Danger # Lgtm let danger say lgtm when there is no violations. @@ -16,13 +18,14 @@ module Danger # @tags lgtm, github # class DangerLgtm < Plugin + include ::Lgtm::ErrorHandleable RANDOM_LGTM_POST_URL = 'https://lgtm.in/g'.freeze # Check status report, say lgtm if no violations # Generates a `markdown` of a lgtm image. # - # @param [image_url] lgtm image url - # @param [https_image_only] fetching https image only if true + # @param [String] image_url lgtm image url + # @param [Boolean] https_image_only https image only if true # # @return [void] # @@ -41,7 +44,6 @@ def check_lgtm(image_url: nil, https_image_only: false) # returns "

LGTM

" when ServiceTemporarilyUnavailable. def fetch_image_url(https_image_only: false) - return unless lgtm_post_url lgtm_post_response = process_request(lgtm_post_url) do |req| req['Accept'] = 'application/json' end @@ -53,6 +55,7 @@ def fetch_image_url(https_image_only: false) return fetch_image_url(https_image_only: true) end url + rescue ::Lgtm::Errors::UnexpectedError; nil end def process_request(url) @@ -68,9 +71,9 @@ def process_request(url) end def lgtm_post_url - lgtm_post_req = process_request(RANDOM_LGTM_POST_URL) - return if lgtm_post_req.code == '503' - lgtm_post_req['location'] + res = process_request(RANDOM_LGTM_POST_URL) + validate_response(res) + res['location'] end def markdown_template(image_url) diff --git a/spec/lgtm_spec.rb b/spec/lgtm_spec.rb index 889beb2..04f4369 100644 --- a/spec/lgtm_spec.rb +++ b/spec/lgtm_spec.rb @@ -2,72 +2,73 @@ require File.expand_path('spec_helper', __dir__) -module Danger - describe Danger::DangerLgtm do - it 'should be a plugin' do - expect(Danger::DangerLgtm.new(nil)).to be_a Danger::Plugin - end - - # - # You should test your custom attributes and methods here - # - describe 'with Dangerfile' do - before do - @dangerfile = testing_dangerfile - @lgtm = @dangerfile.lgtm - end +describe Danger::DangerLgtm do + def mock(request_url: 'https://lgtm.in/p/sSuI4hm0q', + actual_image_url: 'https://example.com/image.jpg') + double( + :[] => request_url, + body: JSON.generate( + actualImageUrl: actual_image_url + ) + ) + end - # Some examples for writing tests - # You should replace these with your own. + it 'should be a plugin' do + expect(Danger::DangerLgtm.new(nil)).to be_a(Danger::Plugin) + end - it 'lgtm with no violation' do - @lgtm.check_lgtm - expect(@dangerfile.status_report[:markdowns].length).to eq(1) - end + describe '#check_lgtm' do + subject do + lgtm.check_lgtm( + https_image_only: https_image_only, + image_url: image_url + ) + end - it 'lgtm with default url is OverQuota' do - allow(Net::HTTP).to receive(:start).and_return(mock(code: '503')) + let(:dangerfile) { testing_dangerfile } + let(:lgtm) { dangerfile.lgtm } + let(:https_image_only) { false } # default false + let(:image_url) {} # default nil - @lgtm.check_lgtm + shared_examples 'returns correctly message' do + it do + allow(Net::HTTP).to receive(:start).and_return(mock) + is_expected - expect(@dangerfile.status_report[:markdowns][0].message) - .to eq("

LGTM

") + expect(dangerfile.status_report[:markdowns][0].message) + .to match(expected_message) end + end - def mock(request_url: 'https://lgtm.in/p/sSuI4hm0q', - actual_image_url: 'https://example.com/image.jpg', - code: '302') - double( - :[] => request_url, - body: JSON.generate( - actualImageUrl: actual_image_url - ), - code: code - ) + context 'with Dangerfile' do + it 'when no violation' do + is_expected + expect(dangerfile.status_report[:markdowns].length).to eq(1) end - it 'pick random pic from lgtm.in' do - allow(Net::HTTP).to receive(:start).and_return(mock) - - @lgtm.check_lgtm - - expect(@dangerfile.status_report[:markdowns][0].message) - .to match(%r{https:\/\/example.com\/image.jpg}) + it 'lgtm with errors' do + allow(lgtm).to receive(:validate_response) + .and_raise(::Lgtm::Errors::UnexpectedError) + is_expected + expect(dangerfile.status_report[:markdowns][0].message) + .to eq("

LGTM

") end - it 'pick random pic from lgtm.in with https_image_only option' do - allow(Net::HTTP).to receive(:start).and_return(mock) - - @lgtm.check_lgtm https_image_only: true + context 'pick random pic from lgtm.in' do + let(:expected_message) { %r{https:\/\/example.com\/image.jpg} } + it_behaves_like 'returns correctly message' + end - expect(@dangerfile.status_report[:markdowns][0].message) - .to match(%r{https:\/\/example.com\/image.jpg}) + context 'pick random pic from lgtm.in with https_image_only option' do + let(:https_image_only) { true } + let(:expected_message) { %r{https:\/\/example.com\/image.jpg} } + it_behaves_like 'returns correctly message' end - it 'use given url' do - @lgtm.check_lgtm image_url: 'http://imgur.com/Irk2wyX.jpg' - expect(@dangerfile.status_report[:markdowns][0].message) - .to match(%r{http:\/\/imgur\.com\/Irk2wyX\.jpg}) + context 'use given url' do + let(:image_url) { 'http://imgur.com/Irk2wyX.jpg' } + let(:expected_message) { %r{http:\/\/imgur\.com\/Irk2wyX\.jpg} } + it_behaves_like 'returns correctly message' end end end