-
Notifications
You must be signed in to change notification settings - Fork 193
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-6708) fix tests for windows agent to agent upgrades #285
(MODULES-6708) fix tests for windows agent to agent upgrades #285
Conversation
Waiting for CLA signature by @speedofdark @speedofdark - We require a Contributor License Agreement (CLA) for people who contribute to Puppet, but we have an easy click-through license with instructions, which is available at https://cla.puppet.com/ Note: if your contribution is trivial and you think it may be exempt from the CLA, please post a short reply to this comment with details. http://docs.puppet.com/community/trivial_patch_exemption.html |
CLA signed by all contributors. |
spec/acceptance/class_spec.rb
Outdated
# puppet service is not started automagically on windows | ||
describe service('puppet') do | ||
it { is_expected.to be_enabled } | ||
it { is_expected.to_not be_running } |
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 appears very odd....Not sure we should be expecting this.
ec3369e
to
51bd509
Compare
* remove tests around configuration migration as they are not managed by the upgrade * changed base version from 3.x to 1.7.0 and associated changes * specified upgrade target as 1.10.0 Three Windows tests are currently red. Two appear to be legitimate failues related to idempotency. The third is related to service startup. Tests have only been updated to make things green, with no other refactoring applied as yet.
51bd509
to
e6d801e
Compare
We reverted the change that Glenn commented on above; this results in a total of 3 red tests for now. This will be addressed separately. |
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.
LGTM. Any reason why 1.7.0 and 1.10.0 were chosen?
1.7.0 was the version shipped in the 2016.4.0 (the initial LTS release), and the module doesn't work well for 5 yet on every OS we support so we use 1.10.0 as the last of the 4 series. We could pick 1.10.12 instead. It doesn't really matter (shrug). |
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.
Changes look sane. Ship It!
@@ -135,24 +134,19 @@ def setup_puppet_on(host, opts = {}) | |||
|
|||
puts "Setup foss puppet on #{host}" | |||
configure_defaults_on host, 'foss' | |||
install_puppet_on host, :version => ENV['PUPPET_CLIENT_VERSION'] || '3.8.6' | |||
install_puppet_agent_on host, :version => ENV['PUPPET_CLIENT_VERSION'] || '1.7.0' |
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.
👍
Three Windows tests are currently red. Two appear to be legitimate failues related to
idempotency. The third is related to service startup.
Tests have only been updated to make things green, with no other refactoring applied
as yet.