Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Regexp support to with_* matchers and improve error messages #17

Conversation

jeffmccune
Copy link
Collaborator

Without this patch it is difficult to match portions of long strings
which are attribute values of resource parameters. For example, the
content parameter of a file resource with a multi-line template.

This patch makes it easier to match portions of the parameter's value by
passing a Regexp instance to the with_ catch all matcher.

The use case looks like:

describe 'with lsbmajdistrelease available' do
  let(:facts) { @facter_facts.merge({'lsbmajdistrelease' => '6'}) }
  it { should_not contain_file('puppetenterprise.repo').with_content(missing_content) }
  it { should contain_package 'pe-ruby-devel' }
  it do
    should contain_file('puppetenterprise.repo').with_content(/baseurl=http/)
    should contain_file('puppetenterprise.repo').with_content(/pe_base/)
    should contain_file('puppetenterprise.repo').with_content(/pe_updates/)
    should contain_file('puppetenterprise.repo').with_content(/pe_extras/)
  end
end

This feature allows the same parameter to be matched multiple times using
multiple different regular expressions.

This patch also improves the error message by presenting the actual value in
the catalog when it does not match the expectation. This change applies to
both String and Regexp expectation matches.

The error output now looks like:

1) pe_devel on redhat el6 os families with lsbmajdistrelease available
   Failure/Error: should contain_file('puppetenterprise.repo').with_content(/JEFF MCCUNE/)
     expected that the catalogue would contain File[puppetenterprise.repo] with content \
     matching `/JEFF MCCUNE/` but its value of `"# KERMIT FROG\n"` does not

Spec tests have been added to exercise the handling of Regexp instances.

@jeffmccune
Copy link
Collaborator Author

@rodjek I should mention I tested this change on both 2.6.7 and 2.7.x and it appears to work fine. I was a bit concerned about the switch to resource.to_hash, but I think it's fine in 2.6 and beyond.

Cheers,
-Jeff

@jeffmccune
Copy link
Collaborator Author

@rodjek Ping?

@bodepd
Copy link
Collaborator

bodepd commented Dec 29, 2011

The code looks fine (and works!)

I have also verified that it works in combination with #18

Just one small comment which in my opinion should not block merging this code:

The wording could be a little better in the case where there are multiple failures:

  1. keystone when specifying class parameters
    Failure/Error: ) end
    expected that the catalogue would contain File[/etc/keystone] with group set to "keystones" but it is set to "keystone" in the catalogue, mode matching /^\d{3}$/ but its value of "0755" does not

I am not quite sure what the reviewed by process is for a pull request. Should I a mend a reviewed by message to the commit and repush?

@jeffmccune
Copy link
Collaborator Author

On Dec 29, 2011, at 2:41 PM, Dan Bode wrote:

The code looks fine (and works!)

I have also verified that it works in combination with #18

Just one small comment which in my opinion should not block merging this code:

The wording could be a little better in the case where there are multiple failures:

  1. keystone when specifying class parameters
    Failure/Error: ) end
    expected that the catalogue would contain File[/etc/keystone] with group set to "keystones" but it is set to "keystone" in the catalogue, mode matching /^\d{3}$/ but its value of "0755" does not

Yeah, I struggled with how to word that failure message well for the arbitrary cases it needs to handle.

What part of the message is jumping out at you?

I take it the run-on sentence around the comma in "...in the catalogue, mode matching…" ?

I am not quite sure what the reviewed by process is for a pull request. Should I a mend a reviewed by message to the commit and repush?

I'll amend my own commit to make it easy since you can't push to my fork. Really all that's required is "this looks good, you can add a reviewed-by: line for me"

-Jeff

Without this patch it is difficult to match portions of long strings
which are attribute values of resource parameters.  For example, the
content parameter of a file resource with a multi-line template.

This patch makes it easier to match portions of the parameter's value by
passing a Regexp instance to the with_ catch all matcher.

The use case looks like:

    describe 'with lsbmajdistrelease available' do
      let(:facts) { @facter_facts.merge({'lsbmajdistrelease' => '6'}) }
      it { should_not contain_file('puppetenterprise.repo').with_content(missing_content) }
      it { should contain_package 'pe-ruby-devel' }
      it do
        should contain_file('puppetenterprise.repo').with_content(/baseurl=http/)
        should contain_file('puppetenterprise.repo').with_content(/pe_base/)
        should contain_file('puppetenterprise.repo').with_content(/pe_updates/)
        should contain_file('puppetenterprise.repo').with_content(/pe_extras/)
      end
    end

This feature allows the same parameter to be matched multiple times using
multiple different regular expressions.

This patch also improves the error message by presenting the actual value in
the catalog when it does not match the expectation.  This change applies to
both String and Regexp expectation matches.

The error output now looks like:

    1) pe_devel on redhat el6 os families with lsbmajdistrelease available
       Failure/Error: should contain_file('puppetenterprise.repo').with_content(/JEFF MCCUNE/)
         expected that the catalogue would contain File[puppetenterprise.repo] with content \
         matching `/JEFF MCCUNE/` but its value of `"# KERMIT FROG\n"` does not

Spec tests have been added to exercise the handling of Regexp instances.

Reviewed-by: Dan Bode
@jeffmccune
Copy link
Collaborator Author

@bodepd I amended the commit. What do you think about this:

expected that the catalogue would contain File[/etc/keystone] with group set to `"keystones"` but it is set to `"keystone"` in the catalogue, and parameter mode matching `/^\d{3}$/` but its value of `"0755"` does not

I inserted "and parameter" into the join of the errors.

@bodepd
Copy link
Collaborator

bodepd commented Dec 29, 2011

Reviewed-By: Dan Bode dan@puppetlabs.com

jeffmccune pushed a commit that referenced this pull request Dec 29, 2011
…_error_message

Add Regexp support to with_* matchers and improve error messages
@jeffmccune jeffmccune merged commit 8d2e3e6 into rodjek:master Dec 29, 2011
binford2k pushed a commit to binford2k/rspec-puppet that referenced this pull request Jul 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants