-
-
Notifications
You must be signed in to change notification settings - Fork 9
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
moving parameters into appropriate place, adding meaning unit test, f… #2
Conversation
…irst pass on reference file, expanding acceptance test to include meaningful example using codename as release
c6a4e75
to
ea3e115
Compare
manifests/repo.pp
Outdated
@@ -25,7 +29,7 @@ | |||
architecture => 'amd64', | |||
comment => $description, | |||
location => 'https://apt.releases.hashicorp.com', | |||
release => 'stable', | |||
release => $facts['os']['distro']['codename'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be done by default, right? https://github.com/puppetlabs/puppetlabs-apt/blob/c5259f83ad20d120d502d8541c75f4ee6021fec8/manifests/source.pp#L75-L83
release => $facts['os']['distro']['codename'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed in 44a4665
spec/classes/repo_spec.rb
Outdated
@@ -1,8 +1,16 @@ | |||
require 'spec_helper' | |||
|
|||
describe 'hashi_stack::repo' do | |||
describe 'hashi_stack::repo', type: 'class' do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AFAIK this isn't needed and rspec-puppet will derive it from the file path (spec/classes
).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed in 44a4665
manifests/repo.pp
Outdated
@@ -5,17 +5,21 @@ | |||
# @example |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably good to add some description now that there's more than 1 example. Something like:
# @example | |
# @example Inclusion using defaults |
spec/acceptance/standard_spec.rb
Outdated
@@ -6,12 +6,12 @@ | |||
# Using puppet_apply as a helper | |||
it 'should work with no errors based on the example' do | |||
pp = <<-EOS | |||
class { 'hashi_stack::repo': } | |||
class { 'hashi_stack::repo': } -> package { 'packer': ensure => installed } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can use multiple lines. How about:
class { 'hashi_stack::repo': } -> package { 'packer': ensure => installed } | |
include hashi_stack::repo | |
package { 'packer': | |
ensure => installed, | |
require => Class['Hashi_stack::Repo'], | |
} |
.fixtures.yml
Outdated
@@ -0,0 +1,8 @@ | |||
fixtures: | |||
forge_modules: | |||
stdlib: puppetlabs/stdlib |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you please use git urls instead of puppet forge? in general we try to run unit tests on unreleased changes from dependencies and forge releases for acceptance tests.
include hashi_stack::repo | ||
package { 'packer': | ||
ensure => installed, | ||
require => Class['Hashi_stack::Repo'], | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Btw, in the future I hope to expand voxpupuli-acceptance I hope to make it easier here to use example files. In theforeman we have shared examples for that.
…irst pass on reference file, expanding acceptance test to include meaningful example