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

use structured facts #154

Merged
merged 1 commit into from
Oct 31, 2018
Merged

Conversation

Dan33l
Copy link
Member

@Dan33l Dan33l commented Oct 31, 2018

Pull Request (PR) description

move out string facts to use structured facts.

This Pull Request (PR) fixes the following issues

operatingsystemrelease: '6.4',
lsbmajdistrelease: '6',
operatingsystemmajrelease: '6'
os: { 'release' => { 'full' => '6.4', 'major' => '6' }, 'name' => 'CentOS', 'family' => 'RedHat' }
Copy link
Member

Choose a reason for hiding this comment

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

How easy would it be to start using rspec-puppet-facts?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure that tests with rspec-puppet-facts is better. But, before 4.0.0 is released i wished the module is with structured facts in its code.
If i have enough time before release, i will propose an other one about tests using rspec-puppet-facts. It is necessary to clean up some parts. I saw code/tests about old OSes.

For the moment, i propose to merge the PR as this.

Copy link
Member

Choose a reason for hiding this comment

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

As long as the facts you've mocked are accurate and you've not just written them in such a way to make the tests pass! :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Excepted if i made an error, i reported same data but with structured facts instead of string facts

Copy link
Member Author

@Dan33l Dan33l Oct 31, 2018

Choose a reason for hiding this comment

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

Tests with rspec-puppet-facts will be done in a futur PR.

@Dan33l Dan33l merged commit 8432216 into voxpupuli:master Oct 31, 2018
@Dan33l Dan33l deleted the use_structured_facts branch October 31, 2018 15:25
@mwhahaha
Copy link

mwhahaha commented Oct 31, 2018

fyi, rspec-puppet-facts doesn't have these facts so it broke folks following master. For context, https://bugs.launchpad.net/tripleo/+bug/1800944 and we are using rspec-puppet-facts (https://github.com/openstack/puppet-tripleo/blob/master/spec/classes/tripleo_profile_base_snmp_spec.rb#L74-L81)

@alexjfisher
Copy link
Member

@mwhahaha rspec-puppet-facts does have those facts, but only if you tell it to use facts from a version of facter >= 3.
See voxpupuli/rspec-puppet-facts@f86d5b0 and theforeman/puppet-puppet@07b9c7c

@alexjfisher
Copy link
Member

and here https://github.com/camptocamp/facterdb/tree/master/facts for versions available to rspec-puppet-facts

@Dan33l Dan33l added this to the 4.0.0 milestone Nov 6, 2018
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.

None yet

3 participants