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

Raise minimum version to Puppet 7.11 #68

Closed
wants to merge 4 commits into from

Conversation

ekohl
Copy link

@ekohl ekohl commented Aug 19, 2023

This avoids the need to have any version specific code and it's the only thing that's testing in CI.

Right now still a draft since I'm not quite sure how it should turn out. Includes #67

ekohl added 4 commits August 19, 2023 12:44
Fixes: 79f7f56 ("(MAINT) Add ruby requirement")
This avoids the need to have any version specific code and it's the only
thing that's testing in CI.
RSpec.configure do |c|
c.module_path = File.join(base_dir, 'fixtures', 'modules')
c.manifest_dir = File.join(base_dir, 'fixtures', 'manifests')
c.manifest = File.join(base_dir, 'fixtures', 'manifests', 'site.pp')

Choose a reason for hiding this comment

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

We utilize the manifest setting when testing our control repository to ensure anything in the environment site.pp is included during tests as shown in the spec/spec_helper.rb fragment below. Is there a better way to accomplish this?

RSpec.configure do |c|
  c.manifest = File.join(File.dirname(File.dirname(__FILE__)), 'manifests', 'site.pp')
end

Copy link
Author

Choose a reason for hiding this comment

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

The reason I dropped it is that the manifest setting is gone. At least in https://www.puppet.com/docs/puppet/7/configuration.html I can no longer find it. I suspect environments now determine this via environment.conf. But feedback is welcome. I may have missed something.

Copy link
Author

Choose a reason for hiding this comment

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

I'm pretty sure I ran into a situation myself where I needed to the manifest, so I think you're right that this should remain.

Choose a reason for hiding this comment

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

It might be the case that Puppet doesn't use this setting directly anymore, but it is used by rspec-puppet internally to prepend additional Puppet code to test:

[site_pp_str, pre_cond, test_manifest(type, manifest_opts), post_cond].compact.join("\n")

def site_pp_str
return '' unless (path = adapter.manifest)
if File.file?(path)
File.read(path)
elsif File.directory?(path)
# Read and concatenate all .pp files.
Dir[File.join(path, '*.pp')].sort.map do |f|
File.read(f)
end.join("\n")
else
''
end
end

Copy link
Author

Choose a reason for hiding this comment

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

Yes, I found this in our Infra repo where I use it to test our environment, rather than individual modules: https://github.com/theforeman/foreman-infra/blob/9d05130a5ada4fd734898b6e14b93bd292c92717/puppet/spec/spec_helper.rb#L29

@jordanbreen28
Copy link

thanks for doing most of the heavy lifting @ekohl, closing in favour of #73

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants