Remove dependency on puppet-common #197
Conversation
| @@ -0,0 +1,15 @@ | |||
| module Puppet::Parser::Functions | |||
There was a problem hiding this comment.
I think we can get rid of this since we have puppet data types. It means changing Optional[String] $certs_tar to String[1] $certs_tar. The fqdn check should already be redundant. It also means we can remove the if $certs_tar since that looks like a useless check.
There was a problem hiding this comment.
I see. Will give it a try
There was a problem hiding this comment.
I mean the whole function can be removed since we can simply replace all function calls with Puppet 4 data types.
| context 'with empty certs_tar' do | ||
| let(:params) { { certs_tar: '' , foreman_proxy_fqdn: 'example.com' } } | ||
|
|
||
| it { should_not compile.with_all_deps } |
There was a problem hiding this comment.
I think you can leave off the .with_all_deps. Otherwise it could compile with with a missing dependency hiding a real failure. Same with the lower text.
| it { should_not compile.with_all_deps } | ||
| end | ||
|
|
||
| context 'missing foreman_proxy_fqdn' do |
There was a problem hiding this comment.
I think this is redundant since we default to $::fqdn so you actually need to put in an effort to break this I think. We should change the datatype from Optional[String] $parent_fqdn to String[1] $parent_fqdn as well I think.
| @@ -15,4 +15,10 @@ | |||
| describe 'with default parameters' do | |||
| it { should compile.with_all_deps } | |||
| end | |||
|
|
|||
| context 'with empty certs_tar' do | |||
| let(:params) { { certs_tar: '' , foreman_proxy_fqdn: 'example.com' } } | |||
There was a problem hiding this comment.
We can probably leave out the fqdn parameter (and in the basic test too).
There was a problem hiding this comment.
Looks like basic tests needs the default fqdn for one scenario:
Failure/Error: it { should compile.with_all_deps }
error during compilation: Evaluation Error: Error while evaluating a Function Call, The hostname is the same as the provided hostname for the foreman-proxy at /home/travis/build/theforeman/puppet-certs/spec/fixtures/modules/certs/manifests/foreman_proxy_content.pp:22:5 on node travis-job-theforeman-puppet-certs-377315866.travisci.net
certs::qpid_client
on centos-7-x86_64
without parameters
should compile into a catalogue without dependency cycles (FAILED - 1)
| require 'spec_helper' | ||
| require 'puppetlabs_spec_helper/puppetlabs_spec/puppet_internals' | ||
|
|
||
| describe 'validate_file_exists' do |
There was a problem hiding this comment.
This can probably use a test and verify that / does exist.
|
@jturel Can you also remove puppet-common from https://github.com/Katello/katello-installer/blob/master/Puppetfile#L6? |
ah ok! I was missing where the dependency was actually included |
|
Thanks! |
there wasn't a dependency on puppet-common in the manifest. that seems unexpected!