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.

Sign up for GitHub

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

Jump to bottom

Fix travis #324

Merged
merged 5 commits into from
Aug 22, 2016
Merged

Fix travis #324

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b30c81a
Fix TravisCI for Ruby < 2
ghoneycutt Aug 22, 2016
132ce39
Explicitly support Puppet v4.3, 4.4 and 4.5
ghoneycutt Aug 22, 2016
880d2b3
Simplify CI tests by ensuring the use of a newer puppetlabs_spec_helper
ghoneycutt Aug 22, 2016
ca88207
Fix existing bug in spec tests
ghoneycutt Aug 22, 2016
dd9af35
Fix style
ghoneycutt Aug 22, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion .travis.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,14 @@ env:
- PUPPET_GEM_VERSION="~> 4.0.0"
- PUPPET_GEM_VERSION="~> 4.1.0"
- PUPPET_GEM_VERSION="~> 4.2.0"
- PUPPET_GEM_VERSION="~> 4.3.0"
- PUPPET_GEM_VERSION="~> 4.4.0"
- PUPPET_GEM_VERSION="~> 4.5.0"
- PUPPET_GEM_VERSION="~> 4"

sudo: false

script: 'bundle exec metadata-json-lint metadata.json && bundle exec rake validate && bundle exec rake lint && SPEC_OPTS="--format documentation" bundle exec rake spec'
script: 'bundle exec rake validate && bundle exec rake lint && SPEC_OPTS="--format documentation" bundle exec rake spec'

matrix:
fast_finish: true
Expand All @@ -48,6 +51,12 @@ matrix:
env: PUPPET_GEM_VERSION="~> 4.1.0"
- rvm: 1.8.7
env: PUPPET_GEM_VERSION="~> 4.2.0"
- rvm: 1.8.7
env: PUPPET_GEM_VERSION="~> 4.3.0"
- rvm: 1.8.7
env: PUPPET_GEM_VERSION="~> 4.4.0"
- rvm: 1.8.7
env: PUPPET_GEM_VERSION="~> 4.5.0"
- rvm: 1.8.7
env: PUPPET_GEM_VERSION="~> 4"

Expand Down
9 changes: 7 additions & 2 deletions Gemfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ source ENV['GEM_SOURCE'] || "https://rubygems.org"
group :development, :test do
gem 'metadata-json-lint', :require => false
gem 'rspec-puppet', :require => false
gem 'puppetlabs_spec_helper', :require => false
gem 'puppetlabs_spec_helper', '>= 1.1.1'
gem 'puppet-lint', :require => false
gem 'pry', :require => false
gem 'simplecov', :require => false
Expand All @@ -18,7 +18,12 @@ else
end

if RUBY_VERSION >= '1.8.7' and RUBY_VERSION < '2.0'
gem 'json', '~> 1.0'
# json 2.x requires ruby 2.0. Lock to 1.8
gem 'json', '~> 1.8'
# json_pure 2.0.2 requires ruby 2.0. Lock to 2.0.1
gem 'json_pure', '= 2.0.1'
else
gem 'json'
end

if facterversion = ENV['FACTER_GEM_VERSION']
Expand Down
4 changes: 2 additions & 2 deletions manifests/install.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
$python = $::python::version ? {
'system' => 'python',
'pypy' => 'pypy',
default => "${python::version}",
default => "${python::version}", # lint:ignore:only_variable_string
Copy link

Choose a reason for hiding this comment

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

This should be converted to just be default => $python::version rather than disabling the line message. Placing the variable in quotes like this does not cast it to a string, and the behavior both ways is identical. You would need to use sprintf to cast it to a string, but that would actually change the current behavior.

Copy link

Choose a reason for hiding this comment

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

root@testbox:~# cat test.pp 
$version=123
warning(type($version))
warning(type("${version}"))

root@testbox:~# puppet apply test.pp 
Warning: Scope(Class[main]): integer
Warning: Scope(Class[main]): integer

Copy link

@cryptk cryptk Aug 22, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

And doing it another way just to demonstrate the "universality" of how puppet likes to put things into their little boxes:

root@testbox:~# cat test.pp 
$version_int=123
$version_quoted = "${version_int}"
warning(type($version_int))
warning(type($version_quoted))

root@testbox:~# puppet apply test.pp 
Warning: Scope(Class[main]): integer
Warning: Scope(Class[main]): integer

In order to actually cast it into a string, you would need to do something weird like this:

root@testbox:~# cat test2.pp 
$version=123
$version_str = sprintf('"%s"', $version)
warning(type($version_str))

root@cryptkcoding:~# puppet apply test2.pp 
Warning: Scope(Class[main]): string

But again, doing that with sprintf would actually change the current behavior of the module as the current behavior is to treat the $python::version variable as either an int or a str depending on it's contents. Removing the quotes and curly braces will resolve the lint issue while preserving that behavior.

Copy link
Member Author

Choose a reason for hiding this comment

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

@cryptk My only concern is that $python is stringified on purpose and by changing the current behavior it will break something that is expecting that $python is a string.

I'm in favor of doing it your way, though that needs to be tested. Please send a PR that changes the behavior.

Copy link

Choose a reason for hiding this comment

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

I think you misunderstood what I am saying. Placing the variable in quotes like that does NOT stringify it. It is still an integer (assuming that the input is just digits) or a string (if the input is not just digits) either way.

Changing it to read

default => $python::version

is behaviorally identical to the quoted and braced version, just more stylistically correct. Please read my previous line comments which demonstrate this.

}

$pythondev = $::osfamily ? {
Expand Down Expand Up @@ -220,8 +220,8 @@
}

package { 'gunicorn':
name => $python::gunicorn_package_name,
ensure => $gunicorn_ensure,
name => $python::gunicorn_package_name,
}
}
}
2 changes: 1 addition & 1 deletion spec/defines/pip_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@
context "adds proxy to search command if set to latest" do
let (:params) {{ :proxy => "http://my.proxy:3128", :ensure => 'latest' }}
it { is_expected.to contain_exec("pip_install_rpyc").with_command("pip wheel --help > /dev/null 2>&1 && { pip wheel --version > /dev/null 2>&1 || wheel_support_flag='--no-use-wheel'; } ; { pip --log /tmp/pip.log install --upgrade $wheel_support_flag --proxy=http://my.proxy:3128 rpyc || pip --log /tmp/pip.log install --upgrade --proxy=http://my.proxy:3128 rpyc ;}") }
it { is_expected.to contain_exec("pip_install_rpyc").with_unless('pip search --proxy=http://my.proxy:3128 rpyc | grep -i INSTALLED | grep -i latest') }
it { is_expected.to contain_exec("pip_install_rpyc").with_unless('pip search --proxy=http://my.proxy:3128 rpyc | grep -i INSTALLED.*latest') }
end
end

Expand Down