-
Notifications
You must be signed in to change notification settings - Fork 28
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
Refactor unit test #80
Conversation
a6e3f03
to
498a143
Compare
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 usually end up with code like:
describe 'openvmtools' do
on_supported_os.each do |os, os_facts|
context "os #{os}" do
let(:facts) { os_facts.merge(virtual: 'vmware') }
let(:package_name) do
case facts[:os]['family']
when 'RedHat'
'open-vm-tools'
else
'whatever'
end
it { is_expected.to contain_package(package_name) }
end
end
end
end
Note the difference between os_facts
and facts
is that the latter one is the fully merge one where os_facts
is provided by the top layer. You can only use it within it
blocks.
498a143
to
5c6ee95
Compare
'vmware_guestd' | ||
else | ||
'vmtoolsd' | ||
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.
I see missing a end
here.. but maybe my builtin ruby interpreter misinterprets something in the end of the day 🤔
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.
The end is after the last it {} statement
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.
@jay7x is correct that the let
block needs to be closed
e8953f6
to
d063576
Compare
d063576
to
96449dc
Compare
96449dc
to
699a160
Compare
Is it still |
For now I think is ok |
Pull Request (PR) description
Refactoring the unit test to iterate on the supported os
This Pull Request (PR) fixes the following issues
Fixes #79