-
Notifications
You must be signed in to change notification settings - Fork 83
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-6744) Add testmode switcher #158
(MODULES-6744) Add testmode switcher #158
Conversation
This has passed adhoc |
windows_agents.each do |agent| | ||
agent_is_x64 = is_x64(agent) |
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.
If the nodeset includes both 32-bit and 64-bit agents, this doesn't seem right... I think you'd want to filter the nodes for x64, then change the execute_manifest
below to execute_manifest_on
and only use 64-bit agents.
windows_agents.each do |agent| | ||
agent_is_x64 = is_x64(agent) |
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.
With multiple nodes in the nodeset, last one wins, which doesn't seem accurate if there are both 64-bit and 32-bit nodes...
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.
A handful of changes for nodeset iterations can now be done with beaker-testmode_switcher 0.4.0 shipping and adding execute_manifest_on.
end | ||
assert_no_match(/err:/, @result.stdout, 'Expected no error messages.') | ||
agent_is_x64 = is_x64(agent) | ||
end |
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.
Think this end
can be removed, and the tests can iterate over each node in the nodeset as was done previously... now that execute_manifest_on(agent,
can be used.
Replaced previous attempt with the new |
.sync.yml
Outdated
optional: | ||
':system_tests': | ||
- gem: beaker-testmode_switcher | ||
version: '<= 0.2.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.
This looks like an accident - I think we need the 0.4.0
release for execute_manifest_on
here and in Gemfile
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.
Looks good to me, though there should be a smoke test for the module installing on the master without issue. But that's outside the scope of this ticket. edit: pending the gem version update
@ThoughtCrhyme RE "smoke test for the module installing" I thought we'd get that implicitly by testing on with master? |
@glennsarti I don't think we will if we continue to guard on windows_agents like we do here: https://github.com/puppetlabs/puppetlabs-registry/pull/158/files#diff-5e01029f711c726dc52525d83b377506R104 |
But if it's doing a puppet agent -t then the master has to have the module installed in order to compile a catalog. Doesn't matter which client (the master or windows box) actually initiates the agent run. |
Updated both gemfile and sync.yml with correct (newest) versions |
.sync.yml
Outdated
optional: | ||
':system_tests': | ||
- gem: beaker-testmode_switcher | ||
version: '<= 0.4.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.
This sets a maximum version of 0.4.0
... but we want a minimum of 0.4.0
. Any of these would work:
~> 0.4
- pessimistically versioned to< 1.0
= 0.4.0
- pin>= 0.4.0
- anything going forward
First option is probably the safe bet for now...
This commit updates all the supporting files to add beaker-testmode-switcher to this module
This commit converts the tests to use beaker-testmode-switcher.
Updated with new version constraint |
No description provided.