From d6d6e912be562f68976402d7dc4110f9c0bb7c3c Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Mon, 9 Apr 2018 10:16:17 -0700 Subject: [PATCH 1/3] Default to mocha if mock_framework isn't set The mocha framework has been the most common option for module testing. A recent commit to address issues with mocha 1.5 added a check for which framework is configured, but not everyone has explicitly configured a framework. Check explicitly for rspec, and fall-back to mocha otherwise. --- lib/puppetlabs_spec_helper/module_spec_helper.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/puppetlabs_spec_helper/module_spec_helper.rb b/lib/puppetlabs_spec_helper/module_spec_helper.rb index aba48e25..3818e1f1 100644 --- a/lib/puppetlabs_spec_helper/module_spec_helper.rb +++ b/lib/puppetlabs_spec_helper/module_spec_helper.rb @@ -26,10 +26,10 @@ def verify_contents(subject, title, expected_lines) c.parser = 'future' if ENV['FUTURE_PARSER'] == 'yes' c.before :each do - if c.mock_framework == :mocha - Puppet.features.stubs(:root?).returns(true) - else + if c.mock_framework == :rspec allow(Puppet.features).to receive(:root?).and_return(true) + else + Puppet.features.stubs(:root?).returns(true) end # stringify_facts and trusted_node_data were removed in puppet4 From 4cfa1826cb5ec8c408fec74f46cb9d9be47bfdbd Mon Sep 17 00:00:00 2001 From: David Schmitt Date: Tue, 10 Apr 2018 14:20:12 +0100 Subject: [PATCH 2/3] Whitespace fixes --- README.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 16387c90..f43d7424 100644 --- a/README.md +++ b/README.md @@ -345,14 +345,14 @@ remembering to duplicate any existing environment variables: env: - FUTURE_PARSER=yes CI_NODE_TOTAL=2 CI_NODE_INDEX=1 - FUTURE_PARSER=yes CI_NODE_TOTAL=2 CI_NODE_INDEX=2 - + #### Running tests tagged with test tiers -To run tests tagged with risk levels set the ``TEST_TIERS`` environment variable to a comma-separated list of +To run tests tagged with risk levels set the ``TEST_TIERS`` environment variable to a comma-separated list of the appropriate tiers. -For example: to run tests marked ``tier_high => true`` and ``tier_medium => true`` in the same test run set the +For example: to run tests marked ``tier_high => true`` and ``tier_medium => true`` in the same test run set the environment variable``TEST_TIERS=high,medium`` - + Note, if the ``TEST_TIERS`` environment variable is set to empty string or nil, all tiers will be executed. @@ -451,7 +451,7 @@ This is related to a registry security setting requiring elevated privileges to Currently, there are two known approaches to get around this problem. -- run your windows shell (cmd) as an Administrator +- run your windows shell (cmd) as an Administrator or - modify the registry entry settings to allow symbolic links to be created. From 695876f114a284ad6457e200ecd651824c3b37c7 Mon Sep 17 00:00:00 2001 From: David Schmitt Date: Tue, 10 Apr 2018 14:26:20 +0100 Subject: [PATCH 3/3] (PDK-916) ask folks to request mocha if they really want it If a PSH user does not have a `mock_framework` selected before loading `puppetlabs_spec_helper/puppet_spec_helper` warn of the fact that PSH is doing something for them. This does not change the actual behaviour yet, but allows users to select a framework and override the default. --- README.md | 47 +++++++++++++++++++ .../module_spec_helper.rb | 2 +- .../puppet_spec_helper.rb | 27 ++++++++--- 3 files changed, 69 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index f43d7424..26f71179 100644 --- a/README.md +++ b/README.md @@ -463,5 +463,52 @@ The following links may give you some insight into why... [Microsoft TechNet](https://technet.microsoft.com/en-us/library/cc754077.aspx) +mock_with +========= + +There are two major mocking frameworks in modules test suites today: [mocha](https://rubygems.org/gems/mocha) and [rspec-mocks](https://rubygems.org/gems/rspec-mocks). It is recommended to choose rspec-mocks explicitely by specifying `mock_with` either in your `spec_helper.rb` like so: + +``` +RSpec.configure do |c| + c.mock_with :rspec +end +require 'puppetlabs_spec_helper/module_spec_helper' +``` + +or by using the PDK's [`mock_with` option in `.sync.yml`](https://github.com/puppetlabs/pdk-templates#specspec_helperrb) and `pdk update`. + +You can also continue to use mocha by explicitly specifying `:mocha`, following the [mocha documentation](http://gofreerange.com/mocha/docs/). + +Migration +--------- + +To migrate from mocha to rspec-mocks, in many simple cases the following two kinds of changes are all you need: + +Translate all stubs: +``` +context.stubs(:some_method).with(arguments).returns('value') +``` +to this: +``` +allow(context).to receive(:some_method).with(arguments).and_returns('value') +``` + +Translate all expectations: +``` +context.expects(:some_method).with(arguments).returns('value') +``` +to this: +``` +expect(context).to receive(:some_method).with(arguments).and_returns('value') +``` + +Rationale +--------- + +* As a part of the RSpec project, rspec-mocks integration is better. +* mocha is extending base ruby objects with its `stubs`, and `expects` methods, polluting the global namespace, with a potential for subtle and gnarly errors. +* Currently both rspec-mocks and mocha get loaded, leading to test suites that use both. +* puppetlabs_spec_helper loads and configures mocha unconditionally, causing friction when a different mocking framework is wanted. +* mocha is an additional dependency to carry. EOF diff --git a/lib/puppetlabs_spec_helper/module_spec_helper.rb b/lib/puppetlabs_spec_helper/module_spec_helper.rb index 3818e1f1..a230c528 100644 --- a/lib/puppetlabs_spec_helper/module_spec_helper.rb +++ b/lib/puppetlabs_spec_helper/module_spec_helper.rb @@ -26,7 +26,7 @@ def verify_contents(subject, title, expected_lines) c.parser = 'future' if ENV['FUTURE_PARSER'] == 'yes' c.before :each do - if c.mock_framework == :rspec + if c.mock_framework.framework_name == :rspec allow(Puppet.features).to receive(:root?).and_return(true) else Puppet.features.stubs(:root?).returns(true) diff --git a/lib/puppetlabs_spec_helper/puppet_spec_helper.rb b/lib/puppetlabs_spec_helper/puppet_spec_helper.rb index aaf88962..03b2da92 100644 --- a/lib/puppetlabs_spec_helper/puppet_spec_helper.rb +++ b/lib/puppetlabs_spec_helper/puppet_spec_helper.rb @@ -3,12 +3,21 @@ # Don't want puppet getting the command line arguments for rake or autotest ARGV.clear -# This is needed because we're using mocha with rspec instead of Test::Unit or MiniTest -ENV['MOCHA_OPTIONS']='skip_integration' - require 'puppet' require 'rspec/expectations' -require 'mocha/api' + +# Detect whether the module is overriding the choice of mocking framework +# @mock_framework is used since more than seven years, and we need to avoid +# `mock_framework`'s autoloading to distinguish between the default, and +# the module's choice. +# See also below in RSpec.configure +if RSpec.configuration.instance_variable_get(:@mock_framework).nil? + # This is needed because we're using mocha with rspec instead of Test::Unit or MiniTest + ENV['MOCHA_OPTIONS']='skip_integration' + + # Current versions of RSpec already load this for us, but who knows what's used out there? + require 'mocha/api' +end require 'pathname' require 'tmpdir' @@ -118,8 +127,14 @@ def handle(msg) # And here is where we do the main rspec configuration / setup. RSpec.configure do |config| - # Some modules depend on having mocha set up for them - config.mock_with :mocha + # Detect whether the module is overriding the choice of mocking framework + # @mock_framework is used since more than seven years, and we need to avoid + # `mock_framework`'s autoloading to distinguish between the default, and + # the module's choice. + if config.instance_variable_get(:@mock_framework).nil? + RSpec.warn_deprecation('puppetlabs_spec_helper: defaults `mock_with` to `:mocha`. See https://github.com/puppetlabs/puppetlabs_spec_helper#mock_with to choose a sensible value for you') + config.mock_with :mocha + end # determine whether we can use the new API or not, and call the appropriate initializer method. if (defined?(Puppet::Test::TestHelper))