Actually, if used inheritance instead of metaprogramming to differentiate the different applications, then they wouldn't even have to be singletons, we could just call Puppet::Application::Agent.new.run() instead of Puppet::Application[:agent].run, and there'd be no danger of leaking state; the error I saw was when I accidentally nuked all the methods on an application by creating another one of the same name - that would become impossible.
~Jesse On Wed, Apr 21, 2010 at 2:01 PM, Jesse Wolfe <[email protected]> wrote: > I'm tempted to suggest using ruby constants - and maybe even using > singleton subclasses of Application rather than instances - but honestly I > haven't seen a singleton pattern in ruby that I'm 100% happy with. We're > using several different ones in puppet in various places (applications, > indirector models&termini, types and providers...), it might be nice if we > found a one-size-fits all solution. > I don't have any brilliant ideas, yet. > > ~Jesse > > > On Wed, Apr 21, 2010 at 1:32 PM, Luke Kanies <[email protected]> wrote: > >> Nice catch. >> >> I still wonder if there's a better way to track these than to store them >> in @@applications. Like maybe there's a way for an app to declare that it >> should be registered as a global app or something. >> >> I've learned to distrust my code when I need to do what you're doing in >> the before block here -- that is, when I'm testing an artificial construct >> that is somehow global. It's become a code smell for me. >> >> Not really a criticism, esp. since I wrote it in the first place, but if >> you have a more elegant solution, I'm all for it. >> >> >> On Apr 21, 2010, at 11:54 AM, Jesse Wolfe wrote: >> >> Prevent you from making a mistake that I made in testing: Creating two >>> "Application" objects with the same name can cause problems. >>> Surprisingly, changing this led me to rediscover a bug in rspec. >>> >>> Signed-off-by: Jesse Wolfe <[email protected]> >>> --- >>> lib/puppet/application.rb | 4 ++++ >>> spec/unit/application.rb | 16 +++++++--------- >>> 2 files changed, 11 insertions(+), 9 deletions(-) >>> >>> diff --git a/lib/puppet/application.rb b/lib/puppet/application.rb >>> index 0f479b0..c5ddc22 100644 >>> --- a/lib/puppet/application.rb >>> +++ b/lib/puppet/application.rb >>> @@ -255,6 +255,10 @@ class Puppet::Application >>> >>> @name = symbolize(name) >>> >>> + if @@applications[name] >>> + raise Puppet::DevError, "There is already an application >>> named #{name.inspect}" >>> + end >>> + >>> init_default >>> >>> @options = {} >>> diff --git a/spec/unit/application.rb b/spec/unit/application.rb >>> index 87a9009..a290aba 100755 >>> --- a/spec/unit/application.rb >>> +++ b/spec/unit/application.rb >>> @@ -8,8 +8,9 @@ require 'getoptlong' >>> >>> describe Puppet::Application do >>> >>> - before :each do >>> - @app = Puppet::Application.new(:test) >>> + before :all do >>> + # Workaround due to rspec bug #819: This code is being called >>> multiple times >>> + @app = Puppet::Application[:test] || >>> Puppet::Application.new(:test) >>> end >>> >>> it "should have a run entry-point" do >>> @@ -36,6 +37,10 @@ describe Puppet::Application do >>> @app.get_command.should == :main >>> end >>> >>> + it "should not allow you to create another application of the same >>> name" do >>> + lambda{ Puppet::Application.new(:test) }.should >>> raise_error(Puppet::DevError) >>> + end >>> + >>> describe 'when invoking clear!' do >>> before :each do >>> Puppet::Application.run_status = :stop_requested >>> @@ -170,7 +175,6 @@ describe Puppet::Application do >>> ARGV.clear >>> >>> Puppet.settings.stubs(:optparse_addargs).returns([]) >>> - @app = Puppet::Application.new(:test) >>> end >>> >>> after :each do >>> @@ -288,7 +292,6 @@ describe Puppet::Application do >>> describe "when calling default setup" do >>> >>> before :each do >>> - @app = Puppet::Application.new(:test) >>> @app.stubs(:should_parse_config?).returns(false) >>> @app.options.stubs(:[]) >>> end >>> @@ -317,7 +320,6 @@ describe Puppet::Application do >>> describe "when running" do >>> >>> before :each do >>> - @app = Puppet::Application.new(:test) >>> @app.stubs(:run_preinit) >>> @app.stubs(:run_setup) >>> @app.stubs(:parse_options) >>> @@ -411,10 +413,6 @@ describe Puppet::Application do >>> >>> describe "when metaprogramming" do >>> >>> - before :each do >>> - @app = Puppet::Application.new(:test) >>> - end >>> - >>> it "should create a new method with command" do >>> @app.command(:test) do >>> end >>> -- >>> 1.6.3.3 >>> >>> -- >>> You received this message because you are subscribed to the Google Groups >>> "Puppet Developers" group. >>> To post to this group, send email to [email protected]. >>> To unsubscribe from this group, send email to >>> [email protected]<puppet-dev%[email protected]> >>> . >>> For more options, visit this group at >>> http://groups.google.com/group/puppet-dev?hl=en. >>> >>> >> >> -- >> Good judgment comes from experience, and experience comes from bad >> judgment. --Barry LePatner >> --------------------------------------------------------------------- >> Luke Kanies -|- http://puppetlabs.com -|- +1(615)594-8199 >> >> >> -- >> You received this message because you are subscribed to the Google Groups >> "Puppet Developers" group. >> To post to this group, send email to [email protected]. >> To unsubscribe from this group, send email to >> [email protected]<puppet-dev%[email protected]> >> . >> For more options, visit this group at >> http://groups.google.com/group/puppet-dev?hl=en. >> >> > -- You received this message because you are subscribed to the Google Groups "Puppet Developers" group. To post to this group, send email to [email protected]. To unsubscribe from this group, send email to [email protected]. For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.
