(PDK-916) Default to mocha if mock_framework isn't set

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.
 
@@ -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

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.framework_name == :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

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))