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

EPEL only makes sense on RH systems but not Fedora #297

Merged
merged 5 commits into from
Dec 8, 2016
Merged

EPEL only makes sense on RH systems but not Fedora #297

merged 5 commits into from
Dec 8, 2016

Conversation

jcpunk
Copy link
Contributor

@jcpunk jcpunk commented Apr 11, 2016

This update to params.pp only sets EPEL true if it would make sense

@ghoneycutt
Copy link
Member

+1 Though there should be a spec test that tests for the inclusion of the epel class based on the platform. The README should also be updated for the use_epel parameter.

I'll look into the issue with ruby 1.8.7 failing.

@ghoneycutt
Copy link
Member

working in PR #298 to fix ruby 1.8.7

@ghoneycutt
Copy link
Member

If you rebase off of master, it will pull in the changes necessary to get ruby 1.8.7 working for this PR.

@jcpunk
Copy link
Contributor Author

jcpunk commented May 11, 2016

I have no idea how to test for the state of a variable withing a class. Any pointers?

@ghoneycutt
Copy link
Member

You are not testing for a variable, which cannot be done, instead you are testing that a resource is or is not included in the catalog. So the tests would look like

it { should contain_class('epel') }

and

it { should_not contain_class('epel') }

@@ -90,6 +90,13 @@
it { is_expected.to contain_package("python-dev").with_ensure('absent') }
end
end

describe "EPEL need not apply" do
context "default/empty" do
Copy link
Member

Choose a reason for hiding this comment

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

need to set the facts with values for osfamily and operatingsystem. Will need three different tests to correspond with the code in params.

describe 'epel class' do
  context 'when osfamily is RedHat and operatingsystem is Fedora' do
    ...
  end
  context 'when osfamily is RedHat and operatingsystem is Fedora' do
    ...
  end
  context 'when osfamily is RedHat and operatingsystem is not Fedora' do
    ...
  end
  context 'when osfamily is not RedHat' do
    ...
  end
end

to set the facts, it would look like this

context 'whatever' do
  let(:facts) do
    {
      :osfamily        => 'RedHat',
      :operatingsystem => 'Fedora',
    }
end

@jcpunk
Copy link
Contributor Author

jcpunk commented Sep 20, 2016

In this last commit, I believe I've got the tests with the right facts set inserted along with their operating systems.

@ghoneycutt ghoneycutt merged commit ca461e7 into voxpupuli:master Dec 8, 2016
@ghoneycutt
Copy link
Member

Released in 1.18.1

@jcpunk jcpunk deleted the epel-rhonly branch December 8, 2016 20:49
sergiik pushed a commit to sergiik/puppet-python that referenced this pull request Mar 14, 2018
* EPEL only makes sense on RH systems but not Fedora

* Updated documentation, added test

* updated tests: is RHEL has EPEL, is Fedora no EPEL, is RPM based but not RHEL, is Debian
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.

2 participants