-
Notifications
You must be signed in to change notification settings - Fork 582
(PUP-11752) fqdn_rand_string_spec.rb relies on legacy fqdn fact #1294
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch, this will be needed with the next version of Puppet.
Minor style issue, also the commit message needs need adjusting 😄
There could potentially be other modules that are impacted too
@@ -57,7 +57,7 @@ def fqdn_rand_string(max, args = {}) | |||
|
|||
# workaround not being able to use let(:facts) because some tests need | |||
# multiple different hostnames in one context | |||
allow(scope).to receive(:lookupvar).with('::fqdn', {}).and_return(host) | |||
allow(scope).to receive(:lookupvar).with('facts', {}).and_return({ 'networking' => { 'fqdn' => host } }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah the expectation needs to be different based on the version of puppet (before and after this change puppetlabs/puppet@c0d2560). So something like
if Puppet::PUPPETVERSION.to_f < 7.23
allow(scope).to receive(:lookupvar).with('::fqdn', {}).and_return(host)
else
allow(scope).to receive(:lookupvar).with('facts', {}).and_return({ 'networking' => { 'fqdn' => host } })
end
Or there may be a better way to stub out the facts using RspecPuppetFacts
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps this function should just mock fqdn_rand
? It really is fqdn_rand
which gets the fact, so we don't care about the internal implementation:
rand_string += charset[function_fqdn_rand([charset.size, (args + [current.to_s]).join(':')]).to_i] |
|
Since Puppet `7.23.0` `fqdn_rand` uses the modern structured `networking` fact instead of the legacy `fqdn` fact. In this commit we mock the new fact if using `7.23.0` or later. This is a rebase of puppetlabs#1294 and incorporates Aria Li's original change and a variation on Josh Cooper's suggestion for keeping compatibility with Puppet < `7.23.0`
PR #1308 was merged and supersedes this work, so closing this PR. |
Since Puppet `7.23.0` `fqdn_rand` uses the modern structured `networking` fact instead of the legacy `fqdn` fact. In this commit we mock the new fact if using `7.23.0` or later. This is a rebase of puppetlabs#1294 and incorporates Aria Li's original change and a variation on Josh Cooper's suggestion for keeping compatibility with Puppet < `7.23.0`
Since Puppet `7.23.0` `fqdn_rand` uses the modern structured `networking` fact instead of the legacy `fqdn` fact. In this commit we mock the new fact if using `7.23.0` or later. This is a rebase of puppetlabs/puppetlabs-stdlib#1294 and incorporates Aria Li's original change and a variation on Josh Cooper's suggestion for keeping compatibility with Puppet < `7.23.0`
No description provided.