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.

Sign up for GitHub

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

Jump to bottom

Fix for issue 185. #186

Merged
merged 1 commit into from May 28, 2015
Merged

Fix for issue 185. #186

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 14 additions & 2 deletions lib/rspec-puppet/support.rb
View file
Open in desktop
Expand Up @@ -13,7 +13,9 @@ def load_catalogue(type)
code = [import_str, pre_cond, test_manifest(type)].join("\n")
node_name = nodename(type)

catalogue = build_catalog(node_name, facts_hash(node_name), code)
hiera_config_value = self.respond_to?(:hiera_config) ? hiera_config : nil

catalogue = build_catalog(node_name, facts_hash(node_name), hiera_config_value, code)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand why these two lines have been added... Could you help me understand?

It seems like hiera_config_value is never actually used in the build_catalog_without_cache method.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The trick seems to be that adding the configuration as additional argument will change the cache key used in build_catalog(). Without this, the cached catalog is returned unchanged.


RSpec::Puppet::Coverage.filters << "#{type.to_s.capitalize}[#{self.class.display_name.capitalize}]"

Expand Down Expand Up @@ -136,7 +138,17 @@ def setup_puppet
vardir
end

def build_catalog_without_cache(nodename, facts_val, code)
def build_catalog_without_cache(nodename, facts_val, hiera_config_val, code)

# If we're going to rebuild the catalog, we should clear the cached instance
# of Hiera that Puppet is using. This opens the possibility of the catalog
# now being rebuilt against a differently configured Hiera (i.e. :hiera_config
# set differently in one example group vs. another).
# It would be nice if Puppet offered a public API for invalidating their
# cached instance of Hiera, but que sera sera. We will go directly against
# the implementation out of absolute necessity.
HieraPuppet.instance_variable_set('@hiera', nil) if defined? HieraPuppet
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In Puppet core we explicitly reset the entire settings subsystem for every test case. My recommendation is to do something similar here.

Puppet does have an API for this sort of thing, please see Puppet::Test::TestHelper.before_each_test()

If this isn't being called as expected, then we might want to add this behavior to core Puppet. We could do this in such a way that rspec-puppet uses the API if possible, and falls back to modifying Puppet internals if not.

The less 3rd party code reaches in and mucks with Puppet internals, the better.


Puppet[:code] = code

stub_facts! facts_val
Expand Down