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.

Reply via email to