Refactor Specs to Use Newer RSpec Features

Based on this draft -> reviewed patchset (https://review.openstack.org/#/c/67682/), update all of the cookbooks specs to use some more recent RSpec styles, including let(), and shared_example/shared_contexts. This improves the readability/flexibility of the specs, reduces duplicated setups, and even speeds up the tests somewhat.

This refactoring happens in roughly this order:

1. Convert the before() blocks to use let() blocks instead
2. Convert the it() blocks to use the new chef_run method instead of @chef_run variable
3. Remove duplicate before() blocks from it() blocks, and leaving any requisite node setups for that test
4. Convert methods with shared setup (image stubs) in spec_helper to shared_context/include_context
5. Convert methods with shared tests in spec_helper to shared_examples/include_examples
6. Cleanup and unnecessary use if :: in Class/Variable usages and () methods as necessary

More details about each step included below. You can also find the latest RSpec documentation here: https://www.relishapp.com/rspec/

--------------------------------------------------------------
1. Converting before blocks to let blocks
--------------------------------------------------------------

Because the current before blocks include code that takes a fixed path, it is necessary to duplicate it when need in specific tests to configure test specific node settings. For example:

  describe 'ubuntu' do
    before do
      @chef_run = ::ChefSpec::Runner.new ::UBUNTU_OPTS do |n|
        n.set['openstack']['image']['syslog']['use'] = true
        n.set['cpu'] = { 'total' => '1' }
      end
      @chef_run.converge 'openstack-image::api'
    end

at the top of the specs, then later in the file:

    it 'has configurable default_store setting for swift' do
      chef_run = ::ChefSpec::Runner.new ::UBUNTU_OPTS do |n|
        n.set['openstack']['image']['api']['default_store'] = 'swift'
      end
      chef_run.converge 'openstack-image::api'

      expect(chef_run).to upgrade_package 'python-swift'
    end

Now we have duplicate setup code and we're going to run two converges, slowing things down. To combat this, we break the setup code into smaller chunks using the let() syntax. Any block can be called in any order in tests.

  describe 'ubuntu' do
    let(:runner) { ChefSpec::Runner.new(UBUNTU_OPTS) }
    let(:node) { runner.node }
    let(:chef_run) do
      node.set_unless['cpu']['total'] = 1

      runner.converge(described_recipe)
    end

Given the code above, calling :chef_run will instantiate OR reuse :node (from :runner), which will instantiate or reuse :runner, then run converge. No matter how many times we call any of those, they only happen onces and are memoized er spec. Also take note of the described_recipe. We don't need to hard code the recipe name, but rather use the one declared at the very top of the spec file.

Now we can simply call node in our test above before cvalling chef_run without the duplicated setup:

    it 'has configurable default_store setting for swift' do
      node.set['openstack']['image']['api']['default_store'] = 'swift'

      expect(chef_run).to upgrade_package 'python-swift'
    end

On more complex setups where we need to also tweak the UBUNTU_OPTS to instruct ChefSpec to descend into an lwrp, we can juse do this in the specific spec files:

  describe 'ubuntu' do
    let(:runner) { ChefSpec::Runner.new(options) }
    let(:options) { UBUNTU_OPTS.merge(:step_into => ['my_lwrp']) }

We can even alter these now per test if we have to:

    it 'has configurable default_store setting for swift' do
      options.merge!{"vesion" => "13.10"}

      node.set['openstack']['image']['api']['default_store'] = 'swift'

      expect(chef_run).to upgrade_package('python-swift')
    end

or for the initiated:

  describe 'ubuntu' do
    let(:runner) { ChefSpec::Runner.new(options) }
    let(:options) { UBUNTU_OPTS.merge(:step_into => lwrps) }
    let(:lwrps) { ['my_lwrp'] }

Order of the lets() do not matter as they're execute on an as called/as needed basis.

--------------------------------------------------------------
2. Converting @chef_run to chef_run in specs
--------------------------------------------------------------

This is simply a matter of calling the chef_run method rather than using the @chef_run instance variable:

    it 'has configurable default_store setting for swift' do
      expect(@chef_run).to upgrade_package 'python-swift'
    end

    it 'has configurable default_store setting for swift' do
      expect(chef_run).to upgrade_package('python-swift')
    end

Note: the parentheses around the upgrade_package method call for better readability.

--------------------------------------------------------------
3. Remove duplicate before blocks
--------------------------------------------------------------

As covered by #1 above, now that we have top_level chef_run and node objects at our disposal, we can use them directly in specs while removing the setup/before configurations:

    it 'has configurable default_store setting for swift' do
      chef_run = ::ChefSpec::Runner.new ::UBUNTU_OPTS do |n|
        n.set['openstack']['image']['api']['default_store'] = 'swift'
      end
      chef_run.converge 'openstack-image::api'

      expect(chef_run).to upgrade_package 'python-swift'
    end

becomes:

    it 'has configurable default_store setting for swift' do
      node.set['openstack']['image']['api']['default_store'] = 'swift'

      expect(chef_run).to upgrade_package 'python-swift'
    end

--------------------------------------------------------------
4/5 Convert methods in spec_helper to shared_context/examples
--------------------------------------------------------------

Some of the spec_helper.rb files have methods that contain specs that are reused across several recipes.
Rubocop complains about these (complexity/# of lines) and they're not really methods, but shared examples. So, we convert them to the RSpec shared_context/examples syntax.

While the difference between shared_context and shared_examples is hard to parse and they can most times be used interchangeably, the general rule of thumbs seems to be:

If you're sharing mostly setup/before stuff: use shared_context
If you're sharing examples, or want to say "test that this thing behaves like that thing", use shared_examples

So, in the image spec_helpers:

def image_stubs # rubocop:disable MethodLength
  ::Chef::Recipe.any_instance.stub(:address_for)
    .with('lo')
    .and_return('127.0.1.1')
  ::Chef::Recipe.any_instance.stub(:config_by_role)
    .with('rabbitmq-server', 'queue')
    .and_return(
      'host' => 'rabbit-host', 'port' => 'rabbit-port'
    )
  ::Chef::Recipe.any_instance.stub(:get_password).and_return('')
  ::Chef::Recipe.any_instance.stub(:secret)
    .with('secrets', 'openstack_identity_bootstrap_token')
    .and_return('bootstrap-token')
  ::Chef::Recipe.any_instance.stub(:get_password)
    .with("service", 'openstack-image')
    .and_return('glance-pass')
  ::Chef::Application.stub(:fatal!)
end

becomes a shared_context:

shared_context 'image-stubs' do
  before do
    ::Chef::Recipe.any_instance.stub(:address_for)
      .with('lo')
      .and_return('127.0.1.1')
    ::Chef::Recipe.any_instance.stub(:config_by_role)
      .with('rabbitmq-server', 'queue')
      .and_return(
        'host' => 'rabbit-host', 'port' => 'rabbit-port'
      )
    ::Chef::Recipe.any_instance.stub(:get_password).and_return('')
    ::Chef::Recipe.any_instance.stub(:secret)
      .with('secrets', 'openstack_identity_bootstrap_token')
      .and_return('bootstrap-token')
    ::Chef::Recipe.any_instance.stub(:get_password)
      .with("service", 'openstack-image')
      .and_return('glance-pass')
    ::Chef::Application.stub(:fatal!)
  end
end

And to use it, we simple do this in our spec files:

  include_context 'image-stubs'

rather than:

  describe 'openstack-image::api' do
    before { image_stubs }

These can be broken out into require-able files in spec/support if necessary.

The process is nearly identical for shared_examples: In spec_helper:

  def expect_creates_glance_dir # rubocop:disable MethodLength
    describe '/etc/glance' do
      before do
        @dir = @chef_run.directory '/etc/glance'
      end

      it 'has proper owner' do
        expect(@dir.owner).to eq('glance')
        expect(@dir.group).to eq('glance')
      end

      it 'has proper modes' do
        expect(sprintf('%o', @dir.mode)).to eq '700'
      end
    end
  end

becomes:

shared_examples 'glance-directory' do
  describe '/etc/glance' do
    let(:dir) { chef_run.directory('/etc/glance') }

    it 'has proper owner' do
      expect(dir.owner).to eq('glance')
      expect(dir.group).to eq('glance')
    end

    it 'has proper modes' do
      expect(sprintf('%o', dir.mode)).to eq '700'
    end
  end
end

Take note that in this example, we also have converted the before block to match the let() style and added () around the directory call.

Now in our specs, we simply do:

  include_examples 'glance-directory'

--------------------------------------------------------------
6 Remove unneeded :: in Class/Variable usages
--------------------------------------------------------------

:: is the ruby hint to tell it to start at the root of the namespace rather than use the current [nested] namespace when doing variable/class lookups.

Since the likelyhood of us having two variables/classes named REDHAT_OPTS and we don't have a nested class named Chef::Recipe, it's pretty safe to just remove all of these to readability sake.

::UBUNTU_OPTS = {} becomse UBUNTU_OPTS = {}

::Chef::Recipe.any_instance.stub becomes Chef::Recipe.any_instaance.stub

etc, etc