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

(MODULES-7856) Allow optional repositories based on puppet version #258

Merged
merged 3 commits into from
Sep 26, 2018

Conversation

joshcooper
Copy link
Contributor

@joshcooper joshcooper commented Sep 21, 2018

Some types/providers maybe missing when testing against puppet6 as a
gem. Allow an optional repository to be specified which will only be
installed if the current puppet version matches the semantic version
constraint. For example, stdlib will always be installed, but
selinux_core is only installed when testing on puppet6 or above:

repositories:
  stdlib: https://github.com/puppetlabs/puppetlabs-stdlib.git
  selinux_core:
    repo: https://github.com/puppetlabs/puppetlabs-selinux_core.git
    puppet_version: ">= 6.0.0"

@joshcooper joshcooper force-pushed the optional_fixtures branch 2 times, most recently from b055df9 to 29dda22 Compare September 21, 2018 20:14
@joshcooper
Copy link
Contributor Author

Looks like it's failing on puppet 3.0 because that version doesn't depend on or vendor semantic_puppet. I'll take a look in a bit.

@bastelfreak
Copy link
Collaborator

can we please finally drop puppet 3? it is EOL since almost 2 years. is it even on the test mattix on purpose?

@joshcooper joshcooper force-pushed the optional_fixtures branch 2 times, most recently from 0e9828f to cc50f7b Compare September 21, 2018 21:36
@joshcooper
Copy link
Contributor Author

I worked around the puppet 3 issue for now (so the puppet_version attribute is ignored). But I am in favor of dropping puppet 3 from the test matrix.

@codecov-io
Copy link

codecov-io commented Sep 21, 2018

Codecov Report

Merging #258 into master will increase coverage by 0.65%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #258      +/-   ##
==========================================
+ Coverage    39.8%   40.46%   +0.65%     
==========================================
  Files          10       10              
  Lines         726      734       +8     
==========================================
+ Hits          289      297       +8     
  Misses        437      437
Impacted Files Coverage Δ
lib/puppetlabs_spec_helper/tasks/fixtures.rb 31.77% <100%> (+2.64%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 779738e...aa74b9c. Read the comment docs.

@joshcooper
Copy link
Contributor Author

👍 to dropping puppet 3.x from the travis matrix.

@DavidS does this need a PDK ticket? Should I target an earlier branch?

@puppetcla
Copy link

CLA signed by all contributors.

Copy link
Contributor

@DavidS DavidS left a comment

Choose a reason for hiding this comment

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

I'm happy enough to keep the puppet3 guard in there for now. It should be noted in the README though.

a couple of tests in https://github.com/puppetlabs/puppetlabs_spec_helper/blob/master/spec/unit/puppetlabs_spec_helper/tasks/fixtures_spec.rb are also needed.

@davidmalloncares
Copy link

Hey @joshcooper, would you be able to get a look at this one today around David Schmitts review comments? This change is currently causing issues for Modules teams and Vox for Puppet 6 so would be cool if we can get unblocked asap

@joshcooper joshcooper force-pushed the optional_fixtures branch 2 times, most recently from eecfce5 to 27ef706 Compare September 26, 2018 06:15
@joshcooper joshcooper changed the title Allow optional repositories based on puppet version (MODULES-7856) Allow optional repositories based on puppet version Sep 26, 2018
@joshcooper joshcooper force-pushed the optional_fixtures branch 3 times, most recently from 7179f29 to 86a231b Compare September 26, 2018 06:49
Some types/providers maybe missing when testing against puppet6 as a
gem. Allow an optional repository to be specified which will only be
installed if the current puppet version matches the semantic version
constraint. For example, stdlib will always be installed, but
selinux_core is only installed when testing on puppet6 or above:

    repositories:
      stdlib: https://github.com/puppetlabs/puppetlabs-stdlib.git
      selinux_core:
        repo: https://github.com/puppetlabs/puppetlabs-selinux_core.git
        puppet_version: ">= 6.0.0"
@joshcooper
Copy link
Contributor Author

@DavidS this should be good now

  • Added a JIRA ticket
  • Updated docs about puppet_version only working on Puppet 4 and up
  • Reworked the code to test for the SemanticPuppet being defined, instead of relying on puppet version heuristic
  • Added tests

Copy link
Contributor

@DavidS DavidS left a comment

Choose a reason for hiding this comment

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

Thanks for the tests and cleanup. Looks great!

@DavidS DavidS merged commit 100ec27 into puppetlabs:master Sep 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants