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

Add mocked custom facts based on metadata #19

Merged
merged 1 commit into from
Dec 15, 2020
Merged

Conversation

ekohl
Copy link
Member

@ekohl ekohl commented Apr 28, 2020

No description provided.

@alexjfisher
Copy link
Member

I suppose it's not practical to look up a module's complete list of dependencies (ie dependencies of the dependencies listed in metadata.json)??

@ekohl
Copy link
Member Author

ekohl commented Apr 28, 2020

In theory we could look at the fixtures I guess. That might be the closest you get.


# Rough conversion of grepping in the puppet source:
# grep defaultfor lib/puppet/provider/service/*.rb
add_custom_fact :service_provider, ->(os, facts) do
Copy link
Member

Choose a reason for hiding this comment

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

Rubocop was complaining that I should use the lambda syntax, that is the reason why puppet/caddy looks different.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I know. As you can see by the date on the commit this has been around for a while. This repo probably should also gain rubocop.

metadata['dependencies'].each do |dependency|
case normalize_module_name(dependency['name'])
when 'camptocamp/systemd'
add_custom_fact :systemd, ->(os, facts) { facts[:service_provider] == 'systemd' }
Copy link
Member

Choose a reason for hiding this comment

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

We should also add the fact systemd_version. Sometimes a parameter requires a specific version of Systemd.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not opposed, but at this point I'm very busy since the Foreman is getting close to the 2.1 release cycle and a lot has to be done on my end. My goal of this PR was to share my intent so people don't develop the same thing again. I'm happy it did, but right now I don't really have time to test it and push for the major cleanup.

If someone wants to do so, I'd be happy if they could confirm. What I'd normally do is use modulesync to check out all modules, then grep for service_provider in modules/vopupuli/puppet-*/spec and see where it is already mocked. Update those modules to use this change, see if tests pass. If they do, get this merged, released and do a modulesync.

@alexjfisher
Copy link
Member

In theory we could look at the fixtures I guess. That might be the closest you get.

yes, that would probably work a bit better as .fixtures.yml has to include all of those dependencies anyway.

@ekohl
Copy link
Member Author

ekohl commented Apr 29, 2020

yes, that would probably work a bit better as .fixtures.yml has to include all of those dependencies anyway.

We can actually assume fixtures are checked out since rspec requires them. I think we actually have a variable for the module paths somewhere that we can read. Good idea.

@ekohl
Copy link
Member Author

ekohl commented Dec 14, 2020

Rebased and opened voxpupuli/puppet-redis#380 to see if it works. Looks like it does so I'm marking this ready for review.

@ekohl ekohl marked this pull request as ready for review December 14, 2020 23:16
@ekohl ekohl merged commit 2081fa4 into voxpupuli:master Dec 15, 2020
@ekohl ekohl deleted the facts branch December 15, 2020 14:11
@ekohl ekohl added the enhancement New feature or request label Dec 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants