From b9a599171f3fda2affa9381d998e2158a2bf7fac Mon Sep 17 00:00:00 2001 From: Daniil Kouznetsov Date: Thu, 11 Jan 2018 11:22:51 -0500 Subject: [PATCH] [Added] Specify log directory for prepare [finish #154254782] Signed-off-by: Ryan Collins --- .rubocop.yml | 2 +- lib/license_finder/cli/base.rb | 3 ++ lib/license_finder/configuration.rb | 5 +++ lib/license_finder/core.rb | 6 +++ lib/license_finder/package_manager.rb | 9 ++-- lib/license_finder/scanner.rb | 12 ++--- spec/lib/license_finder/cli/main_spec.rb | 4 +- spec/lib/license_finder/configuration_spec.rb | 38 ++++++++++++++++ .../license_finder/package_manager_spec.rb | 45 ++++++++++++------- 9 files changed, 94 insertions(+), 30 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index a8427bef0..dc653354f 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -10,7 +10,7 @@ AmbiguousRegexpLiteral: # Metrics Cops AbcSize: - Max: 30 + Max: 35 BlockLength: Enabled: false # TODO: enable and refactor long methods # ExcludedMethods: ['describe', 'context', 'it', 'shared_examples'] # uncomment once enabled diff --git a/lib/license_finder/cli/base.rb b/lib/license_finder/cli/base.rb index 559d06b10..8ddf1b5b8 100644 --- a/lib/license_finder/cli/base.rb +++ b/lib/license_finder/cli/base.rb @@ -7,6 +7,8 @@ class Base < Thor desc: 'Path to the project. Defaults to current working directory.' class_option :decisions_file, desc: 'Where decisions are saved. Defaults to doc/dependency_decisions.yml.' + class_option :log_directory, + desc: 'Where logs are saved. Defaults to ./lf_logs/$PROJECT/$PACKAGE_MANAGER.log' no_commands do def decisions @@ -41,6 +43,7 @@ def license_finder_config :save, :prepare, :prepare_no_fail, + :log_directory, :format, :columns, :aggregate_paths, diff --git a/lib/license_finder/configuration.rb b/lib/license_finder/configuration.rb index 70b60bffe..1a37fe3f2 100644 --- a/lib/license_finder/configuration.rb +++ b/lib/license_finder/configuration.rb @@ -42,6 +42,11 @@ def decisions_file_path project_path.join(path).expand_path end + def log_directory + path = get(:log_directory) || 'lf_logs' + project_path.join(path).expand_path + end + def project_path Pathname(path_prefix).expand_path end diff --git a/lib/license_finder/core.rb b/lib/license_finder/core.rb index d2d6b4401..21dcb382f 100644 --- a/lib/license_finder/core.rb +++ b/lib/license_finder/core.rb @@ -55,6 +55,7 @@ def decisions end def prepare_projects + clear_logs package_managers = @scanner.active_package_managers package_managers.each do |manager| logger.debug manager.class, 'Running prepare on project' @@ -80,10 +81,15 @@ def current_packages @scanner.active_packages end + def clear_logs + FileUtils.rm config.log_directory if File.directory? config.log_directory + end + def options { logger: logger, project_path: config.project_path, + log_directory: File.join(config.log_directory, project_name), ignored_groups: decisions.ignored_groups, go_full_version: config.go_full_version, gradle_command: config.gradle_command, diff --git a/lib/license_finder/package_manager.rb b/lib/license_finder/package_manager.rb index e0632108a..1a60f8db7 100644 --- a/lib/license_finder/package_manager.rb +++ b/lib/license_finder/package_manager.rb @@ -60,6 +60,7 @@ def initialize(options = {}) @prepare_no_fail = options[:prepare_no_fail] @logger = options[:logger] || Core.default_logger @project_path = options[:project_path] + @log_directory = options[:log_directory] end def active? @@ -111,11 +112,9 @@ def log_errors(stderr) end def log_to_file(contents) - log_dir = File.join(@project_path, 'lf_logs') - log_file_path = File.join(log_dir, 'error.log') - - FileUtils.mkdir_p(log_dir) - File.open(log_file_path, 'w') do |f| + FileUtils.mkdir_p @log_directory + log_file = File.join(@log_directory, "prepare_#{self.class.package_management_command || 'errors'}.log") + File.open(log_file, 'w') do |f| f.write("Prepare command \"#{self.class.prepare_command}\" failed with:\n") f.write("#{contents}\n\n") end diff --git a/lib/license_finder/scanner.rb b/lib/license_finder/scanner.rb index d42ffd89d..0305f65bd 100644 --- a/lib/license_finder/scanner.rb +++ b/lib/license_finder/scanner.rb @@ -3,10 +3,10 @@ class Scanner PACKAGE_MANAGERS = [GoDep, GoWorkspace, Go15VendorExperiment, Glide, Gvt, Govendor, Dep, Bundler, NPM, Pip, Yarn, Bower, Maven, Gradle, CocoaPods, Rebar, Nuget, Carthage, Mix, Conan].freeze - def initialize(options = { project_path: Pathname.new('') }) - @options = options - @project_path = options[:project_path] - @logger = options[:logger] + def initialize(config = { project_path: Pathname.new('') }) + @config = config + @project_path = @config[:project_path] + @logger = @config[:logger] end def active_packages @@ -20,7 +20,7 @@ def active_package_managers active_pm_classes = [] PACKAGE_MANAGERS.each do |pm_class| - active = pm_class.new(@options).active? + active = pm_class.new(@config).active? if active @logger.info pm_class, 'is active', color: :green active_pm_classes << pm_class @@ -32,7 +32,7 @@ def active_package_managers @logger.info 'License Finder', 'No active and installed package managers found for project.', color: :red if active_pm_classes.empty? active_pm_classes -= active_pm_classes.map(&:takes_priority_over) - @package_managers = active_pm_classes.map { |pm_class| pm_class.new(@options) } + @package_managers = active_pm_classes.map { |pm_class| pm_class.new(@config) } end end end diff --git a/spec/lib/license_finder/cli/main_spec.rb b/spec/lib/license_finder/cli/main_spec.rb index 324bce7d9..15ebc658d 100644 --- a/spec/lib/license_finder/cli/main_spec.rb +++ b/spec/lib/license_finder/cli/main_spec.rb @@ -63,7 +63,8 @@ module CLI '--rebar_deps_dir=rebar_dir', '--mix_command=surprise_me', '--mix_deps_dir=mix_dir', - '--prepare' + '--prepare', + '--log_directory=some_logs' ] end @@ -83,6 +84,7 @@ module CLI mix_command: 'surprise_me', mix_deps_dir: 'mix_dir', prepare: true, + log_directory: 'some_logs', logger: { mode: LicenseFinder::Logger::MODE_INFO } } end diff --git a/spec/lib/license_finder/configuration_spec.rb b/spec/lib/license_finder/configuration_spec.rb index cd9b34334..9c371c61f 100644 --- a/spec/lib/license_finder/configuration_spec.rb +++ b/spec/lib/license_finder/configuration_spec.rb @@ -87,6 +87,44 @@ module LicenseFinder end end + describe 'log_directory' do + it 'prefers primary value' do + subject = described_class.new( + { log_directory: 'primary' }, + 'log_directory' => 'secondary' + ) + expect(subject.log_directory.to_s).to end_with 'primary' + end + + it 'accepts saved value' do + subject = described_class.new( + { log_directory: nil }, + 'log_directory' => 'secondary' + ) + expect(subject.log_directory.to_s).to end_with 'secondary' + end + + it 'has default' do + subject = described_class.new( + { log_directory: nil }, + 'log_directory' => nil + ) + expect(subject.log_directory.to_s).to end_with 'lf_logs' + end + + it 'prepends project path to default path if project_path option is set' do + subject = described_class.new({ project_path: 'magic_path' }, {}) + expect(subject.log_directory.to_s).to end_with 'magic_path/lf_logs' + end + + it 'prepends project path to provided value' do + subject = described_class.new({ log_directory: 'primary', + project_path: 'magic_path' }, + 'log_directory' => 'secondary') + expect(subject.log_directory.to_s).to end_with 'magic_path/primary' + end + end + describe 'rebar_deps_dir' do it 'has default' do subject = described_class.new( diff --git a/spec/lib/license_finder/package_manager_spec.rb b/spec/lib/license_finder/package_manager_spec.rb index 9d299dd5c..b4a21b53c 100644 --- a/spec/lib/license_finder/package_manager_spec.rb +++ b/spec/lib/license_finder/package_manager_spec.rb @@ -73,18 +73,23 @@ module LicenseFinder describe '#prepare' do context 'when there is a prepare_command' do + let(:prepare_error) { /Prepare command .* failed/ } + let(:project_path) { '/path/to/project' } + let(:log_directory) { '/path/to/project/logs/project' } + let(:log_file_path) { File.join(log_directory, 'prepare_errors.log') } + let(:quiet_logger) { double(:logger) + let(:subject) { described_class.new logger: quiet_logger, project_path: project_path, log_directory: log_directory } + before do FakeFS.activate! allow(described_class).to receive(:prepare_command).and_return('sh commands') end after do + FakeFS.clear! FakeFS.deactivate! end - let(:prepare_error) { /Prepare command .* failed/ } - let(:project_path) { '/path/to/project' } - it 'succeeds when prepare command runs successfully' do expect(SharedHelpers::Cmd).to receive(:run).with('sh commands').and_return(['output', nil, cmd_success]) @@ -92,40 +97,46 @@ module LicenseFinder end it 'logs warning and exception when prepare command runs into failure' do - logger = double(:logger) expect(SharedHelpers::Cmd).to receive(:run).with('sh commands').and_return(['output', 'failure error msg', cmd_failure]) - expect(logger).to receive(:info).with('sh commands', 'did not succeed.', color: :red) - expect(logger).to receive(:info).with('sh commands', 'failure error msg', color: :red) - subject = described_class.new logger: logger, project_path: project_path + expect(quiet_logger).to receive(:info).with('sh commands', 'did not succeed.', color: :red) + expect(quiet_logger).to receive(:info).with('sh commands', 'failure error msg', color: :red) expect { subject.prepare }.to raise_error(prepare_error) end describe 'logging prepare errors into log file' do - let(:subject) { described_class.new logger: logger, project_path: project_path } + let(:subject) { described_class.new logger: logger, project_path: project_path, log_directory: log_directory } let(:error_msg) { "Prepare command \"sh commands\" failed with:\nfailure error msg\n\n" } - let(:log_dir_path) { File.join(project_path, 'lf_logs') } - let(:log_path) { File.join(log_dir_path, 'error.log') } - before do expect(SharedHelpers::Cmd).to receive(:run).with('sh commands').and_return(['output', 'failure error msg', cmd_failure]) end - it 'logs package manager failure messages in lf_logs/error.log inside project path' do + it 'logs package manager failure messages in log file for package manager' do expect { subject.prepare }.to raise_error(prepare_error) - expect(File.read(log_path)).to eq error_msg + expect(File.read(log_file_path)).to eq error_msg end it 'discards previous logs and starts logging new logs' do - FileUtils.mkdir_p log_dir_path - File.open(log_path, 'w') { |f| f.write('previous logs') } + FileUtils.mkdir_p log_directory + File.open(log_file_path, 'w') { |f| f.write('previous logs') } + expect { subject.prepare }.to raise_error(prepare_error) + expect(File.read(log_file_path)).to eq error_msg + end + + it 'uses default package_management_command as the file name' do + expect { subject.prepare }.to raise_error(prepare_error) + expect(File.exists? log_file_path) + end + + it 'uses defined package_management_command as the file name' do + allow(LicenseFinder::PackageManager).to receive(:package_management_command).and_return('foobar') expect { subject.prepare }.to raise_error(prepare_error) - expect(File.read(log_path)).to eq error_msg + expect(File.exists? File.join(log_directory, 'prepare_foobar.log')).to be_truthy end end context 'with prepare_no_fail' do - let(:subject) { described_class.new logger: logger, prepare_no_fail: true, project_path: project_path } + let(:subject) { described_class.new logger: logger, prepare_no_fail: true, project_path: project_path, log_directory: log_directory } it 'should not throw an error when prepare_command fails' do expect(SharedHelpers::Cmd).to receive(:run).with('sh commands')