From f72e5b0f2682db501af035f88165cf0bb1bf2704 Mon Sep 17 00:00:00 2001 From: Cezary Baginski Date: Sun, 9 Oct 2016 22:59:34 +0200 Subject: [PATCH] rework logger [closes #26] --- lib/ruby_dep/logger.rb | 31 +++++++++ lib/ruby_dep/warning.rb | 9 +-- spec/acceptance/fixtures/nil_logger.rb | 13 ++++ spec/acceptance/fixtures/stdout_logger.rb | 17 +++++ spec/acceptance/warnings_spec.rb | 21 +++++++ spec/lib/ruby_dep/logger_spec.rb | 77 ++++++++++++++++++----- spec/lib/ruby_dep/warning_spec.rb | 37 ++++++----- 7 files changed, 166 insertions(+), 39 deletions(-) create mode 100644 spec/acceptance/fixtures/nil_logger.rb create mode 100644 spec/acceptance/fixtures/stdout_logger.rb diff --git a/lib/ruby_dep/logger.rb b/lib/ruby_dep/logger.rb index 5f906cf..4bfa14f 100644 --- a/lib/ruby_dep/logger.rb +++ b/lib/ruby_dep/logger.rb @@ -1,4 +1,35 @@ +require 'logger' + module RubyDep + def self.logger + @logger ||= stderr_logger + end + + def self.logger=(new_logger) + @logger = new_logger.nil? ? NullLogger.new : new_logger + end + + def self.stderr_logger + ::Logger.new(STDERR).tap do |logger| + logger.formatter = proc { |_,_,_,msg| "#{msg}\n" } + end + end + + # Shamelessly stolen from https://github.com/karafka/null-logger + class NullLogger + LOG_LEVELS = %w(unknown fatal error warn info debug).freeze + + def respond_to_missing?(method_name, include_private = false) + LOG_LEVELS.include?(method_name.to_s) || super + end + + def method_missing(method_name, *args, &block) + LOG_LEVELS.include?(method_name.to_s) ? nil : super + end + end + + # TODO: not used, but kept for the sake of SemVer + # TODO: remove in next major version class Logger def initialize(device, prefix) @device = device diff --git a/lib/ruby_dep/warning.rb b/lib/ruby_dep/warning.rb index 0404ee0..37883b2 100644 --- a/lib/ruby_dep/warning.rb +++ b/lib/ruby_dep/warning.rb @@ -27,7 +27,6 @@ class Warning def initialize @version = RubyVersion.new(RUBY_VERSION, RUBY_ENGINE) - @logger = Logger.new(STDERR, PREFIX) end def show_warnings @@ -53,9 +52,11 @@ def status end def warn_ruby(msg) - @logger.warning(msg) - @logger.notice(recommendation) - @logger.notice(NOTICE_HOW_TO_DISABLE) + RubyDep.logger.tap do |logger| + logger.warn(PREFIX + msg) + logger.info(PREFIX + recommendation) + logger.info(PREFIX + NOTICE_HOW_TO_DISABLE) + end end def recommendation diff --git a/spec/acceptance/fixtures/nil_logger.rb b/spec/acceptance/fixtures/nil_logger.rb new file mode 100644 index 0000000..79afb11 --- /dev/null +++ b/spec/acceptance/fixtures/nil_logger.rb @@ -0,0 +1,13 @@ +require 'ruby_dep/warning' + +# Monkey patch so warning happens on every tested Ruby version +module RubyDep + class RubyVersion + def status + :insecure + end + end +end + +RubyDep.logger = nil +RubyDep::Warning.new.show_warnings diff --git a/spec/acceptance/fixtures/stdout_logger.rb b/spec/acceptance/fixtures/stdout_logger.rb new file mode 100644 index 0000000..d322d20 --- /dev/null +++ b/spec/acceptance/fixtures/stdout_logger.rb @@ -0,0 +1,17 @@ +require 'ruby_dep/warning' + +# Monkey patch so warning happens on every tested Ruby version +module RubyDep + class RubyVersion + def status + :insecure + end + end +end + +RubyDep.logger = Logger.new(STDOUT).tap do |logger| + logger.formatter = proc do |severity,_,_,msg| + "#{severity};" + end +end +RubyDep::Warning.new.show_warnings diff --git a/spec/acceptance/warnings_spec.rb b/spec/acceptance/warnings_spec.rb index e83f2b7..370e437 100644 --- a/spec/acceptance/warnings_spec.rb +++ b/spec/acceptance/warnings_spec.rb @@ -65,4 +65,25 @@ def run_isolated(cmd) end end end + + context 'when a logger is set manually' do + context 'with logger instance' do + let!(:spec) { 'stdout_logger.rb' } + let(:code) do + "o=`#{subcmd}`;"\ + "expected = \"WARN;INFO;INFO;\";" \ + "raise \"Unexpected output: \#{o.inspect}\" unless o == expected" + end + it 'uses the given logger' do + run_isolated(cmd) + end + end + + context 'with nil' do + let!(:spec) { 'nil_logger.rb' } + it 'produces no output' do + run_isolated(cmd) + end + end + end end diff --git a/spec/lib/ruby_dep/logger_spec.rb b/spec/lib/ruby_dep/logger_spec.rb index 9738f3e..f301945 100644 --- a/spec/lib/ruby_dep/logger_spec.rb +++ b/spec/lib/ruby_dep/logger_spec.rb @@ -1,24 +1,69 @@ require 'ruby_dep/logger' -RSpec.describe RubyDep::Logger do - let(:device) { instance_double(IO) } - - describe '#warning' do - context 'with a prefix' do - subject { described_class.new(device, 'foo: ') } - it 'outputs message with prefix' do - expect(device).to receive(:puts).with('foo: bar') - subject.warning('bar') +RSpec.describe RubyDep do + around do |example| + example.run + RubyDep.instance_variable_set(:@logger, nil) + end + + let(:stderr_logger) { instance_double(Logger, 'stderr logger') } + let(:null_logger) { instance_double(RubyDep::NullLogger, 'null logger') } + + let(:string_io) { instance_double(StringIO) } + + before do + allow(StringIO).to receive(:new).and_return(string_io) + allow(RubyDep::NullLogger).to receive(:new).and_return(null_logger) + allow(Logger).to receive(:new).with(STDERR).and_return(stderr_logger) + end + + describe '.logger' do + context 'when not set yet' do + before do + allow(stderr_logger).to receive(:formatter=) + end + + it 'returns stderr_logger' do + expect(described_class.logger).to be(stderr_logger) + end + + it 'sets up a simple formatter' do + expect(stderr_logger).to receive(:formatter=) do |callback| + expect( callback.call('a', 'b', 'c', 'd')).to eq("d\n") + end + described_class.logger + end + + context 'when reset to nil' do + before { described_class.logger = nil } + + it 'returns null logger' do + expect(described_class.logger).to be(null_logger) + end + end + + context 'when set to a logger' do + let(:logger) { instance_double(Logger) } + before { described_class.logger = logger } + + it 'returns given logger' do + expect(described_class.logger).to be(logger) + end end end - end - describe '#notice' do - context 'with a prefix' do - subject { described_class.new(device, 'foo: ') } - it 'outputs message with prefix' do - expect(device).to receive(:puts).with('foo: bar') - subject.notice('bar') + context 'when already set' do + context 'with a custom logger' do + let(:logger) { instance_double(Logger) } + before { described_class.logger = logger } + + context 'when reset' do + before { described_class.logger = nil } + + it 'outputs to null logger' do + expect(described_class.logger).to be(null_logger) + end + end end end end diff --git a/spec/lib/ruby_dep/warning_spec.rb b/spec/lib/ruby_dep/warning_spec.rb index f726264..f6c2c5d 100644 --- a/spec/lib/ruby_dep/warning_spec.rb +++ b/spec/lib/ruby_dep/warning_spec.rb @@ -1,7 +1,7 @@ require 'ruby_dep/warning' RSpec.describe RubyDep::Warning do - let(:logger) { instance_double(RubyDep::Logger) } + let(:logger) { instance_double(Logger) } let(:outdated_ruby) { instance_double(RubyDep::RubyVersion) } let(:up_to_date_ruby) { instance_double(RubyDep::RubyVersion) } @@ -11,10 +11,9 @@ let(:untracked_engine_ruby) { instance_double(RubyDep::RubyVersion) } before do - allow(RubyDep::Logger).to receive(:new) - .with(STDERR, 'RubyDep: WARNING: ').and_return(logger) - allow(logger).to receive(:warning) - allow(logger).to receive(:notice) + allow(RubyDep).to receive(:logger).and_return(logger) + allow(logger).to receive(:warn) + allow(logger).to receive(:info) allow(RubyDep::RubyVersion).to receive(:new) .with(RUBY_VERSION, RUBY_ENGINE).and_return(ruby_version) @@ -74,8 +73,8 @@ def rquote(str) context 'with any outdated Ruby' do let(:ruby_version) { outdated_ruby } it 'does not show anything' do - expect(logger).to_not have_received(:warning) - expect(logger).to_not have_received(:notice) + expect(logger).to_not have_received(:warn) + expect(logger).to_not have_received(:info) end end end @@ -91,8 +90,8 @@ def rquote(str) context 'with any outdated Ruby' do let(:ruby_version) { outdated_ruby } it 'does not show anything' do - expect(logger).to_not have_received(:warning) - expect(logger).to_not have_received(:notice) + expect(logger).to_not have_received(:warn) + expect(logger).to_not have_received(:info) end end end @@ -102,15 +101,15 @@ def rquote(str) context 'with an up-to-date Ruby' do let(:ruby_version) { up_to_date_ruby } it 'does not show anything' do - expect(logger).to_not have_received(:warning) - expect(logger).to_not have_received(:notice) + expect(logger).to_not have_received(:warn) + expect(logger).to_not have_received(:info) end end context 'with a secure but buggy Ruby' do let(:ruby_version) { buggy_ruby } it 'shows warning about bugs' do - expect(logger).to have_received(:warning).with( + expect(logger).to have_received(:warn).with( %r{Your Ruby is outdated\/buggy.}) end @@ -118,14 +117,14 @@ def rquote(str) expected = rquote( 'Your Ruby is: 2.2.4 (buggy). Recommendation: upgrade to'\ ' 2.2.5 or 2.3.1') - expect(logger).to have_received(:notice).with(expected) + expect(logger).to have_received(:info).with(expected) end end context 'with an insecure Ruby' do let(:ruby_version) { insecure_ruby } it 'shows warning about vulnerability' do - expect(logger).to have_received(:warning).with( + expect(logger).to have_received(:warn).with( /Your Ruby has security vulnerabilities!/) end @@ -133,14 +132,14 @@ def rquote(str) expected = rquote( 'Your Ruby is: 2.2.3 (insecure). Recommendation:'\ ' upgrade to 2.2.5 or 2.3.1. (Or, at least to 2.2.4 or 2.3.0)') - expect(logger).to have_received(:notice).with(expected) + expect(logger).to have_received(:info).with(expected) end end context 'with an unsupported Ruby' do let(:ruby_version) { unsupported_ruby } it 'shows warning about vulnerability' do - expect(logger).to have_received(:warning).with( + expect(logger).to have_received(:warn).with( /Your Ruby has security vulnerabilities!/) end @@ -148,7 +147,7 @@ def rquote(str) expected = rquote( 'Your Ruby is: 1.9.3 (insecure). Recommendation: upgrade to 2.2.5'\ ' or 2.3.1. (Or, at least to 2.1.9 or 2.2.4 or 2.3.0)') - expect(logger).to have_received(:notice).with(expected) + expect(logger).to have_received(:info).with(expected) end end @@ -156,7 +155,7 @@ def rquote(str) context 'when the Ruby is not listed' do let(:ruby_version) { untracked_engine_ruby } it 'shows warning about lack of support' do - expect(logger).to have_received(:warning).with( + expect(logger).to have_received(:warn).with( /Your Ruby may not be supported./) end @@ -165,7 +164,7 @@ def rquote(str) "Your Ruby is: 1.2.3 'ironruby' (unrecognized). If you need"\ ' this version supported, please open an issue at'\ ' http://github.com/e2/ruby_dep') - expect(logger).to have_received(:notice).with(expected) + expect(logger).to have_received(:info).with(expected) end end end