Spec: Always include spec_helper

[olleolleolle]

Summary

This PR does one thing: uses the options file .rspec to always include spec_helper. Also: removes the require 'spec_helper' from each spec file which has it.

Details

I was asking myself: "Do we pay any special cost for having the spec helper in each file?" and "Can we have one require instead of 30?"

Motivation and Context

"Can we have one require instead of 30?"

How Has This Been Tested?

Ran the test suite locally.

Types of changes

• Refactor (code change that does not change external functionality)
• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)

[mxygem]

LGTM and 👍 for reducing redundancy! @olleolleolle What were the answers to the questions you asked yourself? :p

[mattwynne]

One reason to have explicit dependencies is performance. If a unit test doesn't need to load spec_helper.rb it will run faster in isolation if it isn't forced to.

It depends on what our spec does whether this will have significant impact or not.

Nevertheless, I personally find it a good practice to minimise dependencies wherever possible.

I've forgotten (and I'm on a phone so can't check) - what does our spec helper actually do?

[olleolleolle]

It requires a few files - cucumber among them. Decides on ANSI coloring. Adds a "pending" method to decide whether or not to run a test.

# frozen_string_literal: true
ENV['CUCUMBER_COLORS'] = nil
$:.unshift(File.dirname(__FILE__))

# For Travis....
require 'cucumber/encoding'

require 'simplecov_setup'

require 'pry'

require 'cucumber'

RSpec.configure do |c|
  c.before do
    ::Cucumber::Term::ANSIColor.coloring = true
  end
end

module RSpec
  module WorkInProgress
    def pending_under(platforms, reason, &block)
      if [platforms].flatten.map(&:to_s).include? RUBY_PLATFORM
        pending "pending under #{platforms.inspect} because: #{reason}", &block
      else
        yield
      end
    end
  end
end

I'm happy to not merge this change, if that's your take on it.

[Ben-Behar]

It sounds like a more fundamental question to ask is: Should the unit tests be runnable in more than one specific environment?

Moving dependencies to .rspec results in the ability to run the tests with another .rspec file, and thus a slightly different environment. While this can be a neat way to specify configuration options based on context, it might lead to a path of surprise to an inexperienced developer. Thoughts?

[mattwynne]

What would be some examples of those different environments @Ben-Behar?

[Ben-Behar]

Well, a trick that I have seen before is to have a .rspec just for development which includes helpful debugging tools, such as require 'pry' or possibly some debugging monkeypatch (def fail ; binding.pry ; end).
Then during the build/testing process, no .rspec file is given, to ensure that the tests are executed in the most minimal environment.

[olleolleolle]

I'll close this PR. Thanks for the interest in it.

We'll manage!

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.