Need help for non-regression tests

[Aurelien Degremont]

Hello

I proposed a patch for a new feature regarding to Mount object.
This is quite simple and described in:
http://projects.reductivelabs.com/issues/2730

Unfortunately Ruby and unit test modules used by puppet are not familiar for me.
Can somebody helps me what I'm doing wrong regarding test in this patch?

http://projects.reductivelabs.com/attachments/745/puppet_mount.diff

Thanks

--
Aurelien Degremont

[Luke Kanies]

To test that the new value works, something like this is good:

it "should support :defined as a value to :ensure" do
Puppet::Type.type(:mount).new(:name => "yay", :ensure => :defined)
[:ensure].should == :defined
end
I could see using 'should_not raise_error', also, but this provides
both that functionality (since an error would propagate if it gets
raised) plus testing that the value actually gets set correctly.

As to 'insync?', I would test it directly, with a set of behaviours.

it "should be in sync if the desired value is set to 'defined' and the
current value is 'defined'" do
mount = Puppet::Type.type(:mount).new :name => "foo", :ensure
=> :defined
mount.must be_insync(:defined)
end

Note the use of 'must' here -- resources already have a 'should'
method, so we've aliased rspec's 'should' method to 'must'.

--
My favorite was a professor at a University I Used To Be Associated
With who claimed that our requirement of a non-alphabetic character in
our passwords was an abridgement of his freedom of speech.
-- Jacob Haller
---------------------------------------------------------------------
Luke Kanies -|- http://puppetlabs.com -|- +1(615)594-8199

[Aurelien Degremont]

Luke Kanies a �crit :

> On Jul 6, 2010, at 7:49 AM, Aurelien Degremont wrote:
>
>> Hello
>>
>> I proposed a patch for a new feature regarding to Mount object.
>> This is quite simple and described in:
>> http://projects.reductivelabs.com/issues/2730
>>
>> Unfortunately Ruby and unit test modules used by puppet are not
>> familiar for me.
>> Can somebody helps me what I'm doing wrong regarding test in this patch?
>>
>> http://projects.reductivelabs.com/attachments/745/puppet_mount.diff
>
> To test that the new value works, something like this is good:

Thanks for your help

> it "should support :defined as a value to :ensure" do
> Puppet::Type.type(:mount).new(:name => "yay", :ensure =>
> :defined)[:ensure].should == :defined
> end
> I could see using 'should_not raise_error', also, but this provides both
> that functionality (since an error would propagate if it gets raised)
> plus testing that the value actually gets set correctly.

I'm a little surprised because all other tests only create the new mount:
i.e.:

it "should support :mounted as a value to :ensure" do
Puppet::Type.type(:mount).new(:name => "yay", :ensure => :mounted)
end

> As to 'insync?', I would test it directly, with a set of behaviours.
>
> it "should be in sync if the desired value is set to 'defined' and the
> current value is 'defined'" do
> mount = Puppet::Type.type(:mount).new :name => "foo", :ensure => :defined
> mount.must be_insync(:defined)
> end
>
> Note the use of 'must' here -- resources already have a 'should' method,
> so we've aliased rspec's 'should' method to 'must'.

Ok
I can update the patch with those tests. Is this ok for being accepted?
Do I make the patch for 0.25.x or master branch?

--
Aurelien Degremont

[Luke Kanies]

On Jul 7, 2010, at 2:27 AM, Aurelien Degremont wrote:

> Luke Kanies a écrit :

Yeah, that works, too, because it'll fail if that value is in valid.
I was responding on my phone and didn't really have a chance to look
at the other tests. :)

>> As to 'insync?', I would test it directly, with a set of behaviours.
>> it "should be in sync if the desired value is set to 'defined' and
>> the current value is 'defined'" do
>> mount = Puppet::Type.type(:mount).new :name => "foo", :ensure
>> => :defined
>> mount.must be_insync(:defined)
>> end
>> Note the use of 'must' here -- resources already have a 'should'
>> method, so we've aliased rspec's 'should' method to 'must'.
>
> Ok
> I can update the patch with those tests. Is this ok for being
> accepted?
> Do I make the patch for 0.25.x or master branch?

Definitely 'master' - 0.25.x only gets fixes for bugs introduced in
the 0.25 release series.

And the patch looks ok to me, but I'm not the main one doing actual
code reviews any more. *cough*Markus*cough*

--
Opportunity is missed by most people because it is dressed in overalls
and looks like work. -- Thomas A. Edison

[Aurelien Degremont]

Thank you.
I've update the ticket with a new patch.

Aur�lien

Luke Kanies a �crit :

> On Jul 7, 2010, at 2:27 AM, Aurelien Degremont wrote:
>

>> Luke Kanies a �crit :