Update generated spec_helper. #1647

Merged
merged 2 commits into from Jul 24, 2014

Conversation

Projects
None yet
3 participants
@myronmarston
Owner

myronmarston commented Jul 23, 2014

  • Add new include_chain_clauses_in_custom_matcher_descriptions
    config option. It will default to true in RSpec 4 so we
    have not put it in the commented-out section.
  • Move verify_partial_doubles config option into non-commented
    out section as well. I want to default it to true in RSpec 4
    as well.
  • Switch from syntax config options to disable_monkey_patching!

Fixes #1645.

/cc @oveddan

myronmarston added some commits Jul 23, 2014

Update generated spec_helper.
- Add new `include_chain_clauses_in_custom_matcher_descriptions`
  config option. It will default to `true` in RSpec 4 so we
  have not put it in the commented-out section.
- Move `verify_partial_doubles` config option into non-commented
  out section as well. I want to default it to `true` in RSpec 4
  as well.
- Switch from `syntax` config options to `disable_monkey_patching!`

Fixes #1645.
@cupakromer

This comment has been minimized.

Show comment Hide comment
@cupakromer

cupakromer Jul 23, 2014

Member

Mostly 👍

Switch from syntax config options to disable_monkey_patching!

I'm fine with including disable_monkey_patching! in the helper and making it the uncommented default in RSpec 4. However, I'd still prefer to retain the syntax config settings commented out in the config.

Personally, I don't use disable_monkey_patching! yet. I'm not sure if I fully will in the future as my preferred default. While I appreciate it's intent, aesthetically, I'm not a fan of RSpec.describe on the outer level. I'd rather not have to hunt down the syntax for syntax or copy-🍝 it across projects.

Member

cupakromer commented Jul 23, 2014

Mostly 👍

Switch from syntax config options to disable_monkey_patching!

I'm fine with including disable_monkey_patching! in the helper and making it the uncommented default in RSpec 4. However, I'd still prefer to retain the syntax config settings commented out in the config.

Personally, I don't use disable_monkey_patching! yet. I'm not sure if I fully will in the future as my preferred default. While I appreciate it's intent, aesthetically, I'm not a fan of RSpec.describe on the outer level. I'd rather not have to hunt down the syntax for syntax or copy-🍝 it across projects.

@myronmarston

This comment has been minimized.

Show comment Hide comment
@myronmarston

myronmarston Jul 23, 2014

Owner

I'm fine with including disable_monkey_patching! in the helper and making it the uncommented default in RSpec 4. However, I'd still prefer to retain the syntax config settings commented out in the config.

It's tricky to retain the old syntax config given that we want those settings commented out while we want the verify_partial_doubles and include_chain_clauses_in_custom_matcher_descriptions configs not commented out...and we've chosen to wrap all the commented out stuff in =begin and =end for simplicity. I don't really want to have two expect_with and mock_with blocks for the commented out vs non-commented out sections...

Owner

myronmarston commented Jul 23, 2014

I'm fine with including disable_monkey_patching! in the helper and making it the uncommented default in RSpec 4. However, I'd still prefer to retain the syntax config settings commented out in the config.

It's tricky to retain the old syntax config given that we want those settings commented out while we want the verify_partial_doubles and include_chain_clauses_in_custom_matcher_descriptions configs not commented out...and we've chosen to wrap all the commented out stuff in =begin and =end for simplicity. I don't really want to have two expect_with and mock_with blocks for the commented out vs non-commented out sections...

@cupakromer

This comment has been minimized.

Show comment Hide comment
@cupakromer

cupakromer Jul 23, 2014

Member

Yeah that's a good point. How about making it commented out in the uncommented section, with a note about the preferred alternative. Maybe something like:

  # rspec-expectations config goes here. You can use an alternate
  # assertion/expectation library such as wrong or the stdlib/minitest
  # assertions if you prefer.
  config.expect_with :rspec do |expectations|
    # RSpec 4 plans on turning `disable_monkey_patching!` on by default.  This
    # limits the available syntax to the non-monkey patched syntax which is
    # recommended. See the next section for more details. If using this
    # setting, remove this comment block.
    #
    # Otherwise, uncomment the following line to enable only the newer,
    # non-monkey-patching expect syntax.  For more details, see: -
    # http://myronmars.to/n/dev-blog/2012/06/rspecs-new-expectation-syntax
    #expectations.syntax = :expect

    # This option will default to `true` in RSpec 4. It makes the `description`
    # and `failure_message` of custom matchers include text for helper methods
    # defined using `chain`, e.g.:
    # be_bigger_than(2).and_smaller_than(4).description
    #   # => "be bigger than 2 and smaller than 4"
    # ...rather than:
    #   # => "be bigger than 2"
    expectations.include_chain_clauses_in_custom_matcher_descriptions = true
  end

  # rspec-mocks config goes here. You can use an alternate test double
  # library (such as bogus or mocha) by changing the `mock_with` option here.
  config.mock_with :rspec do |mocks|
    # RSpec 4 plans on turning `disable_monkey_patching!` on by default.  This
    # limits the available syntax to the non-monkey patched syntax which is
    # recommended. See the next section for more details. If using this
    # setting, remove this comment block.
    #
    # Otherwise, uncomment the following line to enable only the newer,
    # non-monkey-patching mock syntax.  -
    # http://teaisaweso.me/blog/2013/05/27/rspecs-new-message-expectation-syntax/
    #mocks.syntax = :expect

    # Prevents you from mocking or stubbing a method that does not exist on
    # a real object. This is generally recommended, and will default to
    # `true` in RSpec 4.
    mocks.verify_partial_doubles = true
  end
Member

cupakromer commented Jul 23, 2014

Yeah that's a good point. How about making it commented out in the uncommented section, with a note about the preferred alternative. Maybe something like:

  # rspec-expectations config goes here. You can use an alternate
  # assertion/expectation library such as wrong or the stdlib/minitest
  # assertions if you prefer.
  config.expect_with :rspec do |expectations|
    # RSpec 4 plans on turning `disable_monkey_patching!` on by default.  This
    # limits the available syntax to the non-monkey patched syntax which is
    # recommended. See the next section for more details. If using this
    # setting, remove this comment block.
    #
    # Otherwise, uncomment the following line to enable only the newer,
    # non-monkey-patching expect syntax.  For more details, see: -
    # http://myronmars.to/n/dev-blog/2012/06/rspecs-new-expectation-syntax
    #expectations.syntax = :expect

    # This option will default to `true` in RSpec 4. It makes the `description`
    # and `failure_message` of custom matchers include text for helper methods
    # defined using `chain`, e.g.:
    # be_bigger_than(2).and_smaller_than(4).description
    #   # => "be bigger than 2 and smaller than 4"
    # ...rather than:
    #   # => "be bigger than 2"
    expectations.include_chain_clauses_in_custom_matcher_descriptions = true
  end

  # rspec-mocks config goes here. You can use an alternate test double
  # library (such as bogus or mocha) by changing the `mock_with` option here.
  config.mock_with :rspec do |mocks|
    # RSpec 4 plans on turning `disable_monkey_patching!` on by default.  This
    # limits the available syntax to the non-monkey patched syntax which is
    # recommended. See the next section for more details. If using this
    # setting, remove this comment block.
    #
    # Otherwise, uncomment the following line to enable only the newer,
    # non-monkey-patching mock syntax.  -
    # http://teaisaweso.me/blog/2013/05/27/rspecs-new-message-expectation-syntax/
    #mocks.syntax = :expect

    # Prevents you from mocking or stubbing a method that does not exist on
    # a real object. This is generally recommended, and will default to
    # `true` in RSpec 4.
    mocks.verify_partial_doubles = true
  end
@myronmarston

This comment has been minimized.

Show comment Hide comment
@myronmarston

myronmarston Jul 23, 2014

Owner

That would work, but it's soooo much more verbose, and given that we'd like to move in the direction of forcing users to opt-in to whatever monkey patching they want in RSpec 4, using disable_monkey_patching! feels more natural to me. And besides; it's still commented out, so users still have to make changes to the generated files to get that behavior.

I'm curious what others think, though?

Owner

myronmarston commented Jul 23, 2014

That would work, but it's soooo much more verbose, and given that we'd like to move in the direction of forcing users to opt-in to whatever monkey patching they want in RSpec 4, using disable_monkey_patching! feels more natural to me. And besides; it's still commented out, so users still have to make changes to the generated files to get that behavior.

I'm curious what others think, though?

@JonRowe

This comment has been minimized.

Show comment Hide comment
@JonRowe

JonRowe Jul 24, 2014

Owner

This LGTM, it's a bit mixed cause it's suggesting a change as well as adding the new setting but overall I agree with it.

Owner

JonRowe commented Jul 24, 2014

This LGTM, it's a bit mixed cause it's suggesting a change as well as adding the new setting but overall I agree with it.

@cupakromer

This comment has been minimized.

Show comment Hide comment
@cupakromer

cupakromer Jul 24, 2014

Member

Hmm maybe:

    # If using `disable_monkey_patching!` remove this comment block.
    # Otherwise, uncomment the following line to enable only the newer,
    # non-monkey-patching expect syntax.
    # For more details, see:
    # - http://myronmars.to/n/dev-blog/2012/06/rspecs-new-expectation-syntax
    #expectations.syntax = :expect
Member

cupakromer commented Jul 24, 2014

Hmm maybe:

    # If using `disable_monkey_patching!` remove this comment block.
    # Otherwise, uncomment the following line to enable only the newer,
    # non-monkey-patching expect syntax.
    # For more details, see:
    # - http://myronmars.to/n/dev-blog/2012/06/rspecs-new-expectation-syntax
    #expectations.syntax = :expect
@myronmarston

This comment has been minimized.

Show comment Hide comment
@myronmarston

myronmarston Jul 24, 2014

Owner

Hmm maybe:

I think it's confusing to have recommended settings using two forms of comments (# and =begin) and to recommend disabling monkey patching in two ways (via disable_monkey_patching! and via syntax =).

Given that the recommended settings are commented out, and the user has to take conscious action to use them, I think that presenting multiple ways to do the same thing is a bad idea. We're not limiting anyone's flexibility by presenting only one way to do something.

Owner

myronmarston commented Jul 24, 2014

Hmm maybe:

I think it's confusing to have recommended settings using two forms of comments (# and =begin) and to recommend disabling monkey patching in two ways (via disable_monkey_patching! and via syntax =).

Given that the recommended settings are commented out, and the user has to take conscious action to use them, I think that presenting multiple ways to do the same thing is a bad idea. We're not limiting anyone's flexibility by presenting only one way to do something.

@cupakromer

This comment has been minimized.

Show comment Hide comment
@cupakromer

cupakromer Jul 24, 2014

Member

Agreed. This is just something I've grown lazy using. Clearly all I need to remember is: block_arg.syntax = :expect. I think I can manage that. You've already added the links about these to the comment right about disable_monkey_patching! so that's 👍.

Given that this really is just my laziness expressing itself, and the solution is very simple, I'm alright with this moving forward.

Member

cupakromer commented Jul 24, 2014

Agreed. This is just something I've grown lazy using. Clearly all I need to remember is: block_arg.syntax = :expect. I think I can manage that. You've already added the links about these to the comment right about disable_monkey_patching! so that's 👍.

Given that this really is just my laziness expressing itself, and the solution is very simple, I'm alright with this moving forward.

myronmarston added a commit that referenced this pull request Jul 24, 2014

@myronmarston myronmarston merged commit e551ea0 into master Jul 24, 2014

@myronmarston myronmarston deleted the update-generated-config-for-rspec-3-1 branch Jul 24, 2014

@mikeastock mikeastock referenced this pull request in thoughtbot/clearance Jan 18, 2016

Merged

Don't rely on RSpec monkey patching for feature method #632

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment