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

(FM-8355) Add spec_helper_acceptance #210

Merged
merged 1 commit into from
Oct 31, 2019

Conversation

florindragos
Copy link
Contributor

@florindragos florindragos commented Oct 28, 2019

This way, we just need to require 'litmus_spec_helper_acceptance' in the spec_helper_acceptance of every module

@florindragos florindragos requested a review from a team as a code owner October 28, 2019 14:49
@codecov-io
Copy link

codecov-io commented Oct 28, 2019

Codecov Report

Merging #210 into master will decrease coverage by 0.11%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #210      +/-   ##
==========================================
- Coverage   59.73%   59.62%   -0.12%     
==========================================
  Files           6        6              
  Lines         534      535       +1     
==========================================
  Hits          319      319              
- Misses        215      216       +1
Impacted Files Coverage Δ
lib/puppet_litmus.rb 0% <0%> (ø) ⬆️

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 f3803c9...1b17df0. Read the comment docs.

@ekohl
Copy link
Contributor

ekohl commented Oct 28, 2019

Related issue: #48

Copy link

@scotje scotje left a comment

Choose a reason for hiding this comment

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

If you wrap all of this in a module function (e.g.PuppetLitmus.configure!) then you could write some unit tests around its behavior. Further, you could make the method take an options hash or yield a configuration object to a block to allow for more explicit customization, even if the defaults continue to look up things from the ENV and node config files.

Usage in spec_helper_acceptance.rb then looks like:

require 'puppet_litmus'
PuppetLitmus.configure!

or even:

begin
  require 'puppet_litmus'
  PuppetLitmus.configure!
rescue LoadError
  puts 'Unable to load Litmus, did you add it to your Gemfile?'
end

@florindragos
Copy link
Contributor Author

If you wrap all of this in a module function (e.g.PuppetLitmus.configure!) then you could write some unit tests around its behavior. Further, you could make the method take an options hash or yield a configuration object to a block to allow for more explicit customization, even if the defaults continue to look up things from the ENV and node config files.

Usage in spec_helper_acceptance.rb then looks like:

require 'puppet_litmus'
PuppetLitmus.configure!

or even:

begin
  require 'puppet_litmus'
  PuppetLitmus.configure!
rescue LoadError
  puts 'Unable to load Litmus, did you add it to your Gemfile?'
end

We would still need to do include PuppetLitmus to have all the helper methods available for the specs unless there is some ruby magic to include a module by calling one of its instance methods.

@florindragos
Copy link
Contributor Author

Updated code works with

require 'puppet_litmus'

include PuppetLitmus
PuppetLitmus.configure!

@ekohl
Copy link
Contributor

ekohl commented Oct 30, 2019

Looks like there's a merge conflict so a rebase is needed.

@florindragos florindragos force-pushed the acceptance-helper branch 2 times, most recently from 45fdbd4 to 8d786d7 Compare October 30, 2019 13:11
Copy link
Contributor

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

Looking at https://github.com/puppetlabs/puppetlabs_spec_helper/blob/master/lib/puppetlabs_spec_helper/puppetlabs_spec_helper.rb you can use RSpec.configure and call .extend or .include. That looks like a much cleaner solution.

lib/litmus_spec_helper_acceptance.rb Outdated Show resolved Hide resolved
lib/puppet_litmus.rb Outdated Show resolved Hide resolved
@florindragos florindragos force-pushed the acceptance-helper branch 2 times, most recently from b630aab to b508a1f Compare October 30, 2019 13:36
@ekohl
Copy link
Contributor

ekohl commented Oct 31, 2019

With the last upgrade the include PuppetLitmus is no longer needed, correct?

@florindragos
Copy link
Contributor Author

florindragos commented Oct 31, 2019

With the last upgrade the include PuppetLitmus is no longer needed, correct?

yep, just tested

require 'puppet_litmus'
PuppetLitmus.configure!

Copy link
Contributor

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

Thanks! I think this is a much cleaner way.

Currently the wiki documents how to integrate but I wonder if this is now simple enough to just include in README.md. https://github.com/puppetlabs/puppetlabs_spec_helper#usage is IMHO a great example to get started quickly and saves a click. IMHO README.md is also better because it's in the repo and thus versioned.

@michaeltlombardi michaeltlombardi merged commit c6b98f3 into puppetlabs:master Oct 31, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants