From 39427c35c373096d5bc76046e6094092798e1391 Mon Sep 17 00:00:00 2001 From: Thibault Jouan Date: Sun, 25 May 2014 17:44:24 +0000 Subject: [PATCH] Refactor and simplify CLI and Env: * Do not modify Env logger from CLI; * Add verbose attribute to Env; * Implement Env#verbose?; * Remove Env#log_level and Env#log_level=; * Refactor related specs, improve some wording. --- lib/producer/core/cli.rb | 2 +- lib/producer/core/env.rb | 16 +++---- spec/producer/core/cli_spec.rb | 6 +-- spec/producer/core/env_spec.rb | 77 ++++++++++++++++++---------------- 4 files changed, 51 insertions(+), 50 deletions(-) diff --git a/lib/producer/core/cli.rb b/lib/producer/core/cli.rb index 947a722..3d558c8 100644 --- a/lib/producer/core/cli.rb +++ b/lib/producer/core/cli.rb @@ -36,7 +36,7 @@ module Producer @arguments = arguments.inject([]) do |m, e| case e when '-v' - env.log_level = Logger::INFO + env.verbose = true when '-n' env.dry_run = true else diff --git a/lib/producer/core/env.rb b/lib/producer/core/env.rb index 99a7f78..660fe4a 100644 --- a/lib/producer/core/env.rb +++ b/lib/producer/core/env.rb @@ -2,14 +2,14 @@ module Producer module Core class Env attr_reader :input, :output, :registry, :logger - attr_accessor :target, :dry_run + attr_accessor :target, :verbose, :dry_run def initialize(input: $stdin, output: $stdout, remote: nil, registry: {}) + @verbose = @dry_run = false @input = input @output = output - @registry = registry @remote = remote - @dry_run = false + @registry = registry end def remote @@ -27,7 +27,7 @@ module Producer def logger @logger ||= begin logger = Logger.new(output) - logger.level = Logger::ERROR + logger.level = verbose? ? Logger::INFO : Logger::ERROR logger.formatter = LoggerFormatter.new logger end @@ -37,12 +37,8 @@ module Producer logger.info message end - def log_level - logger.level - end - - def log_level=(level) - logger.level = level + def verbose? + @verbose end def dry_run? diff --git a/spec/producer/core/cli_spec.rb b/spec/producer/core/cli_spec.rb index e837102..c99ba30 100644 --- a/spec/producer/core/cli_spec.rb +++ b/spec/producer/core/cli_spec.rb @@ -108,15 +108,15 @@ module Producer::Core end context 'verbose' do - it 'sets env logger level to INFO' do - expect(cli.env.log_level).to eq Logger::INFO + it 'enables env verbose mode' do + expect(cli.env).to be_verbose end end context 'dry run' do let(:options) { %w[-n] } - it 'enables env dry run' do + it 'enables env dry run mode' do expect(cli.env).to be_dry_run end end diff --git a/spec/producer/core/env_spec.rb b/spec/producer/core/env_spec.rb index fbee8fe..b75c2d9 100644 --- a/spec/producer/core/env_spec.rb +++ b/spec/producer/core/env_spec.rb @@ -3,10 +3,10 @@ require 'spec_helper' module Producer::Core describe Env do let(:output) { StringIO.new } - subject(:env) { Env.new(output: output) } + subject(:env) { described_class.new(output: output) } describe '#initialize' do - it 'assigns $stdin as the default output' do + it 'assigns $stdin as the default input' do expect(env.input).to be $stdin end @@ -18,12 +18,16 @@ module Producer::Core expect(env.registry).to be_empty end + it 'assigns verbose as false' do + expect(env.verbose).to be false + end + it 'assigns dry run as false' do expect(env.dry_run).to be false end context 'when output is not given as argument' do - subject(:env) { Env.new } + subject(:env) { described_class.new } it 'assigns $stdout as the default output' do expect(env.output).to be $stdout @@ -60,31 +64,12 @@ module Producer::Core describe '#target' do let(:target) { double 'target' } - it 'returns the defined target' do + it 'returns the assigned target' do env.target = target expect(env.target).to be target end end - describe '#logger' do - it 'returns a logger' do - expect(env.logger).to be_a Logger - end - - it 'uses env output' do - env.logger.error 'some message' - expect(output.string).to include 'some message' - end - - it 'has a log level of ERROR' do - expect(env.log_level).to eq Logger::ERROR - end - - it 'uses our formatter' do - expect(env.logger.formatter).to be_a LoggerFormatter - end - end - describe '#remote' do it 'builds a Remote with the current target' do env.target = 'some_hostname.example' @@ -118,6 +103,33 @@ module Producer::Core end end + describe '#logger' do + it 'returns a logger' do + expect(env.logger).to be_a Logger + end + + it 'uses env output' do + env.logger.error 'some message' + expect(output.string).to include 'some message' + end + + it 'has a log level of ERROR' do + expect(env.logger.level).to eq Logger::ERROR + end + + it 'uses our formatter' do + expect(env.logger.formatter).to be_a LoggerFormatter + end + + context 'when verbose mode is enabled' do + before { env.verbose = true } + + it 'has a log level of INFO' do + expect(env.logger.level).to eq Logger::INFO + end + end + end + describe '#log' do it 'logs an info message through the assigned logger' do expect(env.logger).to receive(:info).with 'message' @@ -125,24 +137,17 @@ module Producer::Core end end - describe '#log_level' do - it 'returns the logger level' do - expect(env.log_level).to eq env.logger.level - end - end - - describe '#log_level=' do - it 'sets the logger level' do - env.log_level = Logger::DEBUG - expect(env.logger.level).to eq Logger::DEBUG + describe '#verbose?' do + it 'returns true when verbose is enabled' do + env.verbose = true + expect(env).to be_verbose end end describe '#dry_run?' do - before { env.dry_run = true } - it 'returns true when dry run is enabled' do - expect(env.dry_run?).to be true + env.dry_run = true + expect(env).to be_dry_run end end end