From 63de321521d06fd18337435758a66663144af2b7 Mon Sep 17 00:00:00 2001 From: Fabio Pelosin Date: Tue, 13 May 2014 15:02:34 +0200 Subject: [PATCH 1/2] [Command] Print warning about exceptions Closes https://github.com/CocoaPods/CLAide/issues/4 --- lib/claide/command.rb | 4 + lib/claide/command/options.rb | 5 +- lib/claide/command/plugins_helper.rb | 106 ++++++++++++++------------- spec/command/options_spec.rb | 7 +- spec/command/plugins_helper_spec.rb | 103 ++++++++++++++++---------- 5 files changed, 130 insertions(+), 95 deletions(-) diff --git a/lib/claide/command.rb b/lib/claide/command.rb index 607e4db..4740a0d 100644 --- a/lib/claide/command.rb +++ b/lib/claide/command.rb @@ -316,6 +316,10 @@ def self.handle_exception(command, exception) # @return [void] # def self.report_error(exception) + plugins = PluginsHelper.plugins_involved_in_exception(exception) + puts '[!] The exceptions involves the following plugins:' \ + "\n - #{plugins.join("\n - ")}\n".ansi.yellow + raise exception end diff --git a/lib/claide/command/options.rb b/lib/claide/command/options.rb index 2af2861..183a484 100644 --- a/lib/claide/command/options.rb +++ b/lib/claide/command/options.rb @@ -67,9 +67,8 @@ def self.handle_root_option(command, argv) def self.print_version(command) puts command.class.version if command.verbose? - prefix = command.class.plugin_prefix - PluginsHelper.plugin_load_paths(prefix).each do |path| - puts PluginsHelper.plugin_info(path) + PluginsHelper.specifications.each do |spec| + puts "#{spec.name}: #{spec.version}" end end end diff --git a/lib/claide/command/plugins_helper.rb b/lib/claide/command/plugins_helper.rb index da1e2c8..e14902d 100644 --- a/lib/claide/command/plugins_helper.rb +++ b/lib/claide/command/plugins_helper.rb @@ -2,72 +2,77 @@ module CLAide class Command - module PluginsHelper - # Loads additional plugins via rubygems looking for files named after the - # `PLUGIN_PREFIX_plugin`. + # Handles plugin related logic logic for the `Command` class. + # + # Plugins are loaded the first time a command run and are identified by the + # prefix specified in the command class. Plugins must adopt the following + # conventions: + # + # - Support being loaded by a file located under the + # `lib/#{plugin_prefix}_plugin` relative path. + # - Be stored in a folder named after the plugin. + # + class PluginsHelper + class << self + # @return [Array] The list of the root directories of the + # loaded plugins. + # + attr_reader :plugin_paths + end + + # @return [Array] Loads plugins via RubyGems looking for files + # named after the `PLUGIN_PREFIX_plugin` and returns the paths of + # the gems loaded successfully. Plugins are required safely. # def self.load_plugins(plugin_prefix) + return if plugin_paths paths = PluginsHelper.plugin_load_paths(plugin_prefix) - loaded_paths = [] + plugin_paths = [] paths.each do |path| - if PluginsHelper.safe_require(path) - loaded_paths << path + if PluginsHelper.safe_require(path.to_s) + plugin_paths << Pathname(path + './../../').cleanpath end end - loaded_paths + + @plugin_paths = plugin_paths end - # Returns the name and the version of the plugin with the given path. - # - # @param [String] path - # The load path of the plugin. - # - # @return [String] A string including the name and the version or a - # failure message. + # @return [Array] The RubyGems specifications for the + # loaded plugins. # - def self.plugin_info(path) - if gemspec = find_gemspec(path) - spec = Gem::Specification.load(gemspec) - end - - if spec - "#{spec.name}: #{spec.version}" - else - "[!] Unable to load a specification for `#{path}`" - end + def self.specifications + PluginsHelper.plugin_paths.map do |path| + specification(path) + end.compact end - # @return [String] Finds the path of the gemspec of a path. The path is - # iterated upwards until a dir with a single gemspec is found. + # @return [Array] The RubyGems specifications for the + # plugin with the given root path. # - # @param [String] path - # The load path of a plugin. + # @param [#to_s] path + # The root path of the plugin. # - def self.find_gemspec(path) - reverse_ascending_paths(path).find do |candidate_path| - glob = Dir.glob("#{candidate_path}/*.gemspec") - if glob.count == 1 - return glob.first - end + def self.specification(path) + glob = Dir.glob("#{path}/*.gemspec") + spec = Gem::Specification.load(glob.first) if glob.count == 1 + unless spec + warn '[!] Unable to load a specification for the plugin ' \ + "`#{path}`".ansi.yellow end - nil + spec end - # @return [String] Returns the list of the parents paths of a path. + # @return [Array] The list of the plugins whose root path appears + # in the backtrace of an exception. # - # @param [String] path - # The path for which the list is needed. + # @param [Exception] exception + # The exception to analyze. # - def self.reverse_ascending_paths(path) - components = path.split('/')[0...-1] - progress = nil - components.map do |component| - if progress - progress = progress + '/' + component - else - progress = component - end - end.reverse + def self.plugins_involved_in_exception(exception) + paths = plugin_paths.select do |plugin_path| + exception.backtrace.any? { |line| line.include?(plugin_path.to_s) } + end + paths.map { |path| path.to_s.split('/').last } end # Returns the paths of the files to require to load the available @@ -77,10 +82,11 @@ def self.reverse_ascending_paths(path) # def self.plugin_load_paths(plugin_prefix) if plugin_prefix && !plugin_prefix.empty? + pattern = "#{plugin_prefix}_plugin" if Gem.respond_to? :find_latest_files - Gem.find_latest_files("#{plugin_prefix}_plugin") + Gem.find_latest_files(pattern) else - Gem.find_files("#{plugin_prefix}_plugin") + Gem.find_files(pattern) end else [] diff --git a/spec/command/options_spec.rb b/spec/command/options_spec.rb index 1cb332a..9db3225 100644 --- a/spec/command/options_spec.rb +++ b/spec/command/options_spec.rb @@ -74,13 +74,10 @@ def @subject.puts(text) end it 'includes plugins version if the verbose flag has been specified' do - path = 'path/to/gems/cocoapods-plugins/lib/cocoapods_plugin.rb' - Command::PluginsHelper.expects(:plugin_load_paths).returns([path]) - Command::PluginsHelper.expects(:plugin_info). - returns('cocoapods_plugin: 1.0') + spec = stub(:name => 'cocoapods_plugin', :version => '1.0') + Command::PluginsHelper.expects(:specifications).returns([spec]) command = Fixture::Command.new(%w(--version --verbose)) - command.class.stubs(:load_plugins) command.class.version = '1.0' @subject.print_version(command) output = @subject.instance_variable_get(:@fixture_output) diff --git a/spec/command/plugins_helper_spec.rb b/spec/command/plugins_helper_spec.rb index 73d62a3..612cc21 100644 --- a/spec/command/plugins_helper_spec.rb +++ b/spec/command/plugins_helper_spec.rb @@ -6,6 +6,71 @@ module CLAide describe Command::PluginsHelper do before do @subject = Command::PluginsHelper + @subject.instance_variable_set(:@plugin_paths, nil) + @subject.stubs(:require) + @path = ROOT + 'spec/fixture/command/plugin_fixture.rb' + Gem.stubs(:find_latest_files).returns([@path]) + end + + describe '::load_plugins' do + it 'requires the plugins paths' do + @subject.expects(:safe_require).with(@path.to_s) + @subject.load_plugins('fixture') + end + + it 'requires the plugins paths' do + @subject.load_plugins('fixture') + @subject.plugin_paths.should == [ + ROOT + 'spec/fixture' + ] + end + + it 'requires the plugins only if they have not been already loaded' do + @subject.expects(:safe_require).with(@path.to_s).once + @subject.load_plugins('fixture') + @subject.load_plugins('fixture') + end + end + + describe '::specifications' do + it 'returns the list of the specifications' do + spec = stub + @subject.load_plugins('fixture') + @subject.expects(:specification).with(@path + '../../').returns(spec) + @subject.specifications.should == [spec] + end + end + + describe '::specification' do + it 'returns the list of the specifications' do + root = @path + '../../' + gemspec_glob = "#{root}/*.gemspec" + gemspec_path = @path + '../../fixtures.gemspec' + spec = stub + Dir.expects(:glob).with(gemspec_glob).returns([gemspec_path]) + Gem::Specification.expects(:load).with(gemspec_path).returns(spec) + @subject.specification(root).should == spec + end + + it 'warns if unable to load a specification' do + root = @path + '../../' + message = '[!] Unable to load a specification for the plugin ' \ + "`#{root}`" + Dir.expects(:glob).returns([]) + @subject.expects(:warn).with(message) + @subject.specification(root).should.nil? + end + end + + describe '::plugins_involved_in_exception' do + it 'returns the list of the plugins involved in an exception' do + backtrace = [(@path + '../command.rb').to_s] + exception = stub(:backtrace => backtrace) + @subject.load_plugins('fixture') + @subject.plugins_involved_in_exception(exception).should == [ + 'fixture' + ] + end end describe '::plugin_load_paths' do @@ -29,45 +94,9 @@ module CLAide end end - describe '::plugin_info' do - it 'returns the information for a given plugin' do - path = 'path/to/gems/cocoapods-plugins/lib/cocoapods_plugin.rb' - gemspec = 'path/to/gems/cocoapods-plugins/cocoapods-plugins.gemspec' - spec = stub(:name => 'cocoapods-plugins', :version => '0.1.0') - @subject.stubs(:find_gemspec).returns(gemspec) - Gem::Specification.expects(:load).with(gemspec).returns(spec) - @subject.plugin_info(path).should == 'cocoapods-plugins: 0.1.0' - end - - it 'returns an error message if the specification could not be loaded' do - path = 'path/to/gems/cocoapods-plugins/lib/cocoapods_plugin.rb' - @subject.stubs(:find_gemspec).returns(nil) - result = @subject.plugin_info(path) - result.should.include('[!] Unable to load a specification for ') - result.should.include('path/to/gems/cocoapods-plugins') - end - end - - describe '::find_gemspec' do - it 'returns the path of the gemspec of a path' do - path = 'path/to/gems/cocoapods-plugins/lib/cocoapods_plugin.rb' - gemspec_glob = 'path/to/gems/cocoapods-plugins/*.gemspec' - gemspec = 'path/to/gems/cocoapods-plugins/cocoapods-plugins.gemspec' - Dir.stubs(:glob).returns([]) - Dir.expects(:glob).with(gemspec_glob).returns([gemspec]) - @subject.find_gemspec(path).should == gemspec - end - end - - describe '::reverse_ascending_paths' do - it 'returns the parent paths of a path' do - @subject.reverse_ascending_paths('dir_1/dir_2/file').should == - %w( dir_1/dir_2 dir_1 ) - end - end - describe '::safe_require' do it 'requires a path catching any exception' do + @subject.unstub(:require) path = ROOT + 'spec/fixture/command/load_error_plugin_fixture.rb' def @subject.puts(text) From 16f3750b557bddbc9145431caf6053b9d9ab5cbf Mon Sep 17 00:00:00 2001 From: Fabio Pelosin Date: Mon, 19 May 2014 17:32:44 +0200 Subject: [PATCH 2/2] [Command] Include subspecs message in error only if any is found --- lib/claide/command.rb | 7 ++++--- spec/command/plugins_helper_spec.rb | 4 ++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/lib/claide/command.rb b/lib/claide/command.rb index 4740a0d..efa73cd 100644 --- a/lib/claide/command.rb +++ b/lib/claide/command.rb @@ -317,9 +317,10 @@ def self.handle_exception(command, exception) # def self.report_error(exception) plugins = PluginsHelper.plugins_involved_in_exception(exception) - puts '[!] The exceptions involves the following plugins:' \ - "\n - #{plugins.join("\n - ")}\n".ansi.yellow - + unless plugins.empty? + puts '[!] The exceptions involves the following plugins:' \ + "\n - #{plugins.join("\n - ")}\n".ansi.yellow + end raise exception end diff --git a/spec/command/plugins_helper_spec.rb b/spec/command/plugins_helper_spec.rb index 612cc21..34db94b 100644 --- a/spec/command/plugins_helper_spec.rb +++ b/spec/command/plugins_helper_spec.rb @@ -21,7 +21,7 @@ module CLAide it 'requires the plugins paths' do @subject.load_plugins('fixture') @subject.plugin_paths.should == [ - ROOT + 'spec/fixture' + ROOT + 'spec/fixture', ] end @@ -68,7 +68,7 @@ module CLAide exception = stub(:backtrace => backtrace) @subject.load_plugins('fixture') @subject.plugins_involved_in_exception(exception).should == [ - 'fixture' + 'fixture', ] end end