From d8dfb1f143ba3e602bec387508c8dad7aab00062 Mon Sep 17 00:00:00 2001 From: Daniel Pittman Date: Sun, 10 Apr 2011 18:59:23 -0700 Subject: (#6962) Implement 'summary' for faces. This adds the methods to the summary builder and runtime instance to support setting and getting a summary of the face. This is a short description used to summarize the purpose of the face in help output. For example, from the help face: "Displays help about puppet subcommands" Reviewed-By: Matt Robinson --- spec/unit/application/indirection_base_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'spec/unit/application') diff --git a/spec/unit/application/indirection_base_spec.rb b/spec/unit/application/indirection_base_spec.rb index 10ebe8e3d..98eb3a118 100755 --- a/spec/unit/application/indirection_base_spec.rb +++ b/spec/unit/application/indirection_base_spec.rb @@ -13,7 +13,7 @@ face = Puppet::Faces::Indirector.define(:testindirection, '0.0.1') do end # REVISIT: This horror is required because we don't allow anything to be # :current except for if it lives on, and is loaded from, disk. --daniel 2011-03-29 -face.version = :current +face.instance_variable_set('@version', :current) Puppet::Faces.register(face) ######################################################################## -- cgit From 9496067f5644972def823cf8c3318aca137b86fe Mon Sep 17 00:00:00 2001 From: Daniel Pittman Date: Mon, 11 Apr 2011 22:25:58 -0700 Subject: maint: avoid making temporary dirs during testing. We used to create temporary directories to have some support files on disk during testing of faces; we don't really need that most of the time, and this updates a test to reflect that reality. Reviewed-By: Matt Robinson --- spec/unit/application/faces_base_spec.rb | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) (limited to 'spec/unit/application') diff --git a/spec/unit/application/faces_base_spec.rb b/spec/unit/application/faces_base_spec.rb index b7a11ad56..a6df55aef 100755 --- a/spec/unit/application/faces_base_spec.rb +++ b/spec/unit/application/faces_base_spec.rb @@ -1,4 +1,4 @@ -#!/usr/bin/env ruby +#!/usr/bin/env rspec require 'spec_helper' require 'puppet/application/faces_base' @@ -9,13 +9,6 @@ end describe Puppet::Application::FacesBase do before :all do - @dir = Dir.mktmpdir - $LOAD_PATH.push(@dir) - FileUtils.mkdir_p(File.join @dir, 'puppet', 'faces') - File.open(File.join(@dir, 'puppet', 'faces', 'basetest.rb'), 'w') do |f| - f.puts "Puppet::Faces.define(:basetest, '0.0.1')" - end - Puppet::Faces.define(:basetest, '0.0.1') do option("--[no-]boolean") option("--mandatory MANDATORY") @@ -28,11 +21,6 @@ describe Puppet::Application::FacesBase do end end - after :all do - FileUtils.remove_entry_secure @dir - $LOAD_PATH.pop - end - let :app do app = Puppet::Application::FacesBase::Basetest.new app.stubs(:exit) -- cgit From 826d5dff531eb624fef91a7298932e9ec5a46231 Mon Sep 17 00:00:00 2001 From: Daniel Pittman Date: Mon, 11 Apr 2011 23:09:18 -0700 Subject: (#6962) delegate global usage to the help face. The global --help handler (also invoked when puppet is run without any other command line options at all) used to spit out a brief and generally not so helpful message. Now that we have a help face that can provide the same information in a much more user-friendly form, we should delegate the function to that when required. Reviewed-By: Matt Robinson --- spec/unit/application/cert_spec.rb | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) (limited to 'spec/unit/application') diff --git a/spec/unit/application/cert_spec.rb b/spec/unit/application/cert_spec.rb index 5b25ab7b8..a1b5eb19a 100755 --- a/spec/unit/application/cert_spec.rb +++ b/spec/unit/application/cert_spec.rb @@ -1,7 +1,5 @@ -#!/usr/bin/env ruby - +#!/usr/bin/env rspec require 'spec_helper' - require 'puppet/application/cert' describe Puppet::Application::Cert do @@ -189,16 +187,6 @@ describe Puppet::Application::Cert do @cert_app.ca = @ca end - it "should SystemExit after printing help message" do - # Make the help method silent for testing; this is a bit nasty, but we - # can't identify a cleaner method. Help welcome. --daniel 2011-02-22 - Puppet.features.stubs(:usage?).returns(false) - @cert_app.stubs(:puts) - - @cert_app.command_line.stubs(:args).returns([]) - expect { @cert_app.parse_options }.should raise_error SystemExit - end - %w{list revoke generate sign print verify fingerprint}.each do |cmd| short = cmd[0,1] [cmd, "--#{cmd}", "-#{short}"].each do |option| -- cgit From 7b4d9367b391f75983868046d30928ebc8411f50 Mon Sep 17 00:00:00 2001 From: Daniel Pittman Date: Tue, 12 Apr 2011 11:45:05 -0700 Subject: (#6962) Move option handling into #parse_options, not #preinit. Logically, the extra work around option parsing for faces belongs in the application parse_options method, not hidden in the step before. This commit moves that to the right place and fixes the fallout from that strange early design decision. Along the way we unify error reporting for invalid options so that all the code paths result in the same externally detected failures. Reviewed-By: Matt Robinson --- spec/unit/application/certificate_spec.rb | 5 ++- spec/unit/application/faces_base_spec.rb | 60 +++++++++++++++++-------------- 2 files changed, 38 insertions(+), 27 deletions(-) (limited to 'spec/unit/application') diff --git a/spec/unit/application/certificate_spec.rb b/spec/unit/application/certificate_spec.rb index 6153d9538..27d6ac81b 100755 --- a/spec/unit/application/certificate_spec.rb +++ b/spec/unit/application/certificate_spec.rb @@ -6,12 +6,15 @@ describe Puppet::Application::Certificate do # so is this actually a valuable test? --daniel 2011-04-07 subject.command_line.stubs(:args).returns %w{list} subject.preinit + subject.parse_options subject.should respond_to(:handle_ca_location) end it "should accept the ca-location option" do subject.command_line.stubs(:args).returns %w{--ca-location local list} - subject.preinit and subject.parse_options and subject.setup + subject.preinit + subject.parse_options + subject.setup subject.arguments.should == [{ :ca_location => "local" }] end end diff --git a/spec/unit/application/faces_base_spec.rb b/spec/unit/application/faces_base_spec.rb index a6df55aef..18bd30295 100755 --- a/spec/unit/application/faces_base_spec.rb +++ b/spec/unit/application/faces_base_spec.rb @@ -23,9 +23,7 @@ describe Puppet::Application::FacesBase do let :app do app = Puppet::Application::FacesBase::Basetest.new - app.stubs(:exit) - app.stubs(:puts) - app.command_line.stubs(:subcommand_name).returns 'subcommand' + app.command_line.stubs(:subcommand_name).returns('subcommand') Puppet::Util::Log.stubs(:newdestination) app end @@ -39,7 +37,7 @@ describe Puppet::Application::FacesBase do end end - describe "#preinit" do + describe "#parse_options" do before :each do app.command_line.stubs(:args).returns %w{} end @@ -56,6 +54,7 @@ describe Puppet::Application::FacesBase do Signal.stubs(:trap) app.command_line.stubs(:args).returns %w{foo} app.preinit + app.parse_options end it "should set the faces based on the type" do @@ -73,54 +72,54 @@ describe Puppet::Application::FacesBase do end it "should fail if no action is given" do - expect { app.preinit }. - should raise_error ArgumentError, /No action given/ + expect { app.preinit; app.parse_options }. + to raise_error OptionParser::MissingArgument, /No action given/ end it "should report a sensible error when options with = fail" do app.command_line.stubs(:args).returns %w{--action=bar foo} - expect { app.preinit }. - should raise_error ArgumentError, /Unknown option "--action"/ + expect { app.preinit; app.parse_options }. + to raise_error OptionParser::InvalidOption, /invalid option: --action/ end it "should fail if an action option is before the action" do app.command_line.stubs(:args).returns %w{--action foo} - expect { app.preinit }. - should raise_error ArgumentError, /Unknown option "--action"/ + expect { app.preinit; app.parse_options }. + to raise_error OptionParser::InvalidOption, /invalid option: --action/ end it "should fail if an unknown option is before the action" do app.command_line.stubs(:args).returns %w{--bar foo} - expect { app.preinit }. - should raise_error ArgumentError, /Unknown option "--bar"/ + expect { app.preinit; app.parse_options }. + to raise_error OptionParser::InvalidOption, /invalid option: --bar/ end - it "should not fail if an unknown option is after the action" do + it "should fail if an unknown option is after the action" do app.command_line.stubs(:args).returns %w{foo --bar} - app.preinit - app.action.name.should == :foo - app.face.should_not be_option :bar - app.action.should_not be_option :bar + expect { app.preinit; app.parse_options }. + to raise_error OptionParser::InvalidOption, /invalid option: --bar/ end it "should accept --bar as an argument to a mandatory option after action" do app.command_line.stubs(:args).returns %w{foo --mandatory --bar} - app.preinit and app.parse_options + app.preinit + app.parse_options app.action.name.should == :foo app.options.should == { :mandatory => "--bar" } end it "should accept --bar as an argument to a mandatory option before action" do app.command_line.stubs(:args).returns %w{--mandatory --bar foo} - app.preinit and app.parse_options + app.preinit + app.parse_options app.action.name.should == :foo app.options.should == { :mandatory => "--bar" } end it "should not skip when --foo=bar is given" do app.command_line.stubs(:args).returns %w{--mandatory=bar --bar foo} - expect { app.preinit }. - should raise_error ArgumentError, /Unknown option "--bar"/ + expect { app.preinit; app.parse_options }. + to raise_error OptionParser::InvalidOption, /invalid option: --bar/ end { "boolean options before" => %w{--trace foo}, @@ -128,7 +127,8 @@ describe Puppet::Application::FacesBase do }.each do |name, args| it "should accept global boolean settings #{name} the action" do app.command_line.stubs(:args).returns args - app.preinit && app.parse_options + app.preinit + app.parse_options Puppet[:trace].should be_true end end @@ -138,7 +138,8 @@ describe Puppet::Application::FacesBase do }.each do |name, args| it "should accept global settings with arguments #{name} the action" do app.command_line.stubs(:args).returns args - app.preinit && app.parse_options + app.preinit + app.parse_options Puppet[:syslogfacility].should == "user1" end end @@ -148,19 +149,25 @@ describe Puppet::Application::FacesBase do describe "#setup" do it "should remove the action name from the arguments" do app.command_line.stubs(:args).returns %w{--mandatory --bar foo} - app.preinit and app.parse_options and app.setup + app.preinit + app.parse_options + app.setup app.arguments.should == [{ :mandatory => "--bar" }] end it "should pass positional arguments" do app.command_line.stubs(:args).returns %w{--mandatory --bar foo bar baz quux} - app.preinit and app.parse_options and app.setup + app.preinit + app.parse_options + app.setup app.arguments.should == ['bar', 'baz', 'quux', { :mandatory => "--bar" }] end end describe "#main" do - before do + before :each do + app.expects(:exit).with(0) + app.face = Puppet::Faces[:basetest, '0.0.1'] app.action = app.face.get_action(:foo) app.format = :pson @@ -174,6 +181,7 @@ describe Puppet::Application::FacesBase do it "should use its render method to render any result" do app.expects(:render).with(app.arguments.length + 1) + app.stubs(:puts) # meh. Don't print nil, thanks. --daniel 2011-04-12 app.main end end -- cgit