Day 17 in the #vDM30in30
Header image taken from https://www.relishapp.com/rspec/rspec-core/docs/example-groups/shared-examples
One of the areas where it’s easy to have WET (We Enjoy Typing) code that you want to DRY (Don’t Repeat Yourself) up, is tests.
When writing tests for similar areas of code, it’s easy to end up copy and pasting. You have to pieces of functionality that need to be tested in the same way, so why not copy and paste the testing code?
Why duplication is bad…
What’s the problem with copy-pasted code? It makes editing more difficult due to unnecessary increases in complexity and length.
Duplication comes with an increased maintenance cost: duplication means more places to be updated when a change occurs, meaning a higher chance of human error when changing, and duplicated code often ends up forgotten or overlooked.
There’s a good summary of the issue here
Meta-programming in Specs
So WET code is bad…how can we fix this in specs?
One solution I’d seen commonly was to just use standard array iteration to fix this:
require 'spec_helper'
describe ReminderMailer, type: :mailer do
# Half the code yay! :D
['daily','weekly'].each do |time_format|
describe time_format do
let(:time_format) { time_format }
let(:user) do
mock_model(User, nickname: 'David',
email: 'david@example.com',
languages: ['Ruby'],
skills: [],
pull_requests: double(:pull_request, year: []),
suggested_projects: [])
end
# How do I do this bit better?
let(:mail) {
if time_format == 'daily'
ReminderMailer.daily(user)
else
ReminderMailer.weekly(user)
end
}
end
However, this isn’t super easy to maintain: I have to change the array and add more logic when new things get created. And what if I want to use this in multiple spec files?
It’s not ideal.
How to do it properly
I actually opened a question on the Code Review Stack Exchange page about a different issue I was having with the spec, and I got the best kind of answer: not only the solution to the question I asked, but a better way of abstracting the test!
tokland gave me a better answer:
describe ReminderMailer, type: :mailer do
let(:user) { ... }
shared_examples "a reminder mailer" do |subject:, body:|
it 'renders the subject' do
expect(mail.subject).to eq(subject)
end
it 'renders the receiver email' do
expect(mail.to).to eq([user.email])
end
it 'renders the sender email' do
expect(mail['From'].to_s).to eq('24 Pull Requests <info@24pullrequests.com>')
end
it 'uses nickname' do
expect(mail.body.encoded).to match(user.nickname)
end
it 'contains periodicity in body' do
expect(mail.body.encoded).to match(body)
end
end
describe 'daily' do
let(:mail) { ReminderMailer.daily(user) }
it_behaves_like "a reminder mailer",
subject: '[24 Pull Requests] Daily Reminder',
body: 'today'
end
describe 'weekly' do
let(:mail) { ReminderMailer.weekly(user) }
it_behaves_like "a reminder mailer",
subject: '[24 Pull Requests] Weekly Reminder',
body: 'week'
end
end
This is using the shared_examples helper, which is a way of making an abstracted spec that you can re-use.
What’s great about this is, if a new email reminder for monthly was implemented, the spec could be extended fairly easily:
describe 'monthly' do
let(:mail) { ReminderMailer.monthly(user) }
it_behaves_like "a reminder mailer",
subject: '[24 Pull Requests] Monthly Reminder'
body: 'monthly'
end
end
An rspec-puppet example
rspec-puppet uses rspec core code under the hood, so it can also used shared_examples.
For example, a pattern I use often is to create a shared_example for the standard checks for a Puppet class, then use a it_behaves_like
loop with rspec-puppet-facts, which creates an array from all the platforms you say you support in the metadata.json
file.
Essentially it means you can test your base code works on all your platforms you say you support with one block of code, rather than having to copy and paste specs for each platform.
An example from my Cockpit module:
require 'spec_helper'
describe 'cockpit' do
shared_examples 'no parameters' do
let(:params) {{ }}
it { should compile.with_all_deps }
it { should create_class('cockpit') }
it { should contain_class('cockpit::params') }
it { should contain_class('cockpit::repo').that_comes_before('Class[cockpit::install]') }
it { should contain_class('cockpit::install').that_comes_before('Class[cockpit::config]') }
it { should contain_class('cockpit::config') }
it { should contain_class('cockpit::service').that_subscribes_to('Class[cockpit::config]') }
it { should contain_ini_setting('Cockpit LoginTitle').with(
:ensure => 'present',
:path => '/etc/cockpit/cockpit.conf',
:section => 'WebService',
:setting => 'LoginTitle',
:value => facts[:fqdn],
:show_diff => true,
) }
it { should contain_ini_setting('Cockpit LoginTitle').with(
:ensure => 'present',
:path => '/etc/cockpit/cockpit.conf',
:section => 'WebService',
:setting => 'LoginTitle',
:value => facts[:fqdn],
:show_diff => true,
) }
it { should contain_ini_setting('Cockpit MaxStartups').with(
:ensure => 'present',
:path => '/etc/cockpit/cockpit.conf',
:section => 'WebService',
:setting => 'MaxStartups',
:value => '10',
:show_diff => true,
) }
it { should contain_ini_setting('Cockpit AllowUnencrypted').with(
:ensure => 'present',
:path => '/etc/cockpit/cockpit.conf',
:section => 'WebService',
:setting => 'AllowUnencrypted',
:value => false,
:show_diff => true,
) }
it { should contain_service('cockpit').with(
:ensure => 'running',
# :enable => 'true'
)}
it { should contain_package('cockpit').with_ensure('installed') }
context 'with custom parameters' do
context 'package name' do
let(:params) {{ 'package_name' => 'custom-package' }}
it { should contain_package("#{params['package_name']}") }
end
context 'package version' do
let(:params) {{ 'package_version' => 'latest' }}
it { should contain_package('cockpit').with_ensure('latest') }
end
context 'manage service' do
let(:params) {{ 'manage_service' => false }}
it { should_not contain_service("cockpit") }
end
context 'service name' do
let(:params) {{ 'service_name' => 'custom-service' }}
it { should contain_service("#{params['service_name']}") }
end
context 'port' do
let(:params) {{ 'port' => '7777' }}
it {
should contain_file('/etc/systemd/system/cockpit.socket.d/listen.conf').
with(:ensure => 'file')
should contain_file('/etc/systemd/system/cockpit.socket.d/listen.conf').
with_content(/ListenStream=7777/)
should contain_file('/etc/systemd/system/cockpit.socket.d/').
with(:ensure => 'directory')
should contain_exec('Cockpit systemctl daemon-reload').with(
:command => 'systemctl daemon-reload',
:refreshonly => true,
:path => facts[:path],
)
}
end
end
end
on_supported_os.each do |os, facts|
context "on #{os}" do
let(:facts) do
facts.merge({
:fqdn => 'cockpit.example.com',
:path => '/usr/lib64/qt-3.3/bin:/usr/local/sbin:/usr/local/bin:/sbin:/bin:/usr/sbin:/usr/bin:/opt/puppetlabs/bin:/root/bin'
})
end
it_behaves_like 'no parameters'
end
end
end
An example from the Puppet codebase
For example, the dnf
application in the new Fedora builds, is almost exactly the same as the yum
applications, but there are a few small differences, such as the error level required to run it, and the command for upgrading packages, but everything else is the same.
So, in the Puppet codebase, there were two unit tests to test this, largely with the same content:
#! /usr/bin/env ruby
require 'spec_helper'
provider_class = Puppet::Type.type(:package).provider(:yum)
describe provider_class do
include PuppetSpec::Fixtures
let(:name) { 'mypackage' }
let(:resource) do
Puppet::Type.type(:package).new(
:name => name,
:ensure => :installed,
:provider => 'yum'
)
end
let(:provider) do
provider = provider_class.new
provider.resource = resource
provider
end
before do
provider_class.stubs(:command).with(:cmd).returns('/usr/bin/yum')
provider.stubs(:rpm).returns 'rpm'
provider.stubs(:get).with(:version).returns '1'
provider.stubs(:get).with(:release).returns '1'
provider.stubs(:get).with(:arch).returns 'i386'
end
describe 'provider features' do
it { is_expected.to be_versionable }
it { is_expected.to be_install_options }
it { is_expected.to be_virtual_packages }
end
# provider should repond to the following methods
[:install, :latest, :update, :purge, :install_options].each do |method|
it "should have a(n) #{method}" do
expect(provider).to respond_to(method)
end
end
describe 'when installing' do
before(:each) do
Puppet::Util.stubs(:which).with("rpm").returns("/bin/rpm")
provider.stubs(:which).with("rpm").returns("/bin/rpm")
Puppet::Util::Execution.expects(:execute).with(["/bin/rpm", "--version"], {:combine => true, :custom_environment => {}, :failonfail => true}).returns("4.10.1\n").at_most_once
Facter.stubs(:value).with(:operatingsystemmajrelease).returns('6')
end
it 'should call yum install for :installed' do
resource.stubs(:should).with(:ensure).returns :installed
Puppet::Util::Execution.expects(:execute).with(['/usr/bin/yum', '-d', '0', '-e', '0', '-y', :install, 'mypackage'])
provider.install
end
context 'on el-5' do
before(:each) do
Facter.stubs(:value).with(:operatingsystemmajrelease).returns('5')
end
it 'should catch yum install failures when status code is wrong' do
resource.stubs(:should).with(:ensure).returns :installed
Puppet::Util::Execution.expects(:execute).with(['/usr/bin/yum', '-e', '0', '-y', :install, name]).returns("No package #{name} available.")
expect {
provider.install
}.to raise_error(Puppet::Error, "Could not find package #{name}")
end
end
it 'should use :install to update' do
provider.expects(:install)
provider.update
end
it 'should be able to set version' do
version = '1.2'
resource[:ensure] = version
Puppet::Util::Execution.expects(:execute).with(['/usr/bin/yum', '-d', '0', '-e', '0', '-y', :install, "#{name}-#{version}"])
provider.stubs(:query).returns :ensure => version
provider.install
end
it 'should handle partial versions specified' do
version = '1.3.4'
resource[:ensure] = version
Puppet::Util::Execution.expects(:execute).with(['/usr/bin/yum', '-d', '0', '-e', '0', '-y', :install, 'mypackage-1.3.4'])
provider.stubs(:query).returns :ensure => '1.3.4-1.el6'
provider.install
end
it 'should be able to downgrade' do
current_version = '1.2'
version = '1.0'
resource[:ensure] = '1.0'
Puppet::Util::Execution.expects(:execute).with(['/usr/bin/yum', '-d', '0', '-e', '0', '-y', :downgrade, "#{name}-#{version}"])
provider.stubs(:query).returns(:ensure => current_version).then.returns(:ensure => version)
provider.install
end
it 'should accept install options' do
resource[:ensure] = :installed
resource[:install_options] = ['-t', {'-x' => 'expackage'}]
Puppet::Util::Execution.expects(:execute).with(['/usr/bin/yum', '-d', '0', '-e', '0', '-y', ['-t', '-x=expackage'], :install, name])
provider.install
end
it 'allow virtual packages' do
resource[:ensure] = :installed
resource[:allow_virtual] = true
Puppet::Util::Execution.expects(:execute).with(['/usr/bin/yum', '-d', '0', '-e', '0', '-y', :list, name]).never
Puppet::Util::Execution.expects(:execute).with(['/usr/bin/yum', '-d', '0', '-e', '0', '-y', :install, name])
provider.install
end
end
describe 'when uninstalling' do
it 'should use erase to purge' do
Puppet::Util::Execution.expects(:execute).with(['/usr/bin/yum', '-y', :erase, name])
provider.purge
end
end
# [...] Even more tests that aren't being specifically tested in DNF
end
However, we had a duplicated spec for DNF:
# puppet/spec/unit/provider/package/dnf_spec.rb
require 'spec_helper'
# Note that much of the functionality of the dnf provider is already tested with yum provider tests,
# as yum is the parent provider.
provider_class = Puppet::Type.type(:package).provider(:dnf)
describe provider_class do
let(:name) { 'mypackage' }
let(:resource) do
Puppet::Type.type(:package).new(
:name => name,
:ensure => :installed,
:provider => 'dnf'
)
end
let(:provider) do
provider = provider_class.new
provider.resource = resource
provider
end
before do
provider_class.stubs(:command).with(:cmd).returns('/usr/bin/dnf')
provider.stubs(:rpm).returns 'rpm'
provider.stubs(:get).with(:version).returns '1'
provider.stubs(:get).with(:release).returns '1'
provider.stubs(:get).with(:arch).returns 'i386'
end
describe 'provider features' do
it { is_expected.to be_versionable }
it { is_expected.to be_install_options }
it { is_expected.to be_virtual_packages }
end
describe "default provider" do
before do
Facter.expects(:value).with(:operatingsystem).returns("fedora")
end
it "should be the default provider on Fedora 22" do
Facter.expects(:value).with(:operatingsystemmajrelease).returns('22')
expect(described_class.default?).to be_truthy
end
it "should be the default provider on Fedora 23" do
Facter.expects(:value).with(:operatingsystemmajrelease).returns('23')
expect(described_class.default?).to be_truthy
end
end
# provider should repond to the following methods
[:install, :latest, :update, :purge, :install_options].each do |method|
it "should have a(n) #{method}" do
expect(provider).to respond_to(method)
end
end
describe 'when installing' do
before(:each) do
Puppet::Util.stubs(:which).with("rpm").returns("/bin/rpm")
provider.stubs(:which).with("rpm").returns("/bin/rpm")
Puppet::Util::Execution.expects(:execute).with(["/bin/rpm", "--version"], {:combine => true, :custom_environment => {}, :failonfail => true}).returns("4.10.1\n").at_most_once
Facter.stubs(:value).with(:operatingsystemmajrelease).returns('22')
end
it 'should call dnf install for :installed' do
resource.stubs(:should).with(:ensure).returns :installed
Puppet::Util::Execution.expects(:execute).with(['/usr/bin/dnf', '-d', '0', '-e', '1', '-y', :install, 'mypackage'])
provider.install
end
it 'should be able to downgrade' do
current_version = '1.2'
version = '1.0'
resource[:ensure] = '1.0'
Puppet::Util::Execution.expects(:execute).with(['/usr/bin/dnf', '-d', '0', '-e', '1', '-y', :downgrade, "#{name}-#{version}"])
provider.stubs(:query).returns(:ensure => current_version).then.returns(:ensure => version)
provider.install
end
it 'should accept install options' do
resource[:ensure] = :installed
resource[:install_options] = ['-t', {'-x' => 'expackage'}]
Puppet::Util::Execution.expects(:execute).with(['/usr/bin/dnf', '-d', '0', '-e', '1', '-y', ['-t', '-x=expackage'], :install, name])
provider.install
end
end
describe 'when uninstalling' do
it 'should use erase to purge' do
Puppet::Util::Execution.expects(:execute).with(['/usr/bin/dnf', '-y', :erase, name])
provider.purge
end
end
describe "executing yum check-update" do
it "passes repos to enable to 'yum check-update'" do
Puppet::Util::Execution.expects(:execute).with do |args, *rest|
expect(args).to eq %w[/usr/bin/dnf check-update --enablerepo=updates --enablerepo=fedoraplus]
end.returns(stub(:exitstatus => 0))
described_class.check_updates(%w[updates fedoraplus], [], [])
end
end
end
So, what we can do is take all the tests from the yum spec, and turn them into a shared_example, which is exactly what I did:
shared_examples "RHEL package provider" do |provider_class, provider_name|
describe provider_name do
let(:name) { 'mypackage' }
let(:resource) do
Puppet::Type.type(:package).new(
:name => name,
:ensure => :installed,
:provider => provider_name
)
end
let(:provider) do
provider = provider_class.new
provider.resource = resource
provider
end
let(:arch) { 'x86_64' }
let(:arch_resource) do
Puppet::Type.type(:package).new(
:name => "#{name}.#{arch}",
:ensure => :installed,
:provider => provider_name
)
end
let(:arch_provider) do
provider = provider_class.new
provider.resource = arch_resource
provider
end
case provider_name
when 'yum'
let(:error_level) { '0' }
when 'dnf'
let(:error_level) { '1' }
when 'tdnf'
let(:error_level) { '1' }
end
case provider_name
when 'yum'
let(:upgrade_command) { 'update' }
when 'dnf'
let(:upgrade_command) { 'upgrade' }
when 'tdnf'
let(:upgrade_command) { 'upgrade' }
end
before do
provider_class.stubs(:command).with(:cmd).returns("/usr/bin/#{provider_name}")
provider.stubs(:rpm).returns 'rpm'
provider.stubs(:get).with(:version).returns '1'
provider.stubs(:get).with(:release).returns '1'
provider.stubs(:get).with(:arch).returns 'i386'
end
describe 'provider features' do
it { is_expected.to be_versionable }
it { is_expected.to be_install_options }
it { is_expected.to be_virtual_packages }
end
# provider should repond to the following methods
[:install, :latest, :update, :purge, :install_options].each do |method|
it "should have a(n) #{method}" do
expect(provider).to respond_to(method)
end
end
describe 'when installing' do
before(:each) do
Puppet::Util.stubs(:which).with("rpm").returns("/bin/rpm")
provider.stubs(:which).with("rpm").returns("/bin/rpm")
Puppet::Util::Execution.expects(:execute).with(["/bin/rpm", "--version"], {:combine => true, :custom_environment => {}, :failonfail => true}).returns("4.10.1\n").at_most_once
Facter.stubs(:value).with(:operatingsystemmajrelease).returns('6')
end
it "should call #{provider_name} install for :installed" do
resource.stubs(:should).with(:ensure).returns :installed
Puppet::Util::Execution.expects(:execute).with(["/usr/bin/#{provider_name}", '-d', '0', '-e', error_level, '-y', :install, 'mypackage'])
provider.install
end
if provider_name == 'yum'
context 'on el-5' do
before(:each) do
Facter.stubs(:value).with(:operatingsystemmajrelease).returns('5')
end
it "should catch #{provider_name} install failures when status code is wrong" do
resource.stubs(:should).with(:ensure).returns :installed
Puppet::Util::Execution.expects(:execute).with(["/usr/bin/#{provider_name}", '-e', error_level, '-y', :install, name]).returns("No package #{name} available.")
expect {
provider.install
}.to raise_error(Puppet::Error, "Could not find package #{name}")
end
end
end
it 'should use :install to update' do
provider.expects(:install)
provider.update
end
it 'should be able to set version' do
version = '1.2'
resource[:ensure] = version
Puppet::Util::Execution.expects(:execute).with(["/usr/bin/#{provider_name}", '-d', '0', '-e', error_level, '-y', :install, "#{name}-#{version}"])
provider.stubs(:query).returns :ensure => version
provider.install
end
it 'should handle partial versions specified' do
version = '1.3.4'
resource[:ensure] = version
Puppet::Util::Execution.expects(:execute).with(["/usr/bin/#{provider_name}", '-d', '0', '-e', error_level, '-y', :install, 'mypackage-1.3.4'])
provider.stubs(:query).returns :ensure => '1.3.4-1.el6'
provider.install
end
it 'should be able to downgrade' do
current_version = '1.2'
version = '1.0'
resource[:ensure] = '1.0'
# etc etc
As you can see, there’s a lot of duplication there between DNF and YUM.
So, I abstracted the tests into a shared_example
, then in our specs for both yum
and dnf
I could simply refer to the shared_example abstraction with behaves_like
:
describe provider_class do
it_behaves_like 'RHEL package provider', provider_class, 'dnf'
end
describe provider_class do
it_behaves_like 'RHEL package provider', provider_class, 'yum'
end
All of this, I did in the PR I did to fix another issue..
Basically a bit of campsite coding aka. opportunistic refactoring, in the hopes that this helped someone in the future.
And, strangely enough, it did end up helping someone…
Helping others
In another PR, a few months after I did that refactoring, Maggie opened a PR to add the tdnf
provider to Puppet.
tdnf
is the package provider for PhotoOS containers. It’s almost virtually identical to dnf
.
So, what’s great about that was, for her PR Maggie only had to add the following spec for tdnf
:
require 'spec_helper'
# Note that much of the functionality of the tdnf provider is already tested with yum provider tests,
# as yum is the parent provider, via dnf
provider_class = Puppet::Type.type(:package).provider(:tdnf)
context 'default' do
it 'should be the default provider on PhotonOS' do
Facter.stubs(:value).with(:osfamily).returns(:redhat)
Facter.stubs(:value).with(:operatingsystem).returns("PhotonOS")
expect(provider_class).to be_default
end
end
describe provider_class do
it_behaves_like 'RHEL package provider', provider_class, 'tdnf'
end
So instead of a third copy-pasted spec test, that would add to the maintenance cost, she only had to add a few extra lines to the shared_example and reference that in her spec test, plus add a PhotonOS specific test inside there.
Conclusion
Hopefully this gave you some ideas on DRYing up some of your rspec code with shared_examples.