On Oct 12, 2010, at 1:47 PM, Nick Lewis wrote:

> On Tue, Oct 12, 2010 at 12:48 PM, Markus Roberts <[email protected]> 
> wrote:
> 
> 
> diff --git a/spec/unit/provider/user/user_role_add_spec.rb 
> b/spec/unit/provider/user/user_role_add_spec.rb
> index b3244f1..cc4fd9a 100644
> --- a/spec/unit/provider/user/user_role_add_spec.rb
> +++ b/spec/unit/provider/user/user_role_add_spec.rb
> @@ -67,12 +67,12 @@ describe provider_class do
>       @provider.create
>     end
> 
> -    it "should set password age rules" do
> +    it "should set password age rules with passwd" do
>       @resource = Puppet::Type.type(:user).new :name => "myuser", 
> :password_min_age => 5, :password_max_age => 10, :provider => :user_role_add
>       @provider = provider_class.new(@resource)
>       @provider.stubs(:user_attributes)
>       @provider.stubs(:execute)
> -      @provider.expects(:execute).with { |cmd, *args| args == ["-m", 5, 
> "-M", 10, "myuser"] }
> +      @provider.expects(:execute).with { |args| args == 
> ["/usr/bin/passwd","-n", 5, "-x", 10, "myuser"] }
>       @provider.create
>     end
>   end
> --
> 1.7.0.4
> 
>  
> +1 to the fix, but not the spec change. Because providers validate the 
> existence of their commands, this spec will now fail on any system which 
> doesn't have a /usr/bin/passwd command. It will return "nil" for the command 
> name, which is why the previous version of the spec was only checking the 
> arguments. I suppose a possibility is to confine the spec to Solaris, which 
> gives some validation that the command is correct for that system. 
> 
> So how should we test (in a testing environment neutral way) that 
> user_role_add correctly tries to use passwd instead of chage?  In other 
> words, how should we test that this fix isn't regressed in some way, rather 
> than saying "well, the code we are trying to test gives inconsistent results 
> depending on who runs the test, so we just don't test that part"? 
> 
> 
> Given the way that commands are set and validated, which happens when the 
> type is first used, I can't really see a way to intercept that from this test 
> (or even from this file). So the command will be nil long before it ever gets 
> to us. So the only test I can see being viable across environments is to stub 
> the command method to return some particular known value, which tells us at 
> least that it is using the command we specified to be the "password" command. 
> Beyond that, we would probably need to have a Solaris-specific test telling 
> us that the command being used is what we think it should be, and that the 
> command actually exists on Solaris. The latter part is key, as that is the 
> only part of this idea which actually could have caught the particular bug 
> it's trying to verify as fixed.


It should make sense to just stub the 'command' method - if you're doing 
anything else, you're testing the function of the command method itself, right?

At least, this is how I've done other tests, and those ones been pretty 
maintainable, I think.

-- 
If it jams, force it. If it breaks, it probably needed replacing
anyway. -- Lowrey's Law
---------------------------------------------------------------------
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].
For more options, visit this group at 
http://groups.google.com/group/puppet-dev?hl=en.

Reply via email to